[PATCH 0/3] KVM: SVM: Fix x2AVIC MSR interception mess

[PATCH 0/3] KVM: SVM: Fix x2AVIC MSR interception mess

[Sean Christopherson]

Fix a variety of bugs in SVM's handling of x2APIC MSR passthrough for x2AVIC,
where KVM disables interception for MSR accesses that aren't accelerated by
hardware (pointless and suboptimal), and also does NOT disable interception
for practically any of the "range of vectors" MSRs, i.e. IRR, ISR, and TMR.

Lightly tested.  Functionally, I'm pretty confident it's correct, but I
haven't done due diligence to verify accesses are being accelerated/intercepted
as expected.

Found by inspection when reviewing a TDX patch to fix a bug where KVM botched
the "range of vectors"[*] (I was curious how other KVM code handled the ranges;
wasn't expecting this...).

I tagged all of this for stable, mainly because handling the
AVIC_UNACCELERATED_ACCESS faults (unlike traps) #VMEXITs requires blind
emulation of the code stream.  Which is "fine", but obviously unnecessary.
I.e. I could be convinced these fixes shouldn't be sent to LTS trees.

[*] https://lore.kernel.org/all/20260318190111.1041924-1-dmaluka@chromium.org

Sean Christopherson (3):
  KVM: SVM: Disable x2AVIC RDMSR interception for MSRs KVM actually
    supports
  KVM: SVM: Always intercept RDMSR for TMCCT (current APIC timer count)
  KVM: SVM: Only disable x2AVIC WRMSR interception for MSRs that are
    accelerated

 arch/x86/kvm/svm/avic.c | 50 +++++++++++++----------------------------
 1 file changed, 15 insertions(+), 35 deletions(-)


base-commit: b89df297a47e641581ee67793592e5c6ae0428f4
-- 
2.53.0.1213.gd9a14994de-goog

[PATCH 1/3] KVM: SVM: Disable x2AVIC RDMSR interception for MSRs KVM actually supports

[Sean Christopherson]

Fix multiple (classes of) bugs with one stone by using KVM's mask of
readable local APIC registers to determine which x2APIC MSRs to pass
through (or not) when toggling x2AVIC on/off.  The existing hand-coded
list of MSRs is wrong on multiple fronts:

 - ARBPRI, DFR, and ICR2 aren't supported by x2APIC; disabling
   interception is nonsensical and suboptimal (the access generates a
   #VMEXIT that requires decoding the instruction).

 - RRR is completely unsupported.

 - AVIC currently fails to pass through the "range of vectors" registers,
   IRR, ISR, and TMR, as e.g. X2APIC_MSR(APIC_IRR) only affects IRR0, and
   thus only disables intercept for vectors 31:0 (which are the *least*
   interesting registers).

Fixes: 4d1d7942e36a ("KVM: SVM: Introduce logic to (de)activate x2AVIC mode")
Cc: stable@vger.kernel.org
Cc: Naveen N Rao (AMD) <naveen@kernel.org>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/avic.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index adf211860949..df974ee290d0 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -122,6 +122,9 @@ static u32 x2avic_max_physical_id;
 static void avic_set_x2apic_msr_interception(struct vcpu_svm *svm,
 					     bool intercept)
 {
+	struct kvm_vcpu *vcpu = &svm->vcpu;
+	u64 x2apic_readable_mask;
+
 	static const u32 x2avic_passthrough_msrs[] = {
 		X2APIC_MSR(APIC_ID),
 		X2APIC_MSR(APIC_LVR),
@@ -162,9 +165,15 @@ static void avic_set_x2apic_msr_interception(struct vcpu_svm *svm,
 	if (!x2avic_enabled)
 		return;
 
+	x2apic_readable_mask = kvm_lapic_readable_reg_mask(vcpu->arch.apic);
+
+	for (i = 0; i < BITS_PER_TYPE(typeof(x2apic_readable_mask)); i++)
+		svm_set_intercept_for_msr(vcpu, APIC_BASE_MSR + i,
+					  MSR_TYPE_R, intercept);
+
 	for (i = 0; i < ARRAY_SIZE(x2avic_passthrough_msrs); i++)
-		svm_set_intercept_for_msr(&svm->vcpu, x2avic_passthrough_msrs[i],
-					  MSR_TYPE_RW, intercept);
+		svm_set_intercept_for_msr(vcpu, x2avic_passthrough_msrs[i],
+					  MSR_TYPE_W, intercept);
 
 	svm->x2avic_msrs_intercepted = intercept;
 }
-- 
2.53.0.1213.gd9a14994de-goog

[PATCH 2/3] KVM: SVM: Always intercept RDMSR for TMCCT (current APIC timer count)

[Sean Christopherson]

Explicitly intercept RDMSR for TMMCT, a.k.a. the current APIC timer count,
when x2AVIC is enabled, as TMMCT reads aren't accelerated by hardware.
Disabling interception is suboptimal as the RDMSR generates an
AVIC_UNACCELERATED_ACCESS fault #VMEXIT, which forces KVM to decode the
instruction to figure out what the guest was trying to access.

Note, the only reason this isn't a fatal bug is that the AVIC architecture
had the foresight to guard against buggy hypervisors.  E.g. if hardware
simply read from the virtual APIC page, the guest would get garbage.

Fixes: 4d1d7942e36a ("KVM: SVM: Introduce logic to (de)activate x2AVIC mode")
Cc: stable@vger.kernel.org
Cc: Naveen N Rao (AMD) <naveen@kernel.org>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/avic.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index df974ee290d0..c9e9872ad880 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -171,6 +171,9 @@ static void avic_set_x2apic_msr_interception(struct vcpu_svm *svm,
 		svm_set_intercept_for_msr(vcpu, APIC_BASE_MSR + i,
 					  MSR_TYPE_R, intercept);
 
+	if (!intercept)
+		svm_enable_intercept_for_msr(vcpu, X2APIC_MSR(APIC_TMCCT), MSR_TYPE_R);
+
 	for (i = 0; i < ARRAY_SIZE(x2avic_passthrough_msrs); i++)
 		svm_set_intercept_for_msr(vcpu, x2avic_passthrough_msrs[i],
 					  MSR_TYPE_W, intercept);
-- 
2.53.0.1213.gd9a14994de-goog

[PATCH 3/3] KVM: SVM: Only disable x2AVIC WRMSR interception for MSRs that are accelerated

[Sean Christopherson]

When x2AVIC is enabled, disable WRMSR interception only for MSRs that are
actually accelerated by hardware.  Disabling interception for MSRs that
aren't accelerated is functionally "fine", but very suboptimal as many
accesses generate AVIC_UNACCELERATED_ACCESS fault #VMEXITs, which requires
KVM to decode the instruction to figure out what the guest was trying to
access.

Note, the set of MSRs that are passed through for write is identical to
VMX's set when IPI virtualization is enabled.  This is not a coincidence,
as x2AVIC is functionally equivalent to APICv+IPIv.

Fixes: 4d1d7942e36a ("KVM: SVM: Introduce logic to (de)activate x2AVIC mode")
Cc: stable@vger.kernel.org
Cc: Naveen N Rao (AMD) <naveen@kernel.org>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/avic.c | 40 ++++------------------------------------
 1 file changed, 4 insertions(+), 36 deletions(-)

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index c9e9872ad880..2b07cc347b90 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -124,39 +124,6 @@ static void avic_set_x2apic_msr_interception(struct vcpu_svm *svm,
 {
 	struct kvm_vcpu *vcpu = &svm->vcpu;
 	u64 x2apic_readable_mask;
-
-	static const u32 x2avic_passthrough_msrs[] = {
-		X2APIC_MSR(APIC_ID),
-		X2APIC_MSR(APIC_LVR),
-		X2APIC_MSR(APIC_TASKPRI),
-		X2APIC_MSR(APIC_ARBPRI),
-		X2APIC_MSR(APIC_PROCPRI),
-		X2APIC_MSR(APIC_EOI),
-		X2APIC_MSR(APIC_RRR),
-		X2APIC_MSR(APIC_LDR),
-		X2APIC_MSR(APIC_DFR),
-		X2APIC_MSR(APIC_SPIV),
-		X2APIC_MSR(APIC_ISR),
-		X2APIC_MSR(APIC_TMR),
-		X2APIC_MSR(APIC_IRR),
-		X2APIC_MSR(APIC_ESR),
-		X2APIC_MSR(APIC_ICR),
-		X2APIC_MSR(APIC_ICR2),
-
-		/*
-		 * Note!  Always intercept LVTT, as TSC-deadline timer mode
-		 * isn't virtualized by hardware, and the CPU will generate a
-		 * #GP instead of a #VMEXIT.
-		 */
-		X2APIC_MSR(APIC_LVTTHMR),
-		X2APIC_MSR(APIC_LVTPC),
-		X2APIC_MSR(APIC_LVT0),
-		X2APIC_MSR(APIC_LVT1),
-		X2APIC_MSR(APIC_LVTERR),
-		X2APIC_MSR(APIC_TMICT),
-		X2APIC_MSR(APIC_TMCCT),
-		X2APIC_MSR(APIC_TDCR),
-	};
 	int i;
 
 	if (intercept == svm->x2avic_msrs_intercepted)
@@ -174,9 +141,10 @@ static void avic_set_x2apic_msr_interception(struct vcpu_svm *svm,
 	if (!intercept)
 		svm_enable_intercept_for_msr(vcpu, X2APIC_MSR(APIC_TMCCT), MSR_TYPE_R);
 
-	for (i = 0; i < ARRAY_SIZE(x2avic_passthrough_msrs); i++)
-		svm_set_intercept_for_msr(vcpu, x2avic_passthrough_msrs[i],
-					  MSR_TYPE_W, intercept);
+	svm_set_intercept_for_msr(vcpu, X2APIC_MSR(APIC_TASKPRI), MSR_TYPE_W, intercept);
+	svm_set_intercept_for_msr(vcpu, X2APIC_MSR(APIC_EOI), MSR_TYPE_W, intercept);
+	svm_set_intercept_for_msr(vcpu, X2APIC_MSR(APIC_SELF_IPI), MSR_TYPE_W, intercept);
+	svm_set_intercept_for_msr(vcpu, X2APIC_MSR(APIC_ICR), MSR_TYPE_W, intercept);
 
 	svm->x2avic_msrs_intercepted = intercept;
 }
-- 
2.53.0.1213.gd9a14994de-goog

Re: [PATCH 1/3] KVM: SVM: Disable x2AVIC RDMSR interception for MSRs KVM actually supports

[Naveen N Rao]

On Thu, Apr 09, 2026 at 03:24:47PM -0700, Sean Christopherson wrote:
> Fix multiple (classes of) bugs with one stone by using KVM's mask of
> readable local APIC registers to determine which x2APIC MSRs to pass
> through (or not) when toggling x2AVIC on/off.  The existing hand-coded
> list of MSRs is wrong on multiple fronts:
> 
>  - ARBPRI, DFR, and ICR2 aren't supported by x2APIC; disabling
>    interception is nonsensical and suboptimal (the access generates a
>    #VMEXIT that requires decoding the instruction).
> 
>  - RRR is completely unsupported.
> 
>  - AVIC currently fails to pass through the "range of vectors" registers,
>    IRR, ISR, and TMR, as e.g. X2APIC_MSR(APIC_IRR) only affects IRR0, and
>    thus only disables intercept for vectors 31:0 (which are the *least*
>    interesting registers).

:facepalm:

We seriously need better selftests for these. Also on my list has been 
to cook up something for your other fix where AVIC gets inhibited for 
non-zero vCPU IDs (with x2AVIC disabled):
http://lore.kernel.org/r/20260112232805.1512361-1-seanjc@google.com

I started looking at Alejandro's series adding AVIC-related binary 
stats, but had to switch to other things. Last I looked, I felt that 
your suggestion to add an "exits" array accounting individual #VMEXITs 
would in particular be helpful:
https://lore.kernel.org/kvm/ZmMjHwavCLk0lRd7@google.com/

Though I'm not sure how standardizing this across VMX and SVM looks 
like, and/or if it will be truly helpful -- we may be interested in 
specific exits, such as AVIC-related exits for some of the tests...
Thoughts?

> 
> Fixes: 4d1d7942e36a ("KVM: SVM: Introduce logic to (de)activate x2AVIC mode")
> Cc: stable@vger.kernel.org
> Cc: Naveen N Rao (AMD) <naveen@kernel.org>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/avic.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index adf211860949..df974ee290d0 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -122,6 +122,9 @@ static u32 x2avic_max_physical_id;
>  static void avic_set_x2apic_msr_interception(struct vcpu_svm *svm,
>  					     bool intercept)
>  {
> +	struct kvm_vcpu *vcpu = &svm->vcpu;
> +	u64 x2apic_readable_mask;
> +
>  	static const u32 x2avic_passthrough_msrs[] = {
>  		X2APIC_MSR(APIC_ID),
>  		X2APIC_MSR(APIC_LVR),
> @@ -162,9 +165,15 @@ static void avic_set_x2apic_msr_interception(struct vcpu_svm *svm,
>  	if (!x2avic_enabled)
>  		return;
>  
> +	x2apic_readable_mask = kvm_lapic_readable_reg_mask(vcpu->arch.apic);
> +
> +	for (i = 0; i < BITS_PER_TYPE(typeof(x2apic_readable_mask)); i++)
> +		svm_set_intercept_for_msr(vcpu, APIC_BASE_MSR + i,
> +					  MSR_TYPE_R, intercept);
> +

Yet to test this series (will get to it next week in more detail), but I 
suppose you meant to use `for_each_set_bit()` or such?


- Naveen

Re: [PATCH 3/3] KVM: SVM: Only disable x2AVIC WRMSR interception for MSRs that are accelerated

[Naveen N Rao]

On Thu, Apr 09, 2026 at 03:24:49PM -0700, Sean Christopherson wrote:
> When x2AVIC is enabled, disable WRMSR interception only for MSRs that are
> actually accelerated by hardware.  Disabling interception for MSRs that
> aren't accelerated is functionally "fine", but very suboptimal as many
> accesses generate AVIC_UNACCELERATED_ACCESS fault #VMEXITs, which requires
> KVM to decode the instruction to figure out what the guest was trying to
> access.

Not sure I follow: as far as I can see, most writes are actually trapped 
except for MSRs that are supposed to be RO?

- Naveen

Re: [PATCH 3/3] KVM: SVM: Only disable x2AVIC WRMSR interception for MSRs that are accelerated

[Sean Christopherson]

On Fri, Apr 10, 2026, Naveen N Rao wrote:
> On Thu, Apr 09, 2026 at 03:24:49PM -0700, Sean Christopherson wrote:
> > When x2AVIC is enabled, disable WRMSR interception only for MSRs that are
> > actually accelerated by hardware.  Disabling interception for MSRs that
> > aren't accelerated is functionally "fine", but very suboptimal as many
> > accesses generate AVIC_UNACCELERATED_ACCESS fault #VMEXITs, which requires
> > KVM to decode the instruction to figure out what the guest was trying to
> > access.
> 
> Not sure I follow: as far as I can see, most writes are actually trapped 
> except for MSRs that are supposed to be RO?

Yeah, that's what I was trying to say by "only for MSRs that are actually
accelerated by hardware".  Oh, but that's bad verbiage, because trap-like
#VMEXITs are also due to lack of acceleration, it's specifically "software is
being stupid" that triggers fault-like exits.

Re: [PATCH 1/3] KVM: SVM: Disable x2AVIC RDMSR interception for MSRs KVM actually supports

[Sean Christopherson]

On Fri, Apr 10, 2026, Naveen N Rao wrote:
> On Thu, Apr 09, 2026 at 03:24:47PM -0700, Sean Christopherson wrote:
> > Fix multiple (classes of) bugs with one stone by using KVM's mask of
> > readable local APIC registers to determine which x2APIC MSRs to pass
> > through (or not) when toggling x2AVIC on/off.  The existing hand-coded
> > list of MSRs is wrong on multiple fronts:
> > 
> >  - ARBPRI, DFR, and ICR2 aren't supported by x2APIC; disabling
> >    interception is nonsensical and suboptimal (the access generates a
> >    #VMEXIT that requires decoding the instruction).
> > 
> >  - RRR is completely unsupported.
> > 
> >  - AVIC currently fails to pass through the "range of vectors" registers,
> >    IRR, ISR, and TMR, as e.g. X2APIC_MSR(APIC_IRR) only affects IRR0, and
> >    thus only disables intercept for vectors 31:0 (which are the *least*
> >    interesting registers).
> 
> :facepalm:
> 
> We seriously need better selftests for these.

+1000.  In general, we need a way to validate "this should exit, this should not".

> Also on my list has been to cook up something for your other fix where AVIC
> gets inhibited for non-zero vCPU IDs (with x2AVIC disabled):
> http://lore.kernel.org/r/20260112232805.1512361-1-seanjc@google.com
> 
> I started looking at Alejandro's series adding AVIC-related binary 
> stats, but had to switch to other things. Last I looked, I felt that 
> your suggestion to add an "exits" array accounting individual #VMEXITs 
> would in particular be helpful:
> https://lore.kernel.org/kvm/ZmMjHwavCLk0lRd7@google.com/

We have per-exit stats (and more!) internally, and it's ugly.   For simplicity,
and/or perhaps to reduce out-of-tree maintenance costs, we did the easy thing of
tracking hardware exit reasons as is.  Which is *extremely* useful, but also
quite wasteful with respect to memory consumption, as a large percentage of exit
counts are unintresting and/or completely unused, especially on SVM due to SVM
having high granularity exit reasons.

> Though I'm not sure how standardizing this across VMX and SVM looks 
> like, and/or if it will be truly helpful

Probably even uglier :-)   The one case I'm confident would benefit from some
amount of standardization is exceptions.  If we simply regurgitate hardware exits,
Intel/VMX will only capture "did an exception occur", whereas AMD/SVM will
capture exactly which exception occurred.  If we *strictly* regurgitated exits,
it would be even worse than that, because NMIs get lumped in with exceptions on
VMX.

There are more cases like that, where VMX and SVM have slightly different exit
granularity.  We'd want to standardize on what gets presented to userspace so
that userspace doesn't have to effectively do the standardization (or worse, can
only implement certain checks for one vendor or the other).

> -- we may be interested in specific exits, such as AVIC-related exits for
> some of the tests...
> Thoughts?

I don't think we should plan on leveraging per-exit stats for testing purposes,
as they may never land upstream, and if they do, it will take some time.  E.g.
even though we find them valuable, I'm far from convinced that what we've got
implemented internally is suitable for upstream.

However!  Idea.  For many testcases, we don't actually care about *what* exit
occurred, only if *an* exit occurred (or not).  I.e. tests really just need a
binary yes/no signal.  Which we _almost_ have today, except stat.exits is polluted
with exits that are host-induced/owned (e.g. IRQs and NMIs) and/or asynchronous
in nature, e.g. the VMX preemption timer (which is arguably host-owned as well).

If KVM provides a stat that increments only on exits that are the direct result
of a guest action, then we can use that in selftests to detect unexpected (or
missed) exits, without having to worry about false failures due to noise from
host IRQs/NMIs/SMIs, etc.

The hardest part is probably figuring out a name :-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 3f3290d5a0a6..a8530a3d0545 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4256,6 +4256,9 @@ static fastpath_t svm_exit_handlers_fastpath(struct kvm_vcpu *vcpu)
        struct vcpu_svm *svm = to_svm(vcpu);
        struct vmcb_control_area *control = &svm->vmcb->control;
 
+       if (is_guest_induced_exit(control->exit_code))
+               ++vcpu->stat.guest_induced_exits;
+
        /*
         * Next RIP must be provided as IRQs are disabled, and accessing guest
         * memory to decode the instruction might fault, i.e. might sleep.
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 967b58a8ab9d..cd7769c412ab 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7524,6 +7524,9 @@ void noinstr vmx_spec_ctrl_restore_host(struct vcpu_vmx *vmx,
 static fastpath_t vmx_exit_handlers_fastpath(struct kvm_vcpu *vcpu,
                                             bool force_immediate_exit)
 {
+       if (is_guest_induced_exit(vmx_get_exit_reason(vcpu).basic))
+               ++vcpu->stat.guest_induced_exits;
+
        /*
         * If L2 is active, some VMX preemption timer exits can be handled in
         * the fastpath even, all other exits must use the slow path.

> > @@ -162,9 +165,15 @@ static void avic_set_x2apic_msr_interception(struct vcpu_svm *svm,
> >  	if (!x2avic_enabled)
> >  		return;
> >  
> > +	x2apic_readable_mask = kvm_lapic_readable_reg_mask(vcpu->arch.apic);
> > +
> > +	for (i = 0; i < BITS_PER_TYPE(typeof(x2apic_readable_mask)); i++)
> > +		svm_set_intercept_for_msr(vcpu, APIC_BASE_MSR + i,
> > +					  MSR_TYPE_R, intercept);
> > +
> 
> Yet to test this series (will get to it next week in more detail), but I 
> suppose you meant to use `for_each_set_bit()` or such?

Heh, my turn for a /facepalm.  I'm glad I admitted I didn't test this very well :-)