> On Jun 21, 2021, at 7:46 AM, Paolo Bonzini <pbonz...@redhat.com> wrote:

> I agree that there's obviously a bug in QEMU.  However, I'm worried of two 
> things with this patch.
> 
> First, the RTC device model has a complicated mechanism to deliver missed 
> ticks of the periodic timer.  This is used with old versions of Windows which 
> lose track of time if periodic timer interrupts are not delivered.  This 
> mechanism (implemented by rtc_coalesced_timer) right now always triggers an 
> interrupt.  You need to change periodic_timer_update to not activate this 
> mechanism if PIE=0, by checking for PIE=1 at the
> 
>    if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW)
> 
> conditional.

Makes sense.

> Second, the firmware could set a nonzero period, and this would cause 
> continuous interruptions of the guest after the firmware stops, due to 
> s->periodic_timer firing.  This is "optimized" by the bug that you are 
> fixing.  To keep the optimization you could:
> 
> - do the timer_mod in periodic_timer_update only if !PF || (PIE && 
> lost_tick_policy==SLEW)
> 
> - in cmos_ioport_read, if !timer_pending(s->periodic_timer) call
> 
>    periodic_timer_update(s, qemu_clock_get_ns(rtc_clock),
>                          s->period, true);
> 
> to update s->next_periodic_time for the next tick and ensure PF will be set.

I might be missing some subtlety here, but by my reading of 
periodic_timer_update(), either one of those changes would result in a delay of 
the next latching of PF by however many ns the CPU was late in reading PF since 
the last time it was latched  Please correct me if I’m wrong about this!

There exists software out there in the wild that depends on PF latching at 
regular intervals regardless if when the CPU reads, it, i.e.:

PF          PF          PF          PF          PF          PF
    C            C                  C      C                  C

-- thorpej


Reply via email to