2020-03-03 00:03:51

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH v2 00/66] KVM: x86: Introduce KVM cpu caps

Introduce what is effectively a KVM-specific copy of the x86_capabilities
array in boot_cpu_data, kvm_cpu_caps. kvm_cpu_caps is initialized by
copying boot_cpu_data.x86_capabilities before ->hardware_setup(). It is
then updated by KVM's CPUID logic (both common x86 and VMX/SVM specific)
to adjust the caps to reflect the CPU that KVM will expose to the guest.

Quick synopsis:
1. Refactor the KVM_GET_SUPPORTED_CPUID stack to consolidate code,
remove crustiness, and set the stage for introducing kvm_cpu_caps.

2. Introduce cpuid_entry_*() accessors/mutators to automatically
handle retrieving the correct reg from a CPUID entry, and to audit
that the entry matches the reserve CPUID lookup entry. The
cpuid_entry_*() helpers make moving the code from common x86 to
vendor code much less risky.

3. Move CPUID adjustments to vendor code in preparation for kvm_cpu_caps,
which will be initialized at load time before the kvm_x86_ops hooks
are ready to be used, i.e. before ->hardware_setup().

4. Introduce kvm_cpu_caps and move all the CPUID code over to kvm_cpu_caps.

5. Use kvm_cpu_cap_has() to kill off a bunch of ->*_supported() hooks.

6. Additional cleanup in tangentially related areas to kill off even more
->*_supported() hooks, including ->set_supported_cpuid().

Tested by verifying the output of KVM_GET_SUPPORTED_CPUID is identical
before and after on every patch on a Haswell and Coffee Lake, and for the
"before vs. after" output on Ice Lake.

Verified correctness when hiding features via Qemu (running this version
of KVM in L1), e.g. that UMIP is correctly emulated for L2 when it's
hidden from L1, on relevant patches.

Boot tested and ran kvm-unit-tests at key points, e.g. large page
handling.

All AMD patches are build-tested only.

v2:
- Opportunistically remove bare "unsigned" usgae. [Vitaly]
- Remove CPUID auditing (Vitaly and Paolo suggested making it
unconditional, then I realized it would trigger false positives).
- Fix a bug in the series that broke SVM features enumeration.
- Only advertise SVM features when nested SVM is enabled. [Paolo]
- Fully remove support for stateful CPUID.0x2. [Vitaly, Paolo]
- Call out in patch 01's changelog that it technically breaks the
ABI, but that no known VMM is affected. [Vitaly, Paolo]
- Use @function instead of hardcoding "2" for thes stateful code (which
eventually gets tossed anyways). [Vitaly]
- Move 0x8000000A into common code and kill ->set_supported_cpuid().
[Vitaly]
- Call out the subtle emulation handling in ->set_supported_cpuid(),
which also gets tossed :-). [Vitaly]
- Fix the BUILG_BUG_ON() in patch 38. [Vitaly]
- Use !! to explicitly cast a u32 to a bool. [Vitaly, Paolo]
- Sort kvm_cpu_cap_mask() calls by leaf number, ascending. [Vitaly]
- Collect reviews. [Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly,
Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly,
Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly,
Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly,
Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly,
Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly,
Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly,
Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Xiaoyao, Xiaoyao, Xiaoyao]

Sean Christopherson (66):
KVM: x86: Return -E2BIG when KVM_GET_SUPPORTED_CPUID hits max entries
KVM: x86: Refactor loop around do_cpuid_func() to separate helper
KVM: x86: Simplify handling of Centaur CPUID leafs
KVM: x86: Clean up error handling in kvm_dev_ioctl_get_cpuid()
KVM: x86: Check userapce CPUID array size after validating sub-leaf
KVM: x86: Move CPUID 0xD.1 handling out of the index>0 loop
KVM: x86: Check for CPUID 0xD.N support before validating array size
KVM: x86: Warn on zero-size save state for valid CPUID 0xD.N sub-leaf
KVM: x86: Refactor CPUID 0xD.N sub-leaf entry creation
KVM: x86: Clean up CPUID 0x7 sub-leaf loop
KVM: x86: Drop the explicit @index from do_cpuid_7_mask()
KVM: x86: Drop redundant boot cpu checks on SSBD feature bits
KVM: x86: Consolidate CPUID array max num entries checking
KVM: x86: Hoist loop counter and terminator to top of
__do_cpuid_func()
KVM: x86: Refactor CPUID 0x4 and 0x8000001d handling
KVM: x86: Encapsulate CPUID entries and metadata in struct
KVM: x86: Drop redundant array size check
KVM: x86: Use common loop iterator when handling CPUID 0xD.N
KVM: VMX: Add helpers to query Intel PT mode
KVM: x86: Calculate the supported xcr0 mask at load time
KVM: x86: Use supported_xcr0 to detect MPX support
KVM: x86: Make kvm_mpx_supported() an inline function
KVM: x86: Clear output regs for CPUID 0x14 if PT isn't exposed to
guest
KVM: x86: Drop explicit @func param from ->set_supported_cpuid()
KVM: x86: Use u32 for holding CPUID register value in helpers
KVM: x86: Replace bare "unsigned" with "unsigned int" in cpuid helpers
KVM: x86: Introduce cpuid_entry_{get,has}() accessors
KVM: x86: Introduce cpuid_entry_{change,set,clear}() mutators
KVM: x86: Refactor cpuid_mask() to auto-retrieve the register
KVM: x86: Handle MPX CPUID adjustment in VMX code
KVM: x86: Handle INVPCID CPUID adjustment in VMX code
KVM: x86: Handle UMIP emulation CPUID adjustment in VMX code
KVM: x86: Handle PKU CPUID adjustment in VMX code
KVM: x86: Handle RDTSCP CPUID adjustment in VMX code
KVM: x86: Handle Intel PT CPUID adjustment in VMX code
KVM: x86: Handle GBPAGE CPUID adjustment for EPT in VMX code
KVM: x86: Refactor handling of XSAVES CPUID adjustment
KVM: x86: Introduce kvm_cpu_caps to replace runtime CPUID masking
KVM: SVM: Convert feature updates from CPUID to KVM cpu caps
KVM: VMX: Convert feature updates from CPUID to KVM cpu caps
KVM: x86: Move XSAVES CPUID adjust to VMX's KVM cpu cap update
KVM: x86: Add a helper to check kernel support when setting cpu cap
KVM: x86: Use KVM cpu caps to mark CR4.LA57 as not-reserved
KVM: x86: Use KVM cpu caps to track UMIP emulation
KVM: x86: Fold CPUID 0x7 masking back into __do_cpuid_func()
KVM: x86: Remove the unnecessary loop on CPUID 0x7 sub-leafs
KVM: x86: Squash CPUID 0x2.0 insanity for modern CPUs
KVM: x86: Remove stateful CPUID handling
KVM: x86: Do host CPUID at load time to mask KVM cpu caps
KVM: x86: Override host CPUID results with kvm_cpu_caps
KVM: x86: Set emulated/transmuted feature bits via kvm_cpu_caps
KVM: x86: Use kvm_cpu_caps to detect Intel PT support
KVM: x86: Do kvm_cpuid_array capacity checks in terminal functions
KVM: x86: Use KVM cpu caps to detect MSR_TSC_AUX virt support
KVM: VMX: Directly use VMX capabilities helper to detect RDTSCP
support
KVM: x86: Check for Intel PT MSR virtualization using KVM cpu caps
KVM: VMX: Directly query Intel PT mode when refreshing PMUs
KVM: SVM: Refactor logging of NPT enabled/disabled
KVM: x86/mmu: Merge kvm_{enable,disable}_tdp() into a common function
KVM: x86/mmu: Configure max page level during hardware setup
KVM: x86: Don't propagate MMU lpage support to memslot.disallow_lpage
KVM: Drop largepages_enabled and its accessor/mutator
KVM: x86: Move VMX's host_efer to common x86 code
KVM: nSVM: Expose SVM features to L1 iff nested is enabled
KVM: nSVM: Advertise and enable NRIPS for L1 iff nrips is enabled
KVM: x86: Move nSVM CPUID 0x8000000A handing into common x86 code

