[Qemu-devel] [RFC 0/4] target-i386: Add "feature-words" CPU object property

[Qemu-devel] [RFC 0/4] target-i386: Add "feature-words" CPU object property

[Eduardo Habkost]

As the work to get X86CPU subclasses static properties defined is taking very
long to get reviewed and we risk not getting it finished in 1.5, I am sending
an alternative mechanism to allow libvirt to probe for CPU model information.

It's interesting to note that this alternative solution is actually _easier_ to
use for libvirt, because libvirt logic is already based on CPUID bit values, not
"feature names", so it should be very easy to convert the current libvirt code
that (incorrectly) query the host CPU directly for available features to instead
start QEMU with "-cpu host" and use the "feature-words" array information to
find out which CPU features are supported by the host.

The property will have two main use cases:
 - Checking host capabilities, by checking the features of the "host"
   CPU model;
 - Checking which features are enabled on each CPU model.

This series has two alternative implementations, so both approaches can get
feedback: patch 3 has a non-qapi implementation, that simply builds the list and
sets the struct fields manually. Patch 4 converts that implementation to a qapi-
based implementation.

(BTW, I really missed some documentation on the list-related visitor functions,
when writing this code)

Example output:

  $ ./QMP/qmp --path=/tmp/m qom-get --path=/machine/unattached/device[1] --property=feature-words
  item[0].cpuid-register: EDX
  item[0].cpuid-input-eax: 1
  item[0].features: 126614521
  item[1].cpuid-register: ECX
  item[1].cpuid-input-eax: 1
  item[1].features: 2155880449
  item[2].cpuid-register: EBX
  item[2].cpuid-input-eax: 7
  item[2].features: 0
  item[2].cpuid-input-ecx: 0
  item[3].cpuid-register: EDX
  item[3].cpuid-input-eax: 2147483649
  item[3].features: 563346425
  item[4].cpuid-register: ECX
  item[4].cpuid-input-eax: 2147483649
  item[4].features: 101
  item[5].cpuid-register: EDX
  item[5].cpuid-input-eax: 3221225473
  item[5].features: 0
  item[6].cpuid-register: EAX
  item[6].cpuid-input-eax: 1073741825
  item[6].features: 0
  item[7].cpuid-register: EDX
  item[7].cpuid-input-eax: 2147483658
  item[7].features: 0

I plan to send another RFC as followup soon, that will add:

 * "-cpu force" (or "nofilter", or something else; I haven't decided about the
   name, yet) flag, to disable feature filtering based on host capabilities.
 * "filtered-features" property, listing which features were filtered out
   because of missing host capabilities (to allow libvirt to implement "enforce"
   mode in a more reliable way).

Eduardo Habkost (4):
  target-i386: Add ECX information to FeatureWordInfo
  target-i386: Move feature word array outside get_register_name_32()
  target-i386: Add "feature-words" property
  target-i386: Use qapi for x86_cpu_get_feature_words()

 .gitignore                       |  2 +
 Makefile                         | 11 +++++
 Makefile.objs                    |  5 +++
 cpu-qapi-schema.json             | 31 ++++++++++++++
 target-i386/Makefile.objs        |  1 +
 target-i386/cpu-qapi-schema.json | 31 ++++++++++++++
 target-i386/cpu.c                | 92 +++++++++++++++++++++++++++++++++-------
 7 files changed, 158 insertions(+), 15 deletions(-)
 create mode 100644 cpu-qapi-schema.json
 create mode 100644 target-i386/cpu-qapi-schema.json

-- 
1.8.1.4

[Qemu-devel] [RFC 1/4] target-i386: Add ECX information to FeatureWordInfo

[Eduardo Habkost]

FEAT_7_0_EBX uses ECX as input, so we have to take that into account
when reporting feature word values.

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

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4b43759..974a8c6 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -151,8 +151,10 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
 
 typedef struct FeatureWordInfo {
     const char **feat_names;
-    uint32_t cpuid_eax; /* Input EAX for CPUID */
-    int cpuid_reg;      /* R_* register constant */
+    uint32_t cpuid_eax;   /* Input EAX for CPUID */
+    bool cpuid_needs_ecx; /* CPUID instruction uses ECX as input */
+    uint32_t cpuid_ecx;   /* Input ECX value for CPUID */
+    int cpuid_reg;        /* output register (R_* constant) */
 } FeatureWordInfo;
 
 static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
@@ -186,7 +188,9 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
     },
     [FEAT_7_0_EBX] = {
         .feat_names = cpuid_7_0_ebx_feature_name,
-        .cpuid_eax = 7, .cpuid_reg = R_EBX,
+        .cpuid_eax = 7,
+        .cpuid_needs_ecx = true, .cpuid_ecx = 0,
+        .cpuid_reg = R_EBX,
     },
 };
 
-- 
1.8.1.4

[Qemu-devel] [RFC 2/4] target-i386: Move feature word array outside get_register_name_32()

[Eduardo Habkost]

The array will be used by the "feature-words" properties to output the
register enum value.

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

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 974a8c6..2020147 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -194,23 +194,24 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
     },
 };
 
+static const char *reg_names_32[CPU_NB_REGS32] = {
+    [R_EAX] = "EAX",
+    [R_ECX] = "ECX",
+    [R_EDX] = "EDX",
+    [R_EBX] = "EBX",
+    [R_ESP] = "ESP",
+    [R_EBP] = "EBP",
+    [R_ESI] = "ESI",
+    [R_EDI] = "EDI",
+};
+
+
 const char *get_register_name_32(unsigned int reg)
 {
-    static const char *reg_names[CPU_NB_REGS32] = {
-        [R_EAX] = "EAX",
-        [R_ECX] = "ECX",
-        [R_EDX] = "EDX",
-        [R_EBX] = "EBX",
-        [R_ESP] = "ESP",
-        [R_EBP] = "EBP",
-        [R_ESI] = "ESI",
-        [R_EDI] = "EDI",
-    };
-
     if (reg > CPU_NB_REGS32) {
         return NULL;
     }
-    return reg_names[reg];
+    return reg_names_32[reg];
 }
 
 /* collects per-function cpuid data
-- 
1.8.1.4

[Qemu-devel] [RFC 3/4] target-i386: Add "feature-words" property

[Eduardo Habkost]

This property will be useful for libvirt, as libvirt already has logic
based on low-level feature bits (not feature names), so it will be
really easy to convert the current libvirt logic to something using the
"feature-words" property.

The property will have two main use cases:
 - Checking host capabilities, by checking the features of the "host"
   CPU model
 - Checking which features are enabled on each CPU model

Example output:

  $ ./QMP/qmp --path=/tmp/m qom-get --path=/machine/unattached/device[1] --property=feature-words
  item[0].cpuid-register: EDX
  item[0].cpuid-input-eax: 1
  item[0].features: 126614521
  item[1].cpuid-register: ECX
  item[1].cpuid-input-eax: 1
  item[1].features: 2155880449
  item[2].cpuid-register: EBX
  item[2].cpuid-input-eax: 7
  item[2].features: 0
  item[2].cpuid-input-ecx: 0
  item[3].cpuid-register: EDX
  item[3].cpuid-input-eax: 2147483649
  item[3].features: 563346425
  item[4].cpuid-register: ECX
  item[4].cpuid-input-eax: 2147483649
  item[4].features: 101
  item[5].cpuid-register: EDX
  item[5].cpuid-input-eax: 3221225473
  item[5].features: 0
  item[6].cpuid-register: EAX
  item[6].cpuid-input-eax: 1073741825
  item[6].features: 0
  item[7].cpuid-register: EDX
  item[7].cpuid-input-eax: 2147483658
  item[7].features: 0

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
* This is the solution not using qapi. The next patch will introduce a
  solution using qapi, so both approaches can get feedback.
* Current solution is not based on the "feature word array" patch,
  but it may be easily rebased and simplified if we decide to include
  the CPUX86State feature_word array later.
---
 target-i386/cpu.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 2020147..ebf6358 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1277,6 +1277,44 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
     cpu->env.tsc_khz = value / 1000;
 }
 
+static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    X86CPU *cpu = X86_CPU(obj);
+    CPUX86State *env = &cpu->env;
+    FeatureWord w;
+    Error *err = NULL;
+    uint32_t feat_values[FEATURE_WORDS] = { };
+
+    /* We need to set the entries one-by-one, by now, while we don't have
+     * a single feature_words array on X86CPU
+     */
+    feat_values[FEAT_1_EDX] = env->cpuid_features;
+    feat_values[FEAT_1_ECX] = env->cpuid_ext_features;
+    feat_values[FEAT_8000_0001_EDX] = env->cpuid_ext2_features;
+    feat_values[FEAT_8000_0001_ECX] = env->cpuid_ext3_features;
+    feat_values[FEAT_C000_0001_EDX] = env->cpuid_ext4_features;
+    feat_values[FEAT_KVM] = env->cpuid_kvm_features;
+    feat_values[FEAT_SVM] = env->cpuid_svm_features;
+    feat_values[FEAT_7_0_EBX] = env->cpuid_7_0_ebx_features;
+
+    visit_start_list(v, "feature-words", &err);
+    for (w = 0; w < FEATURE_WORDS; w++) {
+        FeatureWordInfo *wi = &feature_word_info[w];
+        visit_start_struct(v, NULL, NULL, NULL, 0, &err);
+        visit_type_uint32(v, &wi->cpuid_eax, "cpuid-input-eax", &err);
+        if (wi->cpuid_needs_ecx) {
+            visit_type_uint32(v, &wi->cpuid_ecx, "cpuid-input-ecx", &err);
+        }
+        visit_type_enum(v, &wi->cpuid_reg, reg_names_32, "x86-register-32",
+                        "cpuid-register", &err);
+        visit_type_uint32(v, &feat_values[w], "features", &err);
+        visit_end_struct(v, &err);
+    }
+    visit_end_list(v, &err);
+    error_propagate(errp, err);
+}
+
 static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
 {
     x86_def_t *def;
@@ -2232,6 +2270,9 @@ static void x86_cpu_initfn(Object *obj)
     object_property_add(obj, "tsc-frequency", "int",
                         x86_cpuid_get_tsc_freq,
                         x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
+    object_property_add(obj, "feature-words", "x86-cpu-feature-words",
+                        x86_cpu_get_feature_words,
+                        NULL, NULL, NULL, NULL);
 
     env->cpuid_apic_id = x86_cpu_apic_id_from_index(cs->cpu_index);
 
-- 
1.8.1.4

[Qemu-devel] [RFC 4/4] target-i386: Use qapi for x86_cpu_get_feature_words()

[Eduardo Habkost]

Add a schema for QAPI definitions that will be used by *-user.

Example output:

  $ ./QMP/qmp --path=/tmp/m qom-get --path=/machine/unattached/device[1] --property=feature-words
  item[0].cpuid-register: EDX
  item[0].cpuid-input-eax: 2147483658
  item[0].features: 0
  item[1].cpuid-register: EAX
  item[1].cpuid-input-eax: 1073741825
  item[1].features: 0
  item[2].cpuid-register: EDX
  item[2].cpuid-input-eax: 3221225473
  item[2].features: 0
  item[3].cpuid-register: ECX
  item[3].cpuid-input-eax: 2147483649
  item[3].features: 101
  item[4].cpuid-register: EDX
  item[4].cpuid-input-eax: 2147483649
  item[4].features: 563346425
  item[5].cpuid-register: EBX
  item[5].cpuid-input-eax: 7
  item[5].features: 0
  item[5].cpuid-input-ecx: 0
  item[6].cpuid-register: ECX
  item[6].cpuid-input-eax: 1
  item[6].features: 2155880449
  item[7].cpuid-register: EDX
  item[7].cpuid-input-eax: 1
  item[7].features: 126614521

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
* I am sending this as a separated patch so both alternatives (with qapi
  and without qapi) can be compared and get feedback.
* Current solution is not based on the "feature word array" patch,
  but it may be easily rebased and simplified if we decide to include
  the CPUX86State feature_words array later.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 .gitignore                       |  2 ++
 Makefile                         | 11 ++++++
 Makefile.objs                    |  5 +++
 cpu-qapi-schema.json             | 31 ++++++++++++++++
 target-i386/Makefile.objs        |  1 +
 target-i386/cpu-qapi-schema.json | 31 ++++++++++++++++
 target-i386/cpu.c                | 78 ++++++++++++++++++++++++----------------
 7 files changed, 128 insertions(+), 31 deletions(-)
 create mode 100644 cpu-qapi-schema.json
 create mode 100644 target-i386/cpu-qapi-schema.json

diff --git a/.gitignore b/.gitignore
index 487813a..71408e3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -21,6 +21,8 @@ linux-headers/asm
 qapi-generated
 qapi-types.[ch]
 qapi-visit.[ch]
+cpu-qapi-types.[ch]
+cpu-qapi-visit.[ch]
 qmp-commands.h
 qmp-marshal.c
 qemu-doc.html
diff --git a/Makefile b/Makefile
index 80344d9..efa2971 100644
--- a/Makefile
+++ b/Makefile
@@ -35,6 +35,9 @@ GENERATED_HEADERS = config-host.h qemu-options.def
 GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
 GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c
 
+GENERATED_HEADERS += cpu-qapi-types.h cpu-qapi-visit.h
+GENERATED_SOURCES += cpu-qapi-types.c cpu-qapi-visit.c
+
 GENERATED_HEADERS += trace/generated-events.h
 GENERATED_SOURCES += trace/generated-events.c
 
@@ -209,6 +212,14 @@ qmp-commands.h qmp-marshal.c :\
 $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
 	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o "." < $<, "  GEN   $@")
 
+cpu-qapi-types.c cpu-qapi-types.h :\
+$(SRC_PATH)/cpu-qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o "." -p "cpu-" < $<, "  GEN   $@")
+cpu-qapi-visit.c cpu-qapi-visit.h :\
+$(SRC_PATH)/cpu-qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o "." -p "cpu-" < $<, "  GEN   $@")
+
+
 QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h)
 $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
 
