[Qemu-devel] [PATCH 0/9] x86 CPU subclasses

[Qemu-devel] [PATCH 0/9] x86 CPU subclasses

[Eduardo Habkost]

This is basically a x86 CPU subclasses RFC, preceded by some changes that I
would like to include even if this particular approach to the x86 CPU subclasses
doesn't get accepted (so the first patches are marked as PATCH instead of RFC).

The main differences from the RFC sent by Andreas previously is:
 - As we can't guarantee that class_init will get called only after KVM is
   initialized, the class for "host" initializes everything inside instance_init
 - Because of that difference, there's a separate base class for the predefined
   CPU models (TYPE_X86_PREDEF_CPU), that won't be used for TYPE_X86_HOST_CPU.
 - Instead of moving all x86_def_t fields inline to X86CPUClass, I am embedding
   x86_def_t inside PredefX86CPUClass, so kvm_cpu_fill_host() can keep getting
   x86_def_t* as argument, so they can still be used by
   kvm_check_features_against_host() _and_ by the -cpu "host" initialization
   code.

This series is based on:
 - [PATCH 0/2] Fixes for -cpu host KVM/SVM feature initialization
   Message-Id: <1356719854-16401-1-git-send-email-ehabkost@redhat.com>
 - [PATCH 00/20 v4] x86 CPU cleanup (wave 2)
   Message-Id: <1356724895-24118-1-git-send-email-imammedo@redhat.com>

Git tree for testing:
  git://github.com/ehabkost/qemu-hacks.git x86-cpu-model-classes.RFC.v3
  https://github.com/ehabkost/qemu-hacks/tree/x86-cpu-model-classes.RFC.v3


Eduardo Habkost (9):
  target-i386: Move CPU object creation to cpu.c
  target-i386: Make cpu_x86_create() get Error argument
  target-i386: Simplify cpu_x86_find_by_name() logic
  target-i386: Set feature string parsing results directly on CPU
    object
  target-i386: Move kvm_features/hypervisor initialization to
    cpu_x86_find_by_name()
  target-i386: Move CPU creation code to model name lookup function
  target-i386: CPU subclass for -cpu "host"
  target-i386: CPU subclasses for predefined CPU models
  target-i386: Unify CPU object creation on x86_cpu_create_from_name()

 target-i386/cpu-qom.h |   1 +
 target-i386/cpu.c     | 785 ++++++++++++++++++++++++++++++++++++++------------
 target-i386/cpu.h     |   4 +-
 target-i386/helper.c  |  26 +-
 4 files changed, 614 insertions(+), 202 deletions(-)

-- 
1.7.11.7

[Qemu-devel] [PATCH 1/9] target-i386: Move CPU object creation to cpu.c

[Eduardo Habkost]

As we will need to create the CPU object after splitting the CPU model
string (because we're going to use different subclasses for each CPU
model), move the CPU object creation to cpu_x86_register(), and at the
same time rename cpu_x86_register() to cpu_x86_create().

This will also simplify the CPU creation code to a trivial
cpu_x86_create()+cpu_x86_realize() sequence. This will be useful for
code that have to set additional properties before cpu_x86_realize() is
called (e.g. the PC CPU initialization code, that needs to set APIC IDs
depending on the CPU cores/threads topology).

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v2:
 - Move CPU creation code after cpu_model split (as we will eventually
   use just the "model name" part to find the right CPU class)
 - Small update on comment about feature string before cpu_x86_create()
   function
---
 target-i386/cpu.c    | 18 +++++++++++++++---
 target-i386/cpu.h    |  2 +-
 target-i386/helper.c |  9 ++-------
 3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8cd8b90..8c4be8b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1482,8 +1482,13 @@ static void filter_features_for_kvm(X86CPU *cpu)
 }
 #endif
 
-int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
+/* Create and initialize a X86CPU object, based on the full CPU model string
+ * (that may include "+feature,-feature,feature=xxx,feature" feature strings)
+ */
+X86CPU *cpu_x86_create(const char *cpu_model)
 {
+    X86CPU *cpu = NULL;
+    CPUX86State *env;
     x86_def_t def1, *def = &def1;
     QDict *props = NULL;
     Error *error = NULL;
@@ -1505,6 +1510,10 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
         goto out;
     }
 
+    cpu = X86_CPU(object_new(TYPE_X86_CPU));
+    env = &cpu->env;
+    env->cpu_model_str = cpu_model;
+
     def->kvm_features |= kvm_default_features;
     add_flagname_to_bitmaps("hypervisor", &def->features,
                             &def->ext_features, &def->ext2_features,
@@ -1525,9 +1534,12 @@ out:
     if (error) {
         fprintf(stderr, "%s\n", error_get_pretty(error));
         error_free(error);
-        return -1;
+        if (cpu) {
+            object_delete(OBJECT(cpu));
+        }
+        return NULL;
     }
-    return 0;
+    return cpu;
 }
 
 #if !defined(CONFIG_USER_ONLY)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 8d93393..21b3233 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -979,7 +979,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);
-int cpu_x86_register(X86CPU *cpu, const char *cpu_model);
+X86CPU *cpu_x86_create(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);
diff --git a/target-i386/helper.c b/target-i386/helper.c
index dca1360..7482c97 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1240,15 +1240,10 @@ int cpu_x86_get_descr_debug(CPUX86State *env, unsigned int selector,
 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));
+    cpu = cpu_x86_create(cpu_model);
+    if (!cpu) {
         return NULL;
     }
 
-- 
1.7.11.7

[Qemu-devel] [PATCH 2/9] target-i386: Make cpu_x86_create() get Error argument

[Eduardo Habkost]

Instead of forcing the caller to guess what went wrong while creating
the CPU object, return error information in a Error argument.

Also, as cpu_x86_create() won't print error messages itself anymore,
change cpu_x86_init() to print any error returned by cpu_x86_create()
or cpu_x86_realize().

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v2:
 - Fix include "qemu-error.h" to use the new "qemu/error-report.h"
 - Remove bogus error_free() call just after error_propagate()
---
 target-i386/cpu.c    |  5 ++---
 target-i386/cpu.h    |  2 +-
 target-i386/helper.c | 21 ++++++++++++++-------
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8c4be8b..8c4aecb 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1485,7 +1485,7 @@ static void filter_features_for_kvm(X86CPU *cpu)
 /* Create and initialize a X86CPU object, based on the full CPU model string
  * (that may include "+feature,-feature,feature=xxx,feature" feature strings)
  */
-X86CPU *cpu_x86_create(const char *cpu_model)
+X86CPU *cpu_x86_create(const char *cpu_model, Error **errp)
 {
     X86CPU *cpu = NULL;
     CPUX86State *env;
@@ -1532,8 +1532,7 @@ out:
     QDECREF(props);
     g_strfreev(model_pieces);
     if (error) {
-        fprintf(stderr, "%s\n", error_get_pretty(error));
-        error_free(error);
+        error_propagate(errp, error);
         if (cpu) {
             object_delete(OBJECT(cpu));
         }
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 21b3233..2b45c09 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -979,7 +979,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);
+X86CPU *cpu_x86_create(const char *cpu_model, Error **errp);
 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);
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 7482c97..8b6b4da 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -23,6 +23,7 @@
 #include "sysemu/sysemu.h"
 #include "monitor/monitor.h"
 #endif
+#include "qemu/error-report.h"
 
 //#define DEBUG_MMU
 
