diff mbox

[v2,3/5] hw/ppc/spapr: Look up CPU alias names instead of hard-coding the aliases

Message ID 1470762001-414-4-git-send-email-thuth@redhat.com
State New
Headers show

Commit Message

Thomas Huth Aug. 9, 2016, 4:59 p.m. UTC
Hard-coding the CPU alias names in the spapr_cores[] array has
two big disadvantages:

1) We register a real type with the CPU alias name in
   spapr_cpu_core_register_types() - this prevents us from registering
   a CPU family name in kvm_ppc_register_host_cpu_type() with the same
   name (as we do it for the non-hotpluggable CPU types).

2) It's quite cumbersome to maintain the aliases here in sync with the
   ppc_cpu_aliases list from target-ppc/cpu-models.c.

So let's simply add proper alias lookup to the spapr cpu core code,
too (by checking whether the given model can be used directly, and
if not by trying to look up the given model as an alias name instead).

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/ppc/spapr.c          |  2 +-
 hw/ppc/spapr_cpu_core.c | 38 +++++++++++++++++++++-----------------
 2 files changed, 22 insertions(+), 18 deletions(-)

Comments

Cédric Le Goater Aug. 10, 2016, 8:26 a.m. UTC | #1
On 08/09/2016 06:59 PM, Thomas Huth wrote:
> Hard-coding the CPU alias names in the spapr_cores[] array has
> two big disadvantages:
> 
> 1) We register a real type with the CPU alias name in
>    spapr_cpu_core_register_types() - this prevents us from registering
>    a CPU family name in kvm_ppc_register_host_cpu_type() with the same
>    name (as we do it for the non-hotpluggable CPU types).
> 
> 2) It's quite cumbersome to maintain the aliases here in sync with the
>    ppc_cpu_aliases list from target-ppc/cpu-models.c.

yes. 

may be, in the future, we could go a little further and add a cpu 
'family' in cpu-models.c, listing the cpu models used under a sPAPR 
platform. 

I have been looking at that with the PowerNVCPUCore object and I don't 
think we need the spapr_cpu_core_*_initfn(), we should be able to do 
the same with a class_data.  

Thanks,

C.

