Re: [Qemu-devel] [PATCH for-2.11 01/27] sparc: convert cpu models to SPARC cpu subclasses

[Philippe Mathieu-Daudé]

On 08/18/2017 07:08 AM, Igor Mammedov wrote:

QOMfy cpu models handling introducing propper cpu types
for each cpu model.

Signed-off-by: Igor Mammedov <imamm...@redhat.com>
---
with this and conversion of features to properties,
it would be possible to replace cpu_sparc_init() with
cpu_generic_init() and reuse common -cpu handling
infrastructure.

CC: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
CC: Artyom Tarasenko <atar4q...@gmail.com>
CC: Philippe Mathieu-Daudé <f4...@amsat.org>

v2:
    * make base class abstract (Philippe Mathieu-Daudé <f4...@amsat.org>)
---
   target/sparc/cpu-qom.h |   2 +
   target/sparc/cpu.c     | 121 
+++++++++++++++++++++++++++++++++----------------
   2 files changed, 84 insertions(+), 39 deletions(-)

diff --git a/target/sparc/cpu-qom.h b/target/sparc/cpu-qom.h
index f63af72..af6d57a 100644
--- a/target/sparc/cpu-qom.h
+++ b/target/sparc/cpu-qom.h
@@ -35,6 +35,7 @@
   #define SPARC_CPU_GET_CLASS(obj) \
       OBJECT_GET_CLASS(SPARCCPUClass, (obj), TYPE_SPARC_CPU)

+typedef struct sparc_def_t sparc_def_t;

   /**
    * SPARCCPUClass:
    * @parent_realize: The parent class' realize handler.
@@ -49,6 +50,7 @@ typedef struct SPARCCPUClass {

DeviceRealize parent_realize;

       void (*parent_reset)(CPUState *cpu);
+    sparc_def_t *cpu_def;
   } SPARCCPUClass;

typedef struct SPARCCPU SPARCCPU;

diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index d606eb5..2994c09 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -25,8 +25,6 @@

//#define DEBUG_FEATURES -static int cpu_sparc_find_by_name(sparc_def_t *cpu_def, const char *cpu_model);

-
   /* CPUClass::reset() */
   static void sparc_cpu_reset(CPUState *s)
   {
@@ -111,17 +109,9 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char 
*cpu_model)
   {
       CPUSPARCState *env = &cpu->env;
       char *s = g_strdup(cpu_model);
-    char *featurestr, *name = strtok(s, ",");
-    sparc_def_t def1, *def = &def1;
+    char *featurestr = strtok(s, ",");
       Error *err = NULL;

- if (cpu_sparc_find_by_name(def, name) < 0) {

-        g_free(s);
-        return -1;
-    }
-
-    env->def = g_memdup(def, sizeof(*def));
-
       featurestr = strtok(NULL, ",");
       sparc_cpu_parse_features(CPU(cpu), featurestr, &err);
       g_free(s);
@@ -130,18 +120,18 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char 
*cpu_model)
           return -1;
       }

- env->version = def->iu_version;

-    env->fsr = def->fpu_version;
-    env->nwindows = def->nwindows;
+    env->version = env->def->iu_version;
+    env->fsr = env->def->fpu_version;
+    env->nwindows = env->def->nwindows;
   #if !defined(TARGET_SPARC64)
-    env->mmuregs[0] |= def->mmu_version;
+    env->mmuregs[0] |= env->def->mmu_version;
       cpu_sparc_set_id(env, 0);
-    env->mxccregs[7] |= def->mxcc_version;
+    env->mxccregs[7] |= env->def->mxcc_version;
   #else
-    env->mmu_version = def->mmu_version;
-    env->maxtl = def->maxtl;
-    env->version |= def->maxtl << 8;
-    env->version |= def->nwindows - 1;
+    env->mmu_version = env->def->mmu_version;
+    env->maxtl = env->def->maxtl;
+    env->version |= env->def->maxtl << 8;
+    env->version |= env->def->nwindows - 1;
   #endif
       return 0;
   }
@@ -149,8 +139,19 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char 
*cpu_model)
   SPARCCPU *cpu_sparc_init(const char *cpu_model)
   {
       SPARCCPU *cpu;
+    ObjectClass *oc;
+    char *str, *name;
+
+    str = g_strdup(cpu_model);
+    name = strtok(str, ",");

you can free 'str' once here:

         g_free(str);

+    oc = cpu_class_by_name(TYPE_SPARC_CPU, name);
+    if (oc == NULL) {
+        g_free(str);

drop

+        return NULL;
+    }
+    g_free(str);

drop

I'll fix it up if I have to respin series (note this function is removed within 
series)

[...]

Anyway I can't start Aurelien's image [1] with this commit:

Can't reproduce it locally (either dirty/clean tree rebuild)
with ./configure --enable-debug

$ sparc-softmmu/qemu-system-sparc -hda debian_etch_sparc_small.qcow2
qemu: Unable to find Sparc CPU definition

for this kind of error guest image doesn't matter as error happens during 
machine_init()

[1] https://people.debian.org/~aurel32/qemu/sparc/

It boots just fine with default CPU model:

./sparc-softmmu/qemu-system-sparc -hda debian_etch_sparc_small.qcow2

Sorry I'v been distracted by something else :/
I might have done something wrong, I'll test it again tomorrow.

and with explicitly specified one:

sparc-softmmu/qemu-system-sparc -cpu "Fujitsu MB86904"

I get from "info qom-tree"
...
  /device[0] (Fujitsu-MB86904-sparc-cpu)
...