* [PATCH] goldfish_rtc: Fix tick_offset migration
@ 2025-01-14 21:21 Rodrigo Dias Correa
2025-01-31 0:54 ` Alistair Francis
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Rodrigo Dias Correa @ 2025-01-14 21:21 UTC (permalink / raw)
To: anup.patel, Alistair.Francis; +Cc: qemu-riscv, qemu-devel
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.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] goldfish_rtc: Fix tick_offset migration
2025-01-14 21:21 [PATCH] goldfish_rtc: Fix tick_offset migration Rodrigo Dias Correa
@ 2025-01-31 0:54 ` Alistair Francis
2025-01-31 1:32 ` Alistair Francis
2025-03-06 6:35 ` Michael Tokarev
2 siblings, 0 replies; 6+ messages in thread
From: Alistair Francis @ 2025-01-31 0:54 UTC (permalink / raw)
To: Rodrigo Dias Correa; +Cc: anup.patel, Alistair.Francis, qemu-riscv, qemu-devel
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
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] goldfish_rtc: Fix tick_offset migration
2025-01-14 21:21 [PATCH] goldfish_rtc: Fix tick_offset migration Rodrigo Dias Correa
2025-01-31 0:54 ` Alistair Francis
@ 2025-01-31 1:32 ` Alistair Francis
2025-03-06 6:35 ` Michael Tokarev
2 siblings, 0 replies; 6+ messages in thread
From: Alistair Francis @ 2025-01-31 1:32 UTC (permalink / raw)
To: Rodrigo Dias Correa; +Cc: anup.patel, Alistair.Francis, qemu-riscv, qemu-devel
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
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] goldfish_rtc: Fix tick_offset migration
2025-01-14 21:21 [PATCH] goldfish_rtc: Fix tick_offset migration Rodrigo Dias Correa
2025-01-31 0:54 ` Alistair Francis
2025-01-31 1:32 ` Alistair Francis
@ 2025-03-06 6:35 ` Michael Tokarev
2025-03-06 6:42 ` Michael Tokarev
2 siblings, 1 reply; 6+ messages in thread
From: Michael Tokarev @ 2025-03-06 6:35 UTC (permalink / raw)
To: Rodrigo Dias Correa, anup.patel, Alistair.Francis
Cc: qemu-riscv, qemu-devel, qemu-stable
15.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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] goldfish_rtc: Fix tick_offset migration
2025-03-06 6:35 ` Michael Tokarev
@ 2025-03-06 6:42 ` Michael Tokarev
2025-03-06 7:46 ` Alistair Francis
0 siblings, 1 reply; 6+ messages in thread
From: Michael Tokarev @ 2025-03-06 6:42 UTC (permalink / raw)
To: Rodrigo Dias Correa, anup.patel, Alistair.Francis
Cc: qemu-riscv, qemu-devel, qemu-stable
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] goldfish_rtc: Fix tick_offset migration
2025-03-06 6:42 ` Michael Tokarev
@ 2025-03-06 7:46 ` Alistair Francis
0 siblings, 0 replies; 6+ messages in thread
From: Alistair Francis @ 2025-03-06 7:46 UTC (permalink / raw)
To: Michael Tokarev
Cc: Rodrigo Dias Correa, anup.patel, Alistair.Francis, qemu-riscv,
qemu-devel, qemu-stable
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-03-06 7:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-14 21:21 [PATCH] goldfish_rtc: Fix tick_offset migration Rodrigo Dias Correa
2025-01-31 0:54 ` Alistair Francis
2025-01-31 1:32 ` Alistair Francis
2025-03-06 6:35 ` Michael Tokarev
2025-03-06 6:42 ` Michael Tokarev
2025-03-06 7:46 ` Alistair Francis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).