Re: [Qemu-devel] [PATCH v3 5/5] hpet 'driftfix': add code in hpet_timer() to compensate delayed callbacks and coalesced interrupts

[Marcelo Tosatti]

On Thu, May 05, 2011 at 04:07:19AM -0400, Ulrich Obergfell wrote:
> 
> Hi Marcelo,
>  
> > Other than that, shouldnt reset accounting variables to init state on
> > write to GLOBAL_ENABLE_CFG / writes to main counter?
> 
> I'd suggest to initialize/reset the driftfix-related fields in the
> 'HPETTimer' structure (including the backlog of unaccounted ticks)
> in the following situations.
> 
> 
> - When the guest o/s sets the 'CFG_ENABLE' bit (overall enable) in
>   the General Configuration Register.
> 
>   This is the place in hpet_ram_writel():
> 
>         case HPET_CFG:
>             ...
>             if (activating_bit(old_val, new_val, HPET_CFG_ENABLE)) {
>                 ...
>                 for (i = 0; i < s->num_timers; i++) {
>                     if ((&s->timer[i])->cmp != ~0ULL) {
>                         // initialize / reset fields here
>                         hpet_set_timer(&s->timer[i]);
> 
> 
> - When the guest o/s sets the 'TN_ENABLE' bit (timer N interrupt
>   enable) in the Timer N Configuration and Capabilities Register.
> 
>   This is the place in hpet_ram_writel():
> 
>         case HPET_TN_CFG:
>             ...
>             if (activating_bit(old_val, new_val, HPET_TN_ENABLE)) {
>                 // initialize / reset fields here
>                 hpet_set_timer(timer);
> 
> 
> This should cover cases such as ...
> 
> - first time initialization of HPET & timers during guest o/s boot.
>   (includes kexec and reboot)
> 
> - if a guest o/s stops and restarts the timer.
>   (for example, to change the comparator register value or
>    to switch a timer between periodic mode and non-periodic mode)
> 
> 
> Regards,
> 
> Uli

Uli, looks good.