[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [RFC PATCH] target-ppc: Relax use of generic CPU name for

From: Alexander Graf
Subject: Re: [Qemu-ppc] [RFC PATCH] target-ppc: Relax use of generic CPU name for KVM
Date: Fri, 11 Apr 2014 11:00:39 +0200
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20100101 Thunderbird/24.4.0

On 11.04.14 07:00, Alexey Kardashevskiy wrote:
At the moment generic version-less CPUs are supported via hardcoded aliases.
For example, POWER7 is an alias for POWER7_v2.1. So when QEMU is started
with -cpu POWER7, the POWER7_v2.1 class instance is created.

This approach works for TCG and KVMs other than HV KVM. HV KVM cannot emulate
PVR value so the guest always sees the real PVR. HV KVM will not allow setting
PVR other that the host PVR because of that (the kernel patch for it is on
its way). So in most cases it is impossible to run QEMU with -cpu POWER7
unless the host PVR is exactly the same as the one from the alias (which
is now POWER7_v2.3). It was decided that under HV KVM QEMU should use
-cpu host.

Using "host" CPU type creates a problem for management tools such as libvirt
because they want to know in advance if the destination guest can possibly
run on the destination. Since the "host" type is really not a type and will
always work with any KVM, there is no way for libvirt to know if the migration
will success.

This patch changes aliases handling by lowering their priority and adding
a new CPU generic class the same way as it is done for the "host" CPU class.

This registers additional CPU class derived from the host CPU family.
The name for it is taken from @desc field of the CPU family class.

This moves aliases lookup after CPU class lookup. This is to let new generic
CPU to be found first if it is present and only if it is not (TCG case), use

The nice part about this is that we will also use the alias for CPUs that are not the type we're running on. So if I use -cpu POWER7 on a POWER7 host, it will give me my host's POWER7 CPU. But if I use -cpu 970 on that POWER7 host it will use the alias.


Signed-off-by: Alexey Kardashevskiy <address@hidden>

!!! THIS IS NOT FOR 2.0 !!!

Just an RFC :)

Is that the right direction to go?

I would also remove POWER7_v2.0 and POWER7_v2.1 and leave just one versioned
CPU per family (which is POWER7_v2.3 with POWER7 alias). We do not emulate
these CPUs diffent so it does not make much sense to keep them, one per family
is perfectly enough.

  target-ppc/cpu-models.c     |  4 ----
  target-ppc/cpu-models.h     |  2 --
  target-ppc/kvm.c            | 20 ++++++++++++++++++++
  target-ppc/translate_init.c | 18 +++++++++++-------
  4 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c
index f6c9b3a..57cb4e4 100644
--- a/target-ppc/cpu-models.c
+++ b/target-ppc/cpu-models.c
@@ -1134,10 +1134,6 @@
      POWERPC_DEF("POWER6A",       CPU_POWERPC_POWER6A,                POWER6,
-    POWERPC_DEF("POWER7_v2.0",   CPU_POWERPC_POWER7_v20,             POWER7,
-                "POWER7 v2.0")
-    POWERPC_DEF("POWER7_v2.1",   CPU_POWERPC_POWER7_v21,             POWER7,
-                "POWER7 v2.1")
      POWERPC_DEF("POWER7_v2.3",   CPU_POWERPC_POWER7_v23,             POWER7,
                  "POWER7 v2.3")
      POWERPC_DEF("POWER7+_v2.1",  CPU_POWERPC_POWER7P_v21,            POWER7P,
diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
index 644a126..9a003b4 100644
--- a/target-ppc/cpu-models.h
+++ b/target-ppc/cpu-models.h
@@ -555,8 +555,6 @@ enum {
      CPU_POWERPC_POWER6A            = 0x0F000002,
      CPU_POWERPC_POWER7_BASE        = 0x003F0000,
      CPU_POWERPC_POWER7_MASK        = 0xFFFF0000,
-    CPU_POWERPC_POWER7_v20         = 0x003F0200,
-    CPU_POWERPC_POWER7_v21         = 0x003F0201,

I think it makes sense to do the removal in a separate patch.

      CPU_POWERPC_POWER7_v23         = 0x003F0203,
      CPU_POWERPC_POWER7P_BASE       = 0x004A0000,
      CPU_POWERPC_POWER7P_MASK       = 0xFFFF0000,
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index ff952f0..b2e4db5 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1785,6 +1785,17 @@ bool kvmppc_has_cap_htab_fd(void)
      return cap_htab_fd;
+static PowerPCCPUClass *ppc_cpu_get_family_class(PowerPCCPUClass *pcc)
+    ObjectClass *oc = OBJECT_CLASS(pcc);
+    while (oc && !object_class_is_abstract(oc)) {
+        oc = object_class_get_parent(oc);
+    }
+    return POWERPC_CPU_CLASS(oc);

What if we don't find any? It should never happen, but better put an assert(oc) before the return.

  static int kvm_ppc_register_host_cpu_type(void)
      TypeInfo type_info = {
@@ -1794,6 +1805,7 @@ static int kvm_ppc_register_host_cpu_type(void)
      uint32_t host_pvr = mfpvr();
      PowerPCCPUClass *pvr_pcc;
+    DeviceClass *dc;
pvr_pcc = ppc_cpu_class_by_pvr(host_pvr);
      if (pvr_pcc == NULL) {
@@ -1804,6 +1816,14 @@ static int kvm_ppc_register_host_cpu_type(void)
      type_info.parent = object_class_get_name(OBJECT_CLASS(pvr_pcc));
+    /* Register generic family CPU class for a family */
+    pvr_pcc = ppc_cpu_get_family_class(pvr_pcc);
+    dc = DEVICE_CLASS(pvr_pcc);
+    type_info.parent = object_class_get_name(OBJECT_CLASS(pvr_pcc));
+    type_info.name = g_strdup_printf("%s-"TYPE_POWERPC_CPU, dc->desc);
+    type_register(&type_info);

Heh, nice trick. Just generate the class on the fly like we do for -cpu host. I like it.

      return 0;
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 4d94015..823c63c 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8218,12 +8218,6 @@ static ObjectClass *ppc_cpu_class_by_name(const char 
- for (i = 0; ppc_cpu_aliases[i].alias != NULL; i++) {
-        if (strcmp(ppc_cpu_aliases[i].alias, name) == 0) {
-            return ppc_cpu_class_by_alias(&ppc_cpu_aliases[i]);
-        }
-    }
      list = object_class_get_list(TYPE_POWERPC_CPU, false);
      item = g_slist_find_custom(list, name, ppc_cpu_compare_class_name);
      if (item != NULL) {
@@ -8231,7 +8225,17 @@ static ObjectClass *ppc_cpu_class_by_name(const char 
- return ret;
+    if (ret) {
+        return ret;
+    }
+    for (i = 0; ppc_cpu_aliases[i].alias != NULL; i++) {
+        if (strcmp(ppc_cpu_aliases[i].alias, name) == 0) {
+            return ppc_cpu_class_by_alias(&ppc_cpu_aliases[i]);
+        }
+    }

Classes first aliases later. Very good - makes a lot of sense. I would split this into a separate patch.

Otherwise I like the patch. It's a simple and clean approach to the problem. Now the only thing missing to migration compatibility is the host cpu type query mechanism you can check out with the s390 people.

Also while at it, maybe you should poke your hardware people to ensure that we can disable kernel level features from one POWER version to the next, so that we could support -cpu POWER8 on a POWER9 (or whatever it will be called) system and give users a chance for easy migration.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]