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

Naveen N Rao
2026-04-10T16:53:44Z

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