[RFC PATCH v2 00/20] hw: Mark the device with no migratable fields

[RFC PATCH v2 00/20] hw: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

Since v1:
- Tried to address Dave and Daniel comments
- Added Peter R-b
- Handle GPEX device

This is a proof-of-concept after chatting with Peter Maydell
on IRC last year.

Introduce the vmstate_no_state_to_migrate structure, and
a reference to it: vmstate_qdev_no_state_to_migrate.
Use this reference in devices with no fields to migrate.

This is useful to catch devices missing vmstate, such:
- ads7846
- mcf-uart
- mcf-fec
- versatile_i2c
- ...

v1: https://www.mail-archive.com/qemu-devel@nongnu.org/msg719788.html

Philippe Mathieu-Daudé (20):
  migration/vmstate: Restrict vmstate_dummy to user-mode
  hw/core/qdev: Add vmstate_qdev_no_state_to_migrate
  hw/arm/armv7m: Mark the device with no migratable fields
  hw/arm/aspeed_soc: Mark the device with no migratable fields
  hw/arm/bcm283x: Mark devices with no migratable fields
  hw/arm/msf2-soc: Mark the device with no migratable fields
  hw/core/split-irq: Mark the device with no migratable fields
  hw/cpu/a9mpcore: Mark the device with no migratable fields
  hw/cpu/cluster: Mark the device with no migratable fields
  hw/usb/hcd-ohci: Mark the device with no migratable fields
  hw/intc/arm_gicv2m: Mark the device with no migratable fields
  hw/misc/armsse-cpuid: Mark the device with no migratable fields
  hw/misc/iotkit-sysinfo: Mark the device with no migratable fields
  hw/misc/unimp: Mark the device with no migratable fields
  hw/nubus/mac-nubus-bridge: Mark the device with no migratable fields
  hw/sparc64/sun4u: Mark devices with no migratable fields
  hw/pci-host/gpex: Mark device with no migratable fields
  hw/core/qdev: Display warning for devices missing migration state
  stubs/vmstate: Add VMSTATE_END_OF_LIST to vmstate_user_mode_cpu_dummy
  migration/vmstate: Simplify vmstate for user-mode CPU

 configure                    | 10 ++++++++++
 meson.build                  |  1 +
 hw/usb/hcd-ohci.h            |  2 ++
 include/hw/arm/bcm2836.h     |  5 +++--
 include/hw/arm/msf2-soc.h    | 11 ++++++-----
 include/hw/core/cpu.h        |  2 +-
 include/hw/cpu/a9mpcore.h    |  3 ++-
 include/hw/qdev-core.h       |  2 ++
 include/migration/vmstate.h  |  2 +-
 hw/arm/armv7m.c              |  1 +
 hw/arm/aspeed_soc.c          |  1 +
 hw/arm/bcm2835_peripherals.c |  1 +
 hw/arm/bcm2836.c             |  1 +
 hw/arm/msf2-soc.c            |  1 +
 hw/core/qdev.c               |  8 ++++++++
 hw/core/split-irq.c          |  1 +
 hw/cpu/a9mpcore.c            |  1 +
 hw/cpu/cluster.c             |  1 +
 hw/intc/arm_gicv2m.c         |  2 ++
 hw/misc/armsse-cpuid.c       |  1 +
 hw/misc/iotkit-sysinfo.c     |  1 +
 hw/misc/unimp.c              |  1 +
 hw/nubus/mac-nubus-bridge.c  |  1 +
 hw/pci-host/gpex.c           |  1 +
 hw/sparc64/sun4u.c           |  6 +++++-
 hw/usb/hcd-ohci.c            |  1 +
 migration/vmstate.c          |  7 +++++++
 stubs/vmstate.c              |  7 ++++++-
 28 files changed, 70 insertions(+), 12 deletions(-)

-- 
2.26.2

[RFC PATCH v2 01/20] migration/vmstate: Restrict vmstate_dummy to user-mode

[Philippe Mathieu-Daudé]

