From: Naveen N Rao <naveen@kernel.org>

AVIC and x2AVIC are fully functional since Zen 4, with no known hardware
errata.  Enable AVIC and x2AVIC by default on Zen4+ so long as x2AVIC is
supported (to avoid enabling partial support for APIC virtualization by
default).

Internally, convert "avic" to an integer so that KVM can identify if the
user has asked to explicitly enable or disable AVIC, i.e. so that KVM
doesn't override an explicit 'y' from the user.  Arbitrarily use -1 to
denote auto-mode, and accept the string "auto" for the module param in
addition to standard boolean values, i.e. continue to allow to the user
configure the "avic" module parameter to explicitly enable/disable AVIC.

To again maintain backward compatibility with a standard boolean param,
set KERNEL_PARAM_OPS_FL_NOARG, which tells the params infrastructure to
allow empty values for %true, i.e. to interpret a bare "avic" as "avic=y".
Take care to check for a NULL @val when looking for "auto"!

Lastly, always print "avic" as a boolean, since auto-mode is resolved
during module initialization, i.e. the user should never see "auto" in
sysfs.

Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/avic.c | 39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index e059dcae6945..5cccee755213 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -64,12 +64,31 @@
 
 static_assert(__AVIC_GATAG(AVIC_VM_ID_MASK, AVIC_VCPU_IDX_MASK) == -1u);
 
+#define AVIC_AUTO_MODE -1
+
+static int avic_param_set(const char *val, const struct kernel_param *kp)
+{
+	if (val && sysfs_streq(val, "auto")) {
+		*(int *)kp->arg = AVIC_AUTO_MODE;
+		return 0;
+	}
+
+	return param_set_bint(val, kp);
+}
+static const struct kernel_param_ops avic_ops = {
+	.flags = KERNEL_PARAM_OPS_FL_NOARG,
+	.set = avic_param_set,
+	.get = param_get_bool,
+};
+
 /*
- * enable / disable AVIC.  Because the defaults differ for APICv
- * support between VMX and SVM we cannot use module_param_named.
+ * Enable / disable AVIC.  In "auto" mode (default behavior), AVIC is enabled
+ * for Zen4+ CPUs with x2AVIC (and all other criteria for enablement are met).
  */
-static bool avic;
-module_param(avic, bool, 0444);
+static int avic = AVIC_AUTO_MODE;
+module_param_cb(avic, &avic_ops, &avic, 0444);
+__MODULE_PARM_TYPE(avic, "bool");
+
 module_param(enable_ipiv, bool, 0444);
 
 static bool force_avic;
