[Qemu-devel] [PATCH qom-cpu-next v3 0/4] target-i386: X86CPU subclasses

[Qemu-devel] [PATCH qom-cpu-next v3 0/4] target-i386: X86CPU subclasses

[Andreas Färber]

Hello,

Long announced, here it is after the freeze: My 3rd attempt at CPU subclasses.
v3 is closer to v1 again, slimmed down not to touch x86_def_t much.

It was noticed that in theory a QOM class enumeration could lead to the host
CPU class_init running before kvm_init(). My proposal is to make the class_init
reentrant and to re-run it from target-i386's kvm_arch_init() if necessary.
Similar to target-ppc, -cpu host code is placed into kvm.c rather than cpu.c.

This conversion is an interim solution to get the code structured in a
QOM-friendly way and to reach a hot-plug friendly and cross-target aligned
CPU initialization. It decouples the work of introducing subclasses (to hide
object initialization from cpu_init() and device_add) from any x86-internal
improvements (like Igor's global compat properties set from pc-x.y machines).
The -cpu ? and QMP support is still based on array iteration.

Based on qom-cpu-next queue, which contains some cpu_init() cleanups already.

Available for testing here:
git://github.com/afaerber/qemu-cpu.git qom-cpu-x86-subclasses.v3
https://github.com/afaerber/qemu-cpu/commits/qom-cpu-x86-subclasses.v3

Regards,
Andreas

v2 -> v3:
* Instead of re-coding all CPU definitions as class_init functions, leave
  the built-in definition array in place and place x86_def_t in the class.
* Use kvm_arch_init() hook to assure class_init succeeds for -cpu host.
  Suggested by Eduardo.

v1-> v2:
* Instead of turning x86_def_t into X86CPUInfo to initialize classes,
  drop it completely and register types manually with customizable TypeInfos
* Use new list facilities for printing -cpu ? models
* Adopt new name scheme suggested by Eduardo and ideas from my alpha series
* Keep short names in -cpu ? output for alignment reasons
* Merge cpu_x86_init() into cpu.c:cpu_x86_register()
* Append patch showing Haswell as subclass of SandyBridge

Cc: Anthony Liguori <anthony@codemonkey.ws>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Blue Swirl <blauwirbel@gmail.com>
Cc: Richard Henderson <rth@twiddle.net>

Andreas Färber (4):
  target-i386: Move cpu_x86_init()
  target-i386: Split command line parsing out of cpu_x86_register()
  target-i386: Slim conversion to X86CPU subclasses
  Remove cpudef_setup() hooks

 arch_init.c                |    7 -
 bsd-user/main.c            |    3 -
 include/sysemu/arch_init.h |    1 -
 linux-user/main.c          |    3 -
 target-i386/cpu-qom.h      |   24 ++++
 target-i386/cpu.c          |  326 ++++++++++++++++++--------------------------
 target-i386/cpu.h          |    3 -
 target-i386/helper.c       |   24 ----
 target-i386/kvm.c          |   93 +++++++++++++
 vl.c                       |    7 -
 10 Dateien geändert, 247 Zeilen hinzugefügt(+), 244 Zeilen entfernt(-)

-- 
1.7.10.4

[Qemu-devel] [PATCH qom-cpu-next v3 1/4] target-i386: Move cpu_x86_init()

[Andreas Färber]

Consolidate CPU functions in cpu.c.
Allows to make cpu_x86_register() static.

No functional changes.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-i386/cpu.c    |   26 +++++++++++++++++++++++++-
 target-i386/cpu.h    |    1 -
 target-i386/helper.c |   24 ------------------------
 3 Dateien geändert, 25 Zeilen hinzugefügt(+), 26 Zeilen entfernt(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ea0ce0b..e74802b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1516,7 +1516,7 @@ static void filter_features_for_kvm(X86CPU *cpu)
 }
 #endif
 
-int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
+static int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 {
     CPUX86State *env = &cpu->env;
     x86_def_t def1, *def = &def1;
@@ -1576,6 +1576,30 @@ out:
     return 0;
 }
 
+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;
+    }
+
+    object_property_set_bool(OBJECT(cpu), true, "realized", &error);
+    if (error) {
+        error_free(error);
+        object_delete(OBJECT(cpu));
+        return NULL;
+    }
+    return cpu;
+}
+
 #if !defined(CONFIG_USER_ONLY)
 
 void cpu_clear_apic_feature(CPUX86State *env)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 9e6e1a6..7577e4f 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1002,7 +1002,6 @@ 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);
 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 29217ef..4bf9db7 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1267,30 +1267,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;
-    }
-
-    object_property_set_bool(OBJECT(cpu), true, "realized", &error);
-    if (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.10.4

[Qemu-devel] [PATCH qom-cpu-next v3 2/4] target-i386: Split command line parsing out of cpu_x86_register()

[Andreas Färber]

In order to instantiate a CPU subtype we will need to know which type,
so move the cpu_model splitting into cpu_x86_init().

Parameters need to be set on the X86CPU instance, so move
cpu_x86_parse_featurestr() into cpu_x86_init() as well.

This leaves cpu_x86_register() operating on the model name only.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-i386/cpu.c |   50 +++++++++++++++++++++++++++++++-------------------
 1 Datei geändert, 31 Zeilen hinzugefügt(+), 19 Zeilen entfernt(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e74802b..ee2fd6b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1516,24 +1516,14 @@ static void filter_features_for_kvm(X86CPU *cpu)
 }
 #endif
 
-static int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
+static int cpu_x86_register(X86CPU *cpu, const char *name)
 {
     CPUX86State *env = &cpu->env;
     x86_def_t def1, *def = &def1;
     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");
-        goto out;
-    }
-    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);
         goto out;
@@ -1565,9 +1555,7 @@ static int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
         goto out;
     }
 
-    cpu_x86_parse_featurestr(cpu, features, &error);
 out:
-    g_strfreev(model_pieces);
     if (error) {
         fprintf(stderr, "%s\n", error_get_pretty(error));
         error_free(error);
@@ -1578,26 +1566,50 @@ out:
 
 X86CPU *cpu_x86_init(const char *cpu_model)
 {
-    X86CPU *cpu;
+    X86CPU *cpu = NULL;
     CPUX86State *env;
+    gchar **model_pieces;
+    char *name, *features;
     Error *error = NULL;
 
+    model_pieces = g_strsplit(cpu_model, ",", 2);
+    if (!model_pieces[0]) {
+        error_setg(&error, "Invalid/empty CPU model name");
+        goto error;
+    }
+    name = model_pieces[0];
+    features = model_pieces[1];
+
     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;
+    if (cpu_x86_register(cpu, name) < 0) {
+        goto error;
+    }
+
+    cpu_x86_parse_featurestr(cpu, features, &error);
+    if (error) {
+        goto error;
     }
 
     object_property_set_bool(OBJECT(cpu), true, "realized", &error);
     if (error) {
+        goto error;
+    }
+    g_strfreev(model_pieces);
+    return cpu;
+
+error:
+    g_strfreev(model_pieces);
+    if (error) {
+        fprintf(stderr, "%s\n", error_get_pretty(error));
         error_free(error);
+    }
+    if (cpu != NULL) {
         object_delete(OBJECT(cpu));
-        return NULL;
     }
-    return cpu;
+    return NULL;
 }
 
 #if !defined(CONFIG_USER_ONLY)
-- 
1.7.10.4

[PATCH qom-cpu-next v3 3/4] target-i386: Slim conversion to X86CPU subclasses

[Andreas Färber]

Move x86_def_t definition to header and embed into X86CPUClass.
Register types per built-in model definition.

Move version initialization from x86_cpudef_setup() to class_init.

Inline cpu_x86_register() into the X86CPU initfn.
Since instance_init cannot reports errors, drop error handling.

Replace cpu_x86_find_by_name() with x86_cpu_class_by_name().
Move KVM host vendor override from cpu_x86_find_by_name() to the initfn.

Register host-{i386,x86_64}-cpu type from KVM code to avoid #ifdefs.
Make kvm_cpu_fill_host() a class_init and inline cpu_x86_fill_model_id().

Let kvm_check_features_against_host() obtain host-{i386,86_64}-cpu for
comparison.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-i386/cpu-qom.h |   24 ++++
 target-i386/cpu.c     |  324 +++++++++++++++++--------------------------------
 target-i386/cpu.h     |    2 -
 target-i386/kvm.c     |   93 ++++++++++++++
 4 Dateien geändert, 228 Zeilen hinzugefügt(+), 215 Zeilen entfernt(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 48e6b54..80bf72d 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -30,6 +30,27 @@
 #define TYPE_X86_CPU "i386-cpu"
 #endif
 
+#define TYPE_HOST_X86_CPU "host-" TYPE_X86_CPU
+
+typedef struct x86_def_t {
+    const char *name;
+    uint32_t level;
+    /* vendor is zero-terminated, 12 character ASCII string */
+    char vendor[CPUID_VENDOR_SZ + 1];
+    int family;
+    int model;
+    int stepping;
+    uint32_t features, ext_features, ext2_features, ext3_features;
+    uint32_t kvm_features, svm_features;
+    uint32_t xlevel;
+    char model_id[48];
+    /* 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;
+
 #define X86_CPU_CLASS(klass) \
     OBJECT_CLASS_CHECK(X86CPUClass, (klass), TYPE_X86_CPU)
 #define X86_CPU(obj) \
@@ -41,6 +62,7 @@
  * X86CPUClass:
  * @parent_realize: The parent class' realize handler.
  * @parent_reset: The parent class' reset handler.
+ * @info: Model-specific data.
  *
  * An x86 CPU model or family.
  */
@@ -51,6 +73,8 @@ typedef struct X86CPUClass {
 
     DeviceRealize parent_realize;
     void (*parent_reset)(CPUState *cpu);
+
+    x86_def_t info;
 } X86CPUClass;
 
 /**
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ee2fd6b..6c95740 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -346,25 +346,6 @@ static void add_flagname_to_bitmaps(const char *flagname,
     }
 }
 
-typedef struct x86_def_t {
-    const char *name;
-    uint32_t level;
-    /* vendor is zero-terminated, 12 character ASCII string */
-    char vendor[CPUID_VENDOR_SZ + 1];
-    int family;
-    int model;
-    int stepping;
-    uint32_t features, ext_features, ext2_features, ext3_features;
-    uint32_t kvm_features, svm_features;
-    uint32_t xlevel;
-    char model_id[48];
-    /* 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;
-
 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
 #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
           CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC)
@@ -868,86 +849,6 @@ static x86_def_t builtin_x86_defs[] = {
     },
 };
 
-#ifdef CONFIG_KVM
-static int cpu_x86_fill_model_id(char *str)
-{
-    uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
-    int i;
-
-    for (i = 0; i < 3; i++) {
-        host_cpuid(0x80000002 + i, 0, &eax, &ebx, &ecx, &edx);
-        memcpy(str + i * 16 +  0, &eax, 4);
-        memcpy(str + i * 16 +  4, &ebx, 4);
-        memcpy(str + i * 16 +  8, &ecx, 4);
-        memcpy(str + i * 16 + 12, &edx, 4);
-    }
-    return 0;
-}
-#endif
-
-/* Fill a x86_def_t struct with information about the host CPU, and
- * the CPU features supported by the host hardware + host kernel
- *
- * This function may be called only if KVM is enabled.
- */
-static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
-{
-#ifdef CONFIG_KVM
-    KVMState *s = kvm_state;
-    uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
-
-    assert(kvm_enabled());
-
-    x86_cpu_def->name = "host";
-    host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
-    x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx);
-
-    host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx);
-    x86_cpu_def->family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
-    x86_cpu_def->model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >> 12);
-    x86_cpu_def->stepping = eax & 0x0F;
-
-    x86_cpu_def->level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
-    x86_cpu_def->features = kvm_arch_get_supported_cpuid(s, 0x1, 0, R_EDX);
-    x86_cpu_def->ext_features = kvm_arch_get_supported_cpuid(s, 0x1, 0, R_ECX);
-
-    if (x86_cpu_def->level >= 7) {
-        x86_cpu_def->cpuid_7_0_ebx_features =
-                    kvm_arch_get_supported_cpuid(s, 0x7, 0, R_EBX);
-    } else {
-        x86_cpu_def->cpuid_7_0_ebx_features = 0;
-    }
-
-    x86_cpu_def->xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
-    x86_cpu_def->ext2_features =
-                kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX);
-    x86_cpu_def->ext3_features =
-                kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_ECX);
-
-    cpu_x86_fill_model_id(x86_cpu_def->model_id);
-
-    /* Call Centaur's CPUID instruction. */
-    if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) {
-        host_cpuid(0xC0000000, 0, &eax, &ebx, &ecx, &edx);
-        eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
-        if (eax >= 0xC0000001) {
-            /* Support VIA max extended level */
-            x86_cpu_def->xlevel2 = eax;
-            host_cpuid(0xC0000001, 0, &eax, &ebx, &ecx, &edx);
-            x86_cpu_def->ext4_features =
-                    kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
-        }
-    }
-
-    /* Other KVM-specific feature fields: */
-    x86_cpu_def->svm_features =
-        kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX);
-    x86_cpu_def->kvm_features =
-        kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX);
-
-#endif /* CONFIG_KVM */
-}
-
 static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
 {
     int i;
@@ -975,31 +876,31 @@ static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
 static int kvm_check_features_against_host(X86CPU *cpu)
 {
     CPUX86State *env = &cpu->env;
-    x86_def_t host_def;
+    ObjectClass *host_oc = object_class_by_name(TYPE_HOST_X86_CPU);
+    X86CPUClass *host_xcc = X86_CPU_CLASS(host_oc);
     uint32_t mask;
     int rv, i;
     struct model_features_t ft[] = {
-        {&env->cpuid_features, &host_def.features,
+        {&env->cpuid_features, &host_xcc->info.features,
             FEAT_1_EDX },
-        {&env->cpuid_ext_features, &host_def.ext_features,
+        {&env->cpuid_ext_features, &host_xcc->info.ext_features,
             FEAT_1_ECX },
-        {&env->cpuid_ext2_features, &host_def.ext2_features,
+        {&env->cpuid_ext2_features, &host_xcc->info.ext2_features,
             FEAT_8000_0001_EDX },
-        {&env->cpuid_ext3_features, &host_def.ext3_features,
+        {&env->cpuid_ext3_features, &host_xcc->info.ext3_features,
             FEAT_8000_0001_ECX },
-        {&env->cpuid_ext4_features, &host_def.ext4_features,
+        {&env->cpuid_ext4_features, &host_xcc->info.ext4_features,
             FEAT_C000_0001_EDX },
-        {&env->cpuid_7_0_ebx_features, &host_def.cpuid_7_0_ebx_features,
+        {&env->cpuid_7_0_ebx_features, &host_xcc->info.cpuid_7_0_ebx_features,
             FEAT_7_0_EBX },
-        {&env->cpuid_svm_features, &host_def.svm_features,
+        {&env->cpuid_svm_features, &host_xcc->info.svm_features,
             FEAT_SVM },
-        {&env->cpuid_kvm_features, &host_def.kvm_features,
+        {&env->cpuid_kvm_features, &host_xcc->info.kvm_features,
             FEAT_KVM },
     };
 
     assert(kvm_enabled());
 
-    kvm_cpu_fill_host(&host_def);
     for (rv = 0, i = 0; i < ARRAY_SIZE(ft); ++i) {
         FeatureWord w = ft[i].feat_word;
         FeatureWordInfo *wi = &feature_word_info[w];
@@ -1261,40 +1162,30 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
     cpu->env.tsc_khz = value / 1000;
 }
 
-static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
+static ObjectClass *x86_cpu_class_by_name(const char *name)
 {
-    x86_def_t *def;
-    int i;
+    ObjectClass *oc;
+    char *typename;
 
     if (name == NULL) {
-        return -1;
-    }
-    if (kvm_enabled() && strcmp(name, "host") == 0) {
-        kvm_cpu_fill_host(x86_cpu_def);
-        return 0;
+        return NULL;
     }
 
-    for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
-        def = &builtin_x86_defs[i];
-        if (strcmp(name, def->name) == 0) {
-            memcpy(x86_cpu_def, def, sizeof(*def));
-            /* sysenter isn't supported in 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);
-                x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx);
-            }
-            return 0;
+    if (strcmp(name, "host") == 0) {
+        if (kvm_enabled()) {
+            return object_class_by_name(TYPE_HOST_X86_CPU);
         }
+        return NULL;
     }
 
-    return -1;
+    typename = g_strdup_printf("%s-" TYPE_X86_CPU, name);
+    oc = object_class_by_name(typename);
+    g_free(typename);
+    if (oc != NULL && (!object_class_dynamic_cast(oc, TYPE_X86_CPU) ||
+                       object_class_is_abstract(oc))) {
+        oc = NULL;
+    }
+    return oc;
 }
 
 /* Parse "+feature,-feature,feature=foo" CPU feature string
@@ -1516,60 +1407,13 @@ static void filter_features_for_kvm(X86CPU *cpu)
 }
 #endif
 
-static int cpu_x86_register(X86CPU *cpu, const char *name)
-{
-    CPUX86State *env = &cpu->env;
-    x86_def_t def1, *def = &def1;
-    Error *error = NULL;
-
-    memset(def, 0, sizeof(*def));
-
-    if (cpu_x86_find_by_name(def, name) < 0) {
-        error_setg(&error, "Unable to find CPU definition: %s", name);
-        goto out;
-    }
-
-    if (kvm_enabled()) {
-        def->kvm_features |= kvm_default_features;
-    }
-    def->ext_features |= CPUID_EXT_HYPERVISOR;
-
-    object_property_set_str(OBJECT(cpu), def->vendor, "vendor", &error);
-    object_property_set_int(OBJECT(cpu), def->level, "level", &error);
-    object_property_set_int(OBJECT(cpu), def->family, "family", &error);
-    object_property_set_int(OBJECT(cpu), def->model, "model", &error);
-    object_property_set_int(OBJECT(cpu), def->stepping, "stepping", &error);
-    env->cpuid_features = def->features;
-    env->cpuid_ext_features = def->ext_features;
-    env->cpuid_ext2_features = def->ext2_features;
-    env->cpuid_ext3_features = def->ext3_features;
-    object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", &error);
-    env->cpuid_kvm_features = def->kvm_features;
-    env->cpuid_svm_features = def->svm_features;
-    env->cpuid_ext4_features = def->ext4_features;
-    env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features;
-    env->cpuid_xlevel2 = def->xlevel2;
-
-    object_property_set_str(OBJECT(cpu), def->model_id, "model-id", &error);
-    if (error) {
-        goto out;
-    }
-
-out:
-    if (error) {
-        fprintf(stderr, "%s\n", error_get_pretty(error));
-        error_free(error);
-        return -1;
-    }
-    return 0;
-}
-
 X86CPU *cpu_x86_init(const char *cpu_model)
 {
     X86CPU *cpu = NULL;
     CPUX86State *env;
     gchar **model_pieces;
     char *name, *features;
+    ObjectClass *oc;
     Error *error = NULL;
 
     model_pieces = g_strsplit(cpu_model, ",", 2);
@@ -1580,13 +1424,13 @@ X86CPU *cpu_x86_init(const char *cpu_model)
     name = model_pieces[0];
     features = model_pieces[1];
 
-    cpu = X86_CPU(object_new(TYPE_X86_CPU));
-    env = &cpu->env;
-    env->cpu_model_str = cpu_model;
-
-    if (cpu_x86_register(cpu, name) < 0) {
+    oc = x86_cpu_class_by_name(name);
+    if (oc == NULL) {
         goto error;
     }
+    cpu = X86_CPU(object_new(object_class_get_name(oc)));
+    env = &cpu->env;
+    env->cpu_model_str = cpu_model;
 
     cpu_x86_parse_featurestr(cpu, features, &error);
     if (error) {
@@ -1621,30 +1465,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];
-
-        /* 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;
-            }
-        }
-    }
-}
-
 static void get_cpuid_vendor(CPUX86State *env, uint32_t *ebx,
                              uint32_t *ecx, uint32_t *edx)
 {
@@ -2198,6 +2018,8 @@ static void x86_cpu_initfn(Object *obj)
     CPUState *cs = CPU(obj);
     X86CPU *cpu = X86_CPU(obj);
     CPUX86State *env = &cpu->env;
+    X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
+    const x86_def_t *def = &xcc->info;
     static int inited;
 
     cpu_exec_init(env);
@@ -2227,6 +2049,41 @@ static void x86_cpu_initfn(Object *obj)
                         x86_cpuid_get_tsc_freq,
                         x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
 
+    /* sysenter isn't supported in 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()) {
+        host_cpuid(0, 0, NULL, &env->cpuid_vendor1, &env->cpuid_vendor2,
+                   &env->cpuid_vendor3);
+    } else {
+        object_property_set_str(OBJECT(cpu), def->vendor, "vendor", NULL);
+    }
+
+    object_property_set_int(OBJECT(cpu), def->level, "level", NULL);
+    object_property_set_int(OBJECT(cpu), def->family, "family", NULL);
+    object_property_set_int(OBJECT(cpu), def->model, "model", NULL);
+    object_property_set_int(OBJECT(cpu), def->stepping, "stepping", NULL);
+    env->cpuid_features = def->features;
+    env->cpuid_ext_features = def->ext_features;
+    env->cpuid_ext_features |= CPUID_EXT_HYPERVISOR;
+    env->cpuid_ext2_features = def->ext2_features;
+    env->cpuid_ext3_features = def->ext3_features;
+    object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", NULL);
+    env->cpuid_kvm_features = def->kvm_features;
+    if (kvm_enabled()) {
+        env->cpuid_kvm_features |= kvm_default_features;
+    }
+    env->cpuid_svm_features = def->svm_features;
+    env->cpuid_ext4_features = def->ext4_features;
+    env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features;
+    env->cpuid_xlevel2 = def->xlevel2;
+
+    object_property_set_str(OBJECT(cpu), def->model_id, "model-id", NULL);
+
     env->cpuid_apic_id = x86_cpu_apic_id_from_index(cs->cpu_index);
 
     /* init various static tables used in TCG mode */
@@ -2239,6 +2096,27 @@ static void x86_cpu_initfn(Object *obj)
     }
 }
 
+static void x86_cpu_def_class_init(ObjectClass *oc, void *data)
+{
+    X86CPUClass *xcc = X86_CPU_CLASS(oc);
+    x86_def_t *def = data;
+    int i;
+    static const char *versioned_models[] = { "qemu32", "qemu64", "athlon" };
+
+    memcpy(&xcc->info, def, sizeof(x86_def_t));
+
+    /* Look for specific models that have the QEMU version in .model_id */
+    for (i = 0; i < ARRAY_SIZE(versioned_models); i++) {
+        if (strcmp(versioned_models[i], def->name) == 0) {
+            pstrcpy(xcc->info.model_id, sizeof(xcc->info.model_id),
+                    "QEMU Virtual CPU version ");
+            pstrcat(xcc->info.model_id, sizeof(xcc->info.model_id),
+                    qemu_get_version());
+            break;
+        }
+    }
+}
+
 static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
 {
     X86CPUClass *xcc = X86_CPU_CLASS(oc);
@@ -2250,6 +2128,21 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
 
     xcc->parent_reset = cc->reset;
     cc->reset = x86_cpu_reset;
+
+    cc->class_by_name = x86_cpu_class_by_name;
+}
+
+static void x86_register_cpu_type(const x86_def_t *def)
+{
+    TypeInfo type_info = {
+        .parent = TYPE_X86_CPU,
+        .class_init = x86_cpu_def_class_init,
+        .class_data = (void *)def,
+    };
+
+    type_info.name = g_strdup_printf("%s-" TYPE_X86_CPU, def->name);
+    type_register(&type_info);
+    g_free((void *)type_info.name);
 }
 
 static const TypeInfo x86_cpu_type_info = {
@@ -2257,14 +2150,19 @@ 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,
 };
 
 static void x86_cpu_register_types(void)
 {
+    int i;
+
     type_register_static(&x86_cpu_type_info);
+    for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
+        x86_register_cpu_type(&builtin_x86_defs[i]);
+    }
 }
 
 type_init(x86_cpu_register_types)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 7577e4f..2aef7b7 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -887,7 +887,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);
@@ -1079,7 +1078,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
 
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 9ebf181..dbfa670 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -735,6 +735,80 @@ static int kvm_get_supported_msrs(KVMState *s)
     return ret;
 }
 
+static void kvm_host_cpu_initfn(Object *obj)
+{
+    assert(kvm_enabled());
+}
+
+static bool kvm_host_cpu_needs_class_init;
+
+static void kvm_host_cpu_class_init(ObjectClass *oc, void *data)
+{
+    X86CPUClass *xcc = X86_CPU_CLASS(oc);
+    x86_def_t *x86_cpu_def = &xcc->info;
+    KVMState *s = kvm_state;
+    uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
+    int i;
+
+    if (!kvm_enabled()) {
+        kvm_host_cpu_needs_class_init = true;
+        return;
+    }
+
+    xcc->info.name = "host";
+
+    /* Vendor will be set in initfn if kvm_enabled() */
+
+    host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx);
+    x86_cpu_def->family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
+    x86_cpu_def->model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >> 12);
+    x86_cpu_def->stepping = eax & 0x0F;
+
+    x86_cpu_def->level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
+    x86_cpu_def->features = kvm_arch_get_supported_cpuid(s, 0x1, 0, R_EDX);
+    x86_cpu_def->ext_features = kvm_arch_get_supported_cpuid(s, 0x1, 0, R_ECX);
+
+    if (x86_cpu_def->level >= 7) {
+        x86_cpu_def->cpuid_7_0_ebx_features =
+                    kvm_arch_get_supported_cpuid(s, 0x7, 0, R_EBX);
+    } else {
+        x86_cpu_def->cpuid_7_0_ebx_features = 0;
+    }
+
+    x86_cpu_def->xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
+    x86_cpu_def->ext2_features =
+                kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX);
+    x86_cpu_def->ext3_features =
+                kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_ECX);
+
+    for (i = 0; i < 3; i++) {
+        host_cpuid(0x80000002 + i, 0, &eax, &ebx, &ecx, &edx);
+        memcpy(xcc->info.model_id + i * 16 +  0, &eax, 4);
+        memcpy(xcc->info.model_id + i * 16 +  4, &ebx, 4);
+        memcpy(xcc->info.model_id + i * 16 +  8, &ecx, 4);
+        memcpy(xcc->info.model_id + i * 16 + 12, &edx, 4);
+    }
+
+    /* Call Centaur's CPUID instruction. */
+    if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) {
+        host_cpuid(0xC0000000, 0, &eax, &ebx, &ecx, &edx);
+        eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
+        if (eax >= 0xC0000001) {
+            /* Support VIA max extended level */
+            x86_cpu_def->xlevel2 = eax;
+            host_cpuid(0xC0000001, 0, &eax, &ebx, &ecx, &edx);
+            x86_cpu_def->ext4_features =
+                    kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
+        }
+    }
+
+    /* Other KVM-specific feature fields: */
+    x86_cpu_def->svm_features =
+        kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX);
+    x86_cpu_def->kvm_features =
+        kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX);
+}
+
 int kvm_arch_init(KVMState *s)
 {
     QemuOptsList *list = qemu_find_opts("machine");
@@ -797,6 +871,11 @@ int kvm_arch_init(KVMState *s)
             }
         }
     }
+
+    if (kvm_host_cpu_needs_class_init) {
+        kvm_host_cpu_class_init(object_class_by_name(TYPE_HOST_X86_CPU), NULL);
+    }
+
     return 0;
 }
 
@@ -2330,3 +2409,17 @@ int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id)
     return kvm_deassign_irq_internal(s, dev_id, KVM_DEV_IRQ_GUEST_MSIX |
                                                 KVM_DEV_IRQ_HOST_MSIX);
 }
+
+static const TypeInfo host_x86_cpu_type_info = {
+    .name = TYPE_HOST_X86_CPU,
+    .parent = TYPE_X86_CPU,
+    .instance_init = kvm_host_cpu_initfn,
+    .class_init = kvm_host_cpu_class_init,
+};
+
+static void kvm_register_types(void)
+{
+    type_register_static(&host_x86_cpu_type_info);
+}
+
+type_init(kvm_register_types)
-- 
1.7.10.4

[Qemu-devel] [PATCH qom-cpu-next v3 3/4] target-i386: Slim conversion to X86CPU subclasses

[Andreas Färber]

Move x86_def_t definition to header and embed into X86CPUClass.
Register types per built-in model definition.

Move version initialization from x86_cpudef_setup() to class_init.

Inline cpu_x86_register() into the X86CPU initfn.
Since instance_init cannot reports errors, drop error handling.

Replace cpu_x86_find_by_name() with x86_cpu_class_by_name().
Move KVM host vendor override from cpu_x86_find_by_name() to the initfn.

Register host-{i386,x86_64}-cpu type from KVM code to avoid #ifdefs.
Make kvm_cpu_fill_host() a class_init and inline cpu_x86_fill_model_id().

