[Qemu-devel] [PATCH] ppc: parse cpu features once

[Qemu-devel] [PATCH] ppc: parse cpu features once

[Greg Kurz]

Considering that features are converted to global properties and
global properties are automatically applied to every new instance
of created CPU (at object_new() time), there is no point in
parsing cpu_model string every time a CPU created. So move
parsing outside CPU creation loop and do it only once.

Parsing also should be done before any CPU is created so that
features would affect the first CPU a well.

This patch does that for all PowerPC machine types.

It is based on previous work from Bharata:

https://lists.nongnu.org/archive/html/qemu-devel/2016-06/msg07564.html

Signed-off-by: Greg Kurz <groug@kaod.org>
---

This is a standalone version of patch 2 from my latest series:

https://lists.nongnu.org/archive/html/qemu-devel/2016-07/msg02433.html

The only changes are trivial conflict fixes caused by the introduction
of ppc_cpu_init by patch 1.

---
 hw/ppc/e500.c          |    2 ++
 hw/ppc/mac_newworld.c  |    1 +
 hw/ppc/mac_oldworld.c  |    1 +
 hw/ppc/ppc.c           |   26 ++++++++++++++++++++++++++
 hw/ppc/ppc440_bamboo.c |    1 +
 hw/ppc/ppc4xx_devs.c   |    1 +
 hw/ppc/prep.c          |    1 +
 hw/ppc/spapr.c         |    2 ++
 hw/ppc/virtex_ml507.c  |    1 +
 include/hw/ppc/ppc.h   |    1 +
 10 files changed, 37 insertions(+)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 0cd534df55f8..616d0a07463b 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -814,6 +814,8 @@ void ppce500_init(MachineState *machine, PPCE500Params *params)
         machine->cpu_model = "e500v2_v30";
     }
 
+    ppc_cpu_parse_features(machine->cpu_model);
+
     irqs = g_malloc0(smp_cpus * sizeof(qemu_irq *));
     irqs[0] = g_malloc0(smp_cpus * sizeof(qemu_irq) * OPENPIC_OUTPUT_NB);
     for (i = 0; i < smp_cpus; i++) {
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 7d2510658d0f..bf734a5976ef 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -192,6 +192,7 @@ static void ppc_core99_init(MachineState *machine)
         machine->cpu_model = "G4";
 #endif
     }
