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

[Sean Christopherson <seanjc@google.com>]

On Wed, Sep 02, 2026, Yosry Ahmed wrote:
> On Wed, Sep 2, 2026 at 5:02 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Wed, Sep 02, 2026, Yosry Ahmed wrote:
> > > On Wed, Sep 2, 2026 at 4:20 PM Sean Christopherson <seanjc@google.com> 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@vger.kernel.org
> > > > Cc: Yosry Ahmed <yosry@kernel.org>
> > > > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > > > ---
> > > >  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.

IMO, not worth doing.  At least, not for an immediate fix.

> > 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".

True, I was thinking that we'd want to omit the massaged/sanitized state, but
that's obviously not a requirement.