All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC 00/13] CPU init cleanup + CPU model classes (v2)
@ 2012-08-16 16:58 Eduardo Habkost
  2012-08-16 16:59 ` [Qemu-devel] [RFC 01/13] target-i386/cpu.c: coding style fixes Eduardo Habkost
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: Eduardo Habkost @ 2012-08-16 16:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, stefanha, gleb, vijaymohan.pandarathil, jan.kiszka,
	mtosatti, mdroth, blauwirbel, avi, pbonzini, akong, lersek,
	afaerber

his is a new version of the previous CPU model classes RFC I have sent
previously, rebased on top of Igor's CPU properties series, and other series
from the list.

Igor's series: http://article.gmane.org/gmane.comp.emulators.qemu/165728

Considering that I had rebased Igor's series on top of other code,
it's probably easier to look directly at my git tree:
https://github.com/ehabkost/qemu-hacks/tree/work/cpu-model-classes-v1.10-2012-08-16

The following series were applied before this one, in this order:

 - -cpu help fixes from Peter Maydell (branch from-list/cpu-help-peter-2012-08-10)
 - CPU DeviceState series from Anthony (branch from-list/cpu-qdev-anthony-2012-08-10)
 - My "move CPU models to C" series (branch work/move-cpu-models-to-C-v1.4-2012-08-15)
 - Igor's CPU properties series (branch work/cpu-properties-igor-rebase-v3.2-2012-08-15)

Eduardo Habkost (13):
  target-i386/cpu.c: coding style fixes
  x86_cpudef_setup: coding style change
  i386: x86_def_t: rename 'flags' field
  move CPU x86 object creation to cpu.c
  rename x86_def_t to X86CPUDefinition
  move X86CPUDefinition to cpu-qom.h
  cpu_x86_create: move error handling to end of function
  cpu_x86_register: always initialize 'name' and 'features'
  kill cpu_x86_register()
  cpu_x86_create: reorder parsing of CPU model string and creation of
    CPU object
  check for NULL cpu_model outside cpu_x86_find_by_name
  register a class for each CPU model (v2)
  HACK to initialize types later

 hw/pc.c               |   2 +-
 hw/xen_machine_pv.c   |   2 +-
 target-i386/cpu-qom.h |  21 ++++
 target-i386/cpu.c     | 340 ++++++++++++++++++++++++++++++++------------------
 target-i386/cpu.h     |   5 +-
 target-i386/helper.c  |  24 ----
 6 files changed, 243 insertions(+), 151 deletions(-)

-- 
1.7.11.2

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [RFC 01/13] target-i386/cpu.c: coding style fixes
  2012-08-16 16:58 [Qemu-devel] [RFC 00/13] CPU init cleanup + CPU model classes (v2) Eduardo Habkost
@ 2012-08-16 16:59 ` Eduardo Habkost
  2012-08-16 16:59 ` [Qemu-devel] [RFC 02/13] x86_cpudef_setup: coding style change Eduardo Habkost
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eduardo Habkost @ 2012-08-16 16:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, stefanha, gleb, vijaymohan.pandarathil, jan.kiszka,
	mtosatti, mdroth, blauwirbel, avi, pbonzini, akong, lersek,
	afaerber

Changes to make checkpatch.pl happy:
- Use "sizeof(...)" instead of "sizeof (...)";
- Spaces around ":";
- Use spaces instead of tabs;
- Braces on if statements.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index b306805..5a20cc6 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1551,7 +1551,7 @@ static int cpudef_setfield(const char *name, const char *str, void *opaque)
         g_free((void *)def->name);
         def->name = g_strdup(str);
     } else if (!strcmp(name, "model_id")) {
-        strncpy(def->model_id, str, sizeof (def->model_id));
+        strncpy(def->model_id, str, sizeof(def->model_id));
     } else if (!strcmp(name, "level")) {
         setscalar(&def->level, str, &err)
     } else if (!strcmp(name, "vendor")) {
@@ -1587,7 +1587,7 @@ static int cpudef_setfield(const char *name, const char *str, void *opaque)
  */
 static int cpudef_register(QemuOpts *opts, void *opaque)
 {
-    x86_def_t *def = g_malloc0(sizeof (x86_def_t));
+    x86_def_t *def = g_malloc0(sizeof(x86_def_t));
 
     qemu_opt_foreach(opts, cpudef_setfield, def, 1);
     def->next = x86_defs;
@@ -1874,17 +1874,17 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         }
         break;
     case 0x8000000A:
-	if (env->cpuid_ext3_features & CPUID_EXT3_SVM) {
-		*eax = 0x00000001; /* SVM Revision */
-		*ebx = 0x00000010; /* nr of ASIDs */
-		*ecx = 0;
-		*edx = env->cpuid_svm_features; /* optional features */
-	} else {
-		*eax = 0;
-		*ebx = 0;
-		*ecx = 0;
-		*edx = 0;
-	}
+        if (env->cpuid_ext3_features & CPUID_EXT3_SVM) {
+            *eax = 0x00000001; /* SVM Revision */
+            *ebx = 0x00000010; /* nr of ASIDs */
+            *ecx = 0;
+            *edx = env->cpuid_svm_features; /* optional features */
+        } else {
+            *eax = 0;
+            *ebx = 0;
+            *ecx = 0;
+            *edx = 0;
+        }
         break;
     case 0xC0000000:
         *eax = env->cpuid_xlevel2;
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [RFC 02/13] x86_cpudef_setup: coding style change
  2012-08-16 16:58 [Qemu-devel] [RFC 00/13] CPU init cleanup + CPU model classes (v2) Eduardo Habkost
  2012-08-16 16:59 ` [Qemu-devel] [RFC 01/13] target-i386/cpu.c: coding style fixes Eduardo Habkost
@ 2012-08-16 16:59 ` Eduardo Habkost
  2012-08-16 16:59 ` [Qemu-devel] [RFC 03/13] i386: x86_def_t: rename 'flags' field Eduardo Habkost
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eduardo Habkost @ 2012-08-16 16:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, stefanha, gleb, vijaymohan.pandarathil, jan.kiszka,
	mtosatti, mdroth, blauwirbel, avi, pbonzini, akong, lersek,
	afaerber

