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 <seanjc@xxxxxxxxxx>
> ---
> 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 <yuan.yao@xxxxxxxxx>
> -}
> -
> /*
> * 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
>
