On 01/29/2015 10:04 PM, Igor Mammedov wrote:
On Wed, 14 Jan 2015 15:27:25 +0800
Zhu Guihua <zhugh.f...@cn.fujitsu.com> wrote:

From: Gu Zheng <guz.f...@cn.fujitsu.com>

Move cpu vmstate register from cpu_exec_init into cpu_common_realizefn,
and use cc->get_arch_id as the instance id that suggested by Igor to
fix the migration issue.

Signed-off-by: Gu Zheng <guz.f...@cn.fujitsu.com>
Signed-off-by: Zhu Guihua <zhugh.f...@cn.fujitsu.com>
---
  exec.c            | 32 +++++++++++++++++++-------------
  include/qom/cpu.h |  2 ++
  qom/cpu.c         |  2 ++
  3 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/exec.c b/exec.c
index 081818e..c9ffda6 100644
--- a/exec.c
+++ b/exec.c
@@ -513,10 +513,28 @@ void tcg_cpu_address_space_init(CPUState *cpu, 
AddressSpace *as)
  }
  #endif
+void cpu_vmstate_register(CPUState *cpu)
+{
+    CPUClass *cc = CPU_GET_CLASS(cpu);
+    int cpu_index = cc->get_arch_id(cpu);
that probable would be source migration problems:
because cc->get_arch_id(cpu) depending on topology might
be not sequential, for example: sockets=4,core=3
that would create sparse APIC numbering.

as result migration from old qemu to one with this change
would be rejected due to vmsd id mismatch mismatch.

we need a better way to handle cross version migration
between old/new scheme.
Hi Igor,

    I think to handler cross version migration issue, we only
need to do is converting new 'apic-id' to old 'cpu_index' to match
vmsd id  between old to new scheme.
we can save old cpu_index in alias id. and in order to keep
instance_id differ from alias id. we can use apic_id + maxcpus
as the instance_id. so during migration we can find the corresponding
cpu with instance_id regardless new/old scheme.

I has made a patch and test migrating from old version to new version.
it seems work fine. pls have a look at the attach file. ;)

Thanks,
Chen

+
+    if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
+        vmstate_register(NULL, cpu_index, &vmstate_cpu_common, cpu);
+    }
+#if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
that ifdef block affects only sparc/mips/cris and builds target specific code
while you are trying to call it from target independent qom/cpu.c

I'd suggest leave it where it was or better move into respective
targets realize_fns

+    register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION,
+                    cpu_save, cpu_load, cpu->env_ptr);
+    assert(cc->vmsd == NULL);
+    assert(qdev_get_vmsd(DEVICE(cpu)) == NULL);
+#endif
+    if (cc->vmsd != NULL) {
+        vmstate_register(NULL, cpu_index, cc->vmsd, cpu);
+    }
+}
+
  void cpu_exec_init(CPUArchState *env)
  {
      CPUState *cpu = ENV_GET_CPU(env);
-    CPUClass *cc = CPU_GET_CLASS(cpu);
      CPUState *some_cpu;
      int cpu_index;
@@ -539,18 +557,6 @@ void cpu_exec_init(CPUArchState *env)
  #if defined(CONFIG_USER_ONLY)
      cpu_list_unlock();
  #endif
-    if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
-        vmstate_register(NULL, cpu_index, &vmstate_cpu_common, cpu);
-    }
-#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);
-    assert(qdev_get_vmsd(DEVICE(cpu)) == NULL);
-#endif
-    if (cc->vmsd != NULL) {
-        vmstate_register(NULL, cpu_index, cc->vmsd, cpu);
-    }
And in general do CONFIG_USER_ONLY targets actually need/use
migration code?

  }
