All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Yosry Ahmed <yosry@kernel.org>
Cc: Jim Mattson <jmattson@google.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org,  linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/6] KVM: SVM: Use maxphyaddr in emulator RAX check for VMRUN/VMLOAD/VMSAVE
Date: Wed, 11 Mar 2026 18:38:55 -0700	[thread overview]
Message-ID: <abIZL9ixxKvSjdg8@google.com> (raw)
In-Reply-To: <CAO9r8zN=yryp1F8tJDWKasAe8KO30tWH-A98Uy=EhFn8Lg6WcQ@mail.gmail.com>

On Wed, Mar 11, 2026, Yosry Ahmed wrote:
> > > Hold up, we're getting ahead of ourselves.
> > >
> > > The only legitimate reason the emulator is at all aware of VMSAVE, VMLOAD, and
> > > VMRUN is to deal with #GP due to the RAX check, because hardware checks the GPA
> > > against the host's physical address space.  See commit 82a11e9c6fa2 ("KVM: SVM:
> > > Add emulation support for #GP triggered by SVM instructions").
> > >
> > > The emulator "support" was originally added by commit 01de8b09e606 ("KVM: SVM:
> > > Add intercept checks for SVM instructions"), but AFAICT, for all intents and
> > > purposes that was dead code when it was added, because the emulator doesn't
> > > actually _emulate_ the instructions.  I assume if they aren't intercepted, and
> > > KVM is full on emulating instead of just decoding, they end up at EMULATION_FAILED
> > > and get a #UD or something.
> > >
> > > Outside of forced emulation or code stream rewriting, KVM should _never_ fully
> > > emulate any of the SVM instructions except VMMCALL (and that is a super special
> > > case).  KVM does need to _decode_ the instruction, and it needs to get the
> > > pre-intercept exception checks correct so that KVM correctly injects e.g. #GP
> > > instead of synthesizing a #VMEXIT for the CPL check, but KVM doesn't need to do
> > > *all* of the checks.
> > >
> > > Note, for L2, the SVME check is meaningless, as EFER.SVME has to be set for L2
> > > to be active, i.e. it's L1's responsibility to handle that check.
> > >
> > > Back to the physical address thing, KVM _already_ handles that check in the #GP
> > > path,
> >
> > I guess if KVM is not intercepting #GP, then the hardware injects the
> > #GP and the emulator still doesn't have to worry about it -- because
> > we don't support the case where RAX can be legal from the host's
> > perspective but not the guest's. Makes sense.
> >
> > > it's just wrong too:
> > >
> > >                 /* All SVM instructions expect page aligned RAX */
> > >                 if (svm->vmcb->save.rax & ~PAGE_MASK)
> > >                         goto reinject;
> > >
> > > So I think what we want is to
> > >
> > >   (a) fix the RAX check in gp_interception()
> > >   (b) drop the RAX check in the emulator
> > >   (c) add a CPL check in the emulator (because the intercepted #GP could have
> > >       been due to L2 executing at CPL>0, not due to a bad-but-good RAX).
> 
> Actually, I don't think (c) is needed. In the path where KVM
> intercepts #GP, it doesn't go through the emulation path which ends up
> calling check_svme(), it only uses the emulator to decode the
> instruction.

Oh, dagnabbit, that's stupidly obvious in hindsight.

> AFAICT, we can end up in the emulator only when the CPU does not
> produce a #GP, e.g. when we get a #NPF on the address in RAX. In this
> case, the CPU will have already checked the CPL for us, and the
> validity of the address. The emulator checking EFER.SVME check is
> probably also useless, because with Kevin's patches we should always
> be intercepting VMLOAD/VMSAVE when EFER.SVME is disabled by the guest
> and checking EFER.SVME there anyway.
> 
> Anyway, I want to touch the emulator as little as possible tbh, so I
> will still do (b) because it unblocks this series (removes the wrong
> GPA check that injects #GP), but will defer any further cleanups.

Yeah, works for me.

  reply	other threads:[~2026-03-12  1:38 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-06 21:08 [PATCH v2 0/6] KVM: nSVM: Fix vmcb12 mapping failure handling Yosry Ahmed
2026-03-06 21:08 ` [PATCH v2 1/6] KVM: SVM: Use maxphyaddr in emulator RAX check for VMRUN/VMLOAD/VMSAVE Yosry Ahmed
2026-03-06 22:27   ` Jim Mattson
2026-03-06 22:37     ` Yosry Ahmed
2026-03-06 23:12       ` Jim Mattson
2026-03-06 23:20         ` Yosry Ahmed
2026-03-06 23:45           ` Jim Mattson
2026-03-07  0:32           ` Sean Christopherson
2026-03-11 18:31             ` Yosry Ahmed
2026-03-11 20:07               ` Yosry Ahmed
2026-03-11 20:39                 ` Sean Christopherson
2026-03-11 20:50                   ` Yosry Ahmed
2026-03-11 23:01                     ` Sean Christopherson
2026-03-11 23:22                       ` Yosry Ahmed
2026-03-12  1:27                         ` Yosry Ahmed
2026-03-12  1:38                           ` Sean Christopherson [this message]
2026-03-12 15:50                       ` Yosry Ahmed
2026-03-12 15:54                         ` Sean Christopherson
2026-03-12 16:19                           ` Yosry Ahmed
2026-03-07  0:28         ` Sean Christopherson
2026-03-07  0:31           ` Yosry Ahmed
2026-03-06 21:08 ` [PATCH v2 2/6] KVM: nSVM: Simplify error handling of nested_svm_copy_vmcb12_to_cache() Yosry Ahmed
2026-03-12 18:13   ` Sean Christopherson
2026-03-12 21:01     ` Yosry Ahmed
2026-03-06 21:08 ` [PATCH v2 3/6] KVM: SVM: Treat mapping failures equally in VMLOAD/VMSAVE emulation Yosry Ahmed
2026-03-06 21:08 ` [PATCH v2 4/6] KVM: nSVM: Fail emulation of VMRUN/VMLOAD/VMSAVE if mapping vmcb12 fails Yosry Ahmed
2026-03-07  1:09   ` Yosry Ahmed
2026-03-09 13:56     ` Yosry Ahmed
2026-03-06 21:08 ` [PATCH v2 5/6] KVM: selftests: Rework svm_nested_invalid_vmcb12_gpa Yosry Ahmed
2026-03-06 21:09 ` [PATCH v2 6/6] KVM: selftests: Drop 'invalid' from svm_nested_invalid_vmcb12_gpa's name Yosry Ahmed
2026-04-03 15:13 ` [PATCH v2 0/6] KVM: nSVM: Fix vmcb12 mapping failure handling 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=abIZL9ixxKvSjdg8@google.com \
    --to=seanjc@google.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=yosry@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.