All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property
@ 2013-04-22 19:00 Eduardo Habkost
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 1/9] target-i386: cleanup: Group together level, xlevel, xlevel2 fields Eduardo Habkost
                   ` (9 more replies)
  0 siblings, 10 replies; 32+ messages in thread
From: Eduardo Habkost @ 2013-04-22 19:00 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: libvir-list, Igor Mammedov, Jiri Denemark

This series includes the previous "replace cpuid_*features fields with a feature
word array" series.

The first 4 patches already have a Reviewed-by from Igor, they correspond to v10
plus a small indent fix requested by him.

As the cpuid_*features series was holding my "feature-words"/"filtered-features"
series (previously sent as RFC), I am now sending both as a single series, to
try to get "feature-words"/"filtered-features" some attention and try to get it
included in 1.5.

The "feature-words"/"filtered-features" mechanism is very important for libvirt,
to allow it to ensure the guest is getting the required set of CPU features, as
configured by the user.


Eduardo Habkost (9):
  target-i386: cleanup: Group together level, xlevel, xlevel2 fields
  target-i386/kvm.c: Code formatting changes
  target-i386/cpu.c: Break lines so they don't get too long
  target-i386: Replace cpuid_*features fields with a feature word array
  target-i386: Add ECX information to FeatureWordInfo
  target-i386: Add "feature-words" property
  target-i386: Use FeatureWord loop on filter_features_for_kvm()
  target-i386: Introduce X86CPU.filtered_features field
  target-i386: Add "filtered-features" property to X86CPU

 .gitignore                |   2 +
 Makefile.objs             |   7 +-
 bsd-user/elfload.c        |   2 +-
 bsd-user/main.c           |   4 +-
 hw/i386/kvm/clock.c       |   2 +-
 linux-user/elfload.c      |   2 +-
 linux-user/main.c         |   4 +-
 qapi-schema.json          |  31 +++
 target-i386/cpu-qom.h     |   3 +
 target-i386/cpu.c         | 501 +++++++++++++++++++++++++++++-----------------
 target-i386/cpu.h         |  15 +-
 target-i386/helper.c      |   4 +-
 target-i386/kvm.c         |   5 +-
 target-i386/misc_helper.c |  14 +-
 target-i386/translate.c   |  10 +-
 15 files changed, 385 insertions(+), 221 deletions(-)

-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu 1/9] target-i386: cleanup: Group together level, xlevel, xlevel2 fields
  2013-04-22 19:00 [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property Eduardo Habkost
@ 2013-04-22 19:00 ` Eduardo Habkost
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 2/9] target-i386/kvm.c: Code formatting changes Eduardo Habkost
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 32+ messages in thread
From: Eduardo Habkost @ 2013-04-22 19:00 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: libvir-list, Igor Mammedov, Jiri Denemark

Consolidate level, xlevel, xlevel2 fields in x86_def_t and CPUX86State.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-By: Igor Mammedov <imammedo@redhat.com>
---
Changes v9:
 * Merged "target-i386: Move cpuid_xlevel, cpuid_xlevel2 fields in X86CPU"
   and "target-i386: Move xlevel/xlevel2 in struct x86_def_t"
   in a single patch
---
 target-i386/cpu.c | 4 ++--
 target-i386/cpu.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e2302d8..732cafd 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -349,6 +349,8 @@ static void add_flagname_to_bitmaps(const char *flagname,
 typedef struct x86_def_t {
     const char *name;
     uint32_t level;
+    uint32_t xlevel;
+    uint32_t xlevel2;
     /* vendor is zero-terminated, 12 character ASCII string */
     char vendor[CPUID_VENDOR_SZ + 1];
     int family;
@@ -356,11 +358,9 @@ typedef struct x86_def_t {
     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;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index a1614e8..c621359 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -836,19 +836,19 @@ typedef struct CPUX86State {
 
     /* processor features (e.g. for CPUID insn) */
     uint32_t cpuid_level;
+    uint32_t cpuid_xlevel;
+    uint32_t cpuid_xlevel2;
     uint32_t cpuid_vendor1;
     uint32_t cpuid_vendor2;
     uint32_t cpuid_vendor3;
     uint32_t cpuid_version;
     uint32_t cpuid_features;
     uint32_t cpuid_ext_features;
-    uint32_t cpuid_xlevel;
     uint32_t cpuid_model[12];
     uint32_t cpuid_ext2_features;
     uint32_t cpuid_ext3_features;
     uint32_t cpuid_apic_id;
     /* Store the results of Centaur's CPUID instructions */
-    uint32_t cpuid_xlevel2;
     uint32_t cpuid_ext4_features;
     /* Flags from CPUID[EAX=7,ECX=0].EBX */
     uint32_t cpuid_7_0_ebx_features;
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu 2/9] target-i386/kvm.c: Code formatting changes
  2013-04-22 19:00 [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property Eduardo Habkost
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 1/9] target-i386: cleanup: Group together level, xlevel, xlevel2 fields Eduardo Habkost
@ 2013-04-22 19:00 ` Eduardo Habkost
  2013-05-01 22:55   ` Andreas Färber
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 3/9] target-i386/cpu.c: Break lines so they don't get too long Eduardo Habkost
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 32+ messages in thread
From: Eduardo Habkost @ 2013-04-22 19:00 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: libvir-list, Igor Mammedov, Jiri Denemark

Add appropriate spaces around operators, and break line where it needs
to be broken to allow feature-words array to be introduced without
having too-long lines.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-By: Igor Mammedov <imammedo@redhat.com>
---
Changes v9:
 * 1-char alignment change: keep the opening parenthesis of both sides
   of the "==" operator starting at the same column
---
 target-i386/kvm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 0e7cc81..b5bff33 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -613,7 +613,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
     cpuid_data.cpuid.nent = cpuid_i;
 
     if (((env->cpuid_version >> 8)&0xF) >= 6
-        && (env->cpuid_features&(CPUID_MCE|CPUID_MCA)) == (CPUID_MCE|CPUID_MCA)
+        && (env->cpuid_features & (CPUID_MCE|CPUID_MCA)) ==
+           (CPUID_MCE|CPUID_MCA)
         && kvm_check_extension(cs->kvm_state, KVM_CAP_MCE) > 0) {
         uint64_t mcg_cap;
         int banks;
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu 3/9] target-i386/cpu.c: Break lines so they don't get too long
  2013-04-22 19:00 [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property Eduardo Habkost
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 1/9] target-i386: cleanup: Group together level, xlevel, xlevel2 fields Eduardo Habkost
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 2/9] target-i386/kvm.c: Code formatting changes Eduardo Habkost
@ 2013-04-22 19:00 ` Eduardo Habkost
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 4/9] target-i386: Replace cpuid_*features fields with a feature word array Eduardo Habkost
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 32+ messages in thread
From: Eduardo Habkost @ 2013-04-22 19:00 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: libvir-list, Igor Mammedov, Jiri Denemark

Break lines on kvm_check_features_against_host(), kvm_cpu_fill_host(),
and builtin_x86_defs, so they don't get too long once the *_features
fields are replaced by an array.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-By: Igor Mammedov <imammedo@redhat.com>
---
Changes v9:
 * Merged all "Break lines" patches from previous series into a single
   patch
---
 target-i386/cpu.c | 270 ++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 180 insertions(+), 90 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 732cafd..73ae2ef 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -423,13 +423,17 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 2,
         .stepping = 3,
-        .features = PPRO_FEATURES |
+        .features =
+            PPRO_FEATURES |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
             CPUID_PSE36,
-        .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_CX16 | CPUID_EXT_POPCNT,
-        .ext2_features = (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
+        .ext_features =
+            CPUID_EXT_SSE3 | CPUID_EXT_CX16 | CPUID_EXT_POPCNT,
+        .ext2_features =
+            (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
             CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
-        .ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
+        .ext3_features =
+            CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
             CPUID_EXT3_ABM | CPUID_EXT3_SSE4A,
         .xlevel = 0x8000000A,
     },
@@ -440,12 +444,15 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 16,
         .model = 2,
         .stepping = 3,
-        .features = PPRO_FEATURES |
+        .features =
+            PPRO_FEATURES |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
             CPUID_PSE36 | CPUID_VME | CPUID_HT,
-        .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_CX16 |
+        .ext_features =
+            CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_CX16 |
             CPUID_EXT_POPCNT,
-        .ext2_features = (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
+        .ext2_features =
+            (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
             CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX |
             CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT | CPUID_EXT2_MMXEXT |
             CPUID_EXT2_FFXSR | CPUID_EXT2_PDPE1GB | CPUID_EXT2_RDTSCP,
@@ -453,9 +460,11 @@ static x86_def_t builtin_x86_defs[] = {
                     CPUID_EXT3_CR8LEG,
                     CPUID_EXT3_MISALIGNSSE, CPUID_EXT3_3DNOWPREFETCH,
                     CPUID_EXT3_OSVW, CPUID_EXT3_IBS */
-        .ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
+        .ext3_features =
+            CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
             CPUID_EXT3_ABM | CPUID_EXT3_SSE4A,
-        .svm_features = CPUID_SVM_NPT | CPUID_SVM_LBRV,
+        .svm_features =
+            CPUID_SVM_NPT | CPUID_SVM_LBRV,
         .xlevel = 0x8000001A,
         .model_id = "AMD Phenom(tm) 9550 Quad-Core Processor"
     },
@@ -466,15 +475,19 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 15,
         .stepping = 11,
-        .features = PPRO_FEATURES |
+        .features =
+            PPRO_FEATURES |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
             CPUID_PSE36 | CPUID_VME | CPUID_DTS | CPUID_ACPI | CPUID_SS |
             CPUID_HT | CPUID_TM | CPUID_PBE,
-        .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 |
+        .ext_features =
+            CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 |
             CPUID_EXT_DTES64 | CPUID_EXT_DSCPL | CPUID_EXT_VMX | CPUID_EXT_EST |
             CPUID_EXT_TM2 | CPUID_EXT_CX16 | CPUID_EXT_XTPR | CPUID_EXT_PDCM,
-        .ext2_features = CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
-        .ext3_features = CPUID_EXT3_LAHF_LM,
+        .ext2_features =
+            CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
+        .ext3_features =
+            CPUID_EXT3_LAHF_LM,
         .xlevel = 0x80000008,
         .model_id = "Intel(R) Core(TM)2 Duo CPU     T7700  @ 2.40GHz",
     },
@@ -486,19 +499,23 @@ static x86_def_t builtin_x86_defs[] = {
         .model = 6,
         .stepping = 1,
         /* Missing: CPUID_VME, CPUID_HT */
-        .features = PPRO_FEATURES |
+        .features =
+            PPRO_FEATURES |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
             CPUID_PSE36,
         /* Missing: CPUID_EXT_POPCNT, CPUID_EXT_MONITOR */
-        .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_CX16,
+        .ext_features =
+            CPUID_EXT_SSE3 | CPUID_EXT_CX16,
         /* Missing: CPUID_EXT2_PDPE1GB, CPUID_EXT2_RDTSCP */
-        .ext2_features = (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
+        .ext2_features =
+            (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
             CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
         /* Missing: CPUID_EXT3_LAHF_LM, CPUID_EXT3_CMP_LEG, CPUID_EXT3_EXTAPIC,
                     CPUID_EXT3_CR8LEG, CPUID_EXT3_ABM, CPUID_EXT3_SSE4A,
                     CPUID_EXT3_MISALIGNSSE, CPUID_EXT3_3DNOWPREFETCH,
                     CPUID_EXT3_OSVW, CPUID_EXT3_IBS, CPUID_EXT3_SVM */
-        .ext3_features = 0,
+        .ext3_features =
+            0,
         .xlevel = 0x80000008,
         .model_id = "Common KVM processor"
     },
@@ -509,8 +526,10 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 3,
         .stepping = 3,
-        .features = PPRO_FEATURES,
-        .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_POPCNT,
+        .features =
+            PPRO_FEATURES,
+        .ext_features =
+            CPUID_EXT_SSE3 | CPUID_EXT_POPCNT,
         .xlevel = 0x80000004,
     },
     {
@@ -520,11 +539,15 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 15,
         .model = 6,
         .stepping = 1,
-        .features = PPRO_FEATURES |
+        .features =
+            PPRO_FEATURES |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_PSE36,
-        .ext_features = CPUID_EXT_SSE3,
-        .ext2_features = PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES,
-        .ext3_features = 0,
+        .ext_features =
+            CPUID_EXT_SSE3,
+        .ext2_features =
+            PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES,
+        .ext3_features =
+            0,
         .xlevel = 0x80000008,
         .model_id = "Common 32-bit KVM processor"
     },
@@ -535,12 +558,15 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 14,
         .stepping = 8,
-        .features = PPRO_FEATURES | CPUID_VME |
+        .features =
+            PPRO_FEATURES | CPUID_VME |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_DTS | CPUID_ACPI |
             CPUID_SS | CPUID_HT | CPUID_TM | CPUID_PBE,
-        .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_VMX |
+        .ext_features =
+            CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_VMX |
             CPUID_EXT_EST | CPUID_EXT_TM2 | CPUID_EXT_XTPR | CPUID_EXT_PDCM,
-        .ext2_features = CPUID_EXT2_NX,
+        .ext2_features =
+            CPUID_EXT2_NX,
         .xlevel = 0x80000008,
         .model_id = "Genuine Intel(R) CPU           T2600  @ 2.16GHz",
     },
@@ -551,7 +577,8 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 4,
         .model = 0,
         .stepping = 0,
-        .features = I486_FEATURES,
+        .features =
+            I486_FEATURES,
         .xlevel = 0,
     },
     {
@@ -561,7 +588,8 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 5,
         .model = 4,
         .stepping = 3,
-        .features = PENTIUM_FEATURES,
+        .features =
+            PENTIUM_FEATURES,
         .xlevel = 0,
     },
     {
@@ -571,7 +599,8 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 5,
         .stepping = 2,
-        .features = PENTIUM2_FEATURES,
+        .features =
+            PENTIUM2_FEATURES,
         .xlevel = 0,
     },
     {
@@ -581,7 +610,8 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 7,
         .stepping = 3,
-        .features = PENTIUM3_FEATURES,
+        .features =
+            PENTIUM3_FEATURES,
         .xlevel = 0,
     },
     {
@@ -591,9 +621,11 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 2,
         .stepping = 3,
-        .features = PPRO_FEATURES | CPUID_PSE36 | CPUID_VME | CPUID_MTRR |
+        .features =
+            PPRO_FEATURES | CPUID_PSE36 | CPUID_VME | CPUID_MTRR |
             CPUID_MCA,
-        .ext2_features = (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
+        .ext2_features =
+            (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
             CPUID_EXT2_MMXEXT | CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT,
         .xlevel = 0x80000008,
     },
@@ -605,15 +637,19 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 28,
         .stepping = 2,
-        .features = PPRO_FEATURES |
+        .features =
+            PPRO_FEATURES |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_VME | CPUID_DTS |
             CPUID_ACPI | CPUID_SS | CPUID_HT | CPUID_TM | CPUID_PBE,
             /* Some CPUs got no CPUID_SEP */
-        .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 |
+        .ext_features =
+            CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 |
             CPUID_EXT_DSCPL | CPUID_EXT_EST | CPUID_EXT_TM2 | CPUID_EXT_XTPR,
-        .ext2_features = (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
+        .ext2_features =
+            (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
             CPUID_EXT2_NX,
-        .ext3_features = CPUID_EXT3_LAHF_LM,
+        .ext3_features =
+            CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Intel(R) Atom(TM) CPU N270   @ 1.60GHz",
     },
@@ -624,14 +660,18 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 2,
         .stepping = 3,
-        .features = CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
+        .features =
+            CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features = CPUID_EXT_SSSE3 | CPUID_EXT_SSE3,
-        .ext2_features = CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
-        .ext3_features = CPUID_EXT3_LAHF_LM,
+        .ext_features =
+            CPUID_EXT_SSSE3 | CPUID_EXT_SSE3,
+        .ext2_features =
+            CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
+        .ext3_features =
+            CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Intel Celeron_4x0 (Conroe/Merom Class Core 2)",
     },
@@ -642,15 +682,19 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 2,
         .stepping = 3,
-        .features = CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
+        .features =
+            CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features = CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 |
+        .ext_features =
+            CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 |
              CPUID_EXT_SSE3,
-        .ext2_features = CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
-        .ext3_features = CPUID_EXT3_LAHF_LM,
+        .ext2_features =
+            CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
+        .ext3_features =
+            CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Intel Core 2 Duo P9xxx (Penryn Class Core 2)",
     },
@@ -661,15 +705,19 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 2,
         .stepping = 3,
-        .features = CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
+        .features =
+            CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features = CPUID_EXT_POPCNT | CPUID_EXT_SSE42 | CPUID_EXT_SSE41 |
+        .ext_features =
+            CPUID_EXT_POPCNT | CPUID_EXT_SSE42 | CPUID_EXT_SSE41 |
              CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | CPUID_EXT_SSE3,
-        .ext2_features = CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
-        .ext3_features = CPUID_EXT3_LAHF_LM,
+        .ext2_features =
+            CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
+        .ext3_features =
+            CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Intel Core i7 9xx (Nehalem Class Core i7)",
     },
@@ -680,16 +728,20 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 44,
         .stepping = 1,
-        .features = CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
+        .features =
+            CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features = CPUID_EXT_AES | CPUID_EXT_POPCNT | CPUID_EXT_SSE42 |
+        .ext_features =
+            CPUID_EXT_AES | CPUID_EXT_POPCNT | CPUID_EXT_SSE42 |
              CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 |
              CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3,
-        .ext2_features = CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
-        .ext3_features = CPUID_EXT3_LAHF_LM,
+        .ext2_features =
+            CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
+        .ext3_features =
+            CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Westmere E56xx/L56xx/X56xx (Nehalem-C)",
     },
@@ -700,19 +752,23 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 42,
         .stepping = 1,
-        .features = CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
+        .features =
+            CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features = CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
+        .ext_features =
+            CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
              CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_POPCNT |
              CPUID_EXT_X2APIC | CPUID_EXT_SSE42 | CPUID_EXT_SSE41 |
              CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | CPUID_EXT_PCLMULQDQ |
              CPUID_EXT_SSE3,
-        .ext2_features = CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_NX |
+        .ext2_features =
+            CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_NX |
              CPUID_EXT2_SYSCALL,
-        .ext3_features = CPUID_EXT3_LAHF_LM,
+        .ext3_features =
+            CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Intel Xeon E312xx (Sandy Bridge)",
     },
@@ -723,21 +779,26 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 60,
         .stepping = 1,
-        .features = CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
+        .features =
+            CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features = CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
+        .ext_features =
+            CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
              CPUID_EXT_POPCNT | CPUID_EXT_X2APIC | CPUID_EXT_SSE42 |
              CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 |
              CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3 |
              CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_FMA | CPUID_EXT_MOVBE |
              CPUID_EXT_PCID,
-        .ext2_features = CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_NX |
+        .ext2_features =
+            CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_NX |
              CPUID_EXT2_SYSCALL,
-        .ext3_features = CPUID_EXT3_LAHF_LM,
-        .cpuid_7_0_ebx_features = CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 |
+        .ext3_features =
+            CPUID_EXT3_LAHF_LM,
+        .cpuid_7_0_ebx_features =
+            CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 |
             CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP |
             CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID |
             CPUID_7_0_EBX_RTM,
@@ -751,13 +812,16 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 15,
         .model = 6,
         .stepping = 1,
-        .features = CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
+        .features =
+            CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features = CPUID_EXT_SSE3,
-        .ext2_features = CPUID_EXT2_LM | CPUID_EXT2_FXSR | CPUID_EXT2_MMX |
+        .ext_features =
+            CPUID_EXT_SSE3,
+        .ext2_features =
+            CPUID_EXT2_LM | CPUID_EXT2_FXSR | CPUID_EXT2_MMX |
              CPUID_EXT2_NX | CPUID_EXT2_PSE36 | CPUID_EXT2_PAT |
              CPUID_EXT2_CMOV | CPUID_EXT2_MCA | CPUID_EXT2_PGE |
              CPUID_EXT2_MTRR | CPUID_EXT2_SYSCALL | CPUID_EXT2_APIC |
@@ -773,20 +837,24 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 15,
         .model = 6,
         .stepping = 1,
-        .features = CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
+        .features =
+            CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features = CPUID_EXT_CX16 | CPUID_EXT_SSE3,
-        .ext2_features = CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_FXSR |
+        .ext_features =
+            CPUID_EXT_CX16 | CPUID_EXT_SSE3,
+        .ext2_features =
+            CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_FXSR |
              CPUID_EXT2_MMX | CPUID_EXT2_NX | CPUID_EXT2_PSE36 |
              CPUID_EXT2_PAT | CPUID_EXT2_CMOV | CPUID_EXT2_MCA |
              CPUID_EXT2_PGE | CPUID_EXT2_MTRR | CPUID_EXT2_SYSCALL |
              CPUID_EXT2_APIC | CPUID_EXT2_CX8 | CPUID_EXT2_MCE |
              CPUID_EXT2_PAE | CPUID_EXT2_MSR | CPUID_EXT2_TSC | CPUID_EXT2_PSE |
              CPUID_EXT2_DE | CPUID_EXT2_FPU,
-        .ext3_features = CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
+        .ext3_features =
+            CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
         .xlevel = 0x80000008,
         .model_id = "AMD Opteron 22xx (Gen 2 Class Opteron)",
     },
@@ -797,21 +865,25 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 15,
         .model = 6,
         .stepping = 1,
-        .features = CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
+        .features =
+            CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features = CPUID_EXT_POPCNT | CPUID_EXT_CX16 | CPUID_EXT_MONITOR |
+        .ext_features =
+            CPUID_EXT_POPCNT | CPUID_EXT_CX16 | CPUID_EXT_MONITOR |
              CPUID_EXT_SSE3,
-        .ext2_features = CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_FXSR |
+        .ext2_features =
+            CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_FXSR |
              CPUID_EXT2_MMX | CPUID_EXT2_NX | CPUID_EXT2_PSE36 |
              CPUID_EXT2_PAT | CPUID_EXT2_CMOV | CPUID_EXT2_MCA |
              CPUID_EXT2_PGE | CPUID_EXT2_MTRR | CPUID_EXT2_SYSCALL |
              CPUID_EXT2_APIC | CPUID_EXT2_CX8 | CPUID_EXT2_MCE |
              CPUID_EXT2_PAE | CPUID_EXT2_MSR | CPUID_EXT2_TSC | CPUID_EXT2_PSE |
              CPUID_EXT2_DE | CPUID_EXT2_FPU,
-        .ext3_features = CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A |
+        .ext3_features =
+            CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A |
              CPUID_EXT3_ABM | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
         .xlevel = 0x80000008,
         .model_id = "AMD Opteron 23xx (Gen 3 Class Opteron)",
@@ -823,23 +895,27 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 21,
         .model = 1,
         .stepping = 2,
-        .features = CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
+        .features =
+            CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features = CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
+        .ext_features =
+            CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
              CPUID_EXT_POPCNT | CPUID_EXT_SSE42 | CPUID_EXT_SSE41 |
              CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | CPUID_EXT_PCLMULQDQ |
              CPUID_EXT_SSE3,
-        .ext2_features = CPUID_EXT2_LM | CPUID_EXT2_RDTSCP |
+        .ext2_features =
+            CPUID_EXT2_LM | CPUID_EXT2_RDTSCP |
              CPUID_EXT2_PDPE1GB | CPUID_EXT2_FXSR | CPUID_EXT2_MMX |
              CPUID_EXT2_NX | CPUID_EXT2_PSE36 | CPUID_EXT2_PAT |
              CPUID_EXT2_CMOV | CPUID_EXT2_MCA | CPUID_EXT2_PGE |
              CPUID_EXT2_MTRR | CPUID_EXT2_SYSCALL | CPUID_EXT2_APIC |
              CPUID_EXT2_CX8 | CPUID_EXT2_MCE | CPUID_EXT2_PAE | CPUID_EXT2_MSR |
              CPUID_EXT2_TSC | CPUID_EXT2_PSE | CPUID_EXT2_DE | CPUID_EXT2_FPU,
-        .ext3_features = CPUID_EXT3_FMA4 | CPUID_EXT3_XOP |
+        .ext3_features =
+            CPUID_EXT3_FMA4 | CPUID_EXT3_XOP |
              CPUID_EXT3_3DNOWPREFETCH | CPUID_EXT3_MISALIGNSSE |
              CPUID_EXT3_SSE4A | CPUID_EXT3_ABM | CPUID_EXT3_SVM |
              CPUID_EXT3_LAHF_LM,
@@ -853,23 +929,27 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 21,
         .model = 2,
         .stepping = 0,
-        .features = CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
+        .features =
+            CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features = CPUID_EXT_F16C | CPUID_EXT_AVX | CPUID_EXT_XSAVE |
+        .ext_features =
+            CPUID_EXT_F16C | CPUID_EXT_AVX | CPUID_EXT_XSAVE |
              CPUID_EXT_AES | CPUID_EXT_POPCNT | CPUID_EXT_SSE42 |
              CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_FMA |
              CPUID_EXT_SSSE3 | CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3,
-        .ext2_features = CPUID_EXT2_LM | CPUID_EXT2_RDTSCP |
+        .ext2_features =
+            CPUID_EXT2_LM | CPUID_EXT2_RDTSCP |
              CPUID_EXT2_PDPE1GB | CPUID_EXT2_FXSR | CPUID_EXT2_MMX |
              CPUID_EXT2_NX | CPUID_EXT2_PSE36 | CPUID_EXT2_PAT |
              CPUID_EXT2_CMOV | CPUID_EXT2_MCA | CPUID_EXT2_PGE |
              CPUID_EXT2_MTRR | CPUID_EXT2_SYSCALL | CPUID_EXT2_APIC |
              CPUID_EXT2_CX8 | CPUID_EXT2_MCE | CPUID_EXT2_PAE | CPUID_EXT2_MSR |
              CPUID_EXT2_TSC | CPUID_EXT2_PSE | CPUID_EXT2_DE | CPUID_EXT2_FPU,
-        .ext3_features = CPUID_EXT3_TBM | CPUID_EXT3_FMA4 | CPUID_EXT3_XOP |
+        .ext3_features =
+            CPUID_EXT3_TBM | CPUID_EXT3_FMA4 | CPUID_EXT3_XOP |
              CPUID_EXT3_3DNOWPREFETCH | CPUID_EXT3_MISALIGNSSE |
              CPUID_EXT3_SSE4A | CPUID_EXT3_ABM | CPUID_EXT3_SVM |
              CPUID_EXT3_LAHF_LM,
@@ -918,8 +998,10 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
     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);
+    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 =
@@ -989,21 +1071,29 @@ static int kvm_check_features_against_host(X86CPU *cpu)
     uint32_t mask;
     int rv, i;
     struct model_features_t ft[] = {
-        {&env->cpuid_features, &host_def.features,
+        {&env->cpuid_features,
+            &host_def.features,
             FEAT_1_EDX },
-        {&env->cpuid_ext_features, &host_def.ext_features,
+        {&env->cpuid_ext_features,
+            &host_def.ext_features,
             FEAT_1_ECX },
-        {&env->cpuid_ext2_features, &host_def.ext2_features,
+        {&env->cpuid_ext2_features,
+            &host_def.ext2_features,
             FEAT_8000_0001_EDX },
-        {&env->cpuid_ext3_features, &host_def.ext3_features,
+        {&env->cpuid_ext3_features,
+            &host_def.ext3_features,
             FEAT_8000_0001_ECX },
-        {&env->cpuid_ext4_features, &host_def.ext4_features,
+        {&env->cpuid_ext4_features,
+            &host_def.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_def.cpuid_7_0_ebx_features,
             FEAT_7_0_EBX },
-        {&env->cpuid_svm_features, &host_def.svm_features,
+        {&env->cpuid_svm_features,
+            &host_def.svm_features,
             FEAT_SVM },
-        {&env->cpuid_kvm_features, &host_def.kvm_features,
+        {&env->cpuid_kvm_features,
+            &host_def.kvm_features,
             FEAT_KVM },
     };
 
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu 4/9] target-i386: Replace cpuid_*features fields with a feature word array
  2013-04-22 19:00 [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property Eduardo Habkost
                   ` (2 preceding siblings ...)
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 3/9] target-i386/cpu.c: Break lines so they don't get too long Eduardo Habkost
@ 2013-04-22 19:00 ` Eduardo Habkost
  2013-05-01 23:03   ` Andreas Färber
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 5/9] target-i386: Add ECX information to FeatureWordInfo Eduardo Habkost
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 32+ messages in thread
From: Eduardo Habkost @ 2013-04-22 19:00 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: libvir-list, Igor Mammedov, Jiri Denemark

This replaces the feature-bit fields on both X86CPU and x86_def_t
structs with an array.

With this, we will be able to simplify code that simply does the same
operation on all feature words (e.g. kvm_check_features_against_host(),
filter_features_for_kvm(), add_flagname_to_bitmaps(), CPU feature-bit
property lookup/registration, and the proposed "feature-words" property)

The following field replacements were made on X86CPU and x86_def_t:

  (cpuid_)features         -> features[FEAT_1_EDX]
  (cpuid_)ext_features     -> features[FEAT_1_ECX]
  (cpuid_)ext2_features    -> features[FEAT_8000_0001_EDX]
  (cpuid_)ext3_features    -> features[FEAT_8000_0001_ECX]
  (cpuid_)ext4_features    -> features[FEAT_C000_0001_EDX]
  (cpuid_)kvm_features     -> features[FEAT_KVM]
  (cpuid_)svm_features     -> features[FEAT_SVM]
  (cpuid_)7_0_ebx_features -> features[FEAT_7_0_EBX]

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-By: Igor Mammedov <imammedo@redhat.com>
---
This should help us avoid bugs like the ones introduced when we added
cpuid_7_0_ebx_features. Today, adding a new feature word to the code requires
chaning 5 or 6 different places in the code, and it's very easy to miss a
problem when we forget to update one of those parts. See, for example:

 * The bug solved by commit ffa8c11f0bbf47e1b7a3a62f97bc1da591c6734a;
   (CPUID 7 leaf was not being filtered based on host capabilities)
 * The bug solved by commit 07ca59450c9a0c5df65665ce46aa8487af59a1dd
   (check/enforce flags were not checking all feature flags)

This patch was created solely using a sed script and no manual changes,
to try to avoid mistakes while converting the code, and make it easier
to rebase if necessary. The sed script can be seen at:
  https://gist.github.com/4271991
The script is a lot simpler now, it is basically 8 s/.../.../ commands
and a few insert/delete commands to change the cpu.h and cpu.c structs.

Changes v10:
 * Indent fix on bsd-user/elfload.c

Changes v9:
 * commit message changes: move some text below "---", add table
   showing how each field was replaced

Changes v8:
 * Move line-breaking and code-formatting changes to separate patches
 * Rebase on top of qom-cpu commit b4d31f73
   (qdev: Set device's parent before calling realize() down inheritance chain)

Changes v7:
 * Rebase on top qom-cpu-next
   (commit 3755f0a9d48da07258f4a0ef5e883272799e47b9)

Changes v7:
 * Rebase on top of Andreas' qom-cpu tree (commit
   9260944307077b93a66bf861a467107af986fe47)
 * Break lines on kvm_check_features_against_host()
 * Break the lines on builtin_x86_defs just after the "=".
   This way the feature lists stay on separate lines, this patch gets
   easier to review, and future patches that touches the code around
   builtin_x86_defs will be even easier to review (as they won't need
   to touch the lines containing the fature lists again)

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Conflicts:
	target-i386/kvm.c
---
 bsd-user/elfload.c        |   2 +-
 bsd-user/main.c           |   4 +-
 hw/i386/kvm/clock.c       |   2 +-
 linux-user/elfload.c      |   2 +-
 linux-user/main.c         |   4 +-
 target-i386/cpu.c         | 329 +++++++++++++++++++++++-----------------------
 target-i386/cpu.h         |  11 +-
 target-i386/helper.c      |   4 +-
 target-i386/kvm.c         |   4 +-
 target-i386/misc_helper.c |  14 +-
 target-i386/translate.c   |  10 +-
 11 files changed, 186 insertions(+), 200 deletions(-)

diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
index a6cd3ab..5e20510 100644
--- a/bsd-user/elfload.c
+++ b/bsd-user/elfload.c
@@ -110,7 +110,7 @@ static const char *get_elf_platform(void)
 
 static uint32_t get_elf_hwcap(void)
 {
-  return thread_env->cpuid_features;
+    return thread_env->features[FEAT_1_EDX];
 }
 
 #ifdef TARGET_X86_64
diff --git a/bsd-user/main.c b/bsd-user/main.c
index cc84981..0da3ab9 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -1004,13 +1004,13 @@ int main(int argc, char **argv)
 
     env->cr[0] = CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK;
     env->hflags |= HF_PE_MASK;
-    if (env->cpuid_features & CPUID_SSE) {
+    if (env->features[FEAT_1_EDX] & CPUID_SSE) {
         env->cr[4] |= CR4_OSFXSR_MASK;
         env->hflags |= HF_OSFXSR_MASK;
     }
 #ifndef TARGET_ABI32
     /* enable 64 bit mode if possible */
-    if (!(env->cpuid_ext2_features & CPUID_EXT2_LM)) {
+    if (!(env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM)) {
         fprintf(stderr, "The selected x86 CPU does not support 64 bit mode\n");
         exit(1);
     }
diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
index fa40e28..87d4d0f 100644
--- a/hw/i386/kvm/clock.c
+++ b/hw/i386/kvm/clock.c
@@ -129,7 +129,7 @@ static const TypeInfo kvmclock_info = {
 void kvmclock_create(void)
 {
     if (kvm_enabled() &&
-        first_cpu->cpuid_kvm_features & ((1ULL << KVM_FEATURE_CLOCKSOURCE) |
+        first_cpu->features[FEAT_KVM] & ((1ULL << KVM_FEATURE_CLOCKSOURCE) |
                                          (1ULL << KVM_FEATURE_CLOCKSOURCE2))) {
         sysbus_create_simple("kvmclock", -1, NULL);
     }
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 979b57c..ddef23e 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -137,7 +137,7 @@ static const char *get_elf_platform(void)
 
 static uint32_t get_elf_hwcap(void)
 {
-    return thread_env->cpuid_features;
+    return thread_env->features[FEAT_1_EDX];
 }
 
 #ifdef TARGET_X86_64
diff --git a/linux-user/main.c b/linux-user/main.c
index 4e92a0b..b97b8cf 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3764,13 +3764,13 @@ int main(int argc, char **argv, char **envp)
 
     env->cr[0] = CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK;
     env->hflags |= HF_PE_MASK;
-    if (env->cpuid_features & CPUID_SSE) {
+    if (env->features[FEAT_1_EDX] & CPUID_SSE) {
         env->cr[4] |= CR4_OSFXSR_MASK;
         env->hflags |= HF_OSFXSR_MASK;
     }
 #ifndef TARGET_ABI32
     /* enable 64 bit mode if possible */
-    if (!(env->cpuid_ext2_features & CPUID_EXT2_LM)) {
+    if (!(env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM)) {
         fprintf(stderr, "The selected x86 CPU does not support 64 bit mode\n");
         exit(1);
     }
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 73ae2ef..110ef98 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -356,13 +356,8 @@ typedef struct x86_def_t {
     int family;
     int model;
     int stepping;
-    uint32_t features, ext_features, ext2_features, ext3_features;
-    uint32_t kvm_features, svm_features;
+    FeatureWordArray features;
     char model_id[48];
-    /* Store the results of Centaur's CPUID instructions */
-    uint32_t ext4_features;
-    /* 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)
@@ -423,16 +418,16 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 2,
         .stepping = 3,
-        .features =
+        .features[FEAT_1_EDX] =
             PPRO_FEATURES |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
             CPUID_PSE36,
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_SSE3 | CPUID_EXT_CX16 | CPUID_EXT_POPCNT,
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
             CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
-        .ext3_features =
+        .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
             CPUID_EXT3_ABM | CPUID_EXT3_SSE4A,
         .xlevel = 0x8000000A,
@@ -444,14 +439,14 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 16,
         .model = 2,
         .stepping = 3,
-        .features =
+        .features[FEAT_1_EDX] =
             PPRO_FEATURES |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
             CPUID_PSE36 | CPUID_VME | CPUID_HT,
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_CX16 |
             CPUID_EXT_POPCNT,
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
             CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX |
             CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT | CPUID_EXT2_MMXEXT |
@@ -460,10 +455,10 @@ static x86_def_t builtin_x86_defs[] = {
                     CPUID_EXT3_CR8LEG,
                     CPUID_EXT3_MISALIGNSSE, CPUID_EXT3_3DNOWPREFETCH,
                     CPUID_EXT3_OSVW, CPUID_EXT3_IBS */
-        .ext3_features =
+        .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
             CPUID_EXT3_ABM | CPUID_EXT3_SSE4A,
-        .svm_features =
+        .features[FEAT_SVM] =
             CPUID_SVM_NPT | CPUID_SVM_LBRV,
         .xlevel = 0x8000001A,
         .model_id = "AMD Phenom(tm) 9550 Quad-Core Processor"
@@ -475,18 +470,18 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 15,
         .stepping = 11,
-        .features =
+        .features[FEAT_1_EDX] =
             PPRO_FEATURES |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
             CPUID_PSE36 | CPUID_VME | CPUID_DTS | CPUID_ACPI | CPUID_SS |
             CPUID_HT | CPUID_TM | CPUID_PBE,
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 |
             CPUID_EXT_DTES64 | CPUID_EXT_DSCPL | CPUID_EXT_VMX | CPUID_EXT_EST |
             CPUID_EXT_TM2 | CPUID_EXT_CX16 | CPUID_EXT_XTPR | CPUID_EXT_PDCM,
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
-        .ext3_features =
+        .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_LAHF_LM,
         .xlevel = 0x80000008,
         .model_id = "Intel(R) Core(TM)2 Duo CPU     T7700  @ 2.40GHz",
@@ -499,22 +494,22 @@ static x86_def_t builtin_x86_defs[] = {
         .model = 6,
         .stepping = 1,
         /* Missing: CPUID_VME, CPUID_HT */
-        .features =
+        .features[FEAT_1_EDX] =
             PPRO_FEATURES |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
             CPUID_PSE36,
         /* Missing: CPUID_EXT_POPCNT, CPUID_EXT_MONITOR */
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_SSE3 | CPUID_EXT_CX16,
         /* Missing: CPUID_EXT2_PDPE1GB, CPUID_EXT2_RDTSCP */
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
             CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
         /* Missing: CPUID_EXT3_LAHF_LM, CPUID_EXT3_CMP_LEG, CPUID_EXT3_EXTAPIC,
                     CPUID_EXT3_CR8LEG, CPUID_EXT3_ABM, CPUID_EXT3_SSE4A,
                     CPUID_EXT3_MISALIGNSSE, CPUID_EXT3_3DNOWPREFETCH,
                     CPUID_EXT3_OSVW, CPUID_EXT3_IBS, CPUID_EXT3_SVM */
-        .ext3_features =
+        .features[FEAT_8000_0001_ECX] =
             0,
         .xlevel = 0x80000008,
         .model_id = "Common KVM processor"
@@ -526,9 +521,9 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 3,
         .stepping = 3,
-        .features =
+        .features[FEAT_1_EDX] =
             PPRO_FEATURES,
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_SSE3 | CPUID_EXT_POPCNT,
         .xlevel = 0x80000004,
     },
@@ -539,14 +534,14 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 15,
         .model = 6,
         .stepping = 1,
-        .features =
+        .features[FEAT_1_EDX] =
             PPRO_FEATURES |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_PSE36,
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_SSE3,
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES,
-        .ext3_features =
+        .features[FEAT_8000_0001_ECX] =
             0,
         .xlevel = 0x80000008,
         .model_id = "Common 32-bit KVM processor"
@@ -558,14 +553,14 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 14,
         .stepping = 8,
-        .features =
+        .features[FEAT_1_EDX] =
             PPRO_FEATURES | CPUID_VME |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_DTS | CPUID_ACPI |
             CPUID_SS | CPUID_HT | CPUID_TM | CPUID_PBE,
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_VMX |
             CPUID_EXT_EST | CPUID_EXT_TM2 | CPUID_EXT_XTPR | CPUID_EXT_PDCM,
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_NX,
         .xlevel = 0x80000008,
         .model_id = "Genuine Intel(R) CPU           T2600  @ 2.16GHz",
@@ -577,7 +572,7 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 4,
         .model = 0,
         .stepping = 0,
-        .features =
+        .features[FEAT_1_EDX] =
             I486_FEATURES,
         .xlevel = 0,
     },
@@ -588,7 +583,7 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 5,
         .model = 4,
         .stepping = 3,
-        .features =
+        .features[FEAT_1_EDX] =
             PENTIUM_FEATURES,
         .xlevel = 0,
     },
@@ -599,7 +594,7 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 5,
         .stepping = 2,
-        .features =
+        .features[FEAT_1_EDX] =
             PENTIUM2_FEATURES,
         .xlevel = 0,
     },
@@ -610,7 +605,7 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 7,
         .stepping = 3,
-        .features =
+        .features[FEAT_1_EDX] =
             PENTIUM3_FEATURES,
         .xlevel = 0,
     },
@@ -621,10 +616,10 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 2,
         .stepping = 3,
-        .features =
+        .features[FEAT_1_EDX] =
             PPRO_FEATURES | CPUID_PSE36 | CPUID_VME | CPUID_MTRR |
             CPUID_MCA,
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
             CPUID_EXT2_MMXEXT | CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT,
         .xlevel = 0x80000008,
@@ -637,18 +632,18 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 28,
         .stepping = 2,
-        .features =
+        .features[FEAT_1_EDX] =
             PPRO_FEATURES |
             CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_VME | CPUID_DTS |
             CPUID_ACPI | CPUID_SS | CPUID_HT | CPUID_TM | CPUID_PBE,
             /* Some CPUs got no CPUID_SEP */
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 |
             CPUID_EXT_DSCPL | CPUID_EXT_EST | CPUID_EXT_TM2 | CPUID_EXT_XTPR,
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             (PPRO_FEATURES & CPUID_EXT2_AMD_ALIASES) |
             CPUID_EXT2_NX,
-        .ext3_features =
+        .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Intel(R) Atom(TM) CPU N270   @ 1.60GHz",
@@ -660,17 +655,17 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 2,
         .stepping = 3,
-        .features =
+        .features[FEAT_1_EDX] =
             CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_SSSE3 | CPUID_EXT_SSE3,
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
-        .ext3_features =
+        .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Intel Celeron_4x0 (Conroe/Merom Class Core 2)",
@@ -682,18 +677,18 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 2,
         .stepping = 3,
-        .features =
+        .features[FEAT_1_EDX] =
             CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 |
              CPUID_EXT_SSE3,
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
-        .ext3_features =
+        .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Intel Core 2 Duo P9xxx (Penryn Class Core 2)",
@@ -705,18 +700,18 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 2,
         .stepping = 3,
-        .features =
+        .features[FEAT_1_EDX] =
             CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_POPCNT | CPUID_EXT_SSE42 | CPUID_EXT_SSE41 |
              CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | CPUID_EXT_SSE3,
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
-        .ext3_features =
+        .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Intel Core i7 9xx (Nehalem Class Core i7)",
@@ -728,19 +723,19 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 44,
         .stepping = 1,
-        .features =
+        .features[FEAT_1_EDX] =
             CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_AES | CPUID_EXT_POPCNT | CPUID_EXT_SSE42 |
              CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 |
              CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3,
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
-        .ext3_features =
+        .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Westmere E56xx/L56xx/X56xx (Nehalem-C)",
@@ -752,22 +747,22 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 42,
         .stepping = 1,
-        .features =
+        .features[FEAT_1_EDX] =
             CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
              CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_POPCNT |
              CPUID_EXT_X2APIC | CPUID_EXT_SSE42 | CPUID_EXT_SSE41 |
              CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | CPUID_EXT_PCLMULQDQ |
              CPUID_EXT_SSE3,
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_NX |
              CPUID_EXT2_SYSCALL,
-        .ext3_features =
+        .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_LAHF_LM,
         .xlevel = 0x8000000A,
         .model_id = "Intel Xeon E312xx (Sandy Bridge)",
@@ -779,25 +774,25 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 6,
         .model = 60,
         .stepping = 1,
-        .features =
+        .features[FEAT_1_EDX] =
             CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
              CPUID_EXT_POPCNT | CPUID_EXT_X2APIC | CPUID_EXT_SSE42 |
              CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 |
              CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3 |
              CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_FMA | CPUID_EXT_MOVBE |
              CPUID_EXT_PCID,
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_NX |
              CPUID_EXT2_SYSCALL,
-        .ext3_features =
+        .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_LAHF_LM,
-        .cpuid_7_0_ebx_features =
+        .features[FEAT_7_0_EBX] =
             CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 |
             CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP |
             CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID |
@@ -812,15 +807,15 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 15,
         .model = 6,
         .stepping = 1,
-        .features =
+        .features[FEAT_1_EDX] =
             CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_SSE3,
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_LM | CPUID_EXT2_FXSR | CPUID_EXT2_MMX |
              CPUID_EXT2_NX | CPUID_EXT2_PSE36 | CPUID_EXT2_PAT |
              CPUID_EXT2_CMOV | CPUID_EXT2_MCA | CPUID_EXT2_PGE |
@@ -837,15 +832,15 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 15,
         .model = 6,
         .stepping = 1,
-        .features =
+        .features[FEAT_1_EDX] =
             CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_CX16 | CPUID_EXT_SSE3,
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_FXSR |
              CPUID_EXT2_MMX | CPUID_EXT2_NX | CPUID_EXT2_PSE36 |
              CPUID_EXT2_PAT | CPUID_EXT2_CMOV | CPUID_EXT2_MCA |
@@ -853,7 +848,7 @@ static x86_def_t builtin_x86_defs[] = {
              CPUID_EXT2_APIC | CPUID_EXT2_CX8 | CPUID_EXT2_MCE |
              CPUID_EXT2_PAE | CPUID_EXT2_MSR | CPUID_EXT2_TSC | CPUID_EXT2_PSE |
              CPUID_EXT2_DE | CPUID_EXT2_FPU,
-        .ext3_features =
+        .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
         .xlevel = 0x80000008,
         .model_id = "AMD Opteron 22xx (Gen 2 Class Opteron)",
@@ -865,16 +860,16 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 15,
         .model = 6,
         .stepping = 1,
-        .features =
+        .features[FEAT_1_EDX] =
             CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_POPCNT | CPUID_EXT_CX16 | CPUID_EXT_MONITOR |
              CPUID_EXT_SSE3,
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_FXSR |
              CPUID_EXT2_MMX | CPUID_EXT2_NX | CPUID_EXT2_PSE36 |
              CPUID_EXT2_PAT | CPUID_EXT2_CMOV | CPUID_EXT2_MCA |
@@ -882,7 +877,7 @@ static x86_def_t builtin_x86_defs[] = {
              CPUID_EXT2_APIC | CPUID_EXT2_CX8 | CPUID_EXT2_MCE |
              CPUID_EXT2_PAE | CPUID_EXT2_MSR | CPUID_EXT2_TSC | CPUID_EXT2_PSE |
              CPUID_EXT2_DE | CPUID_EXT2_FPU,
-        .ext3_features =
+        .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A |
              CPUID_EXT3_ABM | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
         .xlevel = 0x80000008,
@@ -895,18 +890,18 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 21,
         .model = 1,
         .stepping = 2,
-        .features =
+        .features[FEAT_1_EDX] =
             CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
              CPUID_EXT_POPCNT | CPUID_EXT_SSE42 | CPUID_EXT_SSE41 |
              CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | CPUID_EXT_PCLMULQDQ |
              CPUID_EXT_SSE3,
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_LM | CPUID_EXT2_RDTSCP |
              CPUID_EXT2_PDPE1GB | CPUID_EXT2_FXSR | CPUID_EXT2_MMX |
              CPUID_EXT2_NX | CPUID_EXT2_PSE36 | CPUID_EXT2_PAT |
@@ -914,7 +909,7 @@ static x86_def_t builtin_x86_defs[] = {
              CPUID_EXT2_MTRR | CPUID_EXT2_SYSCALL | CPUID_EXT2_APIC |
              CPUID_EXT2_CX8 | CPUID_EXT2_MCE | CPUID_EXT2_PAE | CPUID_EXT2_MSR |
              CPUID_EXT2_TSC | CPUID_EXT2_PSE | CPUID_EXT2_DE | CPUID_EXT2_FPU,
-        .ext3_features =
+        .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_FMA4 | CPUID_EXT3_XOP |
              CPUID_EXT3_3DNOWPREFETCH | CPUID_EXT3_MISALIGNSSE |
              CPUID_EXT3_SSE4A | CPUID_EXT3_ABM | CPUID_EXT3_SVM |
@@ -929,18 +924,18 @@ static x86_def_t builtin_x86_defs[] = {
         .family = 21,
         .model = 2,
         .stepping = 0,
-        .features =
+        .features[FEAT_1_EDX] =
             CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
              CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
              CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
              CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
              CPUID_DE | CPUID_FP87,
-        .ext_features =
+        .features[FEAT_1_ECX] =
             CPUID_EXT_F16C | CPUID_EXT_AVX | CPUID_EXT_XSAVE |
              CPUID_EXT_AES | CPUID_EXT_POPCNT | CPUID_EXT_SSE42 |
              CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_FMA |
              CPUID_EXT_SSSE3 | CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3,
-        .ext2_features =
+        .features[FEAT_8000_0001_EDX] =
             CPUID_EXT2_LM | CPUID_EXT2_RDTSCP |
              CPUID_EXT2_PDPE1GB | CPUID_EXT2_FXSR | CPUID_EXT2_MMX |
              CPUID_EXT2_NX | CPUID_EXT2_PSE36 | CPUID_EXT2_PAT |
@@ -948,7 +943,7 @@ static x86_def_t builtin_x86_defs[] = {
              CPUID_EXT2_MTRR | CPUID_EXT2_SYSCALL | CPUID_EXT2_APIC |
              CPUID_EXT2_CX8 | CPUID_EXT2_MCE | CPUID_EXT2_PAE | CPUID_EXT2_MSR |
              CPUID_EXT2_TSC | CPUID_EXT2_PSE | CPUID_EXT2_DE | CPUID_EXT2_FPU,
-        .ext3_features =
+        .features[FEAT_8000_0001_ECX] =
             CPUID_EXT3_TBM | CPUID_EXT3_FMA4 | CPUID_EXT3_XOP |
              CPUID_EXT3_3DNOWPREFETCH | CPUID_EXT3_MISALIGNSSE |
              CPUID_EXT3_SSE4A | CPUID_EXT3_ABM | CPUID_EXT3_SVM |
@@ -998,22 +993,22 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
     x86_cpu_def->stepping = eax & 0x0F;
 
     x86_cpu_def->level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
-    x86_cpu_def->features =
+    x86_cpu_def->features[FEAT_1_EDX] =
         kvm_arch_get_supported_cpuid(s, 0x1, 0, R_EDX);
-    x86_cpu_def->ext_features =
+    x86_cpu_def->features[FEAT_1_ECX] =
         kvm_arch_get_supported_cpuid(s, 0x1, 0, R_ECX);
 
     if (x86_cpu_def->level >= 7) {
-        x86_cpu_def->cpuid_7_0_ebx_features =
+        x86_cpu_def->features[FEAT_7_0_EBX] =
                     kvm_arch_get_supported_cpuid(s, 0x7, 0, R_EBX);
     } else {
-        x86_cpu_def->cpuid_7_0_ebx_features = 0;
+        x86_cpu_def->features[FEAT_7_0_EBX] = 0;
     }
 
     x86_cpu_def->xlevel = kvm_arch_get_supported_cpuid(s, 0x80000000, 0, R_EAX);
-    x86_cpu_def->ext2_features =
+    x86_cpu_def->features[FEAT_8000_0001_EDX] =
                 kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX);
-    x86_cpu_def->ext3_features =
+    x86_cpu_def->features[FEAT_8000_0001_ECX] =
                 kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_ECX);
 
     cpu_x86_fill_model_id(x86_cpu_def->model_id);
@@ -1026,15 +1021,15 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
             /* Support VIA max extended level */
             x86_cpu_def->xlevel2 = eax;
             host_cpuid(0xC0000001, 0, &eax, &ebx, &ecx, &edx);
-            x86_cpu_def->ext4_features =
+            x86_cpu_def->features[FEAT_C000_0001_EDX] =
                     kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
         }
     }
 
     /* Other KVM-specific feature fields: */
-    x86_cpu_def->svm_features =
+    x86_cpu_def->features[FEAT_SVM] =
         kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX);
-    x86_cpu_def->kvm_features =
+    x86_cpu_def->features[FEAT_KVM] =
         kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX);
 
 #endif /* CONFIG_KVM */
@@ -1071,29 +1066,29 @@ static int kvm_check_features_against_host(X86CPU *cpu)
     uint32_t mask;
     int rv, i;
     struct model_features_t ft[] = {
-        {&env->cpuid_features,
-            &host_def.features,
+        {&env->features[FEAT_1_EDX],
+            &host_def.features[FEAT_1_EDX],
             FEAT_1_EDX },
-        {&env->cpuid_ext_features,
-            &host_def.ext_features,
+        {&env->features[FEAT_1_ECX],
+            &host_def.features[FEAT_1_ECX],
             FEAT_1_ECX },
-        {&env->cpuid_ext2_features,
-            &host_def.ext2_features,
+        {&env->features[FEAT_8000_0001_EDX],
+            &host_def.features[FEAT_8000_0001_EDX],
             FEAT_8000_0001_EDX },
-        {&env->cpuid_ext3_features,
-            &host_def.ext3_features,
+        {&env->features[FEAT_8000_0001_ECX],
+            &host_def.features[FEAT_8000_0001_ECX],
             FEAT_8000_0001_ECX },
-        {&env->cpuid_ext4_features,
-            &host_def.ext4_features,
+        {&env->features[FEAT_C000_0001_EDX],
+            &host_def.features[FEAT_C000_0001_EDX],
             FEAT_C000_0001_EDX },
-        {&env->cpuid_7_0_ebx_features,
-            &host_def.cpuid_7_0_ebx_features,
+        {&env->features[FEAT_7_0_EBX],
+            &host_def.features[FEAT_7_0_EBX],
             FEAT_7_0_EBX },
-        {&env->cpuid_svm_features,
-            &host_def.svm_features,
+        {&env->features[FEAT_SVM],
+            &host_def.features[FEAT_SVM],
             FEAT_SVM },
-        {&env->cpuid_kvm_features,
-            &host_def.kvm_features,
+        {&env->features[FEAT_KVM],
+            &host_def.features[FEAT_KVM],
             FEAT_KVM },
     };
 
@@ -1490,22 +1485,22 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
         }
         featurestr = strtok(NULL, ",");
     }
-    env->cpuid_features |= plus_features[FEAT_1_EDX];
-    env->cpuid_ext_features |= plus_features[FEAT_1_ECX];
-    env->cpuid_ext2_features |= plus_features[FEAT_8000_0001_EDX];
-    env->cpuid_ext3_features |= plus_features[FEAT_8000_0001_ECX];
-    env->cpuid_ext4_features |= plus_features[FEAT_C000_0001_EDX];
-    env->cpuid_kvm_features |= plus_features[FEAT_KVM];
-    env->cpuid_svm_features |= plus_features[FEAT_SVM];
-    env->cpuid_7_0_ebx_features |= plus_features[FEAT_7_0_EBX];
-    env->cpuid_features &= ~minus_features[FEAT_1_EDX];
-    env->cpuid_ext_features &= ~minus_features[FEAT_1_ECX];
-    env->cpuid_ext2_features &= ~minus_features[FEAT_8000_0001_EDX];
-    env->cpuid_ext3_features &= ~minus_features[FEAT_8000_0001_ECX];
-    env->cpuid_ext4_features &= ~minus_features[FEAT_C000_0001_EDX];
-    env->cpuid_kvm_features &= ~minus_features[FEAT_KVM];
-    env->cpuid_svm_features &= ~minus_features[FEAT_SVM];
-    env->cpuid_7_0_ebx_features &= ~minus_features[FEAT_7_0_EBX];
+    env->features[FEAT_1_EDX] |= plus_features[FEAT_1_EDX];
+    env->features[FEAT_1_ECX] |= plus_features[FEAT_1_ECX];
+    env->features[FEAT_8000_0001_EDX] |= plus_features[FEAT_8000_0001_EDX];
+    env->features[FEAT_8000_0001_ECX] |= plus_features[FEAT_8000_0001_ECX];
+    env->features[FEAT_C000_0001_EDX] |= plus_features[FEAT_C000_0001_EDX];
+    env->features[FEAT_KVM] |= plus_features[FEAT_KVM];
+    env->features[FEAT_SVM] |= plus_features[FEAT_SVM];
+    env->features[FEAT_7_0_EBX] |= plus_features[FEAT_7_0_EBX];
+    env->features[FEAT_1_EDX] &= ~minus_features[FEAT_1_EDX];
+    env->features[FEAT_1_ECX] &= ~minus_features[FEAT_1_ECX];
+    env->features[FEAT_8000_0001_EDX] &= ~minus_features[FEAT_8000_0001_EDX];
+    env->features[FEAT_8000_0001_ECX] &= ~minus_features[FEAT_8000_0001_ECX];
+    env->features[FEAT_C000_0001_EDX] &= ~minus_features[FEAT_C000_0001_EDX];
+    env->features[FEAT_KVM] &= ~minus_features[FEAT_KVM];
+    env->features[FEAT_SVM] &= ~minus_features[FEAT_SVM];
+    env->features[FEAT_7_0_EBX] &= ~minus_features[FEAT_7_0_EBX];
 
 out:
     return;
@@ -1597,21 +1592,21 @@ static void filter_features_for_kvm(X86CPU *cpu)
     CPUX86State *env = &cpu->env;
     KVMState *s = kvm_state;
 
-    env->cpuid_features &=
+    env->features[FEAT_1_EDX] &=
         kvm_arch_get_supported_cpuid(s, 1, 0, R_EDX);
-    env->cpuid_ext_features &=
+    env->features[FEAT_1_ECX] &=
         kvm_arch_get_supported_cpuid(s, 1, 0, R_ECX);
-    env->cpuid_ext2_features &=
+    env->features[FEAT_8000_0001_EDX] &=
         kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX);
-    env->cpuid_ext3_features &=
+    env->features[FEAT_8000_0001_ECX] &=
         kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_ECX);
-    env->cpuid_svm_features  &=
+    env->features[FEAT_SVM]  &=
         kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX);
-    env->cpuid_7_0_ebx_features &=
+    env->features[FEAT_7_0_EBX] &=
         kvm_arch_get_supported_cpuid(s, 7, 0, R_EBX);
-    env->cpuid_kvm_features &=
+    env->features[FEAT_KVM] &=
         kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX);
-    env->cpuid_ext4_features &=
+    env->features[FEAT_C000_0001_EDX] &=
         kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
 
 }
@@ -1630,24 +1625,24 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
     }
 
     if (kvm_enabled()) {
-        def->kvm_features |= kvm_default_features;
+        def->features[FEAT_KVM] |= kvm_default_features;
     }
-    def->ext_features |= CPUID_EXT_HYPERVISOR;
+    def->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
 
     object_property_set_str(OBJECT(cpu), def->vendor, "vendor", errp);
     object_property_set_int(OBJECT(cpu), def->level, "level", errp);
     object_property_set_int(OBJECT(cpu), def->family, "family", errp);
     object_property_set_int(OBJECT(cpu), def->model, "model", errp);
     object_property_set_int(OBJECT(cpu), def->stepping, "stepping", errp);
-    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;
+    env->features[FEAT_1_EDX] = def->features[FEAT_1_EDX];
+    env->features[FEAT_1_ECX] = def->features[FEAT_1_ECX];
+    env->features[FEAT_8000_0001_EDX] = def->features[FEAT_8000_0001_EDX];
+    env->features[FEAT_8000_0001_ECX] = def->features[FEAT_8000_0001_ECX];
     object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", errp);
-    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->features[FEAT_KVM] = def->features[FEAT_KVM];
+    env->features[FEAT_SVM] = def->features[FEAT_SVM];
+    env->features[FEAT_C000_0001_EDX] = def->features[FEAT_C000_0001_EDX];
+    env->features[FEAT_7_0_EBX] = def->features[FEAT_7_0_EBX];
     env->cpuid_xlevel2 = def->xlevel2;
 
     object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
@@ -1717,7 +1712,7 @@ out:
 
 void cpu_clear_apic_feature(CPUX86State *env)
 {
-    env->cpuid_features &= ~CPUID_APIC;
+    env->features[FEAT_1_EDX] &= ~CPUID_APIC;
 }
 
 #endif /* !CONFIG_USER_ONLY */
@@ -1792,8 +1787,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
     case 1:
         *eax = env->cpuid_version;
         *ebx = (env->cpuid_apic_id << 24) | 8 << 8; /* CLFLUSH size in quad words, Linux wants it. */
-        *ecx = env->cpuid_ext_features;
-        *edx = env->cpuid_features;
+        *ecx = env->features[FEAT_1_ECX];
+        *edx = env->features[FEAT_1_EDX];
         if (cs->nr_cores * cs->nr_threads > 1) {
             *ebx |= (cs->nr_cores * cs->nr_threads) << 16;
             *edx |= 1 << 28;    /* HTT bit */
@@ -1861,7 +1856,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         /* Structured Extended Feature Flags Enumeration Leaf */
         if (count == 0) {
             *eax = 0; /* Maximum ECX value for sub-leaves */
-            *ebx = env->cpuid_7_0_ebx_features; /* Feature flags */
+            *ebx = env->features[FEAT_7_0_EBX]; /* Feature flags */
             *ecx = 0; /* Reserved */
             *edx = 0; /* Reserved */
         } else {
@@ -1896,7 +1891,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         break;
     case 0xD:
         /* Processor Extended State */
-        if (!(env->cpuid_ext_features & CPUID_EXT_XSAVE)) {
+        if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) {
             *eax = 0;
             *ebx = 0;
             *ecx = 0;
@@ -1926,8 +1921,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
     case 0x80000001:
         *eax = env->cpuid_version;
         *ebx = 0;
-        *ecx = env->cpuid_ext3_features;
-        *edx = env->cpuid_ext2_features;
+        *ecx = env->features[FEAT_8000_0001_ECX];
+        *edx = env->features[FEAT_8000_0001_EDX];
 
         /* The Linux kernel checks for the CMPLegacy bit and
          * discards multiple thread information if it is set.
@@ -1968,12 +1963,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
     case 0x80000008:
         /* virtual & phys address size in low 2 bytes. */
 /* XXX: This value must match the one used in the MMU code. */
-        if (env->cpuid_ext2_features & CPUID_EXT2_LM) {
+        if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) {
             /* 64 bit processor */
 /* XXX: The physical address space is limited to 42 bits in exec.c. */
             *eax = 0x00003028; /* 48 bits virtual, 40 bits physical */
         } else {
-            if (env->cpuid_features & CPUID_PSE36) {
+            if (env->features[FEAT_1_EDX] & CPUID_PSE36) {
                 *eax = 0x00000024; /* 36 bits physical */
             } else {
                 *eax = 0x00000020; /* 32 bits physical */
@@ -1987,11 +1982,11 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         }
         break;
     case 0x8000000A:
-        if (env->cpuid_ext3_features & CPUID_EXT3_SVM) {
+        if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_SVM) {
             *eax = 0x00000001; /* SVM Revision */
             *ebx = 0x00000010; /* nr of ASIDs */
             *ecx = 0;
-            *edx = env->cpuid_svm_features; /* optional features */
+            *edx = env->features[FEAT_SVM]; /* optional features */
         } else {
             *eax = 0;
             *ebx = 0;
@@ -2010,7 +2005,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *eax = env->cpuid_version;
         *ebx = 0;
         *ecx = 0;
-        *edx = env->cpuid_ext4_features;
+        *edx = env->features[FEAT_C000_0001_EDX];
         break;
     case 0xC0000002:
     case 0xC0000003:
@@ -2142,7 +2137,7 @@ static void mce_init(X86CPU *cpu)
     unsigned int bank;
 
     if (((cenv->cpuid_version >> 8) & 0xf) >= 6
-        && (cenv->cpuid_features & (CPUID_MCE | CPUID_MCA)) ==
+        && (cenv->features[FEAT_1_EDX] & (CPUID_MCE | CPUID_MCA)) ==
             (CPUID_MCE | CPUID_MCA)) {
         cenv->mcg_cap = MCE_CAP_DEF | MCE_BANKS_DEF;
         cenv->mcg_ctl = ~(uint64_t)0;
@@ -2217,7 +2212,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
     CPUX86State *env = &cpu->env;
     Error *local_err = NULL;
 
-    if (env->cpuid_7_0_ebx_features && env->cpuid_level < 7) {
+    if (env->features[FEAT_7_0_EBX] && env->cpuid_level < 7) {
         env->cpuid_level = 7;
     }
 
@@ -2227,21 +2222,21 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
     if (env->cpuid_vendor1 == CPUID_VENDOR_AMD_1 &&
         env->cpuid_vendor2 == CPUID_VENDOR_AMD_2 &&
         env->cpuid_vendor3 == CPUID_VENDOR_AMD_3) {
-        env->cpuid_ext2_features &= ~CPUID_EXT2_AMD_ALIASES;
-        env->cpuid_ext2_features |= (env->cpuid_features
+        env->features[FEAT_8000_0001_EDX] &= ~CPUID_EXT2_AMD_ALIASES;
+        env->features[FEAT_8000_0001_EDX] |= (env->features[FEAT_1_EDX]
            & CPUID_EXT2_AMD_ALIASES);
     }
 
     if (!kvm_enabled()) {
-        env->cpuid_features &= TCG_FEATURES;
-        env->cpuid_ext_features &= TCG_EXT_FEATURES;
-        env->cpuid_ext2_features &= (TCG_EXT2_FEATURES
+        env->features[FEAT_1_EDX] &= TCG_FEATURES;
+        env->features[FEAT_1_ECX] &= TCG_EXT_FEATURES;
+        env->features[FEAT_8000_0001_EDX] &= (TCG_EXT2_FEATURES
 #ifdef TARGET_X86_64
             | CPUID_EXT2_SYSCALL | CPUID_EXT2_LM
 #endif
             );
-        env->cpuid_ext3_features &= TCG_EXT3_FEATURES;
-        env->cpuid_svm_features &= TCG_SVM_FEATURES;
+        env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES;
+        env->features[FEAT_SVM] &= TCG_SVM_FEATURES;
     } else {
         if (check_cpuid && kvm_check_features_against_host(cpu)
             && enforce_cpuid) {
@@ -2257,7 +2252,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
 #ifndef CONFIG_USER_ONLY
     qemu_register_reset(x86_cpu_machine_reset_cb, cpu);
 
-    if (cpu->env.cpuid_features & CPUID_APIC || smp_cpus > 1) {
+    if (cpu->env.features[FEAT_1_EDX] & CPUID_APIC || smp_cpus > 1) {
         x86_cpu_apic_create(cpu, &local_err);
         if (local_err != NULL) {
             goto out;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index c621359..aca78fc 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -842,16 +842,9 @@ typedef struct CPUX86State {
     uint32_t cpuid_vendor2;
     uint32_t cpuid_vendor3;
     uint32_t cpuid_version;
-    uint32_t cpuid_features;
-    uint32_t cpuid_ext_features;
+    FeatureWordArray features;
     uint32_t cpuid_model[12];
-    uint32_t cpuid_ext2_features;
-    uint32_t cpuid_ext3_features;
     uint32_t cpuid_apic_id;
-    /* Store the results of Centaur's CPUID instructions */
-    uint32_t cpuid_ext4_features;
-    /* Flags from CPUID[EAX=7,ECX=0].EBX */
-    uint32_t cpuid_7_0_ebx_features;
 
     /* MTRRs */
     uint64_t mtrr_fixed[11];
@@ -865,8 +858,6 @@ typedef struct CPUX86State {
     uint8_t soft_interrupt;
     uint8_t has_error_code;
     uint32_t sipi_vector;
-    uint32_t cpuid_kvm_features;
-    uint32_t cpuid_svm_features;
     bool tsc_valid;
     int tsc_khz;
     void *kvm_xsave_buf;
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 282494f..158710a 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -463,7 +463,7 @@ void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4)
         tlb_flush(env, 1);
     }
     /* SSE handling */
-    if (!(env->cpuid_features & CPUID_SSE)) {
+    if (!(env->features[FEAT_1_EDX] & CPUID_SSE)) {
         new_cr4 &= ~CR4_OSFXSR_MASK;
     }
     env->hflags &= ~HF_OSFXSR_MASK;
@@ -471,7 +471,7 @@ void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4)
         env->hflags |= HF_OSFXSR_MASK;
     }
 
-    if (!(env->cpuid_7_0_ebx_features & CPUID_7_0_EBX_SMAP)) {
+    if (!(env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_SMAP)) {
         new_cr4 &= ~CR4_SMAP_MASK;
     }
     env->hflags &= ~HF_SMAP_MASK;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index b5bff33..77f1156 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -454,7 +454,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
     c = &cpuid_data.entries[cpuid_i++];
     memset(c, 0, sizeof(*c));
     c->function = KVM_CPUID_FEATURES;
-    c->eax = env->cpuid_kvm_features;
+    c->eax = env->features[FEAT_KVM];
 
     if (hyperv_enabled()) {
         memcpy(signature, "Hv#1\0\0\0\0\0\0\0\0", 12);
@@ -613,7 +613,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
     cpuid_data.cpuid.nent = cpuid_i;
 
     if (((env->cpuid_version >> 8)&0xF) >= 6
-        && (env->cpuid_features & (CPUID_MCE|CPUID_MCA)) ==
+        && (env->features[FEAT_1_EDX] & (CPUID_MCE|CPUID_MCA)) ==
            (CPUID_MCE|CPUID_MCA)
         && kvm_check_extension(cs->kvm_state, KVM_CAP_MCE) > 0) {
         uint64_t mcg_cap;
diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
index dfbc07b..ec834fc 100644
--- a/target-i386/misc_helper.c
+++ b/target-i386/misc_helper.c
@@ -291,22 +291,22 @@ void helper_wrmsr(CPUX86State *env)
             uint64_t update_mask;
 
             update_mask = 0;
-            if (env->cpuid_ext2_features & CPUID_EXT2_SYSCALL) {
+            if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_SYSCALL) {
                 update_mask |= MSR_EFER_SCE;
             }
-            if (env->cpuid_ext2_features & CPUID_EXT2_LM) {
+            if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) {
                 update_mask |= MSR_EFER_LME;
             }
-            if (env->cpuid_ext2_features & CPUID_EXT2_FFXSR) {
+            if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_FFXSR) {
                 update_mask |= MSR_EFER_FFXSR;
             }
-            if (env->cpuid_ext2_features & CPUID_EXT2_NX) {
+            if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_NX) {
                 update_mask |= MSR_EFER_NXE;
             }
-            if (env->cpuid_ext3_features & CPUID_EXT3_SVM) {
+            if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_SVM) {
                 update_mask |= MSR_EFER_SVME;
             }
-            if (env->cpuid_ext2_features & CPUID_EXT2_FFXSR) {
+            if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_FFXSR) {
                 update_mask |= MSR_EFER_FFXSR;
             }
             cpu_load_efer(env, (env->efer & ~update_mask) |
@@ -513,7 +513,7 @@ void helper_rdmsr(CPUX86State *env)
         val = env->mtrr_deftype;
         break;
     case MSR_MTRRcap:
-        if (env->cpuid_features & CPUID_MTRR) {
+        if (env->features[FEAT_1_EDX] & CPUID_MTRR) {
             val = MSR_MTRRcap_VCNT | MSR_MTRRcap_FIXRANGE_SUPPORT |
                 MSR_MTRRcap_WC_SUPPORTED;
         } else {
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 40f891d..524a0b4 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -8290,11 +8290,11 @@ static inline void gen_intermediate_code_internal(CPUX86State *env,
     if (flags & HF_SOFTMMU_MASK) {
         dc->mem_index = (cpu_mmu_index(env) + 1) << 2;
     }
-    dc->cpuid_features = env->cpuid_features;
-    dc->cpuid_ext_features = env->cpuid_ext_features;
-    dc->cpuid_ext2_features = env->cpuid_ext2_features;
-    dc->cpuid_ext3_features = env->cpuid_ext3_features;
-    dc->cpuid_7_0_ebx_features = env->cpuid_7_0_ebx_features;
+    dc->cpuid_features = env->features[FEAT_1_EDX];
+    dc->cpuid_ext_features = env->features[FEAT_1_ECX];
+    dc->cpuid_ext2_features = env->features[FEAT_8000_0001_EDX];
+    dc->cpuid_ext3_features = env->features[FEAT_8000_0001_ECX];
+    dc->cpuid_7_0_ebx_features = env->features[FEAT_7_0_EBX];
 #ifdef TARGET_X86_64
     dc->lma = (flags >> HF_LMA_SHIFT) & 1;
     dc->code64 = (flags >> HF_CS64_SHIFT) & 1;
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu 5/9] target-i386: Add ECX information to FeatureWordInfo
  2013-04-22 19:00 [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property Eduardo Habkost
                   ` (3 preceding siblings ...)
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 4/9] target-i386: Replace cpuid_*features fields with a feature word array Eduardo Habkost
@ 2013-04-22 19:00 ` Eduardo Habkost
  2013-05-03 15:16   ` Andreas Färber
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 6/9] target-i386: Add "feature-words" property Eduardo Habkost
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 32+ messages in thread
From: Eduardo Habkost @ 2013-04-22 19:00 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: libvir-list, Igor Mammedov, Jiri Denemark

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 110ef98..314931e 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

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

* [Qemu-devel] [PATCH qom-cpu 6/9] target-i386: Add "feature-words" property
  2013-04-22 19:00 [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property Eduardo Habkost
                   ` (4 preceding siblings ...)
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 5/9] target-i386: Add ECX information to FeatureWordInfo Eduardo Habkost
@ 2013-04-22 19:00 ` Eduardo Habkost
  2013-04-22 20:37   ` [Qemu-devel] [libvirt] " Eric Blake
                     ` (2 more replies)
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 7/9] target-i386: Use FeatureWord loop on filter_features_for_kvm() Eduardo Habkost
                   ` (3 subsequent siblings)
  9 siblings, 3 replies; 32+ messages in thread
From: Eduardo Habkost @ 2013-04-22 19:00 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: libvir-list, Igor Mammedov, Jiri Denemark

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: 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>
---
Changes v1 -> v2:
 * Merge the non-qapi and qapi patches, to keep series simpler
 * Use the feature word array series as base, so we don't have
   to set the feature word values one-by-one in the code
 * Change type name of property from "x86-cpu-feature-words" to
   "X86CPUFeatureWordInfo"
 * Remove cpu-qapi-schema.json and simply add the type definitions
   to qapi-schema.json, to keep the changes simpler
   * This required compiling qapi-types.o and qapi-visit.o into
     *-user as well
---
 .gitignore        |  2 ++
 Makefile.objs     |  7 +++++-
 qapi-schema.json  | 31 ++++++++++++++++++++++++
 target-i386/cpu.c | 70 +++++++++++++++++++++++++++++++++++++++++++++----------
 4 files changed, 97 insertions(+), 13 deletions(-)

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.objs b/Makefile.objs
index a473348..1835d37 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -78,10 +78,15 @@ common-obj-$(CONFIG_SMARTCARD_NSS) += $(libcacard-y)
 ######################################################################
 # qapi
 
-common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o
+common-obj-y += qmp-marshal.o
 common-obj-y += qmp.o hmp.o
 endif
 
+######################################################################
+# some qapi visitors are used by both system and user emulation:
+
+common-obj-y += qapi-visit.o qapi-types.o
+
 #######################################################################
 # Target-independent parts used in system and user emulation
 common-obj-y += qemu-log.o
diff --git a/qapi-schema.json b/qapi-schema.json
index 751d3c2..424434f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3505,3 +3505,34 @@
     '*asl_compiler_rev':  'uint32',
     '*file':              'str',
     '*data':              'str' }}
+
+# @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 314931e..757749c 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 "qapi-types.h"
+#include "qapi-visit.h"
 #include "qapi/visitor.h"
 #include "sysemu/arch_init.h"
 
@@ -194,23 +196,34 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
     },
 };
 
+typedef struct X86RegisterInfo32 {
+    /* Name of register */
+    const char *name;
+    /* QAPI enum value 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)
 {
-    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 x86_reg_info_32[reg].name;
 }
 
 /* collects per-function cpuid data
@@ -1360,6 +1373,36 @@ 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;
+    X86CPUFeatureWordInfo word_infos[FEATURE_WORDS] = { };
+    X86CPUFeatureWordInfoList list_entries[FEATURE_WORDS] = { };
+    X86CPUFeatureWordInfoList *list = NULL;
+
+    for (w = 0; w < FEATURE_WORDS; w++) {
+        FeatureWordInfo *wi = &feature_word_info[w];
+        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;
+        qwi->features = env->features[w];
+
+        /* 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_type_X86CPUFeatureWordInfoList(v, &list, "feature-words", &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;
@@ -2348,6 +2391,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", "X86CPUFeatureWordInfo",
+                        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

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

* [Qemu-devel] [PATCH qom-cpu 7/9] target-i386: Use FeatureWord loop on filter_features_for_kvm()
  2013-04-22 19:00 [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property Eduardo Habkost
                   ` (5 preceding siblings ...)
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 6/9] target-i386: Add "feature-words" property Eduardo Habkost
@ 2013-04-22 19:00 ` Eduardo Habkost
  2013-05-03 15:01   ` Eric Blake
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 8/9] target-i386: Introduce X86CPU.filtered_features field Eduardo Habkost
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 32+ messages in thread
From: Eduardo Habkost @ 2013-04-22 19:00 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: libvir-list, Igor Mammedov, Jiri Denemark

Instead of open-coding the filtering code for each feature word, change
the existing code to use the feature_word_info array, that have exactly
the same CPUID eax/ecx/register values for each feature word.

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

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 757749c..bdb94a7 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1638,24 +1638,14 @@ static void filter_features_for_kvm(X86CPU *cpu)
 {
     CPUX86State *env = &cpu->env;
     KVMState *s = kvm_state;
+    FeatureWord w;
 
-    env->features[FEAT_1_EDX] &=
-        kvm_arch_get_supported_cpuid(s, 1, 0, R_EDX);
-    env->features[FEAT_1_ECX] &=
-        kvm_arch_get_supported_cpuid(s, 1, 0, R_ECX);
-    env->features[FEAT_8000_0001_EDX] &=
-        kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_EDX);
-    env->features[FEAT_8000_0001_ECX] &=
-        kvm_arch_get_supported_cpuid(s, 0x80000001, 0, R_ECX);
-    env->features[FEAT_SVM]  &=
-        kvm_arch_get_supported_cpuid(s, 0x8000000A, 0, R_EDX);
-    env->features[FEAT_7_0_EBX] &=
-        kvm_arch_get_supported_cpuid(s, 7, 0, R_EBX);
-    env->features[FEAT_KVM] &=
-        kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX);
-    env->features[FEAT_C000_0001_EDX] &=
-        kvm_arch_get_supported_cpuid(s, 0xC0000001, 0, R_EDX);
-
+    for (w = 0; w < FEATURE_WORDS; w++) {
+        FeatureWordInfo *wi = &feature_word_info[w];
+        env->features[w] &= kvm_arch_get_supported_cpuid(s, wi->cpuid_eax,
+                                                            wi->cpuid_ecx,
+                                                            wi->cpuid_reg);
+    }
 }
 #endif
 
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu 8/9] target-i386: Introduce X86CPU.filtered_features field
  2013-04-22 19:00 [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property Eduardo Habkost
                   ` (6 preceding siblings ...)
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 7/9] target-i386: Use FeatureWord loop on filter_features_for_kvm() Eduardo Habkost
@ 2013-04-22 19:00 ` Eduardo Habkost
  2013-05-03 15:03   ` Eric Blake
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 9/9] target-i386: Add "filtered-features" property to X86CPU Eduardo Habkost
  2013-05-01 22:53 ` [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property Andreas Färber
  9 siblings, 1 reply; 32+ messages in thread
From: Eduardo Habkost @ 2013-04-22 19:00 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: libvir-list, Igor Mammedov, Jiri Denemark

This field will contain the feature bits that were filtered out because
of missing host support.

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

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 08f9eb6..159378f 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -65,6 +65,9 @@ typedef struct X86CPU {
     /*< public >*/
 
     CPUX86State env;
+
+    /* Features that were filtered out because of missing host capabilities */
+    uint32_t filtered_features[FEATURE_WORDS];
 } X86CPU;
 
 static inline X86CPU *x86_env_get_cpu(CPUX86State *env)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index bdb94a7..1178c5f 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1642,9 +1642,12 @@ static void filter_features_for_kvm(X86CPU *cpu)
 
     for (w = 0; w < FEATURE_WORDS; w++) {
         FeatureWordInfo *wi = &feature_word_info[w];
-        env->features[w] &= kvm_arch_get_supported_cpuid(s, wi->cpuid_eax,
-                                                            wi->cpuid_ecx,
-                                                            wi->cpuid_reg);
+        uint32_t host_feat = kvm_arch_get_supported_cpuid(s, wi->cpuid_eax,
+                                                             wi->cpuid_ecx,
+                                                             wi->cpuid_reg);
+        uint32_t requested_features = env->features[w];
+        env->features[w] &= host_feat;
+        cpu->filtered_features[w] = requested_features & ~env->features[w];
     }
 }
 #endif
-- 
1.8.1.4

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

* [Qemu-devel] [PATCH qom-cpu 9/9] target-i386: Add "filtered-features" property to X86CPU
  2013-04-22 19:00 [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property Eduardo Habkost
                   ` (7 preceding siblings ...)
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 8/9] target-i386: Introduce X86CPU.filtered_features field Eduardo Habkost
@ 2013-04-22 19:00 ` Eduardo Habkost
  2013-05-03 15:10   ` Eric Blake
  2013-05-01 22:53 ` [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property Andreas Färber
  9 siblings, 1 reply; 32+ messages in thread
From: Eduardo Habkost @ 2013-04-22 19:00 UTC (permalink / raw)
  To: qemu-devel, Andreas Färber; +Cc: libvir-list, Igor Mammedov, Jiri Denemark

This property will contain all the features that were removed from the
CPU because they are not supported by the host.

This way, libvirt or other management tools can emulate the
check/enforce behavior by checking if filtered-properties is all zeroes,
before starting the guest.

Example output where some features were missing:

  $ ./install/bin/qemu-system-x86_64 -enable-kvm -cpu Haswell,check -S -qmp unix:/tmp/m,server,nowait
  warning: host doesn't support requested feature: CPUID.01H:ECX.fma [bit 12]
  warning: host doesn't support requested feature: CPUID.01H:ECX.movbe [bit 22]
  warning: host doesn't support requested feature: CPUID.01H:ECX.tsc-deadline [bit 24]
  warning: host doesn't support requested feature: CPUID.01H:ECX.xsave [bit 26]
  warning: host doesn't support requested feature: CPUID.01H:ECX.avx [bit 28]
  warning: host doesn't support requested feature: CPUID.07H:EBX.fsgsbase [bit 0]
  warning: host doesn't support requested feature: CPUID.07H:EBX.bmi1 [bit 3]
  warning: host doesn't support requested feature: CPUID.07H:EBX.hle [bit 4]
  warning: host doesn't support requested feature: CPUID.07H:EBX.avx2 [bit 5]
  warning: host doesn't support requested feature: CPUID.07H:EBX.smep [bit 7]
  warning: host doesn't support requested feature: CPUID.07H:EBX.bmi2 [bit 8]
  warning: host doesn't support requested feature: CPUID.07H:EBX.erms [bit 9]
  warning: host doesn't support requested feature: CPUID.07H:EBX.invpcid [bit 10]
  warning: host doesn't support requested feature: CPUID.07H:EBX.rtm [bit 11]
  [...]
  $ ./QMP/qmp --path=/tmp/m qom-get --path=/machine/unattached/device[1] --property=filtered-features
  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: 0
  item[4].cpuid-register: EDX
  item[4].cpuid-input-eax: 2147483649
  item[4].features: 0
  item[5].cpuid-register: EBX
  item[5].cpuid-input-eax: 7
  item[5].features: 4025
  item[5].cpuid-input-ecx: 0
  item[6].cpuid-register: ECX
  item[6].cpuid-input-eax: 1
  item[6].features: 356519936
  item[7].cpuid-register: EDX
  item[7].cpuid-input-eax: 1
  item[7].features: 0

Example output when no feature is missing:

  $ ./install/bin/qemu-system-x86_64 -enable-kvm -cpu Nehalem,enforce -S -qmp unix:/tmp/m,server,nowait
  [...]
  $ ./QMP/qmp --path=/tmp/m qom-get --path=/machine/unattached/device[1]
  --property=filtered-features
  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: 0
  item[4].cpuid-register: EDX
  item[4].cpuid-input-eax: 2147483649
  item[4].features: 0
  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: 0
  item[7].cpuid-register: EDX
  item[7].cpuid-input-eax: 1
  item[7].features: 0

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

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1178c5f..5bcb79c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1373,11 +1373,11 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
     cpu->env.tsc_khz = value / 1000;
 }
 
+/* Generic getter for "feature-words" and "filtered-features" properties */
 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;
+    uint32_t *array = (uint32_t *)opaque;
     FeatureWord w;
     Error *err = NULL;
     X86CPUFeatureWordInfo word_infos[FEATURE_WORDS] = { };
@@ -1391,7 +1391,7 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
         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;
-        qwi->features = env->features[w];
+        qwi->features = array[w];
 
         /* List will be in reverse order, but order shouldn't matter */
         list_entries[w].next = list;
@@ -2386,7 +2386,10 @@ static void x86_cpu_initfn(Object *obj)
                         x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
     object_property_add(obj, "feature-words", "X86CPUFeatureWordInfo",
                         x86_cpu_get_feature_words,
-                        NULL, NULL, NULL, NULL);
+                        NULL, NULL, (void *)env->features, NULL);
+    object_property_add(obj, "filtered-features", "X86CPUFeatureWordInfo",
+                        x86_cpu_get_feature_words,
+                        NULL, NULL, (void *)cpu->filtered_features, NULL);
 
     env->cpuid_apic_id = x86_cpu_apic_id_from_index(cs->cpu_index);
 
-- 
1.8.1.4

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

* Re: [Qemu-devel] [libvirt] [PATCH qom-cpu 6/9] target-i386: Add "feature-words" property
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 6/9] target-i386: Add "feature-words" property Eduardo Habkost
@ 2013-04-22 20:37   ` Eric Blake
  2013-04-23 19:25     ` Eduardo Habkost
  2013-05-03 11:34   ` [Qemu-devel] " Igor Mammedov
  2013-05-03 14:57   ` Eric Blake
  2 siblings, 1 reply; 32+ messages in thread
From: Eric Blake @ 2013-04-22 20:37 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: libvir-list, Igor Mammedov, Jiri Denemark, qemu-devel,
	Andreas Färber

[-- Attachment #1: Type: text/plain, Size: 2230 bytes --]

On 04/22/2013 01:00 PM, Eduardo Habkost wrote:
> 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

If I'm not mistaken, the QMP counterpart that libvirt will use is:

{ "execute":"qom-get",
  "arguments": { "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

And this would then be returned as a JSON array containing struct
members looking like this:

> +{ 'type': 'X86CPUFeatureWordInfo',
> +  'data': { 'cpuid-input-eax': 'int',
> +            '*cpuid-input-ecx': 'int',
> +            'cpuid-register': 'X86CPURegister32',
> +            'features': 'int' } }

Looks reasonable (and better than what we've had in the past!), although
I'll let Jiri Denemark give final say on whether it meets libvirt's needs.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

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

* Re: [Qemu-devel] [libvirt] [PATCH qom-cpu 6/9] target-i386: Add "feature-words" property
  2013-04-22 20:37   ` [Qemu-devel] [libvirt] " Eric Blake
@ 2013-04-23 19:25     ` Eduardo Habkost
  0 siblings, 0 replies; 32+ messages in thread
From: Eduardo Habkost @ 2013-04-23 19:25 UTC (permalink / raw)
  To: Eric Blake
  Cc: libvir-list, Igor Mammedov, Jiri Denemark, qemu-devel,
	Andreas Färber

On Mon, Apr 22, 2013 at 02:37:06PM -0600, Eric Blake wrote:
> On 04/22/2013 01:00 PM, Eduardo Habkost wrote:
> > 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
> 
> If I'm not mistaken, the QMP counterpart that libvirt will use is:
> 
> { "execute":"qom-get",
>   "arguments": { "path":"/machine/unattached/device[1]",
>                  "property":"feature-words" } }

Yes, but note that libvirt needs to look for the X86CPU objects, to find
the actual path as there's no guarantee that it will always be
/machine/unattached/devices[1]. Maybe the CPU hotplug work will end up
offering a predictable path for the X86CPU objects, but this is not
available yet.

> 
> >   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
> 
> And this would then be returned as a JSON array containing struct
> members looking like this:
> 
> > +{ 'type': 'X86CPUFeatureWordInfo',
> > +  'data': { 'cpuid-input-eax': 'int',
> > +            '*cpuid-input-ecx': 'int',
> > +            'cpuid-register': 'X86CPURegister32',
> > +            'features': 'int' } }

Yes.

> 
> Looks reasonable (and better than what we've had in the past!), although
> I'll let Jiri Denemark give final say on whether it meets libvirt's needs.

Thanks for the feedback!

-- 
Eduardo

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

* Re: [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property
  2013-04-22 19:00 [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property Eduardo Habkost
                   ` (8 preceding siblings ...)
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 9/9] target-i386: Add "filtered-features" property to X86CPU Eduardo Habkost
@ 2013-05-01 22:53 ` Andreas Färber
  2013-05-02 19:43   ` Eduardo Habkost
  9 siblings, 1 reply; 32+ messages in thread
From: Andreas Färber @ 2013-05-01 22:53 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: libvir-list, Markus Armbruster, qemu-devel, Luiz Capitulino,
	Anthony Liguori, Igor Mammedov, Jiri Denemark

Am 22.04.2013 21:00, schrieb Eduardo Habkost:
> This series includes the previous "replace cpuid_*features fields with a feature
> word array" series.
> 
> The first 4 patches already have a Reviewed-by from Igor, they correspond to v10
> plus a small indent fix requested by him.
> 
> As the cpuid_*features series was holding my "feature-words"/"filtered-features"
> series (previously sent as RFC), I am now sending both as a single series, to
> try to get "feature-words"/"filtered-features" some attention and try to get it
> included in 1.5.
> 
> The "feature-words"/"filtered-features" mechanism is very important for libvirt,
> to allow it to ensure the guest is getting the required set of CPU features, as
> configured by the user.
> 
> 
> Eduardo Habkost (9):
>   target-i386: cleanup: Group together level, xlevel, xlevel2 fields
>   target-i386/kvm.c: Code formatting changes
>   target-i386/cpu.c: Break lines so they don't get too long
>   target-i386: Replace cpuid_*features fields with a feature word array

Thanks, applied these to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

These had been around for quite some time and I have reviewed the first
three line by line; for the fourth I have looked at the script sources
and trust Igor's review. Thanks for repeatedly rebasing this, Eduardo!


>   target-i386: Add ECX information to FeatureWordInfo

This one is lacking Reviewed-by / Acked-by ...

>   target-i386: Add "feature-words" property
>   target-i386: Use FeatureWord loop on filter_features_for_kvm()

... and this one seems to depend on it.

>   target-i386: Introduce X86CPU.filtered_features field
>   target-i386: Add "filtered-features" property to X86CPU

As mentioned earlier I'd prefer to defer the property design rather than
putting it lightly reviewed into 1.5 and living with some ABI.
If libvirt urgently needs this info, this series needs to be reviewed
and sorted out until the weekend (Hard Freeze on Monday).

Regards,
Andreas

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

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

* Re: [Qemu-devel] [PATCH qom-cpu 2/9] target-i386/kvm.c: Code formatting changes
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 2/9] target-i386/kvm.c: Code formatting changes Eduardo Habkost
@ 2013-05-01 22:55   ` Andreas Färber
  0 siblings, 0 replies; 32+ messages in thread
From: Andreas Färber @ 2013-05-01 22:55 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: libvir-list, Igor Mammedov, Jiri Denemark, qemu-devel

Am 22.04.2013 21:00, schrieb Eduardo Habkost:
> Add appropriate spaces around operators, and break line where it needs
> to be broken to allow feature-words array to be introduced without
> having too-long lines.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> Reviewed-By: Igor Mammedov <imammedo@redhat.com>
> ---
> Changes v9:
>  * 1-char alignment change: keep the opening parenthesis of both sides
>    of the "==" operator starting at the same column
> ---
>  target-i386/kvm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 0e7cc81..b5bff33 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -613,7 +613,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
>      cpuid_data.cpuid.nent = cpuid_i;
>  
>      if (((env->cpuid_version >> 8)&0xF) >= 6
> -        && (env->cpuid_features&(CPUID_MCE|CPUID_MCA)) == (CPUID_MCE|CPUID_MCA)
> +        && (env->cpuid_features & (CPUID_MCE|CPUID_MCA)) ==
> +           (CPUID_MCE|CPUID_MCA)

I've added spaces around the | operator as well. :)

Andreas

>          && kvm_check_extension(cs->kvm_state, KVM_CAP_MCE) > 0) {
>          uint64_t mcg_cap;
>          int banks;
> 


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

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

* Re: [Qemu-devel] [PATCH qom-cpu 4/9] target-i386: Replace cpuid_*features fields with a feature word array
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 4/9] target-i386: Replace cpuid_*features fields with a feature word array Eduardo Habkost
@ 2013-05-01 23:03   ` Andreas Färber
  2013-05-02 15:06     ` Eduardo Habkost
  0 siblings, 1 reply; 32+ messages in thread
From: Andreas Färber @ 2013-05-01 23:03 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: libvir-list, Igor Mammedov, Jiri Denemark, qemu-devel

Am 22.04.2013 21:00, schrieb Eduardo Habkost:
> This replaces the feature-bit fields on both X86CPU and x86_def_t
> structs with an array.
> 
> With this, we will be able to simplify code that simply does the same
> operation on all feature words (e.g. kvm_check_features_against_host(),
> filter_features_for_kvm(), add_flagname_to_bitmaps(), CPU feature-bit
> property lookup/registration, and the proposed "feature-words" property)
> 
> The following field replacements were made on X86CPU and x86_def_t:
> 
>   (cpuid_)features         -> features[FEAT_1_EDX]
>   (cpuid_)ext_features     -> features[FEAT_1_ECX]
>   (cpuid_)ext2_features    -> features[FEAT_8000_0001_EDX]
>   (cpuid_)ext3_features    -> features[FEAT_8000_0001_ECX]
>   (cpuid_)ext4_features    -> features[FEAT_C000_0001_EDX]
>   (cpuid_)kvm_features     -> features[FEAT_KVM]
>   (cpuid_)svm_features     -> features[FEAT_SVM]
>   (cpuid_)7_0_ebx_features -> features[FEAT_7_0_EBX]
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> Reviewed-By: Igor Mammedov <imammedo@redhat.com>
[...]
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 73ae2ef..110ef98 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
[...]
> @@ -1490,22 +1485,22 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
>          }
>          featurestr = strtok(NULL, ",");
>      }
> -    env->cpuid_features |= plus_features[FEAT_1_EDX];
> -    env->cpuid_ext_features |= plus_features[FEAT_1_ECX];
> -    env->cpuid_ext2_features |= plus_features[FEAT_8000_0001_EDX];
> -    env->cpuid_ext3_features |= plus_features[FEAT_8000_0001_ECX];
> -    env->cpuid_ext4_features |= plus_features[FEAT_C000_0001_EDX];
> -    env->cpuid_kvm_features |= plus_features[FEAT_KVM];
> -    env->cpuid_svm_features |= plus_features[FEAT_SVM];
> -    env->cpuid_7_0_ebx_features |= plus_features[FEAT_7_0_EBX];
> -    env->cpuid_features &= ~minus_features[FEAT_1_EDX];
> -    env->cpuid_ext_features &= ~minus_features[FEAT_1_ECX];
> -    env->cpuid_ext2_features &= ~minus_features[FEAT_8000_0001_EDX];
> -    env->cpuid_ext3_features &= ~minus_features[FEAT_8000_0001_ECX];
> -    env->cpuid_ext4_features &= ~minus_features[FEAT_C000_0001_EDX];
> -    env->cpuid_kvm_features &= ~minus_features[FEAT_KVM];
> -    env->cpuid_svm_features &= ~minus_features[FEAT_SVM];
> -    env->cpuid_7_0_ebx_features &= ~minus_features[FEAT_7_0_EBX];
> +    env->features[FEAT_1_EDX] |= plus_features[FEAT_1_EDX];
> +    env->features[FEAT_1_ECX] |= plus_features[FEAT_1_ECX];
> +    env->features[FEAT_8000_0001_EDX] |= plus_features[FEAT_8000_0001_EDX];
> +    env->features[FEAT_8000_0001_ECX] |= plus_features[FEAT_8000_0001_ECX];
> +    env->features[FEAT_C000_0001_EDX] |= plus_features[FEAT_C000_0001_EDX];
> +    env->features[FEAT_KVM] |= plus_features[FEAT_KVM];
> +    env->features[FEAT_SVM] |= plus_features[FEAT_SVM];
> +    env->features[FEAT_7_0_EBX] |= plus_features[FEAT_7_0_EBX];
> +    env->features[FEAT_1_EDX] &= ~minus_features[FEAT_1_EDX];
> +    env->features[FEAT_1_ECX] &= ~minus_features[FEAT_1_ECX];
> +    env->features[FEAT_8000_0001_EDX] &= ~minus_features[FEAT_8000_0001_EDX];
> +    env->features[FEAT_8000_0001_ECX] &= ~minus_features[FEAT_8000_0001_ECX];
> +    env->features[FEAT_C000_0001_EDX] &= ~minus_features[FEAT_C000_0001_EDX];
> +    env->features[FEAT_KVM] &= ~minus_features[FEAT_KVM];
> +    env->features[FEAT_SVM] &= ~minus_features[FEAT_SVM];
> +    env->features[FEAT_7_0_EBX] &= ~minus_features[FEAT_7_0_EBX];
>  
>  out:
>      return;

Can this be done in a loop as a follow-up?

[...]
> @@ -1630,24 +1625,24 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
>      }
>  
>      if (kvm_enabled()) {
> -        def->kvm_features |= kvm_default_features;
> +        def->features[FEAT_KVM] |= kvm_default_features;
>      }
> -    def->ext_features |= CPUID_EXT_HYPERVISOR;
> +    def->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
>  
>      object_property_set_str(OBJECT(cpu), def->vendor, "vendor", errp);
>      object_property_set_int(OBJECT(cpu), def->level, "level", errp);
>      object_property_set_int(OBJECT(cpu), def->family, "family", errp);
>      object_property_set_int(OBJECT(cpu), def->model, "model", errp);
>      object_property_set_int(OBJECT(cpu), def->stepping, "stepping", errp);
> -    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;
> +    env->features[FEAT_1_EDX] = def->features[FEAT_1_EDX];
> +    env->features[FEAT_1_ECX] = def->features[FEAT_1_ECX];
> +    env->features[FEAT_8000_0001_EDX] = def->features[FEAT_8000_0001_EDX];
> +    env->features[FEAT_8000_0001_ECX] = def->features[FEAT_8000_0001_ECX];
>      object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", errp);
> -    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->features[FEAT_KVM] = def->features[FEAT_KVM];
> +    env->features[FEAT_SVM] = def->features[FEAT_SVM];
> +    env->features[FEAT_C000_0001_EDX] = def->features[FEAT_C000_0001_EDX];
> +    env->features[FEAT_7_0_EBX] = def->features[FEAT_7_0_EBX];
>      env->cpuid_xlevel2 = def->xlevel2;
>  
>      object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
[snip]

Dito?

Andreas

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

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

* Re: [Qemu-devel] [PATCH qom-cpu 4/9] target-i386: Replace cpuid_*features fields with a feature word array
  2013-05-01 23:03   ` Andreas Färber
@ 2013-05-02 15:06     ` Eduardo Habkost
  0 siblings, 0 replies; 32+ messages in thread
From: Eduardo Habkost @ 2013-05-02 15:06 UTC (permalink / raw)
  To: Andreas Färber; +Cc: libvir-list, Igor Mammedov, Jiri Denemark, qemu-devel

On Thu, May 02, 2013 at 01:03:01AM +0200, Andreas Färber wrote:
> Am 22.04.2013 21:00, schrieb Eduardo Habkost:
> > This replaces the feature-bit fields on both X86CPU and x86_def_t
> > structs with an array.
> > 
> > With this, we will be able to simplify code that simply does the same
> > operation on all feature words (e.g. kvm_check_features_against_host(),
> > filter_features_for_kvm(), add_flagname_to_bitmaps(), CPU feature-bit
> > property lookup/registration, and the proposed "feature-words" property)
> > 
> > The following field replacements were made on X86CPU and x86_def_t:
> > 
> >   (cpuid_)features         -> features[FEAT_1_EDX]
> >   (cpuid_)ext_features     -> features[FEAT_1_ECX]
> >   (cpuid_)ext2_features    -> features[FEAT_8000_0001_EDX]
> >   (cpuid_)ext3_features    -> features[FEAT_8000_0001_ECX]
> >   (cpuid_)ext4_features    -> features[FEAT_C000_0001_EDX]
> >   (cpuid_)kvm_features     -> features[FEAT_KVM]
> >   (cpuid_)svm_features     -> features[FEAT_SVM]
> >   (cpuid_)7_0_ebx_features -> features[FEAT_7_0_EBX]
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > Reviewed-By: Igor Mammedov <imammedo@redhat.com>
> [...]
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 73ae2ef..110ef98 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> [...]
> > @@ -1490,22 +1485,22 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
> >          }
> >          featurestr = strtok(NULL, ",");
> >      }
> > -    env->cpuid_features |= plus_features[FEAT_1_EDX];
> > -    env->cpuid_ext_features |= plus_features[FEAT_1_ECX];
> > -    env->cpuid_ext2_features |= plus_features[FEAT_8000_0001_EDX];
> > -    env->cpuid_ext3_features |= plus_features[FEAT_8000_0001_ECX];
> > -    env->cpuid_ext4_features |= plus_features[FEAT_C000_0001_EDX];
> > -    env->cpuid_kvm_features |= plus_features[FEAT_KVM];
> > -    env->cpuid_svm_features |= plus_features[FEAT_SVM];
> > -    env->cpuid_7_0_ebx_features |= plus_features[FEAT_7_0_EBX];
> > -    env->cpuid_features &= ~minus_features[FEAT_1_EDX];
> > -    env->cpuid_ext_features &= ~minus_features[FEAT_1_ECX];
> > -    env->cpuid_ext2_features &= ~minus_features[FEAT_8000_0001_EDX];
> > -    env->cpuid_ext3_features &= ~minus_features[FEAT_8000_0001_ECX];
> > -    env->cpuid_ext4_features &= ~minus_features[FEAT_C000_0001_EDX];
> > -    env->cpuid_kvm_features &= ~minus_features[FEAT_KVM];
> > -    env->cpuid_svm_features &= ~minus_features[FEAT_SVM];
> > -    env->cpuid_7_0_ebx_features &= ~minus_features[FEAT_7_0_EBX];
> > +    env->features[FEAT_1_EDX] |= plus_features[FEAT_1_EDX];
> > +    env->features[FEAT_1_ECX] |= plus_features[FEAT_1_ECX];
> > +    env->features[FEAT_8000_0001_EDX] |= plus_features[FEAT_8000_0001_EDX];
> > +    env->features[FEAT_8000_0001_ECX] |= plus_features[FEAT_8000_0001_ECX];
> > +    env->features[FEAT_C000_0001_EDX] |= plus_features[FEAT_C000_0001_EDX];
> > +    env->features[FEAT_KVM] |= plus_features[FEAT_KVM];
> > +    env->features[FEAT_SVM] |= plus_features[FEAT_SVM];
> > +    env->features[FEAT_7_0_EBX] |= plus_features[FEAT_7_0_EBX];
> > +    env->features[FEAT_1_EDX] &= ~minus_features[FEAT_1_EDX];
> > +    env->features[FEAT_1_ECX] &= ~minus_features[FEAT_1_ECX];
> > +    env->features[FEAT_8000_0001_EDX] &= ~minus_features[FEAT_8000_0001_EDX];
> > +    env->features[FEAT_8000_0001_ECX] &= ~minus_features[FEAT_8000_0001_ECX];
> > +    env->features[FEAT_C000_0001_EDX] &= ~minus_features[FEAT_C000_0001_EDX];
> > +    env->features[FEAT_KVM] &= ~minus_features[FEAT_KVM];
> > +    env->features[FEAT_SVM] &= ~minus_features[FEAT_SVM];
> > +    env->features[FEAT_7_0_EBX] &= ~minus_features[FEAT_7_0_EBX];
> >  
> >  out:
> >      return;
> 
> Can this be done in a loop as a follow-up?

Yes, that was exactly the plan. :-)

But it's something to be done after 1.5, I guess?

-- 
Eduardo

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

* Re: [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property
  2013-05-01 22:53 ` [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property Andreas Färber
@ 2013-05-02 19:43   ` Eduardo Habkost
  2013-05-02 19:48     ` Eric Blake
  0 siblings, 1 reply; 32+ messages in thread
From: Eduardo Habkost @ 2013-05-02 19:43 UTC (permalink / raw)
  To: Andreas Färber
  Cc: libvir-list, Markus Armbruster, qemu-devel, Luiz Capitulino,
	Anthony Liguori, Igor Mammedov, Jiri Denemark

On Thu, May 02, 2013 at 12:53:33AM +0200, Andreas Färber wrote:
> > Eduardo Habkost (9):
> >   target-i386: cleanup: Group together level, xlevel, xlevel2 fields
> >   target-i386/kvm.c: Code formatting changes
> >   target-i386/cpu.c: Break lines so they don't get too long
> >   target-i386: Replace cpuid_*features fields with a feature word array
> 
> Thanks, applied these to qom-cpu:
> https://github.com/afaerber/qemu-cpu/commits/qom-cpu
> 
> These had been around for quite some time and I have reviewed the first
> three line by line; for the fourth I have looked at the script sources
> and trust Igor's review. Thanks for repeatedly rebasing this, Eduardo!

Thanks!

> 
> 
> >   target-i386: Add ECX information to FeatureWordInfo
> 
> This one is lacking Reviewed-by / Acked-by ...
> 
> >   target-i386: Add "feature-words" property
> >   target-i386: Use FeatureWord loop on filter_features_for_kvm()
> 
> ... and this one seems to depend on it.
> 
> >   target-i386: Introduce X86CPU.filtered_features field
> >   target-i386: Add "filtered-features" property to X86CPU
> 
> As mentioned earlier I'd prefer to defer the property design rather than
> putting it lightly reviewed into 1.5 and living with some ABI.
> If libvirt urgently needs this info, this series needs to be reviewed
> and sorted out until the weekend (Hard Freeze on Monday).

I consider it an important bugfix for the QEMU+libvirt stack. The
current libvirt behavior (checking CPUID directly; not using the
"enforce" flag; and having its own copy of each CPU model definition) is
unsafe and may break live-migration silently under many circumstances.

-- 
Eduardo

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

* Re: [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property
  2013-05-02 19:43   ` Eduardo Habkost
@ 2013-05-02 19:48     ` Eric Blake
  2013-05-03 14:58       ` Andreas Färber
  0 siblings, 1 reply; 32+ messages in thread
From: Eric Blake @ 2013-05-02 19:48 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: libvir-list, Markus Armbruster, qemu-devel, Luiz Capitulino,
	Anthony Liguori, Igor Mammedov, Jiri Denemark,
	Andreas Färber

[-- Attachment #1: Type: text/plain, Size: 838 bytes --]

On 05/02/2013 01:43 PM, Eduardo Habkost wrote:
>>
>> As mentioned earlier I'd prefer to defer the property design rather than
>> putting it lightly reviewed into 1.5 and living with some ABI.
>> If libvirt urgently needs this info, this series needs to be reviewed
>> and sorted out until the weekend (Hard Freeze on Monday).
> 
> I consider it an important bugfix for the QEMU+libvirt stack. The
> current libvirt behavior (checking CPUID directly; not using the
> "enforce" flag; and having its own copy of each CPU model definition) is
> unsafe and may break live-migration silently under many circumstances.

I agree that libvirt would very much like to have this in 1.5.  How can
I help in reviewing things?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

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

* Re: [Qemu-devel] [PATCH qom-cpu 6/9] target-i386: Add "feature-words" property
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 6/9] target-i386: Add "feature-words" property Eduardo Habkost
  2013-04-22 20:37   ` [Qemu-devel] [libvirt] " Eric Blake
@ 2013-05-03 11:34   ` Igor Mammedov
  2013-05-03 13:17     ` Eduardo Habkost
  2013-05-03 14:57   ` Eric Blake
  2 siblings, 1 reply; 32+ messages in thread
From: Igor Mammedov @ 2013-05-03 11:34 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: libvir-list, Jiri Denemark, qemu-devel, Andreas Färber

On Mon, 22 Apr 2013 16:00:17 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> 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

patch doesn't apply to current qom-cpu, more comments below.

> 
> 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
could item be represented as CPUID function in format used in Intel's SDM?

item[5].CPUID: EAX=7h,ECX=0h
item[5].EBX: 0
item[5].EAX: 0

for simplicity/uniformity ECX could be not optional, always present, and
ignored when not needed.
 
>   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>
> ---
> Changes v1 -> v2:
>  * Merge the non-qapi and qapi patches, to keep series simpler
>  * Use the feature word array series as base, so we don't have
>    to set the feature word values one-by-one in the code
>  * Change type name of property from "x86-cpu-feature-words" to
>    "X86CPUFeatureWordInfo"
>  * Remove cpu-qapi-schema.json and simply add the type definitions
>    to qapi-schema.json, to keep the changes simpler
>    * This required compiling qapi-types.o and qapi-visit.o into
>      *-user as well
> ---
>  .gitignore        |  2 ++
>  Makefile.objs     |  7 +++++-
>  qapi-schema.json  | 31 ++++++++++++++++++++++++
>  target-i386/cpu.c | 70 +++++++++++++++++++++++++++++++++++++++++++++----------
>  4 files changed, 97 insertions(+), 13 deletions(-)
> 
> 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]
still needed?

>  qmp-commands.h
>  qmp-marshal.c
>  qemu-doc.html
> diff --git a/Makefile.objs b/Makefile.objs
> index a473348..1835d37 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -78,10 +78,15 @@ common-obj-$(CONFIG_SMARTCARD_NSS) += $(libcacard-y)
>  ######################################################################
>  # qapi
>  
> -common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o
> +common-obj-y += qmp-marshal.o
>  common-obj-y += qmp.o hmp.o
>  endif
>  
> +######################################################################
> +# some qapi visitors are used by both system and user emulation:
> +
> +common-obj-y += qapi-visit.o qapi-types.o
> +
>  #######################################################################
>  # Target-independent parts used in system and user emulation
>  common-obj-y += qemu-log.o
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 751d3c2..424434f 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3505,3 +3505,34 @@
>      '*asl_compiler_rev':  'uint32',
>      '*file':              'str',
>      '*data':              'str' }}
> +
> +# @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 314931e..757749c 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 "qapi-types.h"
> +#include "qapi-visit.h"
>  #include "qapi/visitor.h"
>  #include "sysemu/arch_init.h"
>  
> @@ -194,23 +196,34 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>      },
>  };
>  
> +typedef struct X86RegisterInfo32 {
> +    /* Name of register */
> +    const char *name;
> +    /* QAPI enum value register */
> +    X86CPURegister32 qapi_enum;
> +} X86RegisterInfo32;
> +
> +#define REGISTER(reg) \
> +    [R_##reg] = { .name = #reg, .qapi_enum = X86_C_P_U_REGISTER32_##reg }
                                                ^^^^
Using auto-generated names like this probably is very fragile.

> +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)
>  {
> -    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 x86_reg_info_32[reg].name;
>  }
>  
>  /* collects per-function cpuid data
> @@ -1360,6 +1373,36 @@ 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;
> +    X86CPUFeatureWordInfo word_infos[FEATURE_WORDS] = { };
> +    X86CPUFeatureWordInfoList list_entries[FEATURE_WORDS] = { };
> +    X86CPUFeatureWordInfoList *list = NULL;
> +
> +    for (w = 0; w < FEATURE_WORDS; w++) {
> +        FeatureWordInfo *wi = &feature_word_info[w];
> +        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;
Is there way not to use qapi_enum at all and use name instead?

> +        qwi->features = env->features[w];
> +
> +        /* List will be in reverse order, but order shouldn't matter */
> +        list_entries[w].next = list;
> +        list_entries[w].value = &word_infos[w];
list_entries[w].value = qwi;

> +        list = &list_entries[w];
> +    }
> +
> +    visit_type_X86CPUFeatureWordInfoList(v, &list, "feature-words", &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;
> @@ -2348,6 +2391,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", "X86CPUFeatureWordInfo",
> +                        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
> 
> 


-- 
Regards,
  Igor

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

* Re: [Qemu-devel] [PATCH qom-cpu 6/9] target-i386: Add "feature-words" property
  2013-05-03 11:34   ` [Qemu-devel] " Igor Mammedov
@ 2013-05-03 13:17     ` Eduardo Habkost
  2013-05-03 14:25       ` Eric Blake
  0 siblings, 1 reply; 32+ messages in thread
From: Eduardo Habkost @ 2013-05-03 13:17 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: libvir-list, Jiri Denemark, qemu-devel, Andreas Färber

On Fri, May 03, 2013 at 01:34:23PM +0200, Igor Mammedov wrote:
> On Mon, 22 Apr 2013 16:00:17 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > 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
> 
> patch doesn't apply to current qom-cpu, more comments below.
> 
> > 
> > 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
> could item be represented as CPUID function in format used in Intel's SDM?
> 

We could, but maybe it would make the interface harder to use and not
easier?

Even when two feature words are returned in the same CPUID leaf, they
are independent and separate feature-words that must be checked
individually by libvirt, so I believe returning one feature-word per
array-item makes more sense. Having an extra item in the array would
make it clear for libvirt that QEMU has a new feature-word that libvirt
doesn't know about, and easier to spot than an extra field in an
existing array item.


> item[5].CPUID: EAX=7h,ECX=0h

What would be the data type of this "CPUID" field? Are you suggesting
returning a string to be parsed manually?


> item[5].EBX: 0
> item[5].EAX: 0
> 
> for simplicity/uniformity ECX could be not optional, always present, and
> ignored when not needed.

Then how would we represent the fact that ECX is optional? If ECX is not
important for a given CPUID leaf, I don't see what's the point of
including it with a fake value.

Note that this interface is not supposed to be a comprehensive "CPUID
dump" with all CPUID bits returned by the CPU, but just a list of "CPU
capability words" that happen to be returned inside CPUID leaves.

>  
> >   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>
> > ---
> > Changes v1 -> v2:
> >  * Merge the non-qapi and qapi patches, to keep series simpler
> >  * Use the feature word array series as base, so we don't have
> >    to set the feature word values one-by-one in the code
> >  * Change type name of property from "x86-cpu-feature-words" to
> >    "X86CPUFeatureWordInfo"
> >  * Remove cpu-qapi-schema.json and simply add the type definitions
> >    to qapi-schema.json, to keep the changes simpler
> >    * This required compiling qapi-types.o and qapi-visit.o into
> >      *-user as well
> > ---
> >  .gitignore        |  2 ++
> >  Makefile.objs     |  7 +++++-
> >  qapi-schema.json  | 31 ++++++++++++++++++++++++
> >  target-i386/cpu.c | 70 +++++++++++++++++++++++++++++++++++++++++++++----------
> >  4 files changed, 97 insertions(+), 13 deletions(-)
> > 
> > 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]
> still needed?

Not needed anymore. Thanks for spotting it.

> 
> >  qmp-commands.h
> >  qmp-marshal.c
> >  qemu-doc.html
> > diff --git a/Makefile.objs b/Makefile.objs
> > index a473348..1835d37 100644
> > --- a/Makefile.objs
> > +++ b/Makefile.objs
> > @@ -78,10 +78,15 @@ common-obj-$(CONFIG_SMARTCARD_NSS) += $(libcacard-y)
> >  ######################################################################
> >  # qapi
> >  
> > -common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o
> > +common-obj-y += qmp-marshal.o
> >  common-obj-y += qmp.o hmp.o
> >  endif
> >  
> > +######################################################################
> > +# some qapi visitors are used by both system and user emulation:
> > +
> > +common-obj-y += qapi-visit.o qapi-types.o
> > +
> >  #######################################################################
> >  # Target-independent parts used in system and user emulation
> >  common-obj-y += qemu-log.o
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 751d3c2..424434f 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -3505,3 +3505,34 @@
> >      '*asl_compiler_rev':  'uint32',
> >      '*file':              'str',
> >      '*data':              'str' }}
> > +
> > +# @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 314931e..757749c 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 "qapi-types.h"
> > +#include "qapi-visit.h"
> >  #include "qapi/visitor.h"
> >  #include "sysemu/arch_init.h"
> >  
> > @@ -194,23 +196,34 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> >      },
> >  };
> >  
> > +typedef struct X86RegisterInfo32 {
> > +    /* Name of register */
> > +    const char *name;
> > +    /* QAPI enum value register */
> > +    X86CPURegister32 qapi_enum;
> > +} X86RegisterInfo32;
> > +
> > +#define REGISTER(reg) \
> > +    [R_##reg] = { .name = #reg, .qapi_enum = X86_C_P_U_REGISTER32_##reg }
>                                                 ^^^^
> Using auto-generated names like this probably is very fragile.

AFAIK, using the auto-generated name is the only way of using qapi enums
and the qapi code generator is supposed to never break this.


[...]
> > +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;
> > +    X86CPUFeatureWordInfo word_infos[FEATURE_WORDS] = { };
> > +    X86CPUFeatureWordInfoList list_entries[FEATURE_WORDS] = { };
> > +    X86CPUFeatureWordInfoList *list = NULL;
> > +
> > +    for (w = 0; w < FEATURE_WORDS; w++) {
> > +        FeatureWordInfo *wi = &feature_word_info[w];
> > +        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;
> Is there way not to use qapi_enum at all and use name instead?

Are you suggesting making the qapi interface be string-based instead of
using an enum? Why?


> 
> > +        qwi->features = env->features[w];
> > +
> > +        /* List will be in reverse order, but order shouldn't matter */
> > +        list_entries[w].next = list;
> > +        list_entries[w].value = &word_infos[w];
> list_entries[w].value = qwi;

Thanks, I will fix this.

> 
> > +        list = &list_entries[w];
> > +    }
> > +
> > +    visit_type_X86CPUFeatureWordInfoList(v, &list, "feature-words", &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;
> > @@ -2348,6 +2391,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", "X86CPUFeatureWordInfo",
> > +                        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
> > 
> > 
> 
> 
> -- 
> Regards,
>   Igor

-- 
Eduardo

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

* Re: [Qemu-devel] [PATCH qom-cpu 6/9] target-i386: Add "feature-words" property
  2013-05-03 13:17     ` Eduardo Habkost
@ 2013-05-03 14:25       ` Eric Blake
  0 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2013-05-03 14:25 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: libvir-list, Igor Mammedov, Jiri Denemark, qemu-devel,
	Andreas Färber

[-- Attachment #1: Type: text/plain, Size: 1957 bytes --]

On 05/03/2013 07:17 AM, Eduardo Habkost wrote:
> 
> We could, but maybe it would make the interface harder to use and not
> easier?
> 
> Even when two feature words are returned in the same CPUID leaf, they
> are independent and separate feature-words that must be checked
> individually by libvirt, so I believe returning one feature-word per
> array-item makes more sense. Having an extra item in the array would
> make it clear for libvirt that QEMU has a new feature-word that libvirt
> doesn't know about, and easier to spot than an extra field in an
> existing array item.

Firmly agree - bundling multiple features into one array item is not nice.

> 
> 
>> item[5].CPUID: EAX=7h,ECX=0h
> 
> What would be the data type of this "CPUID" field? Are you suggesting
> returning a string to be parsed manually?

Anything that requires parsing to break into pieces on the receiving end
implies that it was not correctly represented in JSON in the first
place.  I'd much rather see it kept as multiple fields.

>>> +    for (w = 0; w < FEATURE_WORDS; w++) {
>>> +        FeatureWordInfo *wi = &feature_word_info[w];
>>> +        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;
>> Is there way not to use qapi_enum at all and use name instead?
> 
> Are you suggesting making the qapi interface be string-based instead of
> using an enum? Why?

enum-based is better than string based.  That way, when we add
introspection in qemu 1.6, libvirt can see what enum values to expect,
instead of having an open-ended set of strings with no idea what strings
will be present.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

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

* Re: [Qemu-devel] [PATCH qom-cpu 6/9] target-i386: Add "feature-words" property
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 6/9] target-i386: Add "feature-words" property Eduardo Habkost
  2013-04-22 20:37   ` [Qemu-devel] [libvirt] " Eric Blake
  2013-05-03 11:34   ` [Qemu-devel] " Igor Mammedov
@ 2013-05-03 14:57   ` Eric Blake
  2 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2013-05-03 14:57 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: libvir-list, Igor Mammedov, Jiri Denemark, qemu-devel,
	Andreas Färber

[-- Attachment #1: Type: text/plain, Size: 3758 bytes --]

On 04/22/2013 01:00 PM, Eduardo Habkost wrote:
> 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
> 

>   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

I'm guessing the corresponding JSON passed over QMP would look something
like:

[ ...
  { "cpuid-register": "ECX",
    "cpuid-input-eax": 1,
    "features": 2155880449 },
  { "cpuid-register": "EDX",
    "cpuid-input-eax": 1,
    "features": 126614521 } ]

which libvirt can reasonably parse.

> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v1 -> v2:
>  * Merge the non-qapi and qapi patches, to keep series simpler
>  * Use the feature word array series as base, so we don't have
>    to set the feature word values one-by-one in the code
>  * Change type name of property from "x86-cpu-feature-words" to
>    "X86CPUFeatureWordInfo"
>  * Remove cpu-qapi-schema.json and simply add the type definitions
>    to qapi-schema.json, to keep the changes simpler
>    * This required compiling qapi-types.o and qapi-visit.o into
>      *-user as well
> ---
>  .gitignore        |  2 ++
>  Makefile.objs     |  7 +++++-
>  qapi-schema.json  | 31 ++++++++++++++++++++++++
>  target-i386/cpu.c | 70 +++++++++++++++++++++++++++++++++++++++++++++----------
>  4 files changed, 97 insertions(+), 13 deletions(-)

I'm not sure I'm the best person to review cpu.c, but I can at least
review the interface from what libvirt plans on using:

> +++ b/qapi-schema.json
> @@ -3505,3 +3505,34 @@
>      '*asl_compiler_rev':  'uint32',
>      '*file':              'str',
>      '*data':              'str' }}
> +
> +# @X86CPURegister32
> +#
> +# A X86 32-bit register
> +#
> +# Since: 1.5

Yes, I'd still like to get this into 1.5.  On some enums, we have called
out doc-text for each enum value; but I'm fine with your choice here to
omit that.

> +##
> +{ 'enum': 'X86CPURegister32',
> +  'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ] }

Any reason you favored ALL-CAPS names?  But it doesn't matter to me, as
long as we stick to the name once baked into a release.

> +
> +##
> +# @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' } }

Looks reasonable for the interface side of things.

>  
> +static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque,
> +                                   const char *name, Error **errp)

Indentation looks off.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

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

* Re: [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property
  2013-05-02 19:48     ` Eric Blake
@ 2013-05-03 14:58       ` Andreas Färber
  2013-05-03 15:23         ` Igor Mammedov
  2013-05-03 15:31         ` Eric Blake
  0 siblings, 2 replies; 32+ messages in thread
From: Andreas Färber @ 2013-05-03 14:58 UTC (permalink / raw)
  To: Eric Blake, Eduardo Habkost
  Cc: qemu-devel, libvir-list, Markus Armbruster, Luiz Capitulino,
	Anthony Liguori, Igor Mammedov, Jiri Denemark

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 02.05.2013 21:48, schrieb Eric Blake:
> On 05/02/2013 01:43 PM, Eduardo Habkost wrote:
>>> 
>>> As mentioned earlier I'd prefer to defer the property design
>>> rather than putting it lightly reviewed into 1.5 and living
>>> with some ABI. If libvirt urgently needs this info, this series
>>> needs to be reviewed and sorted out until the weekend (Hard
>>> Freeze on Monday).
>> 
>> I consider it an important bugfix for the QEMU+libvirt stack.
>> The current libvirt behavior (checking CPUID directly; not using
>> the "enforce" flag; and having its own copy of each CPU model
>> definition) is unsafe and may break live-migration silently under
>> many circumstances.
> 
> I agree that libvirt would very much like to have this in 1.5.  How
> can I help in reviewing things?

Apart from the usual QMP considerations that you will know much better
than me, I have two concerns here:
1) Polluting the QOM namespace with this dump-all implementation for
libvirt and interfering with more fine-grained property getters/setters.
2) Basing its design on current code of which we are not sure yet how
it may evolve and having to live with that for ABI stability.
Like I said, I hadn't reviewed that part yet, so couldn't pick it up
on short notice. If we get it respun and reviewed today, I can (try
to) prepare a PULL on Sunday.

On Igor's series (latest: v7 from Feb 25) I had more or less nack'ed
the attempt to introduce f-* properties due to Anthony asking for
verbose QOM property names, so we're in need of a better name, likely
something with "feature" in it, similar to what is being proposed here.
I had also argued with Anthony that QOM's object_property_add_bool()
should allow us to create a container object for accessing features in
a more simple way, such as .../icc/child[0]/cpuid-features/foo rather
than f-foo or feature-foo or foo-feature to avoid the constant
repetition and an unreadable long list of CPU properties, but the
addition of an opaque to support this was turned down.

So it boils down to the questions of where do we want to expose which
information, how should it be structured and where does/will that
information come from. Thanks.

Regards,
Andreas

- -- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)

iQIcBAEBAgAGBQJRg9CkAAoJEPou0S0+fgE/Mu8P/1FFoXTMawQ2o8np/cjOFEze
zv+MJ5DUKZK96PPNoZjsM8y0tmNZ8VT9q578AQuElQiA/AbOaUqEoqL/NB9i9Bqc
PBZw7KwgNkH8Mogw7izOmOybKZbshdin9uBxRugG+Xyg5Nk7oMYkTQV8PLHmAgRc
LxgeMAJHsPY9LXksCNUbZNblK//EQfP90e7v0fU+ys5xrlCFlCl1xRQd9Cw2QvHd
7gECUSlwOlkHY32BFEn/epqay45uZqlECyGXDqrssg5htLM5McbzKCa1sgdQbuqp
HqsO3WdM6jBrse5EApxdoaYmz8Yhl6ls+YOQY+l3DjjhHNcDzxtIqbAK36ErBHFz
9d+NTcXBlGrC0N0L7VZmwLihJ3bT/IIEP7ybLFN/QKHlz4H83pEGftbBpPipqrwq
NZWk7Z6IiOKptxNyBKOa04+2DJvlafgwjysfTf5bjEQ+WDTEeMoubIOZiG9bC1bm
WdqAC6JzQYTpjT3kqbfxGlV8328N3Z1qrVpRZOevkPHpotaaSDa5VVSCOvj6hdJZ
P4L2hq94bskumINJWHZxYEGvrB+6MJfOn73icNSpzyg+2sVw2QVfVAfbe0XfGFag
2JO5sFbl0be8rOh6Y7b2uxltfI1RnGIBmemRQkjP6Z3mynLs7EYKqs8LwpHR0FMm
3oSPrNXELr02m/9eGpsb
=tUDY
-----END PGP SIGNATURE-----

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

* Re: [Qemu-devel] [PATCH qom-cpu 7/9] target-i386: Use FeatureWord loop on filter_features_for_kvm()
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 7/9] target-i386: Use FeatureWord loop on filter_features_for_kvm() Eduardo Habkost
@ 2013-05-03 15:01   ` Eric Blake
  2013-05-06 16:28     ` Andreas Färber
  0 siblings, 1 reply; 32+ messages in thread
From: Eric Blake @ 2013-05-03 15:01 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: libvir-list, Igor Mammedov, Jiri Denemark, qemu-devel,
	Andreas Färber

[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]

On 04/22/2013 01:00 PM, Eduardo Habkost wrote:
> Instead of open-coding the filtering code for each feature word, change
> the existing code to use the feature_word_info array, that have exactly
> the same CPUID eax/ecx/register values for each feature word.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target-i386/cpu.c | 24 +++++++-----------------
>  1 file changed, 7 insertions(+), 17 deletions(-)
> 

> +    for (w = 0; w < FEATURE_WORDS; w++) {
> +        FeatureWordInfo *wi = &feature_word_info[w];
> +        env->features[w] &= kvm_arch_get_supported_cpuid(s, wi->cpuid_eax,
> +                                                            wi->cpuid_ecx,
> +                                                            wi->cpuid_reg);

Indentation is unusual, but the resulting alignment is nicer than having
'wi->' flush under 's'.  I would have written the call in four lines
instead of 3, but that's not essential.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

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

* Re: [Qemu-devel] [PATCH qom-cpu 8/9] target-i386: Introduce X86CPU.filtered_features field
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 8/9] target-i386: Introduce X86CPU.filtered_features field Eduardo Habkost
@ 2013-05-03 15:03   ` Eric Blake
  0 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2013-05-03 15:03 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: libvir-list, Igor Mammedov, Jiri Denemark, qemu-devel,
	Andreas Färber

[-- Attachment #1: Type: text/plain, Size: 1546 bytes --]

On 04/22/2013 01:00 PM, Eduardo Habkost wrote:
> This field will contain the feature bits that were filtered out because
> of missing host support.

Yes, libvirt would definitely like to know that.

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

Reviewed-by: Eric Blake <eblake@redhat.com>

> +++ b/target-i386/cpu.c
> @@ -1642,9 +1642,12 @@ static void filter_features_for_kvm(X86CPU *cpu)
>  
>      for (w = 0; w < FEATURE_WORDS; w++) {
>          FeatureWordInfo *wi = &feature_word_info[w];
> -        env->features[w] &= kvm_arch_get_supported_cpuid(s, wi->cpuid_eax,
> -                                                            wi->cpuid_ecx,
> -                                                            wi->cpuid_reg);
> +        uint32_t host_feat = kvm_arch_get_supported_cpuid(s, wi->cpuid_eax,
> +                                                             wi->cpuid_ecx,
> +                                                             wi->cpuid_reg);

Alignment is still "interesting", but still no impact to the patch review.

> +        uint32_t requested_features = env->features[w];
> +        env->features[w] &= host_feat;
> +        cpu->filtered_features[w] = requested_features & ~env->features[w];
>      }
>  }
>  #endif
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

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

* Re: [Qemu-devel] [PATCH qom-cpu 9/9] target-i386: Add "filtered-features" property to X86CPU
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 9/9] target-i386: Add "filtered-features" property to X86CPU Eduardo Habkost
@ 2013-05-03 15:10   ` Eric Blake
  0 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2013-05-03 15:10 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: libvir-list, Igor Mammedov, Jiri Denemark, qemu-devel,
	Andreas Färber

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

On 04/22/2013 01:00 PM, Eduardo Habkost wrote:
> This property will contain all the features that were removed from the
> CPU because they are not supported by the host.
> 
> This way, libvirt or other management tools can emulate the
> check/enforce behavior by checking if filtered-properties is all zeroes,
> before starting the guest.
> 

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

Definitely wanted by libvirt, and I hope it can be included in 1.5.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

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

* Re: [Qemu-devel] [PATCH qom-cpu 5/9] target-i386: Add ECX information to FeatureWordInfo
  2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 5/9] target-i386: Add ECX information to FeatureWordInfo Eduardo Habkost
@ 2013-05-03 15:16   ` Andreas Färber
  2013-05-03 15:54     ` Eduardo Habkost
  0 siblings, 1 reply; 32+ messages in thread
From: Andreas Färber @ 2013-05-03 15:16 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: libvir-list, Igor Mammedov, Jiri Denemark, qemu-devel

Am 22.04.2013 21:00, schrieb 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 110ef98..314931e 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 */

Why do we need this needs_ field here? eax and reg just seem to be
reindented and the comment reworded for reg.

It just seems to be passed through to has_cpuid_input_ecx, which I
neither see defined nor checked - that data structure already existed
elsewhere as such?

Andreas

> +    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,
>      },
>  };
>  
> 


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

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

* Re: [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property
  2013-05-03 14:58       ` Andreas Färber
@ 2013-05-03 15:23         ` Igor Mammedov
  2013-05-03 15:31         ` Eric Blake
  1 sibling, 0 replies; 32+ messages in thread
From: Igor Mammedov @ 2013-05-03 15:23 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Eduardo Habkost, libvir-list, Markus Armbruster, qemu-devel,
	Luiz Capitulino, Anthony Liguori, Jiri Denemark

On Fri, 03 May 2013 16:58:44 +0200
Andreas Färber <afaerber@suse.de> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Am 02.05.2013 21:48, schrieb Eric Blake:
> > On 05/02/2013 01:43 PM, Eduardo Habkost wrote:
> >>> 
> >>> As mentioned earlier I'd prefer to defer the property design
> >>> rather than putting it lightly reviewed into 1.5 and living
> >>> with some ABI. If libvirt urgently needs this info, this series
> >>> needs to be reviewed and sorted out until the weekend (Hard
> >>> Freeze on Monday).
> >> 
> >> I consider it an important bugfix for the QEMU+libvirt stack.
> >> The current libvirt behavior (checking CPUID directly; not using
> >> the "enforce" flag; and having its own copy of each CPU model
> >> definition) is unsafe and may break live-migration silently under
> >> many circumstances.
> > 
> > I agree that libvirt would very much like to have this in 1.5.  How
> > can I help in reviewing things?
> 
> Apart from the usual QMP considerations that you will know much better
> than me, I have two concerns here:
> 1) Polluting the QOM namespace with this dump-all implementation for
> libvirt and interfering with more fine-grained property getters/setters.
I think feature-words could be replaced with fine-grained feature properties
eventually.

> 2) Basing its design on current code of which we are not sure yet how
> it may evolve and having to live with that for ABI stability.
> Like I said, I hadn't reviewed that part yet, so couldn't pick it up
> on short notice. If we get it respun and reviewed today, I can (try
> to) prepare a PULL on Sunday.
> 
> On Igor's series (latest: v7 from Feb 25) I had more or less nack'ed
> the attempt to introduce f-* properties due to Anthony asking for
I don't recall it being nacked or any other way commented.

> verbose QOM property names, so we're in need of a better name, likely
> something with "feature" in it, similar to what is being proposed here.
> I had also argued with Anthony that QOM's object_property_add_bool()
> should allow us to create a container object for accessing features in
> a more simple way, such as .../icc/child[0]/cpuid-features/foo rather
> than f-foo or feature-foo or foo-feature to avoid the constant
> repetition and an unreadable long list of CPU properties, but the
> addition of an opaque to support this was turned down.
what if FeatureWordArray inherits from Object?
than it would be trivial to create /icc/child[0]/cpuid-features/ where
"cpuid-features" will be FeatureWordArray and each feature it's child?

> So it boils down to the questions of where do we want to expose which
> information, how should it be structured and where does/will that
> information come from. Thanks.
> 
> Regards,
> Andreas
> 
> - -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.19 (GNU/Linux)
> 
> iQIcBAEBAgAGBQJRg9CkAAoJEPou0S0+fgE/Mu8P/1FFoXTMawQ2o8np/cjOFEze
> zv+MJ5DUKZK96PPNoZjsM8y0tmNZ8VT9q578AQuElQiA/AbOaUqEoqL/NB9i9Bqc
> PBZw7KwgNkH8Mogw7izOmOybKZbshdin9uBxRugG+Xyg5Nk7oMYkTQV8PLHmAgRc
> LxgeMAJHsPY9LXksCNUbZNblK//EQfP90e7v0fU+ys5xrlCFlCl1xRQd9Cw2QvHd
> 7gECUSlwOlkHY32BFEn/epqay45uZqlECyGXDqrssg5htLM5McbzKCa1sgdQbuqp
> HqsO3WdM6jBrse5EApxdoaYmz8Yhl6ls+YOQY+l3DjjhHNcDzxtIqbAK36ErBHFz
> 9d+NTcXBlGrC0N0L7VZmwLihJ3bT/IIEP7ybLFN/QKHlz4H83pEGftbBpPipqrwq
> NZWk7Z6IiOKptxNyBKOa04+2DJvlafgwjysfTf5bjEQ+WDTEeMoubIOZiG9bC1bm
> WdqAC6JzQYTpjT3kqbfxGlV8328N3Z1qrVpRZOevkPHpotaaSDa5VVSCOvj6hdJZ
> P4L2hq94bskumINJWHZxYEGvrB+6MJfOn73icNSpzyg+2sVw2QVfVAfbe0XfGFag
> 2JO5sFbl0be8rOh6Y7b2uxltfI1RnGIBmemRQkjP6Z3mynLs7EYKqs8LwpHR0FMm
> 3oSPrNXELr02m/9eGpsb
> =tUDY
> -----END PGP SIGNATURE-----


-- 
Regards,
  Igor

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

* Re: [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property
  2013-05-03 14:58       ` Andreas Färber
  2013-05-03 15:23         ` Igor Mammedov
@ 2013-05-03 15:31         ` Eric Blake
  1 sibling, 0 replies; 32+ messages in thread
From: Eric Blake @ 2013-05-03 15:31 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Eduardo Habkost, libvir-list, Markus Armbruster, qemu-devel,
	Luiz Capitulino, Anthony Liguori, Igor Mammedov, Jiri Denemark

[-- Attachment #1: Type: text/plain, Size: 3854 bytes --]

On 05/03/2013 08:58 AM, Andreas Färber wrote:
> 
>> I agree that libvirt would very much like to have this in 1.5.  How
>> can I help in reviewing things?
> 
> Apart from the usual QMP considerations that you will know much better
> than me, I have two concerns here:
> 1) Polluting the QOM namespace with this dump-all implementation for
> libvirt and interfering with more fine-grained property getters/setters.

Ultimately, it would be nice to have full QOM introspection.  We are
part way there: we know WHAT properties we can query ('qom-list' exists
now), but right now it returns details on the type of a property as a
string, rather than as a full-blown JSON representation of the full
details of that type.  Maybe if we had an additional QMP command that
gave back a full JSON representation of any type, so you can feed the
'type':'str' field from ObjectPropertyInfo in 'qom-list' into the new
command to get a full idea of what each property contains.  With full
introspection, we could then have the option of changing the layout of
the feature-words and filtered-features properties, but if we do make a
change in the future, it would be nice to remain backwards compatible
(adding features, rather than removing).

> 2) Basing its design on current code of which we are not sure yet how
> it may evolve and having to live with that for ABI stability.
> Like I said, I hadn't reviewed that part yet, so couldn't pick it up
> on short notice. If we get it respun and reviewed today, I can (try
> to) prepare a PULL on Sunday.

I guess I see where you are coming from - once we bake in the QOM
property name and libvirt starts relying on it, it becomes harder to
support the generation of that property in the future if the underlying
implementation of how feature bits are represented in qemu changed.  But
the hope is that we have already sanitized the feature word generation
into something that is a lot more maintainable (iterating over a struct
of descriptions of how to get at each feature), and where future changes
would merely be extending (not deleting) from that struct (and therefore
making the corresponding JSON type returned via the QOM property
larger).  And since these two properties are read-only, extending the
JSON struct shouldn't be a problem.

> 
> On Igor's series (latest: v7 from Feb 25) I had more or less nack'ed
> the attempt to introduce f-* properties due to Anthony asking for
> verbose QOM property names, so we're in need of a better name, likely
> something with "feature" in it, similar to what is being proposed here.
> I had also argued with Anthony that QOM's object_property_add_bool()
> should allow us to create a container object for accessing features in
> a more simple way, such as .../icc/child[0]/cpuid-features/foo rather
> than f-foo or feature-foo or foo-feature to avoid the constant
> repetition and an unreadable long list of CPU properties, but the
> addition of an opaque to support this was turned down.

The problem with adding container objects is that you can only access
one feature at a time, instead of all of them in a single array.  I
guess libvirt would cope with either style, but it is nicer to be able
to get everything at once via JSON struct than it is to have to make
multiple qom-get calls.

> 
> So it boils down to the questions of where do we want to expose which
> information, how should it be structured and where does/will that
> information come from. Thanks.

I guess that decision is up to the qemu maintainers - all I can do is
say whether the proposed interface is usable, not whether it is the
ideal interface compared to some other layout of where the property is
attached.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

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

* Re: [Qemu-devel] [PATCH qom-cpu 5/9] target-i386: Add ECX information to FeatureWordInfo
  2013-05-03 15:16   ` Andreas Färber
@ 2013-05-03 15:54     ` Eduardo Habkost
  2013-05-06 16:27       ` Andreas Färber
  0 siblings, 1 reply; 32+ messages in thread
From: Eduardo Habkost @ 2013-05-03 15:54 UTC (permalink / raw)
  To: Andreas Färber; +Cc: libvir-list, Igor Mammedov, Jiri Denemark, qemu-devel

On Fri, May 03, 2013 at 05:16:46PM +0200, Andreas Färber wrote:
> Am 22.04.2013 21:00, schrieb 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 110ef98..314931e 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 */
> 
> Why do we need this needs_ field here? eax and reg just seem to be
> reindented and the comment reworded for reg.

We need it so we know if ECX is used as input on that CPUID leaf, in
other words: so we know if "cpuid-input-ecx" needs to be set on
X86CPUFeatureWordInfo or not.

"cpuid-input-ecx" is optional because most CPUID leafs don't depend on
input ECX value, only EAX. cpuid_needs_ecx has a similar meaning to
KVM_CPUID_FLAG_SIGNIFCANT_INDEX on kvm_cpuid_entry2.flags.

> 
> It just seems to be passed through to has_cpuid_input_ecx, which I
> neither see defined nor checked - that data structure already existed
> elsewhere as such?

X86CPUFeatureWordInfo is generated from the QAPI schema.

> 
> Andreas
> 
> > +    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,
> >      },
> >  };
> >  
> > 
> 
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

-- 
Eduardo

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

* Re: [Qemu-devel] [PATCH qom-cpu 5/9] target-i386: Add ECX information to FeatureWordInfo
  2013-05-03 15:54     ` Eduardo Habkost
@ 2013-05-06 16:27       ` Andreas Färber
  0 siblings, 0 replies; 32+ messages in thread
From: Andreas Färber @ 2013-05-06 16:27 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: libvir-list, Igor Mammedov, Jiri Denemark, qemu-devel

Am 03.05.2013 17:54, schrieb Eduardo Habkost:
> On Fri, May 03, 2013 at 05:16:46PM +0200, Andreas Färber wrote:
>> Am 22.04.2013 21:00, schrieb 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>

Thanks for the explanations, applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas

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

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

* Re: [Qemu-devel] [PATCH qom-cpu 7/9] target-i386: Use FeatureWord loop on filter_features_for_kvm()
  2013-05-03 15:01   ` Eric Blake
@ 2013-05-06 16:28     ` Andreas Färber
  0 siblings, 0 replies; 32+ messages in thread
From: Andreas Färber @ 2013-05-06 16:28 UTC (permalink / raw)
  To: Eric Blake, Eduardo Habkost
  Cc: libvir-list, Igor Mammedov, Jiri Denemark, qemu-devel

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 03.05.2013 17:01, schrieb Eric Blake:
> On 04/22/2013 01:00 PM, Eduardo Habkost wrote:
>> Instead of open-coding the filtering code for each feature word,
>> change the existing code to use the feature_word_info array, that
>> have exactly the same CPUID eax/ecx/register values for each
>> feature word.
>> 
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- 
>> target-i386/cpu.c | 24 +++++++----------------- 1 file changed, 7
>> insertions(+), 17 deletions(-)
>> 
> 
>> +    for (w = 0; w < FEATURE_WORDS; w++) { +
>> FeatureWordInfo *wi = &feature_word_info[w]; +
>> env->features[w] &= kvm_arch_get_supported_cpuid(s,
>> wi->cpuid_eax, +
>> wi->cpuid_ecx, +
>> wi->cpuid_reg);
> 
> Indentation is unusual, but the resulting alignment is nicer than
> having 'wi->' flush under 's'.  I would have written the call in
> four lines instead of 3, but that's not essential.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks, applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas

- -- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)

iQIcBAEBAgAGBQJRh9oXAAoJEPou0S0+fgE/oksP/1SKyK1oTJs9l7GG5atYxav7
7bDM/Pf4wTx63vs19XmvoaKCBBHYVhxM8RhXYRBFn1SHQhiXiJmgTpNDTpE9L+yq
CpoXTyL/oReB32cUEeZYBjYWdG62IV8Ci09+FUhhmf4cduBQTXFV7i5jq/9IDooi
fMUoksVGHeLOHvsUPFp3OSlO24KRCO0z/w8O74/fPzdrmrz3cMRxy7V59/ZHsNPg
lAkXiFu56whYcA62A76HvRW200P75I8BuLq7NCPffFwMf9/N6eba7Wep0J1xnzpG
GkXB7swuOENKRVPbZ6XKrCIAH96kk6WbJaXPrPplP6jATXhhuChlyyskrEz++hne
XjjFAGOLFGrLuD+3eIB4VZ509GE+hUfyAG6JQWf+7BG20scepvsOgaPT/rvNyDjN
LQnL/BJoV1+s4YKuZCIv3km/WQ76oNRNSlmA+R+KKCEfjOsl8Byg3L6/YSE4TaPs
RiNXZTh0HkcyMzTYNdEOcxZcpW0G77jRAV7L3fMPd0TMixY8oKsqW4wg9pvZodLg
yeCRn1N+op9HDj9W9qAq8IXwJiuLEKpCu2hqzVpYJRxws7ZAo7tfI5+ym1AMj4FZ
pEb87EXqEqESzeWJS4XJZxaPw14WaWInWkNXhdq0J3J78UkB2pHnvAr2dDL9FvPx
9o6UzZiavkgcd860Im4J
=4ouF
-----END PGP SIGNATURE-----

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

end of thread, other threads:[~2013-05-06 16:28 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-22 19:00 [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property Eduardo Habkost
2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 1/9] target-i386: cleanup: Group together level, xlevel, xlevel2 fields Eduardo Habkost
2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 2/9] target-i386/kvm.c: Code formatting changes Eduardo Habkost
2013-05-01 22:55   ` Andreas Färber
2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 3/9] target-i386/cpu.c: Break lines so they don't get too long Eduardo Habkost
2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 4/9] target-i386: Replace cpuid_*features fields with a feature word array Eduardo Habkost
2013-05-01 23:03   ` Andreas Färber
2013-05-02 15:06     ` Eduardo Habkost
2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 5/9] target-i386: Add ECX information to FeatureWordInfo Eduardo Habkost
2013-05-03 15:16   ` Andreas Färber
2013-05-03 15:54     ` Eduardo Habkost
2013-05-06 16:27       ` Andreas Färber
2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 6/9] target-i386: Add "feature-words" property Eduardo Habkost
2013-04-22 20:37   ` [Qemu-devel] [libvirt] " Eric Blake
2013-04-23 19:25     ` Eduardo Habkost
2013-05-03 11:34   ` [Qemu-devel] " Igor Mammedov
2013-05-03 13:17     ` Eduardo Habkost
2013-05-03 14:25       ` Eric Blake
2013-05-03 14:57   ` Eric Blake
2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 7/9] target-i386: Use FeatureWord loop on filter_features_for_kvm() Eduardo Habkost
2013-05-03 15:01   ` Eric Blake
2013-05-06 16:28     ` Andreas Färber
2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 8/9] target-i386: Introduce X86CPU.filtered_features field Eduardo Habkost
2013-05-03 15:03   ` Eric Blake
2013-04-22 19:00 ` [Qemu-devel] [PATCH qom-cpu 9/9] target-i386: Add "filtered-features" property to X86CPU Eduardo Habkost
2013-05-03 15:10   ` Eric Blake
2013-05-01 22:53 ` [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property Andreas Färber
2013-05-02 19:43   ` Eduardo Habkost
2013-05-02 19:48     ` Eric Blake
2013-05-03 14:58       ` Andreas Färber
2013-05-03 15:23         ` Igor Mammedov
2013-05-03 15:31         ` Eric Blake

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