Re: [PATCH v3 1/7] Revert "KVM: x86: Skip EFER vs. guest CPUID checks for host-initiated writes"

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] KVM: x86: outdated EFER_LMSLE_MBZ api.rst doc and __kvm_valid_efer() comment
--

commit e9f88969b96c2ee54c7d5d0af947bad813dd2acc
Author: Sean Christopherson <seanjc@google.com>

Revert "KVM: x86: Skip EFER vs. guest CPUID checks for host-initiated writes"

This commit reverts commit 11988499e62b ("KVM: x86: Skip EFER vs. guest CPUID
checks for host-initiated writes") so that host-initiated writes to MSR_EFER
via set_efer() are once again validated against guest CPUID capabilities and
CR0.PG vs. EFER.LME constraints.

> diff --git a/arch/x86/kvm/msrs.c b/arch/x86/kvm/msrs.c
> index e91650af44c20..2df42276fd826 100644
> --- a/arch/x86/kvm/msrs.c
> +++ b/arch/x86/kvm/msrs.c
> @@ -580,8 +580,11 @@ static int do_get_feature_msr(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
>  				 kvm_get_feature_msr);
>  }
>  
> -static bool __kvm_valid_efer(struct kvm_vcpu *vcpu, u64 efer)
> +bool kvm_valid_efer(struct kvm_vcpu *vcpu, u64 efer)
>  {
> +	if (efer & ~kvm_caps.supported_efer_bits)
> +		return false;
> +
>  	if (efer & EFER_AUTOIBRS && !guest_cpu_cap_has(vcpu, X86_FEATURE_AUTOIBRS))
>  		return false;
>  
> @@ -610,33 +613,18 @@ static bool __kvm_valid_efer(struct kvm_vcpu *vcpu, u64 efer)
>  
>  	return true;
>  }
> -
> -bool kvm_valid_efer(struct kvm_vcpu *vcpu, u64 efer)
> -{
> -	if (efer & ~kvm_caps.supported_efer_bits)
> -		return false;
> -
> -	return __kvm_valid_efer(vcpu, efer);
> -}
>  EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_valid_efer);
>  
> -static int set_efer(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> +static int set_efer(struct kvm_vcpu *vcpu, u64 efer)
>  {
>  	u64 old_efer = vcpu->arch.efer;
> -	u64 efer = msr_info->data;
>  	int r;
>  
> -	if (efer & ~kvm_caps.supported_efer_bits)
> +	if (!kvm_valid_efer(vcpu, efer))
>  		return 1;
>  
> -	if (!msr_info->host_initiated) {
> -		if (!__kvm_valid_efer(vcpu, efer))
> -			return 1;
> -
> -		if (is_paging(vcpu) &&
> -		    (vcpu->arch.efer & EFER_LME) != (efer & EFER_LME))
> -			return 1;
> -	}
> +	if (is_paging(vcpu) && (vcpu->arch.efer & EFER_LME) != (efer & EFER_LME))
> +		return 1;

[Severity: Low]
As reported by Sashiko on v1 [1], now that set_efer() in arch/x86/kvm/msrs.c
unconditionally calls kvm_valid_efer() (including the
X86_FEATURE_EFER_LMSLE_MBZ check) even on host-initiated MSR_EFER writes and
folds __kvm_valid_efer() into kvm_valid_efer(), should
Documentation/virt/kvm/api.rst and the comment in efer_trap() in
arch/x86/kvm/svm/svm.c also be updated?

Specifically, commit 7358d13e1f19 ("KVM: x86: Honor the guest's
EFER_LMSLE_MBZ") added the following note under "Long mode segment limits" in
Documentation/virt/kvm/api.rst:

    Note, ``KVM_SET_MSRS`` is exempt from the check, as host-initiated MSR writes
    skip guest CPUID checks so that userspace can set MSRs before it sets guest
    CPUID.  ``KVM_SET_SREGS`` and nested VMRUN are not exempt.

While the selftest failure in test_host_initiated_lmsle() in
tools/testing/selftests/kvm/x86/svm_nested_efer_test.c is addressed later in
this series by commit "KVM: selftests: Expect failure when trying to set EFER
that violates vCPU model", Documentation/virt/kvm/api.rst still states that
KVM_SET_MSRS is exempt from the check.

Additionally, efer_trap() in arch/x86/kvm/svm/svm.c still references the
removed __kvm_valid_efer() helper in its comment:

    /*
     * Clear the EFER_SVME bit from EFER. The SVM code always sets this
     * bit in svm_set_efer(), but __kvm_valid_efer() checks it against
     * whether the guest has X86_FEATURE_SVM - this avoids a failure if
     * the guest doesn't have X86_FEATURE_SVM.
     */

[1] https://lore.kernel.org/r/20261022223052.646891-2-seanjc@google.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030212721.2606393-1-seanjc@google.com?part=1