[PATCH 0/3] nSVM: Minor cleanups for intercepts code

[PATCH 0/3] nSVM: Minor cleanups for intercepts code

[Yosry Ahmed]

A few minor cleanups for nested intercepts code, namely making
recalc_intercepts() more readable and renaming it, and using
vmcb12_is_intercept() instead of open-coding it.

Yosry Ahmed (3):
  KVM: nSVM: Use intuitive local variables in recalc_intercepts()
  KVM: nSVM: Rename recalc_intercepts() to clarify vmcb02 as the target
  KVM: nSVM: Use vmcb12_is_intercept() in
    nested_sync_control_from_vmcb02()

 arch/x86/kvm/svm/nested.c | 37 ++++++++++++++++++-------------------
 arch/x86/kvm/svm/sev.c    |  2 +-
 arch/x86/kvm/svm/svm.c    |  4 ++--
 arch/x86/kvm/svm/svm.h    | 10 +++++-----
 4 files changed, 26 insertions(+), 27 deletions(-)

-- 
2.52.0.457.g6b5491de43-goog

[PATCH 1/3] KVM: nSVM: Use intuitive local variables in recalc_intercepts()

[Yosry Ahmed]

recalc_intercepts() currently uses c, h, g as local variables for the
control area of the current VMCB, vmcb01, and (cached) vmcb12.

The current VMCB should always be vmcb02 when recalc_intercepts() is
executed in guest mode. Use vmcb01/vmcb02 local variables instead to
make it clear the function is updating intercepts in vmcb02 based on the
intercepts in vmcb01 and (cached) vmcb12.

Add a WARNING() if the current VMCB is not in fact vmcb02.

