qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2]booke: timer: Deactivate timer for target_bit


From: Bhushan Bharat-R65777
Subject: Re: [Qemu-devel] [PATCH v2]booke: timer: Deactivate timer for target_bit above 61
Date: Tue, 11 Jun 2013 12:47:44 +0000


> -----Original Message-----
> From: Alexander Graf [mailto:address@hidden
> Sent: Tuesday, June 11, 2013 6:10 PM
> To: Bhushan Bharat-R65777
> Cc: Wood Scott-B07421; Andreas Färber; address@hidden; qemu-
> address@hidden
> Subject: Re: [Qemu-devel] [PATCH v2]booke: timer: Deactivate timer for
> target_bit above 61
> 
> On 06/11/2013 01:40 PM, Bhushan Bharat-R65777 wrote:
> >
> >> -----Original Message-----
> >> From: Alexander Graf [mailto:address@hidden
> >> Sent: Monday, June 10, 2013 11:40 PM
> >> To: Wood Scott-B07421
> >> Cc: Bhushan Bharat-R65777; Andreas Färber; address@hidden; qemu-
> >> address@hidden; Wood Scott-B07421
> >> Subject: Re: [Qemu-devel] [PATCH v2]booke: timer: Deactivate timer
> >> for target_bit above 61
> >>
> >>
> >> On 10.06.2013, at 19:20, Scott Wood wrote:
> >>
> >>> On 06/10/2013 09:26:18 AM, Alexander Graf wrote:
> >>>> On 06/10/2013 02:47 PM, Bhushan Bharat-R65777 wrote:
> >>>>>> -----Original Message-----
> >>>>>> From: Andreas Färber [mailto:address@hidden
> >>>>>> Sent: Monday, June 10, 2013 5:43 PM
> >>>>>> To: Bhushan Bharat-R65777
> >>>>>> Cc: address@hidden; address@hidden; address@hidden;
> >>>>>> Wood
> >>>>>> Scott- B07421; Bhushan Bharat-R65777
> >>>>>> Subject: Re: [Qemu-devel] [PATCH v2]booke: timer: Deactivate
> >>>>>> timer for target_bit above 61 So IIUC we can only allow 63 bits due to
> >>>>>> signedness, thus a maximum of (1<<   62), thus target_bit<= 61.
> >>>>>> Any chance at least the comment can be worded to explain that any
> >>>>>> better? Maybe also use (target-bit + 1>= 63) or period>   INT64_MAX as
> >> condition?
> >>>>> How about this:
> >>>>>      /* QEMU timer supports a maximum timer of INT64_MAX
> >> (0x7fffffff_ffffffff).
> >>>>>       * Run booke fit/wdog timer when
> >>>>>       * ((1ULL<<   target_bit + 1)<   0x40000000_00000000), i.e 
> >>>>> target_bit
> =
> >> 61.
> >>>>>       * Also the time with this maximum target_bit (with current range 
> >>>>> of
> >>>>>       * CPU frequency PowerPC supports) will be many many years. So it 
> >>>>> is
> >>>>>       * pretty safe to stop the timer above this threshold. */
> >>>> How about
> >>>>   /* This timeout will take years to trigger. Treat the timer as
> >>>> disabled. */
> >>> There should be at least a brief mention that it's because the QEMU
> >>> timer can't handle larger values,
> >> If it can't handle higher values, maybe it's better to just set the
> >> timer value to INT64_MAX when we detect an overflow? That would make
> >> the code plainly obvious.
> >>
> > What about below comment (a mix of both :)):
> >
> >      /* Timeout calculated with (target_bit + 1)>  62 will take
> >       * hundreds of years to trigger. Treat the timer as disabled.
> >       * Also this timeout is within the qemu supported maximum
> >       * timeout limit (INT64_MAX.). */
> 
> Ok, next question: Why does return disable the timer?

Actually here disabled means _not_ starting the timer. This function will be 
called to start timer initially and then later it is called to restart after 
every expiry. If we do not start then it is as good as stopped/disabled (it is 
not disabled in TCR). Probably saying "do not start qemu timer" or something 
similar is better than saying disabling the timer.

Thanks
-Bharat

> 
> 
> Alex
> 





reply via email to

[Prev in Thread] Current Thread [Next in Thread]