'vmstate_dummy' is special and only used for user-mode. Rename
it to something more specific.
It was introduced restricted to user-mode in commit c71c3e99b8
("Add a vmstate_dummy struct for CONFIG_USER_ONLY") but this
restriction was later removed in commit 6afc14e92ac ("migration:
Fix warning caused by missing declaration of vmstate_dummy").
Avoid the missing declaration warning by adding a stub for the
symbol, and restore the #ifdef'ry.

Suggested-by: Daniel Berrangé <berrange@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/core/cpu.h       | 2 +-
 include/migration/vmstate.h | 4 +++-
 stubs/vmstate.c             | 4 +++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 140fa32a5e3..c79a58db9b9 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -1132,7 +1132,7 @@ bool target_words_bigendian(void);
 #ifdef CONFIG_SOFTMMU
 extern const VMStateDescription vmstate_cpu_common;
 #else
-#define vmstate_cpu_common vmstate_dummy
+#define vmstate_cpu_common vmstate_user_mode_cpu_dummy
 #endif
 
 #define VMSTATE_CPU() {                                                     \
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 075ee800960..dda65c9987d 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -194,7 +194,9 @@ struct VMStateDescription {
     const VMStateDescription **subsections;
 };
 
-extern const VMStateDescription vmstate_dummy;
+#if defined(CONFIG_USER_ONLY)
+extern const VMStateDescription vmstate_user_mode_cpu_dummy;
+#endif
 
 extern const VMStateInfo vmstate_info_bool;
 
diff --git a/stubs/vmstate.c b/stubs/vmstate.c
index cc4fe41dfc2..8da777a1fb4 100644
--- a/stubs/vmstate.c
+++ b/stubs/vmstate.c
@@ -1,7 +1,9 @@
 #include "qemu/osdep.h"
 #include "migration/vmstate.h"
 
-const VMStateDescription vmstate_dummy = {};
+#if defined(CONFIG_USER_ONLY)
+const VMStateDescription vmstate_user_mode_cpu_dummy = {};
+#endif
 
 int vmstate_register_with_alias_id(VMStateIf *obj,
                                    uint32_t instance_id,
-- 
2.26.2

[RFC PATCH v2 02/20] hw/core/qdev: Add vmstate_qdev_no_state_to_migrate

[Philippe Mathieu-Daudé]

Add vmstate_qdev_no_state_to_migrate, which is simply a
pointer to vmstate_no_state_to_migrate. This way all
qdev devices (including "hw/qdev-core.h") don't have to
include "migration/vmstate.h".

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Unresolved issues:

https://www.mail-archive.com/qemu-devel@nongnu.org/msg721695.html
Peter:
> Does this definitely not put any data into the migration stream?
> We don't want to change what's on the wire for machines that
> use devices that start using this. (If it does by default, it
> would be easy to make the migration code special case the
> magic symbol to act like "no vmsd specified").

https://www.mail-archive.com/qemu-devel@nongnu.org/msg727634.html
Dave:
> I'd need to test it to be sure, but I think if we added a .needed
> to vmstate_no_state_to_migrate with a function that always returned
> false, then I think the stream would stay unchanged.
---
 include/hw/qdev-core.h      | 2 ++
 include/migration/vmstate.h | 1 +
 hw/core/qdev.c              | 3 +++
 migration/vmstate.c         | 7 +++++++
 stubs/vmstate.c             | 7 +++++++
 5 files changed, 20 insertions(+)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index bafc311bfa1..d2c7a46e6a2 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -140,6 +140,8 @@ struct DeviceClass {
     const char *bus_type;
 };
 
+extern const VMStateDescription *vmstate_qdev_no_state_to_migrate;
+
 typedef struct NamedGPIOList NamedGPIOList;
 
 struct NamedGPIOList {
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index dda65c9987d..50559598eac 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -197,6 +197,7 @@ struct VMStateDescription {
 #if defined(CONFIG_USER_ONLY)
 extern const VMStateDescription vmstate_user_mode_cpu_dummy;
 #endif
+extern const VMStateDescription vmstate_no_state_to_migrate;
 
 extern const VMStateInfo vmstate_info_bool;
 
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index cefc5eaa0a9..f0d0afd438d 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -44,6 +44,9 @@
 static bool qdev_hot_added = false;
 bool qdev_hot_removed = false;
 
+const VMStateDescription *vmstate_qdev_no_state_to_migrate =
+        &vmstate_no_state_to_migrate;
+
 const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
 {
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 05f87cdddc5..2c373774dfa 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -20,6 +20,13 @@
 #include "qemu/error-report.h"
 #include "trace.h"
 
+const VMStateDescription vmstate_no_state_to_migrate = {
+    .name = "empty-state",
+    .fields = (VMStateField[]) {
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
                                    void *opaque, JSONWriter *vmdesc);
 static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
diff --git a/stubs/vmstate.c b/stubs/vmstate.c
index 8da777a1fb4..f561f9f39bd 100644
--- a/stubs/vmstate.c
+++ b/stubs/vmstate.c
@@ -5,6 +5,13 @@
 const VMStateDescription vmstate_user_mode_cpu_dummy = {};
 #endif
 
+const VMStateDescription vmstate_no_state_to_migrate = {
+    .name = "empty-state",
+    .fields = (VMStateField[]) {
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 int vmstate_register_with_alias_id(VMStateIf *obj,
                                    uint32_t instance_id,
                                    const VMStateDescription *vmsd,
-- 
2.26.2

[RFC PATCH v2 03/20] hw/arm/armv7m: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

The TYPE_BITBAND device doesn't have fields to migrate.
Be explicit by using vmstate_qdev_no_state_to_migrate.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
v2: Reworded (Peter)
---
 hw/arm/armv7m.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 8224d4ade9f..41ac1b88ab4 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -347,6 +347,7 @@ static void bitband_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = bitband_realize;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
     device_class_set_props(dc, bitband_properties);
 }
 
-- 
2.26.2

[RFC PATCH v2 04/20] hw/arm/aspeed_soc: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/aspeed_soc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 7eefd54ac07..b503d32fef6 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -407,6 +407,7 @@ static void aspeed_soc_class_init(ObjectClass *oc, void *data)
     dc->realize = aspeed_soc_realize;
     /* Reason: Uses serial_hds and nd_table in realize() directly */
     dc->user_creatable = false;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
     device_class_set_props(dc, aspeed_soc_properties);
 }
 
-- 
2.26.2

[RFC PATCH v2 05/20] hw/arm/bcm283x: Mark devices with no migratable fields

[Philippe Mathieu-Daudé]

These devices don't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Add a more descriptive comment to keep a clear separation
between static property vs runtime changeable.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/arm/bcm2836.h     | 5 +++--
 hw/arm/bcm2835_peripherals.c | 1 +
 hw/arm/bcm2836.c             | 1 +
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/hw/arm/bcm2836.h b/include/hw/arm/bcm2836.h
index 6f90cabfa3a..becb6cfd0a7 100644
--- a/include/hw/arm/bcm2836.h
+++ b/include/hw/arm/bcm2836.h
@@ -35,13 +35,14 @@ struct BCM283XState {
     DeviceState parent_obj;
     /*< public >*/
 
-    uint32_t enabled_cpus;
-
     struct {
         ARMCPU core;
     } cpu[BCM283X_NCPUS];
     BCM2836ControlState control;
     BCM2835PeripheralState peripherals;
+
+    /* Properties */
+    uint32_t enabled_cpus;
 };
 
 #endif /* BCM2836_H */
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index dcff13433e5..8cf85f028fd 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -386,6 +386,7 @@ static void bcm2835_peripherals_class_init(ObjectClass *oc, void *data)
     DeviceClass *dc = DEVICE_CLASS(oc);
 
     dc->realize = bcm2835_peripherals_realize;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
 }
 
 static const TypeInfo bcm2835_peripherals_type_info = {
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index de7ade2878e..d2de99147cc 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -176,6 +176,7 @@ static void bcm283x_class_init(ObjectClass *oc, void *data)
 
     /* Reason: Must be wired up in code (see raspi_init() function) */
     dc->user_creatable = false;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
 }
 
 static void bcm2835_class_init(ObjectClass *oc, void *data)
-- 
2.26.2

[RFC PATCH v2 06/20] hw/arm/msf2-soc: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Add a more descriptive comment to keep a clear separation
between static property vs runtime changeable.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/arm/msf2-soc.h | 11 ++++++-----
 hw/arm/msf2-soc.c         |  1 +
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/hw/arm/msf2-soc.h b/include/hw/arm/msf2-soc.h
index d4061846855..41a328c77f9 100644
--- a/include/hw/arm/msf2-soc.h
+++ b/include/hw/arm/msf2-soc.h
@@ -52,6 +52,12 @@ struct MSF2State {
 
     ARMv7MState armv7m;
 
+    MSF2SysregState sysreg;
+    MSSTimerState timer;
+    MSSSpiState spi[MSF2_NUM_SPIS];
+    MSF2EmacState emac;
+
+    /* Properties */
     char *cpu_type;
     char *part_name;
     uint64_t envm_size;
@@ -60,11 +66,6 @@ struct MSF2State {
     uint32_t m3clk;
     uint8_t apb0div;
     uint8_t apb1div;
-
-    MSF2SysregState sysreg;
-    MSSTimerState timer;
-    MSSSpiState spi[MSF2_NUM_SPIS];
-    MSF2EmacState emac;
 };
 
 #endif
diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
index d2c29e82d13..2d163710f54 100644
--- a/hw/arm/msf2-soc.c
+++ b/hw/arm/msf2-soc.c
@@ -224,6 +224,7 @@ static void m2sxxx_soc_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = m2sxxx_soc_realize;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
     device_class_set_props(dc, m2sxxx_soc_properties);
 }
 
-- 
2.26.2

[RFC PATCH v2 07/20] hw/core/split-irq: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/core/split-irq.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/core/split-irq.c b/hw/core/split-irq.c
index 3b90af2e8f9..a7072f922cd 100644
--- a/hw/core/split-irq.c
+++ b/hw/core/split-irq.c
@@ -71,6 +71,7 @@ static void split_irq_class_init(ObjectClass *klass, void *data)
     /* No state to reset or migrate */
     device_class_set_props(dc, split_irq_properties);
     dc->realize = split_irq_realize;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
 
     /* Reason: Needs to be wired up to work */
     dc->user_creatable = false;
-- 
2.26.2

[RFC PATCH v2 08/20] hw/cpu/a9mpcore: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Add a more descriptive comment to keep a clear separation
between static property vs runtime changeable.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/cpu/a9mpcore.h | 3 ++-
 hw/cpu/a9mpcore.c         | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/hw/cpu/a9mpcore.h b/include/hw/cpu/a9mpcore.h
index e0396ab6af7..234ac13be2c 100644
--- a/include/hw/cpu/a9mpcore.h
+++ b/include/hw/cpu/a9mpcore.h
@@ -25,10 +25,11 @@ struct A9MPPrivState {
     SysBusDevice parent_obj;
     /*< public >*/
 
+    /* Properties */
     uint32_t num_cpu;
-    MemoryRegion container;
     uint32_t num_irq;
 
+    MemoryRegion container;
     A9SCUState scu;
     GICState gic;
     A9GTimerState gtimer;
diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
index d03f57e579b..2e1d2d46b5b 100644
--- a/hw/cpu/a9mpcore.c
+++ b/hw/cpu/a9mpcore.c
@@ -175,6 +175,7 @@ static void a9mp_priv_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = a9mp_priv_realize;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
     device_class_set_props(dc, a9mp_priv_properties);
 }
 
-- 
2.26.2

[RFC PATCH v2 09/20] hw/cpu/cluster: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/cpu/cluster.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/cpu/cluster.c b/hw/cpu/cluster.c
index e444b7c29d1..95653a643ad 100644
--- a/hw/cpu/cluster.c
+++ b/hw/cpu/cluster.c
@@ -80,6 +80,7 @@ static void cpu_cluster_class_init(ObjectClass *klass, void *data)
 
     device_class_set_props(dc, cpu_cluster_properties);
     dc->realize = cpu_cluster_realize;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
 
     /* This is not directly for users, CPU children must be attached by code */
     dc->user_creatable = false;
-- 
2.26.2

[RFC PATCH v2 10/20] hw/usb/hcd-ohci: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Add a more descriptive comment to keep a clear separation
between static property vs runtime changeable.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/usb/hcd-ohci.h | 2 ++
 hw/usb/hcd-ohci.c | 1 +
 2 files changed, 3 insertions(+)

diff --git a/hw/usb/hcd-ohci.h b/hw/usb/hcd-ohci.h
index 11ac57058d1..fd4842a352f 100644
--- a/hw/usb/hcd-ohci.h
+++ b/hw/usb/hcd-ohci.h
@@ -101,6 +101,8 @@ struct OHCISysBusState {
     /*< public >*/
 
     OHCIState ohci;
+
+    /* Properties */
     char *masterbus;
     uint32_t num_ports;
     uint32_t firstport;
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index f8c64c8b95b..302aab30992 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -2007,6 +2007,7 @@ static void ohci_sysbus_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = ohci_realize_pxa;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
     dc->desc = "OHCI USB Controller";
     device_class_set_props(dc, ohci_sysbus_properties);
-- 
2.26.2

[RFC PATCH v2 11/20] hw/intc/arm_gicv2m: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Add a more descriptive comment to keep a clear separation
between static property vs runtime changeable.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/intc/arm_gicv2m.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/intc/arm_gicv2m.c b/hw/intc/arm_gicv2m.c
index d564b857eba..664cc9fb032 100644
--- a/hw/intc/arm_gicv2m.c
+++ b/hw/intc/arm_gicv2m.c
@@ -55,6 +55,7 @@ struct ARMGICv2mState {
     MemoryRegion iomem;
     qemu_irq spi[GICV2M_NUM_SPI_MAX];
 
+    /* Properties */
     uint32_t base_spi;
     uint32_t num_spi;
 };
@@ -182,6 +183,7 @@ static void gicv2m_class_init(ObjectClass *klass, void *data)
 
     device_class_set_props(dc, gicv2m_properties);
     dc->realize = gicv2m_realize;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
 }
 
 static const TypeInfo gicv2m_info = {
-- 
2.26.2

[RFC PATCH v2 12/20] hw/misc/armsse-cpuid: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/armsse-cpuid.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/misc/armsse-cpuid.c b/hw/misc/armsse-cpuid.c
index d58138dc28c..61251d538b9 100644
--- a/hw/misc/armsse-cpuid.c
+++ b/hw/misc/armsse-cpuid.c
@@ -115,6 +115,7 @@ static void armsse_cpuid_class_init(ObjectClass *klass, void *data)
      * This device has no guest-modifiable state and so it
      * does not need a reset function or VMState.
      */
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
 
     device_class_set_props(dc, armsse_cpuid_props);
 }
-- 
2.26.2

[RFC PATCH v2 13/20] hw/misc/iotkit-sysinfo: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/iotkit-sysinfo.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/misc/iotkit-sysinfo.c b/hw/misc/iotkit-sysinfo.c
index b2dcfc4376c..8bb9a2ef8b2 100644
--- a/hw/misc/iotkit-sysinfo.c
+++ b/hw/misc/iotkit-sysinfo.c
@@ -120,6 +120,7 @@ static void iotkit_sysinfo_class_init(ObjectClass *klass, void *data)
      * This device has no guest-modifiable state and so it
      * does not need a reset function or VMState.
      */
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
 
     device_class_set_props(dc, iotkit_sysinfo_props);
 }
-- 
2.26.2

[RFC PATCH v2 14/20] hw/misc/unimp: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/unimp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/misc/unimp.c b/hw/misc/unimp.c
index 6cfc5727f0b..e5ede95c124 100644
--- a/hw/misc/unimp.c
+++ b/hw/misc/unimp.c
@@ -81,6 +81,7 @@ static void unimp_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = unimp_realize;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
     device_class_set_props(dc, unimp_properties);
 }
 
-- 
2.26.2

[RFC PATCH v2 15/20] hw/nubus/mac-nubus-bridge: Mark the device with no migratable fields

[Philippe Mathieu-Daudé]

This device doesn't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/nubus/mac-nubus-bridge.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/nubus/mac-nubus-bridge.c b/hw/nubus/mac-nubus-bridge.c
index 7c329300b82..ede36ccc5dd 100644
--- a/hw/nubus/mac-nubus-bridge.c
+++ b/hw/nubus/mac-nubus-bridge.c
@@ -27,6 +27,7 @@ static void mac_nubus_bridge_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->desc = "Nubus bridge";
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
 }
 
 static const TypeInfo mac_nubus_bridge_info = {
-- 
2.26.2

[RFC PATCH v2 16/20] hw/sparc64/sun4u: Mark devices with no migratable fields

[Philippe Mathieu-Daudé]

These devices don't have fields to migrate. Be explicit
by using vmstate_qdev_no_state_to_migrate.

Add a more descriptive comment to keep a clear separation
between static property vs runtime changeable.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sparc64/sun4u.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 0fa13a73302..fdf0aa875be 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -84,12 +84,15 @@ struct hwdef {
 struct EbusState {
     /*< private >*/
     PCIDevice parent_obj;
+    /*< public >*/
 
     ISABus *isa_bus;
     qemu_irq isa_bus_irqs[ISA_NUM_IRQS];
-    uint64_t console_serial_base;
     MemoryRegion bar0;
     MemoryRegion bar1;
+
+    /* Properties */
+    uint64_t console_serial_base;
 };
 
 #define TYPE_EBUS "ebus"
@@ -386,6 +389,7 @@ static void ebus_class_init(ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_SUN_EBUS;
     k->revision = 0x01;
     k->class_id = PCI_CLASS_BRIDGE_OTHER;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
     device_class_set_props(dc, ebus_properties);
 }
 
-- 
2.26.2

[RFC PATCH v2 17/20] hw/pci-host/gpex: Mark device with no migratable fields

[Philippe Mathieu-Daudé]

TYPE_GPEX_HOST does not have internal state to migrate.
Its only interesting state is in the GPEXRootState, which
is a TYPE_GPEX_ROOT_DEVICE which migrates itself.
Explicit there is nothing to migrate by using the special
vmstate_qdev_no_state_to_migrate.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/pci-host/gpex.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
index 2bdbe7b4561..2565dc27ae4 100644
--- a/hw/pci-host/gpex.c
+++ b/hw/pci-host/gpex.c
@@ -115,6 +115,7 @@ static void gpex_host_class_init(ObjectClass *klass, void *data)
 
     hc->root_bus_path = gpex_host_root_bus_path;
     dc->realize = gpex_host_realize;
+    dc->vmsd = vmstate_qdev_no_state_to_migrate;
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
     dc->fw_name = "pci";
 }
-- 
2.26.2

[RFC PATCH v2 18/20] hw/core/qdev: Display warning for devices missing migration state

[Philippe Mathieu-Daudé]

When built with --enable-qdev-debug, QEMU displays warnings
listing devices missing migration state:

  $ qemu-system-arm -S -M spitz
  qemu-system-arm: warning: missing migration state for type: 'pxa270-c0-arm-cpu'
  qemu-system-arm: warning: missing migration state for type: 'serial'
  qemu-system-arm: warning: missing migration state for type: 'pxa2xx-pcmcia'
  qemu-system-arm: warning: missing migration state for type: 'pxa2xx-pcmcia'
  qemu-system-arm: warning: missing migration state for type: 'pxa2xx-i2c-slave'
  qemu-system-arm: warning: missing migration state for type: 'pxa2xx-i2c-slave'
  qemu-system-arm: warning: missing migration state for type: 'ads7846'
  qemu-system-arm: warning: missing migration state for type: 'max1111'

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Unresolved issue:

https://www.mail-archive.com/qemu-devel@nongnu.org/msg721700.html
Peter:
> I think where we'd like to get to is installing a migration
> blocker if the machine has any devices which don't have a vmsd.
> But for that we'd need to be pretty sure we'd got all the devices
> on machines where we care about migration, and we're clearly a
> fair way from that (eg we need to do something about the
> devices like the CPU which don't have a vmsd but handle their
> migration some other way so they don't trigger the condition
> for warning/migration-blocker).
---
 configure      | 10 ++++++++++
 meson.build    |  1 +
 hw/core/qdev.c |  5 +++++
 3 files changed, 16 insertions(+)

diff --git a/configure b/configure
index 155dda124c2..984befbb99d 100755
--- a/configure
+++ b/configure
@@ -383,6 +383,7 @@ blobs="true"
 pkgversion=""
 pie=""
 qom_cast_debug="yes"
+qdev_debug="no"
 trace_backends="log"
 trace_file="trace"
 spice="$default_feature"
@@ -1005,6 +1006,10 @@ for opt do
   ;;
   --enable-qom-cast-debug) qom_cast_debug="yes"
   ;;
+  --disable-qdev-debug) qdev_debug="no"
+  ;;
+  --enable-qdev-debug) qdev_debug="yes"
+  ;;
   --disable-virtfs) virtfs="disabled"
   ;;
   --enable-virtfs) virtfs="enabled"