Documentation/virt/kvm/api.rst | 22 +-
arch/x86/include/asm/kvm_host.h | 15 +-
arch/x86/kvm/cpuid.c | 874 +++++++++++++++-----------------
arch/x86/kvm/cpuid.h | 134 ++++-
arch/x86/kvm/mmu/mmu.c | 29 +-
arch/x86/kvm/svm.c | 130 ++---
arch/x86/kvm/vmx/capabilities.h | 25 +-
arch/x86/kvm/vmx/nested.c | 2 +-
arch/x86/kvm/vmx/pmu_intel.c | 2 +-
arch/x86/kvm/vmx/vmx.c | 121 +++--
arch/x86/kvm/vmx/vmx.h | 5 +-
arch/x86/kvm/x86.c | 48 +-
arch/x86/kvm/x86.h | 10 +-
include/linux/kvm_host.h | 2 -
virt/kvm/kvm_main.c | 13 -
15 files changed, 695 insertions(+), 737 deletions(-)

--
2.24.1


2020-03-03 00:04:03

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH v2 01/66] KVM: x86: Return -E2BIG when KVM_GET_SUPPORTED_CPUID hits max entries

Fix a long-standing bug that causes KVM to return 0 instead of -E2BIG
when userspace's array is insufficiently sized.

This technically breaks backwards compatibility, e.g. a userspace with a
hardcoded cpuid->nent could theoretically be broken as it would see an
error instead of success if cpuid->nent is less than the number of
entries required to fully enumerate the host CPU. But, the lowest known
cpuid->nent hardcoded by a VMM is 100 (lkvm and selftests), and the
largest realistic limit on Intel and AMD is well under a 100. E.g.
Intel's Icelake server with all the bells and whistles tops out at ~60
entries (variable due to SGX sub-leafs), and AMD's CPUID documentation
allows for less than 50 (KVM hard caps CPUID 0xD at a single sub-leaf).

Note, while the Fixes: tag is accurate with respect to the immediate
bug, it's likely that similar bugs in KVM_GET_SUPPORTED_CPUID existed
prior to the refactoring, e.g. Qemu contains a workaround for the broken
KVM_GET_SUPPORTED_CPUID behavior that predates the buggy commit by over
two years. The Qemu workaround is also likely the main reason the bug
has gone unreported for so long.

Qemu hack:
commit 76ae317f7c16aec6b469604b1764094870a75470
Author: Mark McLoughlin <[email protected]>
Date: Tue May 19 18:55:21 2009 +0100

kvm: work around supported cpuid ioctl() brokenness

KVM_GET_SUPPORTED_CPUID has been known to fail to return -E2BIG
when it runs out of entries. Detect this by always trying again
with a bigger table if the ioctl() fills the table.

Fixes: 831bf664e9c1f ("KVM: Refactor and simplify kvm_dev_ioctl_get_supported_cpuid")
Reviewed-by: Vitaly Kuznetsov <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/cpuid.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index b1c469446b07..47ce04762c20 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -908,9 +908,14 @@ int kvm_dev_ioctl_get_cpuid(struct kvm_cpuid2 *cpuid,
goto out_free;

limit = cpuid_entries[nent - 1].eax;
- for (func = ent->func + 1; func <= limit && nent < cpuid->nent && r == 0; ++func)
+ for (func = ent->func + 1; func <= limit && r == 0; ++func) {
+ if (nent >= cpuid->nent) {
+ r = -E2BIG;
+ goto out_free;
+ }
r = do_cpuid_func(&cpuid_entries[nent], func,
&nent, cpuid->nent, type);
+ }

if (r)
goto out_free;
--
2.24.1

2020-03-03 00:04:06

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH v2 18/66] KVM: x86: Use common loop iterator when handling CPUID 0xD.N