Let kvm_check_features_against_host() obtain host-{i386,86_64}-cpu for
comparison.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-i386/cpu-qom.h |   24 ++++
 target-i386/cpu.c     |  324 +++++++++++++++++--------------------------------
 target-i386/cpu.h     |    2 -
 target-i386/kvm.c     |   93 ++++++++++++++
 4 Dateien geändert, 228 Zeilen hinzugefügt(+), 215 Zeilen entfernt(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 48e6b54..80bf72d 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -30,6 +30,27 @@
 #define TYPE_X86_CPU "i386-cpu"
 #endif
 
+#define TYPE_HOST_X86_CPU "host-" TYPE_X86_CPU
+
+typedef struct x86_def_t {
+    const char *name;
+    uint32_t level;
+    /* vendor is zero-terminated, 12 character ASCII string */
+    char vendor[CPUID_VENDOR_SZ + 1];
+    int family;
+    int model;
+    int stepping;
+    uint32_t features, ext_features, ext2_features, ext3_features;
+    uint32_t kvm_features, svm_features;
+    uint32_t xlevel;
+    char model_id[48];
+    /* 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;
+
 #define X86_CPU_CLASS(klass) \
     OBJECT_CLASS_CHECK(X86CPUClass, (klass), TYPE_X86_CPU)
 #define X86_CPU(obj) \
@@ -41,6 +62,7 @@
  * X86CPUClass:
  * @parent_realize: The parent class' realize handler.
  * @parent_reset: The parent class' reset handler.
+ * @info: Model-specific data.
  *
  * An x86 CPU model or family.
  */
@@ -51,6 +73,8 @@ typedef struct X86CPUClass {
 
     DeviceRealize parent_realize;
     void (*parent_reset)(CPUState *cpu);
+
+    x86_def_t info;
 } X86CPUClass;
 
 /**
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ee2fd6b..6c95740 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -346,25 +346,6 @@ static void add_flagname_to_bitmaps(const char *flagname,
     }
 }
 
-typedef struct x86_def_t {
-    const char *name;
-    uint32_t level;
-    /* vendor is zero-terminated, 12 character ASCII string */
-    char vendor[CPUID_VENDOR_SZ + 1];
-    int family;
-    int model;
-    int stepping;
-    uint32_t features, ext_features, ext2_features, ext3_features;
-    uint32_t kvm_features, svm_features;
-    uint32_t xlevel;
-    char model_id[48];
-    /* 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;
-
 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
 #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
           CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC)
@@ -868,86 +849,6 @@ static x86_def_t builtin_x86_defs[] = {
     },
 };
 
-#ifdef CONFIG_KVM
-static int cpu_x86_fill_model_id(char *str)
-{
-    uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
-    int i;
-
-    for (i = 0; i < 3; i++) {
-        host_cpuid(0x80000002 + i, 0, &eax, &ebx, &ecx, &edx);
-        memcpy(str + i * 16 +  0, &eax, 4);
-        memcpy(str + i * 16 +  4, &ebx, 4);
-        memcpy(str + i * 16 +  8, &ecx, 4);
-        memcpy(str + i * 16 + 12, &edx, 4);
-    }
-    return 0;
-}
-#endif
-
-/* Fill a x86_def_t struct with information about the host CPU, and
- * the CPU features supported by the host hardware + host kernel
- *
- * This function may be called only if KVM is enabled.
- */
-static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
-{
-#ifdef CONFIG_KVM
-    KVMState *s = kvm_state;
-    uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
-
-    assert(kvm_enabled());
-
-    x86_cpu_def->name = "host";
-    host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
-    x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx);
-
-    host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx);
-    x86_cpu_def->family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
-    x86_cpu_def->model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >> 12);
-    x86_cpu_def->stepping = eax & 0x0F;
-
-    x86_cpu_def->level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
-    x86_cpu_def->features = kvm_arch_get_supported_cpuid(s, 0x1, 0, R_EDX);
-    x86_cpu_def->ext_features = kvm_arch_get_supported_cpuid(s, 0x1, 0, R_ECX);
-
-    if (x86_cpu_def->level >= 7) {
-        x86_cpu_def->cpuid_7_0_ebx_features =
-                    kvm_arch_get_supported_cpuid(s, 0x7, 0, R_EBX);
-    } else {
-        x86_cpu_def->cpuid_7_0_ebx_features = 0;
-    }
-
-    x86_cpu_def->xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
-    x86_cpu_def->ext2_features =
-                kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX);
-    x86_cpu_def->ext3_features =
-                kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_ECX);
-
-    cpu_x86_fill_model_id(x86_cpu_def->model_id);
-
-    /* Call Centaur's CPUID instruction. */
-    if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) {
-        host_cpuid(0xC0000000, 0, &eax, &ebx, &ecx, &edx);
-        eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
-        if (eax >= 0xC0000001) {
-            /* Support VIA max extended level */
-            x86_cpu_def->xlevel2 = eax;
-            host_cpuid(0xC0000001, 0, &eax, &ebx, &ecx, &edx);
-            x86_cpu_def->ext4_features =
-                    kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
-        }
-    }
-
-    /* Other KVM-specific feature fields: */
-    x86_cpu_def->svm_features =
-        kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX);
-    x86_cpu_def->kvm_features =
-        kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX);
-
-#endif /* CONFIG_KVM */
-}
-
 static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
 {
     int i;
@@ -975,31 +876,31 @@ static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
 static int kvm_check_features_against_host(X86CPU *cpu)
 {
     CPUX86State *env = &cpu->env;
-    x86_def_t host_def;
+    ObjectClass *host_oc = object_class_by_name(TYPE_HOST_X86_CPU);
+    X86CPUClass *host_xcc = X86_CPU_CLASS(host_oc);
     uint32_t mask;
     int rv, i;
     struct model_features_t ft[] = {
-        {&env->cpuid_features, &host_def.features,
+        {&env->cpuid_features, &host_xcc->info.features,
             FEAT_1_EDX },
-        {&env->cpuid_ext_features, &host_def.ext_features,
+        {&env->cpuid_ext_features, &host_xcc->info.ext_features,
             FEAT_1_ECX },
-        {&env->cpuid_ext2_features, &host_def.ext2_features,
+        {&env->cpuid_ext2_features, &host_xcc->info.ext2_features,
             FEAT_8000_0001_EDX },
-        {&env->cpuid_ext3_features, &host_def.ext3_features,
+        {&env->cpuid_ext3_features, &host_xcc->info.ext3_features,
             FEAT_8000_0001_ECX },
-        {&env->cpuid_ext4_features, &host_def.ext4_features,
+        {&env->cpuid_ext4_features, &host_xcc->info.ext4_features,
             FEAT_C000_0001_EDX },
-        {&env->cpuid_7_0_ebx_features, &host_def.cpuid_7_0_ebx_features,
+        {&env->cpuid_7_0_ebx_features, &host_xcc->info.cpuid_7_0_ebx_features,
             FEAT_7_0_EBX },
-        {&env->cpuid_svm_features, &host_def.svm_features,
+        {&env->cpuid_svm_features, &host_xcc->info.svm_features,
             FEAT_SVM },
-        {&env->cpuid_kvm_features, &host_def.kvm_features,
+        {&env->cpuid_kvm_features, &host_xcc->info.kvm_features,
             FEAT_KVM },
     };
 
     assert(kvm_enabled());
 
-    kvm_cpu_fill_host(&host_def);
     for (rv = 0, i = 0; i < ARRAY_SIZE(ft); ++i) {
         FeatureWord w = ft[i].feat_word;
         FeatureWordInfo *wi = &feature_word_info[w];
@@ -1261,40 +1162,30 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
     cpu->env.tsc_khz = value / 1000;
 }
 
-static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
+static ObjectClass *x86_cpu_class_by_name(const char *name)
 {
-    x86_def_t *def;
-    int i;
+    ObjectClass *oc;
+    char *typename;
 
     if (name == NULL) {
-        return -1;
-    }
-    if (kvm_enabled() && strcmp(name, "host") == 0) {
-        kvm_cpu_fill_host(x86_cpu_def);
-        return 0;
+        return NULL;
     }
 
-    for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
-        def = &builtin_x86_defs[i];
-        if (strcmp(name, def->name) == 0) {
-            memcpy(x86_cpu_def, def, sizeof(*def));
-            /* sysenter isn't supported in 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);
-                x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx);
-            }
-            return 0;
+    if (strcmp(name, "host") == 0) {
+        if (kvm_enabled()) {
+            return object_class_by_name(TYPE_HOST_X86_CPU);
         }
+        return NULL;
     }
 
-    return -1;
+    typename = g_strdup_printf("%s-" TYPE_X86_CPU, name);
+    oc = object_class_by_name(typename);
+    g_free(typename);
+    if (oc != NULL && (!object_class_dynamic_cast(oc, TYPE_X86_CPU) ||
+                       object_class_is_abstract(oc))) {
+        oc = NULL;
+    }
+    return oc;
 }
 
 /* Parse "+feature,-feature,feature=foo" CPU feature string
@@ -1516,60 +1407,13 @@ static void filter_features_for_kvm(X86CPU *cpu)
 }
 #endif
 
-static int cpu_x86_register(X86CPU *cpu, const char *name)
-{
-    CPUX86State *env = &cpu->env;
-    x86_def_t def1, *def = &def1;
-    Error *error = NULL;
-
-    memset(def, 0, sizeof(*def));
-
-    if (cpu_x86_find_by_name(def, name) < 0) {
-        error_setg(&error, "Unable to find CPU definition: %s", name);
-        goto out;
-    }
-
-    if (kvm_enabled()) {
-        def->kvm_features |= kvm_default_features;
-    }
-    def->ext_features |= CPUID_EXT_HYPERVISOR;
-
-    object_property_set_str(OBJECT(cpu), def->vendor, "vendor", &error);
-    object_property_set_int(OBJECT(cpu), def->level, "level", &error);
-    object_property_set_int(OBJECT(cpu), def->family, "family", &error);
-    object_property_set_int(OBJECT(cpu), def->model, "model", &error);
-    object_property_set_int(OBJECT(cpu), def->stepping, "stepping", &error);
-    env->cpuid_features = def->features;
-    env->cpuid_ext_features = def->ext_features;
-    env->cpuid_ext2_features = def->ext2_features;
-    env->cpuid_ext3_features = def->ext3_features;
-    object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", &error);
-    env->cpuid_kvm_features = def->kvm_features;
-    env->cpuid_svm_features = def->svm_features;
-    env->cpuid_ext4_features = def->ext4_features;
-    env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features;
-    env->cpuid_xlevel2 = def->xlevel2;
-
-    object_property_set_str(OBJECT(cpu), def->model_id, "model-id", &error);
-    if (error) {
-        goto out;
-    }
-
-out:
-    if (error) {
-        fprintf(stderr, "%s\n", error_get_pretty(error));
-        error_free(error);
-        return -1;
-    }
-    return 0;
-}
-
 X86CPU *cpu_x86_init(const char *cpu_model)
 {
     X86CPU *cpu = NULL;
     CPUX86State *env;
     gchar **model_pieces;
     char *name, *features;
+    ObjectClass *oc;
     Error *error = NULL;
 
     model_pieces = g_strsplit(cpu_model, ",", 2);
@@ -1580,13 +1424,13 @@ X86CPU *cpu_x86_init(const char *cpu_model)
     name = model_pieces[0];
     features = model_pieces[1];
 
-    cpu = X86_CPU(object_new(TYPE_X86_CPU));
-    env = &cpu->env;
-    env->cpu_model_str = cpu_model;
-
-    if (cpu_x86_register(cpu, name) < 0) {
+    oc = x86_cpu_class_by_name(name);
+    if (oc == NULL) {
         goto error;
     }
+    cpu = X86_CPU(object_new(object_class_get_name(oc)));
+    env = &cpu->env;
+    env->cpu_model_str = cpu_model;
 
     cpu_x86_parse_featurestr(cpu, features, &error);
     if (error) {
@@ -1621,30 +1465,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];
-
-        /* 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;
-            }
-        }
-    }
-}
-
 static void get_cpuid_vendor(CPUX86State *env, uint32_t *ebx,
                              uint32_t *ecx, uint32_t *edx)
 {
@@ -2198,6 +2018,8 @@ static void x86_cpu_initfn(Object *obj)
     CPUState *cs = CPU(obj);
     X86CPU *cpu = X86_CPU(obj);
     CPUX86State *env = &cpu->env;
+    X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
+    const x86_def_t *def = &xcc->info;
     static int inited;
 
     cpu_exec_init(env);
@@ -2227,6 +2049,41 @@ static void x86_cpu_initfn(Object *obj)
                         x86_cpuid_get_tsc_freq,
                         x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
 
+    /* sysenter isn't supported in 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()) {
+        host_cpuid(0, 0, NULL, &env->cpuid_vendor1, &env->cpuid_vendor2,
+                   &env->cpuid_vendor3);
+    } else {
+        object_property_set_str(OBJECT(cpu), def->vendor, "vendor", NULL);
+    }
+
+    object_property_set_int(OBJECT(cpu), def->level, "level", NULL);
+    object_property_set_int(OBJECT(cpu), def->family, "family", NULL);
+    object_property_set_int(OBJECT(cpu), def->model, "model", NULL);
+    object_property_set_int(OBJECT(cpu), def->stepping, "stepping", NULL);
+    env->cpuid_features = def->features;
+    env->cpuid_ext_features = def->ext_features;
+    env->cpuid_ext_features |= CPUID_EXT_HYPERVISOR;
+    env->cpuid_ext2_features = def->ext2_features;
+    env->cpuid_ext3_features = def->ext3_features;
+    object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", NULL);
+    env->cpuid_kvm_features = def->kvm_features;
+    if (kvm_enabled()) {
+        env->cpuid_kvm_features |= kvm_default_features;
+    }
+    env->cpuid_svm_features = def->svm_features;
+    env->cpuid_ext4_features = def->ext4_features;
+    env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features;
+    env->cpuid_xlevel2 = def->xlevel2;
+
+    object_property_set_str(OBJECT(cpu), def->model_id, "model-id", NULL);
+
     env->cpuid_apic_id = x86_cpu_apic_id_from_index(cs->cpu_index);
 
     /* init various static tables used in TCG mode */
@@ -2239,6 +2096,27 @@ static void x86_cpu_initfn(Object *obj)
     }
 }
 
+static void x86_cpu_def_class_init(ObjectClass *oc, void *data)
+{
+    X86CPUClass *xcc = X86_CPU_CLASS(oc);
+    x86_def_t *def = data;
+    int i;
+    static const char *versioned_models[] = { "qemu32", "qemu64", "athlon" };
+
+    memcpy(&xcc->info, def, sizeof(x86_def_t));
+
+    /* Look for specific models that have the QEMU version in .model_id */
+    for (i = 0; i < ARRAY_SIZE(versioned_models); i++) {
+        if (strcmp(versioned_models[i], def->name) == 0) {
+            pstrcpy(xcc->info.model_id, sizeof(xcc->info.model_id),
+                    "QEMU Virtual CPU version ");
+            pstrcat(xcc->info.model_id, sizeof(xcc->info.model_id),
+                    qemu_get_version());
+            break;
+        }
+    }
+}
+
 static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
 {
     X86CPUClass *xcc = X86_CPU_CLASS(oc);
@@ -2250,6 +2128,21 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
 
     xcc->parent_reset = cc->reset;
     cc->reset = x86_cpu_reset;
+
+    cc->class_by_name = x86_cpu_class_by_name;
+}
+
+static void x86_register_cpu_type(const x86_def_t *def)
+{
+    TypeInfo type_info = {
+        .parent = TYPE_X86_CPU,
+        .class_init = x86_cpu_def_class_init,
+        .class_data = (void *)def,
+    };
+
+    type_info.name = g_strdup_printf("%s-" TYPE_X86_CPU, def->name);
+    type_register(&type_info);
+    g_free((void *)type_info.name);
 }
 
 static const TypeInfo x86_cpu_type_info = {
@@ -2257,14 +2150,19 @@ 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,
 };
 
 static void x86_cpu_register_types(void)
 {
+    int i;
+
     type_register_static(&x86_cpu_type_info);
+    for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
+        x86_register_cpu_type(&builtin_x86_defs[i]);
+    }
 }
 
 type_init(x86_cpu_register_types)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 7577e4f..2aef7b7 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -887,7 +887,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);
@@ -1079,7 +1078,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
 
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 9ebf181..dbfa670 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -735,6 +735,80 @@ static int kvm_get_supported_msrs(KVMState *s)
     return ret;
 }
 
+static void kvm_host_cpu_initfn(Object *obj)
+{
+    assert(kvm_enabled());
+}
+
+static bool kvm_host_cpu_needs_class_init;
+
+static void kvm_host_cpu_class_init(ObjectClass *oc, void *data)
+{
+    X86CPUClass *xcc = X86_CPU_CLASS(oc);
+    x86_def_t *x86_cpu_def = &xcc->info;
+    KVMState *s = kvm_state;
+    uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
+    int i;
+
+    if (!kvm_enabled()) {
+        kvm_host_cpu_needs_class_init = true;
+        return;
+    }
+
+    xcc->info.name = "host";
+
+    /* Vendor will be set in initfn if kvm_enabled() */
+
+    host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx);
+    x86_cpu_def->family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
+    x86_cpu_def->model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >> 12);
+    x86_cpu_def->stepping = eax & 0x0F;
+
+    x86_cpu_def->level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
+    x86_cpu_def->features = kvm_arch_get_supported_cpuid(s, 0x1, 0, R_EDX);
+    x86_cpu_def->ext_features = kvm_arch_get_supported_cpuid(s, 0x1, 0, R_ECX);
+
+    if (x86_cpu_def->level >= 7) {
+        x86_cpu_def->cpuid_7_0_ebx_features =
+                    kvm_arch_get_supported_cpuid(s, 0x7, 0, R_EBX);
+    } else {
+        x86_cpu_def->cpuid_7_0_ebx_features = 0;
+    }
+
+    x86_cpu_def->xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
+    x86_cpu_def->ext2_features =
+                kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX);
+    x86_cpu_def->ext3_features =
+                kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_ECX);
+
+    for (i = 0; i < 3; i++) {
+        host_cpuid(0x80000002 + i, 0, &eax, &ebx, &ecx, &edx);
+        memcpy(xcc->info.model_id + i * 16 +  0, &eax, 4);
+        memcpy(xcc->info.model_id + i * 16 +  4, &ebx, 4);
+        memcpy(xcc->info.model_id + i * 16 +  8, &ecx, 4);
+        memcpy(xcc->info.model_id + i * 16 + 12, &edx, 4);
+    }
+
+    /* Call Centaur's CPUID instruction. */
+    if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) {
+        host_cpuid(0xC0000000, 0, &eax, &ebx, &ecx, &edx);
+        eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
+        if (eax >= 0xC0000001) {
+            /* Support VIA max extended level */
+            x86_cpu_def->xlevel2 = eax;
+            host_cpuid(0xC0000001, 0, &eax, &ebx, &ecx, &edx);
+            x86_cpu_def->ext4_features =
+                    kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
+        }
+    }
+
+    /* Other KVM-specific feature fields: */
+    x86_cpu_def->svm_features =
+        kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX);
+    x86_cpu_def->kvm_features =
+        kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX);
+}
+
 int kvm_arch_init(KVMState *s)
 {
     QemuOptsList *list = qemu_find_opts("machine");
@@ -797,6 +871,11 @@ int kvm_arch_init(KVMState *s)
             }
         }
     }