diff --git a/Makefile.objs b/Makefile.objs
index f99841c..4099dbb 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -91,6 +91,11 @@ common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o
 common-obj-y += qmp.o hmp.o
 endif
 
+######################################################################
+# qapi types used by both system and user emulation:
+
+common-obj-y += cpu-qapi-types.o cpu-qapi-visit.o
+
 #######################################################################
 # Target-independent parts used in system and user emulation
 common-obj-y += qemu-log.o
diff --git a/cpu-qapi-schema.json b/cpu-qapi-schema.json
new file mode 100644
index 0000000..0ecf528
--- /dev/null
+++ b/cpu-qapi-schema.json
@@ -0,0 +1,31 @@
+##
+# @X86CPURegister32
+#
+# A X86 32-bit register
+#
+# Since: 1.5
+##
+{ 'enum': 'X86CPURegister32',
+  'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ] }
+
+##
+# @X86CPUFeatureWordInfo
+#
+# Information about a X86 CPU feature word
+#
+# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
+#
+# @cpuid-input-ecx: #optional Input ECX value for CPUID instruction for that
+#                   feature word
+#
+# @cpuid-register: Output register containing the feature bits
+#
+# @features: value of output register, containing the feature bits
+#
+# Since: 1.5
+##
+{ 'type': 'X86CPUFeatureWordInfo',
+  'data': { 'cpuid-input-eax': 'int',
+            '*cpuid-input-ecx': 'int',
+            'cpuid-register': 'X86CPURegister32',
+            'features': 'int' } }
diff --git a/target-i386/Makefile.objs b/target-i386/Makefile.objs
index c1d4f05..8b0b483 100644
--- a/target-i386/Makefile.objs
+++ b/target-i386/Makefile.objs
@@ -6,3 +6,4 @@ obj-$(CONFIG_KVM) += kvm.o hyperv.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
 obj-$(CONFIG_LINUX_USER) += ioport-user.o
 obj-$(CONFIG_BSD_USER) += ioport-user.o
+
diff --git a/target-i386/cpu-qapi-schema.json b/target-i386/cpu-qapi-schema.json
new file mode 100644
index 0000000..0ecf528
--- /dev/null
+++ b/target-i386/cpu-qapi-schema.json
@@ -0,0 +1,31 @@
+##
+# @X86CPURegister32
+#
+# A X86 32-bit register
+#
+# Since: 1.5
+##
+{ 'enum': 'X86CPURegister32',
+  'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ] }
+
+##
+# @X86CPUFeatureWordInfo
+#
+# Information about a X86 CPU feature word
+#
+# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word
+#
+# @cpuid-input-ecx: #optional Input ECX value for CPUID instruction for that
+#                   feature word
+#
+# @cpuid-register: Output register containing the feature bits
+#
+# @features: value of output register, containing the feature bits
+#
+# Since: 1.5
+##
+{ 'type': 'X86CPUFeatureWordInfo',
+  'data': { 'cpuid-input-eax': 'int',
+            '*cpuid-input-ecx': 'int',
+            'cpuid-register': 'X86CPURegister32',
+            'features': 'int' } }
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ebf6358..ccd7a27 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -30,6 +30,8 @@
 #include "qemu/config-file.h"
 #include "qapi/qmp/qerror.h"
 
+#include "cpu-qapi-types.h"
+#include "cpu-qapi-visit.h"
 #include "qapi/visitor.h"
 #include "sysemu/arch_init.h"
 
@@ -194,16 +196,26 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
     },
 };
 