@@ -1151,6 +1170,18 @@ void avic_vcpu_unblocking(struct kvm_vcpu *vcpu)
 
 static bool __init avic_want_avic_enable(void)
 {
+	/*
+	 * In "auto" mode, enable AVIC by default for Zen4+ if x2AVIC is
+	 * supported (to avoid enabling partial support by default, and because
+	 * x2AVIC should be supported by all Zen4+ CPUs).  Explicitly check for
+	 * family 0x19 and later (Zen5+), as the kernel's synthetic ZenX flags
+	 * aren't inclusive of previous generations, i.e. the kernel will set
+	 * at most one ZenX feature flag.
+	 */
+	if (avic == AVIC_AUTO_MODE)
+		avic = boot_cpu_has(X86_FEATURE_X2AVIC) &&
+		       (boot_cpu_data.x86 > 0x19 || cpu_feature_enabled(X86_FEATURE_ZEN4));
+
 	if (!avic || !npt_enabled)
 		return false;
 
-- 
2.51.0.470.ga7dc726c21-goog

On Thu, Sep 18, 2025 at 05:21:36PM -0700, Sean Christopherson wrote:
> KVM: SVM: Enable AVIC by default for Zen4+ if x2AVIC is support
							  ^^ supported

> From: Naveen N Rao <naveen@kernel.org>

_ From: Naveen N Rao (AMD) <naveen@kernel.org>

> 
> AVIC and x2AVIC are fully functional since Zen 4, with no known hardware
> errata.  Enable AVIC and x2AVIC by default on Zen4+ so long as x2AVIC is
> supported (to avoid enabling partial support for APIC virtualization by
> default).
> 
> Internally, convert "avic" to an integer so that KVM can identify if the
> user has asked to explicitly enable or disable AVIC, i.e. so that KVM
> doesn't override an explicit 'y' from the user.  Arbitrarily use -1 to
> denote auto-mode, and accept the string "auto" for the module param in
> addition to standard boolean values, i.e. continue to allow to the user
						       allow the user to

> configure the "avic" module parameter to explicitly enable/disable AVIC.
> 
> To again maintain backward compatibility with a standard boolean param,
> set KERNEL_PARAM_OPS_FL_NOARG, which tells the params infrastructure to
> allow empty values for %true, i.e. to interpret a bare "avic" as "avic=y".
> Take care to check for a NULL @val when looking for "auto"!
> 
> Lastly, always print "avic" as a boolean, since auto-mode is resolved
> during module initialization, i.e. the user should never see "auto" in
> sysfs.
> 
> Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/avic.c | 39 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index e059dcae6945..5cccee755213 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -64,12 +64,31 @@
>  
>  static_assert(__AVIC_GATAG(AVIC_VM_ID_MASK, AVIC_VCPU_IDX_MASK) == -1u);
>  
> +#define AVIC_AUTO_MODE -1
> +
> +static int avic_param_set(const char *val, const struct kernel_param *kp)
> +{
> +	if (val && sysfs_streq(val, "auto")) {
> +		*(int *)kp->arg = AVIC_AUTO_MODE;
> +		return 0;
> +	}
> +
> +	return param_set_bint(val, kp);
> +}

Nit: missing newline.

> +static const struct kernel_param_ops avic_ops = {
> +	.flags = KERNEL_PARAM_OPS_FL_NOARG,
> +	.set = avic_param_set,
> +	.get = param_get_bool,
> +};
> +
>  /*
> - * enable / disable AVIC.  Because the defaults differ for APICv
> - * support between VMX and SVM we cannot use module_param_named.
> + * Enable / disable AVIC.  In "auto" mode (default behavior), AVIC is enabled
> + * for Zen4+ CPUs with x2AVIC (and all other criteria for enablement are met).
>   */
> -static bool avic;
> -module_param(avic, bool, 0444);
> +static int avic = AVIC_AUTO_MODE;
> +module_param_cb(avic, &avic_ops, &avic, 0444);
> +__MODULE_PARM_TYPE(avic, "bool");
> +
>  module_param(enable_ipiv, bool, 0444);
>  
>  static bool force_avic;
> @@ -1151,6 +1170,18 @@ void avic_vcpu_unblocking(struct kvm_vcpu *vcpu)
>  
>  static bool __init avic_want_avic_enable(void)
>  {
> +	/*
> +	 * In "auto" mode, enable AVIC by default for Zen4+ if x2AVIC is
> +	 * supported (to avoid enabling partial support by default, and because
> +	 * x2AVIC should be supported by all Zen4+ CPUs).  Explicitly check for
> +	 * family 0x19 and later (Zen5+), as the kernel's synthetic ZenX flags
> +	 * aren't inclusive of previous generations, i.e. the kernel will set
> +	 * at most one ZenX feature flag.
> +	 */
> +	if (avic == AVIC_AUTO_MODE)
> +		avic = boot_cpu_has(X86_FEATURE_X2AVIC) &&

This can use cpu_feature_enabled() as well, I think.

> +		       (boot_cpu_data.x86 > 0x19 || cpu_feature_enabled(X86_FEATURE_ZEN4));
> +
>  	if (!avic || !npt_enabled)
>  		return false;

Otherwise, this LGTM.


- Naveen

On Fri, Sep 19, 2025, Naveen N Rao wrote:
> On Thu, Sep 18, 2025 at 05:21:36PM -0700, Sean Christopherson wrote:
> > @@ -1151,6 +1170,18 @@ void avic_vcpu_unblocking(struct kvm_vcpu *vcpu)
> >  
> >  static bool __init avic_want_avic_enable(void)
> >  {
> > +	/*
> > +	 * In "auto" mode, enable AVIC by default for Zen4+ if x2AVIC is
> > +	 * supported (to avoid enabling partial support by default, and because
> > +	 * x2AVIC should be supported by all Zen4+ CPUs).  Explicitly check for
> > +	 * family 0x19 and later (Zen5+), as the kernel's synthetic ZenX flags
> > +	 * aren't inclusive of previous generations, i.e. the kernel will set
> > +	 * at most one ZenX feature flag.
> > +	 */
> > +	if (avic == AVIC_AUTO_MODE)
> > +		avic = boot_cpu_has(X86_FEATURE_X2AVIC) &&
> 
> This can use cpu_feature_enabled() as well, I think.

It could, but I'm going to leave it as boot_cpu_has() for now, purely because
the existing code uses boot_cpu_has() for X2AVIC and mixing the two adds
"complexity" where none exists.

I'm definitely not opposed to using cpu_feature_enabled() in general, just not
in this case (of course, we could just swap them all, but meh, it's init code).