Re: [PATCH 1/3] KVM: SVM: Check EFER.SVME before updating MSR_VM_CR on guest writes

[Sean Christopherson]

On Thu, Oct 29, 2026, Jim Mattson wrote:
> (Host- initiated writes are allowed to set any valid MSR_VM_CR bits.)

Eww.  Why?  If we allow this, then we'll end up with the mess where the guest
can't write back the current value (if userspace is being weird).  I get that
we want to avoid ordering issues, but there's *already* a required order here,
unless userspace ignores the failure from svm_set_vm_cr() and continues on.

KVM doesn't advertise the MSR as something to be saved/restored, and none of
QEMU, CHV, CrosVM, or our VMM appear to save/restore it.  So why make trouble
for ourselves?

> Fixes: 4a810181c8bc ("KVM: SVM: Implement emulation of vm_cr msr")
> Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
> ---
>  arch/x86/kvm/svm/svm.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 136069dfcd44..3a0a507c94b2 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3015,14 +3015,24 @@ static int svm_complete_emulated_msr(struct kvm_vcpu *vcpu, int err)
>  	return 1;
>  }
>  
> -static int svm_set_vm_cr(struct kvm_vcpu *vcpu, u64 data)
> +static int svm_set_vm_cr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
> -	int svm_dis, chg_mask;
> +	u64 data = msr->data;
> +	int chg_mask;
>  
>  	if (data & ~SVM_VM_CR_VALID_MASK)
>  		return 1;
>  
> +	if (msr->host_initiated) {
> +		svm->nested.vm_cr_msr = data;
> +		return 0;
> +	}
> +
> +	/* check for svm_disable while efer.svme is set */
> +	if ((data & SVM_VM_CR_SVM_DIS_MASK) && (vcpu->arch.efer & EFER_SVME))
> +		return 1;
> +
>  	chg_mask = SVM_VM_CR_VALID_MASK;
>  
>  	if (svm->nested.vm_cr_msr & SVM_VM_CR_SVM_DIS_MASK)
> @@ -3031,12 +3041,6 @@ static int svm_set_vm_cr(struct kvm_vcpu *vcpu, u64 data)
>  	svm->nested.vm_cr_msr &= ~chg_mask;
>  	svm->nested.vm_cr_msr |= (data & chg_mask);
>  
> -	svm_dis = svm->nested.vm_cr_msr & SVM_VM_CR_SVM_DIS_MASK;
> -
> -	/* check for svm_disable while efer.svme is set */
> -	if (svm_dis && (vcpu->arch.efer & EFER_SVME))
> -		return 1;
> -
>  	return 0;
>  }
>  
> @@ -3262,7 +3266,7 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>  		svm->nested.hsave_msr = data & PAGE_MASK;
>  		break;
>  	case MSR_VM_CR:
> -		return svm_set_vm_cr(vcpu, data);
> +		return svm_set_vm_cr(vcpu, msr);
>  	case MSR_VM_IGNNE:
>  		kvm_pr_unimpl_wrmsr(vcpu, ecx, data);
>  		break;
> -- 
> 2.56.0.317.g454f8302e1-goog
>