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: <66bab47847aa378216c39f46e072d1b2039c3e0e.camel@redhat.com>
Date: Thu, 26 Jun 2025 13:34:34 -0400
From: mlevitsk@...hat.com
To: "Naveen N Rao (AMD)" <naveen@...nel.org>, Sean Christopherson
	 <seanjc@...gle.com>, Paolo Bonzini <pbonzini@...hat.com>
Cc: 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, 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.

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

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.

And on Zen4, enable_apicv can be true by default.

But if you insist on to have this patch instead I won't object strongly,
as long as it can be enabled by the user, it doesn't matter that much.

Best regards,
	Maxim Levitsky


> (*) http://lkml.kernel.org/r/Z6JoInXNntIoHLQ8@google.com
> 
> Signed-off-by: Naveen N Rao (AMD) <naveen@...nel.org>
> ---
>  arch/x86/kvm/svm/avic.c | 11 +++++------
>  arch/x86/kvm/svm/svm.c  | 10 +++-------
>  2 files changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index a34c5c3b164e..bf7f91f41a6e 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -1101,12 +1101,11 @@ bool avic_hardware_setup(void)
>  	if (!npt_enabled)
>  		return false;
>  
> -	/* AVIC is a prerequisite for x2AVIC. */
> -	if (!boot_cpu_has(X86_FEATURE_AVIC) && !force_avic) {
> -		if (boot_cpu_has(X86_FEATURE_X2AVIC)) {
> -			pr_warn(FW_BUG "Cannot support x2AVIC due to AVIC is disabled");
> -			pr_warn(FW_BUG "Try enable AVIC using force_avic option");
> -		}
> +	if (!boot_cpu_has(X86_FEATURE_AVIC) && !force_avic)
> +		return false;
> +
> +	if (!force_avic && (boot_cpu_data.x86 < 0x19 || boot_cpu_has(X86_FEATURE_ZEN3))) {
> +		pr_warn("AVIC disabled due to hardware errata. Use force_avic=1 if you really want to enable AVIC.\n");
>  		return false;
>  	}
>  
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index ab11d1d0ec51..9b5356e74384 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -158,12 +158,7 @@ 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);
> +module_param_named(avic, enable_apicv, bool, 0444);
>  module_param(enable_ipiv, bool, 0444);
>  
>  module_param(enable_device_posted_irqs, bool, 0444);
> @@ -5404,7 +5399,8 @@ static __init int svm_hardware_setup(void)
>  			goto err;
>  	}
>  
> -	enable_apicv = avic = avic && avic_hardware_setup();
> +	if (enable_apicv)
> +		enable_apicv = avic_hardware_setup();
>  
>  	if (!enable_apicv) {
>  		enable_ipiv = false;
> 
> base-commit: 7ee45fdd644b138e7a213c6936474161b28d0e1a