* [Qemu-devel] [PATCH v5 0/3] x86: QEMU side support on MSR based features
@ 2018-10-15 4:47 Robert Hoo
2018-10-15 4:47 ` [Qemu-devel] [PATCH v5 1/3] kvm: Add support to KVM_GET_MSR_FEATURE_INDEX_LIST and KVM_GET_MSRS system ioctl Robert Hoo
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Robert Hoo @ 2018-10-15 4:47 UTC (permalink / raw)
To: pbonzini, rth, ehabkost, thomas.lendacky
Cc: qemu-devel, robert.hu, Robert Hoo
KVM side has added the framework (kvm.git:d1d93fa90) to support MSR based features.
Here is the QEMU part, including data structure changes/expanding, referring
functions changes, and the implementations on
KVM_GET_MSR_FEATURE_INDEX_LIST and KVM_GET_MSRS system ioctl.
Changelog:
v5: Re-order patches. Complement feature MSR set routines.
v4:
Re-organize patch set to conform to request of individually build pass.
Add KVM capability check for KVM_GET_MSR_INDEX_LIST before fetch.
Special treatment for MSR_IA32_ARCH_CAPABILITIES.RSBA.
Use more convenient glib wrapper (g_strdup_printf) instead of native
(sprintf).
v3: patch 2&3 in v2 are corrupted. Re-format patches.
v2: coding style changes to pass ./scripts/checkpatch.pl.
Robert Hoo (3):
kvm: Add support to KVM_GET_MSR_FEATURE_INDEX_LIST and KVM_GET_MSRS
system ioctl
x86: Data structure changes to support MSR based features
x86: define a new MSR based feature word --
FEATURE_WORDS_ARCH_CAPABILITIES
include/sysemu/kvm.h | 2 +
target/i386/cpu.c | 217 +++++++++++++++++++++++++++++++++++++++------------
target/i386/cpu.h | 8 ++
target/i386/kvm.c | 91 +++++++++++++++++++++
4 files changed, 266 insertions(+), 52 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v5 1/3] kvm: Add support to KVM_GET_MSR_FEATURE_INDEX_LIST and KVM_GET_MSRS system ioctl
2018-10-15 4:47 [Qemu-devel] [PATCH v5 0/3] x86: QEMU side support on MSR based features Robert Hoo
@ 2018-10-15 4:47 ` Robert Hoo
2018-10-24 9:49 ` Eduardo Habkost
2018-10-15 4:47 ` [Qemu-devel] [PATCH v5 2/3] x86: Data structure changes to support MSR based features Robert Hoo
2018-10-15 4:47 ` [Qemu-devel] [PATCH v5 3/3] x86: define a new MSR based feature word -- FEATURE_WORDS_ARCH_CAPABILITIES Robert Hoo
2 siblings, 1 reply; 13+ messages in thread
From: Robert Hoo @ 2018-10-15 4:47 UTC (permalink / raw)
To: pbonzini, rth, ehabkost, thomas.lendacky
Cc: qemu-devel, robert.hu, Robert Hoo
Add kvm_get_supported_feature_msrs() to get supported MSR feature index list.
Add kvm_arch_get_supported_msr_feature() to get each MSR features value.
Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
---
include/sysemu/kvm.h | 2 ++
target/i386/kvm.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 82 insertions(+)
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 0b64b8e..97d8d9d 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -463,6 +463,8 @@ int kvm_vm_check_extension(KVMState *s, unsigned int extension);
uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
uint32_t index, int reg);
+uint32_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index);
+
void kvm_set_sigmask_len(KVMState *s, unsigned int sigmask_len);
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index dc4047b..db79dad 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -107,6 +107,7 @@ static int has_pit_state2;
static bool has_msr_mcg_ext_ctl;
static struct kvm_cpuid2 *cpuid_cache;
+static struct kvm_msr_list *kvm_feature_msrs;
int kvm_has_pit_state2(void)
{
@@ -420,6 +421,42 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function,
return ret;
}
+uint32_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index)
+{
+ struct {
+ struct kvm_msrs info;
+ struct kvm_msr_entry entries[1];
+ } msr_data;
+ uint32_t ret;
+
+ if (kvm_feature_msrs == NULL) { /* Host doesn't support feature MSRs */
+ return 0;
+ }
+
+ /* Check if requested MSR is supported feature MSR */
+ int i;
+ for (i = 0; i < kvm_feature_msrs->nmsrs; i++)
+ if (kvm_feature_msrs->indices[i] == index) {
+ break;
+ }
+ if (i == kvm_feature_msrs->nmsrs) {
+ return 0; /* if the feature MSR is not supported, simply return 0 */
+ }
+
+ msr_data.info.nmsrs = 1;
+ msr_data.entries[0].index = index;
+
+ ret = kvm_ioctl(s, KVM_GET_MSRS, &msr_data);
+ if (ret != 1) {
+ error_report("KVM get MSR (index=0x%x) feature failed, %s",
+ index, strerror(-ret));
+ exit(1);
+ }
+
+ return msr_data.entries[0].data;
+}
+
+
typedef struct HWPoisonPage {
ram_addr_t ram_addr;
QLIST_ENTRY(HWPoisonPage) list;
@@ -1239,6 +1276,47 @@ void kvm_arch_do_init_vcpu(X86CPU *cpu)
}
}
+static int kvm_get_supported_feature_msrs(KVMState *s)
+{
+ int ret = 0;
+
+ if (kvm_feature_msrs != NULL) {
+ return 0;
+ }
+
+ if (!kvm_check_extension(s, KVM_CAP_GET_MSR_FEATURES)) {
+ return 0;
+ }
+
+ struct kvm_msr_list msr_list;
+
+ msr_list.nmsrs = 0;
+ ret = kvm_ioctl(s, KVM_GET_MSR_FEATURE_INDEX_LIST, &msr_list);
+ if (ret < 0 && ret != -E2BIG) {
+ error_report("Fetch KVM feature MSR list failed: %s",
+ strerror(-ret));
+ return ret;
+ }
+
+ assert(msr_list.nmsrs > 0);
+ kvm_feature_msrs = (struct kvm_msr_list *) \
+ g_malloc0(sizeof(msr_list) +
+ msr_list.nmsrs * sizeof(msr_list.indices[0]));
+
+ kvm_feature_msrs->nmsrs = msr_list.nmsrs;
+ ret = kvm_ioctl(s, KVM_GET_MSR_FEATURE_INDEX_LIST, kvm_feature_msrs);
+
+ if (ret < 0) {
+ error_report("Fetch KVM feature MSR list failed: %s",
+ strerror(-ret));
+ g_free(kvm_feature_msrs);
+ kvm_feature_msrs = NULL;
+ return ret;
+ }
+
+ return 0;
+}
+
static int kvm_get_supported_msrs(KVMState *s)
{
static int kvm_supported_msrs;
@@ -1392,6 +1470,8 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
return ret;
}
+ kvm_get_supported_feature_msrs(s);
+
uname(&utsname);
lm_capable_kernel = strcmp(utsname.machine, "x86_64") == 0;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v5 2/3] x86: Data structure changes to support MSR based features
2018-10-15 4:47 [Qemu-devel] [PATCH v5 0/3] x86: QEMU side support on MSR based features Robert Hoo
2018-10-15 4:47 ` [Qemu-devel] [PATCH v5 1/3] kvm: Add support to KVM_GET_MSR_FEATURE_INDEX_LIST and KVM_GET_MSRS system ioctl Robert Hoo
@ 2018-10-15 4:47 ` Robert Hoo
2018-10-24 9:56 ` Eduardo Habkost
2018-10-24 10:16 ` Eduardo Habkost
2018-10-15 4:47 ` [Qemu-devel] [PATCH v5 3/3] x86: define a new MSR based feature word -- FEATURE_WORDS_ARCH_CAPABILITIES Robert Hoo
2 siblings, 2 replies; 13+ messages in thread
From: Robert Hoo @ 2018-10-15 4:47 UTC (permalink / raw)
To: pbonzini, rth, ehabkost, thomas.lendacky
Cc: qemu-devel, robert.hu, Robert Hoo
Add FeatureWordType indicator in struct FeatureWordInfo.
Change feature_word_info[] accordingly.
Change existing functions that refer to feature_word_info[] accordingly.
Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
---
target/i386/cpu.c | 188 +++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 136 insertions(+), 52 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index c88876d..d191b9c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -770,17 +770,36 @@ static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
/* missing:
CPUID_XSAVE_XSAVEC, CPUID_XSAVE_XSAVES */
+typedef enum FeatureWordType {
+ CPUID_FEATURE_WORD,
+ MSR_FEATURE_WORD,
+} FeatureWordType;
+
typedef struct FeatureWordInfo {
+ FeatureWordType type;
/* 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 */
- int cpuid_reg; /* output register (R_* constant) */
+ union {
+ /* If type==CPUID_FEATURE_WORD */
+ struct {
+ uint32_t eax; /* Input EAX for CPUID */
+ bool needs_ecx; /* CPUID instruction uses ECX as input */
+ uint32_t ecx; /* Input ECX value for CPUID */
+ int reg; /* output register (R_* constant) */
+ } cpuid;
+ /* If type==MSR_FEATURE_WORD */
+ struct {
+ uint32_t index;
+ struct { /*CPUID that enumerate this MSR*/
+ FeatureWord cpuid_class;
+ uint32_t cpuid_flag;
+ } cpuid_dep;
+ } msr;
+ };
uint32_t tcg_features; /* Feature flags supported by TCG */
uint32_t unmigratable_flags; /* Feature flags known to be unmigratable */
uint32_t migratable_flags; /* Feature flags known to be migratable */
@@ -790,6 +809,7 @@ typedef struct FeatureWordInfo {
static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
[FEAT_1_EDX] = {
+ .type = CPUID_FEATURE_WORD,
.feat_names = {
"fpu", "vme", "de", "pse",
"tsc", "msr", "pae", "mce",
@@ -800,10 +820,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
"fxsr", "sse", "sse2", "ss",
"ht" /* Intel htt */, "tm", "ia64", "pbe",
},
- .cpuid_eax = 1, .cpuid_reg = R_EDX,
+ .cpuid = {.eax = 1, .reg = R_EDX, },
.tcg_features = TCG_FEATURES,
},
[FEAT_1_ECX] = {
+ .type = CPUID_FEATURE_WORD,
.feat_names = {
"pni" /* Intel,AMD sse3 */, "pclmulqdq", "dtes64", "monitor",
"ds-cpl", "vmx", "smx", "est",
@@ -814,7 +835,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
"tsc-deadline", "aes", "xsave", NULL /* osxsave */,
"avx", "f16c", "rdrand", "hypervisor",
},
- .cpuid_eax = 1, .cpuid_reg = R_ECX,
+ .cpuid = { .eax = 1, .reg = R_ECX, },
.tcg_features = TCG_EXT_FEATURES,
},
/* Feature names that are already defined on feature_name[] but
@@ -823,6 +844,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
* to features[FEAT_8000_0001_EDX] if and only if CPU vendor is AMD.
*/
[FEAT_8000_0001_EDX] = {
+ .type = CPUID_FEATURE_WORD,
.feat_names = {
NULL /* fpu */, NULL /* vme */, NULL /* de */, NULL /* pse */,
NULL /* tsc */, NULL /* msr */, NULL /* pae */, NULL /* mce */,
@@ -833,10 +855,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL /* fxsr */, "fxsr-opt", "pdpe1gb", "rdtscp",
NULL, "lm", "3dnowext", "3dnow",
},
- .cpuid_eax = 0x80000001, .cpuid_reg = R_EDX,
+ .cpuid = { .eax = 0x80000001, .reg = R_EDX, },
.tcg_features = TCG_EXT2_FEATURES,
},
[FEAT_8000_0001_ECX] = {
+ .type = CPUID_FEATURE_WORD,
.feat_names = {
"lahf-lm", "cmp-legacy", "svm", "extapic",
"cr8legacy", "abm", "sse4a", "misalignsse",
@@ -847,7 +870,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
"perfctr-nb", NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
},
- .cpuid_eax = 0x80000001, .cpuid_reg = R_ECX,
+ .cpuid = { .eax = 0x80000001, .reg = R_ECX, },
.tcg_features = TCG_EXT3_FEATURES,
/*
* TOPOEXT is always allowed but can't be enabled blindly by
@@ -857,6 +880,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
.no_autoenable_flags = CPUID_EXT3_TOPOEXT,
},
[FEAT_C000_0001_EDX] = {
+ .type = CPUID_FEATURE_WORD,
.feat_names = {
NULL, NULL, "xstore", "xstore-en",
NULL, NULL, "xcrypt", "xcrypt-en",
@@ -867,10 +891,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
},
- .cpuid_eax = 0xC0000001, .cpuid_reg = R_EDX,
+ .cpuid = { .eax = 0xC0000001, .reg = R_EDX, },
.tcg_features = TCG_EXT4_FEATURES,
},
[FEAT_KVM] = {
+ .type = CPUID_FEATURE_WORD,
.feat_names = {
"kvmclock", "kvm-nopiodelay", "kvm-mmu", "kvmclock",
"kvm-asyncpf", "kvm-steal-time", "kvm-pv-eoi", "kvm-pv-unhalt",
@@ -881,10 +906,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
"kvmclock-stable-bit", NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
},
- .cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EAX,
+ .cpuid = { .eax = KVM_CPUID_FEATURES, .reg = R_EAX, },
.tcg_features = TCG_KVM_FEATURES,
},
[FEAT_KVM_HINTS] = {
+ .type = CPUID_FEATURE_WORD,
.feat_names = {
"kvm-hint-dedicated", NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
@@ -895,7 +921,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
},
- .cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EDX,
+ .cpuid = { .eax = KVM_CPUID_FEATURES, .reg = R_EDX, },
.tcg_features = TCG_KVM_FEATURES,
/*
* KVM hints aren't auto-enabled by -cpu host, they need to be
@@ -904,6 +930,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
.no_autoenable_flags = ~0U,
},
[FEAT_HYPERV_EAX] = {
+ .type = CPUID_FEATURE_WORD,
.feat_names = {
NULL /* hv_msr_vp_runtime_access */, NULL /* hv_msr_time_refcount_access */,
NULL /* hv_msr_synic_access */, NULL /* hv_msr_stimer_access */,
@@ -918,9 +945,10 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
},
- .cpuid_eax = 0x40000003, .cpuid_reg = R_EAX,
+ .cpuid = { .eax = 0x40000003, .reg = R_EAX, },
},
[FEAT_HYPERV_EBX] = {
+ .type = CPUID_FEATURE_WORD,
.feat_names = {
NULL /* hv_create_partitions */, NULL /* hv_access_partition_id */,
NULL /* hv_access_memory_pool */, NULL /* hv_adjust_message_buffers */,
@@ -934,9 +962,10 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
},
- .cpuid_eax = 0x40000003, .cpuid_reg = R_EBX,
+ .cpuid = { .eax = 0x40000003, .reg = R_EBX, },
},
[FEAT_HYPERV_EDX] = {
+ .type = CPUID_FEATURE_WORD,
.feat_names = {
NULL /* hv_mwait */, NULL /* hv_guest_debugging */,
NULL /* hv_perf_monitor */, NULL /* hv_cpu_dynamic_part */,
@@ -949,9 +978,10 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
},
- .cpuid_eax = 0x40000003, .cpuid_reg = R_EDX,
+ .cpuid = { .eax = 0x40000003, .reg = R_EDX, },
},
[FEAT_SVM] = {
+ .type = CPUID_FEATURE_WORD,
.feat_names = {
"npt", "lbrv", "svm-lock", "nrip-save",
"tsc-scale", "vmcb-clean", "flushbyasid", "decodeassists",
@@ -962,10 +992,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
},
- .cpuid_eax = 0x8000000A, .cpuid_reg = R_EDX,
+ .cpuid = { .eax = 0x8000000A, .reg = R_EDX, },
.tcg_features = TCG_SVM_FEATURES,
},
[FEAT_7_0_EBX] = {
+ .type = CPUID_FEATURE_WORD,
.feat_names = {
"fsgsbase", "tsc-adjust", NULL, "bmi1",
"hle", "avx2", NULL, "smep",
@@ -976,12 +1007,15 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
"clwb", "intel-pt", "avx512pf", "avx512er",
"avx512cd", "sha-ni", "avx512bw", "avx512vl",
},
- .cpuid_eax = 7,
- .cpuid_needs_ecx = true, .cpuid_ecx = 0,
- .cpuid_reg = R_EBX,
+ .cpuid = {
+ .eax = 7,
+ .needs_ecx = true, .ecx = 0,
+ .reg = R_EBX,
+ },
.tcg_features = TCG_7_0_EBX_FEATURES,
},
[FEAT_7_0_ECX] = {
+ .type = CPUID_FEATURE_WORD,
.feat_names = {
NULL, "avx512vbmi", "umip", "pku",
NULL /* ospke */, NULL, "avx512vbmi2", NULL,
@@ -992,12 +1026,15 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL, "cldemote", NULL, NULL,
NULL, NULL, NULL, NULL,
},
- .cpuid_eax = 7,
- .cpuid_needs_ecx = true, .cpuid_ecx = 0,
- .cpuid_reg = R_ECX,
+ .cpuid = {
+ .eax = 7,
+ .needs_ecx = true, .ecx = 0,
+ .reg = R_ECX,
+ },
.tcg_features = TCG_7_0_ECX_FEATURES,
},
[FEAT_7_0_EDX] = {
+ .type = CPUID_FEATURE_WORD,
.feat_names = {
NULL, NULL, "avx512-4vnniw", "avx512-4fmaps",
NULL, NULL, NULL, NULL,
@@ -1008,13 +1045,16 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL, NULL, "spec-ctrl", NULL,
NULL, "arch-capabilities", NULL, "ssbd",
},
- .cpuid_eax = 7,
- .cpuid_needs_ecx = true, .cpuid_ecx = 0,
- .cpuid_reg = R_EDX,
+ .cpuid = {
+ .eax = 7,
+ .needs_ecx = true, .ecx = 0,
+ .reg = R_EDX,
+ },
.tcg_features = TCG_7_0_EDX_FEATURES,
.unmigratable_flags = CPUID_7_0_EDX_ARCH_CAPABILITIES,
},
[FEAT_8000_0007_EDX] = {
+ .type = CPUID_FEATURE_WORD,
.feat_names = {
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
@@ -1025,12 +1065,12 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
},
- .cpuid_eax = 0x80000007,
- .cpuid_reg = R_EDX,
+ .cpuid = { .eax = 0x80000007, .reg = R_EDX, },
.tcg_features = TCG_APM_FEATURES,
.unmigratable_flags = CPUID_APM_INVTSC,
},
[FEAT_8000_0008_EBX] = {
+ .type = CPUID_FEATURE_WORD,
.feat_names = {
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
@@ -1041,12 +1081,12 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
"amd-ssbd", "virt-ssbd", "amd-no-ssb", NULL,
NULL, NULL, NULL, NULL,
},
- .cpuid_eax = 0x80000008,
- .cpuid_reg = R_EBX,
+ .cpuid = { .eax = 0x80000008, .reg = R_EBX, },
.tcg_features = 0,
.unmigratable_flags = 0,
},
[FEAT_XSAVE] = {
+ .type = CPUID_FEATURE_WORD,
.feat_names = {
"xsaveopt", "xsavec", "xgetbv1", "xsaves",
NULL, NULL, NULL, NULL,
@@ -1057,12 +1097,15 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
},
- .cpuid_eax = 0xd,
- .cpuid_needs_ecx = true, .cpuid_ecx = 1,
- .cpuid_reg = R_EAX,
+ .cpuid = {
+ .eax = 0xd,
+ .needs_ecx = true, .ecx = 1,
+ .reg = R_EAX,
+ },
.tcg_features = TCG_XSAVE_FEATURES,
},
[FEAT_6_EAX] = {
+ .type = CPUID_FEATURE_WORD,
.feat_names = {
NULL, NULL, "arat", NULL,
NULL, NULL, NULL, NULL,
@@ -1073,13 +1116,16 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
NULL, NULL, NULL, NULL,
NULL, NULL, NULL, NULL,
},
- .cpuid_eax = 6, .cpuid_reg = R_EAX,
+ .cpuid = { .eax = 6, .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,
+ .type = CPUID_FEATURE_WORD,
+ .cpuid = {
+ .eax = 0xD,
+ .needs_ecx = true, .ecx = 0,
+ .reg = R_EAX,
+ },
.tcg_features = ~0U,
.migratable_flags = XSTATE_FP_MASK | XSTATE_SSE_MASK |
XSTATE_YMM_MASK | XSTATE_BNDREGS_MASK | XSTATE_BNDCSR_MASK |
@@ -1087,9 +1133,12 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
XSTATE_PKRU_MASK,
},
[FEAT_XSAVE_COMP_HI] = {
- .cpuid_eax = 0xD,
- .cpuid_needs_ecx = true, .cpuid_ecx = 0,
- .cpuid_reg = R_EDX,
+ .type = CPUID_FEATURE_WORD,
+ .cpuid = {
+ .eax = 0xD,
+ .needs_ecx = true, .ecx = 0,
+ .reg = R_EDX,
+ },
.tcg_features = ~0U,
},
};
@@ -2975,21 +3024,41 @@ static const TypeInfo host_x86_cpu_type_info = {
#endif
+static char *feature_word_description(FeatureWordInfo *f, uint32_t bit)
+{
+ assert(f->type == CPUID_FEATURE_WORD || f->type == MSR_FEATURE_WORD);
+
+ switch (f->type) {
+ case CPUID_FEATURE_WORD:
+ {
+ const char *reg = get_register_name_32(f->cpuid.reg);
+ assert(reg);
+ return g_strdup_printf("CPUID.%02XH:%s",
+ f->cpuid.eax, reg);
+ }
+ case MSR_FEATURE_WORD:
+ return g_strdup_printf("MSR(%02XH)",
+ f->msr.index);
+ }
+
+ return NULL;
+}
+
static void report_unavailable_features(FeatureWord w, uint32_t mask)
{
FeatureWordInfo *f = &feature_word_info[w];
int i;
+ char *feat_word_str;
for (i = 0; i < 32; ++i) {
if ((1UL << i) & mask) {
- const char *reg = get_register_name_32(f->cpuid_reg);
- assert(reg);
- warn_report("%s doesn't support requested feature: "
- "CPUID.%02XH:%s%s%s [bit %d]",
+ feat_word_str = feature_word_description(f, i);
+ warn_report("%s doesn't support requested feature: %s%s%s [bit %d]",
accel_uses_host_cpuid() ? "host" : "TCG",
- f->cpuid_eax, reg,
+ feat_word_str,
f->feat_names[i] ? "." : "",
f->feat_names[i] ? f->feat_names[i] : "", i);
+ g_free(feat_word_str);
}
}
}
@@ -3233,11 +3302,18 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v,
for (w = 0; w < FEATURE_WORDS; w++) {
FeatureWordInfo *wi = &feature_word_info[w];
+ /*
+ * We didn't have MSR features when "feature-words" was
+ * introduced. Therefore skipped other type entries.
+ */
+ if (wi->type != CPUID_FEATURE_WORD) {
+ continue;
+ }
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->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 = array[w];
/* List will be in reverse order, but order shouldn't matter */
@@ -3610,12 +3686,19 @@ static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
bool migratable_only)
{
FeatureWordInfo *wi = &feature_word_info[w];
- uint32_t r;
+ uint32_t r = 0;
if (kvm_enabled()) {
- r = kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid_eax,
- wi->cpuid_ecx,
- wi->cpuid_reg);
+ switch (wi->type) {
+ case CPUID_FEATURE_WORD:
+ r = kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid.eax,
+ wi->cpuid.ecx,
+ wi->cpuid.reg);
+ break;
+ case MSR_FEATURE_WORD:
+ r = kvm_arch_get_supported_msr_feature(kvm_state, wi->msr.index);
+ break;
+ }
} else if (hvf_enabled()) {
r = hvf_get_supported_cpuid(wi->cpuid_eax,
wi->cpuid_ecx,
@@ -4680,9 +4763,10 @@ static void x86_cpu_adjust_feat_level(X86CPU *cpu, FeatureWord w)
{
CPUX86State *env = &cpu->env;
FeatureWordInfo *fi = &feature_word_info[w];
- uint32_t eax = fi->cpuid_eax;
+ uint32_t eax = fi->cpuid.eax;
uint32_t region = eax & 0xF0000000;
+ assert(feature_word_info[w].type == CPUID_FEATURE_WORD);
if (!env->features[w]) {
return;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v5 3/3] x86: define a new MSR based feature word -- FEATURE_WORDS_ARCH_CAPABILITIES
2018-10-15 4:47 [Qemu-devel] [PATCH v5 0/3] x86: QEMU side support on MSR based features Robert Hoo
2018-10-15 4:47 ` [Qemu-devel] [PATCH v5 1/3] kvm: Add support to KVM_GET_MSR_FEATURE_INDEX_LIST and KVM_GET_MSRS system ioctl Robert Hoo
2018-10-15 4:47 ` [Qemu-devel] [PATCH v5 2/3] x86: Data structure changes to support MSR based features Robert Hoo
@ 2018-10-15 4:47 ` Robert Hoo
2018-10-24 10:06 ` Eduardo Habkost
2 siblings, 1 reply; 13+ messages in thread
From: Robert Hoo @ 2018-10-15 4:47 UTC (permalink / raw)
To: pbonzini, rth, ehabkost, thomas.lendacky
Cc: qemu-devel, robert.hu, Robert Hoo
Note RSBA is specially treated -- no matter host support it or not, qemu
pretends it is supported.
Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
---
target/i386/cpu.c | 31 ++++++++++++++++++++++++++++++-
target/i386/cpu.h | 8 ++++++++
target/i386/kvm.c | 11 +++++++++++
3 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index d191b9c..51c8fd8 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1141,6 +1141,27 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
},
.tcg_features = ~0U,
},
+ /*Below are MSR exposed features*/
+ [FEAT_ARCH_CAPABILITIES] = {
+ .type = MSR_FEATURE_WORD,
+ .feat_names = {
+ "rdctl-no", "ibrs-all", "rsba", "skip-l1dfl-vmentry",
+ "ssb-no", 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,
+ },
+ .msr = {
+ .index = MSR_IA32_ARCH_CAPABILITIES,
+ .cpuid_dep = {
+ FEAT_7_0_EDX,
+ CPUID_7_0_EDX_ARCH_CAPABILITIES
+ }
+ },
+ },
};
typedef struct X86RegisterInfo32 {
@@ -3696,7 +3717,15 @@ static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
wi->cpuid.reg);
break;
case MSR_FEATURE_WORD:
- r = kvm_arch_get_supported_msr_feature(kvm_state, wi->msr.index);
+ /* Special case:
+ * No matter host status, IA32_ARCH_CAPABILITIES.RSBA [bit 2]
+ * is always supported in guest.
+ */
+ if (wi->msr.index == MSR_IA32_ARCH_CAPABILITIES) {
+ r = MSR_ARCH_CAP_RSBA;
+ }
+ r |= kvm_arch_get_supported_msr_feature(kvm_state,
+ wi->msr.index);
break;
}
} else if (hvf_enabled()) {
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 730c06f..52a52ec 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -502,6 +502,7 @@ typedef enum FeatureWord {
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 */
+ FEAT_ARCH_CAPABILITIES,
FEATURE_WORDS,
} FeatureWord;
@@ -730,6 +731,13 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
#define CPUID_TOPOLOGY_LEVEL_SMT (1U << 8)
#define CPUID_TOPOLOGY_LEVEL_CORE (2U << 8)
+/* MSR Feature Bits */
+#define MSR_ARCH_CAP_RDCL_NO (1U << 0)
+#define MSR_ARCH_CAP_IBRS_ALL (1U << 1)
+#define MSR_ARCH_CAP_RSBA (1U << 2)
+#define MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY (1U << 3)
+#define MSR_ARCH_CAP_SSB_NO (1U << 4)
+
#ifndef HYPERV_SPINLOCK_NEVER_RETRY
#define HYPERV_SPINLOCK_NEVER_RETRY 0xFFFFFFFF
#endif
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index db79dad..2f7b40d 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1928,6 +1928,17 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
}
#endif
+ /* If host supports feature MSR, write down. */
+ if (kvm_feature_msrs) {
+ int i;
+ for (i = 0; i < kvm_feature_msrs->nmsrs; i++)
+ if (kvm_feature_msrs->indices[i] == MSR_IA32_ARCH_CAPABILITIES) {
+ kvm_msr_entry_add(cpu, MSR_IA32_ARCH_CAPABILITIES,
+ env->features[FEAT_ARCH_CAPABILITIES]);
+ break;
+ }
+ }
+
/*
* The following MSRs have side effects on the guest or are too heavy
* for normal writeback. Limit them to reset or full state updates.
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v5 1/3] kvm: Add support to KVM_GET_MSR_FEATURE_INDEX_LIST and KVM_GET_MSRS system ioctl
2018-10-15 4:47 ` [Qemu-devel] [PATCH v5 1/3] kvm: Add support to KVM_GET_MSR_FEATURE_INDEX_LIST and KVM_GET_MSRS system ioctl Robert Hoo
@ 2018-10-24 9:49 ` Eduardo Habkost
0 siblings, 0 replies; 13+ messages in thread
From: Eduardo Habkost @ 2018-10-24 9:49 UTC (permalink / raw)
To: Robert Hoo; +Cc: pbonzini, rth, thomas.lendacky, robert.hu, qemu-devel
On Mon, Oct 15, 2018 at 12:47:23PM +0800, Robert Hoo wrote:
> Add kvm_get_supported_feature_msrs() to get supported MSR feature index list.
> Add kvm_arch_get_supported_msr_feature() to get each MSR features value.
>
> Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
--
Eduardo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v5 2/3] x86: Data structure changes to support MSR based features
2018-10-15 4:47 ` [Qemu-devel] [PATCH v5 2/3] x86: Data structure changes to support MSR based features Robert Hoo
@ 2018-10-24 9:56 ` Eduardo Habkost
2018-10-24 10:16 ` Eduardo Habkost
1 sibling, 0 replies; 13+ messages in thread
From: Eduardo Habkost @ 2018-10-24 9:56 UTC (permalink / raw)
To: Robert Hoo; +Cc: pbonzini, rth, thomas.lendacky, robert.hu, qemu-devel
On Mon, Oct 15, 2018 at 12:47:24PM +0800, Robert Hoo wrote:
> Add FeatureWordType indicator in struct FeatureWordInfo.
> Change feature_word_info[] accordingly.
> Change existing functions that refer to feature_word_info[] accordingly.
>
> Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
> ---
> target/i386/cpu.c | 188 +++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 136 insertions(+), 52 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index c88876d..d191b9c 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -770,17 +770,36 @@ static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
> /* missing:
> CPUID_XSAVE_XSAVEC, CPUID_XSAVE_XSAVES */
>
> +typedef enum FeatureWordType {
> + CPUID_FEATURE_WORD,
> + MSR_FEATURE_WORD,
> +} FeatureWordType;
> +
> typedef struct FeatureWordInfo {
> + FeatureWordType type;
> /* 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 */
> - int cpuid_reg; /* output register (R_* constant) */
> + union {
> + /* If type==CPUID_FEATURE_WORD */
> + struct {
> + uint32_t eax; /* Input EAX for CPUID */
> + bool needs_ecx; /* CPUID instruction uses ECX as input */
> + uint32_t ecx; /* Input ECX value for CPUID */
> + int reg; /* output register (R_* constant) */
> + } cpuid;
> + /* If type==MSR_FEATURE_WORD */
> + struct {
> + uint32_t index;
> + struct { /*CPUID that enumerate this MSR*/
> + FeatureWord cpuid_class;
> + uint32_t cpuid_flag;
> + } cpuid_dep;
> + } msr;
> + };
> uint32_t tcg_features; /* Feature flags supported by TCG */
> uint32_t unmigratable_flags; /* Feature flags known to be unmigratable */
> uint32_t migratable_flags; /* Feature flags known to be migratable */
> @@ -790,6 +809,7 @@ typedef struct FeatureWordInfo {
>
> static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> [FEAT_1_EDX] = {
> + .type = CPUID_FEATURE_WORD,
> .feat_names = {
> "fpu", "vme", "de", "pse",
> "tsc", "msr", "pae", "mce",
> @@ -800,10 +820,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> "fxsr", "sse", "sse2", "ss",
> "ht" /* Intel htt */, "tm", "ia64", "pbe",
> },
> - .cpuid_eax = 1, .cpuid_reg = R_EDX,
> + .cpuid = {.eax = 1, .reg = R_EDX, },
> .tcg_features = TCG_FEATURES,
> },
> [FEAT_1_ECX] = {
> + .type = CPUID_FEATURE_WORD,
> .feat_names = {
> "pni" /* Intel,AMD sse3 */, "pclmulqdq", "dtes64", "monitor",
> "ds-cpl", "vmx", "smx", "est",
> @@ -814,7 +835,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> "tsc-deadline", "aes", "xsave", NULL /* osxsave */,
> "avx", "f16c", "rdrand", "hypervisor",
> },
> - .cpuid_eax = 1, .cpuid_reg = R_ECX,
> + .cpuid = { .eax = 1, .reg = R_ECX, },
> .tcg_features = TCG_EXT_FEATURES,
> },
> /* Feature names that are already defined on feature_name[] but
> @@ -823,6 +844,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> * to features[FEAT_8000_0001_EDX] if and only if CPU vendor is AMD.
> */
> [FEAT_8000_0001_EDX] = {
> + .type = CPUID_FEATURE_WORD,
> .feat_names = {
> NULL /* fpu */, NULL /* vme */, NULL /* de */, NULL /* pse */,
> NULL /* tsc */, NULL /* msr */, NULL /* pae */, NULL /* mce */,
> @@ -833,10 +855,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> NULL /* fxsr */, "fxsr-opt", "pdpe1gb", "rdtscp",
> NULL, "lm", "3dnowext", "3dnow",
> },
> - .cpuid_eax = 0x80000001, .cpuid_reg = R_EDX,
> + .cpuid = { .eax = 0x80000001, .reg = R_EDX, },
> .tcg_features = TCG_EXT2_FEATURES,
> },
> [FEAT_8000_0001_ECX] = {
> + .type = CPUID_FEATURE_WORD,
> .feat_names = {
> "lahf-lm", "cmp-legacy", "svm", "extapic",
> "cr8legacy", "abm", "sse4a", "misalignsse",
> @@ -847,7 +870,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> "perfctr-nb", NULL, NULL, NULL,
> NULL, NULL, NULL, NULL,
> },
> - .cpuid_eax = 0x80000001, .cpuid_reg = R_ECX,
> + .cpuid = { .eax = 0x80000001, .reg = R_ECX, },
> .tcg_features = TCG_EXT3_FEATURES,
> /*
> * TOPOEXT is always allowed but can't be enabled blindly by
> @@ -857,6 +880,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> .no_autoenable_flags = CPUID_EXT3_TOPOEXT,
> },
> [FEAT_C000_0001_EDX] = {
> + .type = CPUID_FEATURE_WORD,
> .feat_names = {
> NULL, NULL, "xstore", "xstore-en",
> NULL, NULL, "xcrypt", "xcrypt-en",
> @@ -867,10 +891,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> NULL, NULL, NULL, NULL,
> NULL, NULL, NULL, NULL,
> },
> - .cpuid_eax = 0xC0000001, .cpuid_reg = R_EDX,
> + .cpuid = { .eax = 0xC0000001, .reg = R_EDX, },
> .tcg_features = TCG_EXT4_FEATURES,
> },
> [FEAT_KVM] = {
> + .type = CPUID_FEATURE_WORD,
> .feat_names = {
> "kvmclock", "kvm-nopiodelay", "kvm-mmu", "kvmclock",
> "kvm-asyncpf", "kvm-steal-time", "kvm-pv-eoi", "kvm-pv-unhalt",
> @@ -881,10 +906,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> "kvmclock-stable-bit", NULL, NULL, NULL,
> NULL, NULL, NULL, NULL,
> },
> - .cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EAX,
> + .cpuid = { .eax = KVM_CPUID_FEATURES, .reg = R_EAX, },
> .tcg_features = TCG_KVM_FEATURES,
> },
> [FEAT_KVM_HINTS] = {
> + .type = CPUID_FEATURE_WORD,
> .feat_names = {
> "kvm-hint-dedicated", NULL, NULL, NULL,
> NULL, NULL, NULL, NULL,
> @@ -895,7 +921,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> NULL, NULL, NULL, NULL,
> NULL, NULL, NULL, NULL,
> },
> - .cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EDX,
> + .cpuid = { .eax = KVM_CPUID_FEATURES, .reg = R_EDX, },
> .tcg_features = TCG_KVM_FEATURES,
> /*
> * KVM hints aren't auto-enabled by -cpu host, they need to be
> @@ -904,6 +930,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> .no_autoenable_flags = ~0U,
> },
> [FEAT_HYPERV_EAX] = {
> + .type = CPUID_FEATURE_WORD,
> .feat_names = {
> NULL /* hv_msr_vp_runtime_access */, NULL /* hv_msr_time_refcount_access */,
> NULL /* hv_msr_synic_access */, NULL /* hv_msr_stimer_access */,
> @@ -918,9 +945,10 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> NULL, NULL, NULL, NULL,
> NULL, NULL, NULL, NULL,
> },
> - .cpuid_eax = 0x40000003, .cpuid_reg = R_EAX,
> + .cpuid = { .eax = 0x40000003, .reg = R_EAX, },
> },
> [FEAT_HYPERV_EBX] = {
> + .type = CPUID_FEATURE_WORD,
> .feat_names = {
> NULL /* hv_create_partitions */, NULL /* hv_access_partition_id */,
> NULL /* hv_access_memory_pool */, NULL /* hv_adjust_message_buffers */,
> @@ -934,9 +962,10 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> NULL, NULL, NULL, NULL,
> NULL, NULL, NULL, NULL,
> },
> - .cpuid_eax = 0x40000003, .cpuid_reg = R_EBX,
> + .cpuid = { .eax = 0x40000003, .reg = R_EBX, },
> },
> [FEAT_HYPERV_EDX] = {
> + .type = CPUID_FEATURE_WORD,
> .feat_names = {
> NULL /* hv_mwait */, NULL /* hv_guest_debugging */,
> NULL /* hv_perf_monitor */, NULL /* hv_cpu_dynamic_part */,
> @@ -949,9 +978,10 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> NULL, NULL, NULL, NULL,
> NULL, NULL, NULL, NULL,
> },
> - .cpuid_eax = 0x40000003, .cpuid_reg = R_EDX,
> + .cpuid = { .eax = 0x40000003, .reg = R_EDX, },
> },
> [FEAT_SVM] = {
> + .type = CPUID_FEATURE_WORD,
> .feat_names = {
> "npt", "lbrv", "svm-lock", "nrip-save",
> "tsc-scale", "vmcb-clean", "flushbyasid", "decodeassists",
> @@ -962,10 +992,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> NULL, NULL, NULL, NULL,
> NULL, NULL, NULL, NULL,
> },
> - .cpuid_eax = 0x8000000A, .cpuid_reg = R_EDX,
> + .cpuid = { .eax = 0x8000000A, .reg = R_EDX, },
> .tcg_features = TCG_SVM_FEATURES,
> },
> [FEAT_7_0_EBX] = {
> + .type = CPUID_FEATURE_WORD,
> .feat_names = {
> "fsgsbase", "tsc-adjust", NULL, "bmi1",
> "hle", "avx2", NULL, "smep",
> @@ -976,12 +1007,15 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> "clwb", "intel-pt", "avx512pf", "avx512er",
> "avx512cd", "sha-ni", "avx512bw", "avx512vl",
> },
> - .cpuid_eax = 7,
> - .cpuid_needs_ecx = true, .cpuid_ecx = 0,
> - .cpuid_reg = R_EBX,
> + .cpuid = {
> + .eax = 7,
> + .needs_ecx = true, .ecx = 0,
> + .reg = R_EBX,
> + },
> .tcg_features = TCG_7_0_EBX_FEATURES,
> },
> [FEAT_7_0_ECX] = {
> + .type = CPUID_FEATURE_WORD,
> .feat_names = {
> NULL, "avx512vbmi", "umip", "pku",
> NULL /* ospke */, NULL, "avx512vbmi2", NULL,
> @@ -992,12 +1026,15 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> NULL, "cldemote", NULL, NULL,
> NULL, NULL, NULL, NULL,
> },
> - .cpuid_eax = 7,
> - .cpuid_needs_ecx = true, .cpuid_ecx = 0,
> - .cpuid_reg = R_ECX,
> + .cpuid = {
> + .eax = 7,
> + .needs_ecx = true, .ecx = 0,
> + .reg = R_ECX,
> + },
> .tcg_features = TCG_7_0_ECX_FEATURES,
> },
> [FEAT_7_0_EDX] = {
> + .type = CPUID_FEATURE_WORD,
> .feat_names = {
> NULL, NULL, "avx512-4vnniw", "avx512-4fmaps",
> NULL, NULL, NULL, NULL,
> @@ -1008,13 +1045,16 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> NULL, NULL, "spec-ctrl", NULL,
> NULL, "arch-capabilities", NULL, "ssbd",
> },
> - .cpuid_eax = 7,
> - .cpuid_needs_ecx = true, .cpuid_ecx = 0,
> - .cpuid_reg = R_EDX,
> + .cpuid = {
> + .eax = 7,
> + .needs_ecx = true, .ecx = 0,
> + .reg = R_EDX,
> + },
> .tcg_features = TCG_7_0_EDX_FEATURES,
> .unmigratable_flags = CPUID_7_0_EDX_ARCH_CAPABILITIES,
> },
> [FEAT_8000_0007_EDX] = {
> + .type = CPUID_FEATURE_WORD,
> .feat_names = {
> NULL, NULL, NULL, NULL,
> NULL, NULL, NULL, NULL,
> @@ -1025,12 +1065,12 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> NULL, NULL, NULL, NULL,
> NULL, NULL, NULL, NULL,
> },
> - .cpuid_eax = 0x80000007,
> - .cpuid_reg = R_EDX,
> + .cpuid = { .eax = 0x80000007, .reg = R_EDX, },
> .tcg_features = TCG_APM_FEATURES,
> .unmigratable_flags = CPUID_APM_INVTSC,
> },
> [FEAT_8000_0008_EBX] = {
> + .type = CPUID_FEATURE_WORD,
> .feat_names = {
> NULL, NULL, NULL, NULL,
> NULL, NULL, NULL, NULL,
> @@ -1041,12 +1081,12 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> "amd-ssbd", "virt-ssbd", "amd-no-ssb", NULL,
> NULL, NULL, NULL, NULL,
> },
> - .cpuid_eax = 0x80000008,
> - .cpuid_reg = R_EBX,
> + .cpuid = { .eax = 0x80000008, .reg = R_EBX, },
> .tcg_features = 0,
> .unmigratable_flags = 0,
> },
> [FEAT_XSAVE] = {
> + .type = CPUID_FEATURE_WORD,
> .feat_names = {
> "xsaveopt", "xsavec", "xgetbv1", "xsaves",
> NULL, NULL, NULL, NULL,
> @@ -1057,12 +1097,15 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> NULL, NULL, NULL, NULL,
> NULL, NULL, NULL, NULL,
> },
> - .cpuid_eax = 0xd,
> - .cpuid_needs_ecx = true, .cpuid_ecx = 1,
> - .cpuid_reg = R_EAX,
> + .cpuid = {
> + .eax = 0xd,
> + .needs_ecx = true, .ecx = 1,
> + .reg = R_EAX,
> + },
> .tcg_features = TCG_XSAVE_FEATURES,
> },
> [FEAT_6_EAX] = {
> + .type = CPUID_FEATURE_WORD,
> .feat_names = {
> NULL, NULL, "arat", NULL,
> NULL, NULL, NULL, NULL,
> @@ -1073,13 +1116,16 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> NULL, NULL, NULL, NULL,
> NULL, NULL, NULL, NULL,
> },
> - .cpuid_eax = 6, .cpuid_reg = R_EAX,
> + .cpuid = { .eax = 6, .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,
> + .type = CPUID_FEATURE_WORD,
> + .cpuid = {
> + .eax = 0xD,
> + .needs_ecx = true, .ecx = 0,
> + .reg = R_EAX,
> + },
> .tcg_features = ~0U,
> .migratable_flags = XSTATE_FP_MASK | XSTATE_SSE_MASK |
> XSTATE_YMM_MASK | XSTATE_BNDREGS_MASK | XSTATE_BNDCSR_MASK |
> @@ -1087,9 +1133,12 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> XSTATE_PKRU_MASK,
> },
> [FEAT_XSAVE_COMP_HI] = {
> - .cpuid_eax = 0xD,
> - .cpuid_needs_ecx = true, .cpuid_ecx = 0,
> - .cpuid_reg = R_EDX,
> + .type = CPUID_FEATURE_WORD,
> + .cpuid = {
> + .eax = 0xD,
> + .needs_ecx = true, .ecx = 0,
> + .reg = R_EDX,
> + },
> .tcg_features = ~0U,
> },
> };
> @@ -2975,21 +3024,41 @@ static const TypeInfo host_x86_cpu_type_info = {
>
> #endif
>
> +static char *feature_word_description(FeatureWordInfo *f, uint32_t bit)
> +{
> + assert(f->type == CPUID_FEATURE_WORD || f->type == MSR_FEATURE_WORD);
> +
> + switch (f->type) {
> + case CPUID_FEATURE_WORD:
> + {
> + const char *reg = get_register_name_32(f->cpuid.reg);
> + assert(reg);
> + return g_strdup_printf("CPUID.%02XH:%s",
> + f->cpuid.eax, reg);
> + }
> + case MSR_FEATURE_WORD:
> + return g_strdup_printf("MSR(%02XH)",
> + f->msr.index);
> + }
> +
> + return NULL;
> +}
> +
> static void report_unavailable_features(FeatureWord w, uint32_t mask)
> {
> FeatureWordInfo *f = &feature_word_info[w];
> int i;
> + char *feat_word_str;
>
> for (i = 0; i < 32; ++i) {
> if ((1UL << i) & mask) {
> - const char *reg = get_register_name_32(f->cpuid_reg);
> - assert(reg);
> - warn_report("%s doesn't support requested feature: "
> - "CPUID.%02XH:%s%s%s [bit %d]",
> + feat_word_str = feature_word_description(f, i);
> + warn_report("%s doesn't support requested feature: %s%s%s [bit %d]",
> accel_uses_host_cpuid() ? "host" : "TCG",
> - f->cpuid_eax, reg,
> + feat_word_str,
> f->feat_names[i] ? "." : "",
> f->feat_names[i] ? f->feat_names[i] : "", i);
> + g_free(feat_word_str);
> }
> }
> }
> @@ -3233,11 +3302,18 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v,
>
> for (w = 0; w < FEATURE_WORDS; w++) {
> FeatureWordInfo *wi = &feature_word_info[w];
> + /*
> + * We didn't have MSR features when "feature-words" was
> + * introduced. Therefore skipped other type entries.
> + */
> + if (wi->type != CPUID_FEATURE_WORD) {
> + continue;
> + }
> 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->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 = array[w];
>
> /* List will be in reverse order, but order shouldn't matter */
> @@ -3610,12 +3686,19 @@ static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
> bool migratable_only)
> {
> FeatureWordInfo *wi = &feature_word_info[w];
> - uint32_t r;
> + uint32_t r = 0;
>
> if (kvm_enabled()) {
> - r = kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid_eax,
> - wi->cpuid_ecx,
> - wi->cpuid_reg);
> + switch (wi->type) {
> + case CPUID_FEATURE_WORD:
> + r = kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid.eax,
> + wi->cpuid.ecx,
> + wi->cpuid.reg);
> + break;
> + case MSR_FEATURE_WORD:
> + r = kvm_arch_get_supported_msr_feature(kvm_state, wi->msr.index);
> + break;
> + }
> } else if (hvf_enabled()) {
I just noticed this issue:
You need to check wi->type for the other accelerators too,
otherwise we're going to call hvf_get_supported_cpuid() with
bogus EAX/ECX values. I suggest the fixup below.
The rest of the patch looks good to me.
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 2b58bca62e..bcfe6928c1 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3729,6 +3729,9 @@ static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
break;
}
} else if (hvf_enabled()) {
+ if (wi->type != CPUID_FEATURE_WORD) {
+ return 0;
+ }
r = hvf_get_supported_cpuid(wi->cpuid_eax,
wi->cpuid_ecx,
wi->cpuid_reg);
> r = hvf_get_supported_cpuid(wi->cpuid_eax,
> wi->cpuid_ecx,
> @@ -4680,9 +4763,10 @@ static void x86_cpu_adjust_feat_level(X86CPU *cpu, FeatureWord w)
> {
> CPUX86State *env = &cpu->env;
> FeatureWordInfo *fi = &feature_word_info[w];
> - uint32_t eax = fi->cpuid_eax;
> + uint32_t eax = fi->cpuid.eax;
> uint32_t region = eax & 0xF0000000;
>
> + assert(feature_word_info[w].type == CPUID_FEATURE_WORD);
> if (!env->features[w]) {
> return;
> }
> --
> 1.8.3.1
>
>
--
Eduardo
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/3] x86: define a new MSR based feature word -- FEATURE_WORDS_ARCH_CAPABILITIES
2018-10-15 4:47 ` [Qemu-devel] [PATCH v5 3/3] x86: define a new MSR based feature word -- FEATURE_WORDS_ARCH_CAPABILITIES Robert Hoo
@ 2018-10-24 10:06 ` Eduardo Habkost
2018-10-25 3:16 ` Robert Hoo
2018-10-26 3:01 ` Robert Hoo
0 siblings, 2 replies; 13+ messages in thread
From: Eduardo Habkost @ 2018-10-24 10:06 UTC (permalink / raw)
To: Robert Hoo; +Cc: pbonzini, rth, thomas.lendacky, robert.hu, qemu-devel
On Mon, Oct 15, 2018 at 12:47:25PM +0800, Robert Hoo wrote:
> Note RSBA is specially treated -- no matter host support it or not, qemu
> pretends it is supported.
>
> Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
I am now wondering what else we need to be able to remove
CPUID_7_0_EDX_ARCH_CAPABILITIES from
feature_word_info[FEAT_7_0_EDX].unmigratable_flags.
This series is necessary for that, be I think we still can't let
the VM be migrated if arch-capabilities is enabled and we're
running on a host that doesn't have MSR_IA32_ARCH_CAPABILITIES on
kvm_feature_msrs.
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> target/i386/cpu.c | 31 ++++++++++++++++++++++++++++++-
> target/i386/cpu.h | 8 ++++++++
> target/i386/kvm.c | 11 +++++++++++
> 3 files changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index d191b9c..51c8fd8 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -1141,6 +1141,27 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> },
> .tcg_features = ~0U,
> },
> + /*Below are MSR exposed features*/
> + [FEAT_ARCH_CAPABILITIES] = {
> + .type = MSR_FEATURE_WORD,
> + .feat_names = {
> + "rdctl-no", "ibrs-all", "rsba", "skip-l1dfl-vmentry",
> + "ssb-no", 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,
> + },
> + .msr = {
> + .index = MSR_IA32_ARCH_CAPABILITIES,
> + .cpuid_dep = {
> + FEAT_7_0_EDX,
> + CPUID_7_0_EDX_ARCH_CAPABILITIES
> + }
> + },
> + },
> };
>
> typedef struct X86RegisterInfo32 {
> @@ -3696,7 +3717,15 @@ static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
> wi->cpuid.reg);
> break;
> case MSR_FEATURE_WORD:
> - r = kvm_arch_get_supported_msr_feature(kvm_state, wi->msr.index);
> + /* Special case:
> + * No matter host status, IA32_ARCH_CAPABILITIES.RSBA [bit 2]
> + * is always supported in guest.
> + */
> + if (wi->msr.index == MSR_IA32_ARCH_CAPABILITIES) {
> + r = MSR_ARCH_CAP_RSBA;
> + }
> + r |= kvm_arch_get_supported_msr_feature(kvm_state,
> + wi->msr.index);
> break;
> }
> } else if (hvf_enabled()) {
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 730c06f..52a52ec 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -502,6 +502,7 @@ typedef enum FeatureWord {
> 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 */
> + FEAT_ARCH_CAPABILITIES,
> FEATURE_WORDS,
> } FeatureWord;
>
> @@ -730,6 +731,13 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> #define CPUID_TOPOLOGY_LEVEL_SMT (1U << 8)
> #define CPUID_TOPOLOGY_LEVEL_CORE (2U << 8)
>
> +/* MSR Feature Bits */
> +#define MSR_ARCH_CAP_RDCL_NO (1U << 0)
> +#define MSR_ARCH_CAP_IBRS_ALL (1U << 1)
> +#define MSR_ARCH_CAP_RSBA (1U << 2)
> +#define MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY (1U << 3)
> +#define MSR_ARCH_CAP_SSB_NO (1U << 4)
> +
> #ifndef HYPERV_SPINLOCK_NEVER_RETRY
> #define HYPERV_SPINLOCK_NEVER_RETRY 0xFFFFFFFF
> #endif
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index db79dad..2f7b40d 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -1928,6 +1928,17 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> }
> #endif
>
> + /* If host supports feature MSR, write down. */
> + if (kvm_feature_msrs) {
> + int i;
> + for (i = 0; i < kvm_feature_msrs->nmsrs; i++)
> + if (kvm_feature_msrs->indices[i] == MSR_IA32_ARCH_CAPABILITIES) {
> + kvm_msr_entry_add(cpu, MSR_IA32_ARCH_CAPABILITIES,
> + env->features[FEAT_ARCH_CAPABILITIES]);
> + break;
> + }
> + }
> +
> /*
> * The following MSRs have side effects on the guest or are too heavy
> * for normal writeback. Limit them to reset or full state updates.
> --
> 1.8.3.1
>
>
--
Eduardo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v5 2/3] x86: Data structure changes to support MSR based features
2018-10-15 4:47 ` [Qemu-devel] [PATCH v5 2/3] x86: Data structure changes to support MSR based features Robert Hoo
2018-10-24 9:56 ` Eduardo Habkost
@ 2018-10-24 10:16 ` Eduardo Habkost
2018-10-25 3:06 ` Robert Hoo
1 sibling, 1 reply; 13+ messages in thread
From: Eduardo Habkost @ 2018-10-24 10:16 UTC (permalink / raw)
To: Robert Hoo; +Cc: pbonzini, rth, thomas.lendacky, robert.hu, qemu-devel
On Mon, Oct 15, 2018 at 12:47:24PM +0800, Robert Hoo wrote:
> Add FeatureWordType indicator in struct FeatureWordInfo.
> Change feature_word_info[] accordingly.
> Change existing functions that refer to feature_word_info[] accordingly.
>
> Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
> ---
> target/i386/cpu.c | 188 +++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 136 insertions(+), 52 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index c88876d..d191b9c 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -770,17 +770,36 @@ static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
> /* missing:
> CPUID_XSAVE_XSAVEC, CPUID_XSAVE_XSAVES */
>
> +typedef enum FeatureWordType {
> + CPUID_FEATURE_WORD,
> + MSR_FEATURE_WORD,
> +} FeatureWordType;
> +
> typedef struct FeatureWordInfo {
> + FeatureWordType type;
> /* 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 */
> - int cpuid_reg; /* output register (R_* constant) */
> + union {
> + /* If type==CPUID_FEATURE_WORD */
> + struct {
> + uint32_t eax; /* Input EAX for CPUID */
> + bool needs_ecx; /* CPUID instruction uses ECX as input */
> + uint32_t ecx; /* Input ECX value for CPUID */
> + int reg; /* output register (R_* constant) */
> + } cpuid;
> + /* If type==MSR_FEATURE_WORD */
> + struct {
> + uint32_t index;
> + struct { /*CPUID that enumerate this MSR*/
> + FeatureWord cpuid_class;
> + uint32_t cpuid_flag;
> + } cpuid_dep;
Aren't you going to use this field anywhere? Probably we want to
prevent the VM from starting if a bit is set in the feature world
but the cpuid_dep bit is not set.
e.g.:
qemu-system-x86_64 -cpu Skylake-Client,-arch-capabilities,+rsba
probably should fail to start.
> + } msr;
> + };
[...]
--
Eduardo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v5 2/3] x86: Data structure changes to support MSR based features
2018-10-24 10:16 ` Eduardo Habkost
@ 2018-10-25 3:06 ` Robert Hoo
2018-10-25 13:36 ` Eduardo Habkost
0 siblings, 1 reply; 13+ messages in thread
From: Robert Hoo @ 2018-10-25 3:06 UTC (permalink / raw)
To: Eduardo Habkost
Cc: robert.hu, robert.hu, pbonzini, rth, thomas.lendacky, qemu-devel
On Wed, 2018-10-24 at 07:16 -0300, Eduardo Habkost wrote:
> On Mon, Oct 15, 2018 at 12:47:24PM +0800, Robert Hoo wrote:
> > Add FeatureWordType indicator in struct FeatureWordInfo.
> > Change feature_word_info[] accordingly.
> > Change existing functions that refer to feature_word_info[]
> > accordingly.
> >
> > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
> > ---
> > target/i386/cpu.c | 188 +++++++++++++++++++++++++++++++++++++++---
> > ------------
> > 1 file changed, 136 insertions(+), 52 deletions(-)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index c88876d..d191b9c 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -770,17 +770,36 @@ static void x86_cpu_vendor_words2str(char
> > *dst, uint32_t vendor1,
> > /* missing:
> > CPUID_XSAVE_XSAVEC, CPUID_XSAVE_XSAVES */
> >
> > +typedef enum FeatureWordType {
> > + CPUID_FEATURE_WORD,
> > + MSR_FEATURE_WORD,
> > +} FeatureWordType;
> > +
> > typedef struct FeatureWordInfo {
> > + FeatureWordType type;
> > /* 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 */
> > - int cpuid_reg; /* output register (R_* constant) */
> > + union {
> > + /* If type==CPUID_FEATURE_WORD */
> > + struct {
> > + uint32_t eax; /* Input EAX for CPUID */
> > + bool needs_ecx; /* CPUID instruction uses ECX as input
> > */
> > + uint32_t ecx; /* Input ECX value for CPUID */
> > + int reg; /* output register (R_* constant) */
> > + } cpuid;
> > + /* If type==MSR_FEATURE_WORD */
> > + struct {
> > + uint32_t index;
> > + struct { /*CPUID that enumerate this MSR*/
> > + FeatureWord cpuid_class;
> > + uint32_t cpuid_flag;
> > + } cpuid_dep;
>
> Aren't you going to use this field anywhere? Probably we want to
> prevent the VM from starting if a bit is set in the feature world
> but the cpuid_dep bit is not set.
>
> e.g.:
> qemu-system-x86_64 -cpu Skylake-Client,-arch-capabilities,+rsba
> probably should fail to start.
How about in x86_cpu_filter_features() filters the MSR feature word, if
its dependent CPUID feature bit is not set?
The filter results will be record in cpu->filtered_features, and print
out, like other filtered features.
>
> > + } msr;
> > + };
>
> [...]
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/3] x86: define a new MSR based feature word -- FEATURE_WORDS_ARCH_CAPABILITIES
2018-10-24 10:06 ` Eduardo Habkost
@ 2018-10-25 3:16 ` Robert Hoo
2018-10-26 3:01 ` Robert Hoo
1 sibling, 0 replies; 13+ messages in thread
From: Robert Hoo @ 2018-10-25 3:16 UTC (permalink / raw)
To: Eduardo Habkost
Cc: robert.hu, robert.hu, pbonzini, rth, thomas.lendacky, qemu-devel
On Wed, 2018-10-24 at 07:06 -0300, Eduardo Habkost wrote:
> On Mon, Oct 15, 2018 at 12:47:25PM +0800, Robert Hoo wrote:
> > Note RSBA is specially treated -- no matter host support it or not,
> > qemu
> > pretends it is supported.
> >
> > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
>
> I am now wondering what else we need to be able to remove
> CPUID_7_0_EDX_ARCH_CAPABILITIES from
> feature_word_info[FEAT_7_0_EDX].unmigratable_flags.
Let me know once some thought comes out to you.
>
> This series is necessary for that, be I think we still can't let
> the VM be migrated if arch-capabilities is enabled and we're
> running on a host that doesn't have MSR_IA32_ARCH_CAPABILITIES on
> kvm_feature_msrs.
Agree. So I still keep CPUID_7_0_EDX_ARCH_CAPABILITIES in
feature_word_info[FEAT_7_0_EDX].unmigratable_flags for now.
>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>
> > ---
> > target/i386/cpu.c | 31 ++++++++++++++++++++++++++++++-
> > target/i386/cpu.h | 8 ++++++++
> > target/i386/kvm.c | 11 +++++++++++
> > 3 files changed, 49 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index d191b9c..51c8fd8 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -1141,6 +1141,27 @@ static FeatureWordInfo
> > feature_word_info[FEATURE_WORDS] = {
> > },
> > .tcg_features = ~0U,
> > },
> > + /*Below are MSR exposed features*/
> > + [FEAT_ARCH_CAPABILITIES] = {
> > + .type = MSR_FEATURE_WORD,
> > + .feat_names = {
> > + "rdctl-no", "ibrs-all", "rsba", "skip-l1dfl-vmentry",
> > + "ssb-no", 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,
> > + },
> > + .msr = {
> > + .index = MSR_IA32_ARCH_CAPABILITIES,
> > + .cpuid_dep = {
> > + FEAT_7_0_EDX,
> > + CPUID_7_0_EDX_ARCH_CAPABILITIES
> > + }
> > + },
> > + },
> > };
> >
> > typedef struct X86RegisterInfo32 {
> > @@ -3696,7 +3717,15 @@ static uint32_t
> > x86_cpu_get_supported_feature_word(FeatureWord w,
> > wi-
> > >cpuid.reg);
> > break;
> > case MSR_FEATURE_WORD:
> > - r = kvm_arch_get_supported_msr_feature(kvm_state, wi-
> > >msr.index);
> > + /* Special case:
> > + * No matter host status, IA32_ARCH_CAPABILITIES.RSBA
> > [bit 2]
> > + * is always supported in guest.
> > + */
> > + if (wi->msr.index == MSR_IA32_ARCH_CAPABILITIES) {
> > + r = MSR_ARCH_CAP_RSBA;
> > + }
> > + r |= kvm_arch_get_supported_msr_feature(kvm_state,
> > + wi->msr.index);
> > break;
> > }
> > } else if (hvf_enabled()) {
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index 730c06f..52a52ec 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -502,6 +502,7 @@ typedef enum FeatureWord {
> > 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 */
> > + FEAT_ARCH_CAPABILITIES,
> > FEATURE_WORDS,
> > } FeatureWord;
> >
> > @@ -730,6 +731,13 @@ typedef uint32_t
> > FeatureWordArray[FEATURE_WORDS];
> > #define CPUID_TOPOLOGY_LEVEL_SMT (1U << 8)
> > #define CPUID_TOPOLOGY_LEVEL_CORE (2U << 8)
> >
> > +/* MSR Feature Bits */
> > +#define MSR_ARCH_CAP_RDCL_NO (1U << 0)
> > +#define MSR_ARCH_CAP_IBRS_ALL (1U << 1)
> > +#define MSR_ARCH_CAP_RSBA (1U << 2)
> > +#define MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY (1U << 3)
> > +#define MSR_ARCH_CAP_SSB_NO (1U << 4)
> > +
> > #ifndef HYPERV_SPINLOCK_NEVER_RETRY
> > #define HYPERV_SPINLOCK_NEVER_RETRY 0xFFFFFFFF
> > #endif
> > diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> > index db79dad..2f7b40d 100644
> > --- a/target/i386/kvm.c
> > +++ b/target/i386/kvm.c
> > @@ -1928,6 +1928,17 @@ static int kvm_put_msrs(X86CPU *cpu, int
> > level)
> > }
> > #endif
> >
> > + /* If host supports feature MSR, write down. */
> > + if (kvm_feature_msrs) {
> > + int i;
> > + for (i = 0; i < kvm_feature_msrs->nmsrs; i++)
> > + if (kvm_feature_msrs->indices[i] ==
> > MSR_IA32_ARCH_CAPABILITIES) {
> > + kvm_msr_entry_add(cpu, MSR_IA32_ARCH_CAPABILITIES,
> > + env-
> > >features[FEAT_ARCH_CAPABILITIES]);
> > + break;
> > + }
> > + }
> > +
> > /*
> > * The following MSRs have side effects on the guest or are
> > too heavy
> > * for normal writeback. Limit them to reset or full state
> > updates.
> > --
> > 1.8.3.1
> >
> >
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v5 2/3] x86: Data structure changes to support MSR based features
2018-10-25 3:06 ` Robert Hoo
@ 2018-10-25 13:36 ` Eduardo Habkost
0 siblings, 0 replies; 13+ messages in thread
From: Eduardo Habkost @ 2018-10-25 13:36 UTC (permalink / raw)
To: Robert Hoo; +Cc: robert.hu, pbonzini, rth, thomas.lendacky, qemu-devel
On Thu, Oct 25, 2018 at 11:06:59AM +0800, Robert Hoo wrote:
> On Wed, 2018-10-24 at 07:16 -0300, Eduardo Habkost wrote:
[...]
> > > + struct { /*CPUID that enumerate this MSR*/
> > > + FeatureWord cpuid_class;
> > > + uint32_t cpuid_flag;
> > > + } cpuid_dep;
> >
> > Aren't you going to use this field anywhere? Probably we want to
> > prevent the VM from starting if a bit is set in the feature world
> > but the cpuid_dep bit is not set.
> >
> > e.g.:
> > qemu-system-x86_64 -cpu Skylake-Client,-arch-capabilities,+rsba
> > probably should fail to start.
>
> How about in x86_cpu_filter_features() filters the MSR feature word, if
> its dependent CPUID feature bit is not set?
> The filter results will be record in cpu->filtered_features, and print
> out, like other filtered features.
Sounds good to me.
--
Eduardo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/3] x86: define a new MSR based feature word -- FEATURE_WORDS_ARCH_CAPABILITIES
2018-10-24 10:06 ` Eduardo Habkost
2018-10-25 3:16 ` Robert Hoo
@ 2018-10-26 3:01 ` Robert Hoo
2018-10-26 8:38 ` Eduardo Habkost
1 sibling, 1 reply; 13+ messages in thread
From: Robert Hoo @ 2018-10-26 3:01 UTC (permalink / raw)
To: Eduardo Habkost
Cc: robert.hu, robert.hu, pbonzini, rth, thomas.lendacky, qemu-devel
On Wed, 2018-10-24 at 07:06 -0300, Eduardo Habkost wrote:
> On Mon, Oct 15, 2018 at 12:47:25PM +0800, Robert Hoo wrote:
> > Note RSBA is specially treated -- no matter host support it or not,
> > qemu
> > pretends it is supported.
> >
> > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
>
> I am now wondering what else we need to be able to remove
> CPUID_7_0_EDX_ARCH_CAPABILITIES from
> feature_word_info[FEAT_7_0_EDX].unmigratable_flags.
>
> This series is necessary for that, be I think we still can't let
> the VM be migrated if arch-capabilities is enabled and we're
> running on a host that doesn't have MSR_IA32_ARCH_CAPABILITIES on
> kvm_feature_msrs.
>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>
> > ---
> > target/i386/cpu.c | 31 ++++++++++++++++++++++++++++++-
> > target/i386/cpu.h | 8 ++++++++
> > target/i386/kvm.c | 11 +++++++++++
> > 3 files changed, 49 insertions(+), 1 deletion(-)
> >
[...]
> >
> > typedef struct X86RegisterInfo32 {
> > @@ -3696,7 +3717,15 @@ static uint32_t
> > x86_cpu_get_supported_feature_word(FeatureWord w,
> > wi-
> > >cpuid.reg);
> > break;
> > case MSR_FEATURE_WORD:
> > - r = kvm_arch_get_supported_msr_feature(kvm_state, wi-
> > >msr.index);
> > + /* Special case:
> > + * No matter host status, IA32_ARCH_CAPABILITIES.RSBA
> > [bit 2]
> > + * is always supported in guest.
> > + */
> > + if (wi->msr.index == MSR_IA32_ARCH_CAPABILITIES) {
> > + r = MSR_ARCH_CAP_RSBA;
> > + }
> > + r |= kvm_arch_get_supported_msr_feature(kvm_state,
> > + wi->msr.index);
> > break;
After I add the filtering out MSR feature, whose CPUID dependency fails
, in x86_cpu_filter_features(), 1 issue comes out here:
If running on an old platform that doesn't have ARCH_CAPABILITIES MSR,
but we still pretends it here, then qemu will always print out
"warning: host doesn't support requested feature: MSR(10AH).rsba [bit
2]", with -cpu 'host', which does not look comfortable.
How about remove this hunk for now? leave it to when we fully decide
how to handle ARCH_CAPABILITIES live-migration safely.
> > }
> > } else if (hvf_enabled()) {
[...]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v5 3/3] x86: define a new MSR based feature word -- FEATURE_WORDS_ARCH_CAPABILITIES
2018-10-26 3:01 ` Robert Hoo
@ 2018-10-26 8:38 ` Eduardo Habkost
0 siblings, 0 replies; 13+ messages in thread
From: Eduardo Habkost @ 2018-10-26 8:38 UTC (permalink / raw)
To: Robert Hoo; +Cc: robert.hu, pbonzini, rth, thomas.lendacky, qemu-devel
On Fri, Oct 26, 2018 at 11:01:25AM +0800, Robert Hoo wrote:
> On Wed, 2018-10-24 at 07:06 -0300, Eduardo Habkost wrote:
> > On Mon, Oct 15, 2018 at 12:47:25PM +0800, Robert Hoo wrote:
> > > Note RSBA is specially treated -- no matter host support it or not,
> > > qemu
> > > pretends it is supported.
> > >
> > > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
> >
> > I am now wondering what else we need to be able to remove
> > CPUID_7_0_EDX_ARCH_CAPABILITIES from
> > feature_word_info[FEAT_7_0_EDX].unmigratable_flags.
> >
> > This series is necessary for that, be I think we still can't let
> > the VM be migrated if arch-capabilities is enabled and we're
> > running on a host that doesn't have MSR_IA32_ARCH_CAPABILITIES on
> > kvm_feature_msrs.
> >
> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> >
> > > ---
> > > target/i386/cpu.c | 31 ++++++++++++++++++++++++++++++-
> > > target/i386/cpu.h | 8 ++++++++
> > > target/i386/kvm.c | 11 +++++++++++
> > > 3 files changed, 49 insertions(+), 1 deletion(-)
> > >
> [...]
> > >
> > > typedef struct X86RegisterInfo32 {
> > > @@ -3696,7 +3717,15 @@ static uint32_t
> > > x86_cpu_get_supported_feature_word(FeatureWord w,
> > > wi-
> > > >cpuid.reg);
> > > break;
> > > case MSR_FEATURE_WORD:
> > > - r = kvm_arch_get_supported_msr_feature(kvm_state, wi-
> > > >msr.index);
> > > + /* Special case:
> > > + * No matter host status, IA32_ARCH_CAPABILITIES.RSBA
> > > [bit 2]
> > > + * is always supported in guest.
> > > + */
> > > + if (wi->msr.index == MSR_IA32_ARCH_CAPABILITIES) {
> > > + r = MSR_ARCH_CAP_RSBA;
> > > + }
> > > + r |= kvm_arch_get_supported_msr_feature(kvm_state,
> > > + wi->msr.index);
> > > break;
> After I add the filtering out MSR feature, whose CPUID dependency fails
> , in x86_cpu_filter_features(), 1 issue comes out here:
>
> If running on an old platform that doesn't have ARCH_CAPABILITIES MSR,
> but we still pretends it here, then qemu will always print out
> "warning: host doesn't support requested feature: MSR(10AH).rsba [bit
> 2]", with -cpu 'host', which does not look comfortable.
> How about remove this hunk for now? leave it to when we fully decide
> how to handle ARCH_CAPABILITIES live-migration safely.
I will remove that hunk in x86-next, thanks for noting!
--
Eduardo
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-10-26 8:38 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-15 4:47 [Qemu-devel] [PATCH v5 0/3] x86: QEMU side support on MSR based features Robert Hoo
2018-10-15 4:47 ` [Qemu-devel] [PATCH v5 1/3] kvm: Add support to KVM_GET_MSR_FEATURE_INDEX_LIST and KVM_GET_MSRS system ioctl Robert Hoo
2018-10-24 9:49 ` Eduardo Habkost
2018-10-15 4:47 ` [Qemu-devel] [PATCH v5 2/3] x86: Data structure changes to support MSR based features Robert Hoo
2018-10-24 9:56 ` Eduardo Habkost
2018-10-24 10:16 ` Eduardo Habkost
2018-10-25 3:06 ` Robert Hoo
2018-10-25 13:36 ` Eduardo Habkost
2018-10-15 4:47 ` [Qemu-devel] [PATCH v5 3/3] x86: define a new MSR based feature word -- FEATURE_WORDS_ARCH_CAPABILITIES Robert Hoo
2018-10-24 10:06 ` Eduardo Habkost
2018-10-25 3:16 ` Robert Hoo
2018-10-26 3:01 ` Robert Hoo
2018-10-26 8:38 ` Eduardo Habkost
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.