Skip to content

Commit fe60e8f

Browse files
committed
KVM: x86: Use KVM-governed feature framework to track "XSAVES enabled"
Use the governed feature framework to track if XSAVES is "enabled", i.e. if XSAVES can be used by the guest. Add a comment in the SVM code to explain the very unintuitive logic of deliberately NOT checking if XSAVES is enumerated in the guest CPUID model. No functional change intended. Reviewed-by: Yuan Yao <yuan.yao@intel.com> Link: https://lore.kernel.org/r/20230815203653.519297-7-seanjc@google.com Signed-off-by: Sean Christopherson <seanjc@google.com>
1 parent 662f681 commit fe60e8f

File tree

5 files changed

+35
-24
lines changed

5 files changed

+35
-24
lines changed

arch/x86/include/asm/kvm_host.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -746,7 +746,6 @@ struct kvm_vcpu_arch {
746746
u64 smi_count;
747747
bool at_instruction_boundary;
748748
bool tpr_access_reporting;
749-
bool xsaves_enabled;
750749
bool xfd_no_write_intercept;
751750
u64 ia32_xss;
752751
u64 microcode_version;

arch/x86/kvm/governed_features.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ BUILD_BUG()
66
#define KVM_GOVERNED_X86_FEATURE(x) KVM_GOVERNED_FEATURE(X86_FEATURE_##x)
77

88
KVM_GOVERNED_X86_FEATURE(GBPAGES)
9+
KVM_GOVERNED_X86_FEATURE(XSAVES)
910

1011
#undef KVM_GOVERNED_X86_FEATURE
1112
#undef KVM_GOVERNED_FEATURE

arch/x86/kvm/svm/svm.c

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4244,9 +4244,20 @@ static void svm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
42444244
struct vcpu_svm *svm = to_svm(vcpu);
42454245
struct kvm_cpuid_entry2 *best;
42464246

4247-
vcpu->arch.xsaves_enabled = guest_cpuid_has(vcpu, X86_FEATURE_XSAVE) &&
4248-
boot_cpu_has(X86_FEATURE_XSAVE) &&
4249-
boot_cpu_has(X86_FEATURE_XSAVES);
4247+
/*
4248+
* SVM doesn't provide a way to disable just XSAVES in the guest, KVM
4249+
* can only disable all variants of by disallowing CR4.OSXSAVE from
4250+
* being set. As a result, if the host has XSAVE and XSAVES, and the
4251+
* guest has XSAVE enabled, the guest can execute XSAVES without
4252+
* faulting. Treat XSAVES as enabled in this case regardless of
4253+
* whether it's advertised to the guest so that KVM context switches
4254+
* XSS on VM-Enter/VM-Exit. Failure to do so would effectively give
4255+
* the guest read/write access to the host's XSS.
4256+
*/
4257+
if (boot_cpu_has(X86_FEATURE_XSAVE) &&
4258+
boot_cpu_has(X86_FEATURE_XSAVES) &&
4259+
guest_cpuid_has(vcpu, X86_FEATURE_XSAVE))
4260+
kvm_governed_feature_set(vcpu, X86_FEATURE_XSAVES);
42504261

42514262
/* Update nrips enabled cache */
42524263
svm->nrips_enabled = kvm_cpu_cap_has(X86_FEATURE_NRIPS) &&

arch/x86/kvm/vmx/vmx.c

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4543,16 +4543,19 @@ vmx_adjust_secondary_exec_control(struct vcpu_vmx *vmx, u32 *exec_control,
45434543
* based on a single guest CPUID bit, with a dedicated feature bit. This also
45444544
* verifies that the control is actually supported by KVM and hardware.
45454545
*/
4546-
#define vmx_adjust_sec_exec_control(vmx, exec_control, name, feat_name, ctrl_name, exiting) \
4547-
({ \
4548-
bool __enabled; \
4549-
\
4550-
if (cpu_has_vmx_##name()) { \
4551-
__enabled = guest_cpuid_has(&(vmx)->vcpu, \
4552-
X86_FEATURE_##feat_name); \
4553-
vmx_adjust_secondary_exec_control(vmx, exec_control, \
4554-
SECONDARY_EXEC_##ctrl_name, __enabled, exiting); \
4555-
} \
4546+
#define vmx_adjust_sec_exec_control(vmx, exec_control, name, feat_name, ctrl_name, exiting) \
4547+
({ \
4548+
struct kvm_vcpu *__vcpu = &(vmx)->vcpu; \
4549+
bool __enabled; \
4550+
\
4551+
if (cpu_has_vmx_##name()) { \
4552+
if (kvm_is_governed_feature(X86_FEATURE_##feat_name)) \
4553+
__enabled = guest_can_use(__vcpu, X86_FEATURE_##feat_name); \
4554+
else \
4555+
__enabled = guest_cpuid_has(__vcpu, X86_FEATURE_##feat_name); \
4556+
vmx_adjust_secondary_exec_control(vmx, exec_control, SECONDARY_EXEC_##ctrl_name,\
4557+
__enabled, exiting); \
4558+
} \
45564559
})
45574560

45584561
/* More macro magic for ENABLE_/opt-in versus _EXITING/opt-out controls. */
@@ -4612,10 +4615,7 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
46124615
if (!enable_pml || !atomic_read(&vcpu->kvm->nr_memslots_dirty_logging))
46134616
exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
46144617

4615-
if (cpu_has_vmx_xsaves())
4616-
vmx_adjust_secondary_exec_control(vmx, &exec_control,
4617-
SECONDARY_EXEC_ENABLE_XSAVES,
4618-
vcpu->arch.xsaves_enabled, false);
4618+
vmx_adjust_sec_exec_feature(vmx, &exec_control, xsaves, XSAVES);
46194619

46204620
/*
46214621
* RDPID is also gated by ENABLE_RDTSCP, turn on the control if either
@@ -4634,6 +4634,7 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
46344634
SECONDARY_EXEC_ENABLE_RDTSCP,
46354635
rdpid_or_rdtscp_enabled, false);
46364636
}
4637+
46374638
vmx_adjust_sec_exec_feature(vmx, &exec_control, invpcid, INVPCID);
46384639

46394640
vmx_adjust_sec_exec_exiting(vmx, &exec_control, rdrand, RDRAND);
@@ -7743,10 +7744,9 @@ static void vmx_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
77437744
* to the guest. XSAVES depends on CR4.OSXSAVE, and CR4.OSXSAVE can be
77447745
* set if and only if XSAVE is supported.
77457746
*/
7746-
vcpu->arch.xsaves_enabled = kvm_cpu_cap_has(X86_FEATURE_XSAVES) &&
7747-
boot_cpu_has(X86_FEATURE_XSAVE) &&
7748-
guest_cpuid_has(vcpu, X86_FEATURE_XSAVE) &&
7749-
guest_cpuid_has(vcpu, X86_FEATURE_XSAVES);
7747+
if (boot_cpu_has(X86_FEATURE_XSAVE) &&
7748+
guest_cpuid_has(vcpu, X86_FEATURE_XSAVE))
7749+
kvm_governed_feature_check_and_set(vcpu, X86_FEATURE_XSAVES);
77507750

77517751
vmx_setup_uret_msrs(vmx);
77527752

arch/x86/kvm/x86.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,7 +1015,7 @@ void kvm_load_guest_xsave_state(struct kvm_vcpu *vcpu)
10151015
if (vcpu->arch.xcr0 != host_xcr0)
10161016
xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
10171017

1018-
if (vcpu->arch.xsaves_enabled &&
1018+
if (guest_can_use(vcpu, X86_FEATURE_XSAVES) &&
10191019
vcpu->arch.ia32_xss != host_xss)
10201020
wrmsrl(MSR_IA32_XSS, vcpu->arch.ia32_xss);
10211021
}
@@ -1046,7 +1046,7 @@ void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu)
10461046
if (vcpu->arch.xcr0 != host_xcr0)
10471047
xsetbv(XCR_XFEATURE_ENABLED_MASK, host_xcr0);
10481048

1049-
if (vcpu->arch.xsaves_enabled &&
1049+
if (guest_can_use(vcpu, X86_FEATURE_XSAVES) &&
10501050
vcpu->arch.ia32_xss != host_xss)
10511051
wrmsrl(MSR_IA32_XSS, host_xss);
10521052
}

0 commit comments

Comments
 (0)