linux-kernel - Re: [EARLY RFC] KVM: SVM: Enable AVIC by default from Zen 4

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <aF2VCQyeXULVEl7b@google.com>
Date: Thu, 26 Jun 2025 11:44:25 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: mlevitsk@...hat.com
Cc: "Naveen N Rao (AMD)" <naveen@...nel.org>, Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Vasant Hegde <vasant.hegde@....com>, 
	Suravee Suthikulpanit <suravee.suthikulpanit@....com>
Subject: Re: [EARLY RFC] KVM: SVM: Enable AVIC by default from Zen 4

On Thu, Jun 26, 2025, mlevitsk@...hat.com wrote:
> On Thu, 2025-06-26 at 20:21 +0530, Naveen N Rao (AMD) wrote:
> > This is early RFC to understand if there are any concerns with enabling
> > AVIC by default from Zen 4. There are a few issues related to irq window
> > inhibits (*) that will need to be addressed before we can enable AVIC,
> > but I wanted to understand if there are other issues that I may not be
> > aware of. I will split up the changes and turn this into a proper patch
> > series once there is agreement on how to proceed.
> > 
> > AVIC (and x2AVIC) is fully functional since Zen 4, and has so far been
> > working well in our tests across various workloads. So, enable AVIC by
> > default from Zen 4.
> > 
> > CPUs prior to Zen 4 are affected by hardware errata related to AVIC and
> > workaround for those (erratum #1235) is only just landing upstream. So,
> > it is unlikely that anyone was using AVIC on those CPUs. Start requiring
> > users on those CPUs to pass force_avic=1 to explicitly enable AVIC going
> > forward. This helps convey that AVIC isn't fully enabled (so users are
> > aware of what they are signing up for), while allowing us to make
> > kvm_amd module parameter 'avic' as an alias for 'enable_apicv'
> > simplifying the code.  The only downside is that force_avic taints the
> > kernel, but if this is otherwise agreeable, the taint can be restricted
> > to the AVIC feature bit not being enabled.
> > 
> > Finally, stop complaining that x2AVIC CPUID feature bit is present
> > without basic AVIC feature bit, since that looks to be the way AVIC is
> > being disabled on certain systems and enabling AVIC by default will
> > start printing this warning on systems that have AVIC disabled.
> > 
> 
> Hi,
> 
> 
> IMHO making AVIC default on on Zen4 is a good idea.
> 
> About older systems, I don't know if I fully support the idea of hiding
> the support under force_avic, because AFAIK, other that errata #1235
> there are no other significant issues with AVIC.

Agreed, force_avic should be reserved specifically for the case where AVIC exists
in hardware, but is not enumerated in CPUID.

> In fact errata #1235 is not present on Zen3, and I won't be surprised that
> AVIC was soft-disabled on Zen3 wrongly.

FWIW, the Zen3 systems I have access to don't support AVIC / APIC virtualization
in the IOMMU, so it's not just AVIC being soft-disabled in the CPU.

> IMHO the cleanest way is probably:
> 
> On Zen2 - enable_apicv off by default, when forced to 1, activate
> the workaround for it. AFAIK with my workaround, there really should
> not be any issues, but since hardware is quite old, I am OK to keep it disabled.
> 
> On Zen3, AFAIK the errata #1235 is not present, so its likely that AVIC is
> fully functional as well, except that it is also disabled in IOMMU,
> and that one AFAIK can't be force-enabled.
> 
> I won't object if we remove force_avic altogether and just let the user also explicitly 
> enable avic with enable_apicv=1 on Zen3 as well.

I'm not comfortable ignoring lack of enumerated support without tainting the
kernel.

I don't see any reason to do major surgery, just give "avic" auto -1/0/1 behavior:

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index ab11d1d0ec51..4aa5bec0b1e7 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -158,12 +158,9 @@ module_param(lbrv, int, 0444);
 static int tsc_scaling = true;
 module_param(tsc_scaling, int, 0444);
 
-/*
- * enable / disable AVIC.  Because the defaults differ for APICv
- * support between VMX and SVM we cannot use module_param_named.
- */
-static bool avic;
-module_param(avic, bool, 0444);
+/* Enable AVIC by default only for Zen4+ (negative value = default/auto). */
+static int avic = -1;
+module_param(avic, int, 0444);
 module_param(enable_ipiv, bool, 0444);
 
 module_param(enable_device_posted_irqs, bool, 0444);
@@ -5404,6 +5401,9 @@ static __init int svm_hardware_setup(void)
                        goto err;
        }
 
+       if (avic < 0)
+               avic = boot_cpu_data.x86 > 0x19 || boot_cpu_has(X86_FEATURE_ZEN4);
+
        enable_apicv = avic = avic && avic_hardware_setup();
 
        if (!enable_apicv) {