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

[Naveen N Rao <naveen@kernel.org>]

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