* [PATCH 0/3] Fix RISC-V timer migration issues
@ 2025-09-09 9:46 TANG Tiancheng
2025-09-09 9:46 ` [PATCH 1/3] hw/intc: Save time_delta in RISC-V mtimer VMState TANG Tiancheng
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: TANG Tiancheng @ 2025-09-09 9:46 UTC (permalink / raw)
To: qemu-devel
Cc: Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv, Peter Xu,
Fabiano Rosas, TANG Tiancheng
This patch set fixes several timer-related migration issues in QEMU's
RISC-V implementation that cause timer events to be lost or behave
incorrectly after snapshot save/restore or live migration.
The problems addressed are:
1. ACLINT mtimer time_delta not migrated: The time_delta field in
RISCVAclintMTimerState was missing from vmstate, causing incorrect
mtime values after snapshot restore. This resulted in guest time
appearing "frozen" until enough virtual time elapsed to compensate
for the offset error.
2. ACLINT mtimer timers array not migrated: Active timer events
scheduled via riscv_aclint_mtimer_write_timecmp() were not being
migrated, causing pending timer interrupts to be lost after restore.
3. CPU stimer/vstimer not migrated: The S-mode and VS-mode timer
pointers in CPURISCVState were missing from vmstate_riscv_cpu,
causing supervisor-level timer events to be lost.
The patch set introduces a new VMSTATE_TIMER_PTR_VARRAY macro to handle
migration of variable-length timer pointer arrays, and adds the missing
timer fields to the appropriate vmstate structures.
Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com>
---
TANG Tiancheng (3):
hw/intc: Save time_delta in RISC-V mtimer VMState
hw/intc: Save timers array in RISC-V mtimer VMState
target/riscv: Save stimer and vstimer in CPU vmstate
hw/intc/riscv_aclint.c | 7 +++++--
include/migration/vmstate.h | 14 ++++++++++++++
target/riscv/machine.c | 25 +++++++++++++++++++++++++
3 files changed, 44 insertions(+), 2 deletions(-)
---
base-commit: 6a9fa5ef3230a7d51e0d953a59ee9ef10af705b8
change-id: 20250909-timers-18c2c67b1a2a
Best regards,
--
TANG Tiancheng <lyndra@linux.alibaba.com>
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/3] hw/intc: Save time_delta in RISC-V mtimer VMState 2025-09-09 9:46 [PATCH 0/3] Fix RISC-V timer migration issues TANG Tiancheng @ 2025-09-09 9:46 ` TANG Tiancheng 2025-09-09 12:36 ` Daniel Henrique Barboza 2025-09-09 9:46 ` [PATCH 2/3] hw/intc: Save timers array " TANG Tiancheng 2025-09-09 9:46 ` [PATCH 3/3] target/riscv: Save stimer and vstimer in CPU vmstate TANG Tiancheng 2 siblings, 1 reply; 9+ messages in thread From: TANG Tiancheng @ 2025-09-09 9:46 UTC (permalink / raw) To: qemu-devel Cc: Palmer Dabbelt, Alistair Francis, Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv, Peter Xu, Fabiano Rosas, TANG Tiancheng In QEMU's RISC-V ACLINT timer model, 'mtime' is not stored directly as a state variable. It is computed on demand as: mtime = rtc_r + time_delta where: - 'rtc_r' is the current VM virtual time (in ticks) obtained via cpu_riscv_read_rtc_raw() from QEMU_CLOCK_VIRTUAL. - 'time_delta' is an offset applied when the guest writes a new 'mtime' value via riscv_aclint_mtimer_write(): time_delta = value - rtc_r Under this design, 'rtc_r' is assumed to be monotonically increasing during VM execution. Even if the guest writes an 'mtime' value smaller than the current one (making 'time_delta' negative in signed arithmetic, or underflow in unsigned arithmetic), the computed 'mtime' remains correct because 'rtc_r_new > rtc_r_old': mtime_new = rtc_r_new + (value - rtc_r_old) However, this monotonicity assumption breaks on snapshot load. Before restoring a snapshot, QEMU resets the guest, which calls riscv_aclint_mtimer_reset_enter() to set 'mtime' to 0 and recompute 'time_delta' as: time_delta = 0 - rtc_r_reset Here, the time_delta differs from the value that was present when the snapshot was saved. As a result, subsequent reads produce a fixed offset from the true mtime. This can be observed with the 'date' command inside the guest: after loading a snapshot, the reported time appears "frozen" at the save point, and only resumes correctly after the guest has run long enough to compensate for the erroneous offset. The fix is to treat 'time_delta' as part of the device's migratable state and save/restore it via vmstate. This preserves the correct relation between 'rtc_r' and 'mtime' across snapshot save/load, ensuring 'mtime' continues incrementing from the precise saved value after restore. Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com> --- hw/intc/riscv_aclint.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c index 4623cfa029365c6cbdead4bd4a9f0d8b9e88b939..318a9c8248432a8cd4c3f3fa990739917ecf7ca1 100644 --- a/hw/intc/riscv_aclint.c +++ b/hw/intc/riscv_aclint.c @@ -323,9 +323,10 @@ static void riscv_aclint_mtimer_reset_enter(Object *obj, ResetType type) static const VMStateDescription vmstate_riscv_mtimer = { .name = "riscv_mtimer", - .version_id = 1, - .minimum_version_id = 1, + .version_id = 2, + .minimum_version_id = 2, .fields = (const VMStateField[]) { + VMSTATE_UINT64(time_delta, RISCVAclintMTimerState), VMSTATE_VARRAY_UINT32(timecmp, RISCVAclintMTimerState, num_harts, 0, vmstate_info_uint64, uint64_t), -- 2.43.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] hw/intc: Save time_delta in RISC-V mtimer VMState 2025-09-09 9:46 ` [PATCH 1/3] hw/intc: Save time_delta in RISC-V mtimer VMState TANG Tiancheng @ 2025-09-09 12:36 ` Daniel Henrique Barboza 0 siblings, 0 replies; 9+ messages in thread From: Daniel Henrique Barboza @ 2025-09-09 12:36 UTC (permalink / raw) To: TANG Tiancheng, qemu-devel Cc: Palmer Dabbelt, Alistair Francis, Weiwei Li, Liu Zhiwei, qemu-riscv, Peter Xu, Fabiano Rosas On 9/9/25 6:46 AM, TANG Tiancheng wrote: > In QEMU's RISC-V ACLINT timer model, 'mtime' is not stored directly as a > state variable. It is computed on demand as: > > mtime = rtc_r + time_delta > > where: > - 'rtc_r' is the current VM virtual time (in ticks) obtained via > cpu_riscv_read_rtc_raw() from QEMU_CLOCK_VIRTUAL. > - 'time_delta' is an offset applied when the guest writes a new 'mtime' > value via riscv_aclint_mtimer_write(): > > time_delta = value - rtc_r > > Under this design, 'rtc_r' is assumed to be monotonically increasing > during VM execution. Even if the guest writes an 'mtime' value smaller > than the current one (making 'time_delta' negative in signed arithmetic, > or underflow in unsigned arithmetic), the computed 'mtime' remains > correct because 'rtc_r_new > rtc_r_old': > > mtime_new = rtc_r_new + (value - rtc_r_old) > > However, this monotonicity assumption breaks on snapshot load. > > Before restoring a snapshot, QEMU resets the guest, which calls > riscv_aclint_mtimer_reset_enter() to set 'mtime' to 0 and recompute > 'time_delta' as: > > time_delta = 0 - rtc_r_reset > > Here, the time_delta differs from the value that was present when the > snapshot was saved. As a result, subsequent reads produce a fixed offset > from the true mtime. > > This can be observed with the 'date' command inside the guest: after loading > a snapshot, the reported time appears "frozen" at the save point, and only > resumes correctly after the guest has run long enough to compensate for the > erroneous offset. > > The fix is to treat 'time_delta' as part of the device's migratable > state and save/restore it via vmstate. This preserves the correct > relation between 'rtc_r' and 'mtime' across snapshot save/load, ensuring > 'mtime' continues incrementing from the precise saved value after > restore. > > Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> > Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com> > --- Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > hw/intc/riscv_aclint.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c > index 4623cfa029365c6cbdead4bd4a9f0d8b9e88b939..318a9c8248432a8cd4c3f3fa990739917ecf7ca1 100644 > --- a/hw/intc/riscv_aclint.c > +++ b/hw/intc/riscv_aclint.c > @@ -323,9 +323,10 @@ static void riscv_aclint_mtimer_reset_enter(Object *obj, ResetType type) > > static const VMStateDescription vmstate_riscv_mtimer = { > .name = "riscv_mtimer", > - .version_id = 1, > - .minimum_version_id = 1, > + .version_id = 2, > + .minimum_version_id = 2, > .fields = (const VMStateField[]) { > + VMSTATE_UINT64(time_delta, RISCVAclintMTimerState), > VMSTATE_VARRAY_UINT32(timecmp, RISCVAclintMTimerState, > num_harts, 0, > vmstate_info_uint64, uint64_t), > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] hw/intc: Save timers array in RISC-V mtimer VMState 2025-09-09 9:46 [PATCH 0/3] Fix RISC-V timer migration issues TANG Tiancheng 2025-09-09 9:46 ` [PATCH 1/3] hw/intc: Save time_delta in RISC-V mtimer VMState TANG Tiancheng @ 2025-09-09 9:46 ` TANG Tiancheng 2025-09-09 12:45 ` Daniel Henrique Barboza 2025-09-09 9:46 ` [PATCH 3/3] target/riscv: Save stimer and vstimer in CPU vmstate TANG Tiancheng 2 siblings, 1 reply; 9+ messages in thread From: TANG Tiancheng @ 2025-09-09 9:46 UTC (permalink / raw) To: qemu-devel Cc: Palmer Dabbelt, Alistair Francis, Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv, Peter Xu, Fabiano Rosas, TANG Tiancheng The current 'timecmp' field in vmstate_riscv_mtimer is insufficient to keep timers functional after migration. If an mtimer's entry in 'mtimer->timers' is active at the time the snapshot is taken, it means riscv_aclint_mtimer_write_timecmp() has written to 'mtimecmp' and scheduled a timer into QEMU's main loop 'timer_list'. During snapshot save, these active timers must also be migrated; otherwise, after snapshot load there is no mechanism to restore 'mtimer->timers' back into the 'timer_list', and any pending timer events would be lost. QEMU's migration framework commonly uses VMSTATE_TIMER_xxx macros to save and restore 'QEMUTimer' variables. However, 'timers' is a pointer array with variable length, and vmstate.h did not previously provide a helper macro for such type. This commit adds a new macro, 'VMSTATE_TIMER_PTR_VARRAY', to handle saving and restoring a variable-length array of 'QEMUTimer *'. We then use this macro to migrate the 'mtimer->timers' array, ensuring that timer events remain scheduled correctly after snapshot load. Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com> --- hw/intc/riscv_aclint.c | 6 ++++-- include/migration/vmstate.h | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c index 318a9c8248432a8cd4c3f3fa990739917ecf7ca1..9f4c36e965e2aa379d75c0a9f656177f0dd82a45 100644 --- a/hw/intc/riscv_aclint.c +++ b/hw/intc/riscv_aclint.c @@ -323,13 +323,15 @@ static void riscv_aclint_mtimer_reset_enter(Object *obj, ResetType type) static const VMStateDescription vmstate_riscv_mtimer = { .name = "riscv_mtimer", - .version_id = 2, - .minimum_version_id = 2, + .version_id = 3, + .minimum_version_id = 3, .fields = (const VMStateField[]) { VMSTATE_UINT64(time_delta, RISCVAclintMTimerState), VMSTATE_VARRAY_UINT32(timecmp, RISCVAclintMTimerState, num_harts, 0, vmstate_info_uint64, uint64_t), + VMSTATE_TIMER_PTR_VARRAY(timers, RISCVAclintMTimerState, + num_harts), VMSTATE_END_OF_LIST() } }; diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 1ff7bd9ac425ba67cd5ca7ad97bcf570f9e19abe..255e403e5a103188712425d95a719d181e1a7202 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -522,6 +522,16 @@ extern const VMStateInfo vmstate_info_qlist; .offset = vmstate_offset_array(_s, _f, _type*, _n), \ } +#define VMSTATE_VARRAY_OF_POINTER_UINT32(_field, _state, _field_num, _version, _info, _type) { \ + .name = (stringify(_field)), \ + .version_id = (_version), \ + .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \ + .info = &(_info), \ + .size = sizeof(_type), \ + .flags = VMS_VARRAY_UINT32 | VMS_ARRAY_OF_POINTER | VMS_POINTER, \ + .offset = vmstate_offset_pointer(_state, _field, _type), \ +} + #define VMSTATE_STRUCT_SUB_ARRAY(_field, _state, _start, _num, _version, _vmsd, _type) { \ .name = (stringify(_field)), \ .version_id = (_version), \ @@ -1035,6 +1045,10 @@ extern const VMStateInfo vmstate_info_qlist; #define VMSTATE_TIMER_PTR_ARRAY(_f, _s, _n) \ VMSTATE_ARRAY_OF_POINTER(_f, _s, _n, 0, vmstate_info_timer, QEMUTimer *) +#define VMSTATE_TIMER_PTR_VARRAY(_f, _s, _f_n) \ +VMSTATE_VARRAY_OF_POINTER_UINT32(_f, _s, _f_n, 0, vmstate_info_timer, \ + QEMUTimer *) + #define VMSTATE_TIMER_TEST(_f, _s, _test) \ VMSTATE_SINGLE_TEST(_f, _s, _test, 0, vmstate_info_timer, QEMUTimer) -- 2.43.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] hw/intc: Save timers array in RISC-V mtimer VMState 2025-09-09 9:46 ` [PATCH 2/3] hw/intc: Save timers array " TANG Tiancheng @ 2025-09-09 12:45 ` Daniel Henrique Barboza 2025-09-09 21:34 ` Peter Xu 0 siblings, 1 reply; 9+ messages in thread From: Daniel Henrique Barboza @ 2025-09-09 12:45 UTC (permalink / raw) To: TANG Tiancheng, qemu-devel Cc: Palmer Dabbelt, Alistair Francis, Weiwei Li, Liu Zhiwei, qemu-riscv, Peter Xu, Fabiano Rosas On 9/9/25 6:46 AM, TANG Tiancheng wrote: > The current 'timecmp' field in vmstate_riscv_mtimer is insufficient to keep > timers functional after migration. > > If an mtimer's entry in 'mtimer->timers' is active at the time the snapshot > is taken, it means riscv_aclint_mtimer_write_timecmp() has written to > 'mtimecmp' and scheduled a timer into QEMU's main loop 'timer_list'. > > During snapshot save, these active timers must also be migrated; otherwise, > after snapshot load there is no mechanism to restore 'mtimer->timers' back > into the 'timer_list', and any pending timer events would be lost. > > QEMU's migration framework commonly uses VMSTATE_TIMER_xxx macros to save > and restore 'QEMUTimer' variables. However, 'timers' is a pointer array > with variable length, and vmstate.h did not previously provide a helper > macro for such type. > > This commit adds a new macro, 'VMSTATE_TIMER_PTR_VARRAY', to handle saving > and restoring a variable-length array of 'QEMUTimer *'. We then use this > macro to migrate the 'mtimer->timers' array, ensuring that timer events > remain scheduled correctly after snapshot load. > > Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> > Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com> > --- LGTM but I wonder if changing one of the VMSTATE_TIMER_xxxx macros to accept variable length arrays is better than creating a new macro that only RISC-V cares about. Creating a new macro surely is easier and less messier than changing all existing callers though ... I'll let the migration folks weight in. Thanks, Daniel > hw/intc/riscv_aclint.c | 6 ++++-- > include/migration/vmstate.h | 14 ++++++++++++++ > 2 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c > index 318a9c8248432a8cd4c3f3fa990739917ecf7ca1..9f4c36e965e2aa379d75c0a9f656177f0dd82a45 100644 > --- a/hw/intc/riscv_aclint.c > +++ b/hw/intc/riscv_aclint.c > @@ -323,13 +323,15 @@ static void riscv_aclint_mtimer_reset_enter(Object *obj, ResetType type) > > static const VMStateDescription vmstate_riscv_mtimer = { > .name = "riscv_mtimer", > - .version_id = 2, > - .minimum_version_id = 2, > + .version_id = 3, > + .minimum_version_id = 3, > .fields = (const VMStateField[]) { > VMSTATE_UINT64(time_delta, RISCVAclintMTimerState), > VMSTATE_VARRAY_UINT32(timecmp, RISCVAclintMTimerState, > num_harts, 0, > vmstate_info_uint64, uint64_t), > + VMSTATE_TIMER_PTR_VARRAY(timers, RISCVAclintMTimerState, > + num_harts), > VMSTATE_END_OF_LIST() > } > }; > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > index 1ff7bd9ac425ba67cd5ca7ad97bcf570f9e19abe..255e403e5a103188712425d95a719d181e1a7202 100644 > --- a/include/migration/vmstate.h > +++ b/include/migration/vmstate.h > @@ -522,6 +522,16 @@ extern const VMStateInfo vmstate_info_qlist; > .offset = vmstate_offset_array(_s, _f, _type*, _n), \ > } > > +#define VMSTATE_VARRAY_OF_POINTER_UINT32(_field, _state, _field_num, _version, _info, _type) { \ > + .name = (stringify(_field)), \ > + .version_id = (_version), \ > + .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \ > + .info = &(_info), \ > + .size = sizeof(_type), \ > + .flags = VMS_VARRAY_UINT32 | VMS_ARRAY_OF_POINTER | VMS_POINTER, \ > + .offset = vmstate_offset_pointer(_state, _field, _type), \ > +} > + > #define VMSTATE_STRUCT_SUB_ARRAY(_field, _state, _start, _num, _version, _vmsd, _type) { \ > .name = (stringify(_field)), \ > .version_id = (_version), \ > @@ -1035,6 +1045,10 @@ extern const VMStateInfo vmstate_info_qlist; > #define VMSTATE_TIMER_PTR_ARRAY(_f, _s, _n) \ > VMSTATE_ARRAY_OF_POINTER(_f, _s, _n, 0, vmstate_info_timer, QEMUTimer *) > > +#define VMSTATE_TIMER_PTR_VARRAY(_f, _s, _f_n) \ > +VMSTATE_VARRAY_OF_POINTER_UINT32(_f, _s, _f_n, 0, vmstate_info_timer, \ > + QEMUTimer *) > + > #define VMSTATE_TIMER_TEST(_f, _s, _test) \ > VMSTATE_SINGLE_TEST(_f, _s, _test, 0, vmstate_info_timer, QEMUTimer) > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] hw/intc: Save timers array in RISC-V mtimer VMState 2025-09-09 12:45 ` Daniel Henrique Barboza @ 2025-09-09 21:34 ` Peter Xu 2025-09-10 1:33 ` LIU Zhiwei 0 siblings, 1 reply; 9+ messages in thread From: Peter Xu @ 2025-09-09 21:34 UTC (permalink / raw) To: Daniel Henrique Barboza Cc: TANG Tiancheng, qemu-devel, Palmer Dabbelt, Alistair Francis, Weiwei Li, Liu Zhiwei, qemu-riscv, Fabiano Rosas On Tue, Sep 09, 2025 at 09:45:07AM -0300, Daniel Henrique Barboza wrote: > > > On 9/9/25 6:46 AM, TANG Tiancheng wrote: > > The current 'timecmp' field in vmstate_riscv_mtimer is insufficient to keep > > timers functional after migration. > > > > If an mtimer's entry in 'mtimer->timers' is active at the time the snapshot > > is taken, it means riscv_aclint_mtimer_write_timecmp() has written to > > 'mtimecmp' and scheduled a timer into QEMU's main loop 'timer_list'. > > > > During snapshot save, these active timers must also be migrated; otherwise, > > after snapshot load there is no mechanism to restore 'mtimer->timers' back > > into the 'timer_list', and any pending timer events would be lost. > > > > QEMU's migration framework commonly uses VMSTATE_TIMER_xxx macros to save > > and restore 'QEMUTimer' variables. However, 'timers' is a pointer array > > with variable length, and vmstate.h did not previously provide a helper > > macro for such type. > > > > This commit adds a new macro, 'VMSTATE_TIMER_PTR_VARRAY', to handle saving > > and restoring a variable-length array of 'QEMUTimer *'. We then use this > > macro to migrate the 'mtimer->timers' array, ensuring that timer events > > remain scheduled correctly after snapshot load. > > > > Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> > > Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com> > > --- > > LGTM but I wonder if changing one of the VMSTATE_TIMER_xxxx macros to accept > variable length arrays is better than creating a new macro that only RISC-V > cares about. Creating a new macro surely is easier and less messier than > changing all existing callers though ... > > I'll let the migration folks weight in. Thanks, I didn't see a 2nd user, though.. I'd split VMSTATE_VARRAY_OF_POINTER_UINT32() into a separate patch, then define VMSTATE_TIMER_PTR_VARRAY() in riscv .c files if that's the only user, and if that helps readability. Thanks, > > Daniel > > > > > hw/intc/riscv_aclint.c | 6 ++++-- > > include/migration/vmstate.h | 14 ++++++++++++++ > > 2 files changed, 18 insertions(+), 2 deletions(-) > > > > diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c > > index 318a9c8248432a8cd4c3f3fa990739917ecf7ca1..9f4c36e965e2aa379d75c0a9f656177f0dd82a45 100644 > > --- a/hw/intc/riscv_aclint.c > > +++ b/hw/intc/riscv_aclint.c > > @@ -323,13 +323,15 @@ static void riscv_aclint_mtimer_reset_enter(Object *obj, ResetType type) > > static const VMStateDescription vmstate_riscv_mtimer = { > > .name = "riscv_mtimer", > > - .version_id = 2, > > - .minimum_version_id = 2, > > + .version_id = 3, > > + .minimum_version_id = 3, > > .fields = (const VMStateField[]) { > > VMSTATE_UINT64(time_delta, RISCVAclintMTimerState), > > VMSTATE_VARRAY_UINT32(timecmp, RISCVAclintMTimerState, > > num_harts, 0, > > vmstate_info_uint64, uint64_t), > > + VMSTATE_TIMER_PTR_VARRAY(timers, RISCVAclintMTimerState, > > + num_harts), > > VMSTATE_END_OF_LIST() > > } > > }; > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > > index 1ff7bd9ac425ba67cd5ca7ad97bcf570f9e19abe..255e403e5a103188712425d95a719d181e1a7202 100644 > > --- a/include/migration/vmstate.h > > +++ b/include/migration/vmstate.h > > @@ -522,6 +522,16 @@ extern const VMStateInfo vmstate_info_qlist; > > .offset = vmstate_offset_array(_s, _f, _type*, _n), \ > > } > > +#define VMSTATE_VARRAY_OF_POINTER_UINT32(_field, _state, _field_num, _version, _info, _type) { \ > > + .name = (stringify(_field)), \ > > + .version_id = (_version), \ > > + .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \ > > + .info = &(_info), \ > > + .size = sizeof(_type), \ > > + .flags = VMS_VARRAY_UINT32 | VMS_ARRAY_OF_POINTER | VMS_POINTER, \ > > + .offset = vmstate_offset_pointer(_state, _field, _type), \ > > +} > > + > > #define VMSTATE_STRUCT_SUB_ARRAY(_field, _state, _start, _num, _version, _vmsd, _type) { \ > > .name = (stringify(_field)), \ > > .version_id = (_version), \ > > @@ -1035,6 +1045,10 @@ extern const VMStateInfo vmstate_info_qlist; > > #define VMSTATE_TIMER_PTR_ARRAY(_f, _s, _n) \ > > VMSTATE_ARRAY_OF_POINTER(_f, _s, _n, 0, vmstate_info_timer, QEMUTimer *) > > +#define VMSTATE_TIMER_PTR_VARRAY(_f, _s, _f_n) \ > > +VMSTATE_VARRAY_OF_POINTER_UINT32(_f, _s, _f_n, 0, vmstate_info_timer, \ > > + QEMUTimer *) > > + > > #define VMSTATE_TIMER_TEST(_f, _s, _test) \ > > VMSTATE_SINGLE_TEST(_f, _s, _test, 0, vmstate_info_timer, QEMUTimer) > > > -- Peter Xu ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] hw/intc: Save timers array in RISC-V mtimer VMState 2025-09-09 21:34 ` Peter Xu @ 2025-09-10 1:33 ` LIU Zhiwei 0 siblings, 0 replies; 9+ messages in thread From: LIU Zhiwei @ 2025-09-10 1:33 UTC (permalink / raw) To: Peter Xu, Daniel Henrique Barboza Cc: TANG Tiancheng, qemu-devel, Palmer Dabbelt, Alistair Francis, Weiwei Li, qemu-riscv, Fabiano Rosas [-- Attachment #1: Type: text/plain, Size: 5416 bytes --] On 9/10/25 5:34 AM, Peter Xu wrote: > On Tue, Sep 09, 2025 at 09:45:07AM -0300, Daniel Henrique Barboza wrote: >> >> On 9/9/25 6:46 AM, TANG Tiancheng wrote: >>> The current 'timecmp' field in vmstate_riscv_mtimer is insufficient to keep >>> timers functional after migration. >>> >>> If an mtimer's entry in 'mtimer->timers' is active at the time the snapshot >>> is taken, it means riscv_aclint_mtimer_write_timecmp() has written to >>> 'mtimecmp' and scheduled a timer into QEMU's main loop 'timer_list'. >>> >>> During snapshot save, these active timers must also be migrated; otherwise, >>> after snapshot load there is no mechanism to restore 'mtimer->timers' back >>> into the 'timer_list', and any pending timer events would be lost. >>> >>> QEMU's migration framework commonly uses VMSTATE_TIMER_xxx macros to save >>> and restore 'QEMUTimer' variables. However, 'timers' is a pointer array >>> with variable length, and vmstate.h did not previously provide a helper >>> macro for such type. >>> >>> This commit adds a new macro, 'VMSTATE_TIMER_PTR_VARRAY', to handle saving >>> and restoring a variable-length array of 'QEMUTimer *'. We then use this >>> macro to migrate the 'mtimer->timers' array, ensuring that timer events >>> remain scheduled correctly after snapshot load. >>> >>> Reviewed-by: LIU Zhiwei<zhiwei_liu@linux.alibaba.com> >>> Signed-off-by: TANG Tiancheng<lyndra@linux.alibaba.com> >>> --- >> LGTM but I wonder if changing one of the VMSTATE_TIMER_xxxx macros to accept >> variable length arrays is better than creating a new macro that only RISC-V >> cares about. Creating a new macro surely is easier and less messier than >> changing all existing callers though ... >> >> I'll let the migration folks weight in. Thanks, > I didn't see a 2nd user, though.. > > I'd split VMSTATE_VARRAY_OF_POINTER_UINT32() into a separate patch, OK. > then > define VMSTATE_TIMER_PTR_VARRAY() in riscv .c files if that's the only > user, OK. We will send the v2 patch set later, where VMSTATE_VARRAY_OF_POINTER_UINT32() as a separate patch for migration and the others as a patch set for RISC-V. Thanks, Zhiwei > and if that helps readability. > > Thanks, > >> Daniel >> >> >> >>> hw/intc/riscv_aclint.c | 6 ++++-- >>> include/migration/vmstate.h | 14 ++++++++++++++ >>> 2 files changed, 18 insertions(+), 2 deletions(-) >>> >>> diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c >>> index 318a9c8248432a8cd4c3f3fa990739917ecf7ca1..9f4c36e965e2aa379d75c0a9f656177f0dd82a45 100644 >>> --- a/hw/intc/riscv_aclint.c >>> +++ b/hw/intc/riscv_aclint.c >>> @@ -323,13 +323,15 @@ static void riscv_aclint_mtimer_reset_enter(Object *obj, ResetType type) >>> static const VMStateDescription vmstate_riscv_mtimer = { >>> .name = "riscv_mtimer", >>> - .version_id = 2, >>> - .minimum_version_id = 2, >>> + .version_id = 3, >>> + .minimum_version_id = 3, >>> .fields = (const VMStateField[]) { >>> VMSTATE_UINT64(time_delta, RISCVAclintMTimerState), >>> VMSTATE_VARRAY_UINT32(timecmp, RISCVAclintMTimerState, >>> num_harts, 0, >>> vmstate_info_uint64, uint64_t), >>> + VMSTATE_TIMER_PTR_VARRAY(timers, RISCVAclintMTimerState, >>> + num_harts), >>> VMSTATE_END_OF_LIST() >>> } >>> }; >>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h >>> index 1ff7bd9ac425ba67cd5ca7ad97bcf570f9e19abe..255e403e5a103188712425d95a719d181e1a7202 100644 >>> --- a/include/migration/vmstate.h >>> +++ b/include/migration/vmstate.h >>> @@ -522,6 +522,16 @@ extern const VMStateInfo vmstate_info_qlist; >>> .offset = vmstate_offset_array(_s, _f, _type*, _n), \ >>> } >>> +#define VMSTATE_VARRAY_OF_POINTER_UINT32(_field, _state, _field_num, _version, _info, _type) { \ >>> + .name = (stringify(_field)), \ >>> + .version_id = (_version), \ >>> + .num_offset = vmstate_offset_value(_state, _field_num, uint32_t), \ >>> + .info = &(_info), \ >>> + .size = sizeof(_type), \ >>> + .flags = VMS_VARRAY_UINT32 | VMS_ARRAY_OF_POINTER | VMS_POINTER, \ >>> + .offset = vmstate_offset_pointer(_state, _field, _type), \ >>> +} >>> + >>> #define VMSTATE_STRUCT_SUB_ARRAY(_field, _state, _start, _num, _version, _vmsd, _type) { \ >>> .name = (stringify(_field)), \ >>> .version_id = (_version), \ >>> @@ -1035,6 +1045,10 @@ extern const VMStateInfo vmstate_info_qlist; >>> #define VMSTATE_TIMER_PTR_ARRAY(_f, _s, _n) \ >>> VMSTATE_ARRAY_OF_POINTER(_f, _s, _n, 0, vmstate_info_timer, QEMUTimer *) >>> +#define VMSTATE_TIMER_PTR_VARRAY(_f, _s, _f_n) \ >>> +VMSTATE_VARRAY_OF_POINTER_UINT32(_f, _s, _f_n, 0, vmstate_info_timer, \ >>> + QEMUTimer *) >>> + >>> #define VMSTATE_TIMER_TEST(_f, _s, _test) \ >>> VMSTATE_SINGLE_TEST(_f, _s, _test, 0, vmstate_info_timer, QEMUTimer) >>> [-- Attachment #2: Type: text/html, Size: 6513 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] target/riscv: Save stimer and vstimer in CPU vmstate 2025-09-09 9:46 [PATCH 0/3] Fix RISC-V timer migration issues TANG Tiancheng 2025-09-09 9:46 ` [PATCH 1/3] hw/intc: Save time_delta in RISC-V mtimer VMState TANG Tiancheng 2025-09-09 9:46 ` [PATCH 2/3] hw/intc: Save timers array " TANG Tiancheng @ 2025-09-09 9:46 ` TANG Tiancheng 2025-09-09 12:49 ` Daniel Henrique Barboza 2 siblings, 1 reply; 9+ messages in thread From: TANG Tiancheng @ 2025-09-09 9:46 UTC (permalink / raw) To: qemu-devel Cc: Palmer Dabbelt, Alistair Francis, Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv, Peter Xu, Fabiano Rosas, TANG Tiancheng vmstate_riscv_cpu was missing env.stimer and env.vstimer. Without migrating these QEMUTimer fields, active S/VS-mode timer events are lost after snapshot or migration. Add VMSTATE_TIMER_PTR() entries to save and restore them. Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com> --- target/riscv/machine.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/target/riscv/machine.c b/target/riscv/machine.c index 1600ec44f0b755fdd49fc0df47c2288c9940afe0..51e0567ed30cbab5e791ea904165bc1854709192 100644 --- a/target/riscv/machine.c +++ b/target/riscv/machine.c @@ -400,6 +400,30 @@ static const VMStateDescription vmstate_ssp = { } }; +static bool sstc_timer_needed(void *opaque) +{ + RISCVCPU *cpu = opaque; + CPURISCVState *env = &cpu->env; + + if (!cpu->cfg.ext_sstc) { + return false; + } + + return env->stimer != NULL || env->vstimer != NULL; +} + +static const VMStateDescription vmstate_sstc = { + .name = "cpu/timer", + .version_id = 1, + .minimum_version_id = 1, + .needed = sstc_timer_needed, + .fields = (const VMStateField[]) { + VMSTATE_TIMER_PTR(env.stimer, RISCVCPU), + VMSTATE_TIMER_PTR(env.vstimer, RISCVCPU), + VMSTATE_END_OF_LIST() + } +}; + const VMStateDescription vmstate_riscv_cpu = { .name = "cpu", .version_id = 10, @@ -476,6 +500,7 @@ const VMStateDescription vmstate_riscv_cpu = { &vmstate_elp, &vmstate_ssp, &vmstate_ctr, + &vmstate_sstc, NULL } }; -- 2.43.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] target/riscv: Save stimer and vstimer in CPU vmstate 2025-09-09 9:46 ` [PATCH 3/3] target/riscv: Save stimer and vstimer in CPU vmstate TANG Tiancheng @ 2025-09-09 12:49 ` Daniel Henrique Barboza 0 siblings, 0 replies; 9+ messages in thread From: Daniel Henrique Barboza @ 2025-09-09 12:49 UTC (permalink / raw) To: TANG Tiancheng, qemu-devel Cc: Palmer Dabbelt, Alistair Francis, Weiwei Li, Liu Zhiwei, qemu-riscv, Peter Xu, Fabiano Rosas On 9/9/25 6:46 AM, TANG Tiancheng wrote: > vmstate_riscv_cpu was missing env.stimer and env.vstimer. > Without migrating these QEMUTimer fields, active S/VS-mode > timer events are lost after snapshot or migration. > > Add VMSTATE_TIMER_PTR() entries to save and restore them. > > Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> > Signed-off-by: TANG Tiancheng <lyndra@linux.alibaba.com> > --- Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > target/riscv/machine.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/target/riscv/machine.c b/target/riscv/machine.c > index 1600ec44f0b755fdd49fc0df47c2288c9940afe0..51e0567ed30cbab5e791ea904165bc1854709192 100644 > --- a/target/riscv/machine.c > +++ b/target/riscv/machine.c > @@ -400,6 +400,30 @@ static const VMStateDescription vmstate_ssp = { > } > }; > > +static bool sstc_timer_needed(void *opaque) > +{ > + RISCVCPU *cpu = opaque; > + CPURISCVState *env = &cpu->env; > + > + if (!cpu->cfg.ext_sstc) { > + return false; > + } > + > + return env->stimer != NULL || env->vstimer != NULL; > +} > + > +static const VMStateDescription vmstate_sstc = { > + .name = "cpu/timer", > + .version_id = 1, > + .minimum_version_id = 1, > + .needed = sstc_timer_needed, > + .fields = (const VMStateField[]) { > + VMSTATE_TIMER_PTR(env.stimer, RISCVCPU), > + VMSTATE_TIMER_PTR(env.vstimer, RISCVCPU), > + VMSTATE_END_OF_LIST() > + } > +}; > + > const VMStateDescription vmstate_riscv_cpu = { > .name = "cpu", > .version_id = 10, > @@ -476,6 +500,7 @@ const VMStateDescription vmstate_riscv_cpu = { > &vmstate_elp, > &vmstate_ssp, > &vmstate_ctr, > + &vmstate_sstc, > NULL > } > }; > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-09-10 1:36 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-09 9:46 [PATCH 0/3] Fix RISC-V timer migration issues TANG Tiancheng 2025-09-09 9:46 ` [PATCH 1/3] hw/intc: Save time_delta in RISC-V mtimer VMState TANG Tiancheng 2025-09-09 12:36 ` Daniel Henrique Barboza 2025-09-09 9:46 ` [PATCH 2/3] hw/intc: Save timers array " TANG Tiancheng 2025-09-09 12:45 ` Daniel Henrique Barboza 2025-09-09 21:34 ` Peter Xu 2025-09-10 1:33 ` LIU Zhiwei 2025-09-09 9:46 ` [PATCH 3/3] target/riscv: Save stimer and vstimer in CPU vmstate TANG Tiancheng 2025-09-09 12:49 ` Daniel Henrique Barboza
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).