All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Yosry Ahmed <yosry@kernel.org>
Cc: Kevin Cheng <chengkev@google.com>,
	pbonzini@redhat.com, kvm@vger.kernel.org,
	 linux-kernel@vger.kernel.org
Subject: Re: [PATCH V4 0/4] Align SVM with APM defined behaviors
Date: Mon, 2 Mar 2026 15:17:48 -0800	[thread overview]
Message-ID: <aaYanA9WBSZWjQ8Y@google.com> (raw)
In-Reply-To: <aaXXs4ubgmxf_E1O@google.com>

On Mon, Mar 02, 2026, Sean Christopherson wrote:
> On Mon, Mar 02, 2026, Yosry Ahmed wrote:
> > On Fri, Feb 27, 2026 at 7:33 PM Kevin Cheng <chengkev@google.com> wrote:
> > >
> > > The APM lists the following behaviors
> > >   - The VMRUN, VMLOAD, VMSAVE, CLGI, VMMCALL, and INVLPGA instructions
> > >     can be used when the EFER.SVME is set to 1; otherwise, these
> > >     instructions generate a #UD exception.
> > >   - If VMMCALL instruction is not intercepted, the instruction raises a
> > >     #UD exception.
> > >
> > > The patches in this series fix current SVM bugs that do not adhere to
> > > the APM listed behaviors.
> > >
> > > v3 -> v4:
> > >   - Dropped "KVM: SVM: Inject #UD for STGI if EFER.SVME=0 and SVM Lock
> > >     and DEV are not available" as per Sean
> > >   - Added back STGI and CLGI intercept clearing in init_vmcb to maintain
> > >     previous behavior on intel guests. Previously intel guests always
> > >     had STGI and CLGI intercepts cleared if vgif was enabled. In V3,
> > >     because the clearing of the intercepts was moved from init_vmcb() to
> > >     the !guest_cpuid_is_intel_compatible() case in
> > >     svm_recalc_instruction_intercepts(), the CLGI intercept would be
> > >     indefinitely set on intel guests. I added back the clearing to
> > >     init_vmcb() to retain intel guest behavior before this patch.
> > 
> > I am a bit confused by this. v4 kept initializing the intercepts as
> > cleared for all guests, but we still set the CLGI/STGI intercepts for
> > Intel-compatible guests in svm_recalc_instruction_intercepts() patch
> > 3. So what difference did this make?
> > 
> > Also taking a step back, I am not really sure what's the right thing
> > to do for Intel-compatible guests here. It also seems like even if we
> > set the intercept, svm_set_gif() will clear the STGI intercept, even
> > on Intel-compatible guests.
> > 
> > Maybe we should leave that can of worms alone, go back to removing
> > initializing the CLGI/STGI intercepts in init_vmcb(), and in
> > svm_recalc_instruction_intercepts() set/clear these intercepts based
> > on EFER.SVME alone, irrespective of Intel-compatibility?
> 
> Ya, guest_cpuid_is_intel_compatible() should only be applied to VMLOAD/VMSAVE.
> KVM intercepts VMLOAD/VMSAVE to fixup SYSENTER MSRs, not to inject #UD.  I.e. KVM
> is handling (the absoutely absurd) case that FMS reports an Intel CPU, but the
> guest enables and uses SVM.
> 
> 	/*
> 	 * Intercept VMLOAD if the vCPU model is Intel in order to emulate that
> 	 * VMLOAD drops bits 63:32 of SYSENTER (ignoring the fact that exposing
> 	 * SVM on Intel is bonkers and extremely unlikely to work).
> 	 */
> 	if (guest_cpuid_is_intel_compatible(vcpu))
> 		guest_cpu_cap_clear(vcpu, X86_FEATURE_V_VMSAVE_VMLOAD);
> 
> Sorry for not catching this in previous versions.

Because I got all kinds of confused trying to recall what was different between
v3 and v4, I went ahead and spliced them together.

