Patchwork [qom-cpu,v2,3/8] cpu: Register VMStateDescription through CPUState

login
register
mail settings
Submitter Andreas Färber
Date Feb. 18, 2013, 7:42 p.m.
Message ID <1361216553-4549-4-git-send-email-afaerber@suse.de>
Download mbox | patch
Permalink /patch/221457/
State New
Headers show

Comments

Andreas Färber - Feb. 18, 2013, 7:42 p.m.
In comparison to DeviceClass::vmsd, CPU VMState is split in two,
"cpu_common" and "cpu", and uses cpu_index as instance_id instead of -1.
Therefore add a CPU-specific CPUClass::vmsd field.

Unlike the legacy CPUArchState registration, rather register CPUState.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 exec.c            |   11 +++++++++--
 include/qom/cpu.h |    3 +++
 2 Dateien geändert, 12 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)
Eduardo Habkost - Feb. 18, 2013, 7:57 p.m.
On Mon, Feb 18, 2013 at 08:42:28PM +0100, Andreas Färber wrote:
> In comparison to DeviceClass::vmsd, CPU VMState is split in two,
> "cpu_common" and "cpu", and uses cpu_index as instance_id instead of -1.
> Therefore add a CPU-specific CPUClass::vmsd field.
> 
> Unlike the legacy CPUArchState registration, rather register CPUState.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
[...]
> @@ -266,6 +268,7 @@ CPUState *qemu_get_cpu(int index)
>  void cpu_exec_init(CPUArchState *env)
>  {
>      CPUState *cpu = ENV_GET_CPU(env);
> +    CPUClass *cc = CPU_GET_CLASS(cpu);
>      CPUArchState **penv;
>      int cpu_index;
>  
> @@ -290,11 +293,15 @@ void cpu_exec_init(CPUArchState *env)
>  #if defined(CONFIG_USER_ONLY)
>      cpu_list_unlock();
>  #endif
> -#if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
>      vmstate_register(NULL, cpu_index, &vmstate_cpu_common, env);

Now vmstate_cpu_common is registered even if CPU_SAVE_VERSION isn't
defined and cc->vmsd is NULL. Is this intentional?


> +#if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
>      register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION,
>                      cpu_save, cpu_load, env);
> +    assert(cc->vmsd == NULL);
>  #endif
> +    if (cc->vmsd != NULL) {
> +        vmstate_register(NULL, cpu_index, cc->vmsd, cpu);
> +    }
>  }
>  
[...]
Andreas Färber - Feb. 18, 2013, 8:02 p.m.
Am 18.02.2013 20:57, schrieb Eduardo Habkost:
> On Mon, Feb 18, 2013 at 08:42:28PM +0100, Andreas Färber wrote:
>> In comparison to DeviceClass::vmsd, CPU VMState is split in two,
>> "cpu_common" and "cpu", and uses cpu_index as instance_id instead of -1.
>> Therefore add a CPU-specific CPUClass::vmsd field.
>>
>> Unlike the legacy CPUArchState registration, rather register CPUState.
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
> [...]
>> @@ -266,6 +268,7 @@ CPUState *qemu_get_cpu(int index)
>>  void cpu_exec_init(CPUArchState *env)
>>  {
>>      CPUState *cpu = ENV_GET_CPU(env);
>> +    CPUClass *cc = CPU_GET_CLASS(cpu);
>>      CPUArchState **penv;
>>      int cpu_index;
>>  
>> @@ -290,11 +293,15 @@ void cpu_exec_init(CPUArchState *env)
>>  #if defined(CONFIG_USER_ONLY)
>>      cpu_list_unlock();
>>  #endif
>> -#if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
>>      vmstate_register(NULL, cpu_index, &vmstate_cpu_common, env);
> 
> Now vmstate_cpu_common is registered even if CPU_SAVE_VERSION isn't
> defined and cc->vmsd is NULL. Is this intentional?

Kind of, this was adopted from Juan's series. Apart from alpha and
openrisc, which are fixed later in this series, I had taken care to mark
all unmigratable targets as such via dc->vmsd. Thus I don't see any harm
in it. We could go 100% safe and first mark alpha and openrisc as
unmigratable, then do these changes and some patches later drop it again.

Andreas
Eduardo Habkost - Feb. 19, 2013, 12:47 p.m.
On Mon, Feb 18, 2013 at 09:02:55PM +0100, Andreas Färber wrote:
> Am 18.02.2013 20:57, schrieb Eduardo Habkost:
> > On Mon, Feb 18, 2013 at 08:42:28PM +0100, Andreas Färber wrote:
> >> In comparison to DeviceClass::vmsd, CPU VMState is split in two,
> >> "cpu_common" and "cpu", and uses cpu_index as instance_id instead of -1.
> >> Therefore add a CPU-specific CPUClass::vmsd field.
> >>
> >> Unlike the legacy CPUArchState registration, rather register CPUState.
> >>
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> Signed-off-by: Andreas Färber <afaerber@suse.de>
> >> ---
> > [...]
> >> @@ -266,6 +268,7 @@ CPUState *qemu_get_cpu(int index)
> >>  void cpu_exec_init(CPUArchState *env)
> >>  {
> >>      CPUState *cpu = ENV_GET_CPU(env);
> >> +    CPUClass *cc = CPU_GET_CLASS(cpu);
> >>      CPUArchState **penv;
> >>      int cpu_index;
> >>  
> >> @@ -290,11 +293,15 @@ void cpu_exec_init(CPUArchState *env)
> >>  #if defined(CONFIG_USER_ONLY)
> >>      cpu_list_unlock();
> >>  #endif
> >> -#if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
> >>      vmstate_register(NULL, cpu_index, &vmstate_cpu_common, env);
> > 
> > Now vmstate_cpu_common is registered even if CPU_SAVE_VERSION isn't
> > defined and cc->vmsd is NULL. Is this intentional?
> 
> Kind of, this was adopted from Juan's series. Apart from alpha and
> openrisc, which are fixed later in this series, I had taken care to mark
> all unmigratable targets as such via dc->vmsd. Thus I don't see any harm
> in it. We could go 100% safe and first mark alpha and openrisc as
> unmigratable, then do these changes and some patches later drop it again.

Those targets that are not yet marked as unmigratable are already broken
today, right? So I don't see any harm, either.
Juan Quintela - Feb. 22, 2013, 1:07 p.m.
Andreas Färber <afaerber@suse.de> wrote:
> In comparison to DeviceClass::vmsd, CPU VMState is split in two,
> "cpu_common" and "cpu", and uses cpu_index as instance_id instead of -1.
> Therefore add a CPU-specific CPUClass::vmsd field.
>
> Unlike the legacy CPUArchState registration, rather register CPUState.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>

As said by Andreas, any arch that has no cpu migration support has no
hope for migration.

Later, Juan.

Patch

diff --git a/exec.c b/exec.c
index a41bcb8..84e43bd 100644
--- a/exec.c
+++ b/exec.c
@@ -219,7 +219,7 @@  void cpu_exec_init_all(void)
 #endif
 }
 
-#if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
+#if !defined(CONFIG_USER_ONLY)
 
 static int cpu_common_post_load(void *opaque, int version_id)
 {
@@ -245,6 +245,8 @@  static const VMStateDescription vmstate_cpu_common = {
         VMSTATE_END_OF_LIST()
     }
 };
+#else
+#define vmstate_cpu_common vmstate_dummy
 #endif
 
 CPUState *qemu_get_cpu(int index)
@@ -266,6 +268,7 @@  CPUState *qemu_get_cpu(int index)
 void cpu_exec_init(CPUArchState *env)
 {
     CPUState *cpu = ENV_GET_CPU(env);
+    CPUClass *cc = CPU_GET_CLASS(cpu);
     CPUArchState **penv;
     int cpu_index;
 
@@ -290,11 +293,15 @@  void cpu_exec_init(CPUArchState *env)
 #if defined(CONFIG_USER_ONLY)
     cpu_list_unlock();
 #endif
-#if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
     vmstate_register(NULL, cpu_index, &vmstate_cpu_common, env);
+#if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
     register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION,
                     cpu_save, cpu_load, env);
+    assert(cc->vmsd == NULL);
 #endif
+    if (cc->vmsd != NULL) {
+        vmstate_register(NULL, cpu_index, cc->vmsd, cpu);
+    }
 }
 
 #if defined(TARGET_HAS_ICE)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index ee1a7c8..1106e39 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -44,6 +44,7 @@  typedef struct CPUState CPUState;
  * @class_by_name: Callback to map -cpu command line model name to an
  * instantiatable CPU type.
  * @reset: Callback to reset the #CPUState to its initial state.
+ * @vmsd: State description for migration.
  *
  * Represents a CPU family or model.
  */
@@ -55,6 +56,8 @@  typedef struct CPUClass {
     ObjectClass *(*class_by_name)(const char *cpu_model);
 
     void (*reset)(CPUState *cpu);
+
+    const struct VMStateDescription *vmsd;
 } CPUClass;
 
 struct KVMState;