Use __do_cpuid_func()'s common loop iterator, "i", when enumerating the
sub-leafs for CPUID 0xD now that the CPUID 0xD loop doesn't need to
manual maintain separate counts for the entries index and CPUID index.

No functional changed intended.

Reviewed-by: Vitaly Kuznetsov <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/cpuid.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 4bf4f7d7741e..85f292088d91 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -634,7 +634,6 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
}
break;
case 0xd: {
- int idx;
u64 supported = kvm_supported_xcr0();

entry->eax &= supported;
@@ -658,11 +657,11 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
entry->ecx = 0;
entry->edx = 0;

- for (idx = 2; idx < 64; ++idx) {
- if (!(supported & BIT_ULL(idx)))
+ for (i = 2; i < 64; ++i) {
+ if (!(supported & BIT_ULL(i)))
continue;

- entry = do_host_cpuid(array, function, idx);
+ entry = do_host_cpuid(array, function, i);
if (!entry)
goto out;

--
2.24.1

2020-03-03 00:04:24

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH v2 21/66] KVM: x86: Use supported_xcr0 to detect MPX support

Query supported_xcr0 when checking for MPX support instead of invoking
->mpx_supported() and drop ->mpx_supported() as kvm_mpx_supported() was
its last user. Rename vmx_mpx_supported() to cpu_has_vmx_mpx() to
better align with VMX/VMCS nomenclature.

Modify VMX's adjustment of xcr0 to call cpus_has_vmx_mpx() (renamed from
vmx_mpx_supported()) directly to avoid reading supported_xcr0 before
it's fully configured.

No functional change intended.

Reviewed-by: Xiaoyao Li <[email protected]>
Reviewed-by: Vitaly Kuznetsov <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/include/asm/kvm_host.h | 2 +-
arch/x86/kvm/cpuid.c | 3 +--
arch/x86/kvm/svm.c | 6 ------
arch/x86/kvm/vmx/capabilities.h | 2 +-
arch/x86/kvm/vmx/vmx.c | 3 +--
5 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 5edf6425c747..9a4ae6ef0d7a 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1169,7 +1169,7 @@ struct kvm_x86_ops {
enum x86_intercept_stage stage);
void (*handle_exit_irqoff)(struct kvm_vcpu *vcpu,
enum exit_fastpath_completion *exit_fastpath);
- bool (*mpx_supported)(void);
+
bool (*xsaves_supported)(void);
bool (*umip_emulated)(void);
bool (*pt_supported)(void);
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 1eb775c33c4e..54af2c19388b 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -47,8 +47,7 @@ static u32 xstate_required_size(u64 xstate_bv, bool compacted)

bool kvm_mpx_supported(void)
{
- return ((host_xcr0 & (XFEATURE_MASK_BNDREGS | XFEATURE_MASK_BNDCSR))
- && kvm_x86_ops->mpx_supported());
+ return supported_xcr0 & (XFEATURE_MASK_BNDREGS | XFEATURE_MASK_BNDCSR);
}
EXPORT_SYMBOL_GPL(kvm_mpx_supported);

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 51db8addda04..16c4b7eb6312 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -6079,11 +6079,6 @@ static bool svm_invpcid_supported(void)
return false;
}

-static bool svm_mpx_supported(void)
-{
- return false;
-}
-
static bool svm_xsaves_supported(void)
{
return boot_cpu_has(X86_FEATURE_XSAVES);
@@ -7465,7 +7460,6 @@ static struct kvm_x86_ops svm_x86_ops __ro_after_init = {

.rdtscp_supported = svm_rdtscp_supported,
.invpcid_supported = svm_invpcid_supported,
- .mpx_supported = svm_mpx_supported,
.xsaves_supported = svm_xsaves_supported,
.umip_emulated = svm_umip_emulated,
.pt_supported = svm_pt_supported,
diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h
index 80eec8cffbe2..c00e26570198 100644
--- a/arch/x86/kvm/vmx/capabilities.h
+++ b/arch/x86/kvm/vmx/capabilities.h
@@ -101,7 +101,7 @@ static inline bool cpu_has_load_perf_global_ctrl(void)
(vmcs_config.vmexit_ctrl & VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL);
}

-static inline bool vmx_mpx_supported(void)
+static inline bool cpu_has_vmx_mpx(void)
{
return (vmcs_config.vmexit_ctrl & VM_EXIT_CLEAR_BNDCFGS) &&
(vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_BNDCFGS);
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index cf874c364c8f..17dc4dc2a7f9 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7655,7 +7655,7 @@ static __init int hardware_setup(void)
WARN_ONCE(host_bndcfgs, "KVM: BNDCFGS in host will be lost");
}

- if (!kvm_mpx_supported())
+ if (!cpu_has_vmx_mpx())
supported_xcr0 &= ~(XFEATURE_MASK_BNDREGS |
XFEATURE_MASK_BNDCSR);

@@ -7922,7 +7922,6 @@ static struct kvm_x86_ops vmx_x86_ops __ro_after_init = {

.check_intercept = vmx_check_intercept,
.handle_exit_irqoff = vmx_handle_exit_irqoff,
- .mpx_supported = vmx_mpx_supported,
.xsaves_supported = vmx_xsaves_supported,
.umip_emulated = vmx_umip_emulated,
.pt_supported = vmx_pt_supported,
--
2.24.1

2020-03-03 00:04:43

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH v2 03/66] KVM: x86: Simplify handling of Centaur CPUID leafs

Refactor the handling of the Centaur-only CPUID leaf to detect the leaf
via a runtime query instead of adding a one-off callback in the static
array. When the callback was introduced, there were additional fields
in the array's structs, and more importantly, retpoline wasn't a thing.

No functional change intended.

Reviewed-by: Vitaly Kuznetsov <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/cpuid.c | 32 ++++++++++----------------------
1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index f49fdd06f511..de52cbb46171 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -829,15 +829,7 @@ static int do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 func,
return __do_cpuid_func(entry, func, nent, maxnent);
}

