[Qemu-devel] [PATCH qom-cpu v3 0/6] QOM CPUState migration, next generation

[Qemu-devel] [PATCH qom-cpu v3 0/6] QOM CPUState migration, next generation

[Andreas Färber]

Hello,

As previously reported, alpha and openrisc had prepared VMStateDescriptions
but failed to register them due to lack of CPU_SAVE_VERSION.

Now that CPUState is a DeviceState, it was suggested to register the VMState
of new CPUs the same way devices do. They thereby deviate from legacy CPUs,
which register a separate VMStateDescription for the common state.

The series still concludes with a patch to prevent further silent migration
registration bugs.

Regards,
Andreas

v2 -> v3:
* Prepended patch introducing VMSTATE_CPU() macro for vmstate_cpu_common.
* Prepended patch adding device_class_set_vmsd().
* Added typo fix patch for cpu_class_set_vmsd().
* Register AlphaCPU and OpenRISCCPU vmsd via DeviceClass instead of CPUClass.

Cc: Juan Quintela <quintela@redhat.com>
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>

Cc: Anthony Liguori <anthony@codemonkey.ws>
Cc: Blue Swirl <blauwirbel@gmail.com>
Cc: Aurélien Jarno <aurelien@aurel32.net>

Cc: Richard Henderson <rth@twiddle.net>
Cc: Jia Liu <proljc@gmail.com>

Andreas Färber (6):
  cpu: Fix cpu_class_set_vmsd() documentation
  cpu: Introduce device_class_set_vmsd() helper
  cpu: Introduce VMSTATE_CPU() macro for CPUState
  target-alpha: Register VMStateDescription for AlphaCPU
  target-openrisc: Register VMStateDescription for OpenRISCCPU
  cpu: Guard cpu_{save,load}() definitions

 exec.c                    |  5 ++---
 include/qemu-common.h     |  2 ++
 include/qom/cpu.h         | 37 ++++++++++++++++++++++++++++++++++++-
 target-alpha/cpu-qom.h    |  4 ++++
 target-alpha/cpu.c        |  2 ++
 target-alpha/machine.c    | 28 ++++++++++++++++------------
 target-openrisc/cpu.c     |  1 +
 target-openrisc/cpu.h     |  2 ++
 target-openrisc/machine.c | 27 ++++++++++++++++-----------
 9 files changed, 81 insertions(+), 27 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH qom-cpu v3 1/6] cpu: Fix cpu_class_set_vmsd() documentation

[Andreas Färber]

It's CPUClass::vmsd, not CPUState::vmsd.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 include/qom/cpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index a5bb515..55a85c1 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -226,7 +226,7 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model);
  *
  * The @value argument is intentionally discarded for the non-softmmu targets
  * to avoid linker errors or excessive preprocessor usage. If this behavior
- * is undesired, you should assign #CPUState.vmsd directly instead.
+ * is undesired, you should assign #CPUClass.vmsd directly instead.
  */
 #ifndef CONFIG_USER_ONLY
 static inline void cpu_class_set_vmsd(CPUClass *cc,
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH qom-cpu v3 1/6] cpu: Fix cpu_class_set_vmsd() documentation

[li guang]

在 2013-06-18二的 02:29 +0200，Andreas Färber写道：
> It's CPUClass::vmsd, not CPUState::vmsd.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Reviewed-by: liguang <lig.fnst@cn.fujitsu.com>