+
+    if (kvm_host_cpu_needs_class_init) {
+        kvm_host_cpu_class_init(object_class_by_name(TYPE_HOST_X86_CPU), NULL);
+    }
+
     return 0;
 }
 
@@ -2330,3 +2409,17 @@ int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id)
     return kvm_deassign_irq_internal(s, dev_id, KVM_DEV_IRQ_GUEST_MSIX |
                                                 KVM_DEV_IRQ_HOST_MSIX);
 }
+
+static const TypeInfo host_x86_cpu_type_info = {
+    .name = TYPE_HOST_X86_CPU,
+    .parent = TYPE_X86_CPU,
+    .instance_init = kvm_host_cpu_initfn,
+    .class_init = kvm_host_cpu_class_init,
+};
+
+static void kvm_register_types(void)
+{
+    type_register_static(&host_x86_cpu_type_info);
+}
+
+type_init(kvm_register_types)
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH qom-cpu-next v3 3/4] target-i386: Slim conversion to X86CPU subclasses

[Igor Mammedov]

On Sat,  2 Feb 2013 01:37:07 +0100
Andreas Färber <afaerber@suse.de> wrote:

> Move x86_def_t definition to header and embed into X86CPUClass.
> Register types per built-in model definition.
> 
> Move version initialization from x86_cpudef_setup() to class_init.
> 
> Inline cpu_x86_register() into the X86CPU initfn.
> Since instance_init cannot reports errors, drop error handling.
> 
> Replace cpu_x86_find_by_name() with x86_cpu_class_by_name().
> Move KVM host vendor override from cpu_x86_find_by_name() to the initfn.
> 
> Register host-{i386,x86_64}-cpu type from KVM code to avoid #ifdefs.
> Make kvm_cpu_fill_host() a class_init and inline cpu_x86_fill_model_id().
> 
> Let kvm_check_features_against_host() obtain host-{i386,86_64}-cpu for
> comparison.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  target-i386/cpu-qom.h |   24 ++++
>  target-i386/cpu.c     |  324
> +++++++++++++++++-------------------------------- target-i386/cpu.h
> |    2 - target-i386/kvm.c     |   93 ++++++++++++++
>  4 Dateien geändert, 228 Zeilen hinzugefügt(+), 215 Zeilen entfernt(-)
> 
> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> index 48e6b54..80bf72d 100644
> --- a/target-i386/cpu-qom.h
> +++ b/target-i386/cpu-qom.h
> @@ -30,6 +30,27 @@
>  #define TYPE_X86_CPU "i386-cpu"
>  #endif
>  
> +#define TYPE_HOST_X86_CPU "host-" TYPE_X86_CPU
> +
> +typedef struct x86_def_t {
> +    const char *name;
> +    uint32_t level;
> +    /* vendor is zero-terminated, 12 character ASCII string */
> +    char vendor[CPUID_VENDOR_SZ + 1];
> +    int family;
> +    int model;
> +    int stepping;
> +    uint32_t features, ext_features, ext2_features, ext3_features;
> +    uint32_t kvm_features, svm_features;
> +    uint32_t xlevel;
> +    char model_id[48];
> +    /* 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;
> +
>  #define X86_CPU_CLASS(klass) \
>      OBJECT_CLASS_CHECK(X86CPUClass, (klass), TYPE_X86_CPU)
>  #define X86_CPU(obj) \
> @@ -41,6 +62,7 @@
>   * X86CPUClass:
>   * @parent_realize: The parent class' realize handler.
>   * @parent_reset: The parent class' reset handler.
> + * @info: Model-specific data.
>   *
>   * An x86 CPU model or family.
>   */
> @@ -51,6 +73,8 @@ typedef struct X86CPUClass {
>  
>      DeviceRealize parent_realize;
>      void (*parent_reset)(CPUState *cpu);
> +
> +    x86_def_t info;
Is it necessary to embed it here, could pointer to corresponding static array
be used here?
That way one could avoid extra memcpy in class_init().

>  } X86CPUClass;
>  
>  /**
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index ee2fd6b..6c95740 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -346,25 +346,6 @@ static void add_flagname_to_bitmaps(const char
> *flagname, }
>  }
>  
> -typedef struct x86_def_t {
> -    const char *name;
> -    uint32_t level;
> -    /* vendor is zero-terminated, 12 character ASCII string */
> -    char vendor[CPUID_VENDOR_SZ + 1];
> -    int family;
> -    int model;
> -    int stepping;
> -    uint32_t features, ext_features, ext2_features, ext3_features;
> -    uint32_t kvm_features, svm_features;
> -    uint32_t xlevel;
> -    char model_id[48];
> -    /* 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;
> -
>  #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
>  #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
>            CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC)
> @@ -868,86 +849,6 @@ static x86_def_t builtin_x86_defs[] = {
>      },
>  };
>  
> -#ifdef CONFIG_KVM
> -static int cpu_x86_fill_model_id(char *str)
> -{
> -    uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
> -    int i;
> -
> -    for (i = 0; i < 3; i++) {
> -        host_cpuid(0x80000002 + i, 0, &eax, &ebx, &ecx, &edx);
> -        memcpy(str + i * 16 +  0, &eax, 4);
> -        memcpy(str + i * 16 +  4, &ebx, 4);
> -        memcpy(str + i * 16 +  8, &ecx, 4);
> -        memcpy(str + i * 16 + 12, &edx, 4);
> -    }
> -    return 0;
> -}
> -#endif
> -
> -/* Fill a x86_def_t struct with information about the host CPU, and
> - * the CPU features supported by the host hardware + host kernel
> - *
> - * This function may be called only if KVM is enabled.
> - */
> -static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
> -{
> -#ifdef CONFIG_KVM
> -    KVMState *s = kvm_state;
> -    uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
> -
> -    assert(kvm_enabled());
> -
> -    x86_cpu_def->name = "host";
> -    host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
> -    x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx);
> -
> -    host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx);
> -    x86_cpu_def->family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
> -    x86_cpu_def->model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >> 12);
> -    x86_cpu_def->stepping = eax & 0x0F;
> -
> -    x86_cpu_def->level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
> -    x86_cpu_def->features = kvm_arch_get_supported_cpuid(s, 0x1, 0, R_EDX);
> -    x86_cpu_def->ext_features = kvm_arch_get_supported_cpuid(s, 0x1, 0,
> R_ECX); -
> -    if (x86_cpu_def->level >= 7) {
> -        x86_cpu_def->cpuid_7_0_ebx_features =
> -                    kvm_arch_get_supported_cpuid(s, 0x7, 0, R_EBX);
> -    } else {
> -        x86_cpu_def->cpuid_7_0_ebx_features = 0;
> -    }
> -
> -    x86_cpu_def->xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0,
> R_EAX);
> -    x86_cpu_def->ext2_features =
> -                kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX);
> -    x86_cpu_def->ext3_features =
> -                kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_ECX);
> -
> -    cpu_x86_fill_model_id(x86_cpu_def->model_id);
> -
> -    /* Call Centaur's CPUID instruction. */
> -    if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) {
> -        host_cpuid(0xC0000000, 0, &eax, &ebx, &ecx, &edx);
> -        eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
> -        if (eax >= 0xC0000001) {
> -            /* Support VIA max extended level */
> -            x86_cpu_def->xlevel2 = eax;
> -            host_cpuid(0xC0000001, 0, &eax, &ebx, &ecx, &edx);
> -            x86_cpu_def->ext4_features =
> -                    kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
> -        }
> -    }
> -
> -    /* Other KVM-specific feature fields: */
> -    x86_cpu_def->svm_features =
> -        kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX);
> -    x86_cpu_def->kvm_features =
> -        kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX);
> -
> -#endif /* CONFIG_KVM */
> -}
> -
>  static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
>  {
>      int i;
> @@ -975,31 +876,31 @@ static int unavailable_host_feature(FeatureWordInfo
> *f, uint32_t mask) static int kvm_check_features_against_host(X86CPU *cpu)
>  {
>      CPUX86State *env = &cpu->env;
> -    x86_def_t host_def;
> +    ObjectClass *host_oc = object_class_by_name(TYPE_HOST_X86_CPU);
> +    X86CPUClass *host_xcc = X86_CPU_CLASS(host_oc);
>      uint32_t mask;
>      int rv, i;
>      struct model_features_t ft[] = {
> -        {&env->cpuid_features, &host_def.features,
> +        {&env->cpuid_features, &host_xcc->info.features,
>              FEAT_1_EDX },
> -        {&env->cpuid_ext_features, &host_def.ext_features,
> +        {&env->cpuid_ext_features, &host_xcc->info.ext_features,
>              FEAT_1_ECX },
> -        {&env->cpuid_ext2_features, &host_def.ext2_features,
> +        {&env->cpuid_ext2_features, &host_xcc->info.ext2_features,
>              FEAT_8000_0001_EDX },
> -        {&env->cpuid_ext3_features, &host_def.ext3_features,
> +        {&env->cpuid_ext3_features, &host_xcc->info.ext3_features,
>              FEAT_8000_0001_ECX },
> -        {&env->cpuid_ext4_features, &host_def.ext4_features,
> +        {&env->cpuid_ext4_features, &host_xcc->info.ext4_features,
>              FEAT_C000_0001_EDX },
> -        {&env->cpuid_7_0_ebx_features, &host_def.cpuid_7_0_ebx_features,
> +        {&env->cpuid_7_0_ebx_features,
> &host_xcc->info.cpuid_7_0_ebx_features, FEAT_7_0_EBX },
> -        {&env->cpuid_svm_features, &host_def.svm_features,
> +        {&env->cpuid_svm_features, &host_xcc->info.svm_features,
>              FEAT_SVM },
> -        {&env->cpuid_kvm_features, &host_def.kvm_features,
> +        {&env->cpuid_kvm_features, &host_xcc->info.kvm_features,
>              FEAT_KVM },
>      };
>  
>      assert(kvm_enabled());
>  
> -    kvm_cpu_fill_host(&host_def);
>      for (rv = 0, i = 0; i < ARRAY_SIZE(ft); ++i) {
>          FeatureWord w = ft[i].feat_word;
>          FeatureWordInfo *wi = &feature_word_info[w];
> @@ -1261,40 +1162,30 @@ static void x86_cpuid_set_tsc_freq(Object *obj,
> Visitor *v, void *opaque, cpu->env.tsc_khz = value / 1000;
>  }
>  
> -static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
> +static ObjectClass *x86_cpu_class_by_name(const char *name)
>  {
> -    x86_def_t *def;
> -    int i;
> +    ObjectClass *oc;
> +    char *typename;
>  
>      if (name == NULL) {
> -        return -1;
> -    }
> -    if (kvm_enabled() && strcmp(name, "host") == 0) {
> -        kvm_cpu_fill_host(x86_cpu_def);
> -        return 0;
> +        return NULL;
>      }
>  
> -    for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
> -        def = &builtin_x86_defs[i];
> -        if (strcmp(name, def->name) == 0) {
> -            memcpy(x86_cpu_def, def, sizeof(*def));
> -            /* sysenter isn't supported in 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);
> -                x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx,
> ecx);
> -            }
> -            return 0;
> +    if (strcmp(name, "host") == 0) {
> +        if (kvm_enabled()) {
> +            return object_class_by_name(TYPE_HOST_X86_CPU);
>          }
> +        return NULL;
>      }
block is not necessary, the following block could yield the same result.

>  
> -    return -1;
> +    typename = g_strdup_printf("%s-" TYPE_X86_CPU, name);
> +    oc = object_class_by_name(typename);
> +    g_free(typename);
> +    if (oc != NULL && (!object_class_dynamic_cast(oc, TYPE_X86_CPU) ||
> +                       object_class_is_abstract(oc))) {
> +        oc = NULL;
> +    }
> +    return oc;
>  }
>  
>  /* Parse "+feature,-feature,feature=foo" CPU feature string
> @@ -1516,60 +1407,13 @@ static void filter_features_for_kvm(X86CPU *cpu)
>  }
>  #endif
>  
> -static int cpu_x86_register(X86CPU *cpu, const char *name)
> -{
> -    CPUX86State *env = &cpu->env;
> -    x86_def_t def1, *def = &def1;
> -    Error *error = NULL;
> -
> -    memset(def, 0, sizeof(*def));
> -
> -    if (cpu_x86_find_by_name(def, name) < 0) {
> -        error_setg(&error, "Unable to find CPU definition: %s", name);
> -        goto out;
> -    }
> -
> -    if (kvm_enabled()) {
> -        def->kvm_features |= kvm_default_features;
> -    }
> -    def->ext_features |= CPUID_EXT_HYPERVISOR;
> -
> -    object_property_set_str(OBJECT(cpu), def->vendor, "vendor", &error);
> -    object_property_set_int(OBJECT(cpu), def->level, "level", &error);
> -    object_property_set_int(OBJECT(cpu), def->family, "family", &error);
> -    object_property_set_int(OBJECT(cpu), def->model, "model", &error);
> -    object_property_set_int(OBJECT(cpu), def->stepping, "stepping",
> &error);
> -    env->cpuid_features = def->features;
> -    env->cpuid_ext_features = def->ext_features;
> -    env->cpuid_ext2_features = def->ext2_features;
> -    env->cpuid_ext3_features = def->ext3_features;
> -    object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", &error);
> -    env->cpuid_kvm_features = def->kvm_features;
> -    env->cpuid_svm_features = def->svm_features;
> -    env->cpuid_ext4_features = def->ext4_features;
> -    env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features;
> -    env->cpuid_xlevel2 = def->xlevel2;
> -
> -    object_property_set_str(OBJECT(cpu), def->model_id, "model-id",
> &error);
> -    if (error) {
> -        goto out;
> -    }
> -
> -out:
> -    if (error) {
> -        fprintf(stderr, "%s\n", error_get_pretty(error));
> -        error_free(error);
> -        return -1;
> -    }
> -    return 0;
> -}
> -
>  X86CPU *cpu_x86_init(const char *cpu_model)
>  {
>      X86CPU *cpu = NULL;
>      CPUX86State *env;
>      gchar **model_pieces;
>      char *name, *features;
> +    ObjectClass *oc;
>      Error *error = NULL;
>  
>      model_pieces = g_strsplit(cpu_model, ",", 2);
> @@ -1580,13 +1424,13 @@ X86CPU *cpu_x86_init(const char *cpu_model)
>      name = model_pieces[0];
>      features = model_pieces[1];
>  
> -    cpu = X86_CPU(object_new(TYPE_X86_CPU));
> -    env = &cpu->env;
> -    env->cpu_model_str = cpu_model;
> -
> -    if (cpu_x86_register(cpu, name) < 0) {
> +    oc = x86_cpu_class_by_name(name);
> +    if (oc == NULL) {
make sure error is reported when cpu is not found:
           error_setg(&error, "Can't find CPU: %s", name);

>          goto error;
>      }
> +    cpu = X86_CPU(object_new(object_class_get_name(oc)));
> +    env = &cpu->env;
> +    env->cpu_model_str = cpu_model;
>  
>      cpu_x86_parse_featurestr(cpu, features, &error);
>      if (error) {
> @@ -1621,30 +1465,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];
> -
> -        /* 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;
> -            }
> -        }
> -    }
> -}
> -
>  static void get_cpuid_vendor(CPUX86State *env, uint32_t *ebx,
>                               uint32_t *ecx, uint32_t *edx)
>  {
> @@ -2198,6 +2018,8 @@ static void x86_cpu_initfn(Object *obj)
>      CPUState *cs = CPU(obj);
>      X86CPU *cpu = X86_CPU(obj);
>      CPUX86State *env = &cpu->env;
> +    X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
> +    const x86_def_t *def = &xcc->info;
>      static int inited;
>  
>      cpu_exec_init(env);
> @@ -2227,6 +2049,41 @@ static void x86_cpu_initfn(Object *obj)
>                          x86_cpuid_get_tsc_freq,
>                          x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
>  
> +    /* sysenter isn't supported in 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()) {
> +        host_cpuid(0, 0, NULL, &env->cpuid_vendor1, &env->cpuid_vendor2,
> +                   &env->cpuid_vendor3);
This is not per instance specific, this override would be better done to
sub-classes in kvm_arch_init(). As bonus we would get actual 'vendor' when
doing class introspection provided it's done after kvm_init() is called.


> +    } else {
> +        object_property_set_str(OBJECT(cpu), def->vendor, "vendor", NULL);
> +    }
> +
> +    object_property_set_int(OBJECT(cpu), def->level, "level", NULL);
> +    object_property_set_int(OBJECT(cpu), def->family, "family", NULL);
> +    object_property_set_int(OBJECT(cpu), def->model, "model", NULL);
> +    object_property_set_int(OBJECT(cpu), def->stepping, "stepping", NULL);
> +    env->cpuid_features = def->features;
> +    env->cpuid_ext_features = def->ext_features;
> +    env->cpuid_ext_features |= CPUID_EXT_HYPERVISOR;
> +    env->cpuid_ext2_features = def->ext2_features;
> +    env->cpuid_ext3_features = def->ext3_features;
> +    object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", NULL);
> +    env->cpuid_kvm_features = def->kvm_features;
> +    if (kvm_enabled()) {
> +        env->cpuid_kvm_features |= kvm_default_features;
> +    }
> +    env->cpuid_svm_features = def->svm_features;
> +    env->cpuid_ext4_features = def->ext4_features;
> +    env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features;
> +    env->cpuid_xlevel2 = def->xlevel2;
> +
> +    object_property_set_str(OBJECT(cpu), def->model_id, "model-id", NULL);
All this feature initialization in initfn() is not compatible with
static/global properties because they are set in device_initfn(). But I can
remove properties/features from here as they are converted to static
properties and incrementally move their defaults initialization into
new x86_cpu_def_class_init().

> +
>      env->cpuid_apic_id = x86_cpu_apic_id_from_index(cs->cpu_index);
>  
>      /* init various static tables used in TCG mode */
> @@ -2239,6 +2096,27 @@ static void x86_cpu_initfn(Object *obj)
>      }
>  }
>  
> +static void x86_cpu_def_class_init(ObjectClass *oc, void *data)
> +{
> +    X86CPUClass *xcc = X86_CPU_CLASS(oc);
> +    x86_def_t *def = data;
> +    int i;
> +    static const char *versioned_models[] = { "qemu32", "qemu64",
> "athlon" }; +
> +    memcpy(&xcc->info, def, sizeof(x86_def_t));
perhaps sizeof(xcc->info) would be better?

> +
> +    /* Look for specific models that have the QEMU version in .model_id */
> +    for (i = 0; i < ARRAY_SIZE(versioned_models); i++) {
> +        if (strcmp(versioned_models[i], def->name) == 0) {
> +            pstrcpy(xcc->info.model_id, sizeof(xcc->info.model_id),
> +                    "QEMU Virtual CPU version ");
> +            pstrcat(xcc->info.model_id, sizeof(xcc->info.model_id),
> +                    qemu_get_version());
> +            break;
> +        }
> +    }
> +}
> +
>  static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>  {
>      X86CPUClass *xcc = X86_CPU_CLASS(oc);
> @@ -2250,6 +2128,21 @@ static void x86_cpu_common_class_init(ObjectClass
> *oc, void *data) 
>      xcc->parent_reset = cc->reset;
>      cc->reset = x86_cpu_reset;
> +
> +    cc->class_by_name = x86_cpu_class_by_name;
> +}
> +
> +static void x86_register_cpu_type(const x86_def_t *def)
> +{
> +    TypeInfo type_info = {
> +        .parent = TYPE_X86_CPU,
> +        .class_init = x86_cpu_def_class_init,
> +        .class_data = (void *)def,
> +    };
> +
> +    type_info.name = g_strdup_printf("%s-" TYPE_X86_CPU, def->name);
> +    type_register(&type_info);
> +    g_free((void *)type_info.name);
>  }
>  
>  static const TypeInfo x86_cpu_type_info = {
> @@ -2257,14 +2150,19 @@ 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,
>  };
>  
>  static void x86_cpu_register_types(void)
>  {
> +    int i;
> +
>      type_register_static(&x86_cpu_type_info);
> +    for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
> +        x86_register_cpu_type(&builtin_x86_defs[i]);
> +    }
>  }
>  
>  type_init(x86_cpu_register_types)
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 7577e4f..2aef7b7 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -887,7 +887,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);
> @@ -1079,7 +1078,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
>  
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 9ebf181..dbfa670 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -735,6 +735,80 @@ static int kvm_get_supported_msrs(KVMState *s)
>      return ret;
>  }
>  
> +static void kvm_host_cpu_initfn(Object *obj)
> +{
> +    assert(kvm_enabled());
> +}
> +
> +static bool kvm_host_cpu_needs_class_init;
> +
> +static void kvm_host_cpu_class_init(ObjectClass *oc, void *data)
> +{
> +    X86CPUClass *xcc = X86_CPU_CLASS(oc);
> +    x86_def_t *x86_cpu_def = &xcc->info;
> +    KVMState *s = kvm_state;
> +    uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
> +    int i;
> +
> +    if (!kvm_enabled()) {
> +        kvm_host_cpu_needs_class_init = true;
> +        return;
> +    }
> +
> +    xcc->info.name = "host";
> +
> +    /* Vendor will be set in initfn if kvm_enabled() */
> +
> +    host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx);
> +    x86_cpu_def->family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
> +    x86_cpu_def->model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >> 12);
> +    x86_cpu_def->stepping = eax & 0x0F;
> +
> +    x86_cpu_def->level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
> +    x86_cpu_def->features = kvm_arch_get_supported_cpuid(s, 0x1, 0, R_EDX);
> +    x86_cpu_def->ext_features = kvm_arch_get_supported_cpuid(s, 0x1, 0,
> R_ECX); +
> +    if (x86_cpu_def->level >= 7) {
> +        x86_cpu_def->cpuid_7_0_ebx_features =
> +                    kvm_arch_get_supported_cpuid(s, 0x7, 0, R_EBX);
> +    } else {
> +        x86_cpu_def->cpuid_7_0_ebx_features = 0;
> +    }
> +
> +    x86_cpu_def->xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0,
> R_EAX);
> +    x86_cpu_def->ext2_features =
> +                kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX);
> +    x86_cpu_def->ext3_features =
> +                kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_ECX);
> +
> +    for (i = 0; i < 3; i++) {
> +        host_cpuid(0x80000002 + i, 0, &eax, &ebx, &ecx, &edx);
> +        memcpy(xcc->info.model_id + i * 16 +  0, &eax, 4);
> +        memcpy(xcc->info.model_id + i * 16 +  4, &ebx, 4);
> +        memcpy(xcc->info.model_id + i * 16 +  8, &ecx, 4);
> +        memcpy(xcc->info.model_id + i * 16 + 12, &edx, 4);
> +    }
> +
> +    /* Call Centaur's CPUID instruction. */
> +    if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) {
> +        host_cpuid(0xC0000000, 0, &eax, &ebx, &ecx, &edx);
> +        eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
> +        if (eax >= 0xC0000001) {
> +            /* Support VIA max extended level */
> +            x86_cpu_def->xlevel2 = eax;
> +            host_cpuid(0xC0000001, 0, &eax, &ebx, &ecx, &edx);
> +            x86_cpu_def->ext4_features =
> +                    kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
> +        }
> +    }
> +
> +    /* Other KVM-specific feature fields: */
> +    x86_cpu_def->svm_features =
> +        kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX);
> +    x86_cpu_def->kvm_features =
> +        kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX);
> +}
> +
>  int kvm_arch_init(KVMState *s)
>  {
>      QemuOptsList *list = qemu_find_opts("machine");
> @@ -797,6 +871,11 @@ int kvm_arch_init(KVMState *s)
>              }
>          }
>      }
> +
> +    if (kvm_host_cpu_needs_class_init) {
> +        kvm_host_cpu_class_init(object_class_by_name(TYPE_HOST_X86_CPU),
> NULL);
> +    }
kvm_host_cpu_needs_class_init is set to true in kvm_host_cpu_class_init()
so block is nop.
Why kvm_host_cpu_needs_class_init is needed anyway, why not just call
kvm_host_cpu_class_init() here?


