* [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
* 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
* [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
* 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 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 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
* [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 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 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 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.