-struct kvm_cpuid_param {
- u32 func;
- bool (*qualifier)(const struct kvm_cpuid_param *param);
-};
-
-static bool is_centaur_cpu(const struct kvm_cpuid_param *param)
-{
- return boot_cpu_data.x86_vendor == X86_VENDOR_CENTAUR;
-}
+#define CENTAUR_CPUID_SIGNATURE 0xC0000000

static int get_cpuid_func(struct kvm_cpuid_entry2 *entries, u32 func,
int *nent, int maxnent, unsigned int type)
@@ -845,6 +837,10 @@ static int get_cpuid_func(struct kvm_cpuid_entry2 *entries, u32 func,
u32 limit;
int r;

+ if (func == CENTAUR_CPUID_SIGNATURE &&
+ boot_cpu_data.x86_vendor != X86_VENDOR_CENTAUR)
+ return 0;
+
r = do_cpuid_func(&entries[*nent], func, nent, maxnent, type);
if (r)
return r;
@@ -896,11 +892,8 @@ int kvm_dev_ioctl_get_cpuid(struct kvm_cpuid2 *cpuid,
struct kvm_cpuid_entry2 *cpuid_entries;
int nent = 0, r = -E2BIG, i;

- static const struct kvm_cpuid_param param[] = {
- { .func = 0 },
- { .func = 0x80000000 },
- { .func = 0xC0000000, .qualifier = is_centaur_cpu },
- { .func = KVM_CPUID_SIGNATURE },
+ static const u32 funcs[] = {
+ 0, 0x80000000, CENTAUR_CPUID_SIGNATURE, KVM_CPUID_SIGNATURE,
};

if (cpuid->nent < 1)
@@ -918,14 +911,9 @@ int kvm_dev_ioctl_get_cpuid(struct kvm_cpuid2 *cpuid,
goto out;

r = 0;
- for (i = 0; i < ARRAY_SIZE(param); i++) {
- const struct kvm_cpuid_param *ent = &param[i];
-
- if (ent->qualifier && !ent->qualifier(ent))
- continue;
-
- r = get_cpuid_func(cpuid_entries, ent->func, &nent,
- cpuid->nent, type);
+ for (i = 0; i < ARRAY_SIZE(funcs); i++) {
+ r = get_cpuid_func(cpuid_entries, funcs[i], &nent, cpuid->nent,
+ type);
if (r)
goto out_free;
}
--
2.24.1

2020-03-03 00:04:50

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH v2 30/66] KVM: x86: Handle MPX CPUID adjustment in VMX code

Move the MPX CPUID adjustments into VMX to eliminate an instance of the
undesirable "unsigned f_* = *_supported ? F(*) : 0" pattern in the
common CPUID handling code.

Note, to maintain existing behavior, VMX must manually check for kernel
support for MPX by querying boot_cpu_has(X86_FEATURE_MPX). Previously,
do_cpuid_7_mask() masked MPX based on boot_cpu_data by invoking
cpuid_mask() on the associated cpufeatures word, but cpuid_mask() runs
prior to executing vmx_set_supported_cpuid().

No functional change intended.

Reviewed-by: Vitaly Kuznetsov <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/cpuid.c | 3 +--
arch/x86/kvm/vmx/vmx.c | 14 ++++++++++++--
2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 04343c54a419..43f76b36f461 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -340,7 +340,6 @@ static int __do_cpuid_func_emulated(struct kvm_cpuid_array *array, u32 func)
static inline void do_cpuid_7_mask(struct kvm_cpuid_entry2 *entry)
{
unsigned f_invpcid = kvm_x86_ops->invpcid_supported() ? F(INVPCID) : 0;
- unsigned f_mpx = kvm_mpx_supported() ? F(MPX) : 0;
unsigned f_umip = kvm_x86_ops->umip_emulated() ? F(UMIP) : 0;
unsigned f_intel_pt = kvm_x86_ops->pt_supported() ? F(INTEL_PT) : 0;
unsigned f_la57;
@@ -349,7 +348,7 @@ static inline void do_cpuid_7_mask(struct kvm_cpuid_entry2 *entry)
/* cpuid 7.0.ebx */
const u32 kvm_cpuid_7_0_ebx_x86_features =
F(FSGSBASE) | F(BMI1) | F(HLE) | F(AVX2) | F(SMEP) |
- F(BMI2) | F(ERMS) | f_invpcid | F(RTM) | f_mpx | F(RDSEED) |
+ F(BMI2) | F(ERMS) | f_invpcid | F(RTM) | 0 /*MPX*/ | F(RDSEED) |
F(ADX) | F(SMAP) | F(AVX512IFMA) | F(AVX512F) | F(AVX512PF) |
F(AVX512ER) | F(AVX512CD) | F(CLFLUSHOPT) | F(CLWB) | F(AVX512DQ) |
F(SHA_NI) | F(AVX512BW) | F(AVX512VL) | f_intel_pt;
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 44724e8d0b88..ef3a63ce8a6a 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7126,8 +7126,18 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)

static void vmx_set_supported_cpuid(struct kvm_cpuid_entry2 *entry)
{
- if (entry->function == 1 && nested)
- entry->ecx |= feature_bit(VMX);
+ switch (entry->function) {
+ case 0x1:
+ if (nested)
+ cpuid_entry_set(entry, X86_FEATURE_VMX);
+ break;
+ case 0x7:
+ if (boot_cpu_has(X86_FEATURE_MPX) && kvm_mpx_supported())
+ cpuid_entry_set(entry, X86_FEATURE_MPX);
+ break;
+ default:
+ break;
+ }
}

static void vmx_request_immediate_exit(struct kvm_vcpu *vcpu)
--
2.24.1

2020-03-03 00:05:01

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH v2 02/66] KVM: x86: Refactor loop around do_cpuid_func() to separate helper

Move the guts of kvm_dev_ioctl_get_cpuid()'s CPUID func loop to a
separate helper to improve code readability and pave the way for future
cleanup.

No functional change intended.

Reviewed-by: Vitaly Kuznetsov <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/cpuid.c | 45 ++++++++++++++++++++++++++------------------
1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 47ce04762c20..f49fdd06f511 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -839,6 +839,29 @@ static bool is_centaur_cpu(const struct kvm_cpuid_param *param)
return boot_cpu_data.x86_vendor == X86_VENDOR_CENTAUR;
}

+static int get_cpuid_func(struct kvm_cpuid_entry2 *entries, u32 func,
+ int *nent, int maxnent, unsigned int type)
+{
+ u32 limit;
+ int r;
+
+ r = do_cpuid_func(&entries[*nent], func, nent, maxnent, type);
+ if (r)
+ return r;
+
+ limit = entries[*nent - 1].eax;
+ for (func = func + 1; func <= limit; ++func) {
+ if (*nent >= maxnent)
+ return -E2BIG;
+
+ r = do_cpuid_func(&entries[*nent], func, nent, maxnent, type);
+ if (r)
+ break;
+ }
+
+ return r;
+}
+
static bool sanity_check_entries(struct kvm_cpuid_entry2 __user *entries,
__u32 num_entries, unsigned int ioctl_type)
{
@@ -871,8 +894,8 @@ int kvm_dev_ioctl_get_cpuid(struct kvm_cpuid2 *cpuid,
unsigned int type)
{
struct kvm_cpuid_entry2 *cpuid_entries;
- int limit, nent = 0, r = -E2BIG, i;
- u32 func;
+ int nent = 0, r = -E2BIG, i;
+
static const struct kvm_cpuid_param param[] = {
{ .func = 0 },
{ .func = 0x80000000 },
@@ -901,22 +924,8 @@ int kvm_dev_ioctl_get_cpuid(struct kvm_cpuid2 *cpuid,
if (ent->qualifier && !ent->qualifier(ent))
continue;

- r = do_cpuid_func(&cpuid_entries[nent], ent->func,
- &nent, cpuid->nent, type);
-
- if (r)
- goto out_free;
-
- limit = cpuid_entries[nent - 1].eax;
- for (func = ent->func + 1; func <= limit && r == 0; ++func) {
- if (nent >= cpuid->nent) {
- r = -E2BIG;
- goto out_free;
- }
- r = do_cpuid_func(&cpuid_entries[nent], func,
- &nent, cpuid->nent, type);
- }
-
+ r = get_cpuid_func(cpuid_entries, ent->func, &nent,
+ cpuid->nent, type);
if (r)
goto out_free;
}
--
2.24.1

2020-03-03 14:17:10

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH v2 01/66] KVM: x86: Return -E2BIG when KVM_GET_SUPPORTED_CPUID hits max entries

On 03/03/20 00:56, Sean Christopherson wrote:
> (KVM hard caps CPUID 0xD at a single sub-leaf).

Hmm... no it doesn't?

for (idx = 1, i = 1; idx < 64; ++idx) {
u64 mask = ((u64)1 << idx);
if (*nent >= maxnent)
goto out;

do_host_cpuid(&entry[i], function, idx);
if (idx == 1) {
entry[i].eax &= kvm_cpuid_D_1_eax_x86_features;
cpuid_mask(&entry[i].eax, CPUID_D_1_EAX);
entry[i].ebx = 0;
if (entry[i].eax & (F(XSAVES)|F(XSAVEC)))
entry[i].ebx =
xstate_required_size(supported,
true);
} else {
if (entry[i].eax == 0 || !(supported & mask))
continue;
if (WARN_ON_ONCE(entry[i].ecx & 1))
continue;
}
entry[i].ecx = 0;
entry[i].edx = 0;
++*nent;
++i;
}

I still think the patch is correct, what matters is that no KVM in
existence supports enough processor features to reach 100 or so subleaves.

Paolo

2020-03-03 14:35:44

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH v2 21/66] KVM: x86: Use supported_xcr0 to detect MPX support

On 03/03/20 00:56, Sean Christopherson wrote:
>
> bool kvm_mpx_supported(void)
> {
> - return ((host_xcr0 & (XFEATURE_MASK_BNDREGS | XFEATURE_MASK_BNDCSR))
> - && kvm_x86_ops->mpx_supported());
> + return supported_xcr0 & (XFEATURE_MASK_BNDREGS | XFEATURE_MASK_BNDCSR);
> }

Better check that both bits are set.

Paolo

2020-03-03 15:48:30

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH v2 01/66] KVM: x86: Return -E2BIG when KVM_GET_SUPPORTED_CPUID hits max entries

On Tue, Mar 03, 2020 at 03:16:16PM +0100, Paolo Bonzini wrote:
> On 03/03/20 00:56, Sean Christopherson wrote:
> > (KVM hard caps CPUID 0xD at a single sub-leaf).
>
> Hmm... no it doesn't?
>
> for (idx = 1, i = 1; idx < 64; ++idx) {
> u64 mask = ((u64)1 << idx);
> if (*nent >= maxnent)
> goto out;
>
> do_host_cpuid(&entry[i], function, idx);
> if (idx == 1) {
> entry[i].eax &= kvm_cpuid_D_1_eax_x86_features;
> cpuid_mask(&entry[i].eax, CPUID_D_1_EAX);
> entry[i].ebx = 0;
> if (entry[i].eax & (F(XSAVES)|F(XSAVEC)))
> entry[i].ebx =
> xstate_required_size(supported,
> true);
> } else {
> if (entry[i].eax == 0 || !(supported & mask))
> continue;
> if (WARN_ON_ONCE(entry[i].ecx & 1))
> continue;
> }
> entry[i].ecx = 0;
> entry[i].edx = 0;
> ++*nent;
> ++i;
> }

Ah rats, I was thinking of CPUID 0x7 when I wrote that. Maybe just reword
it to "(KVM hard caps the number of CPUID 0xD sub-leafs)."?

> I still think the patch is correct, what matters is that no KVM in
> existence supports enough processor features to reach 100 or so subleaves.

2020-03-03 16:49:33

by Vitaly Kuznetsov

[permalink] [raw]
Subject: Re: [PATCH v2 00/66] KVM: x86: Introduce KVM cpu caps

Sean Christopherson <[email protected]> writes:

> All AMD patches are build-tested only.

I tried this on my AMD EPYC 7401P with QEMU and '-cpu host' and the only
difference in CPUIDs I see is:

# diff -u cpuid_with cpuid_without
--- cpuid_with 2020-03-03 11:38:34.786583429 -0500
+++ cpuid_without 2020-03-03 11:43:58.103484420 -0500
@@ -454,16 +454,16 @@
SvmRev: SVM revision = 0x1 (1)
SVM Secure Virtual Machine (0x8000000a/edx):
nested paging = true
- LBR virtualization = true
- SVM lock = true
+ LBR virtualization = false
+ SVM lock = false
NRIP save = true
- MSR based TSC rate control = true
- VMCB clean bits support = true
- flush by ASID = true
- decode assists = true
+ MSR based TSC rate control = false
+ VMCB clean bits support = false
+ flush by ASID = false
+ decode assists = false
SSSE3/SSE5 opcode set disable = false
- pause intercept filter = true
- pause filter threshold = true
+ pause intercept filter = false
+ pause filter threshold = false
AVIC: AMD virtual interrupt controller = false
virtualized VMLOAD/VMSAVE = false
virtualized GIF = false

These are all 0x8000000a.EDX and Paolo already highlighted the issue
(PATCH66).

--
Vitaly

2020-03-03 21:44:11

by Jim Mattson

[permalink] [raw]
Subject: Re: [PATCH v2 01/66] KVM: x86: Return -E2BIG when KVM_GET_SUPPORTED_CPUID hits max entries

On Mon, Mar 2, 2020 at 3:57 PM Sean Christopherson
<[email protected]> wrote:
>
> Fix a long-standing bug that causes KVM to return 0 instead of -E2BIG
> when userspace's array is insufficiently sized.
>
> This technically breaks backwards compatibility, e.g. a userspace with a
> hardcoded cpuid->nent could theoretically be broken as it would see an
> error instead of success if cpuid->nent is less than the number of
> entries required to fully enumerate the host CPU. But, the lowest known
> cpuid->nent hardcoded by a VMM is 100 (lkvm and selftests), and the

I have an existence proof for 98. :-)

> largest realistic limit on Intel and AMD is well under a 100. E.g.
> Intel's Icelake server with all the bells and whistles tops out at ~60
> entries (variable due to SGX sub-leafs), and AMD's CPUID documentation
> allows for less than 50 (KVM hard caps CPUID 0xD at a single sub-leaf).
>
> Note, while the Fixes: tag is accurate with respect to the immediate
> bug, it's likely that similar bugs in KVM_GET_SUPPORTED_CPUID existed
> prior to the refactoring, e.g. Qemu contains a workaround for the broken
> KVM_GET_SUPPORTED_CPUID behavior that predates the buggy commit by over
> two years. The Qemu workaround is also likely the main reason the bug
> has gone unreported for so long.
>
> Qemu hack:
> commit 76ae317f7c16aec6b469604b1764094870a75470
> Author: Mark McLoughlin <[email protected]>
> Date: Tue May 19 18:55:21 2009 +0100
>
> kvm: work around supported cpuid ioctl() brokenness
>
> KVM_GET_SUPPORTED_CPUID has been known to fail to return -E2BIG
> when it runs out of entries. Detect this by always trying again
> with a bigger table if the ioctl() fills the table.
>
> Fixes: 831bf664e9c1f ("KVM: Refactor and simplify kvm_dev_ioctl_get_supported_cpuid")
> Reviewed-by: Vitaly Kuznetsov <[email protected]>
> Signed-off-by: Sean Christopherson <[email protected]>
Reviewed-by: Jim Mattson <[email protected]>

2020-03-06 08:30:03

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH v2 00/66] KVM: x86: Introduce KVM cpu caps

