Re: [PATCH v2 2/5] KVM: nSVM: Ignore EFER.LMA if EFER.LME=0 when preparing L2 state

[Yosry Ahmed]

On Wed, Sep 2, 2026 at 5:02 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Wed, Sep 02, 2026, Yosry Ahmed wrote:
> > On Wed, Sep 2, 2026 at 4:20 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> > >
> > > Force EFER.LMA=0 if EFER.LME=0 when preparing L2 state for VMRUN, i.e.
> > > mimic real hardware's behavior of ignoring EFER.LMA if EFER.LME=0.  VMRUN
> > > unfortunately allows the nonsensical combination, i.e. doesn't fail, but
> > > KVM itself has an invariant EFER.LMA can be set et if and only if EFER.LME
> > > is set.   Breaking that invariant can lead to a variety of issue,
> > > particularly in MMU code that keys off EFER.LMA when determining whether to
> > > emulate/virtualization 4/5-level paging versus PAE paging.
> > >
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > Cc: Yosry Ahmed <yosry@xxxxxxxxxx>
> > > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > > ---
> > >  arch/x86/kvm/svm/nested.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > > index 49fb10ad1f9f..23d29597d6bf 100644
> > > --- a/arch/x86/kvm/svm/nested.c
> > > +++ b/arch/x86/kvm/svm/nested.c
> > > @@ -789,6 +789,10 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
> > >
> > >         kvm_set_rflags(vcpu, save->rflags | X86_EFLAGS_FIXED);
> > >
> > > +       /* 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;
> >
> > We sanitize control fields in __nested_copy_vmcb_control_to_cache().
> > Should we similarly sanitize this in __nested_copy_vmcb_save_to_cache()?
>
> Ideally, yes?  In practice, it doesn't work because svm_set_nested_state() loads
> state from "save", not from "save_cached".

I was gonna say let's just pass save_cached to svm_copy_vmrun_state()
instead of "save", but then I realized the SMM Code also uses it. We
can make it work for both (macro time!), but maybe that's not worth
doing. Sigh.

> And even if we fixed that, it would
> then allow userspace to pass in garbage (that is then ignored), i.e. would undo
> patch 1, and I don't want to do that.

I don't think so, patch 1 specifically performs the check on "save".