hw/rtc/goldfish_rtc.c | 43 +++++++++++++------------------------------ 1 file changed, 13 insertions(+), 30 deletions(-)
Instead of migrating the raw tick_offset, goldfish_rtc migrates a
recalculated value based on QEMU_CLOCK_VIRTUAL. As QEMU_CLOCK_VIRTUAL
stands still across a save-and-restore cycle, the guest RTC becomes out
of sync with the host RTC when the VM is restored.
As described in the bug description, it looks like this calculation was
copied from pl031 RTC, which had its tick_offset migration fixed by
Commit 032cfe6a79c8 ("pl031: Correctly migrate state when using -rtc
clock=host").
Migrate the tick_offset directly, adding it as a version-dependent field
to VMState. Keep the old behavior when migrating from previous versions.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2033
Signed-off-by: Rodrigo Dias Correa <r@drigo.nl>
---
As a new field was added to VMState, after this patch, it won't be possible to
migrate to old versions. I'm not sure if this is needed. Please, let me know.
---
hw/rtc/goldfish_rtc.c | 43 +++++++++++++------------------------------
1 file changed, 13 insertions(+), 30 deletions(-)
diff --git a/hw/rtc/goldfish_rtc.c b/hw/rtc/goldfish_rtc.c
index fa1d9051f4..0f1b53e0e4 100644
--- a/hw/rtc/goldfish_rtc.c
+++ b/hw/rtc/goldfish_rtc.c
@@ -178,38 +178,21 @@ static void goldfish_rtc_write(void *opaque, hwaddr offset,
trace_goldfish_rtc_write(offset, value);
}
-static int goldfish_rtc_pre_save(void *opaque)
-{
- uint64_t delta;
- GoldfishRTCState *s = opaque;
-
- /*
- * We want to migrate this offset, which sounds straightforward.
- * Unfortunately, we cannot directly pass tick_offset because
- * rtc_clock on destination Host might not be same source Host.
- *
- * To tackle, this we pass tick_offset relative to vm_clock from
- * source Host and make it relative to rtc_clock at destination Host.
- */
- delta = qemu_clock_get_ns(rtc_clock) -
- qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
- s->tick_offset_vmstate = s->tick_offset + delta;
-
- return 0;
-}
-
static int goldfish_rtc_post_load(void *opaque, int version_id)
{
- uint64_t delta;
GoldfishRTCState *s = opaque;
- /*
- * We extract tick_offset from tick_offset_vmstate by doing
- * reverse math compared to pre_save() function.
- */
- delta = qemu_clock_get_ns(rtc_clock) -
- qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
- s->tick_offset = s->tick_offset_vmstate - delta;
+ if (version_id < 3) {
+ /*
+ * Previous versions didn't migrate tick_offset directly. Instead, they
+ * migrated tick_offset_vmstate, which is a recalculation based on
+ * QEMU_CLOCK_VIRTUAL. We use tick_offset_vmstate when migrating from
+ * older versions.
+ */
+ uint64_t delta = qemu_clock_get_ns(rtc_clock) -
+ qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+ s->tick_offset = s->tick_offset_vmstate - delta;
+ }
goldfish_rtc_set_alarm(s);
@@ -239,8 +222,7 @@ static const MemoryRegionOps goldfish_rtc_ops[2] = {
static const VMStateDescription goldfish_rtc_vmstate = {
.name = TYPE_GOLDFISH_RTC,
- .version_id = 2,
- .pre_save = goldfish_rtc_pre_save,
+ .version_id = 3,
.post_load = goldfish_rtc_post_load,
.fields = (const VMStateField[]) {
VMSTATE_UINT64(tick_offset_vmstate, GoldfishRTCState),
@@ -249,6 +231,7 @@ static const VMStateDescription goldfish_rtc_vmstate = {
VMSTATE_UINT32(irq_pending, GoldfishRTCState),
VMSTATE_UINT32(irq_enabled, GoldfishRTCState),
VMSTATE_UINT32(time_high, GoldfishRTCState),
+ VMSTATE_UINT64_V(tick_offset, GoldfishRTCState, 3),
VMSTATE_END_OF_LIST()
}
};
--
2.34.115.01.2025 00:21, Rodrigo Dias Correa wrote:
> Instead of migrating the raw tick_offset, goldfish_rtc migrates a
> recalculated value based on QEMU_CLOCK_VIRTUAL. As QEMU_CLOCK_VIRTUAL
> stands still across a save-and-restore cycle, the guest RTC becomes out
> of sync with the host RTC when the VM is restored.
>
> As described in the bug description, it looks like this calculation was
> copied from pl031 RTC, which had its tick_offset migration fixed by
> Commit 032cfe6a79c8 ("pl031: Correctly migrate state when using -rtc
> clock=host").
>
> Migrate the tick_offset directly, adding it as a version-dependent field
> to VMState. Keep the old behavior when migrating from previous versions.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2033
Is this a qemu-stable material?
If yes, how far to older stable series it is worth to pick?
(Currently active older stable series are 7.2 and 8.2).
Please add Cc: qemu-stable@ for fixes which are supposed to be picked up
for the stable series.
Thanks,
/mjt
06.03.2025 09:35, Michael Tokarev wrote: >> Migrate the tick_offset directly, adding it as a version-dependent field >> to VMState. Keep the old behavior when migrating from previous versions. >> >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2033 > > Is this a qemu-stable material? It is probably not, since this change adds a new field (and version) to the migration stream. Thanks, /mjt
On Thu, Mar 6, 2025 at 4:44 PM Michael Tokarev <mjt@tls.msk.ru> wrote: > > 06.03.2025 09:35, Michael Tokarev wrote: > > >> Migrate the tick_offset directly, adding it as a version-dependent field > >> to VMState. Keep the old behavior when migrating from previous versions. > >> > >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2033 > > > > Is this a qemu-stable material? > > It is probably not, since this change adds a new field (and version) to the > migration stream. That's fine Alistair > > Thanks, > > /mjt >
On Wed, Jan 15, 2025 at 7:23 AM Rodrigo Dias Correa <r@drigo.nl> wrote:
>
> Instead of migrating the raw tick_offset, goldfish_rtc migrates a
> recalculated value based on QEMU_CLOCK_VIRTUAL. As QEMU_CLOCK_VIRTUAL
> stands still across a save-and-restore cycle, the guest RTC becomes out
> of sync with the host RTC when the VM is restored.
>
> As described in the bug description, it looks like this calculation was
> copied from pl031 RTC, which had its tick_offset migration fixed by
> Commit 032cfe6a79c8 ("pl031: Correctly migrate state when using -rtc
> clock=host").
>
> Migrate the tick_offset directly, adding it as a version-dependent field
> to VMState. Keep the old behavior when migrating from previous versions.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2033
> Signed-off-by: Rodrigo Dias Correa <r@drigo.nl>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> As a new field was added to VMState, after this patch, it won't be possible to
> migrate to old versions. I'm not sure if this is needed. Please, let me know.
> ---
> hw/rtc/goldfish_rtc.c | 43 +++++++++++++------------------------------
> 1 file changed, 13 insertions(+), 30 deletions(-)
>
> diff --git a/hw/rtc/goldfish_rtc.c b/hw/rtc/goldfish_rtc.c
> index fa1d9051f4..0f1b53e0e4 100644
> --- a/hw/rtc/goldfish_rtc.c
> +++ b/hw/rtc/goldfish_rtc.c
> @@ -178,38 +178,21 @@ static void goldfish_rtc_write(void *opaque, hwaddr offset,
> trace_goldfish_rtc_write(offset, value);
> }
>
> -static int goldfish_rtc_pre_save(void *opaque)
> -{
> - uint64_t delta;
> - GoldfishRTCState *s = opaque;
> -
> - /*
> - * We want to migrate this offset, which sounds straightforward.
> - * Unfortunately, we cannot directly pass tick_offset because
> - * rtc_clock on destination Host might not be same source Host.
> - *
> - * To tackle, this we pass tick_offset relative to vm_clock from
> - * source Host and make it relative to rtc_clock at destination Host.
> - */
> - delta = qemu_clock_get_ns(rtc_clock) -
> - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> - s->tick_offset_vmstate = s->tick_offset + delta;
> -
> - return 0;
> -}
> -
> static int goldfish_rtc_post_load(void *opaque, int version_id)
> {
> - uint64_t delta;
> GoldfishRTCState *s = opaque;
>
> - /*
> - * We extract tick_offset from tick_offset_vmstate by doing
> - * reverse math compared to pre_save() function.
> - */
> - delta = qemu_clock_get_ns(rtc_clock) -
> - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> - s->tick_offset = s->tick_offset_vmstate - delta;
> + if (version_id < 3) {
> + /*
> + * Previous versions didn't migrate tick_offset directly. Instead, they
> + * migrated tick_offset_vmstate, which is a recalculation based on
> + * QEMU_CLOCK_VIRTUAL. We use tick_offset_vmstate when migrating from
> + * older versions.
> + */
> + uint64_t delta = qemu_clock_get_ns(rtc_clock) -
> + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> + s->tick_offset = s->tick_offset_vmstate - delta;
> + }
>
> goldfish_rtc_set_alarm(s);
>
> @@ -239,8 +222,7 @@ static const MemoryRegionOps goldfish_rtc_ops[2] = {
>
> static const VMStateDescription goldfish_rtc_vmstate = {
> .name = TYPE_GOLDFISH_RTC,
> - .version_id = 2,
> - .pre_save = goldfish_rtc_pre_save,
> + .version_id = 3,
> .post_load = goldfish_rtc_post_load,
> .fields = (const VMStateField[]) {
> VMSTATE_UINT64(tick_offset_vmstate, GoldfishRTCState),
> @@ -249,6 +231,7 @@ static const VMStateDescription goldfish_rtc_vmstate = {
> VMSTATE_UINT32(irq_pending, GoldfishRTCState),
> VMSTATE_UINT32(irq_enabled, GoldfishRTCState),
> VMSTATE_UINT32(time_high, GoldfishRTCState),
> + VMSTATE_UINT64_V(tick_offset, GoldfishRTCState, 3),
> VMSTATE_END_OF_LIST()
> }
> };
> --
> 2.34.1
>
>
On Wed, Jan 15, 2025 at 7:23 AM Rodrigo Dias Correa <r@drigo.nl> wrote:
>
> Instead of migrating the raw tick_offset, goldfish_rtc migrates a
> recalculated value based on QEMU_CLOCK_VIRTUAL. As QEMU_CLOCK_VIRTUAL
> stands still across a save-and-restore cycle, the guest RTC becomes out
> of sync with the host RTC when the VM is restored.
>
> As described in the bug description, it looks like this calculation was
> copied from pl031 RTC, which had its tick_offset migration fixed by
> Commit 032cfe6a79c8 ("pl031: Correctly migrate state when using -rtc
> clock=host").
>
> Migrate the tick_offset directly, adding it as a version-dependent field
> to VMState. Keep the old behavior when migrating from previous versions.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2033
> Signed-off-by: Rodrigo Dias Correa <r@drigo.nl>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> As a new field was added to VMState, after this patch, it won't be possible to
> migrate to old versions. I'm not sure if this is needed. Please, let me know.
> ---
> hw/rtc/goldfish_rtc.c | 43 +++++++++++++------------------------------
> 1 file changed, 13 insertions(+), 30 deletions(-)
>
> diff --git a/hw/rtc/goldfish_rtc.c b/hw/rtc/goldfish_rtc.c
> index fa1d9051f4..0f1b53e0e4 100644
> --- a/hw/rtc/goldfish_rtc.c
> +++ b/hw/rtc/goldfish_rtc.c
> @@ -178,38 +178,21 @@ static void goldfish_rtc_write(void *opaque, hwaddr offset,
> trace_goldfish_rtc_write(offset, value);
> }
>
> -static int goldfish_rtc_pre_save(void *opaque)
> -{
> - uint64_t delta;
> - GoldfishRTCState *s = opaque;
> -
> - /*
> - * We want to migrate this offset, which sounds straightforward.
> - * Unfortunately, we cannot directly pass tick_offset because
> - * rtc_clock on destination Host might not be same source Host.
> - *
> - * To tackle, this we pass tick_offset relative to vm_clock from
> - * source Host and make it relative to rtc_clock at destination Host.
> - */
> - delta = qemu_clock_get_ns(rtc_clock) -
> - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> - s->tick_offset_vmstate = s->tick_offset + delta;
> -
> - return 0;
> -}
> -
> static int goldfish_rtc_post_load(void *opaque, int version_id)
> {
> - uint64_t delta;
> GoldfishRTCState *s = opaque;
>
> - /*
> - * We extract tick_offset from tick_offset_vmstate by doing
> - * reverse math compared to pre_save() function.
> - */
> - delta = qemu_clock_get_ns(rtc_clock) -
> - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> - s->tick_offset = s->tick_offset_vmstate - delta;
> + if (version_id < 3) {
> + /*
> + * Previous versions didn't migrate tick_offset directly. Instead, they
> + * migrated tick_offset_vmstate, which is a recalculation based on
> + * QEMU_CLOCK_VIRTUAL. We use tick_offset_vmstate when migrating from
> + * older versions.
> + */
> + uint64_t delta = qemu_clock_get_ns(rtc_clock) -
> + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> + s->tick_offset = s->tick_offset_vmstate - delta;
> + }
>
> goldfish_rtc_set_alarm(s);
>
> @@ -239,8 +222,7 @@ static const MemoryRegionOps goldfish_rtc_ops[2] = {
>
> static const VMStateDescription goldfish_rtc_vmstate = {
> .name = TYPE_GOLDFISH_RTC,
> - .version_id = 2,
> - .pre_save = goldfish_rtc_pre_save,
> + .version_id = 3,
> .post_load = goldfish_rtc_post_load,
> .fields = (const VMStateField[]) {
> VMSTATE_UINT64(tick_offset_vmstate, GoldfishRTCState),
> @@ -249,6 +231,7 @@ static const VMStateDescription goldfish_rtc_vmstate = {
> VMSTATE_UINT32(irq_pending, GoldfishRTCState),
> VMSTATE_UINT32(irq_enabled, GoldfishRTCState),
> VMSTATE_UINT32(time_high, GoldfishRTCState),
> + VMSTATE_UINT64_V(tick_offset, GoldfishRTCState, 3),
> VMSTATE_END_OF_LIST()
> }
> };
> --
> 2.34.1
>
>
© 2016 - 2025 Red Hat, Inc.