> +
>      return 0;
>  }
>  
> @@ -2330,3 +2409,17 @@ int kvm_device_msix_deassign(KVMState *s, uint32_t
> dev_id) return kvm_deassign_irq_internal(s, dev_id, KVM_DEV_IRQ_GUEST_MSIX |
>                                                  KVM_DEV_IRQ_HOST_MSIX);
>  }
> +
> +static const TypeInfo host_x86_cpu_type_info = {
> +    .name = TYPE_HOST_X86_CPU,
> +    .parent = TYPE_X86_CPU,
> +    .instance_init = kvm_host_cpu_initfn,
> +    .class_init = kvm_host_cpu_class_init,
> +};
> +
> +static void kvm_register_types(void)
> +{
> +    type_register_static(&host_x86_cpu_type_info);
> +}
> +
> +type_init(kvm_register_types)

Re: [Qemu-devel] [PATCH qom-cpu-next v3 3/4] target-i386: Slim conversion to X86CPU subclasses

[Andreas Färber]

Am 04.02.2013 12:08, schrieb Igor Mammedov:
> On Sat,  2 Feb 2013 01:37:07 +0100
> Andreas Färber <afaerber@suse.de> wrote:
> 
>> Move x86_def_t definition to header and embed into X86CPUClass.
>> Register types per built-in model definition.
>>
>> Move version initialization from x86_cpudef_setup() to class_init.
>>
>> Inline cpu_x86_register() into the X86CPU initfn.
>> Since instance_init cannot reports errors, drop error handling.
>>
>> Replace cpu_x86_find_by_name() with x86_cpu_class_by_name().
>> Move KVM host vendor override from cpu_x86_find_by_name() to the initfn.
>>
>> Register host-{i386,x86_64}-cpu type from KVM code to avoid #ifdefs.
>> Make kvm_cpu_fill_host() a class_init and inline cpu_x86_fill_model_id().
>>
>> Let kvm_check_features_against_host() obtain host-{i386,86_64}-cpu for
>> comparison.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>>  target-i386/cpu-qom.h |   24 ++++
>>  target-i386/cpu.c     |  324
>> +++++++++++++++++-------------------------------- target-i386/cpu.h
>> |    2 - target-i386/kvm.c     |   93 ++++++++++++++
>>  4 Dateien geändert, 228 Zeilen hinzugefügt(+), 215 Zeilen entfernt(-)
>>
>> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
>> index 48e6b54..80bf72d 100644
>> --- a/target-i386/cpu-qom.h
>> +++ b/target-i386/cpu-qom.h
>> @@ -30,6 +30,27 @@
>>  #define TYPE_X86_CPU "i386-cpu"
>>  #endif
>>  
>> +#define TYPE_HOST_X86_CPU "host-" TYPE_X86_CPU
>> +
>> +typedef struct x86_def_t {
>> +    const char *name;
>> +    uint32_t level;
>> +    /* vendor is zero-terminated, 12 character ASCII string */
>> +    char vendor[CPUID_VENDOR_SZ + 1];
>> +    int family;
>> +    int model;
>> +    int stepping;
>> +    uint32_t features, ext_features, ext2_features, ext3_features;
>> +    uint32_t kvm_features, svm_features;
>> +    uint32_t xlevel;
>> +    char model_id[48];
>> +    /* 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;
>> +
>>  #define X86_CPU_CLASS(klass) \
>>      OBJECT_CLASS_CHECK(X86CPUClass, (klass), TYPE_X86_CPU)
>>  #define X86_CPU(obj) \
>> @@ -41,6 +62,7 @@
>>   * X86CPUClass:
>>   * @parent_realize: The parent class' realize handler.
>>   * @parent_reset: The parent class' reset handler.
>> + * @info: Model-specific data.
>>   *
>>   * An x86 CPU model or family.
>>   */
>> @@ -51,6 +73,8 @@ typedef struct X86CPUClass {
>>  
>>      DeviceRealize parent_realize;
>>      void (*parent_reset)(CPUState *cpu);
>> +
>> +    x86_def_t info;
> Is it necessary to embed it here, could pointer to corresponding static array
> be used here?
> That way one could avoid extra memcpy in class_init().

That would require an allocation for -cpu host, which is undesired.
x86_def_t is supposed to die, forgot to add a TODO comment like on ppc.
Patches to do that are being being polished on qom-cpu-ppc.

>>  } X86CPUClass;
>>  
>>  /**
>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>> index ee2fd6b..6c95740 100644
>> --- a/target-i386/cpu.c
>> +++ b/target-i386/cpu.c
>> @@ -346,25 +346,6 @@ static void add_flagname_to_bitmaps(const char
>> *flagname, }
>>  }
>>  
>> -typedef struct x86_def_t {
>> -    const char *name;
>> -    uint32_t level;
>> -    /* vendor is zero-terminated, 12 character ASCII string */
>> -    char vendor[CPUID_VENDOR_SZ + 1];
>> -    int family;
>> -    int model;
>> -    int stepping;
>> -    uint32_t features, ext_features, ext2_features, ext3_features;
>> -    uint32_t kvm_features, svm_features;
>> -    uint32_t xlevel;
>> -    char model_id[48];
>> -    /* 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;
>> -
>>  #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
>>  #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
>>            CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC)
>> @@ -868,86 +849,6 @@ static x86_def_t builtin_x86_defs[] = {
>>      },
>>  };
>>  
>> -#ifdef CONFIG_KVM
>> -static int cpu_x86_fill_model_id(char *str)
>> -{
>> -    uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
>> -    int i;
>> -
>> -    for (i = 0; i < 3; i++) {
>> -        host_cpuid(0x80000002 + i, 0, &eax, &ebx, &ecx, &edx);
>> -        memcpy(str + i * 16 +  0, &eax, 4);
>> -        memcpy(str + i * 16 +  4, &ebx, 4);
>> -        memcpy(str + i * 16 +  8, &ecx, 4);
>> -        memcpy(str + i * 16 + 12, &edx, 4);
>> -    }
>> -    return 0;
>> -}
>> -#endif
>> -
>> -/* Fill a x86_def_t struct with information about the host CPU, and
>> - * the CPU features supported by the host hardware + host kernel
>> - *
>> - * This function may be called only if KVM is enabled.
>> - */
>> -static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
>> -{
>> -#ifdef CONFIG_KVM
>> -    KVMState *s = kvm_state;
>> -    uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
>> -
>> -    assert(kvm_enabled());
>> -
>> -    x86_cpu_def->name = "host";
>> -    host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
>> -    x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx);
>> -
>> -    host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx);
>> -    x86_cpu_def->family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
>> -    x86_cpu_def->model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >> 12);
>> -    x86_cpu_def->stepping = eax & 0x0F;
>> -
>> -    x86_cpu_def->level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
>> -    x86_cpu_def->features = kvm_arch_get_supported_cpuid(s, 0x1, 0, R_EDX);
>> -    x86_cpu_def->ext_features = kvm_arch_get_supported_cpuid(s, 0x1, 0,
>> R_ECX); -
>> -    if (x86_cpu_def->level >= 7) {
>> -        x86_cpu_def->cpuid_7_0_ebx_features =
>> -                    kvm_arch_get_supported_cpuid(s, 0x7, 0, R_EBX);
>> -    } else {
>> -        x86_cpu_def->cpuid_7_0_ebx_features = 0;
>> -    }
>> -
>> -    x86_cpu_def->xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0,
>> R_EAX);
>> -    x86_cpu_def->ext2_features =
>> -                kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX);
>> -    x86_cpu_def->ext3_features =
>> -                kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_ECX);
>> -
>> -    cpu_x86_fill_model_id(x86_cpu_def->model_id);
>> -
>> -    /* Call Centaur's CPUID instruction. */
>> -    if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) {
>> -        host_cpuid(0xC0000000, 0, &eax, &ebx, &ecx, &edx);
>> -        eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
>> -        if (eax >= 0xC0000001) {
>> -            /* Support VIA max extended level */
>> -            x86_cpu_def->xlevel2 = eax;
>> -            host_cpuid(0xC0000001, 0, &eax, &ebx, &ecx, &edx);
>> -            x86_cpu_def->ext4_features =
>> -                    kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
>> -        }
>> -    }
>> -
>> -    /* Other KVM-specific feature fields: */
>> -    x86_cpu_def->svm_features =
>> -        kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX);
>> -    x86_cpu_def->kvm_features =
>> -        kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX);
>> -
>> -#endif /* CONFIG_KVM */
>> -}
>> -
>>  static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
>>  {
>>      int i;
>> @@ -975,31 +876,31 @@ static int unavailable_host_feature(FeatureWordInfo
>> *f, uint32_t mask) static int kvm_check_features_against_host(X86CPU *cpu)
>>  {
>>      CPUX86State *env = &cpu->env;
>> -    x86_def_t host_def;
>> +    ObjectClass *host_oc = object_class_by_name(TYPE_HOST_X86_CPU);
>> +    X86CPUClass *host_xcc = X86_CPU_CLASS(host_oc);
>>      uint32_t mask;
>>      int rv, i;
>>      struct model_features_t ft[] = {
>> -        {&env->cpuid_features, &host_def.features,
>> +        {&env->cpuid_features, &host_xcc->info.features,
>>              FEAT_1_EDX },
>> -        {&env->cpuid_ext_features, &host_def.ext_features,
>> +        {&env->cpuid_ext_features, &host_xcc->info.ext_features,
>>              FEAT_1_ECX },
>> -        {&env->cpuid_ext2_features, &host_def.ext2_features,
>> +        {&env->cpuid_ext2_features, &host_xcc->info.ext2_features,
>>              FEAT_8000_0001_EDX },
>> -        {&env->cpuid_ext3_features, &host_def.ext3_features,
>> +        {&env->cpuid_ext3_features, &host_xcc->info.ext3_features,
>>              FEAT_8000_0001_ECX },
>> -        {&env->cpuid_ext4_features, &host_def.ext4_features,
>> +        {&env->cpuid_ext4_features, &host_xcc->info.ext4_features,
>>              FEAT_C000_0001_EDX },
>> -        {&env->cpuid_7_0_ebx_features, &host_def.cpuid_7_0_ebx_features,
>> +        {&env->cpuid_7_0_ebx_features,
>> &host_xcc->info.cpuid_7_0_ebx_features, FEAT_7_0_EBX },
>> -        {&env->cpuid_svm_features, &host_def.svm_features,
>> +        {&env->cpuid_svm_features, &host_xcc->info.svm_features,
>>              FEAT_SVM },
>> -        {&env->cpuid_kvm_features, &host_def.kvm_features,
>> +        {&env->cpuid_kvm_features, &host_xcc->info.kvm_features,
>>              FEAT_KVM },
>>      };
>>  
>>      assert(kvm_enabled());
>>  
>> -    kvm_cpu_fill_host(&host_def);
>>      for (rv = 0, i = 0; i < ARRAY_SIZE(ft); ++i) {
>>          FeatureWord w = ft[i].feat_word;
>>          FeatureWordInfo *wi = &feature_word_info[w];
>> @@ -1261,40 +1162,30 @@ static void x86_cpuid_set_tsc_freq(Object *obj,
>> Visitor *v, void *opaque, cpu->env.tsc_khz = value / 1000;
>>  }
>>  
>> -static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
>> +static ObjectClass *x86_cpu_class_by_name(const char *name)
>>  {
>> -    x86_def_t *def;
>> -    int i;
>> +    ObjectClass *oc;
>> +    char *typename;
>>  
>>      if (name == NULL) {
>> -        return -1;
>> -    }
>> -    if (kvm_enabled() && strcmp(name, "host") == 0) {
>> -        kvm_cpu_fill_host(x86_cpu_def);
>> -        return 0;
>> +        return NULL;
>>      }
>>  
>> -    for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
>> -        def = &builtin_x86_defs[i];
>> -        if (strcmp(name, def->name) == 0) {
>> -            memcpy(x86_cpu_def, def, sizeof(*def));
>> -            /* sysenter isn't supported in 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);
>> -                x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx,
>> ecx);
>> -            }
>> -            return 0;
>> +    if (strcmp(name, "host") == 0) {
>> +        if (kvm_enabled()) {
>> +            return object_class_by_name(TYPE_HOST_X86_CPU);
>>          }
>> +        return NULL;
>>      }
> block is not necessary, the following block could yield the same result.

No, it doesn't in the !kvm_enabled() case. I tripped over this when I
left the kvm_enabled() check where it was before, falling back to the
below lookup, leading to -cpu host being instantiated for TCG and
originally not asserting (that's why I added the -cpu host initfn).

Symptoms were x86_64 openSUSE .iso claiming no support for 486(?)
processors.

>>  
>> -    return -1;
>> +    typename = g_strdup_printf("%s-" TYPE_X86_CPU, name);
>> +    oc = object_class_by_name(typename);
>> +    g_free(typename);
>> +    if (oc != NULL && (!object_class_dynamic_cast(oc, TYPE_X86_CPU) ||
>> +                       object_class_is_abstract(oc))) {
>> +        oc = NULL;
>> +    }
>> +    return oc;
>>  }
>>  
>>  /* Parse "+feature,-feature,feature=foo" CPU feature string
>> @@ -1516,60 +1407,13 @@ static void filter_features_for_kvm(X86CPU *cpu)
>>  }
>>  #endif
>>  
>> -static int cpu_x86_register(X86CPU *cpu, const char *name)
>> -{
>> -    CPUX86State *env = &cpu->env;
>> -    x86_def_t def1, *def = &def1;
>> -    Error *error = NULL;
>> -
>> -    memset(def, 0, sizeof(*def));
>> -
>> -    if (cpu_x86_find_by_name(def, name) < 0) {
>> -        error_setg(&error, "Unable to find CPU definition: %s", name);
>> -        goto out;
>> -    }
>> -
>> -    if (kvm_enabled()) {
>> -        def->kvm_features |= kvm_default_features;
>> -    }
>> -    def->ext_features |= CPUID_EXT_HYPERVISOR;
>> -
>> -    object_property_set_str(OBJECT(cpu), def->vendor, "vendor", &error);
>> -    object_property_set_int(OBJECT(cpu), def->level, "level", &error);
>> -    object_property_set_int(OBJECT(cpu), def->family, "family", &error);
>> -    object_property_set_int(OBJECT(cpu), def->model, "model", &error);
>> -    object_property_set_int(OBJECT(cpu), def->stepping, "stepping",
>> &error);
>> -    env->cpuid_features = def->features;
>> -    env->cpuid_ext_features = def->ext_features;
>> -    env->cpuid_ext2_features = def->ext2_features;
>> -    env->cpuid_ext3_features = def->ext3_features;
>> -    object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", &error);
>> -    env->cpuid_kvm_features = def->kvm_features;
>> -    env->cpuid_svm_features = def->svm_features;
>> -    env->cpuid_ext4_features = def->ext4_features;
>> -    env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features;
>> -    env->cpuid_xlevel2 = def->xlevel2;
>> -
>> -    object_property_set_str(OBJECT(cpu), def->model_id, "model-id",
>> &error);
>> -    if (error) {
>> -        goto out;
>> -    }
>> -
>> -out:
>> -    if (error) {
>> -        fprintf(stderr, "%s\n", error_get_pretty(error));
>> -        error_free(error);
>> -        return -1;
>> -    }
>> -    return 0;
>> -}
>> -
>>  X86CPU *cpu_x86_init(const char *cpu_model)
>>  {
>>      X86CPU *cpu = NULL;
>>      CPUX86State *env;
>>      gchar **model_pieces;
>>      char *name, *features;
>> +    ObjectClass *oc;
>>      Error *error = NULL;
>>  
>>      model_pieces = g_strsplit(cpu_model, ",", 2);
>> @@ -1580,13 +1424,13 @@ X86CPU *cpu_x86_init(const char *cpu_model)
>>      name = model_pieces[0];
>>      features = model_pieces[1];
>>  
>> -    cpu = X86_CPU(object_new(TYPE_X86_CPU));
>> -    env = &cpu->env;
>> -    env->cpu_model_str = cpu_model;
>> -
>> -    if (cpu_x86_register(cpu, name) < 0) {
>> +    oc = x86_cpu_class_by_name(name);
>> +    if (oc == NULL) {
> make sure error is reported when cpu is not found:
>            error_setg(&error, "Can't find CPU: %s", name);

Why? The callers do that when NULL is returned.

>>          goto error;
>>      }
>> +    cpu = X86_CPU(object_new(object_class_get_name(oc)));
>> +    env = &cpu->env;
>> +    env->cpu_model_str = cpu_model;
>>  
>>      cpu_x86_parse_featurestr(cpu, features, &error);
>>      if (error) {
>> @@ -1621,30 +1465,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];
>> -
>> -        /* 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;
>> -            }
>> -        }
>> -    }
>> -}
>> -
>>  static void get_cpuid_vendor(CPUX86State *env, uint32_t *ebx,
>>                               uint32_t *ecx, uint32_t *edx)
>>  {
>> @@ -2198,6 +2018,8 @@ static void x86_cpu_initfn(Object *obj)
>>      CPUState *cs = CPU(obj);
>>      X86CPU *cpu = X86_CPU(obj);
>>      CPUX86State *env = &cpu->env;
>> +    X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
>> +    const x86_def_t *def = &xcc->info;
>>      static int inited;
>>  
>>      cpu_exec_init(env);
>> @@ -2227,6 +2049,41 @@ static void x86_cpu_initfn(Object *obj)
>>                          x86_cpuid_get_tsc_freq,
>>                          x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
>>  
>> +    /* sysenter isn't supported in 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()) {
>> +        host_cpuid(0, 0, NULL, &env->cpuid_vendor1, &env->cpuid_vendor2,
>> +                   &env->cpuid_vendor3);
> This is not per instance specific, this override would be better done to
> sub-classes in kvm_arch_init().

Hmm... I think the issue I addresses this way was that kvm.c didn't have
access to your static vendor_words2str() helper. Writing the words into
the word fields is more direct than converting them to a string and
writing them back into the words.

> As bonus we would get actual 'vendor' when
> doing class introspection provided it's done after kvm_init() is called.

Doing that would mean that we force class_init of every CPU class to run
every time we start with KVM enabled. Is there any practical downside to
doing it at instance_init time? Properties are set afterwards, so it
doesn't override anything. Where would we want to inspect an X86CPUClass
to have an AMD model say Intel or vice versa? -cpu ? doesn't look at it
today, nor does QMP.

>> +    } else {
>> +        object_property_set_str(OBJECT(cpu), def->vendor, "vendor", NULL);
>> +    }
>> +
>> +    object_property_set_int(OBJECT(cpu), def->level, "level", NULL);
>> +    object_property_set_int(OBJECT(cpu), def->family, "family", NULL);
>> +    object_property_set_int(OBJECT(cpu), def->model, "model", NULL);
>> +    object_property_set_int(OBJECT(cpu), def->stepping, "stepping", NULL);
>> +    env->cpuid_features = def->features;
>> +    env->cpuid_ext_features = def->ext_features;
>> +    env->cpuid_ext_features |= CPUID_EXT_HYPERVISOR;
>> +    env->cpuid_ext2_features = def->ext2_features;
>> +    env->cpuid_ext3_features = def->ext3_features;
>> +    object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", NULL);
>> +    env->cpuid_kvm_features = def->kvm_features;
>> +    if (kvm_enabled()) {
>> +        env->cpuid_kvm_features |= kvm_default_features;
>> +    }
>> +    env->cpuid_svm_features = def->svm_features;
>> +    env->cpuid_ext4_features = def->ext4_features;
>> +    env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features;
>> +    env->cpuid_xlevel2 = def->xlevel2;
>> +
>> +    object_property_set_str(OBJECT(cpu), def->model_id, "model-id", NULL);
> All this feature initialization in initfn() is not compatible with
> static/global properties because they are set in device_initfn(). But I can
> remove properties/features from here as they are converted to static
> properties and incrementally move their defaults initialization into
> new x86_cpu_def_class_init().

Would it help to call the old registration function from here? Or what
exactly would you need on top?

>> +
>>      env->cpuid_apic_id = x86_cpu_apic_id_from_index(cs->cpu_index);
>>  
>>      /* init various static tables used in TCG mode */
>> @@ -2239,6 +2096,27 @@ static void x86_cpu_initfn(Object *obj)
>>      }
>>  }
>>  
>> +static void x86_cpu_def_class_init(ObjectClass *oc, void *data)
>> +{
>> +    X86CPUClass *xcc = X86_CPU_CLASS(oc);
>> +    x86_def_t *def = data;
>> +    int i;
>> +    static const char *versioned_models[] = { "qemu32", "qemu64",
>> "athlon" }; +
>> +    memcpy(&xcc->info, def, sizeof(x86_def_t));
> perhaps sizeof(xcc->info) would be better?

OK.

>> +
>> +    /* Look for specific models that have the QEMU version in .model_id */
>> +    for (i = 0; i < ARRAY_SIZE(versioned_models); i++) {
>> +        if (strcmp(versioned_models[i], def->name) == 0) {
>> +            pstrcpy(xcc->info.model_id, sizeof(xcc->info.model_id),
>> +                    "QEMU Virtual CPU version ");
>> +            pstrcat(xcc->info.model_id, sizeof(xcc->info.model_id),
>> +                    qemu_get_version());
>> +            break;
>> +        }
>> +    }
>> +}
>> +
>>  static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>>  {
>>      X86CPUClass *xcc = X86_CPU_CLASS(oc);
>> @@ -2250,6 +2128,21 @@ static void x86_cpu_common_class_init(ObjectClass
>> *oc, void *data) 
>>      xcc->parent_reset = cc->reset;
>>      cc->reset = x86_cpu_reset;
>> +
>> +    cc->class_by_name = x86_cpu_class_by_name;
>> +}
>> +
>> +static void x86_register_cpu_type(const x86_def_t *def)
>> +{
>> +    TypeInfo type_info = {
>> +        .parent = TYPE_X86_CPU,
>> +        .class_init = x86_cpu_def_class_init,
>> +        .class_data = (void *)def,
>> +    };
>> +
>> +    type_info.name = g_strdup_printf("%s-" TYPE_X86_CPU, def->name);
>> +    type_register(&type_info);
>> +    g_free((void *)type_info.name);
>>  }
>>  
>>  static const TypeInfo x86_cpu_type_info = {
>> @@ -2257,14 +2150,19 @@ 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,
>>  };
>>  
>>  static void x86_cpu_register_types(void)
>>  {
>> +    int i;
>> +
>>      type_register_static(&x86_cpu_type_info);
>> +    for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
>> +        x86_register_cpu_type(&builtin_x86_defs[i]);
>> +    }
>>  }
>>  
>>  type_init(x86_cpu_register_types)
>> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
>> index 7577e4f..2aef7b7 100644
>> --- a/target-i386/cpu.h
>> +++ b/target-i386/cpu.h
>> @@ -887,7 +887,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);
>> @@ -1079,7 +1078,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
>>  
>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>> index 9ebf181..dbfa670 100644
>> --- a/target-i386/kvm.c
>> +++ b/target-i386/kvm.c
>> @@ -735,6 +735,80 @@ static int kvm_get_supported_msrs(KVMState *s)
>>      return ret;
>>  }
>>  
>> +static void kvm_host_cpu_initfn(Object *obj)
>> +{
>> +    assert(kvm_enabled());
>> +}
>> +
>> +static bool kvm_host_cpu_needs_class_init;
>> +
>> +static void kvm_host_cpu_class_init(ObjectClass *oc, void *data)
>> +{
>> +    X86CPUClass *xcc = X86_CPU_CLASS(oc);
>> +    x86_def_t *x86_cpu_def = &xcc->info;
>> +    KVMState *s = kvm_state;
>> +    uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
>> +    int i;
>> +
>> +    if (!kvm_enabled()) {
>> +        kvm_host_cpu_needs_class_init = true;
>> +        return;
>> +    }
>> +
>> +    xcc->info.name = "host";
>> +
>> +    /* Vendor will be set in initfn if kvm_enabled() */
>> +
>> +    host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx);
>> +    x86_cpu_def->family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
>> +    x86_cpu_def->model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >> 12);
>> +    x86_cpu_def->stepping = eax & 0x0F;
>> +
>> +    x86_cpu_def->level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
>> +    x86_cpu_def->features = kvm_arch_get_supported_cpuid(s, 0x1, 0, R_EDX);
>> +    x86_cpu_def->ext_features = kvm_arch_get_supported_cpuid(s, 0x1, 0,
>> R_ECX); +
>> +    if (x86_cpu_def->level >= 7) {
>> +        x86_cpu_def->cpuid_7_0_ebx_features =
>> +                    kvm_arch_get_supported_cpuid(s, 0x7, 0, R_EBX);
>> +    } else {
>> +        x86_cpu_def->cpuid_7_0_ebx_features = 0;
>> +    }
>> +
>> +    x86_cpu_def->xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0,
>> R_EAX);
>> +    x86_cpu_def->ext2_features =
>> +                kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX);
>> +    x86_cpu_def->ext3_features =
>> +                kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_ECX);
>> +
>> +    for (i = 0; i < 3; i++) {
>> +        host_cpuid(0x80000002 + i, 0, &eax, &ebx, &ecx, &edx);
>> +        memcpy(xcc->info.model_id + i * 16 +  0, &eax, 4);
>> +        memcpy(xcc->info.model_id + i * 16 +  4, &ebx, 4);
>> +        memcpy(xcc->info.model_id + i * 16 +  8, &ecx, 4);
>> +        memcpy(xcc->info.model_id + i * 16 + 12, &edx, 4);
>> +    }
>> +
>> +    /* Call Centaur's CPUID instruction. */
>> +    if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) {
>> +        host_cpuid(0xC0000000, 0, &eax, &ebx, &ecx, &edx);
>> +        eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
>> +        if (eax >= 0xC0000001) {
>> +            /* Support VIA max extended level */
>> +            x86_cpu_def->xlevel2 = eax;
>> +            host_cpuid(0xC0000001, 0, &eax, &ebx, &ecx, &edx);
>> +            x86_cpu_def->ext4_features =
>> +                    kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
>> +        }
>> +    }
>> +
>> +    /* Other KVM-specific feature fields: */
>> +    x86_cpu_def->svm_features =
>> +        kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX);
>> +    x86_cpu_def->kvm_features =
>> +        kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX);
>> +}
>> +
>>  int kvm_arch_init(KVMState *s)
>>  {
>>      QemuOptsList *list = qemu_find_opts("machine");
>> @@ -797,6 +871,11 @@ int kvm_arch_init(KVMState *s)
>>              }
>>          }
>>      }
>> +
>> +    if (kvm_host_cpu_needs_class_init) {
>> +        kvm_host_cpu_class_init(object_class_by_name(TYPE_HOST_X86_CPU),
>> NULL);
>> +    }
> kvm_host_cpu_needs_class_init is set to true in kvm_host_cpu_class_init()
> so block is nop.