No functional change intended.

Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
---
 arch/x86/kvm/svm/nested.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index f295a41ec659..2dda52221fd8 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -125,8 +125,7 @@ static bool nested_vmcb_needs_vls_intercept(struct vcpu_svm *svm)
 
 void recalc_intercepts(struct vcpu_svm *svm)
 {
-	struct vmcb_control_area *c, *h;
-	struct vmcb_ctrl_area_cached *g;
+	struct vmcb *vmcb01, *vmcb02;
 	unsigned int i;
 
 	vmcb_mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
@@ -134,14 +133,14 @@ void recalc_intercepts(struct vcpu_svm *svm)
 	if (!is_guest_mode(&svm->vcpu))
 		return;
 
-	c = &svm->vmcb->control;
-	h = &svm->vmcb01.ptr->control;
-	g = &svm->nested.ctl;
+	vmcb01 = svm->vmcb01.ptr;
+	vmcb02 = svm->nested.vmcb02.ptr;
+	WARN_ON_ONCE(svm->vmcb != vmcb02);
 
 	for (i = 0; i < MAX_INTERCEPT; i++)
-		c->intercepts[i] = h->intercepts[i];
+		vmcb02->control.intercepts[i] = vmcb01->control.intercepts[i];
 
-	if (g->int_ctl & V_INTR_MASKING_MASK) {
+	if (svm->nested.ctl.int_ctl & V_INTR_MASKING_MASK) {
 		/*
 		 * If L2 is active and V_INTR_MASKING is enabled in vmcb12,
 		 * disable intercept of CR8 writes as L2's CR8 does not affect
@@ -152,9 +151,9 @@ void recalc_intercepts(struct vcpu_svm *svm)
 		 * the effective RFLAGS.IF for L1 interrupts will never be set
 		 * while L2 is running (L2's RFLAGS.IF doesn't affect L1 IRQs).
 		 */
-		vmcb_clr_intercept(c, INTERCEPT_CR8_WRITE);
-		if (!(svm->vmcb01.ptr->save.rflags & X86_EFLAGS_IF))
-			vmcb_clr_intercept(c, INTERCEPT_VINTR);
+		vmcb_clr_intercept(&vmcb02->control, INTERCEPT_CR8_WRITE);
+		if (!(vmcb01->save.rflags & X86_EFLAGS_IF))
+			vmcb_clr_intercept(&vmcb02->control, INTERCEPT_VINTR);
 	}
 
 	/*
@@ -162,14 +161,14 @@ void recalc_intercepts(struct vcpu_svm *svm)
 	 * flush feature is enabled.
 	 */
 	if (!nested_svm_l2_tlb_flush_enabled(&svm->vcpu))
-		vmcb_clr_intercept(c, INTERCEPT_VMMCALL);
+		vmcb_clr_intercept(&vmcb02->control, INTERCEPT_VMMCALL);
 
 	for (i = 0; i < MAX_INTERCEPT; i++)
-		c->intercepts[i] |= g->intercepts[i];
+		vmcb02->control.intercepts[i] |= svm->nested.ctl.intercepts[i];
 
 	/* If SMI is not intercepted, ignore guest SMI intercept as well  */
 	if (!intercept_smi)
-		vmcb_clr_intercept(c, INTERCEPT_SMI);
+		vmcb_clr_intercept(&vmcb02->control, INTERCEPT_SMI);
 
 	if (nested_vmcb_needs_vls_intercept(svm)) {
 		/*
@@ -177,10 +176,10 @@ void recalc_intercepts(struct vcpu_svm *svm)
 		 * we must intercept these instructions to correctly
 		 * emulate them in case L1 doesn't intercept them.
 		 */
-		vmcb_set_intercept(c, INTERCEPT_VMLOAD);
-		vmcb_set_intercept(c, INTERCEPT_VMSAVE);
+		vmcb_set_intercept(&vmcb02->control, INTERCEPT_VMLOAD);
+		vmcb_set_intercept(&vmcb02->control, INTERCEPT_VMSAVE);
 	} else {
-		WARN_ON(!(c->virt_ext & VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK));
+		WARN_ON(!(vmcb02->control.virt_ext & VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK));
 	}
 }
 
-- 
2.52.0.457.g6b5491de43-goog

Re: [PATCH 1/3] KVM: nSVM: Use intuitive local variables in recalc_intercepts()

[Sean Christopherson]

On Mon, Jan 12, 2026, Yosry Ahmed wrote:
> recalc_intercepts() currently uses c, h, g as local variables for the
> control area of the current VMCB, vmcb01, and (cached) vmcb12.
> 
> The current VMCB should always be vmcb02 when recalc_intercepts() is
> executed in guest mode. Use vmcb01/vmcb02 local variables instead to
> make it clear the function is updating intercepts in vmcb02 based on the
> intercepts in vmcb01 and (cached) vmcb12.
> 
> Add a WARNING() if the current VMCB is not in fact vmcb02.

This belongs in a separate patch.

> No functional change intended.
> 
> Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> ---
>  arch/x86/kvm/svm/nested.c | 31 +++++++++++++++----------------
>  1 file changed, 15 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index f295a41ec659..2dda52221fd8 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -125,8 +125,7 @@ static bool nested_vmcb_needs_vls_intercept(struct vcpu_svm *svm)
>  
>  void recalc_intercepts(struct vcpu_svm *svm)
>  {
> -	struct vmcb_control_area *c, *h;
> -	struct vmcb_ctrl_area_cached *g;
> +	struct vmcb *vmcb01, *vmcb02;
>  	unsigned int i;
>  
>  	vmcb_mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
> @@ -134,14 +133,14 @@ void recalc_intercepts(struct vcpu_svm *svm)
>  	if (!is_guest_mode(&svm->vcpu))
>  		return;
>  
> -	c = &svm->vmcb->control;
> -	h = &svm->vmcb01.ptr->control;
> -	g = &svm->nested.ctl;
> +	vmcb01 = svm->vmcb01.ptr;
> +	vmcb02 = svm->nested.vmcb02.ptr;
> +	WARN_ON_ONCE(svm->vmcb != vmcb02);

If we're going to bother with a WARN, then this code should definitely bail,
because configuring vmcb01 using the nested logic is all but guaranteed to break
L1 in weird ways.

>  	for (i = 0; i < MAX_INTERCEPT; i++)
> -		c->intercepts[i] = h->intercepts[i];
> +		vmcb02->control.intercepts[i] = vmcb01->control.intercepts[i];
>  
> -	if (g->int_ctl & V_INTR_MASKING_MASK) {
> +	if (svm->nested.ctl.int_ctl & V_INTR_MASKING_MASK) {

I vote to keep a pointer to the cached control as vmcb12_ctrl.  Coming from a
nVMX-focused background, I can never remember what svm->nested.ctl holds.  For
me, this is waaaay more intuivite:

	if (vmcb12_ctrl->int_ctl & V_INTR_MASKING_MASK) {

>  	for (i = 0; i < MAX_INTERCEPT; i++)
> -		c->intercepts[i] |= g->intercepts[i];
> +		vmcb02->control.intercepts[i] |= svm->nested.ctl.intercepts[i];

And even more so here:

	for (i = 0; i < MAX_INTERCEPT; i++)
		vmcb02->control.intercepts[i] |= vmcb12_ctrl->intercepts[i];

>  
>  	/* If SMI is not intercepted, ignore guest SMI intercept as well  */
>  	if (!intercept_smi)
> -		vmcb_clr_intercept(c, INTERCEPT_SMI);
> +		vmcb_clr_intercept(&vmcb02->control, INTERCEPT_SMI);
>  
>  	if (nested_vmcb_needs_vls_intercept(svm)) {
>  		/*
> @@ -177,10 +176,10 @@ void recalc_intercepts(struct vcpu_svm *svm)
>  		 * we must intercept these instructions to correctly
>  		 * emulate them in case L1 doesn't intercept them.
>  		 */
> -		vmcb_set_intercept(c, INTERCEPT_VMLOAD);
> -		vmcb_set_intercept(c, INTERCEPT_VMSAVE);
> +		vmcb_set_intercept(&vmcb02->control, INTERCEPT_VMLOAD);
> +		vmcb_set_intercept(&vmcb02->control, INTERCEPT_VMSAVE);
>  	} else {
> -		WARN_ON(!(c->virt_ext & VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK));
> +		WARN_ON(!(vmcb02->control.virt_ext & VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK));

Opportunistically switch this to WARN_ON_ONCE.  Any "unguarded" WARN in KVM
(outside of e.g. __init code) is just asking for a self-DoS.

>  	}
>  }
>  
> -- 
> 2.52.0.457.g6b5491de43-goog
> 

Re: [PATCH 1/3] KVM: nSVM: Use intuitive local variables in recalc_intercepts()

[Yosry Ahmed]

On Wed, Feb 04, 2026 at 09:29:36AM -0800, Sean Christopherson wrote:
> On Mon, Jan 12, 2026, Yosry Ahmed wrote:
> > recalc_intercepts() currently uses c, h, g as local variables for the
> > control area of the current VMCB, vmcb01, and (cached) vmcb12.
> > 
> > The current VMCB should always be vmcb02 when recalc_intercepts() is
> > executed in guest mode. Use vmcb01/vmcb02 local variables instead to
> > make it clear the function is updating intercepts in vmcb02 based on the
> > intercepts in vmcb01 and (cached) vmcb12.
> > 
> > Add a WARNING() if the current VMCB is not in fact vmcb02.
> 
> This belongs in a separate patch.
> 
> > No functional change intended.
> > 
> > Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> > ---
> >  arch/x86/kvm/svm/nested.c | 31 +++++++++++++++----------------
> >  1 file changed, 15 insertions(+), 16 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index f295a41ec659..2dda52221fd8 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -125,8 +125,7 @@ static bool nested_vmcb_needs_vls_intercept(struct vcpu_svm *svm)
> >  
> >  void recalc_intercepts(struct vcpu_svm *svm)
> >  {
> > -	struct vmcb_control_area *c, *h;
> > -	struct vmcb_ctrl_area_cached *g;
> > +	struct vmcb *vmcb01, *vmcb02;
> >  	unsigned int i;
> >  
> >  	vmcb_mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
> > @@ -134,14 +133,14 @@ void recalc_intercepts(struct vcpu_svm *svm)
> >  	if (!is_guest_mode(&svm->vcpu))
> >  		return;
> >  
> > -	c = &svm->vmcb->control;
> > -	h = &svm->vmcb01.ptr->control;
> > -	g = &svm->nested.ctl;
> > +	vmcb01 = svm->vmcb01.ptr;
> > +	vmcb02 = svm->nested.vmcb02.ptr;
> > +	WARN_ON_ONCE(svm->vmcb != vmcb02);
> 
> If we're going to bother with a WARN, then this code should definitely bail,
> because configuring vmcb01 using the nested logic is all but guaranteed to break
> L1 in weird ways.

I can put the WARN + bail in a separate patch.

> 
> >  	for (i = 0; i < MAX_INTERCEPT; i++)
> > -		c->intercepts[i] = h->intercepts[i];
> > +		vmcb02->control.intercepts[i] = vmcb01->control.intercepts[i];
> >  
> > -	if (g->int_ctl & V_INTR_MASKING_MASK) {
> > +	if (svm->nested.ctl.int_ctl & V_INTR_MASKING_MASK) {
> 
> I vote to keep a pointer to the cached control as vmcb12_ctrl.  Coming from a
> nVMX-focused background, I can never remember what svm->nested.ctl holds.  For
> me, this is waaaay more intuivite:

I agree it reads better, but honestly all of nSVM code uses
svm->nested.ctl, and changing its name here just makes things
inconsistent imo.

> 
> 	if (vmcb12_ctrl->int_ctl & V_INTR_MASKING_MASK) {
> 
> >  	for (i = 0; i < MAX_INTERCEPT; i++)
> > -		c->intercepts[i] |= g->intercepts[i];
> > +		vmcb02->control.intercepts[i] |= svm->nested.ctl.intercepts[i];
> 
> And even more so here:
> 
> 	for (i = 0; i < MAX_INTERCEPT; i++)
> 		vmcb02->control.intercepts[i] |= vmcb12_ctrl->intercepts[i];
> 
> >  
> >  	/* If SMI is not intercepted, ignore guest SMI intercept as well  */
> >  	if (!intercept_smi)
> > -		vmcb_clr_intercept(c, INTERCEPT_SMI);
> > +		vmcb_clr_intercept(&vmcb02->control, INTERCEPT_SMI);
> >  
> >  	if (nested_vmcb_needs_vls_intercept(svm)) {
> >  		/*
> > @@ -177,10 +176,10 @@ void recalc_intercepts(struct vcpu_svm *svm)
> >  		 * we must intercept these instructions to correctly
> >  		 * emulate them in case L1 doesn't intercept them.
> >  		 */
> > -		vmcb_set_intercept(c, INTERCEPT_VMLOAD);
> > -		vmcb_set_intercept(c, INTERCEPT_VMSAVE);
> > +		vmcb_set_intercept(&vmcb02->control, INTERCEPT_VMLOAD);
> > +		vmcb_set_intercept(&vmcb02->control, INTERCEPT_VMSAVE);
> >  	} else {
> > -		WARN_ON(!(c->virt_ext & VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK));
> > +		WARN_ON(!(vmcb02->control.virt_ext & VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK));
> 
> Opportunistically switch this to WARN_ON_ONCE.  Any "unguarded" WARN in KVM
> (outside of e.g. __init code) is just asking for a self-DoS.

Will do.

> 
> >  	}
> >  }
> >  
> > -- 
> > 2.52.0.457.g6b5491de43-goog
> > 

Re: [PATCH 1/3] KVM: nSVM: Use intuitive local variables in recalc_intercepts()

[Sean Christopherson]

On Wed, Feb 04, 2026, Yosry Ahmed wrote:
> On Wed, Feb 04, 2026 at 09:29:36AM -0800, Sean Christopherson wrote:
> > 
> > >  	for (i = 0; i < MAX_INTERCEPT; i++)
> > > -		c->intercepts[i] = h->intercepts[i];
> > > +		vmcb02->control.intercepts[i] = vmcb01->control.intercepts[i];
> > >  
> > > -	if (g->int_ctl & V_INTR_MASKING_MASK) {
> > > +	if (svm->nested.ctl.int_ctl & V_INTR_MASKING_MASK) {
> > 
> > I vote to keep a pointer to the cached control as vmcb12_ctrl.  Coming from a
> > nVMX-focused background, I can never remember what svm->nested.ctl holds.  For
> > me, this is waaaay more intuivite:
> 
> I agree it reads better, but honestly all of nSVM code uses svm->nested.ctl,
> and changing its name here just makes things inconsistent imo.

Gotta start somewhere :-)  In all seriousness, if we didn't allow chipping away
to at historical oddities in KVM, the code base would be a disaster.  I'm all for
prioritizing consistency, but I draw the line at "everything else sucks, so this
needs to suck too".

I'm not saying we need to do a wholesale rename, but giving at least
nested_vmcb02_prepare_control() the same treatment will be a huge improvement.
Actually, I'm going to go do that right now...

Re: [PATCH 1/3] KVM: nSVM: Use intuitive local variables in recalc_intercepts()

[Yosry Ahmed]

On Wed, Feb 04, 2026 at 09:55:36AM -0800, Sean Christopherson wrote:
> On Wed, Feb 04, 2026, Yosry Ahmed wrote:
> > On Wed, Feb 04, 2026 at 09:29:36AM -0800, Sean Christopherson wrote:
> > > 
> > > >  	for (i = 0; i < MAX_INTERCEPT; i++)
> > > > -		c->intercepts[i] = h->intercepts[i];
> > > > +		vmcb02->control.intercepts[i] = vmcb01->control.intercepts[i];
> > > >  
> > > > -	if (g->int_ctl & V_INTR_MASKING_MASK) {
> > > > +	if (svm->nested.ctl.int_ctl & V_INTR_MASKING_MASK) {
> > > 
> > > I vote to keep a pointer to the cached control as vmcb12_ctrl.  Coming from a
> > > nVMX-focused background, I can never remember what svm->nested.ctl holds.  For
> > > me, this is waaaay more intuivite:
> > 
> > I agree it reads better, but honestly all of nSVM code uses svm->nested.ctl,
> > and changing its name here just makes things inconsistent imo.
> 
> Gotta start somewhere :-)  In all seriousness, if we didn't allow chipping away
> to at historical oddities in KVM, the code base would be a disaster.  I'm all for
> prioritizing consistency, but I draw the line at "everything else sucks, so this
> needs to suck too".
> 
> I'm not saying we need to do a wholesale rename, but giving at least
> nested_vmcb02_prepare_control() the same treatment will be a huge improvement.
> Actually, I'm going to go do that right now...

For what it's worth, at some point I was going to send a patch to put
svm->nested.ctl and svm->nested.save in an anonymous struct, to end up
with svm->nested.cached_vmcb12.ctl and svm->nested.cached_vmcb12.save,
but the names are too long :)

[PATCH 2/3] KVM: nSVM: Rename recalc_intercepts() to clarify vmcb02 as the target

[Yosry Ahmed]

recalc_intercepts() updates the intercept bits in vmcb02 based on vmcb01
and (cached) vmcb12. However, the name is too generic to make this
clear, and is especially confusing while searching through the code as
it shares the same name as the recalc_intercepts callback in
kvm_x86_ops.

Rename it to nested_vmcb02_recalc_intercepts() (similar to other
nested_vmcb02_* scoped functions), to make it clear what it is doing.

No functional change intended.

Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
---
 arch/x86/kvm/svm/nested.c |  4 ++--
 arch/x86/kvm/svm/sev.c    |  2 +-
 arch/x86/kvm/svm/svm.c    |  4 ++--
 arch/x86/kvm/svm/svm.h    | 10 +++++-----
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 2dda52221fd8..bacb2ac4c59e 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -123,7 +123,7 @@ static bool nested_vmcb_needs_vls_intercept(struct vcpu_svm *svm)
 	return false;
 }
 
-void recalc_intercepts(struct vcpu_svm *svm)
+void nested_vmcb02_recalc_intercepts(struct vcpu_svm *svm)
 {
 	struct vmcb *vmcb01, *vmcb02;
 	unsigned int i;
@@ -918,7 +918,7 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm,
 	 * Merge guest and host intercepts - must be called with vcpu in
 	 * guest-mode to take effect.
 	 */
-	recalc_intercepts(svm);
+	nested_vmcb02_recalc_intercepts(svm);
 }
 
 static void nested_svm_copy_common_state(struct vmcb *from_vmcb, struct vmcb *to_vmcb)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index f59c65abe3cf..f50a95aa41bc 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -4604,7 +4604,7 @@ static void sev_es_init_vmcb(struct vcpu_svm *svm, bool init_event)
 	if (!sev_vcpu_has_debug_swap(svm)) {
 		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_READ);
 		vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_WRITE);
-		recalc_intercepts(svm);
+		nested_vmcb02_recalc_intercepts(svm);
 	} else {
 		/*
 		 * Disable #DB intercept iff DebugSwap is enabled.  KVM doesn't
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 7041498a8091..485c2710d7a4 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -635,7 +635,7 @@ static void set_dr_intercepts(struct vcpu_svm *svm)
 	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_READ);
 	vmcb_set_intercept(&vmcb->control, INTERCEPT_DR7_WRITE);
 
-	recalc_intercepts(svm);
+	nested_vmcb02_recalc_intercepts(svm);
 }
 
 static void clr_dr_intercepts(struct vcpu_svm *svm)
@@ -644,7 +644,7 @@ static void clr_dr_intercepts(struct vcpu_svm *svm)
 
 	vmcb->control.intercepts[INTERCEPT_DR] = 0;
 
-	recalc_intercepts(svm);
+	nested_vmcb02_recalc_intercepts(svm);
 }
 
 static bool msr_write_intercepted(struct kvm_vcpu *vcpu, u32 msr)
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 7d28a739865f..330633291c57 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -357,7 +357,7 @@ struct svm_cpu_data {
 
 DECLARE_PER_CPU(struct svm_cpu_data, svm_data);
 
-void recalc_intercepts(struct vcpu_svm *svm);
+void nested_vmcb02_recalc_intercepts(struct vcpu_svm *svm);
 
 static __always_inline struct kvm_svm *to_kvm_svm(struct kvm *kvm)
 {
@@ -485,7 +485,7 @@ static inline void set_exception_intercept(struct vcpu_svm *svm, u32 bit)
 	WARN_ON_ONCE(bit >= 32);
 	vmcb_set_intercept(&vmcb->control, INTERCEPT_EXCEPTION_OFFSET + bit);
 
-	recalc_intercepts(svm);
+	nested_vmcb02_recalc_intercepts(svm);
 }
 
 static inline void clr_exception_intercept(struct vcpu_svm *svm, u32 bit)
@@ -495,7 +495,7 @@ static inline void clr_exception_intercept(struct vcpu_svm *svm, u32 bit)
 	WARN_ON_ONCE(bit >= 32);
 	vmcb_clr_intercept(&vmcb->control, INTERCEPT_EXCEPTION_OFFSET + bit);
 
-	recalc_intercepts(svm);
+	nested_vmcb02_recalc_intercepts(svm);
 }
 
 static inline void svm_set_intercept(struct vcpu_svm *svm, int bit)
@@ -504,7 +504,7 @@ static inline void svm_set_intercept(struct vcpu_svm *svm, int bit)
 
 	vmcb_set_intercept(&vmcb->control, bit);
 
-	recalc_intercepts(svm);
+	nested_vmcb02_recalc_intercepts(svm);
 }
 
 static inline void svm_clr_intercept(struct vcpu_svm *svm, int bit)
@@ -513,7 +513,7 @@ static inline void svm_clr_intercept(struct vcpu_svm *svm, int bit)
 
 	vmcb_clr_intercept(&vmcb->control, bit);
 
-	recalc_intercepts(svm);
+	nested_vmcb02_recalc_intercepts(svm);
 }
 
 static inline bool svm_is_intercept(struct vcpu_svm *svm, int bit)
-- 
2.52.0.457.g6b5491de43-goog

Re: [PATCH 2/3] KVM: nSVM: Rename recalc_intercepts() to clarify vmcb02 as the target

[Sean Christopherson]

On Mon, Jan 12, 2026, Yosry Ahmed wrote:
> recalc_intercepts() updates the intercept bits in vmcb02 based on vmcb01
> and (cached) vmcb12.

Ah, but it does more than that.  More below.

> However, the name is too generic to make this
> clear, and is especially confusing while searching through the code as
> it shares the same name as the recalc_intercepts callback in
> kvm_x86_ops.
> 
> Rename it to nested_vmcb02_recalc_intercepts() (similar to other
> nested_vmcb02_* scoped functions), to make it clear what it is doing.
> 
> No functional change intended.
> 
> Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> ---
>  arch/x86/kvm/svm/nested.c |  4 ++--
>  arch/x86/kvm/svm/sev.c    |  2 +-
>  arch/x86/kvm/svm/svm.c    |  4 ++--
>  arch/x86/kvm/svm/svm.h    | 10 +++++-----
>  4 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 2dda52221fd8..bacb2ac4c59e 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -123,7 +123,7 @@ static bool nested_vmcb_needs_vls_intercept(struct vcpu_svm *svm)
>  	return false;
>  }
>  
> -void recalc_intercepts(struct vcpu_svm *svm)
> +void nested_vmcb02_recalc_intercepts(struct vcpu_svm *svm)
>  {
>  	struct vmcb *vmcb01, *vmcb02;
>  	unsigned int i;

Drat, I should have responded to the previous patch.  Lurking out of sight is a
pre-existing bug that effectively invalidates this entire rename.

The existing code is:

  void recalc_intercepts(struct vcpu_svm *svm)
  {
	struct vmcb *vmcb01, *vmcb02;
	unsigned int i;

	vmcb_mark_dirty(svm->vmcb, VMCB_INTERCEPTS);  <======= not vmcb01!!!!!

	if (!is_guest_mode(&svm->vcpu))
		return;

When L2 is active, svm->vmcb is vmcb02.  Which, at first glance, _looks_ right,
but (the *horribly* named) recalc_intercepts() isn't _just_ recalculating
intercepts for L2, it's also responsible for marking the VMCB_INTERCEPTS dirty
(obviously).

But what isn't so obvious is that _all_ callers operate on vmcb01, because the
pattern is to modify vmcb01 intercepts, and then merge the new vmcb01 intercepts
with vmcb12, i.e. the "recalc intercepts" aspect is "part 2" of the overall
function.

Lost in all of this is that KVM forgets to mark vmcb01 dirty, and unless there's
a call buried somewhere deep, nested_svm_vmexit() isn't guaranteed to mark
VMCB_INTERCEPTS dirty, e.g. if PAUSE interception is disabled.

It's probably a benign bug in practice, as AMD CPUs don't appear to do anything
with the clean fields, but easy to fix.

As a bonus, fixing that bug yields for even better naming and code.  After the
dust settles, we can end up with this in svm.h:

  void nested_vmcb02_recalc_intercepts(struct vcpu_svm *svm);

  static inline void svm_mark_intercepts_dirty(struct vcpu_svm *svm)
  {
	vmcb_mark_dirty(svm->vmcb01.ptr, VMCB_INTERCEPTS);

	/*
	 * If L2 is active, recalculate the intercepts for vmcb02 to account
	 * for the changes made to vmcb01.  All intercept configuration is done
	 * for vmcb01 and then propagated to vmcb02 to combine KVM's intercepts
	 * with L1's intercepts (from the vmcb12 snapshot).
	 */
	if (is_guest_mode(&svm->vcpu))
		nested_vmcb02_recalc_intercepts(svm);
  }

and this for nested_vmcb02_recalc_intercepts():

  void nested_vmcb02_recalc_intercepts(struct vcpu_svm *svm)
  {
	struct vmcb_ctrl_area_cached *vmcb12_ctrl = &svm->nested.ctl;
	struct vmcb *vmcb02 = svm->nested.vmcb02.ptr;
	struct vmcb *vmcb01 = svm->vmcb01.ptr;
	unsigned int i;

	if (WARN_ON_ONCE(svm->vmcb != vmcb02))
		return;

	...
  }

with the only other caller of nested_vmcb02_recalc_intercepts() being
nested_vmcb02_prepare_control().

Re: [PATCH 2/3] KVM: nSVM: Rename recalc_intercepts() to clarify vmcb02 as the target

[Yosry Ahmed]

On Wed, Feb 04, 2026 at 09:45:52AM -0800, Sean Christopherson wrote:
> On Mon, Jan 12, 2026, Yosry Ahmed wrote:
> > recalc_intercepts() updates the intercept bits in vmcb02 based on vmcb01
> > and (cached) vmcb12.
> 
> Ah, but it does more than that.  More below.
> 
> > However, the name is too generic to make this
> > clear, and is especially confusing while searching through the code as
> > it shares the same name as the recalc_intercepts callback in
> > kvm_x86_ops.
> > 
> > Rename it to nested_vmcb02_recalc_intercepts() (similar to other
> > nested_vmcb02_* scoped functions), to make it clear what it is doing.
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> > ---
> >  arch/x86/kvm/svm/nested.c |  4 ++--
> >  arch/x86/kvm/svm/sev.c    |  2 +-
> >  arch/x86/kvm/svm/svm.c    |  4 ++--
> >  arch/x86/kvm/svm/svm.h    | 10 +++++-----
> >  4 files changed, 10 insertions(+), 10 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 2dda52221fd8..bacb2ac4c59e 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -123,7 +123,7 @@ static bool nested_vmcb_needs_vls_intercept(struct vcpu_svm *svm)
> >  	return false;
> >  }
> >  
> > -void recalc_intercepts(struct vcpu_svm *svm)
> > +void nested_vmcb02_recalc_intercepts(struct vcpu_svm *svm)
> >  {
> >  	struct vmcb *vmcb01, *vmcb02;
> >  	unsigned int i;
> 
> Drat, I should have responded to the previous patch.  Lurking out of sight is a
> pre-existing bug that effectively invalidates this entire rename.
> 
> The existing code is:
> 
>   void recalc_intercepts(struct vcpu_svm *svm)
>   {
> 	struct vmcb *vmcb01, *vmcb02;
> 	unsigned int i;
> 
> 	vmcb_mark_dirty(svm->vmcb, VMCB_INTERCEPTS);  <======= not vmcb01!!!!!
> 
> 	if (!is_guest_mode(&svm->vcpu))
> 		return;
> 
> When L2 is active, svm->vmcb is vmcb02.  Which, at first glance, _looks_ right,
> but (the *horribly* named) recalc_intercepts() isn't _just_ recalculating
> intercepts for L2, it's also responsible for marking the VMCB_INTERCEPTS dirty
> (obviously).
> 
> But what isn't so obvious is that _all_ callers operate on vmcb01, because the
> pattern is to modify vmcb01 intercepts, and then merge the new vmcb01 intercepts
> with vmcb12, i.e. the "recalc intercepts" aspect is "part 2" of the overall
> function.

I think the 4th law of thermodynamics is that any piece of nSVM code has
a bug if you look at it long enough.

> 
> Lost in all of this is that KVM forgets to mark vmcb01 dirty, and unless there's
> a call buried somewhere deep, nested_svm_vmexit() isn't guaranteed to mark
> VMCB_INTERCEPTS dirty, e.g. if PAUSE interception is disabled.
> 
> It's probably a benign bug in practice, as AMD CPUs don't appear to do anything
> with the clean fields, but easy to fix.
> 
> As a bonus, fixing that bug yields for even better naming and code.  After the
> dust settles, we can end up with this in svm.h:
> 
>   void nested_vmcb02_recalc_intercepts(struct vcpu_svm *svm);
> 
>   static inline void svm_mark_intercepts_dirty(struct vcpu_svm *svm)
>   {
> 	vmcb_mark_dirty(svm->vmcb01.ptr, VMCB_INTERCEPTS);
> 
> 	/*
> 	 * If L2 is active, recalculate the intercepts for vmcb02 to account
> 	 * for the changes made to vmcb01.  All intercept configuration is done
> 	 * for vmcb01 and then propagated to vmcb02 to combine KVM's intercepts
> 	 * with L1's intercepts (from the vmcb12 snapshot).
> 	 */
> 	if (is_guest_mode(&svm->vcpu))
> 		nested_vmcb02_recalc_intercepts(svm);
>   }
> 
> and this for nested_vmcb02_recalc_intercepts():
> 
>   void nested_vmcb02_recalc_intercepts(struct vcpu_svm *svm)
>   {
> 	struct vmcb_ctrl_area_cached *vmcb12_ctrl = &svm->nested.ctl;
> 	struct vmcb *vmcb02 = svm->nested.vmcb02.ptr;
> 	struct vmcb *vmcb01 = svm->vmcb01.ptr;
> 	unsigned int i;
> 
> 	if (WARN_ON_ONCE(svm->vmcb != vmcb02))
> 		return;
> 
> 	...
>   }
> 
> with the only other caller of nested_vmcb02_recalc_intercepts() being
> nested_vmcb02_prepare_control().

I think this looks good. Definitely an improvement over what we
currently have :)

[PATCH 3/3] KVM: nSVM: Use vmcb12_is_intercept() in nested_sync_control_from_vmcb02()

[Yosry Ahmed]

Use vmcb12_is_intercept() instead of open-coding the intercept check.

No functional change intended.

Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
---
 arch/x86/kvm/svm/nested.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index bacb2ac4c59e..1c18928369f2 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -534,7 +534,7 @@ void nested_sync_control_from_vmcb02(struct vcpu_svm *svm)
 	 * int_ctl (because it was never recognized while L2 was running).
 	 */
 	if (svm_is_intercept(svm, INTERCEPT_VINTR) &&
-	    !test_bit(INTERCEPT_VINTR, (unsigned long *)svm->nested.ctl.intercepts))
+	    !vmcb12_is_intercept(&svm->nested.ctl, INTERCEPT_VINTR))
 		mask &= ~V_IRQ_MASK;
 
 	if (nested_vgif_enabled(svm))
-- 
2.52.0.457.g6b5491de43-goog

Re: [PATCH 0/3] nSVM: Minor cleanups for intercepts code

[Sean Christopherson]

On Mon, Jan 12, 2026, Yosry Ahmed wrote:
> A few minor cleanups for nested intercepts code, namely making
> recalc_intercepts() more readable and renaming it, and using
> vmcb12_is_intercept() instead of open-coding it.

I'll send a v2.  Fixing the vmcb_mark_dirty() bug yields a fairly different
overall sequence:

  KVM: SVM: Explicitly mark vmcb01 dirty after modifying VMCB intercepts
  KVM: SVM: Separate recalc_intercepts() into nested vs. non-nested parts
  KVM: nSVM: WARN and abort vmcb02 intercepts recalc if vmcb02 isn't active
  KVM: nSVM: Directly (re)calc vmcb02 intercepts from nested_vmcb02_prepare_control()
  KVM: nSVM: Use intuitive local variables in nested_vmcb02_recalc_intercepts()
  KVM: nSVM: Use vmcb12_is_intercept() in nested_sync_control_from_vmcb02()

Re: [PATCH 0/3] nSVM: Minor cleanups for intercepts code

[Yosry Ahmed]

On Wed, Feb 04, 2026 at 09:47:34AM -0800, Sean Christopherson wrote:
> On Mon, Jan 12, 2026, Yosry Ahmed wrote:
> > A few minor cleanups for nested intercepts code, namely making
> > recalc_intercepts() more readable and renaming it, and using
> > vmcb12_is_intercept() instead of open-coding it.
> 
> I'll send a v2.  Fixing the vmcb_mark_dirty() bug yields a fairly different
> overall sequence:

Sounds great to me :)

> 
>   KVM: SVM: Explicitly mark vmcb01 dirty after modifying VMCB intercepts
>   KVM: SVM: Separate recalc_intercepts() into nested vs. non-nested parts
>   KVM: nSVM: WARN and abort vmcb02 intercepts recalc if vmcb02 isn't active
>   KVM: nSVM: Directly (re)calc vmcb02 intercepts from nested_vmcb02_prepare_control()
>   KVM: nSVM: Use intuitive local variables in nested_vmcb02_recalc_intercepts()
>   KVM: nSVM: Use vmcb12_is_intercept() in nested_sync_control_from_vmcb02()