> So let's simply add proper alias lookup to the spapr cpu core code,
> too (by checking whether the given model can be used directly, and
> if not by trying to look up the given model as an alias name instead).
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/ppc/spapr.c          |  2 +-
>  hw/ppc/spapr_cpu_core.c | 38 +++++++++++++++++++++-----------------
>  2 files changed, 22 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 399dcc0..0787c66 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1815,7 +1815,7 @@ static void ppc_spapr_init(MachineState *machine)
>      if (mc->query_hotpluggable_cpus) {
>          char *type = spapr_get_cpu_core_type(machine->cpu_model);
>  
> -        if (!object_class_by_name(type)) {
> +        if (type == NULL) {
>              error_report("Unable to find sPAPR CPU Core definition");
>              exit(1);
>          }
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 716f7c4..bcb483d 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -93,6 +93,19 @@ char *spapr_get_cpu_core_type(const char *model)
>  
>      core_type = g_strdup_printf("%s-%s", model_pieces[0], TYPE_SPAPR_CPU_CORE);
>      g_strfreev(model_pieces);
> +
> +    /* Check whether it exists or whether we have to look up an alias name */
> +    if (!object_class_by_name(core_type)) {
> +        const char *realmodel;
> +
> +        g_free(core_type);
> +        realmodel = ppc_cpu_lookup_alias(model);
> +        if (realmodel) {
> +            return spapr_get_cpu_core_type(realmodel);
> +        }
> +        return NULL;
> +    }
> +
>      return core_type;
>  }
>  
> @@ -354,41 +367,32 @@ typedef struct SPAPRCoreInfo {
>  } SPAPRCoreInfo;
>  
>  static const SPAPRCoreInfo spapr_cores[] = {
> -    /* 970 and aliaes */
> +    /* 970 */
>      { .name = "970_v2.2", .initfn = spapr_cpu_core_970_initfn },
> -    { .name = "970", .initfn = spapr_cpu_core_970_initfn },
>  
> -    /* 970MP variants and aliases */
> +    /* 970MP variants */
>      { .name = "970MP_v1.0", .initfn = spapr_cpu_core_970MP_v10_initfn },
>      { .name = "970mp_v1.0", .initfn = spapr_cpu_core_970MP_v10_initfn },
>      { .name = "970MP_v1.1", .initfn = spapr_cpu_core_970MP_v11_initfn },
>      { .name = "970mp_v1.1", .initfn = spapr_cpu_core_970MP_v11_initfn },
> -    { .name = "970mp", .initfn = spapr_cpu_core_970MP_v11_initfn },
>  
> -    /* POWER5 and aliases */
> +    /* POWER5+ */
>      { .name = "POWER5+_v2.1", .initfn = spapr_cpu_core_POWER5plus_initfn },
> -    { .name = "POWER5+", .initfn = spapr_cpu_core_POWER5plus_initfn },
>  
> -    /* POWER7 and aliases */
> +    /* POWER7 */
>      { .name = "POWER7_v2.3", .initfn = spapr_cpu_core_POWER7_initfn },
> -    { .name = "POWER7", .initfn = spapr_cpu_core_POWER7_initfn },
>  
> -    /* POWER7+ and aliases */
> +    /* POWER7+ */
>      { .name = "POWER7+_v2.1", .initfn = spapr_cpu_core_POWER7plus_initfn },
> -    { .name = "POWER7+", .initfn = spapr_cpu_core_POWER7plus_initfn },
>  
> -    /* POWER8 and aliases */
> +    /* POWER8 */
>      { .name = "POWER8_v2.0", .initfn = spapr_cpu_core_POWER8_initfn },
> -    { .name = "POWER8", .initfn = spapr_cpu_core_POWER8_initfn },
> -    { .name = "power8", .initfn = spapr_cpu_core_POWER8_initfn },
>  
> -    /* POWER8E and aliases */
> +    /* POWER8E */
>      { .name = "POWER8E_v2.1", .initfn = spapr_cpu_core_POWER8E_initfn },
> -    { .name = "POWER8E", .initfn = spapr_cpu_core_POWER8E_initfn },
>  
> -    /* POWER8NVL and aliases */
> +    /* POWER8NVL */
>      { .name = "POWER8NVL_v1.0", .initfn = spapr_cpu_core_POWER8NVL_initfn },
> -    { .name = "POWER8NVL", .initfn = spapr_cpu_core_POWER8NVL_initfn },
>  
>      { .name = NULL }
>  };
>
diff mbox

Patch

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 399dcc0..0787c66 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1815,7 +1815,7 @@  static void ppc_spapr_init(MachineState *machine)
     if (mc->query_hotpluggable_cpus) {
         char *type = spapr_get_cpu_core_type(machine->cpu_model);
 
-        if (!object_class_by_name(type)) {
+        if (type == NULL) {
             error_report("Unable to find sPAPR CPU Core definition");
             exit(1);
         }
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 716f7c4..bcb483d 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -93,6 +93,19 @@  char *spapr_get_cpu_core_type(const char *model)
 
     core_type = g_strdup_printf("%s-%s", model_pieces[0], TYPE_SPAPR_CPU_CORE);
     g_strfreev(model_pieces);
+
+    /* Check whether it exists or whether we have to look up an alias name */
+    if (!object_class_by_name(core_type)) {
+        const char *realmodel;
+
+        g_free(core_type);
+        realmodel = ppc_cpu_lookup_alias(model);
+        if (realmodel) {
+            return spapr_get_cpu_core_type(realmodel);
+        }
+        return NULL;
+    }
+
     return core_type;
 }
 
@@ -354,41 +367,32 @@  typedef struct SPAPRCoreInfo {
 } SPAPRCoreInfo;
 
 static const SPAPRCoreInfo spapr_cores[] = {
-    /* 970 and aliaes */
+    /* 970 */
     { .name = "970_v2.2", .initfn = spapr_cpu_core_970_initfn },
-    { .name = "970", .initfn = spapr_cpu_core_970_initfn },
 
-    /* 970MP variants and aliases */
+    /* 970MP variants */
     { .name = "970MP_v1.0", .initfn = spapr_cpu_core_970MP_v10_initfn },
     { .name = "970mp_v1.0", .initfn = spapr_cpu_core_970MP_v10_initfn },
     { .name = "970MP_v1.1", .initfn = spapr_cpu_core_970MP_v11_initfn },
     { .name = "970mp_v1.1", .initfn = spapr_cpu_core_970MP_v11_initfn },
-    { .name = "970mp", .initfn = spapr_cpu_core_970MP_v11_initfn },
 
-    /* POWER5 and aliases */
+    /* POWER5+ */
     { .name = "POWER5+_v2.1", .initfn = spapr_cpu_core_POWER5plus_initfn },
-    { .name = "POWER5+", .initfn = spapr_cpu_core_POWER5plus_initfn },
 
-    /* POWER7 and aliases */
+    /* POWER7 */
     { .name = "POWER7_v2.3", .initfn = spapr_cpu_core_POWER7_initfn },
-    { .name = "POWER7", .initfn = spapr_cpu_core_POWER7_initfn },
 
-    /* POWER7+ and aliases */
+    /* POWER7+ */
     { .name = "POWER7+_v2.1", .initfn = spapr_cpu_core_POWER7plus_initfn },
-    { .name = "POWER7+", .initfn = spapr_cpu_core_POWER7plus_initfn },
 
-    /* POWER8 and aliases */
+    /* POWER8 */
     { .name = "POWER8_v2.0", .initfn = spapr_cpu_core_POWER8_initfn },
-    { .name = "POWER8", .initfn = spapr_cpu_core_POWER8_initfn },
-    { .name = "power8", .initfn = spapr_cpu_core_POWER8_initfn },
 
-    /* POWER8E and aliases */
+    /* POWER8E */
     { .name = "POWER8E_v2.1", .initfn = spapr_cpu_core_POWER8E_initfn },
-    { .name = "POWER8E", .initfn = spapr_cpu_core_POWER8E_initfn },
 
-    /* POWER8NVL and aliases */
+    /* POWER8NVL */
     { .name = "POWER8NVL_v1.0", .initfn = spapr_cpu_core_POWER8NVL_initfn },
-    { .name = "POWER8NVL", .initfn = spapr_cpu_core_POWER8NVL_initfn },
 
     { .name = NULL }
 };