From: sashiko-bot@kernel.org
To: "Sean Christopherson" <seanjc@google.com>
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH v3 5/7] KVM: x86: Recompute EFER.LMA instead of inheriting the current value on EFER writes
Date: Fri, 30 Oct 2026 22:24:31 +0000 [thread overview]
Message-ID: <sashiko-outbox-191575@kernel.org> (raw)
In-Reply-To: <20261030212721.2606393-6-seanjc@google.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm: x86: guest-triggerable WARN_ON_ONCE in set_efer() via SVM state sync
--
commit 5720ef7430b0bd8d9e28950790604b1d1ec8ce2d
Author: Sean Christopherson <seanjc@google.com>
KVM: x86: Recompute EFER.LMA instead of inheriting the current value on EFER writes
This commit updates set_efer() to recompute EFER_LMA from EFER_LME,
is_paging(vcpu), and is_pae(vcpu) instead of carrying forward the existing
EFER_LMA value from vcpu->arch.efer, and adds a WARN_ON_ONCE() if the
recomputed EFER_LMA differs from the previous value.
> diff --git a/arch/x86/kvm/msrs.c b/arch/x86/kvm/msrs.c
> index c3cb1575fe66e..c55402970a345 100644
> --- a/arch/x86/kvm/msrs.c
> +++ b/arch/x86/kvm/msrs.c
> @@ -638,8 +638,17 @@ static int set_efer(struct kvm_vcpu *vcpu, u64 efer)
> (vcpu->arch.efer & EFER_SVME) && !(efer & EFER_SVME))
> return 1;
>
> - efer &= ~EFER_LMA;
> - efer |= vcpu->arch.efer & EFER_LMA;
> + if ((efer & EFER_LME) && is_paging(vcpu) && is_pae(vcpu))
> + efer |= EFER_LMA;
> + else
> + efer &= ~EFER_LMA;
> +
> + /*
> + * Toggling EFER.LMA on a bare write to EFER should be impossible, as
> + * it means KVM allowed architecturally invalid state at some point in
> + * the past, i.e. allowed EFER.LMA=1 && (!CR0.PG || !CR4.PAE).
> + */
> + WARN_ON_ONCE((efer & EFER_LMA) != (vcpu->arch.efer & EFER_LMA));
[Severity: High]
As reported by Sashiko on v1 [1], can a guest or host userspace trigger this
WARN_ON_ONCE() via SVM paths that leave EFER_LMA in vcpu->arch.efer out of
sync with EFER_LME, X86_CR0_PG, and X86_CR4_PAE?
In arch/x86/kvm/msrs.c:set_efer(), changing EFER_LME while paging is enabled
is rejected earlier:
if (is_paging(vcpu) && (vcpu->arch.efer & EFER_LME) != (efer & EFER_LME))
return 1;
As a result, the recomputed EFER_LMA always equals
((vcpu->arch.efer & EFER_LME) && is_paging(vcpu) && is_pae(vcpu)), and the
WARN_ON_ONCE() fires whenever a vCPU enters set_efer() with vcpu->arch.efer's
EFER_LMA bit out of sync with EFER_LME, X86_CR0_PG, and X86_CR4_PAE. Several
SVM paths appear to still leave EFER_LMA out of sync:
1. In arch/x86/kvm/svm/nested.c:nested_vmcb02_prepare_save() and
arch/x86/kvm/svm/svm.c:svm_set_cr0():
/* SVM ignores EFER.LMA if EFER.LME=0 (instead of failing VMRUN). */
if (!(svm->nested.save.efer & EFER_LME))
svm->nested.save.efer &= ~EFER_LMA;
svm_set_efer(vcpu, svm->nested.save.efer);
svm_set_cr0(vcpu, svm->nested.save.cr0);
svm_set_cr4(vcpu, svm->nested.save.cr4);
where svm_set_cr0() only updates EFER_LMA on a transition of X86_CR0_PG
relative to the previous vcpu->arch.cr0:
if (vcpu->arch.efer & EFER_LME) {
if (!is_paging(vcpu) && (cr0 & X86_CR0_PG)) {
vcpu->arch.efer |= EFER_LMA;
if (!vcpu->arch.guest_state_protected)
svm->vmcb->save.efer |= EFER_LMA | EFER_LME;
}
if (is_paging(vcpu) && !(cr0 & X86_CR0_PG)) {
vcpu->arch.efer &= ~EFER_LMA;
if (!vcpu->arch.guest_state_protected)
svm->vmcb->save.efer &= ~(EFER_LMA | EFER_LME);
}
}
Because L1 has X86_CR0_PG=1 when executing VMRUN, is_paging(vcpu) is already
true when svm_set_cr0() runs. If L1 executes VMRUN with vmcb12->save
containing EFER_SVME=1, EFER_LME=1, EFER_LMA=0, X86_CR0_PG=1, X86_CR0_PE=1,
and X86_CR4_PAE=1 (which nested_vmcb_check_save() accepts because hardware
SVM ignores EFER_LMA in the VMCB save area), L2 runs with EFER_LMA=0 in
vcpu->arch.efer while EFER_LME=1, is_paging(vcpu), and is_pae(vcpu) are all
true. A subsequent WRMSR to MSR_EFER in L2 or KVM_SET_MSRS from userspace
enters set_efer(), computes EFER_LMA=1, and triggers the WARN_ON_ONCE().
2. On RSM from SMM into nested SVM via
emulator_leave_smm() -> svm_leave_smm() -> enter_svm_guest_mode() ->
nested_vmcb02_prepare_save(), emulator_leave_smm() clears X86_CR0_PG=0
before calling svm_leave_smm(). If vmcb12->save in guest memory has
EFER_SVME=1, EFER_LME=1, EFER_LMA=1, and X86_CR0_PG=0,
nested_vmcb02_prepare_save() does not clear EFER_LMA (since EFER_LME=1) and
svm_set_cr0() does not clear EFER_LMA (since is_paging(vcpu) is already
false). Immediately afterward, rsm_load_state_64() calls
__kvm_emulate_msr_write(vcpu, MSR_EFER, smstate->efer & ~EFER_LMA) ->
set_efer(), which computes EFER_LMA=0 against vcpu->arch.efer's EFER_LMA=1
and triggers the WARN_ON_ONCE().
3. In arch/x86/kvm/svm/nested.c:svm_set_nested_state(), the check on L1's
host save state in vmcb01->save only rejects (save->efer & EFER_LMA) &&
!(save->efer & EFER_LME):
if (!(save->cr0 & X86_CR0_PG) ||
!(save->cr0 & X86_CR0_PE) ||
(save->rflags & X86_EFLAGS_VM) ||
((save->efer & EFER_LMA) && !(save->efer & EFER_LME)) ||
!nested_vmcb_check_save(vcpu, &save_cached, false))
goto out_free;
and svm_leave_smm() similarly loads vmcb01->save from guest MSR_VM_HSAVE_PA
memory without checking EFER_LMA, allowing vmcb01->save to hold EFER_LME=1,
EFER_LMA=0, X86_CR0_PG=1, and X86_CR4_PAE=1. On nested_svm_vmexit() from a
paged L2 (where is_paging(vcpu) is already true):
svm_set_efer(vcpu, vmcb01->save.efer);
svm_set_cr0(vcpu, vmcb01->save.cr0 | X86_CR0_PE);
svm_set_cr4(vcpu, vmcb01->save.cr4);
L1 is left with EFER_LMA=0 in vcpu->arch.efer while EFER_LME=1, X86_CR0_PG=1,
and X86_CR4_PAE=1, so the next WRMSR to MSR_EFER or KVM_SET_MSRS triggers
the WARN_ON_ONCE().
4. On SEV-SNP AP creation via sev_snp_init_protected_guest_state() and
kvm_vcpu_reset(), KVM initializes vcpu->arch.cr0 = X86_CR0_ET,
vcpu->arch.cr4 = 0, and vcpu->arch.efer = 0 while the guest VMSA can already
have X86_CR4_PAE=1 in hardware. In arch/x86/kvm/svm/svm.c:cr_trap():
case 0:
old_value = kvm_read_cr0(vcpu);
svm_set_cr0(vcpu, new_value);
kvm_post_set_cr0(vcpu, old_value, new_value);
break;
If the AP traps on an MSR_EFER write setting EFER_LME=1 (while vcpu->arch.cr0
has X86_CR0_PG=0) and then traps on a CR0 write setting X86_CR0_PG=1 without
having trapped on CR4, cr_trap() -> svm_set_cr0() sets
vcpu->arch.efer |= EFER_LMA even though vcpu->arch.cr4 is still 0. A
subsequent MSR_EFER write trap before any CR4 write enters
efer_trap() -> set_efer(), sees !is_pae(vcpu), clears EFER_LMA in
vcpu->arch.efer while the guest is running in long mode, and triggers the
WARN_ON_ONCE().
[1] https://lore.kernel.org/r/sashiko-outbox-182134@kernel.org
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261030212721.2606393-1-seanjc@google.com?part=5
next prev parent reply other threads:[~2026-10-30 22:24 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-30 21:27 [PATCH v3 0/7] KVM: x86: More EFER.LMA vs. CR4.PAE fixes Sean Christopherson
2026-10-30 21:27 ` [PATCH v3 1/7] Revert "KVM: x86: Skip EFER vs. guest CPUID checks for host-initiated writes" Sean Christopherson
2026-10-30 21:39 ` sashiko-bot
2026-10-30 21:27 ` [PATCH v3 2/7] KVM: x86: Treat EFER.LMA, not EFER.LMA, as an EFER MMU role bit Sean Christopherson
2026-10-30 21:27 ` [PATCH v3 3/7] KVM: x86: Reject clearing EFER.SVME via KVM_SYNC_X86_SREGS if L2 is active Sean Christopherson
2026-10-30 21:27 ` [PATCH v3 4/7] KVM: nSVM: Synthesize #GP instead of SHUTDOWN if L2 attempts to clear EFER.SVME Sean Christopherson
2026-10-30 21:27 ` [PATCH v3 5/7] KVM: x86: Recompute EFER.LMA instead of inheriting the current value on EFER writes Sean Christopherson
2026-10-30 22:24 ` sashiko-bot [this message]
2026-10-30 21:27 ` [PATCH v3 6/7] KVM: selftests: Update nSVM EFER test to expect #GP when clearing SVME in L2 Sean Christopherson
2026-10-30 21:27 ` [PATCH v3 7/7] KVM: selftests: Expect failure when trying to set EFER that violates vCPU model Sean Christopherson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=sashiko-outbox-191575@kernel.org \
--to=sashiko-bot@kernel.org \
--cc=kvm@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
--cc=seanjc@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox