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
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)
>
>
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
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)
>>>
© 2016 - 2025 Red Hat, Inc.