#if defined(TARGET_HAS_ICE)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 2098f1c..936afcd 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -562,6 +562,8 @@ void cpu_interrupt(CPUState *cpu, int mask);
#endif /* USER_ONLY */ +void cpu_vmstate_register(CPUState *cpu);
+
  #ifdef CONFIG_SOFTMMU
  static inline void cpu_unassigned_access(CPUState *cpu, hwaddr addr,
                                           bool is_write, bool is_exec,
diff --git a/qom/cpu.c b/qom/cpu.c
index 9c68fa4..a639822 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -302,6 +302,8 @@ static void cpu_common_realizefn(DeviceState *dev, Error 
**errp)
  {
      CPUState *cpu = CPU(dev);
+ cpu_vmstate_register(cpu);
+
      if (dev->hotplugged) {
          cpu_synchronize_post_init(cpu);
          cpu_resume(cpu);
.


diff --git a/exec.c b/exec.c
index bda96c6..6f3a90d 100644
--- a/exec.c
+++ b/exec.c
@@ -516,10 +516,12 @@ void tcg_cpu_address_space_init(CPUState *cpu, AddressSpace *as)
 void cpu_vmstate_register(CPUState *cpu)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
-    int cpu_index = cc->get_arch_id(cpu);
+    int cpu_index = cc->get_arch_id(cpu) + max_cpus;
+    int compat_index = cc->get_compat_id(cpu);
 
     if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
-        vmstate_register(NULL, cpu_index, &vmstate_cpu_common, cpu);
+        vmstate_register_with_alias_id(NULL, cpu_index, &vmstate_cpu_common,
+                                       cpu, compat_index, 3);
     }
 #if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
     register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION,
@@ -528,7 +530,8 @@ void cpu_vmstate_register(CPUState *cpu)
     assert(qdev_get_vmsd(DEVICE(cpu)) == NULL);
 #endif
     if (cc->vmsd != NULL) {
-        vmstate_register(NULL, cpu_index, cc->vmsd, cpu);
+        vmstate_register_with_alias_id(NULL, cpu_index, cc->vmsd,
+                                       cpu, compat_index, 3);
     }
 }
 
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 0592b4d..3d0bcbf 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -130,6 +130,7 @@ typedef struct CPUClass {
     void (*dump_statistics)(CPUState *cpu, FILE *f,
                             fprintf_function cpu_fprintf, int flags);
     int64_t (*get_arch_id)(CPUState *cpu);
+    int64_t (*get_compat_id)(CPUState *cpu);
     bool (*get_paging_enabled)(const CPUState *cpu);
     void (*get_memory_mapping)(CPUState *cpu, MemoryMappingList *list,
                                Error **errp);
diff --git a/qom/cpu.c b/qom/cpu.c
index a639822..48dee41 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -323,6 +323,11 @@ static int64_t cpu_common_get_arch_id(CPUState *cpu)
     return cpu->cpu_index;
 }
 
+static int64_t cpu_common_get_compat_id(CPUState *cpu)
+{
+    return cpu->cpu_index;
+}
+
 static void cpu_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -332,6 +337,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
     k->parse_features = cpu_common_parse_features;
     k->reset = cpu_common_reset;
     k->get_arch_id = cpu_common_get_arch_id;
+    k->get_compat_id = cpu_common_get_compat_id;
     k->has_work = cpu_common_has_work;
     k->get_paging_enabled = cpu_common_get_paging_enabled;
     k->get_memory_mapping = cpu_common_get_memory_mapping;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e25789a..8c024b9 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2794,13 +2794,15 @@ static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
 {
     DeviceState *apic_state = cpu->apic_state;
     CPUClass *cc = CPU_GET_CLASS(CPU(cpu));
+    int cpu_index = cc->get_arch_id(CPU(cpu)) + max_cpus;
+    int compat_index = cc->get_compat_id(CPU(cpu));
 
     if (apic_state == NULL) {
         return;
     }
 
-    vmstate_register(0, cc->get_arch_id(CPU(cpu)),
-                     &vmstate_apic_common, apic_state);
+    vmstate_register_with_alias_id(NULL, cpu_index, &vmstate_apic_common,
+                                   apic_state, compat_index, 3);
 
     if (qdev_init(cpu->apic_state)) {
         error_setg(errp, "APIC device '%s' could not be initialized",
@@ -3042,6 +3044,15 @@ static int64_t x86_cpu_get_arch_id(CPUState *cs)
     return env->cpuid_apic_id;
 }
 
+static int64_t x86_cpu_get_compat_id(CPUState *cs)
+{
+    X86CPU *cpu = X86_CPU(cs);
+    CPUX86State *env = &cpu->env;
+
+    return x86_compat_index_from_apic_id(smp_cores, smp_threads,
+                                         env->cpuid_apic_id);
+}
+
 static bool x86_cpu_get_paging_enabled(const CPUState *cs)
 {
     X86CPU *cpu = X86_CPU(cs);
@@ -3122,6 +3133,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
     cc->gdb_read_register = x86_cpu_gdb_read_register;
     cc->gdb_write_register = x86_cpu_gdb_write_register;
     cc->get_arch_id = x86_cpu_get_arch_id;
+    cc->get_compat_id = x86_cpu_get_compat_id;
     cc->get_paging_enabled = x86_cpu_get_paging_enabled;
 #ifdef CONFIG_USER_ONLY
     cc->handle_mmu_fault = x86_cpu_handle_mmu_fault;
diff --git a/target-i386/topology.h b/target-i386/topology.h
index dcb4988..42d1d07 100644
--- a/target-i386/topology.h
+++ b/target-i386/topology.h
@@ -150,4 +150,20 @@ static inline void x86_topo_ids_from_apic_id(unsigned nr_cores,
     topo->smt_id = apic_id & offset_mask;
 }
 
+static inline unsigned x86_compat_index_from_apic_id(unsigned nr_cores,
+                                                     unsigned nr_threads,
+                                                     apic_id_t apic_id)
+{
+    X86CPUTopoInfo topo;
+
+    x86_topo_ids_from_apic_id(nr_cores, nr_threads, apic_id, &topo);
+
+
+    return topo.pkg_id * nr_cores * nr_threads +
+           topo.core_id * nr_threads +
+           topo.smt_id;
+}
+
+
+
 #endif /* TARGET_I386_TOPOLOGY_H */

Reply via email to