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 for Android: free password hash cracker in your pocket

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

Message-ID: <aWkdF8gz1IDssQOd@google.com>
Date: Thu, 15 Jan 2026 09:00:07 -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 Thu, Jan 15, 2026, Yosry Ahmed wrote:
> On Wed, Jan 14, 2026 at 05:39:13PM -0800, Sean Christopherson wrote:
> > On Mon, Jan 12, 2026, Yosry Ahmed wrote:
> > 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);
> > }
> 
> Maybe svm_check_gif_events() or svm_check_gif_interrupts()?
> 
> Or maybe it's clearer if we just put the checks in a helper like
> svm_waiting_for_gif() or svm_pending_gif_interrupt().

This was my first idea as well, though I would name it svm_has_pending_gif_event()
to better align with kvm_vcpu_has_events().

I suggested a single helper because I don't love that how to react to the pending
event is duplicated.  But I definitely don't object to open coding the request if
the consensus is that it's more readable overall.

> Then in svm_recalc_instruction_intercepts() we do:
> 
> 	/*
> 	 * If there is a pending interrupt controlled by GIF, set
> 	 * KVM_REQ_EVENT to re-evaluate if the intercept needs to be set
> 	 * again to track when GIF is re-enabled (e.g. for NMI
> 	 * injection).
> 	 */
> 	svm_clr_intercept(svm, INTERCEPT_STGI);	
> 	if (svm_pending_gif_interrupt())
> 		kvm_make_request(KVM_REQ_EVENT, &svm->vcpu);
> 
> and in svm_set_gif() it reads well semantically:
> 	
> 	enable_gif(svm);
> 	if (svm_pending_gif_interrupt())
> 		kvm_make_request(KVM_REQ_EVENT, &svm->vcpu);