Nope. It's a question of initialization ordering:

> Why kvm_host_cpu_needs_class_init is needed anyway, why not just call
> kvm_host_cpu_class_init() here?

i) class_init may be called before kvm_enabled() evaluates to true, such
as for -cpu ?, so we need to handle that case gracefully.

ii) The regular case is that the class_init is run in response to
object_new() from pc.c, in which case kvm_enabled() evaluates to true
and the needs_class_init remains false.

Thus, both cases are used in practice.

Regards,
Andreas

>> +
>>      return 0;
>>  }
>>  
>> @@ -2330,3 +2409,17 @@ int kvm_device_msix_deassign(KVMState *s, uint32_t
>> dev_id) return kvm_deassign_irq_internal(s, dev_id, KVM_DEV_IRQ_GUEST_MSIX |
>>                                                  KVM_DEV_IRQ_HOST_MSIX);
>>  }
>> +
>> +static const TypeInfo host_x86_cpu_type_info = {
>> +    .name = TYPE_HOST_X86_CPU,
>> +    .parent = TYPE_X86_CPU,
>> +    .instance_init = kvm_host_cpu_initfn,
>> +    .class_init = kvm_host_cpu_class_init,
>> +};
>> +
>> +static void kvm_register_types(void)
>> +{
>> +    type_register_static(&host_x86_cpu_type_info);
>> +}
>> +
>> +type_init(kvm_register_types)

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH qom-cpu-next v3 3/4] target-i386: Slim conversion to X86CPU subclasses

