Re: [Qemu-ppc] [PATCH 3/4] Watchdog exit handling support

[Bhushan Bharat-R65777]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Friday, July 06, 2012 6:30 AM
> To: Bhushan Bharat-R65777
> Cc: Wood Scott-B07421; address@hidden; address@hidden;
> address@hidden
> Subject: Re: [PATCH 3/4] Watchdog exit handling support
> 
> On 07/05/2012 07:43 PM, Bhushan Bharat-R65777 wrote:
> >
> >
> >> -----Original Message-----
> >> From: Wood Scott-B07421
> >> Sent: Friday, July 06, 2012 1:57 AM
> >> To: Bhushan Bharat-R65777
> >> Cc: Wood Scott-B07421; address@hidden; address@hidden;
> >> address@hidden
> >> Subject: Re: [PATCH 3/4] Watchdog exit handling support
> >>
> >> On 07/04/2012 06:13 AM, Bhushan Bharat-R65777 wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Wood Scott-B07421
> >>>> Sent: Friday, June 29, 2012 3:57 AM
> >>>> To: Bhushan Bharat-R65777
> >>>> Cc: address@hidden; address@hidden; address@hidden;
> >>>> Bhushan Bharat-
> >>>> R65777
> >>>> Subject: Re: [PATCH 3/4] Watchdog exit handling support
> >>>>
> >>>> On 06/28/2012 12:39 AM, Bharat Bhushan wrote:
> >>>>> This patch adds the support to handle the exit caused by watchdog
> >>>>> (KVM_EXIT_WDT). In the handling we clear the TSR register.
> >>>>
> >>>> I'm not sure what the logical split is between this patch and 4/4...
> >>>
> >>> Ok I will merge 3/4 and 4/4.
> >>>
> >>>>
> >>>>> diff --git a/hw/ppc_booke.c b/hw/ppc_booke.c index
> >>>>> 837a5b6..a9fba15
> >>>>> 100644
> >>>>> --- a/hw/ppc_booke.c
> >>>>> +++ b/hw/ppc_booke.c
> >>>>> @@ -203,6 +203,11 @@ static void booke_wdt_cb(void *opaque)
> >>>>>                               booke_timer->wdt_timer);  }
> >>>>>
> >>>>> +void ppc_booke_wdt_clear_tsr(CPUPPCState *env, target_ulong tsr) {
> >>>>> +    env->spr[SPR_BOOKE_TSR] = tsr & ~(TSR_ENW | TSR_WIS |
> >>>>> +TSR_WRS_MASK); }
> >>>>> +
> >>>>
> >>>> We should probably call this function before returning to KVM, at
> >>>> least after we halt for debug, possibly other times.
> >>>
> >>> Why clearing watchdog related bits in TSR? This will just reset the
> >>> watchdog state machine. Do we actually want to reset the state
> >>> machine or want watchdog interrupt to not occur when exiting to KVM for
> debug.
> >>
> >> We want to prevent long delays in QEMU (especially for debug halt)
> >> from causing watchdog actions (interrupt, reset, etc).  Resetting the
> >> state machine seems like the way to do that.
> >>
> >>> Does possibly other times mean except KVM_RUN?
> >>
> >> Not sure what you mean here.  Don't we always use KVM_RUN to enter the 
> >> guest?
> >> My point was we definitely want this after we had the guest halted,
> >> and we may want to consider doing it for some other heavyweight exits.
> >
> > Your initial comment was "We should probably call this function before
> > returning to KVM". Which suggest that ioctls calls made by QEMU to
> > enter KVM. I am a bit confused, now you want to avoid long delays in
> > QEMU? This suggest whenever we exit to qemu, right?
> 
> Whenever we exit to QEMU is one possibility, though it's probably too 
> aggressive
> (if the guest gets stuck in a loop that contains a QEMU exit, the watchdog 
> won't
> catch it).  Another option is to reset the state machine only when the guest
> resumes from a debug halt.

Ok.
Resetting the state machine does not disable watchdog, it will just ensure the 
longest time for debug halt etc. Is that ok ?

Thanks
-Bharat

> 
> -Scott