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.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/kvm_host.h | 1 -
arch/x86/kvm/governed_features.h | 1 +
arch/x86/kvm/svm/svm.c | 17 ++++++++++++---
arch/x86/kvm/vmx/vmx.c | 36 ++++++++++++++++----------------
arch/x86/kvm/x86.c | 4 ++--
5 files changed, 35 insertions(+), 24 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 60d430b4650f..9f57aa33798b 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -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;
diff --git a/arch/x86/kvm/governed_features.h b/arch/x86/kvm/governed_features.h
index b29c15d5e038..b896a64e4ac3 100644
--- a/arch/x86/kvm/governed_features.h
+++ b/arch/x86/kvm/governed_features.h
@@ -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
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 6aaa3c7b4578..d67f6e23dcd2 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4273,9 +4273,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) &&
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 22975cc949b7..6314ca32a5cf 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -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. */
@@ -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
@@ -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);
@@ -7745,10 +7746,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);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index eba35d43e3fe..34945c7dba38 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1016,7 +1016,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);
}
@@ -1047,7 +1047,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);
}
--
2.41.0.694.ge786442a9b-googOn Tue, Aug 15, 2023 at 01:36:44PM -0700, Sean Christopherson wrote:
> 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.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/include/asm/kvm_host.h | 1 -
> arch/x86/kvm/governed_features.h | 1 +
> arch/x86/kvm/svm/svm.c | 17 ++++++++++++---
> arch/x86/kvm/vmx/vmx.c | 36 ++++++++++++++++----------------
> arch/x86/kvm/x86.c | 4 ++--
> 5 files changed, 35 insertions(+), 24 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 60d430b4650f..9f57aa33798b 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -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;
> diff --git a/arch/x86/kvm/governed_features.h b/arch/x86/kvm/governed_features.h
> index b29c15d5e038..b896a64e4ac3 100644
> --- a/arch/x86/kvm/governed_features.h
> +++ b/arch/x86/kvm/governed_features.h
> @@ -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
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 6aaa3c7b4578..d67f6e23dcd2 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -4273,9 +4273,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) &&
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 22975cc949b7..6314ca32a5cf 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -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. */
> @@ -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
> @@ -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);
> @@ -7745,10 +7746,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))
Should above 2 be X86_FEATURE_XSAVES ? XSAVE and XSAVES have different
cpuid definition.
Otherwise X86_FEATURE_XSAVES is allowed in governor even XSAVES
is not exposed to guest cpuid, with unnecessary context switches.
> + kvm_governed_feature_check_and_set(vcpu, X86_FEATURE_XSAVES);
>
> vmx_setup_uret_msrs(vmx);
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index eba35d43e3fe..34945c7dba38 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1016,7 +1016,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);
> }
> @@ -1047,7 +1047,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);
> }
> --
> 2.41.0.694.ge786442a9b-goog
>
On Wed, Aug 16, 2023 at 10:58:41AM +0800, Yuan Yao wrote:
> On Tue, Aug 15, 2023 at 01:36:44PM -0700, Sean Christopherson wrote:
> > 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.
> >
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> > arch/x86/include/asm/kvm_host.h | 1 -
> > arch/x86/kvm/governed_features.h | 1 +
> > arch/x86/kvm/svm/svm.c | 17 ++++++++++++---
> > arch/x86/kvm/vmx/vmx.c | 36 ++++++++++++++++----------------
> > arch/x86/kvm/x86.c | 4 ++--
> > 5 files changed, 35 insertions(+), 24 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 60d430b4650f..9f57aa33798b 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -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;
> > diff --git a/arch/x86/kvm/governed_features.h b/arch/x86/kvm/governed_features.h
> > index b29c15d5e038..b896a64e4ac3 100644
> > --- a/arch/x86/kvm/governed_features.h
> > +++ b/arch/x86/kvm/governed_features.h
> > @@ -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
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 6aaa3c7b4578..d67f6e23dcd2 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -4273,9 +4273,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) &&
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index 22975cc949b7..6314ca32a5cf 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -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. */
> > @@ -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
> > @@ -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);
> > @@ -7745,10 +7746,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))
>
> Should above 2 be X86_FEATURE_XSAVES ? XSAVE and XSAVES have different
> cpuid definition.
> Otherwise X86_FEATURE_XSAVES is allowed in governor even XSAVES
> is not exposed to guest cpuid, with unnecessary context switches.
Oh! false alarm.
I just forgot that kvm_governed_feature_check_and_set() does checks
on kvm cpu cap and guest cpuid set, thus no problem.
Reviewed-by: Yuan Yao <yuan.yao@intel.com>
>
> > + kvm_governed_feature_check_and_set(vcpu, X86_FEATURE_XSAVES);
> >
> > vmx_setup_uret_msrs(vmx);
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index eba35d43e3fe..34945c7dba38 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -1016,7 +1016,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);
> > }
> > @@ -1047,7 +1047,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);
> > }
> > --
> > 2.41.0.694.ge786442a9b-goog
> >
© 2016 - 2025 Red Hat, Inc.