Re: [PATCH v3 6/6] KVM: SVM: Enable AVIC by default for Zen4+ if x2AVIC is support

[Naveen N Rao]

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@xxxxxxxxxx>

_ From: Naveen N Rao (AMD) <naveen@xxxxxxxxxx>

> 
> 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@xxxxxxxxxx>
> Co-developed-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
>  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