Ping for review from the migration folks on this one, and especially on the followup query at the bottom about version_ids in the vmstates using this macro.
thanks -- PMM On Mon, 19 Oct 2026 at 15:58, Peter Maydell <[email protected]> wrote: > > On Mon, 19 Oct 2026 at 14:16, Peter Maydell <[email protected]> wrote: > > > > The VMSTATE_PTIMER macro is defined to pass a 'version' argument of 1 > > to VMSTATE_STRUCT_POINTER_TEST_V. This means that the field will > > only be used in migration when the vmstate struct that uses > > VMSTATE_PTIMER has a version_id of at least 1. This doesn't match > > how we define other VMSTATE_FOO which don't take an explicit > > version_id, which is to use a version argument of 0 (e.g. all the > > common macros like VMSTATE_UINT32 are written to pass 0 to their > > VMSTATE_UINT32_V counterparts, and so on). It also doesn't match > > how we define VMSTATE_PTIMER_ARRAY. > > > > The practical effect of this is that users of VMSTATE_PTIMER in > > vmstate structs which explicitly or implicitly have a version_id of 0 > > were actually not getting their ptimers migrated. > > > > This affects: > > vmstate_xlnx_csu_dma in wh/dma/xlnx_csu_dma.c > > vmstate_k230_wdt in hw/watchdog/k230_wdt.c > > vmstate_imx2_wdt in hw/watchdog/wdt_imx2.c > > > > which are used in: > > * Arm imx-based machines > > * Arm Xilinx versal and zynq machines > > * RISC-V K230 machine > > > > Most ptimer users are unaffected because they took the usual approach > > of explicitly setting their starting version_id to 1. > > > > Fix the version_id in this field. This is a migration compatibility > > break for all the above listed devices/machines. Since the timers > > involved are in devices where the timer is usually stopped (e.g. the > > watchdogs only have the timer active when the watchdog is enabled and > > counting down), the consequences of the bug are likely not visible to > > most users. > > > > We could perhaps attempt to preserve migration compatibility here, > > but none of the affected machine types are versioned and so we do not > > promise cross-version compatibility, so do the simple thing. > > > > Fixes: 852f771ec9e5c ("vmstate: port pmtimer") > > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4705 > > Signed-off-by: Peter Maydell <[email protected]> > > --- > > Not cc: stable because it's a migration format change and we > > don't generally backport those. > > > > As an aside, is the expected way to write a new vmstate to give it > > an explicit version_id of 1? If so, maybe we should look at the > > vmstate structs which don't define one and so get the default 0. > > Or is that just some odd quirk I've picked up because lots of > > existing code uses 1 as its first version ID value? > > > > --- > > include/hw/core/ptimer.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/hw/core/ptimer.h b/include/hw/core/ptimer.h > > index 4dc02b0de47..8513f8917e3 100644 > > --- a/include/hw/core/ptimer.h > > +++ b/include/hw/core/ptimer.h > > @@ -300,7 +300,7 @@ void ptimer_stop(ptimer_state *s); > > extern const VMStateDescription vmstate_ptimer; > > > > #define VMSTATE_PTIMER(_field, _state) \ > > - VMSTATE_STRUCT_POINTER_V(_field, _state, 1, vmstate_ptimer, > > ptimer_state) > > + VMSTATE_STRUCT_POINTER_V(_field, _state, 0, vmstate_ptimer, > > ptimer_state) > > > > #define VMSTATE_PTIMER_ARRAY(_f, _s, _n) \ > > VMSTATE_ARRAY_OF_POINTER_TO_STRUCT(_f, _s, _n, 0, \ > > Followup query: should we also bump the version_id on the three > affected vmstate structs, to make it clearer that the migration > compat break has happened? Or does it not matter? > > thanks > -- PMM