[Igor Mammedov]

On Mon, 04 Feb 2013 13:52:32 +0100
Andreas Färber <afaerber@suse.de> wrote:

> Am 04.02.2013 12:08, schrieb Igor Mammedov:
> > On Sat,  2 Feb 2013 01:37:07 +0100
> > Andreas Färber <afaerber@suse.de> wrote:
> > 
> >> Move x86_def_t definition to header and embed into X86CPUClass.
> >> Register types per built-in model definition.
> >>
> >> Move version initialization from x86_cpudef_setup() to class_init.
> >>
> >> Inline cpu_x86_register() into the X86CPU initfn.
> >> Since instance_init cannot reports errors, drop error handling.
> >>
> >> Replace cpu_x86_find_by_name() with x86_cpu_class_by_name().
> >> Move KVM host vendor override from cpu_x86_find_by_name() to the initfn.
> >>
> >> Register host-{i386,x86_64}-cpu type from KVM code to avoid #ifdefs.
> >> Make kvm_cpu_fill_host() a class_init and inline cpu_x86_fill_model_id().
> >>
> >> Let kvm_check_features_against_host() obtain host-{i386,86_64}-cpu for
> >> comparison.
> >>
> >> Signed-off-by: Andreas Färber <afaerber@suse.de>
> >> ---
> >>  target-i386/cpu-qom.h |   24 ++++
> >>  target-i386/cpu.c     |  324
> >> +++++++++++++++++-------------------------------- target-i386/cpu.h
> >> |    2 - target-i386/kvm.c     |   93 ++++++++++++++
> >>  4 Dateien geändert, 228 Zeilen hinzugefügt(+), 215 Zeilen entfernt(-)
> >>
> >> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> >> index 48e6b54..80bf72d 100644
> >> --- a/target-i386/cpu-qom.h
> >> +++ b/target-i386/cpu-qom.h
[...]

> >>   * An x86 CPU model or family.
> >>   */
> >> @@ -51,6 +73,8 @@ typedef struct X86CPUClass {
> >>  
> >>      DeviceRealize parent_realize;
> >>      void (*parent_reset)(CPUState *cpu);
> >> +
> >> +    x86_def_t info;
> > Is it necessary to embed it here, could pointer to corresponding static
> > array be used here?
> > That way one could avoid extra memcpy in class_init().
> 
> That would require an allocation for -cpu host, which is undesired.
not necessary, it could be local static x86_def_t in target-i386/kvm.c in
host class_init(). Any way change is cosmetic and I do not insist on it.

> x86_def_t is supposed to die, forgot to add a TODO comment like on ppc.
> Patches to do that are being being polished on qom-cpu-ppc.
+1

> 
> >>  } X86CPUClass;
> >>  
> >>  /**
[...]

> >> *name) +static ObjectClass *x86_cpu_class_by_name(const char *name)
> >>  {
> >> -    x86_def_t *def;
> >> -    int i;
> >> +    ObjectClass *oc;
> >> +    char *typename;
> >>  
> >>      if (name == NULL) {
> >> -        return -1;
> >> -    }
> >> -    if (kvm_enabled() && strcmp(name, "host") == 0) {
> >> -        kvm_cpu_fill_host(x86_cpu_def);
> >> -        return 0;
> >> +        return NULL;
> >>      }
> >>  
> >> -    for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
> >> -        def = &builtin_x86_defs[i];
> >> -        if (strcmp(name, def->name) == 0) {
> >> -            memcpy(x86_cpu_def, def, sizeof(*def));
> >> -            /* sysenter isn't supported in 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);
> >> -                x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx,
> >> ecx);
> >> -            }
> >> -            return 0;
> >> +    if (strcmp(name, "host") == 0) {
> >> +        if (kvm_enabled()) {
> >> +            return object_class_by_name(TYPE_HOST_X86_CPU);
> >>          }
> >> +        return NULL;
> >>      }
> > block is not necessary, the following block could yield the same result.
> 
> No, it doesn't in the !kvm_enabled() case. I tripped over this when I
> left the kvm_enabled() check where it was before, falling back to the
> below lookup, leading to -cpu host being instantiated for TCG and
> originally not asserting (that's why I added the -cpu host initfn).
Maybe TYPE_HOST_X86_CPU shouldn't be registered at all if kvm_enabled() == 0.

> 
> Symptoms were x86_64 openSUSE .iso claiming no support for 486(?)
> processors.
> 
> >>  
> >> -    return -1;
> >> +    typename = g_strdup_printf("%s-" TYPE_X86_CPU, name);
> >> +    oc = object_class_by_name(typename);
> >> +    g_free(typename);
> >> +    if (oc != NULL && (!object_class_dynamic_cast(oc, TYPE_X86_CPU) ||
> >> +                       object_class_is_abstract(oc))) {
> >> +        oc = NULL;
> >> +    }
> >> +    return oc;
> >>  }
> >>  
[...]

> >> @@ -1580,13 +1424,13 @@ X86CPU *cpu_x86_init(const char *cpu_model)
> >>      name = model_pieces[0];
> >>      features = model_pieces[1];
> >>  
> >> -    cpu = X86_CPU(object_new(TYPE_X86_CPU));
> >> -    env = &cpu->env;
> >> -    env->cpu_model_str = cpu_model;
> >> -
> >> -    if (cpu_x86_register(cpu, name) < 0) {
> >> +    oc = x86_cpu_class_by_name(name);
> >> +    if (oc == NULL) {
> > make sure error is reported when cpu is not found:
> >            error_setg(&error, "Can't find CPU: %s", name);
> 
> Why? The callers do that when NULL is returned.
The callers do not exactly know why NULL is returned, wouldn't it better
to specify error message at the point of origin.

> 
> >>          goto error;
> >>      }
> >> +    cpu = X86_CPU(object_new(object_class_get_name(oc)));
> >> +    env = &cpu->env;
> >> +    env->cpu_model_str = cpu_model;
> >>  
> >>      cpu_x86_parse_featurestr(cpu, features, &error);
> >>      if (error) {
[...]

> >> @@ -2198,6 +2018,8 @@ static void x86_cpu_initfn(Object *obj)
> >>      CPUState *cs = CPU(obj);
> >>      X86CPU *cpu = X86_CPU(obj);
> >>      CPUX86State *env = &cpu->env;
> >> +    X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
> >> +    const x86_def_t *def = &xcc->info;
> >>      static int inited;
> >>  
> >>      cpu_exec_init(env);
> >> @@ -2227,6 +2049,41 @@ static void x86_cpu_initfn(Object *obj)
> >>                          x86_cpuid_get_tsc_freq,
> >>                          x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
> >>  
> >> +    /* sysenter isn't supported in 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()) {
> >> +        host_cpuid(0, 0, NULL, &env->cpuid_vendor1, &env->cpuid_vendor2,
> >> +                   &env->cpuid_vendor3);
> > This is not per instance specific, this override would be better done to
> > sub-classes in kvm_arch_init().
> 
> Hmm... I think the issue I addresses this way was that kvm.c didn't have
> access to your static vendor_words2str() helper. Writing the words into
> the word fields is more direct than converting them to a string and
> writing them back into the words.
you have these correct vendor words ready after you called
kvm_host_cpu_class_init() in kvm_arch_init(), so just direct copy from host
class would do.

> 
> > As bonus we would get actual 'vendor' when
> > doing class introspection provided it's done after kvm_init() is called.
> 
> Doing that would mean that we force class_init of every CPU class to run
> every time we start with KVM enabled.
I'd ask if there is any particular downside in calling every class_init()?

> Is there any practical downside to
> doing it at instance_init time? Properties are set afterwards, so it
> doesn't override anything. Where would we want to inspect an X86CPUClass
> to have an AMD model say Intel or vice versa? -cpu ? doesn't look at it
> today, nor does QMP.
Presently nothing changes if we do it in kvm_init() or initfn() time since
none of users displays actual vendor value, but it looks more like a bug. 

Downside of doing it initfn() is that static properties defaults and global
properties are set by device_initfn() before cpu_initfn() is called, so
it renders all that was set via it overridden by a later x86_cpu_initfn().

So if there is no really big drawback in calling every x86 cpu class_init(),
I'd prefer to utilize features provided by static properties, than hacking
class wide vendor override per each cpu instance.

As alternative x86 cpu could have a hook that is called from
x86_cpu_class_init() that does kvm specific overrides and set by
kvm_arch_init(). Then there is no need to initialize all types in
kvm_arch_init(). The problem with it is if class is initialized before
kvm_init(), then override won't be applied ever.

> 
> >> +    } else {
> >> +        object_property_set_str(OBJECT(cpu), def->vendor, "vendor",
> >> NULL);
> >> +    }
> >> +
> >> +    object_property_set_int(OBJECT(cpu), def->level, "level", NULL);
> >> +    object_property_set_int(OBJECT(cpu), def->family, "family", NULL);
> >> +    object_property_set_int(OBJECT(cpu), def->model, "model", NULL);
> >> +    object_property_set_int(OBJECT(cpu), def->stepping, "stepping",
> >> NULL);
> >> +    env->cpuid_features = def->features;
> >> +    env->cpuid_ext_features = def->ext_features;
> >> +    env->cpuid_ext_features |= CPUID_EXT_HYPERVISOR;
> >> +    env->cpuid_ext2_features = def->ext2_features;
> >> +    env->cpuid_ext3_features = def->ext3_features;
> >> +    object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", NULL);
> >> +    env->cpuid_kvm_features = def->kvm_features;
> >> +    if (kvm_enabled()) {
> >> +        env->cpuid_kvm_features |= kvm_default_features;
> >> +    }
> >> +    env->cpuid_svm_features = def->svm_features;
> >> +    env->cpuid_ext4_features = def->ext4_features;
> >> +    env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features;
> >> +    env->cpuid_xlevel2 = def->xlevel2;
> >> +
> >> +    object_property_set_str(OBJECT(cpu), def->model_id, "model-id",
> >> NULL);
> > All this feature initialization in initfn() is not compatible with
> > static/global properties because they are set in device_initfn(). But I
> > can remove properties/features from here as they are converted to static
> > properties and incrementally move their defaults initialization into
> > new x86_cpu_def_class_init().
> 
> Would it help to call the old registration function from here? Or what
> exactly would you need on top?
Nope, see previous answer to why setting defaults here is not desired.
After conversion, defaults will be set via static properties defaults and then
on top of it, global properties will be applied in device_initfn().
Eventually -cpu xxx,foo should set global properties for cpu type and cpu
type name for machine to use. no manual property setting between object_new()
and realize() unless it is instance specific.

[...]
> >> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> >> index 9ebf181..dbfa670 100644
> >> --- a/target-i386/kvm.c
> >> +++ b/target-i386/kvm.c
> >> @@ -735,6 +735,80 @@ static int kvm_get_supported_msrs(KVMState *s)
> >>      return ret;
> >>  }
> >>  
> >> +static void kvm_host_cpu_initfn(Object *obj)
> >> +{
> >> +    assert(kvm_enabled());
> >> +}
> >> +
> >> +static bool kvm_host_cpu_needs_class_init;
> >> +
> >> +static void kvm_host_cpu_class_init(ObjectClass *oc, void *data)
> >> +{
> >> +    X86CPUClass *xcc = X86_CPU_CLASS(oc);
> >> +    x86_def_t *x86_cpu_def = &xcc->info;
> >> +    KVMState *s = kvm_state;
> >> +    uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
> >> +    int i;
> >> +
> >> +    if (!kvm_enabled()) {
> >> +        kvm_host_cpu_needs_class_init = true;
> >> +        return;
> >> +    }
> >> +
> >> +    xcc->info.name = "host";
> >> +
> >> +    /* Vendor will be set in initfn if kvm_enabled() */
> >> +
> >> +    host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx);
> >> +    x86_cpu_def->family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0xFF);
> >> +    x86_cpu_def->model = ((eax >> 4) & 0x0F) | ((eax & 0xF0000) >> 12);
> >> +    x86_cpu_def->stepping = eax & 0x0F;
> >> +
> >> +    x86_cpu_def->level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
> >> +    x86_cpu_def->features = kvm_arch_get_supported_cpuid(s, 0x1, 0,
> >> R_EDX);
> >> +    x86_cpu_def->ext_features = kvm_arch_get_supported_cpuid(s, 0x1, 0,
> >> R_ECX); +
> >> +    if (x86_cpu_def->level >= 7) {
> >> +        x86_cpu_def->cpuid_7_0_ebx_features =
> >> +                    kvm_arch_get_supported_cpuid(s, 0x7, 0, R_EBX);
> >> +    } else {
> >> +        x86_cpu_def->cpuid_7_0_ebx_features = 0;
> >> +    }
> >> +
> >> +    x86_cpu_def->xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0,
> >> R_EAX);
> >> +    x86_cpu_def->ext2_features =
> >> +                kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX);
> >> +    x86_cpu_def->ext3_features =
> >> +                kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_ECX);
> >> +
> >> +    for (i = 0; i < 3; i++) {
> >> +        host_cpuid(0x80000002 + i, 0, &eax, &ebx, &ecx, &edx);
> >> +        memcpy(xcc->info.model_id + i * 16 +  0, &eax, 4);
> >> +        memcpy(xcc->info.model_id + i * 16 +  4, &ebx, 4);
> >> +        memcpy(xcc->info.model_id + i * 16 +  8, &ecx, 4);
> >> +        memcpy(xcc->info.model_id + i * 16 + 12, &edx, 4);
> >> +    }
> >> +
> >> +    /* Call Centaur's CPUID instruction. */
> >> +    if (!strcmp(x86_cpu_def->vendor, CPUID_VENDOR_VIA)) {
> >> +        host_cpuid(0xC0000000, 0, &eax, &ebx, &ecx, &edx);
> >> +        eax = kvm_arch_get_supported_cpuid(s, 0xC0000000, 0, R_EAX);
> >> +        if (eax >= 0xC0000001) {
> >> +            /* Support VIA max extended level */
> >> +            x86_cpu_def->xlevel2 = eax;
> >> +            host_cpuid(0xC0000001, 0, &eax, &ebx, &ecx, &edx);
> >> +            x86_cpu_def->ext4_features =
> >> +                    kvm_arch_get_supported_cpuid(s, 0xC0000001, 0,
> >> R_EDX);
> >> +        }
> >> +    }
> >> +
> >> +    /* Other KVM-specific feature fields: */
> >> +    x86_cpu_def->svm_features =
> >> +        kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX);
> >> +    x86_cpu_def->kvm_features =
> >> +        kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX);
> >> +}
> >> +
> >>  int kvm_arch_init(KVMState *s)
> >>  {
> >>      QemuOptsList *list = qemu_find_opts("machine");
> >> @@ -797,6 +871,11 @@ int kvm_arch_init(KVMState *s)
> >>              }
> >>          }
> >>      }
> >> +
> >> +    if (kvm_host_cpu_needs_class_init) {
> >> +        kvm_host_cpu_class_init(object_class_by_name(TYPE_HOST_X86_CPU),
> >> NULL);
> >> +    }
> > kvm_host_cpu_needs_class_init is set to true in kvm_host_cpu_class_init()
> > so block is nop.
> 
> Nope. It's a question of initialization ordering:
Yep, I see now, type_register_static(&host_x86_cpu_type_info) creates
fake(uninitialized) host class and then it is initialized when
kvm_arch_init() is called.