> ---
>  include/qom/cpu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index a5bb515..55a85c1 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -226,7 +226,7 @@ ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model);
>   *
>   * The @value argument is intentionally discarded for the non-softmmu targets
>   * to avoid linker errors or excessive preprocessor usage. If this behavior
> - * is undesired, you should assign #CPUState.vmsd directly instead.
> + * is undesired, you should assign #CPUClass.vmsd directly instead.
>   */
>  #ifndef CONFIG_USER_ONLY
>  static inline void cpu_class_set_vmsd(CPUClass *cc,

Re: [Qemu-devel] [PATCH qom-cpu v3 1/6] cpu: Fix cpu_class_set_vmsd() documentation

[Juan Quintela]

Andreas Farber <afaerber@suse.de> wrote:
> It's CPUClass::vmsd, not CPUState::vmsd.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Reviewed-by: Juan Quintela <quintela@redhat.com> 

[Qemu-devel] [PATCH qom-cpu v3 2/6] cpu: Introduce device_class_set_vmsd() helper

[Andreas Färber]

It's the equivalent to cpu_class_set_vmsd(), to assign
DeviceClass::vmsd. It wasn't needed before since only static,
unmigratable VMStateDescriptions were assigned so far.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 include/qom/cpu.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 55a85c1..397219b 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -239,6 +239,27 @@ static inline void cpu_class_set_vmsd(CPUClass *cc,
 #endif
 
 /**
+ * device_class_set_vmsd:
+ * @dc: Device class
+ * @value: Value to set. Unused for %CONFIG_USER_ONLY.
+ *
+ * Sets #VMStateDescription for @dc.
+ *
+ * The @value argument is intentionally discarded for the non-softmmu targets
+ * to avoid linker errors or excessive preprocessor usage. If this behavior
+ * is undesired, you should assign #DeviceClass.vmsd directly instead.
+ */
+#ifndef CONFIG_USER_ONLY
+static inline void device_class_set_vmsd(DeviceClass *dc,
+                                         const struct VMStateDescription *value)
+{
+    dc->vmsd = value;
+}
+#else
+#define device_class_set_vmsd(dc, value) ((dc)->vmsd = NULL)
+#endif
+
+/**
  * qemu_cpu_has_work:
  * @cpu: The vCPU to check.
  *
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH qom-cpu v3 2/6] cpu: Introduce device_class_set_vmsd() helper

[Juan Quintela]

Andreas Farber <afaerber@suse.de> wrote:
> It's the equivalent to cpu_class_set_vmsd(), to assign
> DeviceClass::vmsd. It wasn't needed before since only static,
> unmigratable VMStateDescriptions were assigned so far.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Reviewed-by: Juan Quintela <quintela@redhat.com>

[Qemu-devel] [PATCH qom-cpu v3 3/6] cpu: Introduce VMSTATE_CPU() macro for CPUState

[Andreas Färber]

To be used to embed common CPU state into CPU subclasses.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 exec.c            |  5 ++---
 include/qom/cpu.h | 14 ++++++++++++++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/exec.c b/exec.c
index 5b8b40d..0bee004 100644
--- a/exec.c
+++ b/exec.c
@@ -246,7 +246,7 @@ static int cpu_common_post_load(void *opaque, int version_id)
     return 0;
 }
 
-static const VMStateDescription vmstate_cpu_common = {
+const VMStateDescription vmstate_cpu_common = {
     .name = "cpu_common",
     .version_id = 1,
     .minimum_version_id = 1,
@@ -258,8 +258,7 @@ static const VMStateDescription vmstate_cpu_common = {
         VMSTATE_END_OF_LIST()
     }
 };
-#else
-#define vmstate_cpu_common vmstate_dummy
+
 #endif
 
 CPUState *qemu_get_cpu(int index)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 397219b..3e8cc47 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -378,4 +378,18 @@ void cpu_reset_interrupt(CPUState *cpu, int mask);
  */
 void cpu_resume(CPUState *cpu);
 
+#ifdef CONFIG_SOFTMMU
+extern const struct VMStateDescription vmstate_cpu_common;
+#else
+#define vmstate_cpu_common vmstate_dummy
+#endif
+
+#define VMSTATE_CPU() {                                                     \
+    .name = "parent_obj",                                                   \
+    .size = sizeof(CPUState),                                               \
+    .vmsd = &vmstate_cpu_common,                                            \
+    .flags = VMS_STRUCT,                                                    \
+    .offset = 0,                                                            \
+}
+
 #endif
-- 
1.8.1.4

[Qemu-devel] [PATCH qom-cpu v3 4/6] target-alpha: Register VMStateDescription for AlphaCPU

[Andreas Färber]

Commit b758aca1f6cdb175634812b79f5560c36c902d00 (target-alpha: Enable
the alpha-softmmu target.) introduced cpu_{save,load}() functions but
didn't define CPU_SAVE_VERSION, so they were never registered.

Drop cpu_{save,load}() and register the VMStateDescription via DeviceClass.
This operates on the AlphaCPU object instead of CPUAlphaState.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-alpha/cpu-qom.h |  4 ++++
 target-alpha/cpu.c     |  2 ++
 target-alpha/machine.c | 28 ++++++++++++++++------------
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/target-alpha/cpu-qom.h b/target-alpha/cpu-qom.h
index 32ee286..ee10ed6 100644
--- a/target-alpha/cpu-qom.h
+++ b/target-alpha/cpu-qom.h
@@ -74,6 +74,10 @@ static inline AlphaCPU *alpha_env_get_cpu(CPUAlphaState *env)
 
 #define ENV_OFFSET offsetof(AlphaCPU, env)
 
+#ifndef CONFIG_USER_ONLY
+extern const struct VMStateDescription vmstate_alpha_cpu;
+#endif
+
 void alpha_cpu_do_interrupt(CPUState *cpu);
 
 #endif
diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
index cad1716..8252cea 100644
--- a/target-alpha/cpu.c
+++ b/target-alpha/cpu.c
@@ -21,6 +21,7 @@
 
 #include "cpu.h"
 #include "qemu-common.h"
+#include "migration/vmstate.h"
 
 
 static void alpha_cpu_realizefn(DeviceState *dev, Error **errp)
@@ -264,6 +265,7 @@ static void alpha_cpu_class_init(ObjectClass *oc, void *data)
 
     cc->class_by_name = alpha_cpu_class_by_name;
     cc->do_interrupt = alpha_cpu_do_interrupt;
+    device_class_set_vmsd(dc, &vmstate_alpha_cpu);
 }
 
 static const TypeInfo alpha_cpu_type_info = {
diff --git a/target-alpha/machine.c b/target-alpha/machine.c
index 1c9edd1..889f2fc 100644
--- a/target-alpha/machine.c
+++ b/target-alpha/machine.c
@@ -20,7 +20,7 @@ static const VMStateInfo vmstate_fpcr = {
     .put = put_fpcr,
 };
 
-static VMStateField vmstate_cpu_fields[] = {
+static VMStateField vmstate_env_fields[] = {
     VMSTATE_UINTTL_ARRAY(ir, CPUAlphaState, 31),
     VMSTATE_UINTTL_ARRAY(fir, CPUAlphaState, 31),
     /* Save the architecture value of the fpcr, not the internally
@@ -68,20 +68,24 @@ static VMStateField vmstate_cpu_fields[] = {
     VMSTATE_END_OF_LIST()
 };
 
-static const VMStateDescription vmstate_cpu = {
-    .name = "cpu",
+static const VMStateDescription vmstate_env = {
+    .name = "env",
     .version_id = 1,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
-    .fields = vmstate_cpu_fields,
+    .fields = vmstate_env_fields,
 };
 
-void cpu_save(QEMUFile *f, void *opaque)
-{
-    vmstate_save_state(f, &vmstate_cpu, opaque);
-}
+static VMStateField vmstate_cpu_fields[] = {
+    VMSTATE_CPU(),
+    VMSTATE_STRUCT(env, AlphaCPU, 1, vmstate_env, CPUAlphaState),
+    VMSTATE_END_OF_LIST()
+};
 
-int cpu_load(QEMUFile *f, void *opaque, int version_id)
-{
-    return vmstate_load_state(f, &vmstate_cpu, opaque, version_id);
-}
+const VMStateDescription vmstate_alpha_cpu = {
+    .name = "cpu",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = vmstate_cpu_fields,
+};
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH qom-cpu v3 4/6] target-alpha: Register VMStateDescription for AlphaCPU

[Juan Quintela]

Andreas Färber <afaerber@suse.de> wrote:
> Commit b758aca1f6cdb175634812b79f5560c36c902d00 (target-alpha: Enable
> the alpha-softmmu target.) introduced cpu_{save,load}() functions but
> didn't define CPU_SAVE_VERSION, so they were never registered.
>
> Drop cpu_{save,load}() and register the VMStateDescription via DeviceClass.
> This operates on the AlphaCPU object instead of CPUAlphaState.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Reviewed-by: Juan Quintela <quintela@redhat.com> 

Re: [Qemu-devel] [PATCH qom-cpu v3 4/6] target-alpha: Register VMStateDescription for AlphaCPU

[Richard Henderson]

On 06/17/2013 05:29 PM, Andreas Färber wrote:
> Commit b758aca1f6cdb175634812b79f5560c36c902d00 (target-alpha: Enable
> the alpha-softmmu target.) introduced cpu_{save,load}() functions but
> didn't define CPU_SAVE_VERSION, so they were never registered.
> 
> Drop cpu_{save,load}() and register the VMStateDescription via DeviceClass.
> This operates on the AlphaCPU object instead of CPUAlphaState.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  target-alpha/cpu-qom.h |  4 ++++
>  target-alpha/cpu.c     |  2 ++
>  target-alpha/machine.c | 28 ++++++++++++++++------------
>  3 files changed, 22 insertions(+), 12 deletions(-)

Acked-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH qom-cpu v3 5/6] target-openrisc: Register VMStateDescription for OpenRISCCPU

[Andreas Färber]

Since commit e67db06e9f6d7e514ee2a9b9b769ecd42977f6fb (target-or32: Add
target stubs and QOM cpu) a VMStateDescription existed, but
CPU_SAVE_VERSION was not set, so it was never registered.

Drop cpu_{save,load}() and register VMStateDescription via DeviceState.
Use a version_id of 1 and specify minimum versions as well.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-openrisc/cpu.c     |  1 +
 target-openrisc/cpu.h     |  2 ++
 target-openrisc/machine.c | 27 ++++++++++++++++-----------
 3 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
index ffe14f3..f8703a0 100644
--- a/target-openrisc/cpu.c
+++ b/target-openrisc/cpu.c
@@ -149,6 +149,7 @@ static void openrisc_cpu_class_init(ObjectClass *oc, void *data)
 
     cc->class_by_name = openrisc_cpu_class_by_name;
     cc->do_interrupt = openrisc_cpu_do_interrupt;
+    device_class_set_vmsd(dc, &vmstate_openrisc_cpu);
 }
 
 static void cpu_register(const OpenRISCCPUInfo *info)
diff --git a/target-openrisc/cpu.h b/target-openrisc/cpu.h
index b9c55ba..aee7769 100644
--- a/target-openrisc/cpu.h
+++ b/target-openrisc/cpu.h
@@ -360,6 +360,8 @@ int cpu_openrisc_signal_handler(int host_signum, void *pinfo, void *puc);
 #define cpu_signal_handler cpu_openrisc_signal_handler
 
 #ifndef CONFIG_USER_ONLY
+extern const struct VMStateDescription vmstate_openrisc_cpu;
+
 /* hw/openrisc_pic.c */
 void cpu_openrisc_pic_init(OpenRISCCPU *cpu);
 
diff --git a/target-openrisc/machine.c b/target-openrisc/machine.c
index cba9811..6f864fe 100644
--- a/target-openrisc/machine.c
+++ b/target-openrisc/machine.c
@@ -20,8 +20,11 @@
 #include "hw/hw.h"
 #include "hw/boards.h"
 
-static const VMStateDescription vmstate_cpu = {
-    .name = "cpu",
+static const VMStateDescription vmstate_env = {
+    .name = "env",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32_ARRAY(gpr, CPUOpenRISCState, 32),
         VMSTATE_UINT32(sr, CPUOpenRISCState),
@@ -36,12 +39,14 @@ static const VMStateDescription vmstate_cpu = {
     }
 };
 
-void cpu_save(QEMUFile *f, void *opaque)
-{
-    vmstate_save_state(f, &vmstate_cpu, opaque);
-}
-
-int cpu_load(QEMUFile *f, void *opaque, int version_id)
-{
-    return vmstate_load_state(f, &vmstate_cpu, opaque, version_id);
-}
+const VMStateDescription vmstate_openrisc_cpu = {
+    .name = "cpu",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_CPU(),
+        VMSTATE_STRUCT(env, OpenRISCCPU, 1, vmstate_env, CPUOpenRISCState),
+        VMSTATE_END_OF_LIST()
+    }
+};
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH qom-cpu v3 5/6] target-openrisc: Register VMStateDescription for OpenRISCCPU

[Juan Quintela]

Andreas Färber <afaerber@suse.de> wrote:
> Since commit e67db06e9f6d7e514ee2a9b9b769ecd42977f6fb (target-or32: Add
> target stubs and QOM cpu) a VMStateDescription existed, but
> CPU_SAVE_VERSION was not set, so it was never registered.
>
> Drop cpu_{save,load}() and register VMStateDescription via DeviceState.
> Use a version_id of 1 and specify minimum versions as well.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Reviewed-by: Juan Quintela <quintela@redhat.com>

[Qemu-devel] [PATCH qom-cpu v3 6/6] cpu: Guard cpu_{save, load}() definitions

[Andreas Färber]

A few targets already managed to implement cpu_save() and cpu_load()
without defining CPU_SAVE_VERSION that causes them to be registered.

Guard the prototypes with CPU_SAVE_VERSION to avoid this happening again
until all targets are converted to VMState (or QIDL).

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 include/qemu-common.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index 3c91375..6010bba 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -279,8 +279,10 @@ bool tcg_enabled(void);
 void cpu_exec_init_all(void);
 
 /* CPU save/load.  */
+#ifdef CPU_SAVE_VERSION
 void cpu_save(QEMUFile *f, void *opaque);
 int cpu_load(QEMUFile *f, void *opaque, int version_id);
+#endif
 
 /* Unblock cpu */
 void qemu_cpu_kick_self(void);
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH qom-cpu v3 6/6] cpu: Guard cpu_{save, load}() definitions

[Juan Quintela]

Andreas Färber <afaerber@suse.de> wrote:
> A few targets already managed to implement cpu_save() and cpu_load()
> without defining CPU_SAVE_VERSION that causes them to be registered.
>
> Guard the prototypes with CPU_SAVE_VERSION to avoid this happening again
> until all targets are converted to VMState (or QIDL).
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Re: [Qemu-devel] [PATCH qom-cpu v3 0/6] QOM CPUState migration, next generation

[Andreas Färber]

Am 18.06.2013 02:29, schrieb Andreas Färber:
> Andreas Färber (6):
>   cpu: Fix cpu_class_set_vmsd() documentation
>   cpu: Introduce device_class_set_vmsd() helper
>   cpu: Introduce VMSTATE_CPU() macro for CPUState
>   target-alpha: Register VMStateDescription for AlphaCPU
>   target-openrisc: Register VMStateDescription for OpenRISCCPU
>   cpu: Guard cpu_{save,load}() definitions

Thanks for the quick review, applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

This caused some trivial conflicts with my CPUState part 10 series in
target-alpha and target-openpic class_init as well as include/qom/cpu.h.
Resolved on qom-cpu-10 Git branch.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg