On 10/22/24 17:23, Kevin Wolf wrote:
Am 22.10.2024 um 15:40 hat Cédric Le Goater geschrieben:
   static const VMStateDescription vmstate_aspeed_smc = {
       .name = "aspeed.smc",
-    .version_id = 2,
+    .version_id = 3,
       .minimum_version_id = 2,
       .fields = (const VMStateField[]) {
           VMSTATE_UINT32_ARRAY(regs, AspeedSMCState, ASPEED_SMC_R_MAX),
           VMSTATE_UINT8(snoop_index, AspeedSMCState),
           VMSTATE_UINT8(snoop_dummies, AspeedSMCState),
+        VMSTATE_BOOL(unselect, AspeedSMCState),
           VMSTATE_END_OF_LIST()
       }
   };

I think this will break migration compatibility. In order to enable
at least forward migration, it should be:

      VMSTATE_BOOL_V(unselect, AspeedSMCState, 3),

This is correct. I will fix the patch.

Some background,

The aspeed machines are fully emulated and the Aspeed SoC models are not
part of any virt* machines (yet). So migration support is a bit of a
theory. We have done our best to maintain some support, compatibility
not being a priority. IOW, it's not perfectly tuned as on virt machines.

Also, on ARM, migration of the CPU secure mode (I think this is the reason,
Peter please correct me !) is not supported and if migration is initiated
after Linux has started, the machine will hang.

That's a good reason not to implement backwards migration for now, it
would only complicate things. But as long as we claim to be migratable
by having VMStateDescriptions and even increasing version_id, we should
at least try to keep that part correct.

Sure. Thanks for pointing that out.

However, if one day, an aspeed model becomes part of a virt machine, we
should be more careful. I would start by resetting all vmstate versions
to 1!

Why would you reset it? Keeping 3 (or whatever it will be by then) as
the first serious supported version shouldn't hurt and probably avoids
some confusion.
 OK . We'll see when we get to that point. Who knows. The Aspeed machines
have gotten much bigger than we expected !

C.

Reply via email to