Make source code lines shorter.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 5a20cc6..1382e8e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1611,20 +1611,23 @@ void x86_cpudef_setup(void)
     static const char *model_with_versions[] = { "qemu32", "qemu64", "athlon" };
 
     for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); ++i) {
-        builtin_x86_defs[i].next = x86_defs;
-        builtin_x86_defs[i].flags = 1;
+        x86_def_t *def = &builtin_x86_defs[i];
+        def->next = x86_defs;
+        def->flags = 1;
 
         /* Look for specific "cpudef" models that */
         /* have the QEMU version in .model_id */
         for (j = 0; j < ARRAY_SIZE(model_with_versions); j++) {
-            if (strcmp(model_with_versions[j], builtin_x86_defs[i].name) == 0) {
-                pstrcpy(builtin_x86_defs[i].model_id, sizeof(builtin_x86_defs[i].model_id), "QEMU Virtual CPU version ");
-                pstrcat(builtin_x86_defs[i].model_id, sizeof(builtin_x86_defs[i].model_id), qemu_get_version());
+            if (strcmp(model_with_versions[j], def->name) == 0) {
+                pstrcpy(def->model_id, sizeof(def->model_id),
+                        "QEMU Virtual CPU version ");
+                pstrcat(def->model_id, sizeof(def->model_id),
+                        qemu_get_version());
                 break;
             }
         }
 
-        x86_defs = &builtin_x86_defs[i];
+        x86_defs = def;
     }
 #if !defined(CONFIG_USER_ONLY)
     qemu_opts_foreach(qemu_find_opts("cpudef"), cpudef_register, NULL, 0);
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [RFC 03/13] i386: x86_def_t: rename 'flags' field
  2012-08-16 16:58 [Qemu-devel] [RFC 00/13] CPU init cleanup + CPU model classes (v2) Eduardo Habkost
  2012-08-16 16:59 ` [Qemu-devel] [RFC 01/13] target-i386/cpu.c: coding style fixes Eduardo Habkost
  2012-08-16 16:59 ` [Qemu-devel] [RFC 02/13] x86_cpudef_setup: coding style change Eduardo Habkost
@ 2012-08-16 16:59 ` Eduardo Habkost
  2012-08-16 16:59 ` [Qemu-devel] [RFC 04/13] move CPU x86 object creation to cpu.c Eduardo Habkost
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eduardo Habkost @ 2012-08-16 16:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, stefanha, gleb, vijaymohan.pandarathil, jan.kiszka,
	mtosatti, mdroth, blauwirbel, avi, pbonzini, akong, lersek,
	afaerber

The field is being used for a single purpose: to indicate if the CPU
model is a builtin one. So, instead of an opaque and confusing 'flags'
field name, use a boolean 'is_builtin' field to indicate if the model is
a built-in one.

No behavior change, just a field rename.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1382e8e..d7153f3 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -221,7 +221,7 @@ typedef struct x86_def_t {
     uint32_t xlevel;
     char model_id[48];
     int vendor_override;
-    uint32_t flags;
+    bool is_builtin;
     /* Store the results of Centaur's CPUID instructions */
     uint32_t ext4_features;
     uint32_t xlevel2;
@@ -1430,7 +1430,7 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
     char buf[256];
 
     for (def = x86_defs; def; def = def->next) {
-        snprintf(buf, sizeof (buf), def->flags ? "[%s]": "%s", def->name);
+        snprintf(buf, sizeof(buf), def->is_builtin ? "[%s]" : "%s", def->name);
         (*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf, def->model_id);
     }
     if (kvm_enabled()) {
@@ -1613,7 +1613,7 @@ void x86_cpudef_setup(void)
     for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); ++i) {
         x86_def_t *def = &builtin_x86_defs[i];
         def->next = x86_defs;
-        def->flags = 1;
+        def->is_builtin = true;
 
         /* Look for specific "cpudef" models that */
         /* have the QEMU version in .model_id */
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [RFC 04/13] move CPU x86 object creation to cpu.c
  2012-08-16 16:58 [Qemu-devel] [RFC 00/13] CPU init cleanup + CPU model classes (v2) Eduardo Habkost
                   ` (2 preceding siblings ...)
  2012-08-16 16:59 ` [Qemu-devel] [RFC 03/13] i386: x86_def_t: rename 'flags' field Eduardo Habkost
@ 2012-08-16 16:59 ` Eduardo Habkost
  2012-08-16 16:59 ` [Qemu-devel] [RFC 05/13] rename x86_def_t to X86CPUDefinition Eduardo Habkost
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eduardo Habkost @ 2012-08-16 16:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, stefanha, gleb, vijaymohan.pandarathil, jan.kiszka,
	mtosatti, mdroth, blauwirbel, avi, pbonzini, akong, lersek,
	afaerber

Eventually all of the init code will become just a simple object_new()
call, but right now we need to reorder and split some of the steps
invoved in the CPU model string parsing, and it will be easier to do
that inside cpu.c, by now.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/pc.c              |  2 +-
 hw/xen_machine_pv.c  |  2 +-
 target-i386/cpu.c    | 18 ++++++++++++++++++
 target-i386/cpu.h    |  4 ++--
 target-i386/helper.c | 24 ------------------------
 5 files changed, 22 insertions(+), 28 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 8c639f3..5f93d86 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -911,7 +911,7 @@ static X86CPU *pc_new_cpu(const char *cpu_model)
     X86CPU *cpu;
     CPUX86State *env;
 
-    cpu = cpu_x86_init(cpu_model);
+    cpu = cpu_x86_create(cpu_model);
     if (cpu == NULL) {
         fprintf(stderr, "Unable to find x86 CPU definition\n");
         exit(1);
diff --git a/hw/xen_machine_pv.c b/hw/xen_machine_pv.c
index 4017f43..f966da0 100644
--- a/hw/xen_machine_pv.c
+++ b/hw/xen_machine_pv.c
@@ -49,7 +49,7 @@ static void xen_init_pv(ram_addr_t ram_size,
         cpu_model = "qemu32";
 #endif
     }
-    cpu = cpu_x86_init(cpu_model);
+    cpu = cpu_x86_create(cpu_model);
     qdev_init_nofail(DEVICE(cpu));
     env = &cpu->env;
     env->halted = 1;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index d7153f3..5734570 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1503,6 +1503,24 @@ out:
     return 0;
 }
 
+X86CPU *cpu_x86_create(const char *cpu_model)
+{
+    X86CPU *cpu;
+    CPUX86State *env;
+
+    cpu = X86_CPU(object_new(TYPE_X86_CPU));
+    env = &cpu->env;
+    env->cpu_model_str = cpu_model;
+
+    if (cpu_x86_register(cpu, cpu_model) < 0) {
+        object_delete(OBJECT(cpu));
+        return NULL;
+    }
+
+    x86_cpu_realize(OBJECT(cpu), NULL);
+    return cpu;
+}
+
 #if !defined(CONFIG_USER_ONLY)
 /* interpret radix and convert from string to arbitrary scalar,
  * otherwise flag failure
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 3562612..6f48ba8 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -809,7 +809,6 @@ typedef struct CPUX86State {
 
 #include "cpu-qom.h"
 
-X86CPU *cpu_x86_init(const char *cpu_model);
 int cpu_x86_exec(CPUX86State *s);
 void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 void x86_cpudef_setup(void);
@@ -925,6 +924,7 @@ int cpu_x86_signal_handler(int host_signum, void *pinfo,
 void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
                    uint32_t *eax, uint32_t *ebx,
                    uint32_t *ecx, uint32_t *edx);
+X86CPU *cpu_x86_create(const char *cpu_model);
 int cpu_x86_register(X86CPU *cpu, const char *cpu_model);
 void cpu_clear_apic_feature(CPUX86State *env);
 void host_cpuid(uint32_t function, uint32_t count,
@@ -985,7 +985,7 @@ uint64_t cpu_get_tsc(CPUX86State *env);
 
 static inline CPUX86State *cpu_init(const char *cpu_model)
 {
-    X86CPU *cpu = cpu_x86_init(cpu_model);
+    X86CPU *cpu = cpu_x86_create(cpu_model);
     if (cpu == NULL) {
         return NULL;
     }
diff --git a/target-i386/helper.c b/target-i386/helper.c
index a0e4c89..d51d9eb 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1147,30 +1147,6 @@ int cpu_x86_get_descr_debug(CPUX86State *env, unsigned int selector,
     return 1;
 }
 
-X86CPU *cpu_x86_init(const char *cpu_model)
-{
-    X86CPU *cpu;
-    CPUX86State *env;
-    Error *error = NULL;
-
-    cpu = X86_CPU(object_new(TYPE_X86_CPU));
-    env = &cpu->env;
-    env->cpu_model_str = cpu_model;
-
-    if (cpu_x86_register(cpu, cpu_model) < 0) {
-        object_delete(OBJECT(cpu));
-        return NULL;
-    }
-
-    x86_cpu_realize(OBJECT(cpu), &error);
-    if (error_is_set(&error)) {
-        error_free(error);
-        object_delete(OBJECT(cpu));
-        return NULL;
-    }
-    return cpu;
-}
-
 #if !defined(CONFIG_USER_ONLY)
 void do_cpu_init(X86CPU *cpu)
 {
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [RFC 05/13] rename x86_def_t to X86CPUDefinition
  2012-08-16 16:58 [Qemu-devel] [RFC 00/13] CPU init cleanup + CPU model classes (v2) Eduardo Habkost
                   ` (3 preceding siblings ...)
  2012-08-16 16:59 ` [Qemu-devel] [RFC 04/13] move CPU x86 object creation to cpu.c Eduardo Habkost
@ 2012-08-16 16:59 ` Eduardo Habkost
  2012-08-16 16:59 ` [Qemu-devel] [RFC 06/13] move X86CPUDefinition to cpu-qom.h Eduardo Habkost
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eduardo Habkost @ 2012-08-16 16:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, stefanha, gleb, vijaymohan.pandarathil, jan.kiszka,
	mtosatti, mdroth, blauwirbel, avi, pbonzini, akong, lersek,
	afaerber

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 146 ++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 98 insertions(+), 48 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 5734570..0c816d9 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -207,9 +207,7 @@ static bool lookup_feature(uint32_t *pval, const char *s, const char *e,
     return found;
 }
 
-typedef struct x86_def_t {
-    struct x86_def_t *next;
-    const char *name;
+typedef struct X86CPUDefinition {
     uint32_t level;
     char vendor[CPUID_VENDOR_SZ + 1];
     int family;
@@ -221,13 +219,12 @@ typedef struct x86_def_t {
     uint32_t xlevel;
     char model_id[48];
     int vendor_override;
-    bool is_builtin;
     /* Store the results of Centaur's CPUID instructions */
     uint32_t ext4_features;
     uint32_t xlevel2;
     /* The feature bits on CPUID[EAX=7,ECX=0].EBX */
     uint32_t cpuid_7_0_ebx_features;
-} x86_def_t;
+} X86CPUDefinition;
 
 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
 #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
