[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [ PATCH ] hpet: recover timer offset correctly
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [ PATCH ] hpet: recover timer offset correctly |
Date: |
Wed, 20 Dec 2017 09:40:37 +0000 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
* Pavel Dovgalyuk (address@hidden) wrote:
> > From: Dr. David Alan Gilbert [mailto:address@hidden
> > * Pavel Dovgalyuk (address@hidden) wrote:
> > > HPET saves its state by calculating the current time and recovers timer
> > > offset using this calculated value. But these calculations include
> > > divisions and multiplications. Therefore the timer state cannot be
> > > recovered
> > > precise enough.
> > > This patch introduces saving of the original value of the offset to
> > > preserve the determinism of the timer.
> >
> > Please put the extra data in a subsection and tie the subsection to a
> > property enabled by the machine type. That avoids breaking backwards
> > migration compatibility.
>
> I thought about it and haven't found how to preserve the backward migration
> compatibility
> without breaking everything else.
> Loading old (or compatible) snapshot without changing the version will
> corrupt the offset in
> post_load.
There are lots of ways; for example you could initialise hpet_offset to
a dummy value in a pre-load and spot that in the post-load instead of
the version_id, or set a flag in a post-load on the subsection.
Dave
> Pavel Dovgalyuk
> >
> > > Signed-off-by: Maria Klimushenkova <address@hidden>
> > > Signed-off-by: Pavel Dovgalyuk <address@hidden>
> > >
> > > ---
> > > hw/timer/hpet.c | 12 +++++++++---
> > > 1 file changed, 9 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
> > > index 577371b..4cf6a5b 100644
> > > --- a/hw/timer/hpet.c
> > > +++ b/hw/timer/hpet.c
> > > @@ -221,7 +221,9 @@ static int hpet_pre_save(void *opaque)
> > > HPETState *s = opaque;
> > >
> > > /* save current counter value */
> > > - s->hpet_counter = hpet_get_ticks(s);
> > > + if (hpet_enabled(s)) {
> > > + s->hpet_counter = hpet_get_ticks(s);
> > > + }
> > >
> > > return 0;
> > > }
> > > @@ -252,7 +254,10 @@ static int hpet_post_load(void *opaque, int
> > > version_id)
> > > HPETState *s = opaque;
> > >
> > > /* Recalculate the offset between the main counter and guest time */
> > > - s->hpet_offset = ticks_to_ns(s->hpet_counter) -
> > > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> > > + if (version_id <= 2) {
> > > + s->hpet_offset = ticks_to_ns(s->hpet_counter)
> > > + - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> > > + }
> > >
> > > /* Push number of timers into capability returned via HPET_ID */
> > > s->capability &= ~HPET_ID_NUM_TIM_MASK;
> > > @@ -303,7 +308,7 @@ static const VMStateDescription vmstate_hpet_timer = {
> > >
> > > static const VMStateDescription vmstate_hpet = {
> > > .name = "hpet",
> > > - .version_id = 2,
> > > + .version_id = 3,
> > > .minimum_version_id = 1,
> > > .pre_save = hpet_pre_save,
> > > .pre_load = hpet_pre_load,
> > > @@ -312,6 +317,7 @@ static const VMStateDescription vmstate_hpet = {
> > > VMSTATE_UINT64(config, HPETState),
> > > VMSTATE_UINT64(isr, HPETState),
> > > VMSTATE_UINT64(hpet_counter, HPETState),
> > > + VMSTATE_UINT64_V(hpet_offset, HPETState, 3),
> > > VMSTATE_UINT8_V(num_timers, HPETState, 2),
> > > VMSTATE_VALIDATE("num_timers in range",
> > > hpet_validate_num_timers),
> > > VMSTATE_STRUCT_VARRAY_UINT8(timer, HPETState, num_timers, 0,
> > >
> > --
> > Dr. David Alan Gilbert / address@hidden / Manchester, UK
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK