Re: [PATCH 1/4] KVM: nSVM: Reject KVM_SET_NESTED_STATE if L1 has EFER.LMA=1 && EFER.LME=0

[Sean Christopherson <seanjc@google.com>]

On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> On Wed, Aug 26, 2026 at 2:18 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > Reject KVM_SET_NESTED_STATE if the incoming L1 host state has what is
> > effectively an impossible EFER combination of LMA=1 but LME=0, i.e. if the
> > state says long mode is active but not enabled.  Unlike VMX, SVM doesn't
> > have an explicit consistent check for the illegal combination; presumably
> > hardware simply ignores EFER.LMA if EFER.LME=0.
> >
> > Unfortunately, KVM doesn't ignore EFER.LMA in this case and consumes the
> > illegal state when constructing the shadow MMU for L2.  E.g. if userspace
> > also clears CR4.PAE, then kvm_calc_cpu_role() will compute a role with 4 or
> > 5 levels of paging, but shadow_mmu_init_context() will wire up the MMU to
> > use the paging32 template, which maxes out its levels at 2.
> 
> Isn't the "right" thing to do what hardware (presumably) does and
> ignore EFER.LMA if EFER.LME=0?

No, because (a) this is KVM uAPI, not emulation of hardware, and (b) it's a check
on L1 state, not L2 state.  It should be impossible for L1 state to have this
combination through "natural" means, and so a snapshot provided by KVM should
never have this combo either, which there's zero reason to allow userspace to
provide garbage.

> > Note, the "real badness" is effectively the same as what happened with the
> > nVMX bug fixed by commit 112e66017bff ("KVM: nVMX: add missing consistency
> > checks for CR0 and CR4").  Unfortunately, the sanity check added by commit
> > 72e2fb24a0b0 ("KVM: x86/mmu: Bug the VM if a vCPU ends up in long mode
> > without PAE enabled") doesn't work for this case, since L2 state is active
> > at the time of the page fault, but it's L1 that has the bad state.
> >
> > Fixes: cc440cdad5b7 ("KVM: nSVM: implement KVM_GET_NESTED_STATE and KVM_SET_NESTED_STATE")
> > Cc: stable@vger.kernel.org
> > Cc: Yosry Ahmed <yosry@kernel.org>
> > Reported-by: Stefan Teodorescu <fane@google.com>
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >  arch/x86/kvm/svm/nested.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 73f37b050d0a..49fb10ad1f9f 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -2028,6 +2028,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
> >         if (!(save->cr0 & X86_CR0_PG) ||
> >             !(save->cr0 & X86_CR0_PE) ||
> >             (save->rflags & X86_EFLAGS_VM) ||
> > +           ((save->efer & EFER_LMA) && !(save->efer & EFER_LME)) ||
> 
> I just realized I have no idea why we check X86_CR0_PE and
> X86_EFLAGS_VM here. Commit 6906e06db9b04 ("KVM: nSVM: Add missing
> checks for reserved bits to svm_set_nested_state()") says it's to do
> the same checks as VMRUN, but I don't think that's actually the case?
> The checks here seem arbitrary to me?

Again, this is L1 state when L2 is active (the !KVM_STATE_NESTED_GUEST_MODE path
has already bailed), and VMRUN "can only be executed in protected mode with SVM
enabled".  Amusingly, the APM says #VMEXIT "Forces CR0.PE = 1, RFLAGS.VM = 0.",
so I guess it means business.

If anything is wrong, it's the CR0.PG check.  Presumably that got carried forward
from commit c0725420cfdc ("KVM: SVM: Add helper functions for nested SVM").  I
don't see anything in the APM that requires paging to be enabled, and nothing in
that ancient series points at concrete documentation either.