@@ -1239,21 +1240,27 @@ int cpu_x86_get_descr_debug(CPUX86State *env, unsigned int selector,
 
 X86CPU *cpu_x86_init(const char *cpu_model)
 {
-    X86CPU *cpu;
+    X86CPU *cpu = NULL;
     Error *error = NULL;
 
-    cpu = cpu_x86_create(cpu_model);
-    if (!cpu) {
-        return NULL;
+    cpu = cpu_x86_create(cpu_model, &error);
+    if (error) {
+        goto error;
     }
 
     x86_cpu_realize(OBJECT(cpu), &error);
     if (error) {
-        error_free(error);
-        object_delete(OBJECT(cpu));
-        return NULL;
+        goto error;
     }
     return cpu;
+
+error:
+    if (cpu) {
+        object_delete(OBJECT(cpu));
+    }
+    error_report("%s", error_get_pretty(error));
+    error_free(error);
+    return NULL;
 }
 
 #if !defined(CONFIG_USER_ONLY)
-- 
1.7.11.7

[Qemu-devel] [PATCH 3/9] target-i386: Simplify cpu_x86_find_by_name() logic

[Eduardo Habkost]

Move the check for "host" to beginning of function, so instead of a
confusing if/else-if/else mess we now have just two obvious if/else
blocks:

1) Special case for "host";
2) General case for CPU model lookup on x86_defs list.

This way, we will be able to easily move those two parts to separate
class instance_init functions.

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

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8c4aecb..fecd674 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1213,20 +1213,24 @@ static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t *def, Error **errp)
 
 static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
 {
-    x86_def_t *def;
 
-    for (def = x86_defs; def; def = def->next) {
-        if (name && !strcmp(name, def->name)) {
-            break;
-        }
-    }
     if (kvm_enabled() && name && strcmp(name, "host") == 0) {
 #ifdef CONFIG_KVM
         kvm_cpu_fill_host(x86_cpu_def);
 #endif
-    } else if (!def) {
-        return -1;
     } else {
+        x86_def_t *def;
+
+        for (def = x86_defs; def; def = def->next) {
+            if (name && !strcmp(name, def->name)) {
+                break;
+            }
+        }
+
+        if (!def) {
+            return -1;
+        }
+
         memcpy(x86_cpu_def, def, sizeof(*def));
         /* sysenter isn't supported on compatibility mode on AMD, syscall
          * isn't supported in compatibility mode on Intel.
-- 
1.7.11.7

[Qemu-devel] [RFC 4/9] target-i386: Set feature string parsing results directly on CPU object

[Eduardo Habkost]

Note that this is temporary: just like we would eventually kill usage of
x86_def_t inside cpu_x86_parse_featurestr(), we will eventually kill
usage of X86CPU inside that function as well.

This will help us keep the x86_def_t usage restricted to the CPU object
creation code.

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

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index fecd674..aa96535 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1266,7 +1266,7 @@ static void cpu_x86_set_props(X86CPU *cpu, QDict *features, Error **errp)
 
 /* Parse "+feature,-feature,feature=foo" CPU feature string
  */
-static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features,
+static int cpu_x86_parse_featurestr(X86CPU *cpu, char *features,
                                     QDict **props)
 {
     char *featurestr; /* Single 'key=value" string being parsed */
@@ -1337,7 +1337,7 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features,
                     fprintf(stderr, "bad numerical value %s\n", val);
                     goto error;
                 }
-                x86_cpu_def->tsc_khz = tsc_freq / 1000;
+                cpu->env.tsc_khz = tsc_freq / 1000;
             } else if (!strcmp(featurestr, "hv_spinlocks")) {
                 char *err;
                 numvalue = strtoul(val, &err, 0);
@@ -1363,20 +1363,20 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features,
             goto error;
         }
     }
-    x86_cpu_def->features |= plus_features;
-    x86_cpu_def->ext_features |= plus_ext_features;
-    x86_cpu_def->ext2_features |= plus_ext2_features;
-    x86_cpu_def->ext3_features |= plus_ext3_features;
-    x86_cpu_def->kvm_features |= plus_kvm_features;
-    x86_cpu_def->svm_features |= plus_svm_features;
-    x86_cpu_def->cpuid_7_0_ebx_features |= plus_7_0_ebx_features;
-    x86_cpu_def->features &= ~minus_features;
-    x86_cpu_def->ext_features &= ~minus_ext_features;
-    x86_cpu_def->ext2_features &= ~minus_ext2_features;
-    x86_cpu_def->ext3_features &= ~minus_ext3_features;
-    x86_cpu_def->kvm_features &= ~minus_kvm_features;
-    x86_cpu_def->svm_features &= ~minus_svm_features;
-    x86_cpu_def->cpuid_7_0_ebx_features &= ~minus_7_0_ebx_features;
+    cpu->env.cpuid_features |= plus_features;
+    cpu->env.cpuid_ext_features |= plus_ext_features;
+    cpu->env.cpuid_ext2_features |= plus_ext2_features;
+    cpu->env.cpuid_ext3_features |= plus_ext3_features;
+    cpu->env.cpuid_kvm_features |= plus_kvm_features;
+    cpu->env.cpuid_svm_features |= plus_svm_features;
+    cpu->env.cpuid_7_0_ebx_features |= plus_7_0_ebx_features;
+    cpu->env.cpuid_features &= ~minus_features;
+    cpu->env.cpuid_ext_features &= ~minus_ext_features;
+    cpu->env.cpuid_ext2_features &= ~minus_ext2_features;
+    cpu->env.cpuid_ext3_features &= ~minus_ext3_features;
+    cpu->env.cpuid_kvm_features &= ~minus_kvm_features;
+    cpu->env.cpuid_svm_features &= ~minus_svm_features;
+    cpu->env.cpuid_7_0_ebx_features &= ~minus_7_0_ebx_features;
     g_strfreev(feat_array);
     return 0;
 
@@ -1524,12 +1524,13 @@ X86CPU *cpu_x86_create(const char *cpu_model, Error **errp)
                             &def->ext3_features, &def->kvm_features,
                             &def->svm_features, &def->cpuid_7_0_ebx_features);
 
-    if (cpu_x86_parse_featurestr(def, features, &props) < 0) {
+    cpudef_2_x86_cpu(cpu, def, &error);
+
+    if (cpu_x86_parse_featurestr(cpu, features, &props) < 0) {
         error_setg(&error, "Invalid cpu_model string format: %s", cpu_model);
         goto out;
     }
 
-    cpudef_2_x86_cpu(cpu, def, &error);
     cpu_x86_set_props(cpu, props, &error);
 
 out:
-- 
1.7.11.7

[Qemu-devel] [RFC 5/9] target-i386: Move kvm_features/hypervisor initialization to cpu_x86_find_by_name()

[Eduardo Habkost]

Move the initialization to the cpu_x86_find_by_name(), inside the block
for predefined CPU models.

That code is not necessary for -cpu "host", because:

- kvm_features doesn't need to be set because kvm_cpu_fill_host()
  already sets all bits supported by the host.
- the CPUID_EXT_HYPERVISOR flag doesn't need to be manually set because
  kvm_arch_get_supported_cpuid() already sets it.

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 aa96535..b3bd2f5 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1244,6 +1244,15 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
             host_cpuid(0, 0, NULL, &ebx, &ecx, &edx);
             x86cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx);
         }
+
+        x86_cpu_def->kvm_features |= kvm_default_features;
+        add_flagname_to_bitmaps("hypervisor", &x86_cpu_def->features,
+                                &x86_cpu_def->ext_features,
+                                &x86_cpu_def->ext2_features,
+                                &x86_cpu_def->ext3_features,
+                                &x86_cpu_def->kvm_features,
+                                &x86_cpu_def->svm_features,
+                                &x86_cpu_def->cpuid_7_0_ebx_features);
     }
 
     return 0;
@@ -1518,12 +1527,6 @@ X86CPU *cpu_x86_create(const char *cpu_model, Error **errp)
     env = &cpu->env;
     env->cpu_model_str = cpu_model;
 
