* [Qemu-devel] [PATCH 0/7] target-i386: xsave CPUID handling refactor
@ 2016-09-23 19:45 Eduardo Habkost
2016-09-23 19:45 ` [Qemu-devel] [PATCH 1/7] target-i386: Move feature name arrays inside FeatureWordInfo Eduardo Habkost
` (7 more replies)
0 siblings, 8 replies; 17+ messages in thread
From: Eduardo Habkost @ 2016-09-23 19:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Richard Henderson, Igor Mammedov
This series refactor the xsave CPUID handling so it won't
silently disable any XSAVE components on CPUID[0xD] in case the
host doesn't support it. It will instead use the exisitng
check/enforce logic for filtering the CPUID bits and checking for
host-side support.
This series is available on git at:
https://github.com/ehabkost/qemu-hacks.git work/xsave-cpuid-cleanup
The series is based on my x86-next branch, that contains other
CPUID-related changes:
https://github.com/ehabkost/qemu.git x8-next
Eduardo Habkost (7):
target-i386: Move feature name arrays inside FeatureWordInfo
target-i386: Don't try to enable PT State xsave component
target-i386: xsave: Calculate enabled components only once
target-i386: xsave: Simplify CPUID[0xD,0].{EAX,EDX} calculation
target-i386: xsave: Helper function to calculate xsave area size
target-i386: xsave: Calculate set of xsave components on realize
target-i386: Move xsave component mask to features array
target-i386/cpu.c | 457 +++++++++++++++++++++++++++---------------------------
target-i386/cpu.h | 2 +
2 files changed, 230 insertions(+), 229 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 1/7] target-i386: Move feature name arrays inside FeatureWordInfo
2016-09-23 19:45 [Qemu-devel] [PATCH 0/7] target-i386: xsave CPUID handling refactor Eduardo Habkost
@ 2016-09-23 19:45 ` Eduardo Habkost
2016-09-23 20:01 ` Richard Henderson
2016-09-23 19:45 ` [Qemu-devel] [PATCH 2/7] target-i386: Don't try to enable PT State xsave component Eduardo Habkost
` (6 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Eduardo Habkost @ 2016-09-23 19:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Richard Henderson, Igor Mammedov
It makes it easier to guarantee the arrays are the right size,
and to find information when looking at the code.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 370 +++++++++++++++++++++++++-----------------------------
1 file changed, 170 insertions(+), 200 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index a5d3b1a..cc07fdb 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -181,185 +181,6 @@ static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
dst[CPUID_VENDOR_SZ] = '\0';
}
-/* feature flags taken from "Intel Processor Identification and the CPUID
- * Instruction" and AMD's "CPUID Specification". In cases of disagreement
- * between feature naming conventions, aliases may be added.
- */
-static const char *feature_name[] = {
- "fpu", "vme", "de", "pse",
- "tsc", "msr", "pae", "mce",
- "cx8", "apic", NULL, "sep",
- "mtrr", "pge", "mca", "cmov",
- "pat", "pse36", "pn" /* Intel psn */, "clflush" /* Intel clfsh */,
- NULL, "ds" /* Intel dts */, "acpi", "mmx",
- "fxsr", "sse", "sse2", "ss",
- "ht" /* Intel htt */, "tm", "ia64", "pbe",
-};
-static const char *ext_feature_name[] = {
- "pni|sse3" /* Intel,AMD sse3 */, "pclmulqdq|pclmuldq", "dtes64", "monitor",
- "ds_cpl", "vmx", "smx", "est",
- "tm2", "ssse3", "cid", NULL,
- "fma", "cx16", "xtpr", "pdcm",
- NULL, "pcid", "dca", "sse4.1|sse4_1",
- "sse4.2|sse4_2", "x2apic", "movbe", "popcnt",
- "tsc-deadline", "aes", "xsave", "osxsave",
- "avx", "f16c", "rdrand", "hypervisor",
-};
-/* Feature names that are already defined on feature_name[] but are set on
- * CPUID[8000_0001].EDX on AMD CPUs don't have their names on
- * ext2_feature_name[]. They are copied automatically to cpuid_ext2_features
- * if and only if CPU vendor is AMD.
- */
-static const char *ext2_feature_name[] = {
- NULL /* fpu */, NULL /* vme */, NULL /* de */, NULL /* pse */,
- NULL /* tsc */, NULL /* msr */, NULL /* pae */, NULL /* mce */,
- NULL /* cx8 */ /* AMD CMPXCHG8B */, NULL /* apic */, NULL, "syscall",
- NULL /* mtrr */, NULL /* pge */, NULL /* mca */, NULL /* cmov */,
- NULL /* pat */, NULL /* pse36 */, NULL, NULL /* Linux mp */,
- "nx|xd", NULL, "mmxext", NULL /* mmx */,
- NULL /* fxsr */, "fxsr_opt|ffxsr", "pdpe1gb" /* AMD Page1GB */, "rdtscp",
- NULL, "lm|i64", "3dnowext", "3dnow",
-};
-static const char *ext3_feature_name[] = {
- "lahf_lm" /* AMD LahfSahf */, "cmp_legacy", "svm", "extapic" /* AMD ExtApicSpace */,
- "cr8legacy" /* AMD AltMovCr8 */, "abm", "sse4a", "misalignsse",
- "3dnowprefetch", "osvw", "ibs", "xop",
- "skinit", "wdt", NULL, "lwp",
- "fma4", "tce", NULL, "nodeid_msr",
- NULL, "tbm", "topoext", "perfctr_core",
- "perfctr_nb", NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
-};
-
-static const char *ext4_feature_name[] = {
- NULL, NULL, "xstore", "xstore-en",
- NULL, NULL, "xcrypt", "xcrypt-en",
- "ace2", "ace2-en", "phe", "phe-en",
- "pmm", "pmm-en", NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
-};
-
-static const char *kvm_feature_name[] = {
- "kvmclock", "kvm_nopiodelay", "kvm_mmu", "kvmclock",
- "kvm_asyncpf", "kvm_steal_time", "kvm_pv_eoi", "kvm_pv_unhalt",
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- "kvmclock-stable-bit", NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
-};
-
-static const char *hyperv_priv_feature_name[] = {
- NULL /* hv_msr_vp_runtime_access */, NULL /* hv_msr_time_refcount_access */,
- NULL /* hv_msr_synic_access */, NULL /* hv_msr_stimer_access */,
- NULL /* hv_msr_apic_access */, NULL /* hv_msr_hypercall_access */,
- NULL /* hv_vpindex_access */, NULL /* hv_msr_reset_access */,
- NULL /* hv_msr_stats_access */, NULL /* hv_reftsc_access */,
- NULL /* hv_msr_idle_access */, NULL /* hv_msr_frequency_access */,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
-};
-
-static const char *hyperv_ident_feature_name[] = {
- NULL /* hv_create_partitions */, NULL /* hv_access_partition_id */,
- NULL /* hv_access_memory_pool */, NULL /* hv_adjust_message_buffers */,
- NULL /* hv_post_messages */, NULL /* hv_signal_events */,
- NULL /* hv_create_port */, NULL /* hv_connect_port */,
- NULL /* hv_access_stats */, NULL, NULL, NULL /* hv_debugging */,
- NULL /* hv_cpu_power_management */, NULL /* hv_configure_profiler */,
- NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
-};
-
-static const char *hyperv_misc_feature_name[] = {
- NULL /* hv_mwait */, NULL /* hv_guest_debugging */,
- NULL /* hv_perf_monitor */, NULL /* hv_cpu_dynamic_part */,
- NULL /* hv_hypercall_params_xmm */, NULL /* hv_guest_idle_state */,
- NULL, NULL,
- NULL, NULL, NULL /* hv_guest_crash_msr */, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
-};
-
-static const char *svm_feature_name[] = {
- "npt", "lbrv", "svm_lock", "nrip_save",
- "tsc_scale", "vmcb_clean", "flushbyasid", "decodeassists",
- NULL, NULL, "pause_filter", NULL,
- "pfthreshold", NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
-};
-
-static const char *cpuid_7_0_ebx_feature_name[] = {
- "fsgsbase", "tsc_adjust", NULL, "bmi1",
- "hle", "avx2", NULL, "smep",
- "bmi2", "erms", "invpcid", "rtm",
- NULL, NULL, "mpx", NULL,
- "avx512f", "avx512dq", "rdseed", "adx",
- "smap", "avx512ifma", "pcommit", "clflushopt",
- "clwb", NULL, "avx512pf", "avx512er",
- "avx512cd", NULL, "avx512bw", "avx512vl",
-};
-
-static const char *cpuid_7_0_ecx_feature_name[] = {
- NULL, "avx512vbmi", "umip", "pku",
- "ospke", NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, "rdpid", NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
-};
-
-static const char *cpuid_apm_edx_feature_name[] = {
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- "invtsc", NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
-};
-
-static const char *cpuid_xsave_feature_name[] = {
- "xsaveopt", "xsavec", "xgetbv1", "xsaves",
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
-};
-
-static const char *cpuid_6_feature_name[] = {
- NULL, NULL, "arat", NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
- NULL, NULL, NULL, NULL,
-};
-
#define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
#define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC)
@@ -425,7 +246,12 @@ static const char *cpuid_6_feature_name[] = {
CPUID_XSAVE_XSAVEC, CPUID_XSAVE_XSAVES */
typedef struct FeatureWordInfo {
- const char **feat_names;
+ /* feature flags names are taken from "Intel Processor Identification and
+ * the CPUID Instruction" and AMD's "CPUID Specification".
+ * In cases of disagreement between feature naming conventions,
+ * aliases may be added.
+ */
+ const char *feat_names[32];
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 */
@@ -436,82 +262,230 @@ typedef struct FeatureWordInfo {
static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
[FEAT_1_EDX] = {
- .feat_names = feature_name,
+ .feat_names = {
+ "fpu", "vme", "de", "pse",
+ "tsc", "msr", "pae", "mce",
+ "cx8", "apic", NULL, "sep",
+ "mtrr", "pge", "mca", "cmov",
+ "pat", "pse36", "pn" /* Intel psn */, "clflush" /* Intel clfsh */,
+ NULL, "ds" /* Intel dts */, "acpi", "mmx",
+ "fxsr", "sse", "sse2", "ss",
+ "ht" /* Intel htt */, "tm", "ia64", "pbe",
+ },
.cpuid_eax = 1, .cpuid_reg = R_EDX,
.tcg_features = TCG_FEATURES,
},
[FEAT_1_ECX] = {
- .feat_names = ext_feature_name,
+ .feat_names = {
+ "pni|sse3" /* Intel,AMD sse3 */, "pclmulqdq|pclmuldq", "dtes64", "monitor",
+ "ds_cpl", "vmx", "smx", "est",
+ "tm2", "ssse3", "cid", NULL,
+ "fma", "cx16", "xtpr", "pdcm",
+ NULL, "pcid", "dca", "sse4.1|sse4_1",
+ "sse4.2|sse4_2", "x2apic", "movbe", "popcnt",
+ "tsc-deadline", "aes", "xsave", "osxsave",
+ "avx", "f16c", "rdrand", "hypervisor",
+ },
.cpuid_eax = 1, .cpuid_reg = R_ECX,
.tcg_features = TCG_EXT_FEATURES,
},
+ /* Feature names that are already defined on feature_name[] but
+ * are set on CPUID[8000_0001].EDX on AMD CPUs don't have their
+ * names on feat_names below. They are copied automatically
+ * to features[FEAT_8000_0001_EDX] if and only if CPU vendor is AMD.
+ */
[FEAT_8000_0001_EDX] = {
- .feat_names = ext2_feature_name,
+ .feat_names = {
+ NULL /* fpu */, NULL /* vme */, NULL /* de */, NULL /* pse */,
+ NULL /* tsc */, NULL /* msr */, NULL /* pae */, NULL /* mce */,
+ NULL /* cx8 */, NULL /* apic */, NULL, "syscall",
+ NULL /* mtrr */, NULL /* pge */, NULL /* mca */, NULL /* cmov */,
+ NULL /* pat */, NULL /* pse36 */, NULL, NULL /* Linux mp */,
+ "nx|xd", NULL, "mmxext", NULL /* mmx */,
+ NULL /* fxsr */, "fxsr_opt|ffxsr", "pdpe1gb", "rdtscp",
+ NULL, "lm|i64", "3dnowext", "3dnow",
+ },
.cpuid_eax = 0x80000001, .cpuid_reg = R_EDX,
.tcg_features = TCG_EXT2_FEATURES,
},
[FEAT_8000_0001_ECX] = {
- .feat_names = ext3_feature_name,
+ .feat_names = {
+ "lahf_lm", "cmp_legacy", "svm", "extapic",
+ "cr8legacy", "abm", "sse4a", "misalignsse",
+ "3dnowprefetch", "osvw", "ibs", "xop",
+ "skinit", "wdt", NULL, "lwp",
+ "fma4", "tce", NULL, "nodeid_msr",
+ NULL, "tbm", "topoext", "perfctr_core",
+ "perfctr_nb", NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ },
.cpuid_eax = 0x80000001, .cpuid_reg = R_ECX,
.tcg_features = TCG_EXT3_FEATURES,
},
[FEAT_C000_0001_EDX] = {
- .feat_names = ext4_feature_name,
+ .feat_names = {
+ NULL, NULL, "xstore", "xstore-en",
+ NULL, NULL, "xcrypt", "xcrypt-en",
+ "ace2", "ace2-en", "phe", "phe-en",
+ "pmm", "pmm-en", NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ },
.cpuid_eax = 0xC0000001, .cpuid_reg = R_EDX,
.tcg_features = TCG_EXT4_FEATURES,
},
[FEAT_KVM] = {
- .feat_names = kvm_feature_name,
+ .feat_names = {
+ "kvmclock", "kvm_nopiodelay", "kvm_mmu", "kvmclock",
+ "kvm_asyncpf", "kvm_steal_time", "kvm_pv_eoi", "kvm_pv_unhalt",
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ "kvmclock-stable-bit", NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ },
.cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EAX,
.tcg_features = TCG_KVM_FEATURES,
},
[FEAT_HYPERV_EAX] = {
- .feat_names = hyperv_priv_feature_name,
+ .feat_names = {
+ NULL /* hv_msr_vp_runtime_access */, NULL /* hv_msr_time_refcount_access */,
+ NULL /* hv_msr_synic_access */, NULL /* hv_msr_stimer_access */,
+ NULL /* hv_msr_apic_access */, NULL /* hv_msr_hypercall_access */,
+ NULL /* hv_vpindex_access */, NULL /* hv_msr_reset_access */,
+ NULL /* hv_msr_stats_access */, NULL /* hv_reftsc_access */,
+ NULL /* hv_msr_idle_access */, NULL /* hv_msr_frequency_access */,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ },
.cpuid_eax = 0x40000003, .cpuid_reg = R_EAX,
},
[FEAT_HYPERV_EBX] = {
- .feat_names = hyperv_ident_feature_name,
+ .feat_names = {
+ NULL /* hv_create_partitions */, NULL /* hv_access_partition_id */,
+ NULL /* hv_access_memory_pool */, NULL /* hv_adjust_message_buffers */,
+ NULL /* hv_post_messages */, NULL /* hv_signal_events */,
+ NULL /* hv_create_port */, NULL /* hv_connect_port */,
+ NULL /* hv_access_stats */, NULL, NULL, NULL /* hv_debugging */,
+ NULL /* hv_cpu_power_management */, NULL /* hv_configure_profiler */,
+ NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ },
.cpuid_eax = 0x40000003, .cpuid_reg = R_EBX,
},
[FEAT_HYPERV_EDX] = {
- .feat_names = hyperv_misc_feature_name,
+ .feat_names = {
+ NULL /* hv_mwait */, NULL /* hv_guest_debugging */,
+ NULL /* hv_perf_monitor */, NULL /* hv_cpu_dynamic_part */,
+ NULL /* hv_hypercall_params_xmm */, NULL /* hv_guest_idle_state */,
+ NULL, NULL,
+ NULL, NULL, NULL /* hv_guest_crash_msr */, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ },
.cpuid_eax = 0x40000003, .cpuid_reg = R_EDX,
},
[FEAT_SVM] = {
- .feat_names = svm_feature_name,
+ .feat_names = {
+ "npt", "lbrv", "svm_lock", "nrip_save",
+ "tsc_scale", "vmcb_clean", "flushbyasid", "decodeassists",
+ NULL, NULL, "pause_filter", NULL,
+ "pfthreshold", NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ },
.cpuid_eax = 0x8000000A, .cpuid_reg = R_EDX,
.tcg_features = TCG_SVM_FEATURES,
},
[FEAT_7_0_EBX] = {
- .feat_names = cpuid_7_0_ebx_feature_name,
+ .feat_names = {
+ "fsgsbase", "tsc_adjust", NULL, "bmi1",
+ "hle", "avx2", NULL, "smep",
+ "bmi2", "erms", "invpcid", "rtm",
+ NULL, NULL, "mpx", NULL,
+ "avx512f", "avx512dq", "rdseed", "adx",
+ "smap", "avx512ifma", "pcommit", "clflushopt",
+ "clwb", NULL, "avx512pf", "avx512er",
+ "avx512cd", NULL, "avx512bw", "avx512vl",
+ },
.cpuid_eax = 7,
.cpuid_needs_ecx = true, .cpuid_ecx = 0,
.cpuid_reg = R_EBX,
.tcg_features = TCG_7_0_EBX_FEATURES,
},
[FEAT_7_0_ECX] = {
- .feat_names = cpuid_7_0_ecx_feature_name,
+ .feat_names = {
+ NULL, "avx512vbmi", "umip", "pku",
+ "ospke", NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, "rdpid", NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ },
.cpuid_eax = 7,
.cpuid_needs_ecx = true, .cpuid_ecx = 0,
.cpuid_reg = R_ECX,
.tcg_features = TCG_7_0_ECX_FEATURES,
},
[FEAT_8000_0007_EDX] = {
- .feat_names = cpuid_apm_edx_feature_name,
+ .feat_names = {
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ "invtsc", NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ },
.cpuid_eax = 0x80000007,
.cpuid_reg = R_EDX,
.tcg_features = TCG_APM_FEATURES,
.unmigratable_flags = CPUID_APM_INVTSC,
},
[FEAT_XSAVE] = {
- .feat_names = cpuid_xsave_feature_name,
+ .feat_names = {
+ "xsaveopt", "xsavec", "xgetbv1", "xsaves",
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ },
.cpuid_eax = 0xd,
.cpuid_needs_ecx = true, .cpuid_ecx = 1,
.cpuid_reg = R_EAX,
.tcg_features = TCG_XSAVE_FEATURES,
},
[FEAT_6_EAX] = {
- .feat_names = cpuid_6_feature_name,
+ .feat_names = {
+ NULL, NULL, "arat", NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ },
.cpuid_eax = 6, .cpuid_reg = R_EAX,
.tcg_features = TCG_6_EAX_FEATURES,
},
@@ -711,8 +685,7 @@ static void add_flagname_to_bitmaps(const char *flagname,
FeatureWord w;
for (w = 0; w < FEATURE_WORDS; w++) {
FeatureWordInfo *wi = &feature_word_info[w];
- if (wi->feat_names &&
- lookup_feature(&words[w], flagname, NULL, wi->feat_names)) {
+ if (lookup_feature(&words[w], flagname, NULL, wi->feat_names)) {
break;
}
}
@@ -3324,9 +3297,6 @@ static void x86_cpu_register_feature_bit_props(X86CPU *cpu,
char **names;
FeatureWordInfo *fi = &feature_word_info[w];
- if (!fi->feat_names) {
- return;
- }
if (!fi->feat_names[bitnr]) {
return;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 2/7] target-i386: Don't try to enable PT State xsave component
2016-09-23 19:45 [Qemu-devel] [PATCH 0/7] target-i386: xsave CPUID handling refactor Eduardo Habkost
2016-09-23 19:45 ` [Qemu-devel] [PATCH 1/7] target-i386: Move feature name arrays inside FeatureWordInfo Eduardo Habkost
@ 2016-09-23 19:45 ` Eduardo Habkost
2016-09-23 20:04 ` Richard Henderson
2016-09-23 19:45 ` [Qemu-devel] [PATCH 3/7] target-i386: xsave: Calculate enabled components only once Eduardo Habkost
` (5 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Eduardo Habkost @ 2016-09-23 19:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Richard Henderson, Igor Mammedov
The code that calculates the set of supported XSAVE components on
CPUID looks at ext_save_areas to find out which components should
be enabled. However, if there are zeroed entries in the
ext_save_areas array, the
((env->features[esa->feature] & esa->bits) == esa->bits)
check will always succeed and QEMU will unconditionally try to
enable the component.
Luckily this never caused any problems because the only missing
entry in ext_save_areas is the PT State component (bit 8), and
KVM currently doesn't support it (so it was cleared on ena_mask).
But the code was still incorrect and would break if KVM starts
returning CPUID[EAX=0xD,ECX=0].EAX[bit 8] as supported on
GET_SUPPORTED_CPUID.
Fix the problem by changing the code to not enable a XSAVE
component if ExtSaveArea::bits is zero.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index cc07fdb..25ab4f8 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2514,7 +2514,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
*ecx = 0x240;
for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
const ExtSaveArea *esa = &x86_ext_save_areas[i];
- if ((env->features[esa->feature] & esa->bits) == esa->bits
+ if ((env->features[esa->feature] & esa->bits)
&& ((ena_mask >> i) & 1) != 0) {
if (i < 32) {
*eax |= 1u << i;
@@ -2530,7 +2530,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
*eax = env->features[FEAT_XSAVE];
} else if (count < ARRAY_SIZE(x86_ext_save_areas)) {
const ExtSaveArea *esa = &x86_ext_save_areas[count];
- if ((env->features[esa->feature] & esa->bits) == esa->bits
+ if ((env->features[esa->feature] & esa->bits)
&& ((ena_mask >> count) & 1) != 0) {
*eax = esa->size;
*ebx = esa->offset;
@@ -2766,7 +2766,7 @@ static void x86_cpu_reset(CPUState *s)
}
for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
const ExtSaveArea *esa = &x86_ext_save_areas[i];
- if ((env->features[esa->feature] & esa->bits) == esa->bits) {
+ if (env->features[esa->feature] & esa->bits) {
xcr0 |= 1ull << i;
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 3/7] target-i386: xsave: Calculate enabled components only once
2016-09-23 19:45 [Qemu-devel] [PATCH 0/7] target-i386: xsave CPUID handling refactor Eduardo Habkost
2016-09-23 19:45 ` [Qemu-devel] [PATCH 1/7] target-i386: Move feature name arrays inside FeatureWordInfo Eduardo Habkost
2016-09-23 19:45 ` [Qemu-devel] [PATCH 2/7] target-i386: Don't try to enable PT State xsave component Eduardo Habkost
@ 2016-09-23 19:45 ` Eduardo Habkost
2016-09-23 20:05 ` Richard Henderson
2016-09-23 19:45 ` [Qemu-devel] [PATCH 4/7] target-i386: xsave: Simplify CPUID[0xD, 0].{EAX, EDX} calculation Eduardo Habkost
` (4 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Eduardo Habkost @ 2016-09-23 19:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Richard Henderson, Igor Mammedov
Instead of checking both env->features and ena_mask at two
different places in the CPUID code, initialize ena_mask based on
the features that are enabled for the CPU, and then clear
unsupported bits based on kvm_arch_get_supported_cpuid().
The results should be exactly the same, but it will make it
easier to move the mask calculation elsewhare, and reuse
x86_cpu_filter_features() for the kvm_arch_get_supported_cpuid()
check.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 25ab4f8..9968581 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2490,7 +2490,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
*ebx &= 0xffff; /* The count doesn't need to be reliable. */
break;
case 0xD: {
- KVMState *s = cs->kvm_state;
uint64_t ena_mask;
int i;
@@ -2502,20 +2501,28 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) {
break;
}
+
+ ena_mask = (XSTATE_FP_MASK | XSTATE_SSE_MASK);
+ for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
+ const ExtSaveArea *esa = &x86_ext_save_areas[i];
+ if (env->features[esa->feature] & esa->bits) {
+ ena_mask |= (1ULL << i);
+ }
+ }
+
if (kvm_enabled()) {
- ena_mask = kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX);
- ena_mask <<= 32;
- ena_mask |= kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX);
- } else {
- ena_mask = -1;
+ KVMState *s = cs->kvm_state;
+ uint64_t kvm_mask = kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX);
+ kvm_mask <<= 32;
+ kvm_mask |= kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX);
+ ena_mask &= kvm_mask;
}
if (count == 0) {
*ecx = 0x240;
for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
const ExtSaveArea *esa = &x86_ext_save_areas[i];
- if ((env->features[esa->feature] & esa->bits)
- && ((ena_mask >> i) & 1) != 0) {
+ if ((ena_mask >> i) & 1) {
if (i < 32) {
*eax |= 1u << i;
} else {
@@ -2530,8 +2537,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
*eax = env->features[FEAT_XSAVE];
} else if (count < ARRAY_SIZE(x86_ext_save_areas)) {
const ExtSaveArea *esa = &x86_ext_save_areas[count];
- if ((env->features[esa->feature] & esa->bits)
- && ((ena_mask >> count) & 1) != 0) {
+ if ((ena_mask >> count) & 1) {
*eax = esa->size;
*ebx = esa->offset;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 4/7] target-i386: xsave: Simplify CPUID[0xD, 0].{EAX, EDX} calculation
2016-09-23 19:45 [Qemu-devel] [PATCH 0/7] target-i386: xsave CPUID handling refactor Eduardo Habkost
` (2 preceding siblings ...)
2016-09-23 19:45 ` [Qemu-devel] [PATCH 3/7] target-i386: xsave: Calculate enabled components only once Eduardo Habkost
@ 2016-09-23 19:45 ` Eduardo Habkost
2016-09-23 20:06 ` Richard Henderson
2016-09-23 19:45 ` [Qemu-devel] [PATCH 5/7] target-i386: xsave: Helper function to calculate xsave area size Eduardo Habkost
` (3 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Eduardo Habkost @ 2016-09-23 19:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Richard Henderson, Igor Mammedov
Instead of assigning individual bits in a loop, just copy the
values from ena_mask.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 9968581..7e66003 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2523,15 +2523,11 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
const ExtSaveArea *esa = &x86_ext_save_areas[i];
if ((ena_mask >> i) & 1) {
- if (i < 32) {
- *eax |= 1u << i;
- } else {
- *edx |= 1u << (i - 32);
- }
*ecx = MAX(*ecx, esa->offset + esa->size);
}
}
- *eax |= ena_mask & (XSTATE_FP_MASK | XSTATE_SSE_MASK);
+ *eax = ena_mask;
+ *edx = ena_mask >> 32;
*ebx = *ecx;
} else if (count == 1) {
*eax = env->features[FEAT_XSAVE];
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 5/7] target-i386: xsave: Helper function to calculate xsave area size
2016-09-23 19:45 [Qemu-devel] [PATCH 0/7] target-i386: xsave CPUID handling refactor Eduardo Habkost
` (3 preceding siblings ...)
2016-09-23 19:45 ` [Qemu-devel] [PATCH 4/7] target-i386: xsave: Simplify CPUID[0xD, 0].{EAX, EDX} calculation Eduardo Habkost
@ 2016-09-23 19:45 ` Eduardo Habkost
2016-09-23 20:07 ` Richard Henderson
2016-09-23 19:45 ` [Qemu-devel] [PATCH 6/7] target-i386: xsave: Calculate set of xsave components on realize Eduardo Habkost
` (2 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Eduardo Habkost @ 2016-09-23 19:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Richard Henderson, Igor Mammedov
Move the xsave area size calculation from cpu_x86_cpuid() inside
its own function. While doing it, change it to use the XSAVE area
struct sizes for the initial size, instead of the magic 0x240
number.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 7e66003..9034d8e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -548,6 +548,20 @@ static const ExtSaveArea x86_ext_save_areas[] = {
.size = sizeof(XSavePKRU) },
};
+static uint32_t xsave_area_size(uint64_t mask)
+{
+ int i;
+ uint64_t ret = sizeof(X86LegacyXSaveArea) + sizeof(X86XSaveHeader);
+
+ for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
+ const ExtSaveArea *esa = &x86_ext_save_areas[i];
+ if ((mask >> i) & 1) {
+ ret = MAX(ret, esa->offset + esa->size);
+ }
+ }
+ return ret;
+}
+
const char *get_register_name_32(unsigned int reg)
{
if (reg >= CPU_NB_REGS32) {
@@ -2519,13 +2533,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
}
if (count == 0) {
- *ecx = 0x240;
- for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
- const ExtSaveArea *esa = &x86_ext_save_areas[i];
- if ((ena_mask >> i) & 1) {
- *ecx = MAX(*ecx, esa->offset + esa->size);
- }
- }
+ *ecx = xsave_area_size(ena_mask);;
*eax = ena_mask;
*edx = ena_mask >> 32;
*ebx = *ecx;
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 6/7] target-i386: xsave: Calculate set of xsave components on realize
2016-09-23 19:45 [Qemu-devel] [PATCH 0/7] target-i386: xsave CPUID handling refactor Eduardo Habkost
` (4 preceding siblings ...)
2016-09-23 19:45 ` [Qemu-devel] [PATCH 5/7] target-i386: xsave: Helper function to calculate xsave area size Eduardo Habkost
@ 2016-09-23 19:45 ` Eduardo Habkost
2016-09-23 20:09 ` Richard Henderson
2016-09-27 20:06 ` Eduardo Habkost
2016-09-23 19:45 ` [Qemu-devel] [PATCH 7/7] target-i386: Move xsave component mask to features array Eduardo Habkost
2016-09-27 12:45 ` [Qemu-devel] [PATCH 0/7] target-i386: xsave CPUID handling refactor Eduardo Habkost
7 siblings, 2 replies; 17+ messages in thread
From: Eduardo Habkost @ 2016-09-23 19:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Richard Henderson, Igor Mammedov
Instead of doing complex calculations and calling
kvm_arch_get_supported_cpuid() inside cpu_x86_cpuid(), calculate
the set of required XSAVE components earlier, at realize time.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 51 ++++++++++++++++++++++++++++-----------------------
target-i386/cpu.h | 1 +
2 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 9034d8e..e6525e7 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2504,9 +2504,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
*ebx &= 0xffff; /* The count doesn't need to be reliable. */
break;
case 0xD: {
- uint64_t ena_mask;
- int i;
-
/* Processor Extended State */
*eax = 0;
*ebx = 0;
@@ -2516,32 +2513,16 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
break;
}
- ena_mask = (XSTATE_FP_MASK | XSTATE_SSE_MASK);
- for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
- const ExtSaveArea *esa = &x86_ext_save_areas[i];
- if (env->features[esa->feature] & esa->bits) {
- ena_mask |= (1ULL << i);
- }
- }
-
- if (kvm_enabled()) {
- KVMState *s = cs->kvm_state;
- uint64_t kvm_mask = kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX);
- kvm_mask <<= 32;
- kvm_mask |= kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX);
- ena_mask &= kvm_mask;
- }
-
if (count == 0) {
- *ecx = xsave_area_size(ena_mask);;
- *eax = ena_mask;
- *edx = ena_mask >> 32;
+ *ecx = xsave_area_size(env->xsave_components);
+ *eax = env->xsave_components;
+ *edx = env->xsave_components >> 32;
*ebx = *ecx;
} else if (count == 1) {
*eax = env->features[FEAT_XSAVE];
} else if (count < ARRAY_SIZE(x86_ext_save_areas)) {
const ExtSaveArea *esa = &x86_ext_save_areas[count];
- if ((ena_mask >> count) & 1) {
+ if ((env->xsave_components >> count) & 1) {
*eax = esa->size;
*ebx = esa->offset;
}
@@ -2971,6 +2952,29 @@ static void x86_cpu_adjust_feat_level(X86CPU *cpu, FeatureWord w)
}
}
+/* Calculate XSAVE components based on the configured CPU feature flags */
+static void x86_cpu_enable_xsave_components(X86CPU *cpu)
+{
+ CPUX86State *env = &cpu->env;
+ int i;
+
+ env->xsave_components = (XSTATE_FP_MASK | XSTATE_SSE_MASK);
+ for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
+ const ExtSaveArea *esa = &x86_ext_save_areas[i];
+ if (env->features[esa->feature] & esa->bits) {
+ env->xsave_components |= (1ULL << i);
+ }
+ }
+
+ if (kvm_enabled()) {
+ KVMState *s = kvm_state;
+ uint64_t kvm_mask = kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX);
+ kvm_mask <<= 32;
+ kvm_mask |= kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX);
+ env->xsave_components &= kvm_mask;
+ }
+}
+
#define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \
(env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && \
(env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3)
@@ -3016,6 +3020,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
cpu->env.features[w] &= ~minus_features[w];
}
+ x86_cpu_enable_xsave_components(cpu);
/* CPUID[EAX=7,ECX=0].EBX always increased level automatically: */
x86_cpu_adjust_feat_level(cpu, FEAT_7_0_EBX);
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index aaa45f0..6c457ed 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1122,6 +1122,7 @@ typedef struct CPUX86State {
uint32_t cpuid_vendor3;
uint32_t cpuid_version;
FeatureWordArray features;
+ uint64_t xsave_components;
uint32_t cpuid_model[12];
/* MTRRs */
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 7/7] target-i386: Move xsave component mask to features array
2016-09-23 19:45 [Qemu-devel] [PATCH 0/7] target-i386: xsave CPUID handling refactor Eduardo Habkost
` (5 preceding siblings ...)
2016-09-23 19:45 ` [Qemu-devel] [PATCH 6/7] target-i386: xsave: Calculate set of xsave components on realize Eduardo Habkost
@ 2016-09-23 19:45 ` Eduardo Habkost
2016-09-23 20:20 ` Richard Henderson
2016-09-27 12:45 ` [Qemu-devel] [PATCH 0/7] target-i386: xsave CPUID handling refactor Eduardo Habkost
7 siblings, 1 reply; 17+ messages in thread
From: Eduardo Habkost @ 2016-09-23 19:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Richard Henderson, Igor Mammedov
This will reuse the existing check/enforce logic in
x86_cpu_filter_features() to check the xsave component bits
against GET_SUPPORTED_CPUID.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 42 ++++++++++++++++++++++++++++--------------
target-i386/cpu.h | 3 ++-
2 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e6525e7..b2c3e17 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -489,6 +489,18 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
.cpuid_eax = 6, .cpuid_reg = R_EAX,
.tcg_features = TCG_6_EAX_FEATURES,
},
+ [FEAT_XSAVE_COMP_LO] = {
+ .cpuid_eax = 0xD,
+ .cpuid_needs_ecx = true, .cpuid_ecx = 0,
+ .cpuid_reg = R_EAX,
+ .tcg_features = ~0U,
+ },
+ [FEAT_XSAVE_COMP_HI] = {
+ .cpuid_eax = 0xD,
+ .cpuid_needs_ecx = true, .cpuid_ecx = 0,
+ .cpuid_reg = R_EDX,
+ .tcg_features = ~0U,
+ },
};
typedef struct X86RegisterInfo32 {
@@ -562,6 +574,12 @@ static uint32_t xsave_area_size(uint64_t mask)
return ret;
}
+static inline uint64_t x86_cpu_xsave_components(X86CPU *cpu)
+{
+ return ((uint64_t)cpu->env.features[FEAT_XSAVE_COMP_HI]) << 32 |
+ cpu->env.features[FEAT_XSAVE_COMP_LO];
+}
+
const char *get_register_name_32(unsigned int reg)
{
if (reg >= CPU_NB_REGS32) {
@@ -2514,15 +2532,15 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
}
if (count == 0) {
- *ecx = xsave_area_size(env->xsave_components);
- *eax = env->xsave_components;
- *edx = env->xsave_components >> 32;
+ *ecx = xsave_area_size(x86_cpu_xsave_components(cpu));
+ *eax = env->features[FEAT_XSAVE_COMP_LO];
+ *edx = env->features[FEAT_XSAVE_COMP_HI];
*ebx = *ecx;
} else if (count == 1) {
*eax = env->features[FEAT_XSAVE];
} else if (count < ARRAY_SIZE(x86_ext_save_areas)) {
- const ExtSaveArea *esa = &x86_ext_save_areas[count];
- if ((env->xsave_components >> count) & 1) {
+ if ((x86_cpu_xsave_components(cpu) >> count) & 1) {
+ const ExtSaveArea *esa = &x86_ext_save_areas[count];
*eax = esa->size;
*ebx = esa->offset;
}
@@ -2957,22 +2975,18 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu)
{
CPUX86State *env = &cpu->env;
int i;
+ uint64_t mask;
- env->xsave_components = (XSTATE_FP_MASK | XSTATE_SSE_MASK);
+ mask = (XSTATE_FP_MASK | XSTATE_SSE_MASK);
for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
const ExtSaveArea *esa = &x86_ext_save_areas[i];
if (env->features[esa->feature] & esa->bits) {
- env->xsave_components |= (1ULL << i);
+ mask |= (1ULL << i);
}
}
- if (kvm_enabled()) {
- KVMState *s = kvm_state;
- uint64_t kvm_mask = kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX);
- kvm_mask <<= 32;
- kvm_mask |= kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX);
- env->xsave_components &= kvm_mask;
- }
+ env->features[FEAT_XSAVE_COMP_LO] = mask;
+ env->features[FEAT_XSAVE_COMP_HI] = mask >> 32;
}
#define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 6c457ed..1cb32ae 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -453,6 +453,8 @@ typedef enum FeatureWord {
FEAT_SVM, /* CPUID[8000_000A].EDX */
FEAT_XSAVE, /* CPUID[EAX=0xd,ECX=1].EAX */
FEAT_6_EAX, /* CPUID[6].EAX */
+ FEAT_XSAVE_COMP_LO, /* CPUID[EAX=0xd,ECX=0].EAX */
+ FEAT_XSAVE_COMP_HI, /* CPUID[EAX=0xd,ECX=0].EDX */
FEATURE_WORDS,
} FeatureWord;
@@ -1122,7 +1124,6 @@ typedef struct CPUX86State {
uint32_t cpuid_vendor3;
uint32_t cpuid_version;
FeatureWordArray features;
- uint64_t xsave_components;
uint32_t cpuid_model[12];
/* MTRRs */
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 1/7] target-i386: Move feature name arrays inside FeatureWordInfo
2016-09-23 19:45 ` [Qemu-devel] [PATCH 1/7] target-i386: Move feature name arrays inside FeatureWordInfo Eduardo Habkost
@ 2016-09-23 20:01 ` Richard Henderson
0 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2016-09-23 20:01 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel; +Cc: Paolo Bonzini, Igor Mammedov
On 09/23/2016 12:45 PM, Eduardo Habkost wrote:
> It makes it easier to guarantee the arrays are the right size,
> and to find information when looking at the code.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> target-i386/cpu.c | 370 +++++++++++++++++++++++++-----------------------------
> 1 file changed, 170 insertions(+), 200 deletions(-)
Reviewed-by: Richard Henderson <rth@twiddle.net>
> static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> [FEAT_1_EDX] = {
> - .feat_names = feature_name,
> + .feat_names = {
> + "fpu", "vme", "de", "pse",
> + "tsc", "msr", "pae", "mce",
> + "cx8", "apic", NULL, "sep",
> + "mtrr", "pge", "mca", "cmov",
> + "pat", "pse36", "pn" /* Intel psn */, "clflush" /* Intel clfsh */,
> + NULL, "ds" /* Intel dts */, "acpi", "mmx",
> + "fxsr", "sse", "sse2", "ss",
> + "ht" /* Intel htt */, "tm", "ia64", "pbe",
> + },
Unrelated, but can we make this feature_word_info structure const? It may
require the addition of const to other function parameters, in which case the
change should be a separate patch.
r~
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 2/7] target-i386: Don't try to enable PT State xsave component
2016-09-23 19:45 ` [Qemu-devel] [PATCH 2/7] target-i386: Don't try to enable PT State xsave component Eduardo Habkost
@ 2016-09-23 20:04 ` Richard Henderson
0 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2016-09-23 20:04 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel; +Cc: Paolo Bonzini, Igor Mammedov
On 09/23/2016 12:45 PM, Eduardo Habkost wrote:
> The code that calculates the set of supported XSAVE components on
> CPUID looks at ext_save_areas to find out which components should
> be enabled. However, if there are zeroed entries in the
> ext_save_areas array, the
> ((env->features[esa->feature] & esa->bits) == esa->bits)
> check will always succeed and QEMU will unconditionally try to
> enable the component.
>
> Luckily this never caused any problems because the only missing
> entry in ext_save_areas is the PT State component (bit 8), and
> KVM currently doesn't support it (so it was cleared on ena_mask).
> But the code was still incorrect and would break if KVM starts
> returning CPUID[EAX=0xD,ECX=0].EAX[bit 8] as supported on
> GET_SUPPORTED_CPUID.
>
> Fix the problem by changing the code to not enable a XSAVE
> component if ExtSaveArea::bits is zero.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> target-i386/cpu.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Richard Henderson <rth@twiddle.net>
r~
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 3/7] target-i386: xsave: Calculate enabled components only once
2016-09-23 19:45 ` [Qemu-devel] [PATCH 3/7] target-i386: xsave: Calculate enabled components only once Eduardo Habkost
@ 2016-09-23 20:05 ` Richard Henderson
0 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2016-09-23 20:05 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel; +Cc: Paolo Bonzini, Igor Mammedov
On 09/23/2016 12:45 PM, Eduardo Habkost wrote:
> Instead of checking both env->features and ena_mask at two
> different places in the CPUID code, initialize ena_mask based on
> the features that are enabled for the CPU, and then clear
> unsupported bits based on kvm_arch_get_supported_cpuid().
>
> The results should be exactly the same, but it will make it
> easier to move the mask calculation elsewhare, and reuse
> x86_cpu_filter_features() for the kvm_arch_get_supported_cpuid()
> check.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> target-i386/cpu.c | 26 ++++++++++++++++----------
> 1 file changed, 16 insertions(+), 10 deletions(-)
Reviewed-by: Richard Henderson <rth@twiddle.net>
r~
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 4/7] target-i386: xsave: Simplify CPUID[0xD, 0].{EAX, EDX} calculation
2016-09-23 19:45 ` [Qemu-devel] [PATCH 4/7] target-i386: xsave: Simplify CPUID[0xD, 0].{EAX, EDX} calculation Eduardo Habkost
@ 2016-09-23 20:06 ` Richard Henderson
0 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2016-09-23 20:06 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel; +Cc: Paolo Bonzini, Igor Mammedov
On 09/23/2016 12:45 PM, Eduardo Habkost wrote:
> Instead of assigning individual bits in a loop, just copy the
> values from ena_mask.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> target-i386/cpu.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
Reviewed-by: Richard Henderson <rth@twiddle.net>
r~
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 5/7] target-i386: xsave: Helper function to calculate xsave area size
2016-09-23 19:45 ` [Qemu-devel] [PATCH 5/7] target-i386: xsave: Helper function to calculate xsave area size Eduardo Habkost
@ 2016-09-23 20:07 ` Richard Henderson
0 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2016-09-23 20:07 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel; +Cc: Paolo Bonzini, Igor Mammedov
On 09/23/2016 12:45 PM, Eduardo Habkost wrote:
> Move the xsave area size calculation from cpu_x86_cpuid() inside
> its own function. While doing it, change it to use the XSAVE area
> struct sizes for the initial size, instead of the magic 0x240
> number.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> target-i386/cpu.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
Reviewed-by: Richard Henderson <rth@twiddle.net>
r~
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 6/7] target-i386: xsave: Calculate set of xsave components on realize
2016-09-23 19:45 ` [Qemu-devel] [PATCH 6/7] target-i386: xsave: Calculate set of xsave components on realize Eduardo Habkost
@ 2016-09-23 20:09 ` Richard Henderson
2016-09-27 20:06 ` Eduardo Habkost
1 sibling, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2016-09-23 20:09 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel; +Cc: Paolo Bonzini, Igor Mammedov
On 09/23/2016 12:45 PM, Eduardo Habkost wrote:
> Instead of doing complex calculations and calling
> kvm_arch_get_supported_cpuid() inside cpu_x86_cpuid(), calculate
> the set of required XSAVE components earlier, at realize time.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> target-i386/cpu.c | 51 ++++++++++++++++++++++++++++-----------------------
> target-i386/cpu.h | 1 +
> 2 files changed, 29 insertions(+), 23 deletions(-)
Reviewed-by: Richard Henderson <rth@twiddle.net>
> @@ -2504,9 +2504,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> *ebx &= 0xffff; /* The count doesn't need to be reliable. */
> break;
> case 0xD: {
> - uint64_t ena_mask;
> - int i;
> -
> /* Processor Extended State */
> *eax = 0;
> *ebx = 0;
We should be able to drop the braces around this case as well, please.
r~
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 7/7] target-i386: Move xsave component mask to features array
2016-09-23 19:45 ` [Qemu-devel] [PATCH 7/7] target-i386: Move xsave component mask to features array Eduardo Habkost
@ 2016-09-23 20:20 ` Richard Henderson
0 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2016-09-23 20:20 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel; +Cc: Paolo Bonzini, Igor Mammedov
On 09/23/2016 12:45 PM, Eduardo Habkost wrote:
> This will reuse the existing check/enforce logic in
> x86_cpu_filter_features() to check the xsave component bits
> against GET_SUPPORTED_CPUID.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> target-i386/cpu.c | 42 ++++++++++++++++++++++++++++--------------
> target-i386/cpu.h | 3 ++-
> 2 files changed, 30 insertions(+), 15 deletions(-)
Reviewed-by: Richard Henderson <rth@twiddle.net>
r~
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7] target-i386: xsave CPUID handling refactor
2016-09-23 19:45 [Qemu-devel] [PATCH 0/7] target-i386: xsave CPUID handling refactor Eduardo Habkost
` (6 preceding siblings ...)
2016-09-23 19:45 ` [Qemu-devel] [PATCH 7/7] target-i386: Move xsave component mask to features array Eduardo Habkost
@ 2016-09-27 12:45 ` Eduardo Habkost
7 siblings, 0 replies; 17+ messages in thread
From: Eduardo Habkost @ 2016-09-27 12:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Igor Mammedov, Richard Henderson
On Fri, Sep 23, 2016 at 04:45:29PM -0300, Eduardo Habkost wrote:
> This series refactor the xsave CPUID handling so it won't
> silently disable any XSAVE components on CPUID[0xD] in case the
> host doesn't support it. It will instead use the exisitng
> check/enforce logic for filtering the CPUID bits and checking for
> host-side support.
>
> This series is available on git at:
> https://github.com/ehabkost/qemu-hacks.git work/xsave-cpuid-cleanup
>
> The series is based on my x86-next branch, that contains other
> CPUID-related changes:
> https://github.com/ehabkost/qemu.git x8-next
>
> Eduardo Habkost (7):
> target-i386: Move feature name arrays inside FeatureWordInfo
> target-i386: Don't try to enable PT State xsave component
> target-i386: xsave: Calculate enabled components only once
> target-i386: xsave: Simplify CPUID[0xD,0].{EAX,EDX} calculation
> target-i386: xsave: Helper function to calculate xsave area size
> target-i386: xsave: Calculate set of xsave components on realize
> target-i386: Move xsave component mask to features array
Queued on x86-next.
--
Eduardo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 6/7] target-i386: xsave: Calculate set of xsave components on realize
2016-09-23 19:45 ` [Qemu-devel] [PATCH 6/7] target-i386: xsave: Calculate set of xsave components on realize Eduardo Habkost
2016-09-23 20:09 ` Richard Henderson
@ 2016-09-27 20:06 ` Eduardo Habkost
1 sibling, 0 replies; 17+ messages in thread
From: Eduardo Habkost @ 2016-09-27 20:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Igor Mammedov, Richard Henderson
On Fri, Sep 23, 2016 at 04:45:35PM -0300, Eduardo Habkost wrote:
[...]
> @@ -2971,6 +2952,29 @@ static void x86_cpu_adjust_feat_level(X86CPU *cpu, FeatureWord w)
> }
> }
>
> +/* Calculate XSAVE components based on the configured CPU feature flags */
> +static void x86_cpu_enable_xsave_components(X86CPU *cpu)
> +{
> + CPUX86State *env = &cpu->env;
> + int i;
> +
> + env->xsave_components = (XSTATE_FP_MASK | XSTATE_SSE_MASK);
We shouldn't set xsave_components if XSAVE is disabled. The following fix was
squashed while applying:
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e6525e7..8bef3cf 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2958,6 +2958,10 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu)
CPUX86State *env = &cpu->env;
int i;
+ if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) {
+ return;
+ }
+
env->xsave_components = (XSTATE_FP_MASK | XSTATE_SSE_MASK);
for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
const ExtSaveArea *esa = &x86_ext_save_areas[i];
> + for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) {
> + const ExtSaveArea *esa = &x86_ext_save_areas[i];
> + if (env->features[esa->feature] & esa->bits) {
> + env->xsave_components |= (1ULL << i);
> + }
> + }
> +
> + if (kvm_enabled()) {
> + KVMState *s = kvm_state;
> + uint64_t kvm_mask = kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX);
> + kvm_mask <<= 32;
> + kvm_mask |= kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX);
> + env->xsave_components &= kvm_mask;
> + }
> +}
> +
> #define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \
> (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && \
> (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3)
> @@ -3016,6 +3020,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> cpu->env.features[w] &= ~minus_features[w];
> }
>
> + x86_cpu_enable_xsave_components(cpu);
>
> /* CPUID[EAX=7,ECX=0].EBX always increased level automatically: */
> x86_cpu_adjust_feat_level(cpu, FEAT_7_0_EBX);
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index aaa45f0..6c457ed 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -1122,6 +1122,7 @@ typedef struct CPUX86State {
> uint32_t cpuid_vendor3;
> uint32_t cpuid_version;
> FeatureWordArray features;
> + uint64_t xsave_components;
> uint32_t cpuid_model[12];
>
> /* MTRRs */
> --
> 2.7.4
>
>
--
Eduardo
^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-09-27 20:06 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-23 19:45 [Qemu-devel] [PATCH 0/7] target-i386: xsave CPUID handling refactor Eduardo Habkost
2016-09-23 19:45 ` [Qemu-devel] [PATCH 1/7] target-i386: Move feature name arrays inside FeatureWordInfo Eduardo Habkost
2016-09-23 20:01 ` Richard Henderson
2016-09-23 19:45 ` [Qemu-devel] [PATCH 2/7] target-i386: Don't try to enable PT State xsave component Eduardo Habkost
2016-09-23 20:04 ` Richard Henderson
2016-09-23 19:45 ` [Qemu-devel] [PATCH 3/7] target-i386: xsave: Calculate enabled components only once Eduardo Habkost
2016-09-23 20:05 ` Richard Henderson
2016-09-23 19:45 ` [Qemu-devel] [PATCH 4/7] target-i386: xsave: Simplify CPUID[0xD, 0].{EAX, EDX} calculation Eduardo Habkost
2016-09-23 20:06 ` Richard Henderson
2016-09-23 19:45 ` [Qemu-devel] [PATCH 5/7] target-i386: xsave: Helper function to calculate xsave area size Eduardo Habkost
2016-09-23 20:07 ` Richard Henderson
2016-09-23 19:45 ` [Qemu-devel] [PATCH 6/7] target-i386: xsave: Calculate set of xsave components on realize Eduardo Habkost
2016-09-23 20:09 ` Richard Henderson
2016-09-27 20:06 ` Eduardo Habkost
2016-09-23 19:45 ` [Qemu-devel] [PATCH 7/7] target-i386: Move xsave component mask to features array Eduardo Habkost
2016-09-23 20:20 ` Richard Henderson
2016-09-27 12:45 ` [Qemu-devel] [PATCH 0/7] target-i386: xsave CPUID handling refactor Eduardo Habkost
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).