[EARLY RFC] KVM: SVM: Enable AVIC by default from Zen 4

[EARLY RFC] KVM: SVM: Enable AVIC by default from Zen 4

[Naveen N Rao (AMD)]

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.

(*) http://lkml.kernel.org/r/Z6JoInXNntIoHLQ8@google.com

Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.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
-- 
2.49.0

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

[mlevitsk]

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@kernel.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

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

[Sean Christopherson]

On Thu, Jun 26, 2025, mlevitsk@redhat.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) {

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

[mlevitsk]

On Thu, 2025-06-26 at 11:44 -0700, Sean Christopherson wrote:
> On Thu, Jun 26, 2025, mlevitsk@redhat.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.

Yes, I mentioned that, but still practically speaking AVIC on Zen2 is equivalent
to APICv on Intel CPUs of the same generation, and on Zen3 AVIC is equavalent to
many Intel client systems which do have APICv but not posted interrupts, like my
laptop (I hate this).

> 
> > 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.

The kernel can still be tainted in this case, just that it is technically possible to drop 
force_avic, and instead just allow user to pass avic=1 instead, since it is
not on by default and KVM can still print the same warning and taint the kernel
when user passes avic=1 on Zen3.

Back when I implemented this, I just wanted to be a bit safer, a bit more explicit that
this uses an undocumented feature.

It doesn't matter much though.

> 
> 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) {
> 

I also have nothing against this to be honest, its OK to keep it as is IMHO.

Best regards,
	Maxim Levitsky

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

[Jim Mattson]

On Thu, Jun 26, 2025 at 12:15 PM <mlevitsk@redhat.com> wrote:
>
>
> I also have nothing against this to be honest, its OK to keep it as is IMHO.

I would like to see it enabled by default (when appropriate) so that
we get better coverage. Of course, we should not do this before we
feel the code is ready.

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

[Naveen N Rao]

Hi Maxim, Sean,
Thanks for the feedback!

On Thu, Jun 26, 2025 at 03:14:42PM -0400, mlevitsk@redhat.com wrote:
> On Thu, 2025-06-26 at 11:44 -0700, Sean Christopherson wrote:
> > On Thu, Jun 26, 2025, mlevitsk@redhat.com wrote:
> > > On Thu, 2025-06-26 at 20:21 +0530, Naveen N Rao (AMD) wrote:
> > > 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.
> 
> The kernel can still be tainted in this case, just that it is technically possible to drop 
> force_avic, and instead just allow user to pass avic=1 instead, since it is
> not on by default and KVM can still print the same warning and taint the kernel
> when user passes avic=1 on Zen3.

This will be a problem in scenarios where it is desirable to enable AVIC 
only if it is enabled on the system, but not otherwise. As an example, 
there are deployments where the 'avic' kernel module parameter is 
enabled fleet-wide (across Zen3/Zen4/Zen5), and it is expected that it 
be only enabled when supported on the system.

> 
> Back when I implemented this, I just wanted to be a bit safer, a bit more explicit that
> this uses an undocumented feature.
> 
> It doesn't matter much though.
> 
> > 
> > I don't see any reason to do major surgery, just give "avic" auto -1/0/1 behavior:

I am wary of breaking existing users/deployments on Zen4/Zen5 enabling 
AVIC by specifying avic=on, or avic=true today. That's primarily the 
reason I chose not to change 'avic' into an integer. Also, post module 
load, sysfs reports the value for 'avic' as a 'Y' or 'N' today. So if 
there are scripts relying on that, those will break if we change 'avic' 
into an integer.

For Zen1/Zen2, as I mentioned, it is unlikely that anyone today is 
enabling AVIC and expecting it to work since the workaround is only just 
hitting upstream. So, I'm hoping requiring force_avic=1 should be ok 
with the taint removed.

Longer term, once we get wider testing with the workaround on Zen1/Zen2, 
we can consider relaxing the need for force_avic, at which point AVIC 
can be default enabled and force_avic can be limited to scenarios where 
AVIC support is not advertised.


Thanks,
Naveen

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

[Sean Christopherson]

On Fri, Jun 27, 2025, Naveen N Rao wrote:
> > Back when I implemented this, I just wanted to be a bit safer, a bit more explicit that
> > this uses an undocumented feature.
> > 
> > It doesn't matter much though.
> > 
> > > 
> > > I don't see any reason to do major surgery, just give "avic" auto -1/0/1 behavior:
> 
> I am wary of breaking existing users/deployments on Zen4/Zen5 enabling 
> AVIC by specifying avic=on, or avic=true today. That's primarily the 
> reason I chose not to change 'avic' into an integer. Also, post module 
> load, sysfs reports the value for 'avic' as a 'Y' or 'N' today. So if 
> there are scripts relying on that, those will break if we change 'avic' 
> into an integer.

That's easy enough to handle, e.g. see nx_huge_pages_ops for a very similar case
where KVM has "auto" behavior (and a "never" case too), but otherwise treats the
param like a bool.

> For Zen1/Zen2, as I mentioned, it is unlikely that anyone today is 
> enabling AVIC and expecting it to work since the workaround is only just 
> hitting upstream. So, I'm hoping requiring force_avic=1 should be ok 
> with the taint removed.

But if that's the motivation, changing the semantics of force_avic doesn't make
any sense.  Once the workaround lands, the only reason for force_avic to exist
is to allow forcing KVM to enable AVIC even when it's not supported.

> Longer term, once we get wider testing with the workaround on Zen1/Zen2, 
> we can consider relaxing the need for force_avic, at which point AVIC 
> can be default enabled

I don't see why the default value for "avic" needs to be tied to force_avic.
If we're not confident that AVIC is 100% functional and a net positive for the
vast majority of setups/workloads on Zen1/Zen2, then simply leave "avic" off by
default for those CPUs.  If we ever want to enable AVIC by default across the
board, we can simply change the default value of "avic".

But to be honest, I don't see any reason to bother trying to enable AVIC by default
for Zen1/Zen2.  There's a very real risk that doing so would regress existing users
that have been running setups for ~6 years, and we can't fudge around AVIC being
hidden on Zen3 (and the IOMMU not supporting it at all), i.e. enabling AVIC by
default only for Zen4+ provides a cleaner story for end users.

> and force_avic can be limited to scenarios where AVIC support is not
> advertised.
> 
> 
> Thanks,
> Naveen
> 

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

[Naveen N Rao]

On Mon, Jul 07, 2025 at 04:21:53PM -0700, Sean Christopherson wrote:
> On Fri, Jun 27, 2025, Naveen N Rao wrote:
> > > Back when I implemented this, I just wanted to be a bit safer, a bit more explicit that
> > > this uses an undocumented feature.
> > > 
> > > It doesn't matter much though.
> > > 
> > > > 
> > > > I don't see any reason to do major surgery, just give "avic" auto -1/0/1 behavior:
> > 
> > I am wary of breaking existing users/deployments on Zen4/Zen5 enabling 
> > AVIC by specifying avic=on, or avic=true today. That's primarily the 
> > reason I chose not to change 'avic' into an integer. Also, post module 
> > load, sysfs reports the value for 'avic' as a 'Y' or 'N' today. So if 
> > there are scripts relying on that, those will break if we change 'avic' 
> > into an integer.
> 
> That's easy enough to handle, e.g. see nx_huge_pages_ops for a very similar case
> where KVM has "auto" behavior (and a "never" case too), but otherwise treats the
> param like a bool.

Nice! Looks like I can re-use existing callbacks for this too:
    static const struct kernel_param_ops avic_ops = {
	    .flags = KERNEL_PARAM_OPS_FL_NOARG,
	    .set = param_set_bint,
	    .get = param_get_bool,
    };

    /* enable/disable AVIC (-1 = auto) */
    int avic = -1;
    module_param_cb(avic, &avic_ops, &avic, 0444);
    __MODULE_PARM_TYPE(avic, "bool");

> 
> > For Zen1/Zen2, as I mentioned, it is unlikely that anyone today is 
> > enabling AVIC and expecting it to work since the workaround is only just 
> > hitting upstream. So, I'm hoping requiring force_avic=1 should be ok 
> > with the taint removed.
> 
> But if that's the motivation, changing the semantics of force_avic doesn't make
> any sense.  Once the workaround lands, the only reason for force_avic to exist
> is to allow forcing KVM to enable AVIC even when it's not supported.

Indeed.

> 
> > Longer term, once we get wider testing with the workaround on Zen1/Zen2, 
> > we can consider relaxing the need for force_avic, at which point AVIC 
> > can be default enabled
> 
> I don't see why the default value for "avic" needs to be tied to force_avic.
> If we're not confident that AVIC is 100% functional and a net positive for the
> vast majority of setups/workloads on Zen1/Zen2, then simply leave "avic" off by
> default for those CPUs.  If we ever want to enable AVIC by default across the
> board, we can simply change the default value of "avic".
> 
> But to be honest, I don't see any reason to bother trying to enable AVIC by default
> for Zen1/Zen2.  There's a very real risk that doing so would regress existing users
> that have been running setups for ~6 years, and we can't fudge around AVIC being
> hidden on Zen3 (and the IOMMU not supporting it at all), i.e. enabling AVIC by
> default only for Zen4+ provides a cleaner story for end users.

Works for me. I completely agree with that.


Thanks,
Naveen

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

[Maciej S. Szmigiero]

On 18.07.2025 10:19, Naveen N Rao wrote:
> On Mon, Jul 07, 2025 at 04:21:53PM -0700, Sean Christopherson wrote:
>> On Fri, Jun 27, 2025, Naveen N Rao wrote:
>>>> Back when I implemented this, I just wanted to be a bit safer, a bit more explicit that
>>>> this uses an undocumented feature.
>>>>
>>>> It doesn't matter much though.
>>>>
>>>>>
>>>>> I don't see any reason to do major surgery, just give "avic" auto -1/0/1 behavior:
>>>
>>> I am wary of breaking existing users/deployments on Zen4/Zen5 enabling
>>> AVIC by specifying avic=on, or avic=true today. That's primarily the
>>> reason I chose not to change 'avic' into an integer. Also, post module
>>> load, sysfs reports the value for 'avic' as a 'Y' or 'N' today. So if
>>> there are scripts relying on that, those will break if we change 'avic'
>>> into an integer.
>>
>> That's easy enough to handle, e.g. see nx_huge_pages_ops for a very similar case
>> where KVM has "auto" behavior (and a "never" case too), but otherwise treats the
>> param like a bool.
> 
> Nice! Looks like I can re-use existing callbacks for this too:
>      static const struct kernel_param_ops avic_ops = {
> 	    .flags = KERNEL_PARAM_OPS_FL_NOARG,
> 	    .set = param_set_bint,
> 	    .get = param_get_bool,
>      };
> 
>      /* enable/disable AVIC (-1 = auto) */
>      int avic = -1;
>      module_param_cb(avic, &avic_ops, &avic, 0444);
>      __MODULE_PARM_TYPE(avic, "bool");
> 
>>
>>> For Zen1/Zen2, as I mentioned, it is unlikely that anyone today is
>>> enabling AVIC and expecting it to work since the workaround is only just
>>> hitting upstream. So, I'm hoping requiring force_avic=1 should be ok
>>> with the taint removed.
>>
>> But if that's the motivation, changing the semantics of force_avic doesn't make
>> any sense.  Once the workaround lands, the only reason for force_avic to exist
>> is to allow forcing KVM to enable AVIC even when it's not supported.
> 
> Indeed.
> 
>>
>>> Longer term, once we get wider testing with the workaround on Zen1/Zen2,
>>> we can consider relaxing the need for force_avic, at which point AVIC
>>> can be default enabled
>>
>> I don't see why the default value for "avic" needs to be tied to force_avic.
>> If we're not confident that AVIC is 100% functional and a net positive for the
>> vast majority of setups/workloads on Zen1/Zen2, then simply leave "avic" off by
>> default for those CPUs.  If we ever want to enable AVIC by default across the
>> board, we can simply change the default value of "avic".
>>
>> But to be honest, I don't see any reason to bother trying to enable AVIC by default
>> for Zen1/Zen2.  There's a very real risk that doing so would regress existing users
>> that have been running setups for ~6 years, and we can't fudge around AVIC being
>> hidden on Zen3 (and the IOMMU not supporting it at all), i.e. enabling AVIC by
>> default only for Zen4+ provides a cleaner story for end users.
> 
> Works for me. I completely agree with that.

Some Zen3 platforms (at least Ryzen SKUs) *do* enumerate AVIC in CPUID:

> $ cat /proc/cpuinfo | grep -E '(model[[:space:]]{2,})|family|step' | head -n3
> cpu family      : 25
> model           : 33
> stepping        : 2
>> $   cat /proc/cpuinfo | grep avic | head -n1
> flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov
> pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt pdpe1gb
> rdtscp lm constant_tsc rep_good nopl xtopology nonstop_tsc cpuid extd_apicid
> aperfmperf rapl pni pclmulqdq monitor ssse3 fma cx16 sse4_1 sse4_2 x2apic movbe
> popcnt aes xsave avx f16c rdrand lahf_lm cmp_legacy svm extapic cr8_legacy
> abm sse4a misalignsse 3dnowprefetch osvw ibs skinit wdt tce topoext perfctr_core
> perfctr_nb bpext perfctr_llc mwaitx cpb cat_l3 cdp_l3 hw_pstate ssbd mba ibrs
> ibpb stibp vmmcall fsgsbase bmi1 avx2 smep bmi2 erms invpcid cqm rdt_a rdseed
> adx smap clflushopt clwb sha_ni xsaveopt xsavec xgetbv1 xsaves cqm_llc
> cqm_occup_llc cqm_mbm_total cqm_mbm_local user_shstk clzero irperf xsaveerptr
> rdpru wbnoinvd arat npt lbrv svm_lock nrip_save tsc_scale vmcb_clean flushbyasid
> decodeassists pausefilter pfthreshold               -> avic <-
> v_vmsave_vmload vgif v_spec_ctrl umip pku ospke vaes vpclmulqdq rdpid overflow_recov
> succor smca fsrm debug_swap
>
> $ cat /sys/module/kvm_amd/parameters/force_avic
> N
>
> $ dmesg | grep AVIC
> kvm_amd: AVIC enabled

As you can see, currently AVIC works there even without force_avic=1 so why
now hide it behind that parameter if errata #1235 is supposedly not present
on Zen3?

Also, this platform is apparently confident enough that the AVIC silicon is
working correctly there to expose it in CPUID - maybe because that's CPU
stepping 2 instead of the initial 0?
> Thanks,
> Naveen

Thanks,
Maciej

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

[Sean Christopherson]

On Tue, Aug 19, 2025, Maciej S. Szmigiero wrote:
> On 18.07.2025 10:19, Naveen N Rao wrote:
> As you can see, currently AVIC works there even without force_avic=1 so why
> now hide it behind that parameter if errata #1235 is supposedly not present
> on Zen3?

Yeah, I'm aligned with keeping the current semantics for force_avic (and by
extension for avic), and I'm pretty sure Naveen is as well.

> Also, this platform is apparently confident enough that the AVIC silicon is
> working correctly there to expose it in CPUID - maybe because that's CPU
> stepping 2 instead of the initial 0?

My vote is still to only enable AVIC by default for Zen4+.  The story for Zen3
and earlier is just too messy.