+    ppc_cpu_parse_features(machine->cpu_model);
     for (i = 0; i < smp_cpus; i++) {
         cpu = cpu_ppc_init(machine->cpu_model);
         if (cpu == NULL) {
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 447948746b1a..f4678b13fe58 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -112,6 +112,7 @@ static void ppc_heathrow_init(MachineState *machine)
     /* init CPUs */
     if (machine->cpu_model == NULL)
         machine->cpu_model = "G3";
+    ppc_cpu_parse_features(machine->cpu_model);
     for (i = 0; i < smp_cpus; i++) {
         cpu = cpu_ppc_init(machine->cpu_model);
         if (cpu == NULL) {
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index e4252528a69d..89458690097f 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -33,6 +33,7 @@
 #include "hw/timer/m48t59.h"
 #include "qemu/log.h"
 #include "qemu/error-report.h"
+#include "qapi/error.h"
 #include "hw/loader.h"
 #include "sysemu/kvm.h"
 #include "kvm_ppc.h"
@@ -1350,3 +1351,28 @@ PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id)
 
     return NULL;
 }
+
+void ppc_cpu_parse_features(const char *cpu_model)
+{
+    CPUClass *cc;
+    ObjectClass *oc;
+    const char *typename;
+    gchar **model_pieces;
+
+    model_pieces = g_strsplit(cpu_model, ",", 2);
+    if (!model_pieces[0]) {
+        error_report("Invalid/empty CPU model name");
+        exit(1);
+    }
+
+    oc = cpu_class_by_name(TYPE_POWERPC_CPU, model_pieces[0]);
+    if (oc == NULL) {
+        error_report("Unable to find CPU definition: %s", model_pieces[0]);
+        exit(1);
+    }
+
+    typename = object_class_get_name(oc);
+    cc = CPU_CLASS(oc);
+    cc->parse_features(typename, model_pieces[1], &error_fatal);
+    g_strfreev(model_pieces);
+}
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index 5c535b18a20d..f778ad84a792 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -186,6 +186,7 @@ static void bamboo_init(MachineState *machine)
     if (machine->cpu_model == NULL) {
         machine->cpu_model = "440EP";
     }
+    ppc_cpu_parse_features(machine->cpu_model);
     cpu = cpu_ppc_init(machine->cpu_model);
     if (cpu == NULL) {
         fprintf(stderr, "Unable to initialize CPU!\n");
diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
index e7f413e49d08..a350ebf26437 100644
--- a/hw/ppc/ppc4xx_devs.c
+++ b/hw/ppc/ppc4xx_devs.c
@@ -56,6 +56,7 @@ PowerPCCPU *ppc4xx_init(const char *cpu_model,
     CPUPPCState *env;
 
     /* init CPUs */
+    ppc_cpu_parse_features(cpu_model);
     cpu = cpu_ppc_init(cpu_model);
     if (cpu == NULL) {
         fprintf(stderr, "Unable to find PowerPC %s CPU definition\n",
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 054af1e8b481..2faaf7f0784a 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -508,6 +508,7 @@ static void ppc_prep_init(MachineState *machine)
     /* init CPUs */
     if (machine->cpu_model == NULL)
         machine->cpu_model = "602";
+    ppc_cpu_parse_features(machine->cpu_model);
     for (i = 0; i < smp_cpus; i++) {
         cpu = cpu_ppc_init(machine->cpu_model);
         if (cpu == NULL) {
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7f33a1b2b57d..d11b845497c8 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1803,6 +1803,8 @@ static void ppc_spapr_init(MachineState *machine)
         machine->cpu_model = kvm_enabled() ? "host" : "POWER7";
     }
 
+    ppc_cpu_parse_features(machine->cpu_model);
+
     if (smc->dr_cpu_enabled) {
         char *type = spapr_get_cpu_core_type(machine->cpu_model);
 
diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index b97d96685cf1..bbc41d86c851 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -96,6 +96,7 @@ static PowerPCCPU *ppc440_init_xilinx(ram_addr_t *ram_size,
     CPUPPCState *env;
     qemu_irq *irqs;
 
+    ppc_cpu_parse_features(cpu_model);
     cpu = cpu_ppc_init(cpu_model);
     if (cpu == NULL) {
         fprintf(stderr, "Unable to initialize CPU!\n");
diff --git a/include/hw/ppc/ppc.h b/include/hw/ppc/ppc.h
index 5617dc4a2c04..f0d1cf0a0f64 100644
--- a/include/hw/ppc/ppc.h
+++ b/include/hw/ppc/ppc.h
@@ -106,4 +106,5 @@ enum {
 /* ppc_booke.c */
 void ppc_booke_timers_init(PowerPCCPU *cpu, uint32_t freq, uint32_t flags);
 
+void ppc_cpu_parse_features(const char *cpu_model);
 #endif

Re: [Qemu-devel] [PATCH] ppc: parse cpu features once

[Eduardo Habkost]

On Wed, Jul 13, 2016 at 10:07:07AM +0200, Greg Kurz wrote:
> Considering that features are converted to global properties and
> global properties are automatically applied to every new instance
> of created CPU (at object_new() time), there is no point in
> parsing cpu_model string every time a CPU created. So move
> parsing outside CPU creation loop and do it only once.
> 
> Parsing also should be done before any CPU is created so that
> features would affect the first CPU a well.
> 
> This patch does that for all PowerPC machine types.
> 
> It is based on previous work from Bharata:
> 
> https://lists.nongnu.org/archive/html/qemu-devel/2016-06/msg07564.html
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Most of the cases below already call cpu_ppc_init(), which calls
cpu_generic_init(), which calls cc->parse_features(). This means
this patch is not fixing any bug on those cases, but is just as
an improvement towards removing parse_features() from
cpu_generic_init().

spapr, on the other hand, really needs a bug fix to make it call
cc->parse_features() in the path where core objects are created.

So, I suggest we split this patch so we can include the spapr bug
fix in 2.7, and change the other machines (that use
cpu_ppc_init()/cpu_generic_init()) after 2.7.

-- 
Eduardo