@@ -1048,6 +1053,7 @@ for opt do
       debug="yes"
       strip_opt="no"
       fortify_source="no"
+      qdev_debug="yes"
   ;;
   --enable-sanitizers) sanitizers="yes"
   ;;
@@ -5912,6 +5918,10 @@ if test "$qom_cast_debug" = "yes" ; then
   echo "CONFIG_QOM_CAST_DEBUG=y" >> $config_host_mak
 fi
 
+if test "$qdev_debug" = "yes" ; then
+  echo "CONFIG_QDEV_DEBUG=y" >> $config_host_mak
+fi
+
 echo "CONFIG_COROUTINE_BACKEND=$coroutine" >> $config_host_mak
 if test "$coroutine_pool" = "yes" ; then
   echo "CONFIG_COROUTINE_POOL=1" >> $config_host_mak
diff --git a/meson.build b/meson.build
index 3d889857a09..545c8f9f88b 100644
--- a/meson.build
+++ b/meson.build
@@ -2472,6 +2472,7 @@
 summary_info += {'TPM support':       config_host.has_key('CONFIG_TPM')}
 summary_info += {'libssh support':    config_host.has_key('CONFIG_LIBSSH')}
 summary_info += {'QOM debugging':     config_host.has_key('CONFIG_QOM_CAST_DEBUG')}