@@ -267,15 +264,24 @@ typedef struct x86_def_t {
           CPUID_EXT3_CR8LEG | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A)
 #define TCG_SVM_FEATURES 0
 
+
+typedef struct X86CPUModelTableEntry {
+    const char *name;
+    struct X86CPUModelTableEntry *next;
+    bool is_builtin;
+    X86CPUDefinition cpudef;
+} X86CPUModelTableEntry;
+
 /* maintains list of cpu model definitions
  */
-static x86_def_t *x86_defs = {NULL};
+static X86CPUModelTableEntry *x86_defs = {NULL};
 
 /* built-in cpu model definitions (deprecated)
  */
-static x86_def_t builtin_x86_defs[] = {
+static X86CPUModelTableEntry builtin_x86_defs[] = {
     {
-        .name = "qemu64",
+      .name = "qemu64",
+      .cpudef = {
         .level = 4,
         .vendor = CPUID_VENDOR_AMD,
         .family = 6,
@@ -290,9 +296,11 @@ static x86_def_t builtin_x86_defs[] = {
         .ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
             CPUID_EXT3_ABM | CPUID_EXT3_SSE4A,
         .xlevel = 0x8000000A,
+      },
     },
     {
-        .name = "phenom",
+      .name = "phenom",
+      .cpudef = {
         .level = 5,
         .vendor = CPUID_VENDOR_AMD,
         .family = 16,
@@ -316,9 +324,11 @@ static x86_def_t builtin_x86_defs[] = {
         .svm_features = CPUID_SVM_NPT | CPUID_SVM_LBRV,
         .xlevel = 0x8000001A,
         .model_id = "AMD Phenom(tm) 9550 Quad-Core Processor"
+      },
     },
     {
-        .name = "core2duo",
+      .name = "core2duo",
+      .cpudef = {
         .level = 10,
         .family = 6,
         .model = 15,
@@ -334,9 +344,11 @@ static x86_def_t builtin_x86_defs[] = {
         .ext3_features = CPUID_EXT3_LAHF_LM,
         .xlevel = 0x80000008,
         .model_id = "Intel(R) Core(TM)2 Duo CPU     T7700  @ 2.40GHz",
+      },
     },
     {
-        .name = "kvm64",
+      .name = "kvm64",
+      .cpudef = {
         .level = 5,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 15,
@@ -358,9 +370,11 @@ static x86_def_t builtin_x86_defs[] = {
         .ext3_features = 0,
         .xlevel = 0x80000008,
         .model_id = "Common KVM processor"
+      },
     },
     {
-        .name = "qemu32",
+      .name = "qemu32",
+      .cpudef = {
         .level = 4,
         .family = 6,
         .model = 3,
@@ -368,9 +382,11 @@ static x86_def_t builtin_x86_defs[] = {
         .features = PPRO_FEATURES,
         .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_POPCNT,
         .xlevel = 0x80000004,
+      },
     },
     {
-        .name = "kvm32",
+      .name = "kvm32",
+      .cpudef = {
         .level = 5,
         .family = 15,
         .model = 6,
@@ -382,9 +398,11 @@ static x86_def_t builtin_x86_defs[] = {
         .ext3_features = 0,
         .xlevel = 0x80000008,
         .model_id = "Common 32-bit KVM processor"
+      },
     },
     {
-        .name = "coreduo",
+      .name = "coreduo",
+      .cpudef = {
         .level = 10,
         .family = 6,
         .model = 14,
@@ -397,45 +415,55 @@ static x86_def_t builtin_x86_defs[] = {
         .ext2_features = CPUID_EXT2_NX,
         .xlevel = 0x80000008,
         .model_id = "Genuine Intel(R) CPU           T2600  @ 2.16GHz",
+      },
     },
     {
-        .name = "486",
+      .name = "486",
+      .cpudef = {
         .level = 1,
         .family = 4,
         .model = 0,
         .stepping = 0,
         .features = I486_FEATURES,
         .xlevel = 0,
+      },
     },
     {
-        .name = "pentium",
+      .name = "pentium",
+      .cpudef = {
         .level = 1,
         .family = 5,
         .model = 4,
         .stepping = 3,
         .features = PENTIUM_FEATURES,
         .xlevel = 0,
+      },
     },
     {
-        .name = "pentium2",
+      .name = "pentium2",
+      .cpudef = {
         .level = 2,
         .family = 6,
         .model = 5,
         .stepping = 2,
         .features = PENTIUM2_FEATURES,
         .xlevel = 0,
+      },
     },
     {
-        .name = "pentium3",
+      .name = "pentium3",
+      .cpudef = {
         .level = 2,
         .family = 6,
         .model = 7,
         .stepping = 3,
         .features = PENTIUM3_FEATURES,
         .xlevel = 0,
+      },
     },
     {
-        .name = "athlon",
+      .name = "athlon",
+      .cpudef = {
         .level = 2,
         .vendor = CPUID_VENDOR_AMD,
         .family = 6,
@@ -444,9 +472,11 @@ static x86_def_t builtin_x86_defs[] = {
         .features = PPRO_FEATURES | CPUID_PSE36 | CPUID_VME | CPUID_MTRR | CPUID_MCA,
         .ext2_features = (PPRO_FEATURES & EXT2_FEATURE_MASK) | CPUID_EXT2_MMXEXT | CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT,
         .xlevel = 0x80000008,
+      },
     },
     {
-        .name = "n270",
+      .name = "n270",
+      .cpudef = {
         /* original is on level 10 */
         .level = 5,
         .family = 6,
@@ -462,9 +492,11 @@ static x86_def_t builtin_x86_defs[] = {
         .ext3_features = CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Intel(R) Atom(TM) CPU N270   @ 1.60GHz",
+      },
     },
     {
-        .name = "Conroe",
+      .name = "Conroe",
+      .cpudef = {
         .level = 2,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
@@ -480,9 +512,11 @@ static x86_def_t builtin_x86_defs[] = {
         .ext3_features = CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Intel Celeron_4x0 (Conroe/Merom Class Core 2)",
+      },
     },
     {
-        .name = "Penryn",
+      .name = "Penryn",
+      .cpudef = {
         .level = 2,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
@@ -499,9 +533,11 @@ static x86_def_t builtin_x86_defs[] = {
         .ext3_features = CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Intel Core 2 Duo P9xxx (Penryn Class Core 2)",
+      },
     },
     {
-        .name = "Nehalem",
+      .name = "Nehalem",
+      .cpudef = {
         .level = 2,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
@@ -518,9 +554,11 @@ static x86_def_t builtin_x86_defs[] = {
         .ext3_features = CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Intel Core i7 9xx (Nehalem Class Core i7)",
+      },
     },
     {
-        .name = "Westmere",
+      .name = "Westmere",
+      .cpudef = {
         .level = 11,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
@@ -538,9 +576,11 @@ static x86_def_t builtin_x86_defs[] = {
         .ext3_features = CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Westmere E56xx/L56xx/X56xx (Nehalem-C)",
+      },
     },
     {
-        .name = "SandyBridge",
+      .name = "SandyBridge",
+      .cpudef = {
         .level = 0xd,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
@@ -561,9 +601,11 @@ static x86_def_t builtin_x86_defs[] = {
         .ext3_features = CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Intel Xeon E312xx (Sandy Bridge)",
+      },
     },
     {
-        .name = "Opteron_G1",
+      .name = "Opteron_G1",
+      .cpudef = {
         .level = 5,
         .vendor = CPUID_VENDOR_AMD,
         .family = 15,
@@ -583,9 +625,11 @@ static x86_def_t builtin_x86_defs[] = {
              CPUID_EXT2_TSC | CPUID_EXT2_PSE | CPUID_EXT2_DE | CPUID_EXT2_FPU,
         .xlevel = 0x80000008,
         .model_id = "AMD Opteron 240 (Gen 1 Class Opteron)",
+      },
     },
     {
-        .name = "Opteron_G2",
+      .name = "Opteron_G2",
+      .cpudef = {
         .level = 5,
         .vendor = CPUID_VENDOR_AMD,
         .family = 15,
@@ -607,9 +651,11 @@ static x86_def_t builtin_x86_defs[] = {
         .ext3_features = CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
         .xlevel = 0x80000008,
         .model_id = "AMD Opteron 22xx (Gen 2 Class Opteron)",
+      },
     },
     {
-        .name = "Opteron_G3",
+      .name = "Opteron_G3",
+      .cpudef = {
         .level = 5,
         .vendor = CPUID_VENDOR_AMD,
         .family = 15,
@@ -633,9 +679,11 @@ static x86_def_t builtin_x86_defs[] = {
              CPUID_EXT3_ABM | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
         .xlevel = 0x80000008,
         .model_id = "AMD Opteron 23xx (Gen 3 Class Opteron)",
+      },
     },
     {
-        .name = "Opteron_G4",
+      .name = "Opteron_G4",
+      .cpudef = {
         .level = 0xd,
         .vendor = CPUID_VENDOR_AMD,
         .family = 21,
@@ -663,6 +711,7 @@ static x86_def_t builtin_x86_defs[] = {
              CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000001A,
         .model_id = "AMD Opteron 62xx class CPU",
+      },
     },
 };
 
@@ -681,12 +730,12 @@ static int cpu_x86_fill_model_id(char *str)
     return 0;
 }
 
-static int cpu_x86_fill_host(x86_def_t *x86_cpu_def)
+/* Fill X86CPUDefinition struct with host CPU features */
+static int cpu_x86_fill_host(X86CPUDefinition *x86_cpu_def)
 {
     uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
     int i;
 
-    x86_cpu_def->name = "host";
     host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
     x86_cpu_def->level = eax;
     for (i = 0; i < 4; i++) {
@@ -761,7 +810,7 @@ static int unavailable_host_feature(struct model_features_t *f, uint32_t mask)
 static int check_features_against_host(X86CPU *cpu)
 {
     CPUX86State *env = &cpu->env;
-    x86_def_t host_def;
+    X86CPUDefinition host_def;
     uint32_t mask;
     int rv, i;
     struct model_features_t ft[] = {
@@ -1269,7 +1318,7 @@ x86_cpuid_set_vendor_override(Object *obj, Visitor *v, void *opaque,
     env->cpuid_vendor_override = value;
 }
 
-static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t *def, Error **errp)
+static void cpudef_2_x86_cpu(X86CPU *cpu, X86CPUDefinition *def, Error **errp)
 {
     CPUX86State *env = &cpu->env;
 
@@ -1366,10 +1415,10 @@ static void cpu_x86_set_props(X86CPU *cpu, QDict *features, Error **errp)
     }
 }
 
-static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
+static int cpu_x86_find_by_name(X86CPU *cpu, X86CPUDefinition *x86_cpu_def,
                                 const char *cpu_model, Error **errp)
 {
-    x86_def_t *def;
+    X86CPUModelTableEntry *def;
 
     if (!cpu_model) {
         error_set(errp, QERR_INVALID_PARAMETER_VALUE, "cpu_model", "NULL");
@@ -1426,12 +1475,12 @@ static void listflags(char *buf, int bufsize, uint32_t fbits,
 /* generate CPU information. */
 void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 {
-    x86_def_t *def;
+    X86CPUModelTableEntry *def;
     char buf[256];
 
     for (def = x86_defs; def; def = def->next) {
         snprintf(buf, sizeof(buf), def->is_builtin ? "[%s]" : "%s", def->name);
-        (*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf, def->model_id);
+        (*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf, def->cpudef.model_id);
     }
     if (kvm_enabled()) {
         (*cpu_fprintf)(f, "x86 %16s\n", "[host]");
@@ -1450,7 +1499,7 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
 {
     CpuDefinitionInfoList *cpu_list = NULL;
-    x86_def_t *def;
+    X86CPUModelTableEntry *def;
 
     for (def = x86_defs; def; def = def->next) {
         CpuDefinitionInfoList *entry;
@@ -1470,7 +1519,7 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
 
 int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 {
-    x86_def_t def1, *def = &def1;
+    X86CPUDefinition def1, *def = &def1;
     Error *error = NULL;
     QDict *features;
     char *name;
@@ -1558,16 +1607,17 @@ static void setfeatures(uint32_t *pval, const char *str,
     }
 }
 
-/* map config file options to x86_def_t form
+/* map config file options to X86CPUDefinition form
  */
 static int cpudef_setfield(const char *name, const char *str, void *opaque)
 {
-    x86_def_t *def = opaque;
+    X86CPUModelTableEntry *deft = opaque;
+    X86CPUDefinition *def = &deft->cpudef;
     int err = 0;
 
     if (!strcmp(name, "name")) {
-        g_free((void *)def->name);
-        def->name = g_strdup(str);
+        g_free((void *)deft->name);
+        deft->name = g_strdup(str);
     } else if (!strcmp(name, "model_id")) {
         strncpy(def->model_id, str, sizeof(def->model_id));
     } else if (!strcmp(name, "level")) {
@@ -1601,11 +1651,11 @@ static int cpudef_setfield(const char *name, const char *str, void *opaque)
     return (0);
 }
 
-/* register config file entry as x86_def_t
+/* register config file entry as X86CPUDefinition
  */
 static int cpudef_register(QemuOpts *opts, void *opaque)
 {
-    x86_def_t *def = g_malloc0(sizeof(x86_def_t));
+    X86CPUModelTableEntry *def = g_malloc0(sizeof(X86CPUModelTableEntry));
 
     qemu_opt_foreach(opts, cpudef_setfield, def, 1);
     def->next = x86_defs;
@@ -1629,7 +1679,7 @@ void x86_cpudef_setup(void)
     static const char *model_with_versions[] = { "qemu32", "qemu64", "athlon" };
 
     for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); ++i) {
-        x86_def_t *def = &builtin_x86_defs[i];
+        X86CPUModelTableEntry *def = &builtin_x86_defs[i];
         def->next = x86_defs;
         def->is_builtin = true;
 
@@ -1637,9 +1687,9 @@ void x86_cpudef_setup(void)
         /* have the QEMU version in .model_id */
         for (j = 0; j < ARRAY_SIZE(model_with_versions); j++) {
             if (strcmp(model_with_versions[j], def->name) == 0) {
-                pstrcpy(def->model_id, sizeof(def->model_id),
+                pstrcpy(def->cpudef.model_id, sizeof(def->cpudef.model_id),
                         "QEMU Virtual CPU version ");
-                pstrcat(def->model_id, sizeof(def->model_id),
+                pstrcat(def->cpudef.model_id, sizeof(def->cpudef.model_id),
                         qemu_get_version());
                 break;
             }
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [RFC 06/13] move X86CPUDefinition to cpu-qom.h
  2012-08-16 16:58 [Qemu-devel] [RFC 00/13] CPU init cleanup + CPU model classes (v2) Eduardo Habkost
                   ` (4 preceding siblings ...)
  2012-08-16 16:59 ` [Qemu-devel] [RFC 05/13] rename x86_def_t to X86CPUDefinition Eduardo Habkost
@ 2012-08-16 16:59 ` Eduardo Habkost
  2012-08-16 16:59 ` [Qemu-devel] [RFC 07/13] cpu_x86_create: move error handling to end of function Eduardo Habkost
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eduardo Habkost @ 2012-08-16 16:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, stefanha, gleb, vijaymohan.pandarathil, jan.kiszka,
	mtosatti, mdroth, blauwirbel, avi, pbonzini, akong, lersek,
	afaerber

From: Eduardo Habkost <ehabkost@thinair.local>

The struct will be used by X86CPUClass, too.

Signed-off-by: Eduardo Habkost <ehabkost@thinair.local>
---
 target-i386/cpu-qom.h | 19 +++++++++++++++++++
 target-i386/cpu.c     | 18 ------------------
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 5901140..2cd4f1a 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -37,6 +37,25 @@
 #define X86_CPU_GET_CLASS(obj) \
     OBJECT_GET_CLASS(X86CPUClass, (obj), TYPE_X86_CPU)
 
+typedef struct X86CPUDefinition {
+    uint32_t level;
+    char vendor[CPUID_VENDOR_SZ + 1];
+    int family;
+    int model;
+    int stepping;
+    int tsc_khz;
+    uint32_t features, ext_features, ext2_features, ext3_features;
+    uint32_t kvm_features, svm_features;
+    uint32_t xlevel;
+    char model_id[48];
+    int vendor_override;
+    /* Store the results of Centaur's CPUID instructions */
+    uint32_t ext4_features;
+    uint32_t xlevel2;
+    /* The feature bits on CPUID[EAX=7,ECX=0].EBX */
+    uint32_t cpuid_7_0_ebx_features;
+} X86CPUDefinition;
+
 /**
  * X86CPUClass:
  * @parent_reset: The parent class' reset handler.
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0c816d9..da7b5af 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -207,24 +207,6 @@ static bool lookup_feature(uint32_t *pval, const char *s, const char *e,
     return found;
 }
 
-typedef struct X86CPUDefinition {
-    uint32_t level;
-    char vendor[CPUID_VENDOR_SZ + 1];
-    int family;
-    int model;
-    int stepping;
-    int tsc_khz;
-    uint32_t features, ext_features, ext2_features, ext3_features;
-    uint32_t kvm_features, svm_features;
-    uint32_t xlevel;
-    char model_id[48];
-    int vendor_override;
-    /* Store the results of Centaur's CPUID instructions */
-    uint32_t ext4_features;
-    uint32_t xlevel2;
-    /* The feature bits on CPUID[EAX=7,ECX=0].EBX */
-    uint32_t cpuid_7_0_ebx_features;
-} X86CPUDefinition;
 
 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
 #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [RFC 07/13] cpu_x86_create: move error handling to end of function
  2012-08-16 16:58 [Qemu-devel] [RFC 00/13] CPU init cleanup + CPU model classes (v2) Eduardo Habkost
                   ` (5 preceding siblings ...)
  2012-08-16 16:59 ` [Qemu-devel] [RFC 06/13] move X86CPUDefinition to cpu-qom.h Eduardo Habkost
@ 2012-08-16 16:59 ` Eduardo Habkost
  2012-08-16 16:59 ` [Qemu-devel] [RFC 08/13] cpu_x86_register: always initialize 'name' and 'features' Eduardo Habkost
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eduardo Habkost @ 2012-08-16 16:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, stefanha, gleb, vijaymohan.pandarathil, jan.kiszka,
	mtosatti, mdroth, blauwirbel, avi, pbonzini, akong, lersek,
	afaerber

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index da7b5af..0bf62da 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1544,12 +1544,14 @@ X86CPU *cpu_x86_create(const char *cpu_model)
     env->cpu_model_str = cpu_model;
 
     if (cpu_x86_register(cpu, cpu_model) < 0) {
-        object_delete(OBJECT(cpu));
-        return NULL;
+        goto error;
     }
 
     x86_cpu_realize(OBJECT(cpu), NULL);
     return cpu;
+error:
+    object_delete(OBJECT(cpu));
+    return NULL;
 }
 
 #if !defined(CONFIG_USER_ONLY)
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [RFC 08/13] cpu_x86_register: always initialize 'name' and 'features'
  2012-08-16 16:58 [Qemu-devel] [RFC 00/13] CPU init cleanup + CPU model classes (v2) Eduardo Habkost
                   ` (6 preceding siblings ...)
  2012-08-16 16:59 ` [Qemu-devel] [RFC 07/13] cpu_x86_create: move error handling to end of function Eduardo Habkost
@ 2012-08-16 16:59 ` Eduardo Habkost
  2012-08-16 16:59 ` [Qemu-devel] [RFC 09/13] kill cpu_x86_register() Eduardo Habkost
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eduardo Habkost @ 2012-08-16 16:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, stefanha, gleb, vijaymohan.pandarathil, jan.kiszka,
	mtosatti, mdroth, blauwirbel, avi, pbonzini, akong, lersek,
	afaerber

It's safer to initialize them than require that
compat_normalize_cpu_model() always initialize them, even on errors.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0bf62da..c48de43 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1503,8 +1503,8 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 {
     X86CPUDefinition def1, *def = &def1;
     Error *error = NULL;
-    QDict *features;
-    char *name;
+    QDict *features = NULL;
+    char *name = NULL;
 
     /* for CPU subclasses should go into cpu_x86_init() before object_new() */
     compat_normalize_cpu_model(cpu_model, &name, &features, &error);
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [RFC 09/13] kill cpu_x86_register()
  2012-08-16 16:58 [Qemu-devel] [RFC 00/13] CPU init cleanup + CPU model classes (v2) Eduardo Habkost
                   ` (7 preceding siblings ...)
  2012-08-16 16:59 ` [Qemu-devel] [RFC 08/13] cpu_x86_register: always initialize 'name' and 'features' Eduardo Habkost
@ 2012-08-16 16:59 ` Eduardo Habkost
  2012-08-16 16:59 ` [Qemu-devel] [RFC 10/13] cpu_x86_create: reorder parsing of CPU model string and creation of CPU object Eduardo Habkost
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eduardo Habkost @ 2012-08-16 16:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, stefanha, gleb, vijaymohan.pandarathil, jan.kiszka,
	mtosatti, mdroth, blauwirbel, avi, pbonzini, akong, lersek,
	afaerber

Merge it with cpu_x86_create().

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 42 ++++++++++++++++++------------------------
 target-i386/cpu.h |  1 -
 2 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index c48de43..e7f32fc 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1499,57 +1499,51 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
     return cpu_list;
 }
 
-int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
+X86CPU *cpu_x86_create(const char *cpu_model)
 {
+    X86CPU *cpu;
+    CPUX86State *env;
     X86CPUDefinition def1, *def = &def1;
     Error *error = NULL;
     QDict *features = NULL;
     char *name = NULL;
 
+    cpu = X86_CPU(object_new(TYPE_X86_CPU));
+    env = &cpu->env;
+    env->cpu_model_str = cpu_model;
+
     /* for CPU subclasses should go into cpu_x86_init() before object_new() */
     compat_normalize_cpu_model(cpu_model, &name, &features, &error);
     if (error_is_set(&error)) {
-        goto out;
+        goto error;
     }
 
     /* this block should be replaced by CPU subclasses */
     memset(def, 0, sizeof(*def));
     if (cpu_x86_find_by_name(cpu, def, name, &error) < 0) {
-        goto out;
+        goto error;
     }
     cpudef_2_x86_cpu(cpu, def, &error);
 
     /* for CPU subclasses should go between object_new() and
      * x86_cpu_realize() */
     cpu_x86_set_props(cpu, features, &error);
-
-out:
-    QDECREF(features);
-    g_free(name);
     if (error_is_set(&error)) {
-        fprintf(stderr, "%s\n", error_get_pretty(error));
-        error_free(error);
-        return -1;
-    }
-    return 0;
-}
-
-X86CPU *cpu_x86_create(const char *cpu_model)
-{
-    X86CPU *cpu;
-    CPUX86State *env;
-
-    cpu = X86_CPU(object_new(TYPE_X86_CPU));
-    env = &cpu->env;
-    env->cpu_model_str = cpu_model;
-
-    if (cpu_x86_register(cpu, cpu_model) < 0) {
         goto error;
     }
 
+    QDECREF(features);
+    g_free(name);
+
     x86_cpu_realize(OBJECT(cpu), NULL);
     return cpu;
 error:
+    QDECREF(features);
+    g_free(name);
+    if (error_is_set(&error)) {
+        fprintf(stderr, "%s\n", error_get_pretty(error));
+        error_free(error);
+    }
     object_delete(OBJECT(cpu));
     return NULL;
 }
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 6f48ba8..f2ee814 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -925,7 +925,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
                    uint32_t *eax, uint32_t *ebx,
                    uint32_t *ecx, uint32_t *edx);
 X86CPU *cpu_x86_create(const char *cpu_model);
-int cpu_x86_register(X86CPU *cpu, const char *cpu_model);
 void cpu_clear_apic_feature(CPUX86State *env);
 void host_cpuid(uint32_t function, uint32_t count,
                 uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [RFC 10/13] cpu_x86_create: reorder parsing of CPU model string and creation of CPU object
  2012-08-16 16:58 [Qemu-devel] [RFC 00/13] CPU init cleanup + CPU model classes (v2) Eduardo Habkost
                   ` (8 preceding siblings ...)
  2012-08-16 16:59 ` [Qemu-devel] [RFC 09/13] kill cpu_x86_register() Eduardo Habkost
@ 2012-08-16 16:59 ` Eduardo Habkost
  2012-08-16 16:59 ` [Qemu-devel] [RFC 11/13] check for NULL cpu_model outside cpu_x86_find_by_name Eduardo Habkost
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Eduardo Habkost @ 2012-08-16 16:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, stefanha, gleb, vijaymohan.pandarathil, jan.kiszka,
	mtosatti, mdroth, blauwirbel, avi, pbonzini, akong, lersek,
	afaerber

A step towards making the creation of CPU objects use the CPU model name
as class name.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e7f32fc..2e24e00 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1508,18 +1508,19 @@ X86CPU *cpu_x86_create(const char *cpu_model)
     QDict *features = NULL;
     char *name = NULL;
 
-    cpu = X86_CPU(object_new(TYPE_X86_CPU));
-    env = &cpu->env;
-    env->cpu_model_str = cpu_model;
-
     /* for CPU subclasses should go into cpu_x86_init() before object_new() */
     compat_normalize_cpu_model(cpu_model, &name, &features, &error);
     if (error_is_set(&error)) {
-        goto error;
+        goto error_normalize;
     }
 
     /* this block should be replaced by CPU subclasses */
     memset(def, 0, sizeof(*def));
+
+    cpu = X86_CPU(object_new(TYPE_X86_CPU));
+    env = &cpu->env;
+    env->cpu_model_str = cpu_model;
+
     if (cpu_x86_find_by_name(cpu, def, name, &error) < 0) {
         goto error;
     }
@@ -1538,13 +1539,14 @@ X86CPU *cpu_x86_create(const char *cpu_model)
     x86_cpu_realize(OBJECT(cpu), NULL);
     return cpu;
 error:
+    object_delete(OBJECT(cpu));
+error_normalize:
     QDECREF(features);
     g_free(name);
     if (error_is_set(&error)) {
         fprintf(stderr, "%s\n", error_get_pretty(error));
         error_free(error);
     }
-    object_delete(OBJECT(cpu));
     return NULL;
 }
 
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [RFC 11/13] check for NULL cpu_model outside cpu_x86_find_by_name
  2012-08-16 16:58 [Qemu-devel] [RFC 00/13] CPU init cleanup + CPU model classes (v2) Eduardo Habkost
                   ` (9 preceding siblings ...)
  2012-08-16 16:59 ` [Qemu-devel] [RFC 10/13] cpu_x86_create: reorder parsing of CPU model string and creation of CPU object Eduardo Habkost
@ 2012-08-16 16:59 ` Eduardo Habkost
  2012-08-16 16:59 ` [Qemu-devel] [RFC 12/13] register a class for each CPU model (v2) Eduardo Habkost
  2012-08-16 16:59 ` [Qemu-devel] [RFC 13/13] HACK to initialize types later Eduardo Habkost
  12 siblings, 0 replies; 14+ messages in thread
From: Eduardo Habkost @ 2012-08-16 16:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, stefanha, gleb, vijaymohan.pandarathil, jan.kiszka,
	mtosatti, mdroth, blauwirbel, avi, pbonzini, akong, lersek,
	afaerber

cpu_x86_find_by_name() will go away when we move to CPU model classes.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 2e24e00..9209bb1 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1402,11 +1402,6 @@ static int cpu_x86_find_by_name(X86CPU *cpu, X86CPUDefinition *x86_cpu_def,
 {
     X86CPUModelTableEntry *def;
 
-    if (!cpu_model) {
-        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "cpu_model", "NULL");
-        return -1;
-    }
-
     for (def = x86_defs; def; def = def->next) {
         if (!strcmp(cpu_model, def->name)) {
             break;
@@ -1521,6 +1516,11 @@ X86CPU *cpu_x86_create(const char *cpu_model)
     env = &cpu->env;
     env->cpu_model_str = cpu_model;
 
+    if (!cpu_model) {
+        error_set(&error, QERR_INVALID_PARAMETER_VALUE, "cpu_model", "NULL");
+        goto error;
+    }
+
     if (cpu_x86_find_by_name(cpu, def, name, &error) < 0) {
         goto error;
     }
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [RFC 12/13] register a class for each CPU model (v2)
  2012-08-16 16:58 [Qemu-devel] [RFC 00/13] CPU init cleanup + CPU model classes (v2) Eduardo Habkost
                   ` (10 preceding siblings ...)
  2012-08-16 16:59 ` [Qemu-devel] [RFC 11/13] check for NULL cpu_model outside cpu_x86_find_by_name Eduardo Habkost
@ 2012-08-16 16:59 ` Eduardo Habkost
  2012-08-16 16:59 ` [Qemu-devel] [RFC 13/13] HACK to initialize types later Eduardo Habkost
  12 siblings, 0 replies; 14+ messages in thread
From: Eduardo Habkost @ 2012-08-16 16:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, stefanha, gleb, vijaymohan.pandarathil, jan.kiszka,
	mtosatti, mdroth, blauwirbel, avi, pbonzini, akong, lersek,
	afaerber

The trick here is to replace only the cpu_x86_find_by_name() logic and nothing
else. So, the only difference in relation to the previous code is that instead
of looking at the CPU model table on cpu_x86_create()/cpu_x86_find_by_name(),
we just use the right CPU class, that will already contain the CPU model
definition inside it.

I'm not sure what would be the best naming convention for the CPU
classes. I'm using "<arch>-cpu.<model name>" (as TYPE_X86CPU is
"<arch>-cpu").

Note: This patch won't work as-is, yet, because of initialization ordering
problems. The next patch will be a hack to make this work, by now.  Reordering
the initialization will be easier once we eliminate the support for cpudef
config sections.

Changes v1 -> v2:
 - Rebase on top of CPU properties series
 - Use a static type (with a different class init function) for the
   "-cpu host" class

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu-qom.h |   2 +
 target-i386/cpu.c     | 114 ++++++++++++++++++++++++++++++++++----------------
 2 files changed, 79 insertions(+), 37 deletions(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 2cd4f1a..6a003ff 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -68,6 +68,8 @@ typedef struct X86CPUClass {
     /*< public >*/
 
     void (*parent_reset)(CPUState *cpu);
+
+    X86CPUDefinition cpudef;
 } X86CPUClass;
 
 /**
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 9209bb1..4f718af 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1397,28 +1397,6 @@ static void cpu_x86_set_props(X86CPU *cpu, QDict *features, Error **errp)
     }
 }
 
-static int cpu_x86_find_by_name(X86CPU *cpu, X86CPUDefinition *x86_cpu_def,
-                                const char *cpu_model, Error **errp)
-{
-    X86CPUModelTableEntry *def;
-
-    for (def = x86_defs; def; def = def->next) {
-        if (!strcmp(cpu_model, def->name)) {
-            break;
-        }
-    }
-    if (kvm_enabled() && strcmp(cpu_model, "host") == 0) {
-        cpu_x86_fill_host(x86_cpu_def);
-    } else if (!def) {
-        error_set(errp, QERR_DEVICE_NOT_FOUND, cpu_model);
-        return -1;
-    } else {
-        memcpy(x86_cpu_def, def, sizeof(*def));
-    }
-
-    return 0;
-}
-
 /* generate a composite string into buf of all cpuid names in featureset
  * selected by fbits.  indicate truncation at bufsize in the event of overflow.
  * if flags, suppress names undefined in featureset.
@@ -1494,45 +1472,54 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
     return cpu_list;
 }
 
+/* Build class name for specific CPU model */
+static char *build_cpu_class_name(const char *model)
+{
+    return g_strdup_printf("%s.%s", TYPE_X86_CPU, model);
+}
+
 X86CPU *cpu_x86_create(const char *cpu_model)
 {
     X86CPU *cpu;
+    ObjectClass *cpu_class;
     CPUX86State *env;
     X86CPUDefinition def1, *def = &def1;
     Error *error = NULL;
     QDict *features = NULL;
     char *name = NULL;
+    char *class_name = NULL;
 
-    /* for CPU subclasses should go into cpu_x86_init() before object_new() */
     compat_normalize_cpu_model(cpu_model, &name, &features, &error);
     if (error_is_set(&error)) {
         goto error_normalize;
     }
 
-    /* this block should be replaced by CPU subclasses */
-    memset(def, 0, sizeof(*def));
-
-    cpu = X86_CPU(object_new(TYPE_X86_CPU));
-    env = &cpu->env;
-    env->cpu_model_str = cpu_model;
-
     if (!cpu_model) {
         error_set(&error, QERR_INVALID_PARAMETER_VALUE, "cpu_model", "NULL");
-        goto error;
+        goto error_normalize;
     }
 
-    if (cpu_x86_find_by_name(cpu, def, name, &error) < 0) {
-        goto error;
+    class_name = build_cpu_class_name(name);
+    cpu_class = object_class_by_name(class_name);
+    if (!cpu_class) {
+        error_set(&error, QERR_DEVICE_NOT_FOUND, cpu_model);
+        goto error_normalize;
     }
+
+    cpu = X86_CPU(object_new(class_name));
+    env = &cpu->env;
+    env->cpu_model_str = cpu_model;
+
+    *def = X86_CPU_GET_CLASS(cpu)->cpudef;
+
     cpudef_2_x86_cpu(cpu, def, &error);
 
-    /* for CPU subclasses should go between object_new() and
-     * x86_cpu_realize() */
     cpu_x86_set_props(cpu, features, &error);
     if (error_is_set(&error)) {
         goto error;
     }
 
+    g_free(class_name);
     QDECREF(features);
     g_free(name);
 
@@ -1541,6 +1528,7 @@ X86CPU *cpu_x86_create(const char *cpu_model)
 error:
     object_delete(OBJECT(cpu));
 error_normalize:
+    g_free(class_name);
     QDECREF(features);
     g_free(name);
     if (error_is_set(&error)) {
@@ -2206,15 +2194,67 @@ static const TypeInfo x86_cpu_type_info = {
     .name = TYPE_X86_CPU,
     .parent = TYPE_CPU,
     .instance_size = sizeof(X86CPU),
-    .instance_init = x86_cpu_initfn,
-    .abstract = false,
+    .abstract = true,
     .class_size = sizeof(X86CPUClass),
     .class_init = x86_cpu_common_class_init,
 };
 
+static void x86_cpu_class_init(ObjectClass *oc, void *data)
+{
+    X86CPUClass *xcc = X86_CPU_CLASS(oc);
+    X86CPUDefinition *def = data;
+
+    xcc->cpudef = *def;
+}
+
+/* Register a CPU class for a specific X86CPUDefinintion */
+static void x86_cpu_register_class(const char *name, X86CPUDefinition *def)
+{
+    char *class_name = build_cpu_class_name(name);
+    TypeInfo type = {
+        .name = class_name,
+        .parent = TYPE_X86_CPU,
+        .instance_size = sizeof(X86CPU),
+        .instance_init = x86_cpu_initfn,
+        .class_size = sizeof(X86CPUClass),
+        .class_init = x86_cpu_class_init,
+        .class_data = def,
+    };
+    type_register(&type);
+    g_free(class_name);
+}
+
+static void x86_cpu_host_class_init(ObjectClass *oc, void *data)
+{
+    X86CPUClass *xcc = X86_CPU_CLASS(oc);
+
+    cpu_x86_fill_host(&xcc->cpudef);
+}
+
+static TypeInfo x86_cpu_host_type_info = {
+    .name = TYPE_X86_CPU ".host",
+    .parent = TYPE_X86_CPU,
+    .instance_size = sizeof(X86CPU),
+    .instance_init = x86_cpu_initfn,
+    .class_size = sizeof(X86CPUClass),
+    .class_init = x86_cpu_host_class_init,
+};
+
 static void x86_cpu_register_types(void)
 {
+    X86CPUModelTableEntry *def;
+
+    /* Abstract X86CPU class */
     type_register_static(&x86_cpu_type_info);
+
+    /* -cpu host class */
+    type_register_static(&x86_cpu_host_type_info);
+
+    /* One class for each CPU model: */
+    for (def = x86_defs; def; def = def->next) {
+        x86_cpu_register_class(def->name, &def->cpudef);
+    }
+
 }
 
 type_init(x86_cpu_register_types)
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [RFC 13/13] HACK to initialize types later
  2012-08-16 16:58 [Qemu-devel] [RFC 00/13] CPU init cleanup + CPU model classes (v2) Eduardo Habkost
                   ` (11 preceding siblings ...)
  2012-08-16 16:59 ` [Qemu-devel] [RFC 12/13] register a class for each CPU model (v2) Eduardo Habkost
@ 2012-08-16 16:59 ` Eduardo Habkost
  12 siblings, 0 replies; 14+ messages in thread
From: Eduardo Habkost @ 2012-08-16 16:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, stefanha, gleb, vijaymohan.pandarathil, jan.kiszka,
	mtosatti, mdroth, blauwirbel, avi, pbonzini, akong, lersek,
	afaerber

This is only a temporary hack so that this series can be tested while we don't
kill the support for "cpudef" config sections.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4f718af..5c382f1 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1638,6 +1638,8 @@ void cpu_clear_apic_feature(CPUX86State *env)
 
 #endif /* !CONFIG_USER_ONLY */
 
+static void x86_cpu_register_types(void);
+
 /* register "cpudef" models defined in configuration file.  Here we first
  * preload any built-in definitions
  */
@@ -1668,6 +1670,8 @@ void x86_cpudef_setup(void)
 #if !defined(CONFIG_USER_ONLY)
     qemu_opts_foreach(qemu_find_opts("cpudef"), cpudef_register, NULL, 0);
 #endif
+
+    x86_cpu_register_types();
 }
 
 static void get_cpuid_vendor(CPUX86State *env, uint32_t *ebx,
@@ -2257,4 +2261,5 @@ static void x86_cpu_register_types(void)
 
 }
 
-type_init(x86_cpu_register_types)
+//HACK: the function is being called from x86_cpudef_setup()
+//type_init(x86_cpu_register_types)
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2012-08-16 18:23 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-16 16:58 [Qemu-devel] [RFC 00/13] CPU init cleanup + CPU model classes (v2) Eduardo Habkost
2012-08-16 16:59 ` [Qemu-devel] [RFC 01/13] target-i386/cpu.c: coding style fixes Eduardo Habkost
2012-08-16 16:59 ` [Qemu-devel] [RFC 02/13] x86_cpudef_setup: coding style change Eduardo Habkost
2012-08-16 16:59 ` [Qemu-devel] [RFC 03/13] i386: x86_def_t: rename 'flags' field Eduardo Habkost
2012-08-16 16:59 ` [Qemu-devel] [RFC 04/13] move CPU x86 object creation to cpu.c Eduardo Habkost
2012-08-16 16:59 ` [Qemu-devel] [RFC 05/13] rename x86_def_t to X86CPUDefinition Eduardo Habkost
2012-08-16 16:59 ` [Qemu-devel] [RFC 06/13] move X86CPUDefinition to cpu-qom.h Eduardo Habkost
2012-08-16 16:59 ` [Qemu-devel] [RFC 07/13] cpu_x86_create: move error handling to end of function Eduardo Habkost
2012-08-16 16:59 ` [Qemu-devel] [RFC 08/13] cpu_x86_register: always initialize 'name' and 'features' Eduardo Habkost
2012-08-16 16:59 ` [Qemu-devel] [RFC 09/13] kill cpu_x86_register() Eduardo Habkost
2012-08-16 16:59 ` [Qemu-devel] [RFC 10/13] cpu_x86_create: reorder parsing of CPU model string and creation of CPU object Eduardo Habkost
2012-08-16 16:59 ` [Qemu-devel] [RFC 11/13] check for NULL cpu_model outside cpu_x86_find_by_name Eduardo Habkost
2012-08-16 16:59 ` [Qemu-devel] [RFC 12/13] register a class for each CPU model (v2) Eduardo Habkost
2012-08-16 16:59 ` [Qemu-devel] [RFC 13/13] HACK to initialize types later Eduardo Habkost

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.