Re: [PATCH 2/3] hw/intc: Save timers array in RISC-V mtimer VMState

[LIU Zhiwei]

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_...@linux.alibaba.com>
Signed-off-by: TANG Tiancheng<lyn...@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)