+summary_info += {'QDEV debugging':    config_host.has_key('CONFIG_QDEV_DEBUG')}
 summary_info += {'Live block migration': config_host.has_key('CONFIG_LIVE_BLOCK_MIGRATION')}
 summary_info += {'lzo support':       lzo.found()}
 summary_info += {'snappy support':    snappy.found()}
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index f0d0afd438d..9a73a242fa4 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -792,6 +792,11 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
                                                &local_err) < 0) {
                 goto post_realize_fail;
             }
+        } else {
+#ifdef CONFIG_QDEV_DEBUG
+            warn_report("missing migration state for type: '%s'",
+                        object_get_typename(OBJECT(dev)));
+#endif
         }
 
         /*
-- 
2.26.2

[RFC PATCH v2 19/20] stubs/vmstate: Add VMSTATE_END_OF_LIST to vmstate_user_mode_cpu_dummy

[Philippe Mathieu-Daudé]

Add a name and end marker to the vmstate_user_mode_cpu_dummy variable.

Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 stubs/vmstate.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/stubs/vmstate.c b/stubs/vmstate.c
index f561f9f39bd..1d0e03e233b 100644
--- a/stubs/vmstate.c
+++ b/stubs/vmstate.c
@@ -2,7 +2,12 @@
 #include "migration/vmstate.h"
 
 #if defined(CONFIG_USER_ONLY)
-const VMStateDescription vmstate_user_mode_cpu_dummy = {};
+const VMStateDescription vmstate_user_mode_cpu_dummy = {
+    .name = "cpu_common_user",
+    .fields = (VMStateField[]) {
+        VMSTATE_END_OF_LIST()
+    },
+};
 #endif
 
 const VMStateDescription vmstate_no_state_to_migrate = {
-- 
2.26.2

[RFC PATCH v2 20/20] migration/vmstate: Simplify vmstate for user-mode CPU

[Philippe Mathieu-Daudé]

User-mode wants an empty vmstate for the CPUs. We can
use the generic vmstate_no_state_to_migrate object which
is the same.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/core/cpu.h       | 2 +-
 include/migration/vmstate.h | 3 ---
 stubs/vmstate.c             | 9 ---------
 3 files changed, 1 insertion(+), 13 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index c79a58db9b9..01e75cc7403 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -1132,7 +1132,7 @@ bool target_words_bigendian(void);
 #ifdef CONFIG_SOFTMMU
 extern const VMStateDescription vmstate_cpu_common;
 #else
-#define vmstate_cpu_common vmstate_user_mode_cpu_dummy
+#define vmstate_cpu_common vmstate_no_state_to_migrate
 #endif
 
 #define VMSTATE_CPU() {                                                     \
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 50559598eac..dfe20b5caa1 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -194,9 +194,6 @@ struct VMStateDescription {
     const VMStateDescription **subsections;
 };
 
-#if defined(CONFIG_USER_ONLY)
-extern const VMStateDescription vmstate_user_mode_cpu_dummy;
-#endif
 extern const VMStateDescription vmstate_no_state_to_migrate;
 
 extern const VMStateInfo vmstate_info_bool;
diff --git a/stubs/vmstate.c b/stubs/vmstate.c
index 1d0e03e233b..c360a929f60 100644
--- a/stubs/vmstate.c
+++ b/stubs/vmstate.c
@@ -1,15 +1,6 @@
 #include "qemu/osdep.h"
 #include "migration/vmstate.h"
 
-#if defined(CONFIG_USER_ONLY)
-const VMStateDescription vmstate_user_mode_cpu_dummy = {
-    .name = "cpu_common_user",
-    .fields = (VMStateField[]) {
-        VMSTATE_END_OF_LIST()
-    },
-};
-#endif
-
 const VMStateDescription vmstate_no_state_to_migrate = {
     .name = "empty-state",
     .fields = (VMStateField[]) {
-- 
2.26.2

Re: [RFC PATCH v2 16/20] hw/sparc64/sun4u: Mark devices with no migratable fields

[Artyom Tarasenko]

[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]

вс, 17 янв. 2021 г., 20:26 Philippe Mathieu-Daudé <f4bug@amsat.org>:

> These devices don't have fields to migrate. Be explicit
> by using vmstate_qdev_no_state_to_migrate.
>
> Add a more descriptive comment to keep a clear separation
> between static property vs runtime changeable.
>

Nice, thanks for this!


> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>

Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>

---
>  hw/sparc64/sun4u.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 0fa13a73302..fdf0aa875be 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -84,12 +84,15 @@ struct hwdef {
>  struct EbusState {
>      /*< private >*/
>      PCIDevice parent_obj;
> +    /*< public >*/
>
>      ISABus *isa_bus;
>      qemu_irq isa_bus_irqs[ISA_NUM_IRQS];
> -    uint64_t console_serial_base;
>      MemoryRegion bar0;
>      MemoryRegion bar1;
> +
> +    /* Properties */
> +    uint64_t console_serial_base;
>  };
>
>  #define TYPE_EBUS "ebus"
> @@ -386,6 +389,7 @@ static void ebus_class_init(ObjectClass *klass, void
> *data)
>      k->device_id = PCI_DEVICE_ID_SUN_EBUS;
>      k->revision = 0x01;
>      k->class_id = PCI_CLASS_BRIDGE_OTHER;
> +    dc->vmsd = vmstate_qdev_no_state_to_migrate;
>      device_class_set_props(dc, ebus_properties);
>  }
>
> --
> 2.26.2
>
>