-    def->kvm_features |= kvm_default_features;
-    add_flagname_to_bitmaps("hypervisor", &def->features,
-                            &def->ext_features, &def->ext2_features,
-                            &def->ext3_features, &def->kvm_features,
-                            &def->svm_features, &def->cpuid_7_0_ebx_features);
-
     cpudef_2_x86_cpu(cpu, def, &error);
 
     if (cpu_x86_parse_featurestr(cpu, features, &props) < 0) {
-- 
1.7.11.7

[Qemu-devel] [RFC 6/9] target-i386: Move CPU creation code to model name lookup function

[Eduardo Habkost]

While moving the code, rename the resulting function from
cpu_x86_find_by_name() to x86_cpu_create_from_name().

Note that this adds duplicate object_new() and cpudef_2_x86_cpu() calls.
This is because the "host" and predefined CPU classes will be converted
in separate steps. After both parts are converted to use subclasses, we
can unify the class lookup and object creation code for both cases.

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

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index b3bd2f5..c824c08 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1211,12 +1211,19 @@ static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t *def, Error **errp)
     object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
 }
 
-static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
+static X86CPU *x86_cpu_create_from_name(const char *name, Error **errp)
 {
+    Error *error = NULL;
+    X86CPU *cpu = NULL;
+    x86_def_t def1, *x86_cpu_def = &def1;
+
+    memset(&def1, 0, sizeof(def1));
 
     if (kvm_enabled() && name && strcmp(name, "host") == 0) {
 #ifdef CONFIG_KVM
+        cpu = X86_CPU(object_new(TYPE_X86_CPU));
         kvm_cpu_fill_host(x86_cpu_def);
+        cpudef_2_x86_cpu(cpu, x86_cpu_def, &error);
 #endif
     } else {
         x86_def_t *def;
@@ -1228,9 +1235,11 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
         }
 
         if (!def) {
-            return -1;
+            error_setg(&error, "Unable to find CPU definition: %s", name);
+            goto out;
         }
 
+        cpu = X86_CPU(object_new(TYPE_X86_CPU));
         memcpy(x86_cpu_def, def, sizeof(*def));
         /* sysenter isn't supported on compatibility mode on AMD, syscall
          * isn't supported in compatibility mode on Intel.
@@ -1253,9 +1262,19 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
                                 &x86_cpu_def->kvm_features,
                                 &x86_cpu_def->svm_features,
                                 &x86_cpu_def->cpuid_7_0_ebx_features);
+
+        cpudef_2_x86_cpu(cpu, x86_cpu_def, &error);
     }
 
-    return 0;
+out:
+    if (error) {
+        error_propagate(errp, error);
+        if (cpu) {
+            object_delete(OBJECT(cpu));
+        }
+        return NULL;
+    }
+    return cpu;
 }
 
 /* Set features on X86CPU object based on a provide key,value list */
@@ -1502,14 +1521,11 @@ X86CPU *cpu_x86_create(const char *cpu_model, Error **errp)
 {
     X86CPU *cpu = NULL;
     CPUX86State *env;
-    x86_def_t def1, *def = &def1;
     QDict *props = NULL;
     Error *error = NULL;
     char *name, *features;
     gchar **model_pieces;
 
-    memset(def, 0, sizeof(*def));
-
     model_pieces = g_strsplit(cpu_model, ",", 2);
     if (!model_pieces[0]) {
         error_setg(&error, "Invalid/empty CPU model name");
@@ -1518,17 +1534,14 @@ X86CPU *cpu_x86_create(const char *cpu_model, Error **errp)
     name = model_pieces[0];
     features = model_pieces[1];
 
-    if (cpu_x86_find_by_name(def, name) < 0) {
-        error_setg(&error, "Unable to find CPU definition: %s", name);
+    cpu = x86_cpu_create_from_name(name, &error);
+    if (error) {
         goto out;
     }
 
-    cpu = X86_CPU(object_new(TYPE_X86_CPU));
     env = &cpu->env;
     env->cpu_model_str = cpu_model;
 
-    cpudef_2_x86_cpu(cpu, def, &error);
-
     if (cpu_x86_parse_featurestr(cpu, features, &props) < 0) {
         error_setg(&error, "Invalid cpu_model string format: %s", cpu_model);
         goto out;
-- 
1.7.11.7

[Qemu-devel] [RFC 7/9] target-i386: CPU subclass for -cpu "host"

[Eduardo Habkost]

Note that we are initializing the CPU features inside instance_init (and
not storing any CPU feature information inside the class struct) because
kvm_cpu_fill_host() needs KVM to be initialized, and we can't guarantee
that KVM will be initialized when class_init is called.

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

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index c824c08..2b6cc3b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -330,6 +330,14 @@ typedef struct x86_def_t {
 #define TCG_SVM_FEATURES 0
 #define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP)
 
+
+/* CPU class name definitions: */
+
+#define CPU_CLASS_NAME(name) (name "-" TYPE_X86_CPU)
+
+#define TYPE_X86_HOST_CPU CPU_CLASS_NAME("host")
+
+
 /* maintains list of cpu model definitions
  */
 static x86_def_t *x86_defs = {NULL};
@@ -1221,9 +1229,7 @@ static X86CPU *x86_cpu_create_from_name(const char *name, Error **errp)
 
     if (kvm_enabled() && name && strcmp(name, "host") == 0) {
 #ifdef CONFIG_KVM
-        cpu = X86_CPU(object_new(TYPE_X86_CPU));
-        kvm_cpu_fill_host(x86_cpu_def);
-        cpudef_2_x86_cpu(cpu, x86_cpu_def, &error);
+        cpu = X86_CPU(object_new(TYPE_X86_HOST_CPU));
 #endif
     } else {
         x86_def_t *def;
@@ -2168,9 +2174,42 @@ static const TypeInfo x86_cpu_type_info = {
     .class_init = x86_cpu_common_class_init,
 };
 
+#ifdef CONFIG_KVM
+
+static void x86_host_cpu_initfn(Object *obj)
+{
+    X86CPU *cpu = X86_CPU(obj);
+    Error *err = NULL;
+    x86_def_t cpudef;
+
+    memset(&cpudef, 0, sizeof(cpudef));
+    kvm_cpu_fill_host(&cpudef);
+    cpudef_2_x86_cpu(cpu, &cpudef, &err);
+
+    if (err) {
+        error_report("unexpected cpu init error: %s", error_get_pretty(err));
+        exit(1);
+    }
+}
+
+static const TypeInfo x86_host_cpu_type_info = {
+    .name = TYPE_X86_HOST_CPU,
+    .parent = TYPE_X86_CPU,
+    .instance_size = sizeof(X86CPU),
+    .instance_init = x86_host_cpu_initfn,
+    .abstract = false,
+    .class_size = sizeof(X86CPUClass),
+};
+
+#endif /* CONFIG_KVM */
+
+
 static void x86_cpu_register_types(void)
 {
     type_register_static(&x86_cpu_type_info);
+#ifdef CONFIG_KVM
+    type_register_static(&x86_host_cpu_type_info);
+#endif
 }
 
 type_init(x86_cpu_register_types)
-- 
1.7.11.7

[Qemu-devel] [RFC 8/9] target-i386: CPU subclasses for predefined CPU models

[Eduardo Habkost]

This uses a separate base class (TYPE_X86_PREDEF_CPU) for all the
predefined CPU models, because those classes will have a x86_def_t
struct embedded into the class struct (while TYPE_X86_HOST_CPU, for
example, doesn't have it). The code that was inside
x86_cpu_create_from_name() is now inside the instance_init function for
TYPE_X86_PREDEF_CPU.

The qemu_get_version() hacks inside x86_cpudef_setup() were moved to the
class_init functions of qemu64, qemu32, and athlon CPU models.

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

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 2b6cc3b..fa7fab0 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -273,8 +273,6 @@ static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features,
 }
 
 typedef struct x86_def_t {
-    struct x86_def_t *next;
-    const char *name;
     uint32_t level;
     char vendor[CPUID_VENDOR_SZ + 1];
     int family;
@@ -335,18 +333,51 @@ typedef struct x86_def_t {
 
 #define CPU_CLASS_NAME(name) (name "-" TYPE_X86_CPU)
 
+/* -cpu "host" */
 #define TYPE_X86_HOST_CPU CPU_CLASS_NAME("host")
 
 
-/* maintains list of cpu model definitions
- */
-static x86_def_t *x86_defs = {NULL};
+/* Base class for predefined CPU models: */
+#define TYPE_X86_PREDEF_CPU TYPE_X86_CPU "-predefined"
+
+#define PREDEF_X86_CPU_CLASS(klass) \
+    OBJECT_CLASS_CHECK(PredefX86CPUClass, (klass), TYPE_X86_PREDEF_CPU)
+#define PREDEF_X86_CPU(obj) \
+    OBJECT_CHECK(X86CPU, (obj), TYPE_X86_PREDEF_CPU)
+#define PREDEF_X86_CPU_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(PredefX86CPUClass, (obj), TYPE_X86_PREDEF_CPU)
 
-/* built-in cpu model definitions (deprecated)
+
+/**
+ * PredefX86CPUClass:
+ *
+ * A predefined x86 CPU model.
  */
-static x86_def_t builtin_x86_defs[] = {
-    {
-        .name = "qemu64",
+typedef struct PredefX86CPUClass {
+    /*< private >*/
+    X86CPUClass parent_class;
+    /*< public >*/
+
+    x86_def_t cpudef;
+} PredefX86CPUClass;
+
+
+static void x86_cpu_predef_initfn(Object *obj);
+
+static const TypeInfo predef_x86_cpu_type_info = {
+    .name = TYPE_X86_PREDEF_CPU,
+    .parent = TYPE_X86_CPU,
+    .instance_size = sizeof(X86CPU),
+    .instance_init = x86_cpu_predef_initfn,
+    .abstract = true,
+    .class_size = sizeof(PredefX86CPUClass),
+};
+
+
+static void x86_cpu_class_init_qemu64(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 4,
         .vendor = CPUID_VENDOR_AMD,
         .family = 6,
@@ -361,9 +392,24 @@ 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",
+    };
+    pstrcpy(cc->cpudef.model_id, sizeof(cc->cpudef.model_id), "QEMU Virtual CPU version ");
+    pstrcat(cc->cpudef.model_id, sizeof(cc->cpudef.model_id), qemu_get_version());
+}
+
+static const TypeInfo x86_cpu_qemu64_type_info = {
+    .name = CPU_CLASS_NAME("qemu64"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_qemu64,
+};
+
+static void x86_cpu_class_init_phenom(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 5,
         .vendor = CPUID_VENDOR_AMD,
         .family = 16,
@@ -387,9 +433,22 @@ 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",
+    };
+}
+
+static const TypeInfo x86_cpu_phenom_type_info = {
+    .name = CPU_CLASS_NAME("phenom"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_phenom,
+};
+
+static void x86_cpu_class_init_core2duo(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 10,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
@@ -406,9 +465,22 @@ 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",
+    };
+}
+
+static const TypeInfo x86_cpu_core2duo_type_info = {
+    .name = CPU_CLASS_NAME("core2duo"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_core2duo,
+};
+
+static void x86_cpu_class_init_kvm64(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 5,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 15,
@@ -430,9 +502,22 @@ static x86_def_t builtin_x86_defs[] = {
         .ext3_features = 0,
         .xlevel = 0x80000008,
         .model_id = "Common KVM processor"
-    },
-    {
-        .name = "qemu32",
+    };
+}
+
+static const TypeInfo x86_cpu_kvm64_type_info = {
+    .name = CPU_CLASS_NAME("kvm64"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_kvm64,
+};
+
+static void x86_cpu_class_init_qemu32(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 4,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
@@ -441,9 +526,24 @@ static x86_def_t builtin_x86_defs[] = {
         .features = PPRO_FEATURES,
         .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_POPCNT,
         .xlevel = 0x80000004,
-    },
-    {
-        .name = "kvm32",
+    };
+    pstrcpy(cc->cpudef.model_id, sizeof(cc->cpudef.model_id), "QEMU Virtual CPU version ");
+    pstrcat(cc->cpudef.model_id, sizeof(cc->cpudef.model_id), qemu_get_version());
+}
+
+static const TypeInfo x86_cpu_qemu32_type_info = {
+    .name = CPU_CLASS_NAME("qemu32"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_qemu32,
+};
+
+static void x86_cpu_class_init_kvm32(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 5,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 15,
@@ -456,9 +556,22 @@ static x86_def_t builtin_x86_defs[] = {
         .ext3_features = 0,
         .xlevel = 0x80000008,
         .model_id = "Common 32-bit KVM processor"
-    },
-    {
-        .name = "coreduo",
+    };
+}
+
+static const TypeInfo x86_cpu_kvm32_type_info = {
+    .name = CPU_CLASS_NAME("kvm32"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_kvm32,
+};
+
+static void x86_cpu_class_init_coreduo(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 10,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
@@ -472,9 +585,22 @@ 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",
+    };
+}
+
+static const TypeInfo x86_cpu_coreduo_type_info = {
+    .name = CPU_CLASS_NAME("coreduo"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_coreduo,
+};
+
+static void x86_cpu_class_init_486(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 1,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 4,
@@ -482,9 +608,22 @@ static x86_def_t builtin_x86_defs[] = {
         .stepping = 0,
         .features = I486_FEATURES,
         .xlevel = 0,
-    },
-    {
-        .name = "pentium",
+    };
+}
+
+static const TypeInfo x86_cpu_486_type_info = {
+    .name = CPU_CLASS_NAME("486"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_486,
+};
+
+static void x86_cpu_class_init_pentium(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 1,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 5,
@@ -492,9 +631,22 @@ static x86_def_t builtin_x86_defs[] = {
         .stepping = 3,
         .features = PENTIUM_FEATURES,
         .xlevel = 0,
-    },
-    {
-        .name = "pentium2",
+    };
+}
+
+static const TypeInfo x86_cpu_pentium_type_info = {
+    .name = CPU_CLASS_NAME("pentium"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_pentium,
+};
+
+static void x86_cpu_class_init_pentium2(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 2,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
@@ -502,9 +654,22 @@ static x86_def_t builtin_x86_defs[] = {
         .stepping = 2,
         .features = PENTIUM2_FEATURES,
         .xlevel = 0,
-    },
-    {
-        .name = "pentium3",
+    };
+}
+
+static const TypeInfo x86_cpu_pentium2_type_info = {
+    .name = CPU_CLASS_NAME("pentium2"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_pentium2,
+};
+
+static void x86_cpu_class_init_pentium3(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 2,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
@@ -512,9 +677,22 @@ static x86_def_t builtin_x86_defs[] = {
         .stepping = 3,
         .features = PENTIUM3_FEATURES,
         .xlevel = 0,
-    },
-    {
-        .name = "athlon",
+    };
+}
+
+static const TypeInfo x86_cpu_pentium3_type_info = {
+    .name = CPU_CLASS_NAME("pentium3"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_pentium3,
+};
+
+static void x86_cpu_class_init_athlon(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 2,
         .vendor = CPUID_VENDOR_AMD,
         .family = 6,
@@ -525,9 +703,24 @@ static x86_def_t builtin_x86_defs[] = {
         .ext2_features = (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
             CPUID_EXT2_MMXEXT | CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT,
         .xlevel = 0x80000008,
-    },
-    {
-        .name = "n270",
+    };
+    pstrcpy(cc->cpudef.model_id, sizeof(cc->cpudef.model_id), "QEMU Virtual CPU version ");
+    pstrcat(cc->cpudef.model_id, sizeof(cc->cpudef.model_id), qemu_get_version());
+}
+
+static const TypeInfo x86_cpu_athlon_type_info = {
+    .name = CPU_CLASS_NAME("athlon"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_athlon,
+};
+
+static void x86_cpu_class_init_n270(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         /* original is on level 10 */
         .level = 5,
         .vendor = CPUID_VENDOR_INTEL,
@@ -545,9 +738,22 @@ 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",
+    };
+}
+
+static const TypeInfo x86_cpu_n270_type_info = {
+    .name = CPU_CLASS_NAME("n270"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_n270,
+};
+
+static void x86_cpu_class_init_Conroe(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 2,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
@@ -563,9 +769,22 @@ 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",
+    };
+}
+
+static const TypeInfo x86_cpu_Conroe_type_info = {
+    .name = CPU_CLASS_NAME("Conroe"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_Conroe,
+};
+
+static void x86_cpu_class_init_Penryn(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 2,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
@@ -582,9 +801,22 @@ 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",
+    };
+}
+
+static const TypeInfo x86_cpu_Penryn_type_info = {
+    .name = CPU_CLASS_NAME("Penryn"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_Penryn,
+};
+
+static void x86_cpu_class_init_Nehalem(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 2,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
@@ -601,9 +833,22 @@ 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",
+    };
+}
+
+static const TypeInfo x86_cpu_Nehalem_type_info = {
+    .name = CPU_CLASS_NAME("Nehalem"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_Nehalem,
+};
+
+static void x86_cpu_class_init_Westmere(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 11,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
@@ -621,9 +866,22 @@ 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",
+    };
+}
+
+static const TypeInfo x86_cpu_Westmere_type_info = {
+    .name = CPU_CLASS_NAME("Westmere"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_Westmere,
+};
+
+static void x86_cpu_class_init_SandyBridge(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 0xd,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
@@ -644,9 +902,22 @@ static x86_def_t builtin_x86_defs[] = {
         .ext3_features = CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Intel Xeon E312xx (Sandy Bridge)",
-    },
-    {
-        .name = "Haswell",
+    };
+}
+
+static const TypeInfo x86_cpu_SandyBridge_type_info = {
+    .name = CPU_CLASS_NAME("SandyBridge"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_SandyBridge,
+};
+
+static void x86_cpu_class_init_Haswell(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 0xd,
         .vendor = CPUID_VENDOR_INTEL,
         .family = 6,
@@ -672,9 +943,22 @@ static x86_def_t builtin_x86_defs[] = {
             CPUID_7_0_EBX_RTM,
         .xlevel = 0x8000000A,
         .model_id = "Intel Core Processor (Haswell)",
-    },
-    {
-        .name = "Opteron_G1",
+    };
+}
+
+static const TypeInfo x86_cpu_Haswell_type_info = {
+    .name = CPU_CLASS_NAME("Haswell"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_Haswell,
+};
+
+static void x86_cpu_class_init_Opteron_G1(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 5,
         .vendor = CPUID_VENDOR_AMD,
         .family = 15,
@@ -694,9 +978,22 @@ 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",
+    };
+}
+
+static const TypeInfo x86_cpu_Opteron_G1_type_info = {
+    .name = CPU_CLASS_NAME("Opteron_G1"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_Opteron_G1,
+};
+
+static void x86_cpu_class_init_Opteron_G2(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 5,
         .vendor = CPUID_VENDOR_AMD,
         .family = 15,
@@ -718,9 +1015,22 @@ 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",
+    };
+}
+
+static const TypeInfo x86_cpu_Opteron_G2_type_info = {
+    .name = CPU_CLASS_NAME("Opteron_G2"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_Opteron_G2,
+};
+
+static void x86_cpu_class_init_Opteron_G3(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 5,
         .vendor = CPUID_VENDOR_AMD,
         .family = 15,
@@ -744,9 +1054,22 @@ 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",
+    };
+}
+
+static const TypeInfo x86_cpu_Opteron_G3_type_info = {
+    .name = CPU_CLASS_NAME("Opteron_G3"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_Opteron_G3,
+};
+
+static void x86_cpu_class_init_Opteron_G4(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 0xd,
         .vendor = CPUID_VENDOR_AMD,
         .family = 21,
@@ -774,9 +1097,22 @@ static x86_def_t builtin_x86_defs[] = {
              CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000001A,
         .model_id = "AMD Opteron 62xx class CPU",
-    },
-    {
-        .name = "Opteron_G5",
+    };
+}
+
+static const TypeInfo x86_cpu_Opteron_G4_type_info = {
+    .name = CPU_CLASS_NAME("Opteron_G4"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_Opteron_G4,
+};
+
+static void x86_cpu_class_init_Opteron_G5(ObjectClass *oc, void *data)
+{
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_CLASS(oc);
+    cc->cpudef = (x86_def_t) {
         .level = 0xd,
         .vendor = CPUID_VENDOR_AMD,
         .family = 21,
@@ -804,7 +1140,16 @@ static x86_def_t builtin_x86_defs[] = {
              CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000001A,
         .model_id = "AMD Opteron 63xx class CPU",
-    },
+    };
+}
+
+static const TypeInfo x86_cpu_Opteron_G5_type_info = {
+    .name = CPU_CLASS_NAME("Opteron_G5"),
+    .parent = TYPE_X86_PREDEF_CPU,
+    .instance_size = sizeof(X86CPU),
+    .abstract = false,
+    .class_size = sizeof(PredefX86CPUClass),
+    .class_init = x86_cpu_class_init_Opteron_G5,
 };
 
 static void x86cpu_vendor_words2str(char *dst, uint32_t ebx, uint32_t ecx,
@@ -847,7 +1192,6 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
 
     assert(kvm_enabled());
 
-    x86_cpu_def->name = "host";
     host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
     x86cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx);
 
@@ -1219,60 +1563,67 @@ static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t *def, Error **errp)
     object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
 }
 
+static void x86_cpu_predef_initfn(Object *obj)
+{
+    X86CPU *cpu = X86_CPU(obj);
+    PredefX86CPUClass *cc = PREDEF_X86_CPU_GET_CLASS(cpu);
+    Error *error = NULL;
+    x86_def_t cpudef;
+
+    memcpy(&cpudef, &cc->cpudef, sizeof(cpudef));
+    /* sysenter isn't supported on compatibility mode on AMD, syscall
+     * isn't supported in compatibility mode on Intel.
+     * Normally we advertise the actual cpu vendor, but you can override
+     * this using the 'vendor' property if you want to use KVM's
+     * sysenter/syscall emulation in compatibility mode and when doing
+     * cross vendor migration
+     */
+    if (kvm_enabled()) {
+        uint32_t  ebx = 0, ecx = 0, edx = 0;
+        host_cpuid(0, 0, NULL, &ebx, &ecx, &edx);
+        x86cpu_vendor_words2str(cpudef.vendor, ebx, edx, ecx);
+    }
+
+    cpudef.kvm_features |= kvm_default_features;
+    add_flagname_to_bitmaps("hypervisor", &cpudef.features,
+                            &cpudef.ext_features,
+                            &cpudef.ext2_features,
+                            &cpudef.ext3_features,
+                            &cpudef.kvm_features,
+                            &cpudef.svm_features,
+                            &cpudef.cpuid_7_0_ebx_features);
+
+    cpudef_2_x86_cpu(cpu, &cpudef, &error);
+
+    if (error) {
+        error_report("cpu_init: %s", error_get_pretty(error));
+        exit(1);
+    }
+}
+
 static X86CPU *x86_cpu_create_from_name(const char *name, Error **errp)
 {
     Error *error = NULL;
     X86CPU *cpu = NULL;
-    x86_def_t def1, *x86_cpu_def = &def1;
+    char *class_name = g_strdup_printf(CPU_CLASS_NAME("%s"), name);
 
-    memset(&def1, 0, sizeof(def1));
 
     if (kvm_enabled() && name && strcmp(name, "host") == 0) {
 #ifdef CONFIG_KVM
         cpu = X86_CPU(object_new(TYPE_X86_HOST_CPU));
 #endif
     } else {
-        x86_def_t *def;
-
-        for (def = x86_defs; def; def = def->next) {
-            if (name && !strcmp(name, def->name)) {
-                break;
-            }
-        }
 
-        if (!def) {
+        if (!object_class_by_name(class_name)) {
             error_setg(&error, "Unable to find CPU definition: %s", name);
             goto out;
         }
 
-        cpu = X86_CPU(object_new(TYPE_X86_CPU));
-        memcpy(x86_cpu_def, def, sizeof(*def));
-        /* sysenter isn't supported on compatibility mode on AMD, syscall
-         * isn't supported in compatibility mode on Intel.
-         * Normally we advertise the actual cpu vendor, but you can override
-         * this using the 'vendor' property if you want to use KVM's
-         * sysenter/syscall emulation in compatibility mode and when doing
-         * cross vendor migration
-         */
-        if (kvm_enabled()) {
-            uint32_t  ebx = 0, ecx = 0, edx = 0;
-            host_cpuid(0, 0, NULL, &ebx, &ecx, &edx);
-            x86cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx);
-        }
-
-        x86_cpu_def->kvm_features |= kvm_default_features;
-        add_flagname_to_bitmaps("hypervisor", &x86_cpu_def->features,
-                                &x86_cpu_def->ext_features,
-                                &x86_cpu_def->ext2_features,
-                                &x86_cpu_def->ext3_features,
-                                &x86_cpu_def->kvm_features,
-                                &x86_cpu_def->svm_features,
-                                &x86_cpu_def->cpuid_7_0_ebx_features);
-
-        cpudef_2_x86_cpu(cpu, x86_cpu_def, &error);
+        cpu = X86_CPU(object_new(class_name));
     }
 
 out:
+    g_free(class_name);
     if (error) {
         error_propagate(errp, error);
         if (cpu) {
@@ -1452,13 +1803,9 @@ 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;
     char buf[256];
 
-    for (def = x86_defs; def; def = def->next) {
-        snprintf(buf, sizeof(buf), "%s", def->name);
-        (*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf, def->model_id);
-    }
+    /***** TODO: class listing */
     if (kvm_enabled()) {
         (*cpu_fprintf)(f, "x86 %16s\n", "[host]");
     }
@@ -1476,20 +1823,7 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
 {
     CpuDefinitionInfoList *cpu_list = NULL;
-    x86_def_t *def;
-
-    for (def = x86_defs; def; def = def->next) {
-        CpuDefinitionInfoList *entry;
-        CpuDefinitionInfo *info;
-
-        info = g_malloc0(sizeof(*info));
-        info->name = g_strdup(def->name);
-
-        entry = g_malloc0(sizeof(*entry));
-        entry->value = info;
-        entry->next = cpu_list;
-        cpu_list = entry;
-    }
+    /***** TODO: class listing */
 
     return cpu_list;
 }
@@ -1577,33 +1911,6 @@ void cpu_clear_apic_feature(CPUX86State *env)
 
 #endif /* !CONFIG_USER_ONLY */
 
-/* Initialize list of CPU models, filling some non-static fields if necessary
- */
-void x86_cpudef_setup(void)
-{
-    int i, j;
-    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];
-        def->next = x86_defs;
-
-        /* 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], 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 = def;
-    }
-}
-
 static void get_cpuid_vendor(CPUX86State *env, uint32_t *ebx,
                              uint32_t *ecx, uint32_t *edx)
 {
@@ -2169,7 +2476,7 @@ static const TypeInfo x86_cpu_type_info = {
     .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,
 };
@@ -2210,6 +2517,31 @@ static void x86_cpu_register_types(void)
 #ifdef CONFIG_KVM
     type_register_static(&x86_host_cpu_type_info);
 #endif
+    type_register_static(&predef_x86_cpu_type_info);
+    type_register_static(&x86_cpu_qemu64_type_info);
+    type_register_static(&x86_cpu_phenom_type_info);
+    type_register_static(&x86_cpu_core2duo_type_info);
+    type_register_static(&x86_cpu_kvm64_type_info);
+    type_register_static(&x86_cpu_qemu32_type_info);
+    type_register_static(&x86_cpu_kvm32_type_info);
+    type_register_static(&x86_cpu_coreduo_type_info);
+    type_register_static(&x86_cpu_486_type_info);
+    type_register_static(&x86_cpu_pentium_type_info);
+    type_register_static(&x86_cpu_pentium2_type_info);
+    type_register_static(&x86_cpu_pentium3_type_info);
+    type_register_static(&x86_cpu_athlon_type_info);
+    type_register_static(&x86_cpu_n270_type_info);
+    type_register_static(&x86_cpu_Conroe_type_info);
+    type_register_static(&x86_cpu_Penryn_type_info);
+    type_register_static(&x86_cpu_Nehalem_type_info);
+    type_register_static(&x86_cpu_Westmere_type_info);
+    type_register_static(&x86_cpu_SandyBridge_type_info);
+    type_register_static(&x86_cpu_Haswell_type_info);
+    type_register_static(&x86_cpu_Opteron_G1_type_info);
+    type_register_static(&x86_cpu_Opteron_G2_type_info);
+    type_register_static(&x86_cpu_Opteron_G3_type_info);
+    type_register_static(&x86_cpu_Opteron_G4_type_info);
+    type_register_static(&x86_cpu_Opteron_G5_type_info);
 }
 
 type_init(x86_cpu_register_types)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 2b45c09..6c41500 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -864,7 +864,6 @@ typedef struct CPUX86State {
 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);
 int cpu_x86_support_mca_broadcast(CPUX86State *env);
 
 int cpu_get_pic_interrupt(CPUX86State *s);
@@ -1046,7 +1045,6 @@ static inline CPUX86State *cpu_init(const char *cpu_model)
 #define cpu_gen_code cpu_x86_gen_code
 #define cpu_signal_handler cpu_x86_signal_handler
 #define cpu_list x86_cpu_list
-#define cpudef_setup	x86_cpudef_setup
 
 #define CPU_SAVE_VERSION 12
 
-- 
1.7.11.7

[Qemu-devel] [RFC 9/9] target-i386: Unify CPU object creation on x86_cpu_create_from_name()

[Eduardo Habkost]

To keep the kvm_enabled() check for "host" working, a 'kvm_required'
field was added to X86CPUClass.

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

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 332916a..fc58f83 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -49,6 +49,7 @@ typedef struct X86CPUClass {
     /*< public >*/
 
     void (*parent_reset)(CPUState *cpu);
+    bool kvm_required;
 } X86CPUClass;
 
 /**
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index fa7fab0..7beacdf 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1605,23 +1605,24 @@ static X86CPU *x86_cpu_create_from_name(const char *name, Error **errp)
 {
     Error *error = NULL;
     X86CPU *cpu = NULL;
+    ObjectClass *oc;
+    X86CPUClass *cc;
     char *class_name = g_strdup_printf(CPU_CLASS_NAME("%s"), name);
 
+    oc = object_class_by_name(class_name);
+    if (!oc) {
+        error_setg(&error, "Unable to find CPU definition: %s", name);
+        goto out;
+    }
 
-    if (kvm_enabled() && name && strcmp(name, "host") == 0) {
-#ifdef CONFIG_KVM
-        cpu = X86_CPU(object_new(TYPE_X86_HOST_CPU));
-#endif
-    } else {
-
-        if (!object_class_by_name(class_name)) {
-            error_setg(&error, "Unable to find CPU definition: %s", name);
-            goto out;
-        }
-
-        cpu = X86_CPU(object_new(class_name));
+    cc = X86_CPU_CLASS(oc);
+    if (cc->kvm_required && !kvm_enabled()) {
+        error_setg(&error, "CPU model '%s' requires KVM", name);
+        goto out;
     }
 
+    cpu = X86_CPU(object_new(class_name));
+
 out:
     g_free(class_name);
     if (error) {
@@ -2499,6 +2500,12 @@ static void x86_host_cpu_initfn(Object *obj)
     }
 }
 
+static void x86_host_cpu_class_init(ObjectClass *oc, void *data)
+{
+    X86CPUClass *cc = X86_CPU_CLASS(oc);
+    cc->kvm_required = true;
+}
+
 static const TypeInfo x86_host_cpu_type_info = {
     .name = TYPE_X86_HOST_CPU,
     .parent = TYPE_X86_CPU,
@@ -2506,6 +2513,7 @@ static const TypeInfo x86_host_cpu_type_info = {
     .instance_init = x86_host_cpu_initfn,
     .abstract = false,
     .class_size = sizeof(X86CPUClass),
+    .class_init = x86_host_cpu_class_init,
 };
 
 #endif /* CONFIG_KVM */
-- 
1.7.11.7

Re: [Qemu-devel] [RFC 7/9] target-i386: CPU subclass for -cpu "host"

[Igor Mammedov]

On Fri, 28 Dec 2012 18:34:04 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> Note that we are initializing the CPU features inside instance_init (and
> not storing any CPU feature information inside the class struct) because
> kvm_cpu_fill_host() needs KVM to be initialized, and we can't guarantee
> that KVM will be initialized when class_init is called.
initializing defaults in initfn will be broken after we convert features into
static properties due to all initfn()s are called before static properties
defaults are set.

Is it possible to initialize kvm first before calling class_init().

> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target-i386/cpu.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index c824c08..2b6cc3b 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -330,6 +330,14 @@ typedef struct x86_def_t {
>  #define TCG_SVM_FEATURES 0
>  #define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP)
>  
> +
> +/* CPU class name definitions: */
> +
> +#define CPU_CLASS_NAME(name) (name "-" TYPE_X86_CPU)
> +
> +#define TYPE_X86_HOST_CPU CPU_CLASS_NAME("host")
> +
> +
>  /* maintains list of cpu model definitions
>   */
>  static x86_def_t *x86_defs = {NULL};
> @@ -1221,9 +1229,7 @@ static X86CPU *x86_cpu_create_from_name(const char *name, Error **errp)
>  
>      if (kvm_enabled() && name && strcmp(name, "host") == 0) {
>  #ifdef CONFIG_KVM
> -        cpu = X86_CPU(object_new(TYPE_X86_CPU));
> -        kvm_cpu_fill_host(x86_cpu_def);
> -        cpudef_2_x86_cpu(cpu, x86_cpu_def, &error);
> +        cpu = X86_CPU(object_new(TYPE_X86_HOST_CPU));
>  #endif
>      } else {
>          x86_def_t *def;
> @@ -2168,9 +2174,42 @@ static const TypeInfo x86_cpu_type_info = {
>      .class_init = x86_cpu_common_class_init,
>  };
>  
> +#ifdef CONFIG_KVM
> +
> +static void x86_host_cpu_initfn(Object *obj)
> +{
> +    X86CPU *cpu = X86_CPU(obj);
> +    Error *err = NULL;
> +    x86_def_t cpudef;
> +
> +    memset(&cpudef, 0, sizeof(cpudef));
> +    kvm_cpu_fill_host(&cpudef);
> +    cpudef_2_x86_cpu(cpu, &cpudef, &err);
> +
> +    if (err) {
> +        error_report("unexpected cpu init error: %s", error_get_pretty(err));
> +        exit(1);
> +    }
> +}
> +
> +static const TypeInfo x86_host_cpu_type_info = {
> +    .name = TYPE_X86_HOST_CPU,
> +    .parent = TYPE_X86_CPU,
> +    .instance_size = sizeof(X86CPU),
> +    .instance_init = x86_host_cpu_initfn,
> +    .abstract = false,
> +    .class_size = sizeof(X86CPUClass),
> +};
> +
> +#endif /* CONFIG_KVM */
> +
> +
>  static void x86_cpu_register_types(void)
>  {
>      type_register_static(&x86_cpu_type_info);
> +#ifdef CONFIG_KVM
> +    type_register_static(&x86_host_cpu_type_info);
> +#endif
>  }
>  
>  type_init(x86_cpu_register_types)
> -- 
> 1.7.11.7
> 


-- 
Regards,
  Igor

Re: [Qemu-devel] [RFC 7/9] target-i386: CPU subclass for -cpu "host"

[Igor Mammedov]

On Wed, 2 Jan 2013 20:00:26 +0100
Igor Mammedov <imammedo@redhat.com> wrote:

> On Fri, 28 Dec 2012 18:34:04 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > Note that we are initializing the CPU features inside instance_init (and
> > not storing any CPU feature information inside the class struct) because
> > kvm_cpu_fill_host() needs KVM to be initialized, and we can't guarantee
> > that KVM will be initialized when class_init is called.
> initializing defaults in initfn will be broken after we convert features into
> static properties due to all initfn()s are called before static properties
> defaults are set.
Never mind, It was my wish-full thinking;
currently device_initfn() sets static property defaults, then sets global
properties and only then children initfn()s are called.

> 
> Is it possible to initialize kvm first before calling class_init().
> 
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  target-i386/cpu.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 42 insertions(+), 3 deletions(-)
> > 
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index c824c08..2b6cc3b 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -330,6 +330,14 @@ typedef struct x86_def_t {
> >  #define TCG_SVM_FEATURES 0
> >  #define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP)
> >  
> > +
> > +/* CPU class name definitions: */
> > +
> > +#define CPU_CLASS_NAME(name) (name "-" TYPE_X86_CPU)
> > +
> > +#define TYPE_X86_HOST_CPU CPU_CLASS_NAME("host")
> > +
> > +
> >  /* maintains list of cpu model definitions
> >   */
> >  static x86_def_t *x86_defs = {NULL};
> > @@ -1221,9 +1229,7 @@ static X86CPU *x86_cpu_create_from_name(const char *name, Error **errp)
> >  
> >      if (kvm_enabled() && name && strcmp(name, "host") == 0) {
> >  #ifdef CONFIG_KVM
> > -        cpu = X86_CPU(object_new(TYPE_X86_CPU));
> > -        kvm_cpu_fill_host(x86_cpu_def);
> > -        cpudef_2_x86_cpu(cpu, x86_cpu_def, &error);
> > +        cpu = X86_CPU(object_new(TYPE_X86_HOST_CPU));
> >  #endif
> >      } else {
> >          x86_def_t *def;
> > @@ -2168,9 +2174,42 @@ static const TypeInfo x86_cpu_type_info = {
> >      .class_init = x86_cpu_common_class_init,
> >  };
> >  
> > +#ifdef CONFIG_KVM
> > +
> > +static void x86_host_cpu_initfn(Object *obj)
> > +{
> > +    X86CPU *cpu = X86_CPU(obj);
> > +    Error *err = NULL;
> > +    x86_def_t cpudef;
> > +
> > +    memset(&cpudef, 0, sizeof(cpudef));
> > +    kvm_cpu_fill_host(&cpudef);
> > +    cpudef_2_x86_cpu(cpu, &cpudef, &err);
> > +
> > +    if (err) {
> > +        error_report("unexpected cpu init error: %s", error_get_pretty(err));
> > +        exit(1);
> > +    }
> > +}
> > +
> > +static const TypeInfo x86_host_cpu_type_info = {
> > +    .name = TYPE_X86_HOST_CPU,
> > +    .parent = TYPE_X86_CPU,
> > +    .instance_size = sizeof(X86CPU),
> > +    .instance_init = x86_host_cpu_initfn,
> > +    .abstract = false,
> > +    .class_size = sizeof(X86CPUClass),
> > +};
> > +
> > +#endif /* CONFIG_KVM */
> > +
> > +
> >  static void x86_cpu_register_types(void)
> >  {
> >      type_register_static(&x86_cpu_type_info);
> > +#ifdef CONFIG_KVM
> > +    type_register_static(&x86_host_cpu_type_info);
> > +#endif
> >  }
> >  
> >  type_init(x86_cpu_register_types)
> > -- 
> > 1.7.11.7
> > 
> 
> 
> -- 
> Regards,
>   Igor
> 


-- 
Regards,
  Igor

Re: [Qemu-devel] [RFC 7/9] target-i386: CPU subclass for -cpu "host"

[Eduardo Habkost]

On Wed, Jan 02, 2013 at 08:00:26PM +0100, Igor Mammedov wrote:
> On Fri, 28 Dec 2012 18:34:04 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > Note that we are initializing the CPU features inside instance_init (and
> > not storing any CPU feature information inside the class struct) because
> > kvm_cpu_fill_host() needs KVM to be initialized, and we can't guarantee
> > that KVM will be initialized when class_init is called.
> initializing defaults in initfn will be broken after we convert features into
> static properties due to all initfn()s are called before static properties
> defaults are set.

You are right because of the plan to use global properties. I didn't
care about property defaults because the -cpu host class wouldn't have
introspectable defaults, but global properties need to work properly so
"-cpu host,+foobar,enforce" keep working.


> 
> Is it possible to initialize kvm first before calling class_init().

Summarizing what was discussed on IRC:

 - class_init is too early, because it may be called before we even know
   what's the accel configuration requested by the user (and I believe
   this is by design, as we want to allow a client to list all available
   classes and their available properties before configuring anything).
   So we can't call kvm_arch_get_supported_cpuid() on class_init.
 - CPU instance_init is too late because global properties are set
   inside device_initfn(), before the CPU class instance_init function
   is called.

So we need something to be called after kvm_init() is called, but before
the CPU objects are created. We already have that: kvm_init() itself.  :-)

So I will try to send a new RFC that has a function like:

  void kvm_finish_cpu_host_class_init(KVMState *s, X86CPUClass *cc);

and kvm_init() would then call:

  kvm_finish_cpu_host_class_init(kvm_state, X86_CPU_CLASS(object_class_by_name(CPU_CLASS_NAME("host"))));

We considered other more complex approaches (like making the CPU feature
properties tristate [on/off/host] instead of boolean), but that would be
simply a further improvement, maybe to be proposed as part of the x86
CPU properties work.

> 
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  target-i386/cpu.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 42 insertions(+), 3 deletions(-)
> > 
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index c824c08..2b6cc3b 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -330,6 +330,14 @@ typedef struct x86_def_t {
> >  #define TCG_SVM_FEATURES 0
> >  #define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP)
> >  
> > +
> > +/* CPU class name definitions: */
> > +
> > +#define CPU_CLASS_NAME(name) (name "-" TYPE_X86_CPU)
> > +
> > +#define TYPE_X86_HOST_CPU CPU_CLASS_NAME("host")
> > +
> > +
> >  /* maintains list of cpu model definitions
> >   */
> >  static x86_def_t *x86_defs = {NULL};
> > @@ -1221,9 +1229,7 @@ static X86CPU *x86_cpu_create_from_name(const char *name, Error **errp)
> >  
> >      if (kvm_enabled() && name && strcmp(name, "host") == 0) {
> >  #ifdef CONFIG_KVM
> > -        cpu = X86_CPU(object_new(TYPE_X86_CPU));
> > -        kvm_cpu_fill_host(x86_cpu_def);
> > -        cpudef_2_x86_cpu(cpu, x86_cpu_def, &error);
> > +        cpu = X86_CPU(object_new(TYPE_X86_HOST_CPU));
> >  #endif
> >      } else {
> >          x86_def_t *def;
> > @@ -2168,9 +2174,42 @@ static const TypeInfo x86_cpu_type_info = {
> >      .class_init = x86_cpu_common_class_init,
> >  };
> >  
> > +#ifdef CONFIG_KVM
> > +
> > +static void x86_host_cpu_initfn(Object *obj)
> > +{
> > +    X86CPU *cpu = X86_CPU(obj);
> > +    Error *err = NULL;
> > +    x86_def_t cpudef;
> > +
> > +    memset(&cpudef, 0, sizeof(cpudef));
> > +    kvm_cpu_fill_host(&cpudef);
> > +    cpudef_2_x86_cpu(cpu, &cpudef, &err);
> > +
> > +    if (err) {
> > +        error_report("unexpected cpu init error: %s", error_get_pretty(err));
> > +        exit(1);
> > +    }
> > +}
> > +
> > +static const TypeInfo x86_host_cpu_type_info = {
> > +    .name = TYPE_X86_HOST_CPU,
> > +    .parent = TYPE_X86_CPU,
> > +    .instance_size = sizeof(X86CPU),
> > +    .instance_init = x86_host_cpu_initfn,
> > +    .abstract = false,
> > +    .class_size = sizeof(X86CPUClass),
> > +};
> > +
> > +#endif /* CONFIG_KVM */
> > +
> > +
> >  static void x86_cpu_register_types(void)
> >  {
> >      type_register_static(&x86_cpu_type_info);
> > +#ifdef CONFIG_KVM
> > +    type_register_static(&x86_host_cpu_type_info);
> > +#endif
> >  }
> >  
> >  type_init(x86_cpu_register_types)
> > -- 
> > 1.7.11.7
> > 
> 
> 
> -- 
> Regards,
>   Igor

-- 
Eduardo

Re: [Qemu-devel] [RFC 7/9] target-i386: CPU subclass for -cpu "host"

[Eduardo Habkost]

On Wed, Jan 02, 2013 at 09:07:26PM +0100, Igor Mammedov wrote:
> On Wed, 2 Jan 2013 20:00:26 +0100
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > On Fri, 28 Dec 2012 18:34:04 -0200
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > 
> > > Note that we are initializing the CPU features inside instance_init (and
> > > not storing any CPU feature information inside the class struct) because
> > > kvm_cpu_fill_host() needs KVM to be initialized, and we can't guarantee
> > > that KVM will be initialized when class_init is called.
> > initializing defaults in initfn will be broken after we convert features into
> > static properties due to all initfn()s are called before static properties
> > defaults are set.
> Never mind, It was my wish-full thinking;
> currently device_initfn() sets static property defaults, then sets global
> properties and only then children initfn()s are called.

So it won't break because of property defaults, but instead it will
break because we need global properties to work properly. It's even
worse because I didn't care about property defaults for -cpu host, but
we care about keeping global properties working (so
"-cpu host,+foobar,enforce" works).

-- 
Eduardo