Track "VMX exposed to L1" via a governed feature flag instead of using a
dedicated helper to provide the same functionality. The main goal is to
drive convergence between VMX and SVM with respect to querying features
that are controllable via module param (SVM likes to cache nested
features), avoiding the guest CPUID lookups at runtime is just a bonus
and unlikely to provide any meaningful performance benefits.
No functional change intended.
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/governed_features.h | 1 +
arch/x86/kvm/vmx/nested.c | 7 ++++---
arch/x86/kvm/vmx/vmx.c | 21 ++++++---------------
arch/x86/kvm/vmx/vmx.h | 1 -
4 files changed, 11 insertions(+), 19 deletions(-)
diff --git a/arch/x86/kvm/governed_features.h b/arch/x86/kvm/governed_features.h
index b896a64e4ac3..22446614bf49 100644
--- a/arch/x86/kvm/governed_features.h
+++ b/arch/x86/kvm/governed_features.h
@@ -7,6 +7,7 @@ BUILD_BUG()
KVM_GOVERNED_X86_FEATURE(GBPAGES)
KVM_GOVERNED_X86_FEATURE(XSAVES)
+KVM_GOVERNED_X86_FEATURE(VMX)
#undef KVM_GOVERNED_X86_FEATURE
#undef KVM_GOVERNED_FEATURE
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 22e08d30baef..c5ec0ef51ff7 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -6426,7 +6426,7 @@ static int vmx_get_nested_state(struct kvm_vcpu *vcpu,
vmx = to_vmx(vcpu);
vmcs12 = get_vmcs12(vcpu);
- if (nested_vmx_allowed(vcpu) &&
+ if (guest_can_use(vcpu, X86_FEATURE_VMX) &&
(vmx->nested.vmxon || vmx->nested.smm.vmxon)) {
kvm_state.hdr.vmx.vmxon_pa = vmx->nested.vmxon_ptr;
kvm_state.hdr.vmx.vmcs12_pa = vmx->nested.current_vmptr;
@@ -6567,7 +6567,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu,
if (kvm_state->flags & ~KVM_STATE_NESTED_EVMCS)
return -EINVAL;
} else {
- if (!nested_vmx_allowed(vcpu))
+ if (!guest_can_use(vcpu, X86_FEATURE_VMX))
return -EINVAL;
if (!page_address_valid(vcpu, kvm_state->hdr.vmx.vmxon_pa))
@@ -6601,7 +6601,8 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu,
return -EINVAL;
if ((kvm_state->flags & KVM_STATE_NESTED_EVMCS) &&
- (!nested_vmx_allowed(vcpu) || !vmx->nested.enlightened_vmcs_enabled))
+ (!guest_can_use(vcpu, X86_FEATURE_VMX) ||
+ !vmx->nested.enlightened_vmcs_enabled))
return -EINVAL;
vmx_leave_nested(vcpu);
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 3100ed62615c..fdf932cfc64d 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1894,17 +1894,6 @@ static void vmx_write_tsc_multiplier(struct kvm_vcpu *vcpu)
vmcs_write64(TSC_MULTIPLIER, vcpu->arch.tsc_scaling_ratio);
}
-/*
- * nested_vmx_allowed() checks whether a guest should be allowed to use VMX
- * instructions and MSRs (i.e., nested VMX). Nested VMX is disabled for
- * all guests if the "nested" module option is off, and can also be disabled
- * for a single guest by disabling its VMX cpuid bit.
- */
-bool nested_vmx_allowed(struct kvm_vcpu *vcpu)
-{
- return nested && guest_cpuid_has(vcpu, X86_FEATURE_VMX);
-}
-
/*
* Userspace is allowed to set any supported IA32_FEATURE_CONTROL regardless of
* guest CPUID. Note, KVM allows userspace to set "VMX in SMX" to maintain
@@ -2032,7 +2021,7 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
[msr_info->index - MSR_IA32_SGXLEPUBKEYHASH0];
break;
case KVM_FIRST_EMULATED_VMX_MSR ... KVM_LAST_EMULATED_VMX_MSR:
- if (!nested_vmx_allowed(vcpu))
+ if (!guest_can_use(vcpu, X86_FEATURE_VMX))
return 1;
if (vmx_get_vmx_msr(&vmx->nested.msrs, msr_info->index,
&msr_info->data))
@@ -2340,7 +2329,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
case KVM_FIRST_EMULATED_VMX_MSR ... KVM_LAST_EMULATED_VMX_MSR:
if (!msr_info->host_initiated)
return 1; /* they are read-only */
- if (!nested_vmx_allowed(vcpu))
+ if (!guest_can_use(vcpu, X86_FEATURE_VMX))
return 1;
return vmx_set_vmx_msr(vcpu, msr_index, data);
case MSR_IA32_RTIT_CTL:
@@ -7727,13 +7716,15 @@ static void vmx_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
guest_cpuid_has(vcpu, X86_FEATURE_XSAVE))
kvm_governed_feature_check_and_set(vcpu, X86_FEATURE_XSAVES);
+ kvm_governed_feature_check_and_set(vcpu, X86_FEATURE_VMX);
+
vmx_setup_uret_msrs(vmx);
if (cpu_has_secondary_exec_ctrls())
vmcs_set_secondary_exec_control(vmx,
vmx_secondary_exec_control(vmx));
- if (nested_vmx_allowed(vcpu))
+ if (guest_can_use(vcpu, X86_FEATURE_VMX))
vmx->msr_ia32_feature_control_valid_bits |=
FEAT_CTL_VMX_ENABLED_INSIDE_SMX |
FEAT_CTL_VMX_ENABLED_OUTSIDE_SMX;
@@ -7742,7 +7733,7 @@ static void vmx_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
~(FEAT_CTL_VMX_ENABLED_INSIDE_SMX |
FEAT_CTL_VMX_ENABLED_OUTSIDE_SMX);
- if (nested_vmx_allowed(vcpu))
+ if (guest_can_use(vcpu, X86_FEATURE_VMX))
nested_vmx_cr_fixed1_bits_update(vcpu);
if (boot_cpu_has(X86_FEATURE_INTEL_PT) &&
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index cde902b44d97..c2130d2c8e24 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -374,7 +374,6 @@ struct kvm_vmx {
u64 *pid_table;
};
-bool nested_vmx_allowed(struct kvm_vcpu *vcpu);
void vmx_vcpu_load_vmcs(struct kvm_vcpu *vcpu, int cpu,
struct loaded_vmcs *buddy);
int allocate_vpid(void);
--
2.41.0.487.g6d72f3e995-goog
On Fri, Jul 28, 2023 at 06:16:00PM -0700, Sean Christopherson wrote:
> Track "VMX exposed to L1" via a governed feature flag instead of using a
> dedicated helper to provide the same functionality. The main goal is to
> drive convergence between VMX and SVM with respect to querying features
> that are controllable via module param (SVM likes to cache nested
> features), avoiding the guest CPUID lookups at runtime is just a bonus
> and unlikely to provide any meaningful performance benefits.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
> arch/x86/kvm/governed_features.h | 1 +
> arch/x86/kvm/vmx/nested.c | 7 ++++---
> arch/x86/kvm/vmx/vmx.c | 21 ++++++---------------
> arch/x86/kvm/vmx/vmx.h | 1 -
> 4 files changed, 11 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/kvm/governed_features.h b/arch/x86/kvm/governed_features.h
> index b896a64e4ac3..22446614bf49 100644
> --- a/arch/x86/kvm/governed_features.h
> +++ b/arch/x86/kvm/governed_features.h
> @@ -7,6 +7,7 @@ BUILD_BUG()
>
> KVM_GOVERNED_X86_FEATURE(GBPAGES)
> KVM_GOVERNED_X86_FEATURE(XSAVES)
> +KVM_GOVERNED_X86_FEATURE(VMX)
>
> #undef KVM_GOVERNED_X86_FEATURE
> #undef KVM_GOVERNED_FEATURE
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 22e08d30baef..c5ec0ef51ff7 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -6426,7 +6426,7 @@ static int vmx_get_nested_state(struct kvm_vcpu *vcpu,
> vmx = to_vmx(vcpu);
> vmcs12 = get_vmcs12(vcpu);
>
> - if (nested_vmx_allowed(vcpu) &&
> + if (guest_can_use(vcpu, X86_FEATURE_VMX) &&
> (vmx->nested.vmxon || vmx->nested.smm.vmxon)) {
> kvm_state.hdr.vmx.vmxon_pa = vmx->nested.vmxon_ptr;
> kvm_state.hdr.vmx.vmcs12_pa = vmx->nested.current_vmptr;
> @@ -6567,7 +6567,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu,
> if (kvm_state->flags & ~KVM_STATE_NESTED_EVMCS)
> return -EINVAL;
> } else {
> - if (!nested_vmx_allowed(vcpu))
> + if (!guest_can_use(vcpu, X86_FEATURE_VMX))
> return -EINVAL;
>
> if (!page_address_valid(vcpu, kvm_state->hdr.vmx.vmxon_pa))
> @@ -6601,7 +6601,8 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu,
> return -EINVAL;
>
> if ((kvm_state->flags & KVM_STATE_NESTED_EVMCS) &&
> - (!nested_vmx_allowed(vcpu) || !vmx->nested.enlightened_vmcs_enabled))
> + (!guest_can_use(vcpu, X86_FEATURE_VMX) ||
> + !vmx->nested.enlightened_vmcs_enabled))
> return -EINVAL;
>
> vmx_leave_nested(vcpu);
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 3100ed62615c..fdf932cfc64d 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -1894,17 +1894,6 @@ static void vmx_write_tsc_multiplier(struct kvm_vcpu *vcpu)
> vmcs_write64(TSC_MULTIPLIER, vcpu->arch.tsc_scaling_ratio);
> }
>
> -/*
> - * nested_vmx_allowed() checks whether a guest should be allowed to use VMX
> - * instructions and MSRs (i.e., nested VMX). Nested VMX is disabled for
> - * all guests if the "nested" module option is off, and can also be disabled
> - * for a single guest by disabling its VMX cpuid bit.
> - */
> -bool nested_vmx_allowed(struct kvm_vcpu *vcpu)
> -{
> - return nested && guest_cpuid_has(vcpu, X86_FEATURE_VMX);
the removed nested here already covered by
kvm_cpu_cap_set(X86_FEATURE_VMX) from vmx_set_cpu_caps().
Reviewed-by: Yuan Yao <[email protected]>
> -}
> -
> /*
> * Userspace is allowed to set any supported IA32_FEATURE_CONTROL regardless of
> * guest CPUID. Note, KVM allows userspace to set "VMX in SMX" to maintain
> @@ -2032,7 +2021,7 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> [msr_info->index - MSR_IA32_SGXLEPUBKEYHASH0];
> break;
> case KVM_FIRST_EMULATED_VMX_MSR ... KVM_LAST_EMULATED_VMX_MSR:
> - if (!nested_vmx_allowed(vcpu))
> + if (!guest_can_use(vcpu, X86_FEATURE_VMX))
> return 1;
> if (vmx_get_vmx_msr(&vmx->nested.msrs, msr_info->index,
> &msr_info->data))
> @@ -2340,7 +2329,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> case KVM_FIRST_EMULATED_VMX_MSR ... KVM_LAST_EMULATED_VMX_MSR:
> if (!msr_info->host_initiated)
> return 1; /* they are read-only */
> - if (!nested_vmx_allowed(vcpu))
> + if (!guest_can_use(vcpu, X86_FEATURE_VMX))
> return 1;
> return vmx_set_vmx_msr(vcpu, msr_index, data);
> case MSR_IA32_RTIT_CTL:
> @@ -7727,13 +7716,15 @@ static void vmx_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
> guest_cpuid_has(vcpu, X86_FEATURE_XSAVE))
> kvm_governed_feature_check_and_set(vcpu, X86_FEATURE_XSAVES);
>
> + kvm_governed_feature_check_and_set(vcpu, X86_FEATURE_VMX);
> +
> vmx_setup_uret_msrs(vmx);
>
> if (cpu_has_secondary_exec_ctrls())
> vmcs_set_secondary_exec_control(vmx,
> vmx_secondary_exec_control(vmx));
>
> - if (nested_vmx_allowed(vcpu))
> + if (guest_can_use(vcpu, X86_FEATURE_VMX))
> vmx->msr_ia32_feature_control_valid_bits |=
> FEAT_CTL_VMX_ENABLED_INSIDE_SMX |
> FEAT_CTL_VMX_ENABLED_OUTSIDE_SMX;
> @@ -7742,7 +7733,7 @@ static void vmx_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
> ~(FEAT_CTL_VMX_ENABLED_INSIDE_SMX |
> FEAT_CTL_VMX_ENABLED_OUTSIDE_SMX);
>
> - if (nested_vmx_allowed(vcpu))
> + if (guest_can_use(vcpu, X86_FEATURE_VMX))
> nested_vmx_cr_fixed1_bits_update(vcpu);
>
> if (boot_cpu_has(X86_FEATURE_INTEL_PT) &&
> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> index cde902b44d97..c2130d2c8e24 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -374,7 +374,6 @@ struct kvm_vmx {
> u64 *pid_table;
> };
>
> -bool nested_vmx_allowed(struct kvm_vcpu *vcpu);
> void vmx_vcpu_load_vmcs(struct kvm_vcpu *vcpu, int cpu,
> struct loaded_vmcs *buddy);
> int allocate_vpid(void);
> --
> 2.41.0.487.g6d72f3e995-goog
>