Re: [Qemu-devel] [PATCH 2/5] mc146818rtc: fix clock lost after scaling coalesced irq

[Paolo Bonzini]

On 12/04/2017 11:51, address@hidden wrote:
> +            int current_irq_coalesced = s->irq_coalesced;
> +
> +            s->irq_coalesced = (current_irq_coalesced * s->period) / period;
> +
> +            /*
> +             * calculate the lost clock after it is scaled which should be
> +             * compensated in the next interrupt.
> +             */
> +            lost_clock += current_irq_coalesced * s->period -
> +                            s->irq_coalesced * period;

This is:

   lost_clock = current_irq_coalesced * s->period -
        (current_irq_coalesced * s->period) / period * period;

i.e.

   /* When switching from a shorter to a longer period, scale down the
    * missing ticks since we expect the OS handler to treat the delayed
    * ticks as longer.  Any leftovers are put back into next_irq_clock.
    *
    * When switching to a shorter period, scale up the missing ticks
    * since we expect the OS handler to treat the delayed ticks as
    * shorter.
    */
   lost_clock = (s->irq_coalesced * s->period) % period;
   s->irq_coalesced = (s->irq_coalesced * s->period) / period;

Is this correct?

Paolo

> +            DPRINTF_C("cmos: coalesced irqs scaled from %d to %d, %ld clocks 
> "
> +                      "are compensated.\n",
> +                      current_irq_coalesced, s->irq_coalesced, lost_clock);
>          }
>          s->period = period;
>  #endif
> @@ -170,7 +193,7 @@ static void periodic_timer_update(RTCState *s, int64_t 
> current_time)
>          cur_clock =
>              muldiv64(current_time, RTC_CLOCK_RATE, NANOSECONDS_PER_SECOND);
>  
> -        next_irq_clock = (cur_clock & ~(period - 1)) + period;
> +        next_irq_clock = cur_clock + period - lost_clock;
>          s->next_periodic_time = muldiv64(next_irq_clock, 
> NANOSECONDS_PER_SECOND,
>                                           RTC_CLOCK_RATE) + 1;
>          timer_mod(s->periodic_timer, s->next_periodic_time);
> --