Re: [Qemu-devel] [PATCH 2/5] mc146818rtc: fix clock lost after scaling coalesced irq
On 05/03/2017 11:15 PM, Paolo Bonzini wrote: On 12/04/2017 11:51, guangrong.x...@gmail.com 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? Yes, it is correct, it looks smarter, will apply it in the next version. Thanks!
Re: [Qemu-devel] [PATCH 2/5] mc146818rtc: fix clock lost after scaling coalesced irq
On 12/04/2017 11:51, guangrong.x...@gmail.com 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); > --
[Qemu-devel] [PATCH 2/5] mc146818rtc: fix clock lost after scaling coalesced irq
From: Xiao GuangrongIf the period is changed by re-configuring RegA, the coalesced irq will be scaled to reflect the new period, however, it calculates the new interrupt number like this: s->irq_coalesced = (s->irq_coalesced * s->period) / period; There are some clocks will be lost if they are not enough to be squeezed to a single new period that will cause the VM clock slower In order to fix the issue, we calculate the interrupt window based on the precise clock rather than period, then the clocks lost during period is scaled can be compensated properly Signed-off-by: Xiao Guangrong --- hw/timer/mc146818rtc.c | 39 +++ 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c index 749e206..649678c 100644 --- a/hw/timer/mc146818rtc.c +++ b/hw/timer/mc146818rtc.c @@ -146,23 +146,46 @@ static void rtc_coalesced_timer(void *opaque) } #endif +static int period_code_to_clock(int period_code) +{ +/* periodic timer is disabled. */ +if (!period_code) { +return 0; +} + +if (period_code <= 2) { +period_code += 7; +} + +/* period in 32 Khz cycles */ +return 1 << (period_code - 1); +} + /* handle periodic timer */ static void periodic_timer_update(RTCState *s, int64_t current_time) { int period_code, period; -int64_t cur_clock, next_irq_clock; +int64_t cur_clock, next_irq_clock, lost_clock = 0; period_code = s->cmos_data[RTC_REG_A] & 0x0f; if (period_code != 0 && (s->cmos_data[RTC_REG_B] & REG_B_PIE)) { -if (period_code <= 2) -period_code += 7; -/* period in 32 Khz cycles */ -period = 1 << (period_code - 1); +period = period_code_to_clock(period_code); #ifdef TARGET_I386 if (period != s->period) { -s->irq_coalesced = (s->irq_coalesced * s->period) / period; -DPRINTF_C("cmos: coalesced irqs scaled to %d\n", s->irq_coalesced); +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; +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); -- 2.9.3