linux-kernel - Re: [PATCH V2 1/5] KVM: SVM: Move STGI and CLGI intercept handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aWhFQcNa8SKd679a@google.com>
Date: Wed, 14 Jan 2026 17:39:13 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Yosry Ahmed <yosry.ahmed@...ux.dev>
Cc: Kevin Cheng <chengkev@...gle.com>, pbonzini@...hat.com, kvm@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2 1/5] KVM: SVM: Move STGI and CLGI intercept handling

On Mon, Jan 12, 2026, Yosry Ahmed wrote:
> On Mon, Jan 12, 2026 at 05:45:31PM +0000, Kevin Cheng wrote:
> > Similar to VMLOAD/VMSAVE intercept handling, move the STGI/CLGI
> > intercept handling to svm_recalc_instruction_intercepts().
> > ---
> >  arch/x86/kvm/svm/svm.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 24d59ccfa40d9..6373a25d85479 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -1010,6 +1010,11 @@ static void svm_recalc_instruction_intercepts(struct kvm_vcpu *vcpu)
> >  			svm_clr_intercept(svm, INTERCEPT_VMSAVE);
> >  			svm->vmcb->control.virt_ext |= VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK;
> >  		}
> > +
> > +		if (vgif) {
> > +			svm_clr_intercept(svm, INTERCEPT_STGI);
> 
> Could this cause a problem with NMI window tracking?

Yes.

> svm_enable_nmi_window() sets INTERCEPT_STGI to detect when NMIs are
> enabled, and it's later cleared by svm_set_gif(). If we recalc
> intercepts in between we will clear the intercept here and miss NMI
> enablement.
> 
> We could move the logic to set/clear INTERCEPT_STGI for NMI window
> tracking here as well, but then we'll need to recalc intercepts in
> svm_enable_nmi_window() and svm_set_gif(), which could be expensive.
> 
> The alternative is perhaps setting a flag when INTERCEPT_STGI is set in
> svm_enable_nmi_window() and avoid clearing the intercept here if the
> flag is set.
> 
> Not sure what's the best way forward here.

First things first, the changelog needs to state _why_ the code is being moved.
"To be like VMLOAD/VMSAVE" doesn't suffice, because my initial answer was going
to be "well just don't move the code" (I already forgot the context of v1).

But moving the code is needed to fix the missing #UD in "Recalc instructions
intercepts when EFER.SVME is toggled".

As for how to fix this, a few ideas:

 1. Set KVM_REQ_EVENT to force KVM to re-evulate all events.  kvm_check_and_inject_events()
    will see the pending NMI and/or SMI, that the NMI/SMI is not allowed, and
    re-call enable_{nmi,smi}_window().

 2. Manually check for pending+blocked NMI/SMIs.

 3. Combine parts of #1 and #2.  Set KVM_REQ_EVENT, but only if there's a pending
    NMI or SMI.

 4. Add flags to vcpu_svm to explicitly track if a vCPU has an NMI/SMI window,
    similar to what we're planning on doing for IRQs[*], and use that to more
    confidently do the right thing when recomputing intercepts.

I don't love any of those ideas.  Ah, at least not until I poke around KVM.  In
svm_set_gif() there's already this:

		if (svm->vcpu.arch.smi_pending ||
		    svm->vcpu.arch.nmi_pending ||
		    kvm_cpu_has_injectable_intr(&svm->vcpu) ||
		    kvm_apic_has_pending_init_or_sipi(&svm->vcpu))
			kvm_make_request(KVM_REQ_EVENT, &svm->vcpu);

So I think it makes sense to bundle that into a helper, e.g. (no idea what to
call it)

static bool svm_think_of_a_good_name(struct kvm_vcpu *vcpu)
{
	if (svm->vcpu.arch.smi_pending ||
	    svm->vcpu.arch.nmi_pending ||
	    kvm_cpu_has_injectable_intr(&svm->vcpu) ||
	    kvm_apic_has_pending_init_or_sipi(&svm->vcpu))
		kvm_make_request(KVM_REQ_EVENT, &svm->vcpu);
}

And then call that from svm_recalc_instruction_intercepts().  That implements #3
in a fairly maintainable way (we'll hopefully notice sooner than later if we break
svm_set_gif()).

https://lore.kernel.org/all/26732815475bf1c5ba672bc3b1785265f1a994e6.1752819570.git.naveen@kernel.org

> > +			svm_clr_intercept(svm, INTERCEPT_CLGI);
> > +		}
> >  	}
> >  }
> >  
> > @@ -1147,11 +1152,8 @@ static void init_vmcb(struct kvm_vcpu *vcpu, bool init_event)
> >  	if (vnmi)
> >  		svm->vmcb->control.int_ctl |= V_NMI_ENABLE_MASK;
> >  
> > -	if (vgif) {
> > -		svm_clr_intercept(svm, INTERCEPT_STGI);
> > -		svm_clr_intercept(svm, INTERCEPT_CLGI);
> > +	if (vgif)
> >  		svm->vmcb->control.int_ctl |= V_GIF_ENABLE_MASK;
> > -	}
> >  
> >  	if (vcpu->kvm->arch.bus_lock_detection_enabled)
> >  		svm_set_intercept(svm, INTERCEPT_BUSLOCK);
> > -- 
> > 2.52.0.457.g6b5491de43-goog
> >