Does the below look right?  If so, I'll formally post just patches 1 and 3 as v5.
I'll take 2 and 4 directly from here; I want to switch the ordering anyways so
that the vgif movement immediately precedes the Recalc "instructions" patch.

	/*
	 * Intercept instructions that #UD if EFER.SVME=0, as SVME must be set
	 * even when running the guest, i.e. hardware will only ever see
	 * EFER.SVME=1.
	 *
	 * No need to toggle any of the vgif/vls/etc. enable bits here, as they
	 * are set when the VMCB is initialized and never cleared (if the
	 * relevant intercepts are set, the enablements are meaningless anyway).
	 */
	if (!(vcpu->arch.efer & EFER_SVME)) {
		svm_set_intercept(svm, INTERCEPT_VMLOAD);
		svm_set_intercept(svm, INTERCEPT_VMSAVE);
		svm_set_intercept(svm, INTERCEPT_CLGI);
		svm_set_intercept(svm, INTERCEPT_STGI);
	} else {
		/*
		 * If hardware supports Virtual VMLOAD VMSAVE then enable it
		 * in VMCB and clear intercepts to avoid #VMEXIT.
		 */
		if (guest_cpuid_is_intel_compatible(vcpu)) {
			svm_set_intercept(svm, INTERCEPT_VMLOAD);
			svm_set_intercept(svm, INTERCEPT_VMSAVE);
		} else if (vls) {
			svm_clr_intercept(svm, INTERCEPT_VMLOAD);
			svm_clr_intercept(svm, INTERCEPT_VMSAVE);
		}

		/*
		 * Process pending events when clearing STGI/CLGI intercepts if
		 * there's at least one pending event that is masked by GIF, so
		 * that KVM re-evaluates if the intercept needs to be set again
		 * to track when GIF is re-enabled (e.g. for NMI injection).
		 */
		if (vgif) {
			svm_clr_intercept(svm, INTERCEPT_CLGI);
			svm_clr_intercept(svm, INTERCEPT_STGI);

			if (svm_has_pending_gif_event(svm))
				kvm_make_request(KVM_REQ_EVENT, &svm->vcpu);
		}
	}

where init_vmcb() is (like v3):

	if (vnmi)
		svm->vmcb->control.int_ctl |= V_NMI_ENABLE_MASK;

	if (vgif)
		svm->vmcb->control.int_ctl |= V_GIF_ENABLE_MASK;

	if (vls)
		svm->vmcb->control.virt_ext |= VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK;

  reply	other threads:[~2026-03-02 23:17 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-28  3:33 [PATCH V4 0/4] Align SVM with APM defined behaviors Kevin Cheng
2026-02-28  3:33 ` [PATCH V4 1/4] KVM: SVM: Move STGI and CLGI intercept handling Kevin Cheng
2026-02-28  3:33 ` [PATCH V4 2/4] KVM: SVM: Inject #UD for INVLPGA if EFER.SVME=0 Kevin Cheng
2026-02-28  3:33 ` [PATCH V4 3/4] KVM: SVM: Recalc instructions intercepts when EFER.SVME is toggled Kevin Cheng
2026-02-28  3:33 ` [PATCH V4 4/4] KVM: SVM: Raise #UD if VMMCALL instruction is not intercepted Kevin Cheng
2026-03-03  2:22   ` Sean Christopherson
2026-03-03  9:36     ` Vitaly Kuznetsov
2026-03-02 16:21 ` [PATCH V4 0/4] Align SVM with APM defined behaviors Yosry Ahmed
2026-03-02 18:32   ` Sean Christopherson
2026-03-02 23:17     ` Sean Christopherson [this message]
2026-03-03  0:34       ` Sean Christopherson
2026-03-03 21:56         ` Kevin Cheng
2026-03-03 22:08           ` Sean Christopherson
2026-03-03 22:27             ` Kevin Cheng
2026-03-03 21:52       ` Kevin Cheng
2026-03-03 21:48   ` Kevin Cheng
2026-03-05 17:08 ` 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=aaYanA9WBSZWjQ8Y@google.com \
    --to=seanjc@google.com \
    --cc=chengkev@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.