-static const char *reg_names_32[CPU_NB_REGS32] = {
-    [R_EAX] = "EAX",
-    [R_ECX] = "ECX",
-    [R_EDX] = "EDX",
-    [R_EBX] = "EBX",
-    [R_ESP] = "ESP",
-    [R_EBP] = "EBP",
-    [R_ESI] = "ESI",
-    [R_EDI] = "EDI",
+typedef struct X86RegisterInfo32 {
+    /* Name of 32-bit register */
+    const char *name;
+    /* QAPI enum value for 32-bit register */
+    X86CPURegister32 qapi_enum;
+} X86RegisterInfo32;
+
+#define REGISTER(reg) \
+    [R_##reg] = { .name = #reg, .qapi_enum = X86_C_P_U_REGISTER32_##reg }
+X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = {
+    REGISTER(EAX),
+    REGISTER(ECX),
+    REGISTER(EDX),
+    REGISTER(EBX),
+    REGISTER(ESP),
+    REGISTER(EBP),
+    REGISTER(ESI),
+    REGISTER(EDI),
 };
+#undef REGISTER
 
 
 const char *get_register_name_32(unsigned int reg)
@@ -211,7 +223,7 @@ const char *get_register_name_32(unsigned int reg)
     if (reg > CPU_NB_REGS32) {
         return NULL;
     }
-    return reg_names_32[reg];
+    return x86_reg_info_32[reg].name;
 }
 
 /* collects per-function cpuid data
@@ -1284,34 +1296,38 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
     CPUX86State *env = &cpu->env;
     FeatureWord w;
     Error *err = NULL;
-    uint32_t feat_values[FEATURE_WORDS] = { };
+    X86CPUFeatureWordInfo word_infos[FEATURE_WORDS] = { };
+    X86CPUFeatureWordInfoList list_entries[FEATURE_WORDS] = { };
+    X86CPUFeatureWordInfoList *list = NULL;
 
     /* We need to set the entries one-by-one, by now, while we don't have
      * a single feature_words array on X86CPU
      */
-    feat_values[FEAT_1_EDX] = env->cpuid_features;
-    feat_values[FEAT_1_ECX] = env->cpuid_ext_features;
-    feat_values[FEAT_8000_0001_EDX] = env->cpuid_ext2_features;
-    feat_values[FEAT_8000_0001_ECX] = env->cpuid_ext3_features;
-    feat_values[FEAT_C000_0001_EDX] = env->cpuid_ext4_features;
-    feat_values[FEAT_KVM] = env->cpuid_kvm_features;
-    feat_values[FEAT_SVM] = env->cpuid_svm_features;
-    feat_values[FEAT_7_0_EBX] = env->cpuid_7_0_ebx_features;
-
-    visit_start_list(v, "feature-words", &err);
+    word_infos[FEAT_1_EDX].features = env->cpuid_features;
+    word_infos[FEAT_1_ECX].features = env->cpuid_ext_features;
+    word_infos[FEAT_8000_0001_EDX].features = env->cpuid_ext2_features;
+    word_infos[FEAT_8000_0001_ECX].features = env->cpuid_ext3_features;
+    word_infos[FEAT_C000_0001_EDX].features = env->cpuid_ext4_features;
+    word_infos[FEAT_KVM].features = env->cpuid_kvm_features;
+    word_infos[FEAT_SVM].features = env->cpuid_svm_features;
+    word_infos[FEAT_7_0_EBX].features = env->cpuid_7_0_ebx_features;
+
+    /* Build the rest of the list fields */
     for (w = 0; w < FEATURE_WORDS; w++) {
         FeatureWordInfo *wi = &feature_word_info[w];
-        visit_start_struct(v, NULL, NULL, NULL, 0, &err);
-        visit_type_uint32(v, &wi->cpuid_eax, "cpuid-input-eax", &err);
-        if (wi->cpuid_needs_ecx) {
-            visit_type_uint32(v, &wi->cpuid_ecx, "cpuid-input-ecx", &err);
-        }
-        visit_type_enum(v, &wi->cpuid_reg, reg_names_32, "x86-register-32",
-                        "cpuid-register", &err);
-        visit_type_uint32(v, &feat_values[w], "features", &err);
-        visit_end_struct(v, &err);
+        X86CPUFeatureWordInfo *qwi = &word_infos[w];
+        qwi->cpuid_input_eax = wi->cpuid_eax;
+        qwi->has_cpuid_input_ecx = wi->cpuid_needs_ecx;
+        qwi->cpuid_input_ecx = wi->cpuid_ecx;
+        qwi->cpuid_register = x86_reg_info_32[wi->cpuid_reg].qapi_enum;
+
+        /* List will be in reverse order, but order shouldn't matter */
+        list_entries[w].next = list;
+        list_entries[w].value = &word_infos[w];
+        list = &list_entries[w];
     }
-    visit_end_list(v, &err);
+
+    visit_type_X86CPUFeatureWordInfoList(v, &list, "feature-words", &err);
     error_propagate(errp, err);
 }
 
-- 
1.8.1.4

Re: [Qemu-devel] [RFC 2/4] target-i386: Move feature word array outside get_register_name_32()

[Eduardo Habkost]

On Mon, Apr 01, 2013 at 04:46:47PM -0300, Eduardo Habkost wrote:
> The array will be used by the "feature-words" properties to output the
> register enum value.

Oops, the patch subject line is wrong: it was supposed to say "Move
register name array".

> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target-i386/cpu.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 974a8c6..2020147 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -194,23 +194,24 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>      },
>  };
>  
> +static const char *reg_names_32[CPU_NB_REGS32] = {
> +    [R_EAX] = "EAX",
> +    [R_ECX] = "ECX",
> +    [R_EDX] = "EDX",
> +    [R_EBX] = "EBX",
> +    [R_ESP] = "ESP",
> +    [R_EBP] = "EBP",
> +    [R_ESI] = "ESI",
> +    [R_EDI] = "EDI",
> +};
> +
> +
>  const char *get_register_name_32(unsigned int reg)
>  {
> -    static const char *reg_names[CPU_NB_REGS32] = {
> -        [R_EAX] = "EAX",
> -        [R_ECX] = "ECX",
> -        [R_EDX] = "EDX",
> -        [R_EBX] = "EBX",
> -        [R_ESP] = "ESP",
> -        [R_EBP] = "EBP",
> -        [R_ESI] = "ESI",
> -        [R_EDI] = "EDI",
> -    };
> -
>      if (reg > CPU_NB_REGS32) {
>          return NULL;
>      }
> -    return reg_names[reg];
> +    return reg_names_32[reg];
>  }
>  
>  /* collects per-function cpuid data
> -- 
> 1.8.1.4
> 
> 

-- 
Eduardo