[-- Attachment #2: Type: text/html, Size: 2655 bytes --]

Re: [RFC PATCH v2 01/20] migration/vmstate: Restrict vmstate_dummy to user-mode

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (f4bug@amsat.org) wrote:
> 'vmstate_dummy' is special and only used for user-mode. Rename
> it to something more specific.
> It was introduced restricted to user-mode in commit c71c3e99b8
> ("Add a vmstate_dummy struct for CONFIG_USER_ONLY") but this
> restriction was later removed in commit 6afc14e92ac ("migration:
> Fix warning caused by missing declaration of vmstate_dummy").
> Avoid the missing declaration warning by adding a stub for the
> symbol, and restore the #ifdef'ry.
> 
> Suggested-by: Daniel Berrangé <berrange@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  include/hw/core/cpu.h       | 2 +-
>  include/migration/vmstate.h | 4 +++-
>  stubs/vmstate.c             | 4 +++-
>  3 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 140fa32a5e3..c79a58db9b9 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -1132,7 +1132,7 @@ bool target_words_bigendian(void);
>  #ifdef CONFIG_SOFTMMU
>  extern const VMStateDescription vmstate_cpu_common;
>  #else
> -#define vmstate_cpu_common vmstate_dummy
> +#define vmstate_cpu_common vmstate_user_mode_cpu_dummy
>  #endif
>  
>  #define VMSTATE_CPU() {                                                     \
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 075ee800960..dda65c9987d 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -194,7 +194,9 @@ struct VMStateDescription {
>      const VMStateDescription **subsections;
>  };
>  
> -extern const VMStateDescription vmstate_dummy;
> +#if defined(CONFIG_USER_ONLY)
> +extern const VMStateDescription vmstate_user_mode_cpu_dummy;
> +#endif
>  
>  extern const VMStateInfo vmstate_info_bool;
>  
> diff --git a/stubs/vmstate.c b/stubs/vmstate.c
> index cc4fe41dfc2..8da777a1fb4 100644
> --- a/stubs/vmstate.c
> +++ b/stubs/vmstate.c
> @@ -1,7 +1,9 @@
>  #include "qemu/osdep.h"
>  #include "migration/vmstate.h"
>  
> -const VMStateDescription vmstate_dummy = {};
> +#if defined(CONFIG_USER_ONLY)
> +const VMStateDescription vmstate_user_mode_cpu_dummy = {};
> +#endif
>  
>  int vmstate_register_with_alias_id(VMStateIf *obj,
>                                     uint32_t instance_id,
> -- 
> 2.26.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [RFC PATCH v2 15/20] hw/nubus/mac-nubus-bridge: Mark the device with no migratable fields

[Laurent Vivier]

Le 17/01/2021 à 20:24, Philippe Mathieu-Daudé a écrit :
> This device doesn't have fields to migrate. Be explicit
> by using vmstate_qdev_no_state_to_migrate.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/nubus/mac-nubus-bridge.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/nubus/mac-nubus-bridge.c b/hw/nubus/mac-nubus-bridge.c
> index 7c329300b82..ede36ccc5dd 100644
> --- a/hw/nubus/mac-nubus-bridge.c
> +++ b/hw/nubus/mac-nubus-bridge.c
> @@ -27,6 +27,7 @@ static void mac_nubus_bridge_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
>      dc->desc = "Nubus bridge";
> +    dc->vmsd = vmstate_qdev_no_state_to_migrate;
>  }
>  
>  static const TypeInfo mac_nubus_bridge_info = {
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [RFC PATCH v2 10/20] hw/usb/hcd-ohci: Mark the device with no migratable fields

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (f4bug@amsat.org) wrote:
> This device doesn't have fields to migrate. Be explicit
> by using vmstate_qdev_no_state_to_migrate.
> 
> Add a more descriptive comment to keep a clear separation
> between static property vs runtime changeable.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

OK,

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


although I think it's quite interesting; I think we have
a base class which has data to migrate which expects any
child classes to migrate it's data; so marking it as
not actually having any state is not quite right.

> ---
>  hw/usb/hcd-ohci.h | 2 ++
>  hw/usb/hcd-ohci.c | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/hw/usb/hcd-ohci.h b/hw/usb/hcd-ohci.h
> index 11ac57058d1..fd4842a352f 100644
> --- a/hw/usb/hcd-ohci.h
> +++ b/hw/usb/hcd-ohci.h
> @@ -101,6 +101,8 @@ struct OHCISysBusState {
>      /*< public >*/
>  
>      OHCIState ohci;
> +
> +    /* Properties */
>      char *masterbus;
>      uint32_t num_ports;
>      uint32_t firstport;
> diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> index f8c64c8b95b..302aab30992 100644
> --- a/hw/usb/hcd-ohci.c
> +++ b/hw/usb/hcd-ohci.c
> @@ -2007,6 +2007,7 @@ static void ohci_sysbus_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
>      dc->realize = ohci_realize_pxa;
> +    dc->vmsd = vmstate_qdev_no_state_to_migrate;
>      set_bit(DEVICE_CATEGORY_USB, dc->categories);
>      dc->desc = "OHCI USB Controller";
>      device_class_set_props(dc, ohci_sysbus_properties);
> -- 
> 2.26.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [RFC PATCH v2 02/20] hw/core/qdev: Add vmstate_qdev_no_state_to_migrate

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (f4bug@amsat.org) wrote:
> Add vmstate_qdev_no_state_to_migrate, which is simply a
> pointer to vmstate_no_state_to_migrate. This way all
> qdev devices (including "hw/qdev-core.h") don't have to
> include "migration/vmstate.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Unresolved issues:
> 
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg721695.html
> Peter:
> > Does this definitely not put any data into the migration stream?
> > We don't want to change what's on the wire for machines that
> > use devices that start using this. (If it does by default, it
> > would be easy to make the migration code special case the
> > magic symbol to act like "no vmsd specified").
> 
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg727634.html
> Dave:
> > I'd need to test it to be sure, but I think if we added a .needed
> > to vmstate_no_state_to_migrate with a function that always returned
> > false, then I think the stream would stay unchanged.

Yes I still think you need that; if you only use this for base classes
rather than devices themselves you're probably OK; but if you use it on
a device I think you'll end up with an empty-state entry in the
migration stream.

Dave

> ---
>  include/hw/qdev-core.h      | 2 ++
>  include/migration/vmstate.h | 1 +
>  hw/core/qdev.c              | 3 +++
>  migration/vmstate.c         | 7 +++++++
>  stubs/vmstate.c             | 7 +++++++
>  5 files changed, 20 insertions(+)
> 
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index bafc311bfa1..d2c7a46e6a2 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -140,6 +140,8 @@ struct DeviceClass {
>      const char *bus_type;
>  };
>  
> +extern const VMStateDescription *vmstate_qdev_no_state_to_migrate;
> +
>  typedef struct NamedGPIOList NamedGPIOList;
>  
>  struct NamedGPIOList {
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index dda65c9987d..50559598eac 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -197,6 +197,7 @@ struct VMStateDescription {
>  #if defined(CONFIG_USER_ONLY)
>  extern const VMStateDescription vmstate_user_mode_cpu_dummy;
>  #endif
> +extern const VMStateDescription vmstate_no_state_to_migrate;
>  
>  extern const VMStateInfo vmstate_info_bool;
>  
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index cefc5eaa0a9..f0d0afd438d 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -44,6 +44,9 @@
>  static bool qdev_hot_added = false;
>  bool qdev_hot_removed = false;
>  
> +const VMStateDescription *vmstate_qdev_no_state_to_migrate =
> +        &vmstate_no_state_to_migrate;
> +
>  const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
>  {
>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index 05f87cdddc5..2c373774dfa 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -20,6 +20,13 @@
>  #include "qemu/error-report.h"
>  #include "trace.h"
>  
> +const VMStateDescription vmstate_no_state_to_migrate = {
> +    .name = "empty-state",
> +    .fields = (VMStateField[]) {
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
>                                     void *opaque, JSONWriter *vmdesc);
>  static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
> diff --git a/stubs/vmstate.c b/stubs/vmstate.c
> index 8da777a1fb4..f561f9f39bd 100644
> --- a/stubs/vmstate.c
> +++ b/stubs/vmstate.c
> @@ -5,6 +5,13 @@
>  const VMStateDescription vmstate_user_mode_cpu_dummy = {};
>  #endif
>  
> +const VMStateDescription vmstate_no_state_to_migrate = {
> +    .name = "empty-state",
> +    .fields = (VMStateField[]) {
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  int vmstate_register_with_alias_id(VMStateIf *obj,
>                                     uint32_t instance_id,
>                                     const VMStateDescription *vmsd,
> -- 
> 2.26.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [RFC PATCH v2 02/20] hw/core/qdev: Add vmstate_qdev_no_state_to_migrate

[Peter Maydell]

On Sun, 17 Jan 2021 at 19:25, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Add vmstate_qdev_no_state_to_migrate, which is simply a
> pointer to vmstate_no_state_to_migrate. This way all
> qdev devices (including "hw/qdev-core.h") don't have to
> include "migration/vmstate.h".
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Unresolved issues:
>
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg721695.html
> Peter:
> > Does this definitely not put any data into the migration stream?
> > We don't want to change what's on the wire for machines that
> > use devices that start using this. (If it does by default, it
> > would be easy to make the migration code special case the
> > magic symbol to act like "no vmsd specified").
>
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg727634.html
> Dave:
> > I'd need to test it to be sure, but I think if we added a .needed
> > to vmstate_no_state_to_migrate with a function that always returned
> > false, then I think the stream would stay unchanged.
> ---

It should be easy to test -- just do a 'savevm' of a running
system with a machine model that uses one of the devices that
has been marked as "no state to migrate", then apply the patchseries,
and see if 'loadvm' works or not.

thanks
-- PMM

Re: [RFC PATCH v2 01/20] migration/vmstate: Restrict vmstate_dummy to user-mode

[Peter Maydell]

On Sun, 17 Jan 2021 at 19:24, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> 'vmstate_dummy' is special and only used for user-mode. Rename
> it to something more specific.
> It was introduced restricted to user-mode in commit c71c3e99b8
> ("Add a vmstate_dummy struct for CONFIG_USER_ONLY") but this
> restriction was later removed in commit 6afc14e92ac ("migration:
> Fix warning caused by missing declaration of vmstate_dummy").
> Avoid the missing declaration warning by adding a stub for the
> symbol, and restore the #ifdef'ry.

So what is the actual use of vmstate_dummy ? I had a grep
through and as far as I can see the points where vmstate_cpu_common
is used are all in softmmu-only code. I tried this patch
and QEMU seems to compile OK:

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 140fa32a5e3..a827417a4d8 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -1131,8 +1131,6 @@ bool target_words_bigendian(void);

 #ifdef CONFIG_SOFTMMU
 extern const VMStateDescription vmstate_cpu_common;
-#else
-#define vmstate_cpu_common vmstate_dummy
 #endif

 #define VMSTATE_CPU() {                                                     \
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 075ee800960..8df7b69f389 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -194,8 +194,6 @@ struct VMStateDescription {
     const VMStateDescription **subsections;
 };

-extern const VMStateDescription vmstate_dummy;
-
 extern const VMStateInfo vmstate_info_bool;

 extern const VMStateInfo vmstate_info_int8;
diff --git a/stubs/vmstate.c b/stubs/vmstate.c
index cc4fe41dfc2..8513d9204e4 100644
--- a/stubs/vmstate.c
+++ b/stubs/vmstate.c
@@ -1,8 +1,6 @@
 #include "qemu/osdep.h"
 #include "migration/vmstate.h"

-const VMStateDescription vmstate_dummy = {};
-
 int vmstate_register_with_alias_id(VMStateIf *obj,
                                    uint32_t instance_id,
                                    const VMStateDescription *vmsd,

thanks
-- PMM

Re: [RFC PATCH v2 01/20] migration/vmstate: Restrict vmstate_dummy to user-mode

[Philippe Mathieu-Daudé]

On 1/19/21 2:50 PM, Peter Maydell wrote:
> On Sun, 17 Jan 2021 at 19:24, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> 'vmstate_dummy' is special and only used for user-mode. Rename
>> it to something more specific.
>> It was introduced restricted to user-mode in commit c71c3e99b8
>> ("Add a vmstate_dummy struct for CONFIG_USER_ONLY") but this
>> restriction was later removed in commit 6afc14e92ac ("migration:
>> Fix warning caused by missing declaration of vmstate_dummy").
>> Avoid the missing declaration warning by adding a stub for the
>> symbol, and restore the #ifdef'ry.
> 
> So what is the actual use of vmstate_dummy ? I had a grep
> through and as far as I can see the points where vmstate_cpu_common
> is used are all in softmmu-only code.

No clue, maybe simply remnant from unfinished work?

> I tried this patch
> and QEMU seems to compile OK:
> 
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 140fa32a5e3..a827417a4d8 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -1131,8 +1131,6 @@ bool target_words_bigendian(void);
> 
>  #ifdef CONFIG_SOFTMMU
>  extern const VMStateDescription vmstate_cpu_common;
> -#else
> -#define vmstate_cpu_common vmstate_dummy
>  #endif
> 
>  #define VMSTATE_CPU() {                                                     \

Great! Maybe even restricting VMSTATE_CPU() to softmmu-only:

-- >8 --
@@ -1131,9 +1131,6 @@ bool target_words_bigendian(void);

 #ifdef CONFIG_SOFTMMU
 extern const VMStateDescription vmstate_cpu_common;
-#else
-#define vmstate_cpu_common vmstate_dummy
-#endif

 #define VMSTATE_CPU() {
     \
     .name = "parent_obj",
     \
@@ -1142,6 +1139,7 @@ extern const VMStateDescription vmstate_cpu_common;
     .flags = VMS_STRUCT,
     \
     .offset = 0,
     \
 }
+#endif

 #endif /* NEED_CPU_H */
---

I'll wait if David/Juan have any comment, else respin based
on your patch.

Thanks,

Phil.

Re: [RFC PATCH v2 19/20] stubs/vmstate: Add VMSTATE_END_OF_LIST to vmstate_user_mode_cpu_dummy

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (f4bug@amsat.org) wrote:
> Add a name and end marker to the vmstate_user_mode_cpu_dummy variable.
> 
> Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  stubs/vmstate.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/stubs/vmstate.c b/stubs/vmstate.c
> index f561f9f39bd..1d0e03e233b 100644
> --- a/stubs/vmstate.c
> +++ b/stubs/vmstate.c
> @@ -2,7 +2,12 @@
>  #include "migration/vmstate.h"
>  
>  #if defined(CONFIG_USER_ONLY)
> -const VMStateDescription vmstate_user_mode_cpu_dummy = {};
> +const VMStateDescription vmstate_user_mode_cpu_dummy = {
> +    .name = "cpu_common_user",
> +    .fields = (VMStateField[]) {
> +        VMSTATE_END_OF_LIST()
> +    },
> +};
>  #endif
>  
>  const VMStateDescription vmstate_no_state_to_migrate = {
> -- 
> 2.26.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [RFC PATCH v2 01/20] migration/vmstate: Restrict vmstate_dummy to user-mode

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (f4bug@amsat.org) wrote:
> On 1/19/21 2:50 PM, Peter Maydell wrote:
> > On Sun, 17 Jan 2021 at 19:24, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >>
> >> 'vmstate_dummy' is special and only used for user-mode. Rename
> >> it to something more specific.
> >> It was introduced restricted to user-mode in commit c71c3e99b8
> >> ("Add a vmstate_dummy struct for CONFIG_USER_ONLY") but this
> >> restriction was later removed in commit 6afc14e92ac ("migration:
> >> Fix warning caused by missing declaration of vmstate_dummy").
> >> Avoid the missing declaration warning by adding a stub for the
> >> symbol, and restore the #ifdef'ry.
> > 
> > So what is the actual use of vmstate_dummy ? I had a grep
> > through and as far as I can see the points where vmstate_cpu_common
> > is used are all in softmmu-only code.
> 
> No clue, maybe simply remnant from unfinished work?

Not sure either; but it looks like Paolo fixed some of it up in d9f24bf5724
a few months ago; prior to that cpu_exec_unrealizefn used it even on a
USER_ONLY build.

Dave

> > I tried this patch
> > and QEMU seems to compile OK:
> > 
> > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> > index 140fa32a5e3..a827417a4d8 100644
> > --- a/include/hw/core/cpu.h
> > +++ b/include/hw/core/cpu.h
> > @@ -1131,8 +1131,6 @@ bool target_words_bigendian(void);
> > 
> >  #ifdef CONFIG_SOFTMMU
> >  extern const VMStateDescription vmstate_cpu_common;
> > -#else
> > -#define vmstate_cpu_common vmstate_dummy
> >  #endif
> > 
> >  #define VMSTATE_CPU() {                                                     \
> 
> Great! Maybe even restricting VMSTATE_CPU() to softmmu-only:
> 
> -- >8 --
> @@ -1131,9 +1131,6 @@ bool target_words_bigendian(void);
> 
>  #ifdef CONFIG_SOFTMMU
>  extern const VMStateDescription vmstate_cpu_common;
> -#else
> -#define vmstate_cpu_common vmstate_dummy
> -#endif
> 
>  #define VMSTATE_CPU() {
>      \
>      .name = "parent_obj",
>      \
> @@ -1142,6 +1139,7 @@ extern const VMStateDescription vmstate_cpu_common;
>      .flags = VMS_STRUCT,
>      \
>      .offset = 0,
>      \
>  }
> +#endif
> 
>  #endif /* NEED_CPU_H */
> ---
> 
> I'll wait if David/Juan have any comment, else respin based
> on your patch.
> 
> Thanks,
> 
> Phil.
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [RFC PATCH v2 04/20] hw/arm/aspeed_soc: Mark the device with no migratable fields

[Cédric Le Goater]

On 1/17/21 8:24 PM, Philippe Mathieu-Daudé wrote:
> This device doesn't have fields to migrate. Be explicit
> by using vmstate_qdev_no_state_to_migrate.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.