Re: [PATCH RFC v2 1/3] x86/hvm/rtc: Don't run the vpt timer when !REG_B.PIE.

[Tim Deegan <tim@xen.org>]

At 19:51 +0000 on 14 Feb (1392403919), Andrew Cooper wrote:
> From: Tim Deegan <tim@xen.org>
> 
> If the guest has not asked for interrupts, don't run the vpt timer
> to generate them.  This is a prerequisite for a patch to simplify how
> the vpt interacts with the RTC, and also gets rid of a timer series in
> Xen in a case where it's unlikely to be needed.
> 
> Instead, calculate the correct value for REG_C.PF whenever REG_C is
> read or PIE is enabled.  This allow a guest to poll for the PF bit
> while not asking for actual timer interrupts.  Such a guest would no
> longer get the benefit of the vpt's timer modes.
> 
> Signed-off-by: Tim Deegan <tim@xen.org>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> ---
> 
> Changes in v2:
>  * Reduce scope of `now` in rtc_timer_update()
>  * Merge PIE logic in REG_B write
>  * Tightly couple setting s->period with creating/destroying timers, so the
>    timer change gets properly recreated when the guest sets REG_B.PIE

Thanks for sorting out that bug, but I think in this version the !PIE
case won't work.  check_for_pf_ticks() uses s->period to figure out
whether to set PF, so it needs to be set whenever the REG_A selector
is configured, even if the timer's not running.

It looks like always setting s->period == 0 just before the call to
rtc_timer_update in the REG_B write (i.e. not just in the !PIE case)
would DTRT, but is that what you tried earlier?

Er, that is, here:

>          rtc_update_irq(s);
> -        if ( (data & RTC_PIE) && !(orig & RTC_PIE) )
> +        if ( (data ^ orig) & RTC_PIE )
> +        {
> +            if ( !(data & RTC_PIE) )
> +            {
> +                destroy_periodic_time(&s->pt);
> +                s->period = 0;
> +            }
>              rtc_timer_update(s);
> +        }
>          if ( (data ^ orig) & RTC_SET )

do this:

> +        if ( (data ^ orig) & RTC_PIE )
> +        {
> +            destroy_periodic_time(&s->pt);
> +            s->period = 0;
>              rtc_timer_update(s);
> +        }

Tim.