On 03/03/20 00:56, Sean Christopherson wrote:
> Introduce what is effectively a KVM-specific copy of the x86_capabilities
> array in boot_cpu_data, kvm_cpu_caps. kvm_cpu_caps is initialized by
> copying boot_cpu_data.x86_capabilities before ->hardware_setup(). It is
> then updated by KVM's CPUID logic (both common x86 and VMX/SVM specific)
> to adjust the caps to reflect the CPU that KVM will expose to the guest.
>
> Quick synopsis:
> 1. Refactor the KVM_GET_SUPPORTED_CPUID stack to consolidate code,
> remove crustiness, and set the stage for introducing kvm_cpu_caps.
>
> 2. Introduce cpuid_entry_*() accessors/mutators to automatically
> handle retrieving the correct reg from a CPUID entry, and to audit
> that the entry matches the reserve CPUID lookup entry. The
> cpuid_entry_*() helpers make moving the code from common x86 to
> vendor code much less risky.
>
> 3. Move CPUID adjustments to vendor code in preparation for kvm_cpu_caps,
> which will be initialized at load time before the kvm_x86_ops hooks
> are ready to be used, i.e. before ->hardware_setup().
>
> 4. Introduce kvm_cpu_caps and move all the CPUID code over to kvm_cpu_caps.
>
> 5. Use kvm_cpu_cap_has() to kill off a bunch of ->*_supported() hooks.
>
> 6. Additional cleanup in tangentially related areas to kill off even more
> ->*_supported() hooks, including ->set_supported_cpuid().
>
> Tested by verifying the output of KVM_GET_SUPPORTED_CPUID is identical
> before and after on every patch on a Haswell and Coffee Lake, and for the
> "before vs. after" output on Ice Lake.
>
> Verified correctness when hiding features via Qemu (running this version
> of KVM in L1), e.g. that UMIP is correctly emulated for L2 when it's
> hidden from L1, on relevant patches.
>
> Boot tested and ran kvm-unit-tests at key points, e.g. large page
> handling.
>
> All AMD patches are build-tested only.

I put the complete series on the cpu-caps branch of kvm.git.

Thanks,

Paolo

> v2:
> - Opportunistically remove bare "unsigned" usgae. [Vitaly]
> - Remove CPUID auditing (Vitaly and Paolo suggested making it
> unconditional, then I realized it would trigger false positives).
> - Fix a bug in the series that broke SVM features enumeration.
> - Only advertise SVM features when nested SVM is enabled. [Paolo]
> - Fully remove support for stateful CPUID.0x2. [Vitaly, Paolo]
> - Call out in patch 01's changelog that it technically breaks the
> ABI, but that no known VMM is affected. [Vitaly, Paolo]
> - Use @function instead of hardcoding "2" for thes stateful code (which
> eventually gets tossed anyways). [Vitaly]
> - Move 0x8000000A into common code and kill ->set_supported_cpuid().
> [Vitaly]
> - Call out the subtle emulation handling in ->set_supported_cpuid(),
> which also gets tossed :-). [Vitaly]
> - Fix the BUILG_BUG_ON() in patch 38. [Vitaly]
> - Use !! to explicitly cast a u32 to a bool. [Vitaly, Paolo]
> - Sort kvm_cpu_cap_mask() calls by leaf number, ascending. [Vitaly]
> - Collect reviews. [Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly,
> Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly,
> Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly,
> Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly,
> Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly,
> Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly,
> Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Vitaly,
> Vitaly, Vitaly, Vitaly, Vitaly, Vitaly, Xiaoyao, Xiaoyao, Xiaoyao]
>
> Sean Christopherson (66):
> KVM: x86: Return -E2BIG when KVM_GET_SUPPORTED_CPUID hits max entries
> KVM: x86: Refactor loop around do_cpuid_func() to separate helper
> KVM: x86: Simplify handling of Centaur CPUID leafs
> KVM: x86: Clean up error handling in kvm_dev_ioctl_get_cpuid()
> KVM: x86: Check userapce CPUID array size after validating sub-leaf
> KVM: x86: Move CPUID 0xD.1 handling out of the index>0 loop
> KVM: x86: Check for CPUID 0xD.N support before validating array size
> KVM: x86: Warn on zero-size save state for valid CPUID 0xD.N sub-leaf
> KVM: x86: Refactor CPUID 0xD.N sub-leaf entry creation
> KVM: x86: Clean up CPUID 0x7 sub-leaf loop
> KVM: x86: Drop the explicit @index from do_cpuid_7_mask()
> KVM: x86: Drop redundant boot cpu checks on SSBD feature bits
> KVM: x86: Consolidate CPUID array max num entries checking
> KVM: x86: Hoist loop counter and terminator to top of
> __do_cpuid_func()
> KVM: x86: Refactor CPUID 0x4 and 0x8000001d handling
> KVM: x86: Encapsulate CPUID entries and metadata in struct
> KVM: x86: Drop redundant array size check
> KVM: x86: Use common loop iterator when handling CPUID 0xD.N
> KVM: VMX: Add helpers to query Intel PT mode
> KVM: x86: Calculate the supported xcr0 mask at load time
> KVM: x86: Use supported_xcr0 to detect MPX support
> KVM: x86: Make kvm_mpx_supported() an inline function
> KVM: x86: Clear output regs for CPUID 0x14 if PT isn't exposed to
> guest
> KVM: x86: Drop explicit @func param from ->set_supported_cpuid()
> KVM: x86: Use u32 for holding CPUID register value in helpers
> KVM: x86: Replace bare "unsigned" with "unsigned int" in cpuid helpers
> KVM: x86: Introduce cpuid_entry_{get,has}() accessors
> KVM: x86: Introduce cpuid_entry_{change,set,clear}() mutators
> KVM: x86: Refactor cpuid_mask() to auto-retrieve the register
> KVM: x86: Handle MPX CPUID adjustment in VMX code
> KVM: x86: Handle INVPCID CPUID adjustment in VMX code
> KVM: x86: Handle UMIP emulation CPUID adjustment in VMX code
> KVM: x86: Handle PKU CPUID adjustment in VMX code
> KVM: x86: Handle RDTSCP CPUID adjustment in VMX code
> KVM: x86: Handle Intel PT CPUID adjustment in VMX code
> KVM: x86: Handle GBPAGE CPUID adjustment for EPT in VMX code
> KVM: x86: Refactor handling of XSAVES CPUID adjustment
> KVM: x86: Introduce kvm_cpu_caps to replace runtime CPUID masking
> KVM: SVM: Convert feature updates from CPUID to KVM cpu caps
> KVM: VMX: Convert feature updates from CPUID to KVM cpu caps
> KVM: x86: Move XSAVES CPUID adjust to VMX's KVM cpu cap update
> KVM: x86: Add a helper to check kernel support when setting cpu cap
> KVM: x86: Use KVM cpu caps to mark CR4.LA57 as not-reserved
> KVM: x86: Use KVM cpu caps to track UMIP emulation
> KVM: x86: Fold CPUID 0x7 masking back into __do_cpuid_func()
> KVM: x86: Remove the unnecessary loop on CPUID 0x7 sub-leafs
> KVM: x86: Squash CPUID 0x2.0 insanity for modern CPUs
> KVM: x86: Remove stateful CPUID handling
> KVM: x86: Do host CPUID at load time to mask KVM cpu caps
> KVM: x86: Override host CPUID results with kvm_cpu_caps
> KVM: x86: Set emulated/transmuted feature bits via kvm_cpu_caps
> KVM: x86: Use kvm_cpu_caps to detect Intel PT support
> KVM: x86: Do kvm_cpuid_array capacity checks in terminal functions
> KVM: x86: Use KVM cpu caps to detect MSR_TSC_AUX virt support
> KVM: VMX: Directly use VMX capabilities helper to detect RDTSCP
> support
> KVM: x86: Check for Intel PT MSR virtualization using KVM cpu caps
> KVM: VMX: Directly query Intel PT mode when refreshing PMUs
> KVM: SVM: Refactor logging of NPT enabled/disabled
> KVM: x86/mmu: Merge kvm_{enable,disable}_tdp() into a common function
> KVM: x86/mmu: Configure max page level during hardware setup
> KVM: x86: Don't propagate MMU lpage support to memslot.disallow_lpage
> KVM: Drop largepages_enabled and its accessor/mutator
> KVM: x86: Move VMX's host_efer to common x86 code
> KVM: nSVM: Expose SVM features to L1 iff nested is enabled
> KVM: nSVM: Advertise and enable NRIPS for L1 iff nrips is enabled
> KVM: x86: Move nSVM CPUID 0x8000000A handing into common x86 code
>
> Documentation/virt/kvm/api.rst | 22 +-
> arch/x86/include/asm/kvm_host.h | 15 +-
> arch/x86/kvm/cpuid.c | 874 +++++++++++++++-----------------
> arch/x86/kvm/cpuid.h | 134 ++++-
> arch/x86/kvm/mmu/mmu.c | 29 +-
> arch/x86/kvm/svm.c | 130 ++---
> arch/x86/kvm/vmx/capabilities.h | 25 +-
> arch/x86/kvm/vmx/nested.c | 2 +-
> arch/x86/kvm/vmx/pmu_intel.c | 2 +-
> arch/x86/kvm/vmx/vmx.c | 121 +++--
> arch/x86/kvm/vmx/vmx.h | 5 +-
> arch/x86/kvm/x86.c | 48 +-
> arch/x86/kvm/x86.h | 10 +-
> include/linux/kvm_host.h | 2 -
> virt/kvm/kvm_main.c | 13 -
> 15 files changed, 695 insertions(+), 737 deletions(-)
>

2020-03-09 20:14:15

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH v2 00/66] KVM: x86: Introduce KVM cpu caps

On Fri, Mar 06, 2020 at 09:27:48AM +0100, Paolo Bonzini wrote:
> I put the complete series on the cpu-caps branch of kvm.git.

Looks good, arrived at more or less the same end result when rebasing my
local branch to kvm/queue. CPUID diff and smoke test on Icelake ran clean.

For supported_xss, would it make sense to handle it purely in common x86
code, e.g. stub in something similar to supported_xcr0? KVM_SUPPORTED_XSS
would be 0 for now. I assume whatever XSAVES features are supported will
be "supported" by both VMX and SVM, in the sense that VMX/SVM won't need
to mask off features that can exist on their respective hardware but can't
be exposed to the guest.

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 4dca3579e740..c6e9910d1149 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1371,8 +1371,6 @@ static __init void svm_set_cpu_caps(void)
{
kvm_set_cpu_caps();

- supported_xss = 0;
-
/* CPUID 0x80000001 and 0x8000000A (SVM features) */
if (nested) {
kvm_cpu_cap_set(X86_FEATURE_SVM);
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 8001070b209c..e91a84bb251c 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7126,7 +7126,6 @@ static __init void vmx_set_cpu_caps(void)
kvm_cpu_cap_set(X86_FEATURE_UMIP);

/* CPUID 0xD.1 */
- supported_xss = 0;
if (!vmx_xsaves_supported())
kvm_cpu_cap_clear(X86_FEATURE_XSAVES);

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 96e897d38a63..29cfe80db4b4 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9628,6 +9628,8 @@ int kvm_arch_hardware_setup(void)

if (!kvm_cpu_cap_has(X86_FEATURE_XSAVES))
supported_xss = 0;
+ else
+ supported_xss = host_xss & KVM_SUPPORTED_XSS;

cr4_reserved_bits = kvm_host_cr4_reserved_bits(&boot_cpu_data);

2020-03-11 18:38:54

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH v2 00/66] KVM: x86: Introduce KVM cpu caps

On 09/03/20 21:11, Sean Christopherson wrote:
>
> For supported_xss, would it make sense to handle it purely in common x86
> code, e.g. stub in something similar to supported_xcr0? KVM_SUPPORTED_XSS
> would be 0 for now. I assume whatever XSAVES features are supported will
> be "supported" by both VMX and SVM, in the sense that VMX/SVM won't need
> to mask off features that can exist on their respective hardware but can't
> be exposed to the guest.

I preferred to keep it safe because I'm not sure if in the future there
will be an XSAVES feature that absolutely needs to be swapped atomically
via VMCB fields. Since SVM does not have an MSR load/save area, it's
not impossible that such a feature would have to be supported only on VMX.

Paolo

> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 4dca3579e740..c6e9910d1149 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -1371,8 +1371,6 @@ static __init void svm_set_cpu_caps(void)
> {
> kvm_set_cpu_caps();
>
> - supported_xss = 0;
> -
> /* CPUID 0x80000001 and 0x8000000A (SVM features) */
> if (nested) {
> kvm_cpu_cap_set(X86_FEATURE_SVM);
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 8001070b209c..e91a84bb251c 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -7126,7 +7126,6 @@ static __init void vmx_set_cpu_caps(void)
> kvm_cpu_cap_set(X86_FEATURE_UMIP);
>
> /* CPUID 0xD.1 */
> - supported_xss = 0;
> if (!vmx_xsaves_supported())
> kvm_cpu_cap_clear(X86_FEATURE_XSAVES);
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 96e897d38a63..29cfe80db4b4 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -9628,6 +9628,8 @@ int kvm_arch_hardware_setup(void)
>
> if (!kvm_cpu_cap_has(X86_FEATURE_XSAVES))
> supported_xss = 0;
> + else
> + supported_xss = host_xss & KVM_SUPPORTED_XSS;
>
> cr4_reserved_bits = kvm_host_cr4_reserved_bits(&boot_cpu_data);
>