On 04/07/19 18:02, Peter Maydell wrote: > I've had a report that the way the PL031 model handles time > across a vm save/reload fails to correctly advance the guest > RTC when the host RTC has advanced between the save and reload. > I looked at the code and my correspondent's analysis (which > I quote below, lightly edited) looks correct to me, but I'm not > entirely sure how our RTC stuff is supposed to work. Paolo, > you wrote this (way back in commit b0f26631bc5179006) -- any opinions? > > In the pl031 RTC device. the current time is given by: > > int64_t now = qemu_clock_get_ns(rtc_clock); > return s->tick_offset + now / NANOSECONDS_PER_SECOND; > > On save we do: > > /* tick_offset is base_time - rtc_clock base time. Instead, we want to > * store the base time relative to the QEMU_CLOCK_VIRTUAL for > backwards-compatibility. */ > int64_t delta = qemu_clock_get_ns(rtc_clock) - > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > s->tick_offset_vmstate = s->tick_offset + delta / NANOSECONDS_PER_SECOND; > > On restore: > > int64_t delta = qemu_clock_get_ns(rtc_clock) - > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > s->tick_offset = s->tick_offset_vmstate - delta / NANOSECONDS_PER_SECOND; > > So, no matter what is requested, if "qemu_clock_get_ns(rtc_clock)" > increases (eg, because host time increased), then tick_offset > reduces, which makes time follow QEMU_CLOCK_VIRTUAL no matter what > was requested on qemu's command line. > > (That is, because we migrate "offset relative to CLOCK_VIRTUAL" > and CLOCK_VIRTUAL does not advance when the VM is stopped, > we don't get the right behaviour of "offset is relative to > the new CLOCK_RTC, which might have advanced".).
You're right, the compatibility causes wrong behavior for the default -rtc settings (the RC pauses across migration). The right thing to do would be to store the base rather than the offset: that is, you store the time at which LR was written. Then the offset is s->lr - s->base and it's independent of the machine on which the rtc_clock is being read. By the way, the data sheet says "the counter and match values are compared in a comparator. When both values are equal, the RTCINTR interrupt is asserted HIGH"; QEMU compares the RTC value (read from RTC_DR) and not the counter value, but Linux code seems to expect QEMU's behavior. Paolo