Skip to content

Commit

Permalink
KVM: x86: Use KVM-governed feature framework to track "XSAVES enabled"
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
sean-jc committed Aug 17, 2023
1 parent 662f681 commit fe60e8f
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 24 deletions.
1 change: 0 additions & 1 deletion arch/x86/include/asm/kvm_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,6 @@ struct kvm_vcpu_arch {
u64 smi_count;
bool at_instruction_boundary;
bool tpr_access_reporting;
bool xsaves_enabled;
bool xfd_no_write_intercept;
u64 ia32_xss;
u64 microcode_version;
Expand Down
1 change: 1 addition & 0 deletions arch/x86/kvm/governed_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ BUILD_BUG()
#define KVM_GOVERNED_X86_FEATURE(x) KVM_GOVERNED_FEATURE(X86_FEATURE_##x)

KVM_GOVERNED_X86_FEATURE(GBPAGES)
KVM_GOVERNED_X86_FEATURE(XSAVES)

#undef KVM_GOVERNED_X86_FEATURE
#undef KVM_GOVERNED_FEATURE
17 changes: 14 additions & 3 deletions arch/x86/kvm/svm/svm.c
Original file line number Diff line number Diff line change
Expand Up @@ -4244,9 +4244,20 @@ static void svm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
struct vcpu_svm *svm = to_svm(vcpu);
struct kvm_cpuid_entry2 *best;

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

/* Update nrips enabled cache */
svm->nrips_enabled = kvm_cpu_cap_has(X86_FEATURE_NRIPS) &&
Expand Down
36 changes: 18 additions & 18 deletions arch/x86/kvm/vmx/vmx.c
Original file line number Diff line number Diff line change
Expand Up @@ -4543,16 +4543,19 @@ vmx_adjust_secondary_exec_control(struct vcpu_vmx *vmx, u32 *exec_control,
* based on a single guest CPUID bit, with a dedicated feature bit. This also
* verifies that the control is actually supported by KVM and hardware.
*/
#define vmx_adjust_sec_exec_control(vmx, exec_control, name, feat_name, ctrl_name, exiting) \
({ \
bool __enabled; \
\
if (cpu_has_vmx_##name()) { \
__enabled = guest_cpuid_has(&(vmx)->vcpu, \
X86_FEATURE_##feat_name); \
vmx_adjust_secondary_exec_control(vmx, exec_control, \
SECONDARY_EXEC_##ctrl_name, __enabled, exiting); \
} \
#define vmx_adjust_sec_exec_control(vmx, exec_control, name, feat_name, ctrl_name, exiting) \
({ \
struct kvm_vcpu *__vcpu = &(vmx)->vcpu; \
bool __enabled; \
\
if (cpu_has_vmx_##name()) { \
if (kvm_is_governed_feature(X86_FEATURE_##feat_name)) \
__enabled = guest_can_use(__vcpu, X86_FEATURE_##feat_name); \
else \
__enabled = guest_cpuid_has(__vcpu, X86_FEATURE_##feat_name); \
vmx_adjust_secondary_exec_control(vmx, exec_control, SECONDARY_EXEC_##ctrl_name,\
__enabled, exiting); \
} \
})

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

if (cpu_has_vmx_xsaves())
vmx_adjust_secondary_exec_control(vmx, &exec_control,
SECONDARY_EXEC_ENABLE_XSAVES,
vcpu->arch.xsaves_enabled, false);
vmx_adjust_sec_exec_feature(vmx, &exec_control, xsaves, XSAVES);

/*
* RDPID is also gated by ENABLE_RDTSCP, turn on the control if either
Expand All @@ -4634,6 +4634,7 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
SECONDARY_EXEC_ENABLE_RDTSCP,
rdpid_or_rdtscp_enabled, false);
}

vmx_adjust_sec_exec_feature(vmx, &exec_control, invpcid, INVPCID);

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

vmx_setup_uret_msrs(vmx);

Expand Down
4 changes: 2 additions & 2 deletions arch/x86/kvm/x86.c
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,7 @@ void kvm_load_guest_xsave_state(struct kvm_vcpu *vcpu)
if (vcpu->arch.xcr0 != host_xcr0)
xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);

if (vcpu->arch.xsaves_enabled &&
if (guest_can_use(vcpu, X86_FEATURE_XSAVES) &&
vcpu->arch.ia32_xss != host_xss)
wrmsrl(MSR_IA32_XSS, vcpu->arch.ia32_xss);
}
Expand Down Expand Up @@ -1046,7 +1046,7 @@ void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu)
if (vcpu->arch.xcr0 != host_xcr0)
xsetbv(XCR_XFEATURE_ENABLED_MASK, host_xcr0);

if (vcpu->arch.xsaves_enabled &&
if (guest_can_use(vcpu, X86_FEATURE_XSAVES) &&
vcpu->arch.ia32_xss != host_xss)
wrmsrl(MSR_IA32_XSS, host_xss);
}
Expand Down

0 comments on commit fe60e8f

Please sign in to comment.