> 
> > Why kvm_host_cpu_needs_class_init is needed anyway, why not just call
> > kvm_host_cpu_class_init() here?
> 
> i) class_init may be called before kvm_enabled() evaluates to true, such
> as for -cpu ?, so we need to handle that case gracefully.
We do not print host cpu type at -cpu ? now, it's implied that host cpu is
available if started with --enable-kvm.

> 
> ii) The regular case is that the class_init is run in response to
> object_new() from pc.c, in which case kvm_enabled() evaluates to true
> and the needs_class_init remains false.
could we register host type in kvm_arch_init() time, and avoid calling
kvm_host_cpu_class_init() twice?
It also would allow to remove host look-up hack in x86_cpu_class_by_name().

> 
> Thus, both cases are used in practice.
> 
> Regards,
> Andreas
> 
> >> +
> >>      return 0;
> >>  }
> >>  
> >> @@ -2330,3 +2409,17 @@ int kvm_device_msix_deassign(KVMState *s, uint32_t
> >> dev_id) return kvm_deassign_irq_internal(s, dev_id,
> >> KVM_DEV_IRQ_GUEST_MSIX | KVM_DEV_IRQ_HOST_MSIX);
> >>  }
> >> +
> >> +static const TypeInfo host_x86_cpu_type_info = {
> >> +    .name = TYPE_HOST_X86_CPU,
> >> +    .parent = TYPE_X86_CPU,
> >> +    .instance_init = kvm_host_cpu_initfn,
> >> +    .class_init = kvm_host_cpu_class_init,
> >> +};
> >> +
> >> +static void kvm_register_types(void)
> >> +{
> >> +    type_register_static(&host_x86_cpu_type_info);
> >> +}
> >> +
> >> +type_init(kvm_register_types)
> 

Re: [PATCH qom-cpu-next v3 3/4] target-i386: Slim conversion to X86CPU subclasses

[Igor Mammedov]

On Mon, 4 Feb 2013 17:05:01 +0100
Igor Mammedov <imammedo@redhat.com> wrote:

> On Mon, 04 Feb 2013 13:52:32 +0100
> Andreas Färber <afaerber@suse.de> wrote:
> 
> > Am 04.02.2013 12:08, schrieb Igor Mammedov:
> > > On Sat,  2 Feb 2013 01:37:07 +0100
> > > Andreas Färber <afaerber@suse.de> wrote:
> > > 
[...]
> 
> > >> @@ -2198,6 +2018,8 @@ static void x86_cpu_initfn(Object *obj)
> > >>      CPUState *cs = CPU(obj);
> > >>      X86CPU *cpu = X86_CPU(obj);
> > >>      CPUX86State *env = &cpu->env;
> > >> +    X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
> > >> +    const x86_def_t *def = &xcc->info;
> > >>      static int inited;
> > >>  
> > >>      cpu_exec_init(env);
> > >> @@ -2227,6 +2049,41 @@ static void x86_cpu_initfn(Object *obj)
> > >>                          x86_cpuid_get_tsc_freq,
> > >>                          x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
> > >>  
> > >> +    /* sysenter isn't supported in 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()) {
> > >> +        host_cpuid(0, 0, NULL, &env->cpuid_vendor1, &env->cpuid_vendor2,
> > >> +                   &env->cpuid_vendor3);
> > > This is not per instance specific, this override would be better done to
> > > sub-classes in kvm_arch_init().
> > 
> > Hmm... I think the issue I addresses this way was that kvm.c didn't have
> > access to your static vendor_words2str() helper. Writing the words into
> > the word fields is more direct than converting them to a string and
> > writing them back into the words.
> you have these correct vendor words ready after you called
> kvm_host_cpu_class_init() in kvm_arch_init(), so just direct copy from host
> class would do.
I was talking nonsense here.
Why not to make vendor_words2str() helper not static and use it in kvm.c
instead of doing partial init of host class and then reuse the value you got
to override vendor in other classes?

[...]

-- 
Regards,
  Igor

[Qemu-devel] [PATCH qom-cpu-next v3 4/4] Remove cpudef_setup() hooks

[Andreas Färber]

QOM (and KVM) infrastructure have obsoleted x86_cpudef_setup().
Drop the conditional callers that are now unused.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 arch_init.c                |    7 -------
 bsd-user/main.c            |    3 ---
 include/sysemu/arch_init.h |    1 -
 linux-user/main.c          |    3 ---
 vl.c                       |    7 -------
 5 Dateien geändert, 21 Zeilen entfernt(-)

diff --git a/arch_init.c b/arch_init.c
index 8da868b..2495f2c 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1119,13 +1119,6 @@ void do_smbios_option(const char *optarg)
 #endif
 }
 
-void cpudef_init(void)
-{
-#if defined(cpudef_setup)
-    cpudef_setup(); /* parse cpu definitions in target config file */
-#endif
-}
-
 int audio_available(void)
 {
 #ifdef HAS_AUDIO
diff --git a/bsd-user/main.c b/bsd-user/main.c
index ae24723..1084db6 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -762,9 +762,6 @@ int main(int argc, char **argv)
     }
 
     cpu_model = NULL;
-#if defined(cpudef_setup)
-    cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
-#endif
 
     optind = 1;
     for(;;) {
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 5fc780c..84a7f9a 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -27,7 +27,6 @@ extern const uint32_t arch_type;
 void select_soundhw(const char *optarg);
 void do_acpitable_option(const char *optarg);
 void do_smbios_option(const char *optarg);
-void cpudef_init(void);
 int audio_available(void);
 void audio_init(ISABus *isa_bus, PCIBus *pci_bus);
 int tcg_available(void);
diff --git a/linux-user/main.c b/linux-user/main.c
index 3df8aa2..d98802e 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3475,9 +3475,6 @@ int main(int argc, char **argv, char **envp)
     }
 
     cpu_model = NULL;
-#if defined(cpudef_setup)
-    cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
-#endif
 
     /* init debug */
     cpu_set_log_filename(log_file);
diff --git a/vl.c b/vl.c
index f094f04..2e75c9a 100644
--- a/vl.c
+++ b/vl.c
@@ -3798,13 +3798,6 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
-    /* Init CPU def lists, based on config
-     * - Must be called after all the qemu_read_config_file() calls
-     * - Must be called before list_cpus()
-     * - Must be called before machine->init()
-     */
-    cpudef_init();
-
     if (cpu_model && is_help_option(cpu_model)) {
         list_cpus(stdout, &fprintf, cpu_model);
         exit(0);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH qom-cpu-next v3 0/4] target-i386: X86CPU subclasses

[Andreas Färber]

Am 02.02.2013 01:37, schrieb Andreas Färber:
> Hello,
> 
> Long announced, here it is after the freeze: My 3rd attempt at CPU subclasses.
> v3 is closer to v1 again, slimmed down not to touch x86_def_t much.
> 
> It was noticed that in theory a QOM class enumeration could lead to the host
> CPU class_init running before kvm_init(). My proposal is to make the class_init
> reentrant and to re-run it from target-i386's kvm_arch_init() if necessary.
> Similar to target-ppc, -cpu host code is placed into kvm.c rather than cpu.c.
> 
> This conversion is an interim solution to get the code structured in a
> QOM-friendly way and to reach a hot-plug friendly and cross-target aligned
> CPU initialization. It decouples the work of introducing subclasses (to hide
> object initialization from cpu_init() and device_add) from any x86-internal
> improvements (like Igor's global compat properties set from pc-x.y machines).
> The -cpu ? and QMP support is still based on array iteration.
> 
> Based on qom-cpu-next queue, which contains some cpu_init() cleanups already.
> 
> Available for testing here:
> git://github.com/afaerber/qemu-cpu.git qom-cpu-x86-subclasses.v3
> https://github.com/afaerber/qemu-cpu/commits/qom-cpu-x86-subclasses.v3

Required a trivial update for s/object_delete/object_unref/: qom-cpu-x86

Andreas

> Regards,
> Andreas
> 
> v2 -> v3:
> * Instead of re-coding all CPU definitions as class_init functions, leave
>   the built-in definition array in place and place x86_def_t in the class.
> * Use kvm_arch_init() hook to assure class_init succeeds for -cpu host.
>   Suggested by Eduardo.
> 
> v1-> v2:
> * Instead of turning x86_def_t into X86CPUInfo to initialize classes,
>   drop it completely and register types manually with customizable TypeInfos
> * Use new list facilities for printing -cpu ? models
> * Adopt new name scheme suggested by Eduardo and ideas from my alpha series
> * Keep short names in -cpu ? output for alignment reasons
> * Merge cpu_x86_init() into cpu.c:cpu_x86_register()
> * Append patch showing Haswell as subclass of SandyBridge
> 
> Cc: Anthony Liguori <anthony@codemonkey.ws>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Blue Swirl <blauwirbel@gmail.com>
> Cc: Richard Henderson <rth@twiddle.net>
> 
> Andreas Färber (4):
>   target-i386: Move cpu_x86_init()
>   target-i386: Split command line parsing out of cpu_x86_register()
>   target-i386: Slim conversion to X86CPU subclasses
>   Remove cpudef_setup() hooks
> 
>  arch_init.c                |    7 -
>  bsd-user/main.c            |    3 -
>  include/sysemu/arch_init.h |    1 -
>  linux-user/main.c          |    3 -
>  target-i386/cpu-qom.h      |   24 ++++
>  target-i386/cpu.c          |  326 ++++++++++++++++++--------------------------
>  target-i386/cpu.h          |    3 -
>  target-i386/helper.c       |   24 ----
>  target-i386/kvm.c          |   93 +++++++++++++
>  vl.c                       |    7 -
>  10 Dateien geändert, 247 Zeilen hinzugefügt(+), 244 Zeilen entfernt(-)

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg