[PATCH v3 0/6] KVM: SVM: Enable AVIC for Zen4+ (if x2AVIC)

[PATCH v3 0/6] KVM: SVM: Enable AVIC for Zen4+ (if x2AVIC)

[Sean Christopherson]

Enable AVIC by default for Zen4+, so long as x2AVIC is supported (which should
be the case if AVIC is supported).

v3:
 - Don't advise the user to enable force_avic. [Naveen]
 - Gather AVIC related module params in avic.c (by moving code/helpers to
   avic.c).
 - Print "AVIC enabled" even when it's forced.
 - Enable by default iff x2AVIC is supported.
 - Use "auto" to select KVM's automatic/default behavior.

v2: https://lore.kernel.org/all/cover.1756993734.git.naveen@kernel.org

v1: http://lkml.kernel.org/r/20250626145122.2228258-1-naveen@kernel.org

Naveen N Rao (1):
  KVM: SVM: Enable AVIC by default for Zen4+ if x2AVIC is support

Sean Christopherson (5):
  KVM: SVM: Move x2AVIC MSR interception helper to avic.c
  KVM: SVM: Update "APICv in x2APIC without x2AVIC" in avic.c, not svm.c
  KVM: SVM: Always print "AVIC enabled" separately, even when force
    enabled
  KVM: SVM: Don't advise the user to do force_avic=y (when x2AVIC is
    detected)
  KVM: SVM: Move global "avic" variable to avic.c

 arch/x86/kvm/svm/avic.c | 149 +++++++++++++++++++++++++++++++++-------
 arch/x86/kvm/svm/svm.c  |  62 +----------------
 arch/x86/kvm/svm/svm.h  |   4 +-
 3 files changed, 125 insertions(+), 90 deletions(-)


base-commit: c8fbf7ceb2ae3f64b0c377c8c21f6df577a13eb4
-- 
2.51.0.470.ga7dc726c21-goog

[PATCH v3 1/6] KVM: SVM: Move x2AVIC MSR interception helper to avic.c

[Sean Christopherson]

Move svm_set_x2apic_msr_interception() to avic.c as it's only relevant
when x2AVIC is enabled/supported and only called by AVIC code.  In
addition to scoping AVIC code to avic.c, this will allow burying the
global x2avic_enabled variable in avic.

Opportunistically rename the helper to explicitly scope it to "avic".

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/avic.c | 57 ++++++++++++++++++++++++++++++++++++++---
 arch/x86/kvm/svm/svm.c  | 49 -----------------------------------
 arch/x86/kvm/svm/svm.h  |  1 -
 3 files changed, 54 insertions(+), 53 deletions(-)

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index a34c5c3b164e..478a18208a76 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -79,6 +79,57 @@ static bool next_vm_id_wrapped = 0;
 static DEFINE_SPINLOCK(svm_vm_data_hash_lock);
 bool x2avic_enabled;
 
+
+static void avic_set_x2apic_msr_interception(struct vcpu_svm *svm,
+					     bool intercept)
+{
+	static const u32 x2avic_passthrough_msrs[] = {
+		X2APIC_MSR(APIC_ID),
+		X2APIC_MSR(APIC_LVR),
+		X2APIC_MSR(APIC_TASKPRI),
+		X2APIC_MSR(APIC_ARBPRI),
+		X2APIC_MSR(APIC_PROCPRI),
+		X2APIC_MSR(APIC_EOI),
+		X2APIC_MSR(APIC_RRR),
+		X2APIC_MSR(APIC_LDR),
+		X2APIC_MSR(APIC_DFR),
+		X2APIC_MSR(APIC_SPIV),
+		X2APIC_MSR(APIC_ISR),
+		X2APIC_MSR(APIC_TMR),
+		X2APIC_MSR(APIC_IRR),
+		X2APIC_MSR(APIC_ESR),
+		X2APIC_MSR(APIC_ICR),
+		X2APIC_MSR(APIC_ICR2),
+
+		/*
+		 * Note!  Always intercept LVTT, as TSC-deadline timer mode
+		 * isn't virtualized by hardware, and the CPU will generate a
+		 * #GP instead of a #VMEXIT.
+		 */
+		X2APIC_MSR(APIC_LVTTHMR),
+		X2APIC_MSR(APIC_LVTPC),
+		X2APIC_MSR(APIC_LVT0),
+		X2APIC_MSR(APIC_LVT1),
+		X2APIC_MSR(APIC_LVTERR),
+		X2APIC_MSR(APIC_TMICT),
+		X2APIC_MSR(APIC_TMCCT),
+		X2APIC_MSR(APIC_TDCR),
+	};
+	int i;
+
+	if (intercept == svm->x2avic_msrs_intercepted)
+		return;
+
+	if (!x2avic_enabled)
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(x2avic_passthrough_msrs); i++)
+		svm_set_intercept_for_msr(&svm->vcpu, x2avic_passthrough_msrs[i],
+					  MSR_TYPE_RW, intercept);
+
+	svm->x2avic_msrs_intercepted = intercept;
+}
+
 static void avic_activate_vmcb(struct vcpu_svm *svm)
 {
 	struct vmcb *vmcb = svm->vmcb01.ptr;
@@ -99,7 +150,7 @@ static void avic_activate_vmcb(struct vcpu_svm *svm)
 		vmcb->control.int_ctl |= X2APIC_MODE_MASK;
 		vmcb->control.avic_physical_id |= X2AVIC_MAX_PHYSICAL_ID;
 		/* Disabling MSR intercept for x2APIC registers */
-		svm_set_x2apic_msr_interception(svm, false);
+		avic_set_x2apic_msr_interception(svm, false);
 	} else {
 		/*
 		 * Flush the TLB, the guest may have inserted a non-APIC
@@ -110,7 +161,7 @@ static void avic_activate_vmcb(struct vcpu_svm *svm)
 		/* For xAVIC and hybrid-xAVIC modes */
 		vmcb->control.avic_physical_id |= AVIC_MAX_PHYSICAL_ID;
 		/* Enabling MSR intercept for x2APIC registers */
-		svm_set_x2apic_msr_interception(svm, true);
+		avic_set_x2apic_msr_interception(svm, true);
 	}
 }
 
@@ -130,7 +181,7 @@ static void avic_deactivate_vmcb(struct vcpu_svm *svm)
 		return;
 
 	/* Enabling MSR intercept for x2APIC registers */
-	svm_set_x2apic_msr_interception(svm, true);
+	avic_set_x2apic_msr_interception(svm, true);
 }
 
 /* Note:
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 67f4eed01526..3bcb88b2e617 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -736,55 +736,6 @@ static void svm_recalc_lbr_msr_intercepts(struct kvm_vcpu *vcpu)
 		svm_set_intercept_for_msr(vcpu, MSR_IA32_DEBUGCTLMSR, MSR_TYPE_RW, intercept);
 }
 
-void svm_set_x2apic_msr_interception(struct vcpu_svm *svm, bool intercept)
-{
-	static const u32 x2avic_passthrough_msrs[] = {
-		X2APIC_MSR(APIC_ID),
-		X2APIC_MSR(APIC_LVR),
-		X2APIC_MSR(APIC_TASKPRI),
-		X2APIC_MSR(APIC_ARBPRI),
-		X2APIC_MSR(APIC_PROCPRI),
-		X2APIC_MSR(APIC_EOI),
-		X2APIC_MSR(APIC_RRR),
-		X2APIC_MSR(APIC_LDR),
-		X2APIC_MSR(APIC_DFR),
-		X2APIC_MSR(APIC_SPIV),
-		X2APIC_MSR(APIC_ISR),
-		X2APIC_MSR(APIC_TMR),
-		X2APIC_MSR(APIC_IRR),
-		X2APIC_MSR(APIC_ESR),
-		X2APIC_MSR(APIC_ICR),
-		X2APIC_MSR(APIC_ICR2),
-
-		/*
-		 * Note!  Always intercept LVTT, as TSC-deadline timer mode
-		 * isn't virtualized by hardware, and the CPU will generate a
-		 * #GP instead of a #VMEXIT.
-		 */
-		X2APIC_MSR(APIC_LVTTHMR),
-		X2APIC_MSR(APIC_LVTPC),
-		X2APIC_MSR(APIC_LVT0),
-		X2APIC_MSR(APIC_LVT1),
-		X2APIC_MSR(APIC_LVTERR),
-		X2APIC_MSR(APIC_TMICT),
-		X2APIC_MSR(APIC_TMCCT),
-		X2APIC_MSR(APIC_TDCR),
-	};
-	int i;
-
-	if (intercept == svm->x2avic_msrs_intercepted)
-		return;
-
-	if (!x2avic_enabled)
-		return;
-
-	for (i = 0; i < ARRAY_SIZE(x2avic_passthrough_msrs); i++)
-		svm_set_intercept_for_msr(&svm->vcpu, x2avic_passthrough_msrs[i],
-					  MSR_TYPE_RW, intercept);
-
-	svm->x2avic_msrs_intercepted = intercept;
-}
-
 void svm_vcpu_free_msrpm(void *msrpm)
 {
 	__free_pages(virt_to_page(msrpm), get_order(MSRPM_SIZE));
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 5d39c0b17988..1e612bbfd36d 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -699,7 +699,6 @@ void svm_set_gif(struct vcpu_svm *svm, bool value);
 int svm_invoke_exit_handler(struct kvm_vcpu *vcpu, u64 exit_code);
 void set_msr_interception(struct kvm_vcpu *vcpu, u32 *msrpm, u32 msr,
 			  int read, int write);
-void svm_set_x2apic_msr_interception(struct vcpu_svm *svm, bool disable);
 void svm_complete_interrupt_delivery(struct kvm_vcpu *vcpu, int delivery_mode,
 				     int trig_mode, int vec);
 
-- 
2.51.0.470.ga7dc726c21-goog

Re: [PATCH v3 1/6] KVM: SVM: Move x2AVIC MSR interception helper to avic.c

[Naveen N Rao]

On Thu, Sep 18, 2025 at 05:21:31PM -0700, Sean Christopherson wrote:
> Move svm_set_x2apic_msr_interception() to avic.c as it's only relevant
> when x2AVIC is enabled/supported and only called by AVIC code.  In
> addition to scoping AVIC code to avic.c, this will allow burying the
> global x2avic_enabled variable in avic.
> 
> Opportunistically rename the helper to explicitly scope it to "avic".
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/avic.c | 57 ++++++++++++++++++++++++++++++++++++++---
>  arch/x86/kvm/svm/svm.c  | 49 -----------------------------------
>  arch/x86/kvm/svm/svm.h  |  1 -
>  3 files changed, 54 insertions(+), 53 deletions(-)

Reviewed-by: Naveen N Rao (AMD) <naveen@kernel.org>

- Naveen

[PATCH v3 2/6] KVM: SVM: Update "APICv in x2APIC without x2AVIC" in avic.c, not svm.c

[Sean Christopherson]

Set the "allow_apicv_in_x2apic_without_x2apic_virtualization" flag as part
of avic_hardware_setup() instead of handling in svm_hardware_setup(), and
make x2avic_enabled local to avic.c (setting the flag was the only use in
svm.c).

Opportunistically tag avic_hardware_setup() with __init to make it clear
that nothing untoward is happening with svm_x86_ops.

No functional change intended (aside from the side effects of tagging
avic_hardware_setup() with __init).

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/avic.c | 6 ++++--
 arch/x86/kvm/svm/svm.c  | 4 +---
 arch/x86/kvm/svm/svm.h  | 3 +--
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index 478a18208a76..683411442476 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -77,7 +77,7 @@ static DEFINE_HASHTABLE(svm_vm_data_hash, SVM_VM_DATA_HASH_BITS);
 static u32 next_vm_id = 0;
 static bool next_vm_id_wrapped = 0;
 static DEFINE_SPINLOCK(svm_vm_data_hash_lock);
-bool x2avic_enabled;
+static bool x2avic_enabled;
 
 
 static void avic_set_x2apic_msr_interception(struct vcpu_svm *svm,
@@ -1147,7 +1147,7 @@ void avic_vcpu_unblocking(struct kvm_vcpu *vcpu)
  * - Hypervisor can support both xAVIC and x2AVIC in the same guest.
  * - The mode can be switched at run-time.
  */
-bool avic_hardware_setup(void)
+bool __init avic_hardware_setup(struct kvm_x86_ops *svm_ops)
 {
 	if (!npt_enabled)
 		return false;
@@ -1182,6 +1182,8 @@ bool avic_hardware_setup(void)
 	x2avic_enabled = boot_cpu_has(X86_FEATURE_X2AVIC);
 	if (x2avic_enabled)
 		pr_info("x2AVIC enabled\n");
+	else
+		svm_ops->allow_apicv_in_x2apic_without_x2apic_virtualization = true;
 
 	/*
 	 * Disable IPI virtualization for AMD Family 17h CPUs (Zen1 and Zen2)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 3bcb88b2e617..d4643dce7c91 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -5354,15 +5354,13 @@ static __init int svm_hardware_setup(void)
 			goto err;
 	}
 
-	enable_apicv = avic = avic && avic_hardware_setup();
+	enable_apicv = avic = avic && avic_hardware_setup(&svm_x86_ops);
 
 	if (!enable_apicv) {
 		enable_ipiv = false;
 		svm_x86_ops.vcpu_blocking = NULL;
 		svm_x86_ops.vcpu_unblocking = NULL;
 		svm_x86_ops.vcpu_get_apicv_inhibit_reasons = NULL;
-	} else if (!x2avic_enabled) {
-		svm_x86_ops.allow_apicv_in_x2apic_without_x2apic_virtualization = true;
 	}
 
 	if (vls) {
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 1e612bbfd36d..811513c8b566 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -48,7 +48,6 @@ extern bool npt_enabled;
 extern int nrips;
 extern int vgif;
 extern bool intercept_smi;
-extern bool x2avic_enabled;
 extern bool vnmi;
 extern int lbrv;
 
@@ -800,7 +799,7 @@ extern struct kvm_x86_nested_ops svm_nested_ops;
 	BIT(APICV_INHIBIT_REASON_PHYSICAL_ID_TOO_BIG)	\
 )
 
-bool avic_hardware_setup(void);
+bool __init avic_hardware_setup(struct kvm_x86_ops *svm_ops);
 int avic_ga_log_notifier(u32 ga_tag);
 void avic_vm_destroy(struct kvm *kvm);
 int avic_vm_init(struct kvm *kvm);
-- 
2.51.0.470.ga7dc726c21-goog

Re: [PATCH v3 2/6] KVM: SVM: Update "APICv in x2APIC without x2AVIC" in avic.c, not svm.c

[Naveen N Rao]

On Thu, Sep 18, 2025 at 05:21:32PM -0700, Sean Christopherson wrote:
> Set the "allow_apicv_in_x2apic_without_x2apic_virtualization" flag as part
> of avic_hardware_setup() instead of handling in svm_hardware_setup(), and
> make x2avic_enabled local to avic.c (setting the flag was the only use in
> svm.c).
> 
> Opportunistically tag avic_hardware_setup() with __init to make it clear
> that nothing untoward is happening with svm_x86_ops.
> 
> No functional change intended (aside from the side effects of tagging
> avic_hardware_setup() with __init).
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/avic.c | 6 ++++--
>  arch/x86/kvm/svm/svm.c  | 4 +---
>  arch/x86/kvm/svm/svm.h  | 3 +--
>  3 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index 478a18208a76..683411442476 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -77,7 +77,7 @@ static DEFINE_HASHTABLE(svm_vm_data_hash, SVM_VM_DATA_HASH_BITS);
>  static u32 next_vm_id = 0;
>  static bool next_vm_id_wrapped = 0;
>  static DEFINE_SPINLOCK(svm_vm_data_hash_lock);
> -bool x2avic_enabled;
> +static bool x2avic_enabled;
>  
>  
>  static void avic_set_x2apic_msr_interception(struct vcpu_svm *svm,
> @@ -1147,7 +1147,7 @@ void avic_vcpu_unblocking(struct kvm_vcpu *vcpu)
>   * - Hypervisor can support both xAVIC and x2AVIC in the same guest.
>   * - The mode can be switched at run-time.
>   */
> -bool avic_hardware_setup(void)
> +bool __init avic_hardware_setup(struct kvm_x86_ops *svm_ops)
>  {
>  	if (!npt_enabled)
>  		return false;
> @@ -1182,6 +1182,8 @@ bool avic_hardware_setup(void)
>  	x2avic_enabled = boot_cpu_has(X86_FEATURE_X2AVIC);
>  	if (x2avic_enabled)
>  		pr_info("x2AVIC enabled\n");
> +	else
> +		svm_ops->allow_apicv_in_x2apic_without_x2apic_virtualization = true;

I'm not entirely convinced that this is better since svm_x86_ops fields 
are now being updated outside of svm.c. But, I do see your point about 
limiting x2avic_enabled to avic.c

Would it be better to name this field as svm_x86_ops here too, so it is 
at least easy to grep and find?

Otherwise, for this patch:
Acked-by: Naveen N Rao (AMD) <naveen@kernel.org>


- Naveen

Re: [PATCH v3 2/6] KVM: SVM: Update "APICv in x2APIC without x2AVIC" in avic.c, not svm.c

[Sean Christopherson]

+Intel folks (question at the bottom regarding vt_x86_ops)

On Fri, Sep 19, 2025, Naveen N Rao wrote:
> On Thu, Sep 18, 2025 at 05:21:32PM -0700, Sean Christopherson wrote:
> > Set the "allow_apicv_in_x2apic_without_x2apic_virtualization" flag as part
> > of avic_hardware_setup() instead of handling in svm_hardware_setup(), and
> > make x2avic_enabled local to avic.c (setting the flag was the only use in
> > svm.c).
> > 
> > Opportunistically tag avic_hardware_setup() with __init to make it clear
> > that nothing untoward is happening with svm_x86_ops.
> > 
> > No functional change intended (aside from the side effects of tagging
> > avic_hardware_setup() with __init).
> > 
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >  arch/x86/kvm/svm/avic.c | 6 ++++--
> >  arch/x86/kvm/svm/svm.c  | 4 +---
> >  arch/x86/kvm/svm/svm.h  | 3 +--
> >  3 files changed, 6 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> > index 478a18208a76..683411442476 100644
> > --- a/arch/x86/kvm/svm/avic.c
> > +++ b/arch/x86/kvm/svm/avic.c
> > @@ -77,7 +77,7 @@ static DEFINE_HASHTABLE(svm_vm_data_hash, SVM_VM_DATA_HASH_BITS);
> >  static u32 next_vm_id = 0;
> >  static bool next_vm_id_wrapped = 0;
> >  static DEFINE_SPINLOCK(svm_vm_data_hash_lock);
> > -bool x2avic_enabled;
> > +static bool x2avic_enabled;
> >  
> >  
> >  static void avic_set_x2apic_msr_interception(struct vcpu_svm *svm,
> > @@ -1147,7 +1147,7 @@ void avic_vcpu_unblocking(struct kvm_vcpu *vcpu)
> >   * - Hypervisor can support both xAVIC and x2AVIC in the same guest.
> >   * - The mode can be switched at run-time.
> >   */
> > -bool avic_hardware_setup(void)
> > +bool __init avic_hardware_setup(struct kvm_x86_ops *svm_ops)
> >  {
> >  	if (!npt_enabled)
> >  		return false;
> > @@ -1182,6 +1182,8 @@ bool avic_hardware_setup(void)
> >  	x2avic_enabled = boot_cpu_has(X86_FEATURE_X2AVIC);
> >  	if (x2avic_enabled)
> >  		pr_info("x2AVIC enabled\n");
> > +	else
> > +		svm_ops->allow_apicv_in_x2apic_without_x2apic_virtualization = true;
> 
> I'm not entirely convinced that this is better since svm_x86_ops fields 
> are now being updated outside of svm.c.

That doesn't bother me, e.g. the pmu_ops buried in svm_x86_ops come from
arch/x86/kvm/svm/pmu.c.  Eww, and arch/x86/kvm/svm/svm_onhyperv.h already accesses
svm_x86_ops, but in the grossest way possible.

FWIW, I don't love splitting the APIC virtualization updates between
svm_hardware_setup() and avic_hardware_setup(), but overall I do think that's the
best balance between bundling all code together and making it easy(ish) for
readers to figure out what's going on.

> But, I do see your point about  limiting x2avic_enabled to avic.c
> 
> Would it be better to name this field as svm_x86_ops here too, so it is 
> at least easy to grep and find?

I didn't want to create a potential variable shadowing "problem".  The alternative
would be to expose svm_x86_ops via svm.h.  That's not my first choice, but it's
the route that was taken for the TDX vs. VMX split (vt_init_ops and vt_x86_ops
are globally visible, even though they _could_ have been explicitly passed in
as parameters to {tdx,vmx}_hardware_setup()).

Question then.  Does anyone have a preference/opinion between explicitly passing
in ops to vendor specific helpers, vs. making {svm,vt}_x86_ops globally visible?

I don't love creating "hidden" dependencies, in quotes because in this case it's
relatively easy to establish that the setup() helpers modify {svm,vt}_x86_ops,
i.e. surprises should be rare.

On the other hand, I do agree it's helpful to be able to see exactly where
{svm,vt}_x86_ops are updated.

And most importantly, I want to be consistent between VMX and SVM, i.e. I want
to pick one and stick with it.

Re: [PATCH v3 2/6] KVM: SVM: Update "APICv in x2APIC without x2AVIC" in avic.c, not svm.c

[Chao Gao]

>Question then.  Does anyone have a preference/opinion between explicitly passing
>in ops to vendor specific helpers, vs. making {svm,vt}_x86_ops globally visible?
>
>I don't love creating "hidden" dependencies, in quotes because in this case it's
>relatively easy to establish that the setup() helpers modify {svm,vt}_x86_ops,
>i.e. surprises should be rare.
>
>On the other hand, I do agree it's helpful to be able to see exactly where
>{svm,vt}_x86_ops are updated.

I think passing in ops to vendor-specific helpers looks a bit indirect as the
parameter should always be svm_x86_ops for AMD or vt_x86_ops for Intel.

I slightly prefer making {svm,vt}_x86_ops globally visible.

>
>And most importantly, I want to be consistent between VMX and SVM, i.e. I want
>to pick one and stick with it.

[PATCH v3 3/6] KVM: SVM: Always print "AVIC enabled" separately, even when force enabled

[Sean Christopherson]

Print the customary "AVIC enabled" informational message even when AVIC is
force enabled on a system that doesn't advertise supported for AVIC in
CPUID, as not printing the standard message can confuse users and tools.

Opportunistically clean up the scary message when AVIC is force enabled,
but keep it as separate message so that it is printed at level "warn",
versus the standard message only being printed for level "info".

Suggested-by: Naveen N Rao <naveen@kernel.org>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/avic.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index 683411442476..bafef2f75af2 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -1167,16 +1167,15 @@ bool __init avic_hardware_setup(struct kvm_x86_ops *svm_ops)
 		return false;
 	}
 
-	if (boot_cpu_has(X86_FEATURE_AVIC)) {
-		pr_info("AVIC enabled\n");
-	} else if (force_avic) {
-		/*
-		 * Some older systems does not advertise AVIC support.
-		 * See Revision Guide for specific AMD processor for more detail.
-		 */
-		pr_warn("AVIC is not supported in CPUID but force enabled");
-		pr_warn("Your system might crash and burn");
-	}
+	/*
+	 * Print a scary message if AVIC is force enabled to make it abundantly
+	 * clear that ignoring CPUID could have repercussions.  See Revision
+	 * Guide for specific AMD processor for more details.
+	 */
+	if (!boot_cpu_has(X86_FEATURE_AVIC))
+		pr_warn("AVIC unsupported in CPUID but force enabled, your system might crash and burn\n");
+
+	pr_info("AVIC enabled\n");
 
 	/* AVIC is a prerequisite for x2AVIC. */
 	x2avic_enabled = boot_cpu_has(X86_FEATURE_X2AVIC);
-- 
2.51.0.470.ga7dc726c21-goog

Re: [PATCH v3 3/6] KVM: SVM: Always print "AVIC enabled" separately, even when force enabled

[Naveen N Rao]

On Thu, Sep 18, 2025 at 05:21:33PM -0700, Sean Christopherson wrote:
> Print the customary "AVIC enabled" informational message even when AVIC is
> force enabled on a system that doesn't advertise supported for AVIC in
						   ^^ support

> CPUID, as not printing the standard message can confuse users and tools.
> 
> Opportunistically clean up the scary message when AVIC is force enabled,
> but keep it as separate message so that it is printed at level "warn",
		 a separate message

> versus the standard message only being printed for level "info".
> 
> Suggested-by: Naveen N Rao <naveen@kernel.org>

Please use (with attribution):
Naveen N Rao (AMD) <naveen@kernel.org>

> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/avic.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)

Reviewed-by: Naveen N Rao (AMD) <naveen@kernel.org>


- Naveen

[PATCH v3 4/6] KVM: SVM: Don't advise the user to do force_avic=y (when x2AVIC is detected)

[Sean Christopherson]

Don't advise the end user to try to force enable AVIC when x2AVIC is
reported as supported in CPUID, as forcefully enabling AVIC isn't something
that should be done lightly.  E.g. some Zen4 client systems hide AVIC but
leave x2AVIC behind, and while such a configuration is indeed due to buggy
firmware in the sense the reporting x2AVIC without AVIC is nonsensical,
KVM has no idea _why_ firmware disabled AVIC in the first place.

Suggesting that the user try to run with force_avic=y is sketchy even if
the user explicitly tries to enable AVIC, and will be downright
irresponsible once KVM starts enabling AVIC by default.  Alternatively,
KVM could print the message only when the user explicitly asks for AVIC,
but running with force_avic=y isn't something that should be encouraged
for random users.  force_avic is a useful knob for developers and perhaps
even advanced users, but isn't something that KVM should advertise broadly.

Opportunistically append a newline to the pr_warn() so that it prints out
immediately, and tweak the message to say that AVIC is unsupported instead
of disabled (disabled suggests that the kernel/KVM is somehow responsible).

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

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index bafef2f75af2..497d755c206f 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -1154,10 +1154,8 @@ bool __init avic_hardware_setup(struct kvm_x86_ops *svm_ops)
 
 	/* 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_X2AVIC))
+			pr_warn(FW_BUG "Cannot enable x2AVIC, AVIC is unsupported\n");
 		return false;
 	}
 
-- 
2.51.0.470.ga7dc726c21-goog

Re: [PATCH v3 4/6] KVM: SVM: Don't advise the user to do force_avic=y (when x2AVIC is detected)

[Naveen N Rao]

On Thu, Sep 18, 2025 at 05:21:34PM -0700, Sean Christopherson wrote:
> Don't advise the end user to try to force enable AVIC when x2AVIC is
> reported as supported in CPUID,
			in CPUID but AVIC isn't,

> as forcefully enabling AVIC isn't something
> that should be done lightly.  E.g. some Zen4 client systems hide AVIC but
> leave x2AVIC behind, and while such a configuration is indeed due to buggy
> firmware in the sense the reporting x2AVIC without AVIC is nonsensical,
			^^ that

> KVM has no idea _why_ firmware disabled AVIC in the first place.
> 
> Suggesting that the user try to run with force_avic=y is sketchy even if
> the user explicitly tries to enable AVIC, and will be downright
> irresponsible once KVM starts enabling AVIC by default.  Alternatively,
> KVM could print the message only when the user explicitly asks for AVIC,
> but running with force_avic=y isn't something that should be encouraged
> for random users.  force_avic is a useful knob for developers and perhaps
> even advanced users, but isn't something that KVM should advertise broadly.
> 
> Opportunistically append a newline to the pr_warn() so that it prints out
> immediately, and tweak the message to say that AVIC is unsupported instead
> of disabled (disabled suggests that the kernel/KVM is somehow responsible).
> 
> Suggested-by: Naveen N Rao <naveen@kernel.org>

Suggested-by: Naveen N Rao (AMD) <naveen@kernel.org>

> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/avic.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)

Reviewed-by: Naveen N Rao (AMD) <naveen@kernel.org>


- Naveen

[PATCH v3 5/6] KVM: SVM: Move global "avic" variable to avic.c

[Sean Christopherson]

Move "avic" to avic.c so that it's colocated with the other AVIC specific
globals and module params, and so that avic_hardware_setup() is a bit more
self-contained, e.g. similar to sev_hardware_setup().

Deliberately set enable_apicv in svm.c as it's already globally visible
(defined by kvm.ko, not by kvm-amd.ko), and to clearly capture the
dependency on enable_apicv being initialized (svm_hardware_setup() clears
several AVIC-specific hooks when enable_apicv is disabled).

Alternatively, clearing of the hooks (and enable_ipiv) could be moved to
avic_hardware_setup(), but that's not obviously better, e.g. it's helpful
to isolate the setting of enable_apicv when reading code from the generic
x86 side of the world.

No functional change intended.

Cc: Naveen N Rao (AMD) <naveen@kernel.org>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/avic.c | 32 ++++++++++++++++++++++++--------
 arch/x86/kvm/svm/svm.c  | 11 +----------
 2 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index 497d755c206f..e059dcae6945 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -64,6 +64,14 @@
 
 static_assert(__AVIC_GATAG(AVIC_VM_ID_MASK, AVIC_VCPU_IDX_MASK) == -1u);
 
+/*
+ * 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(enable_ipiv, bool, 0444);
+
 static bool force_avic;
 module_param_unsafe(force_avic, bool, 0444);
 
@@ -1141,15 +1149,9 @@ void avic_vcpu_unblocking(struct kvm_vcpu *vcpu)
 	avic_vcpu_load(vcpu, vcpu->cpu);
 }
 
-/*
- * Note:
- * - The module param avic enable both xAPIC and x2APIC mode.
- * - Hypervisor can support both xAVIC and x2AVIC in the same guest.
- * - The mode can be switched at run-time.
- */
-bool __init avic_hardware_setup(struct kvm_x86_ops *svm_ops)
+static bool __init avic_want_avic_enable(void)
 {
-	if (!npt_enabled)
+	if (!avic || !npt_enabled)
 		return false;
 
 	/* AVIC is a prerequisite for x2AVIC. */
@@ -1174,6 +1176,20 @@ bool __init avic_hardware_setup(struct kvm_x86_ops *svm_ops)
 		pr_warn("AVIC unsupported in CPUID but force enabled, your system might crash and burn\n");
 
 	pr_info("AVIC enabled\n");
+	return true;
+}
+
+/*
+ * Note:
+ * - The module param avic enable both xAPIC and x2APIC mode.
+ * - Hypervisor can support both xAVIC and x2AVIC in the same guest.
+ * - The mode can be switched at run-time.
+ */
+bool __init avic_hardware_setup(struct kvm_x86_ops *svm_ops)
+{
+	avic = avic_want_avic_enable();
+	if (!avic)
+		return false;
 
 	/* AVIC is a prerequisite for x2AVIC. */
 	x2avic_enabled = boot_cpu_has(X86_FEATURE_X2AVIC);
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index d4643dce7c91..c473246f8881 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -158,14 +158,6 @@ 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(enable_ipiv, bool, 0444);
-
 module_param(enable_device_posted_irqs, bool, 0444);
 
 bool __read_mostly dump_invalid_vmcb;
@@ -5354,8 +5346,7 @@ static __init int svm_hardware_setup(void)
 			goto err;
 	}
 
-	enable_apicv = avic = avic && avic_hardware_setup(&svm_x86_ops);
-
+	enable_apicv = avic_hardware_setup(&svm_x86_ops);
 	if (!enable_apicv) {
 		enable_ipiv = false;
 		svm_x86_ops.vcpu_blocking = NULL;
-- 
2.51.0.470.ga7dc726c21-goog

Re: [PATCH v3 5/6] KVM: SVM: Move global "avic" variable to avic.c

[Naveen N Rao]

On Thu, Sep 18, 2025 at 05:21:35PM -0700, Sean Christopherson wrote:
> Move "avic" to avic.c so that it's colocated with the other AVIC specific
> globals and module params, and so that avic_hardware_setup() is a bit more
> self-contained, e.g. similar to sev_hardware_setup().
> 
> Deliberately set enable_apicv in svm.c as it's already globally visible
> (defined by kvm.ko, not by kvm-amd.ko), and to clearly capture the
> dependency on enable_apicv being initialized (svm_hardware_setup() clears
> several AVIC-specific hooks when enable_apicv is disabled).
> 
> Alternatively, clearing of the hooks (and enable_ipiv) could be moved to
> avic_hardware_setup(), but that's not obviously better, e.g. it's helpful
> to isolate the setting of enable_apicv when reading code from the generic
> x86 side of the world.
> 
> No functional change intended.
> 
> Cc: Naveen N Rao (AMD) <naveen@kernel.org>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/avic.c | 32 ++++++++++++++++++++++++--------
>  arch/x86/kvm/svm/svm.c  | 11 +----------
>  2 files changed, 25 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index 497d755c206f..e059dcae6945 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -64,6 +64,14 @@
>  
>  static_assert(__AVIC_GATAG(AVIC_VM_ID_MASK, AVIC_VCPU_IDX_MASK) == -1u);
>  
> +/*
> + * 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(enable_ipiv, bool, 0444);
> +
>  static bool force_avic;
>  module_param_unsafe(force_avic, bool, 0444);
>  
> @@ -1141,15 +1149,9 @@ void avic_vcpu_unblocking(struct kvm_vcpu *vcpu)
>  	avic_vcpu_load(vcpu, vcpu->cpu);
>  }
>  
> -/*
> - * Note:
> - * - The module param avic enable both xAPIC and x2APIC mode.
> - * - Hypervisor can support both xAVIC and x2AVIC in the same guest.
> - * - The mode can be switched at run-time.
> - */
> -bool __init avic_hardware_setup(struct kvm_x86_ops *svm_ops)
> +static bool __init avic_want_avic_enable(void)

Maybe avic_can_enable()?

>  {
> -	if (!npt_enabled)
> +	if (!avic || !npt_enabled)
>  		return false;
>  
>  	/* AVIC is a prerequisite for x2AVIC. */
> @@ -1174,6 +1176,20 @@ bool __init avic_hardware_setup(struct kvm_x86_ops *svm_ops)
>  		pr_warn("AVIC unsupported in CPUID but force enabled, your system might crash and burn\n");
>  
>  	pr_info("AVIC enabled\n");

I think it would be good to keep this in avic_hardware_setup() alongside 
the message printing "x2AVIC enabled".

Otherwise:
Acked-by: Naveen N Rao (AMD) <naveen@kernel.org>


- Naveen

Re: [PATCH v3 5/6] KVM: SVM: Move global "avic" variable to avic.c

[Sean Christopherson]

On Fri, Sep 19, 2025, Naveen N Rao wrote:
> On Thu, Sep 18, 2025 at 05:21:35PM -0700, Sean Christopherson wrote:
> > @@ -1141,15 +1149,9 @@ void avic_vcpu_unblocking(struct kvm_vcpu *vcpu)
> >  	avic_vcpu_load(vcpu, vcpu->cpu);
> >  }
> >  
> > -/*
> > - * Note:
> > - * - The module param avic enable both xAPIC and x2APIC mode.
> > - * - Hypervisor can support both xAVIC and x2AVIC in the same guest.
> > - * - The mode can be switched at run-time.
> > - */
> > -bool __init avic_hardware_setup(struct kvm_x86_ops *svm_ops)
> > +static bool __init avic_want_avic_enable(void)
> 
> Maybe avic_can_enable()?

That was actualy one of my first names, but I didn't want to use "can" because
(to me at least) that doesn't capture that the helper is incorporating input from
the user, i.e. that it's also checking what the user "wants".

I agree the name isn't great.  Does avic_want_avic_enabled() read any better?

> >  {
> > -	if (!npt_enabled)
> > +	if (!avic || !npt_enabled)
> >  		return false;
> >  
> >  	/* AVIC is a prerequisite for x2AVIC. */
> > @@ -1174,6 +1176,20 @@ bool __init avic_hardware_setup(struct kvm_x86_ops *svm_ops)
> >  		pr_warn("AVIC unsupported in CPUID but force enabled, your system might crash and burn\n");
> >  
> >  	pr_info("AVIC enabled\n");
> 
> I think it would be good to keep this in avic_hardware_setup() alongside 
> the message printing "x2AVIC enabled".

+1, looks waaay better that way.

Re: [PATCH v3 5/6] KVM: SVM: Move global "avic" variable to avic.c

[Naveen N Rao]

On Fri, Sep 19, 2025 at 07:44:29AM -0700, Sean Christopherson wrote:
> On Fri, Sep 19, 2025, Naveen N Rao wrote:
> > On Thu, Sep 18, 2025 at 05:21:35PM -0700, Sean Christopherson wrote:
> > > @@ -1141,15 +1149,9 @@ void avic_vcpu_unblocking(struct kvm_vcpu *vcpu)
> > >  	avic_vcpu_load(vcpu, vcpu->cpu);
> > >  }
> > >  
> > > -/*
> > > - * Note:
> > > - * - The module param avic enable both xAPIC and x2APIC mode.
> > > - * - Hypervisor can support both xAVIC and x2AVIC in the same guest.
> > > - * - The mode can be switched at run-time.
> > > - */
> > > -bool __init avic_hardware_setup(struct kvm_x86_ops *svm_ops)
> > > +static bool __init avic_want_avic_enable(void)
> > 
> > Maybe avic_can_enable()?
> 
> That was actualy one of my first names, but I didn't want to use "can" because
> (to me at least) that doesn't capture that the helper is incorporating input from
> the user, i.e. that it's also checking what the user "wants".

Makes sense.

> 
> I agree the name isn't great.  Does avic_want_avic_enabled() read any better?

Yes, though I think avic_want_enabled() suffices. But, I'm ok if you 
want it to be explicit.


- Naveen

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

[Sean Christopherson]

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

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

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

[Sean Christopherson]

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

Re: [PATCH v3 0/6] KVM: SVM: Enable AVIC for Zen4+ (if x2AVIC)

[Naveen N Rao]

On Thu, Sep 18, 2025 at 05:21:30PM -0700, Sean Christopherson wrote:
> Enable AVIC by default for Zen4+, so long as x2AVIC is supported (which should
> be the case if AVIC is supported).
> 
> v3:
>  - Don't advise the user to enable force_avic. [Naveen]
>  - Gather AVIC related module params in avic.c (by moving code/helpers to
>    avic.c).
>  - Print "AVIC enabled" even when it's forced.
>  - Enable by default iff x2AVIC is supported.
>  - Use "auto" to select KVM's automatic/default behavior.
> 
> v2: https://lore.kernel.org/all/cover.1756993734.git.naveen@kernel.org
> 
> v1: http://lkml.kernel.org/r/20250626145122.2228258-1-naveen@kernel.org
> 
> Naveen N Rao (1):
>   KVM: SVM: Enable AVIC by default for Zen4+ if x2AVIC is support
> 
> Sean Christopherson (5):
>   KVM: SVM: Move x2AVIC MSR interception helper to avic.c
>   KVM: SVM: Update "APICv in x2APIC without x2AVIC" in avic.c, not svm.c
>   KVM: SVM: Always print "AVIC enabled" separately, even when force
>     enabled
>   KVM: SVM: Don't advise the user to do force_avic=y (when x2AVIC is
>     detected)
>   KVM: SVM: Move global "avic" variable to avic.c
> 
>  arch/x86/kvm/svm/avic.c | 149 +++++++++++++++++++++++++++++++++-------
>  arch/x86/kvm/svm/svm.c  |  62 +----------------
>  arch/x86/kvm/svm/svm.h  |   4 +-
>  3 files changed, 125 insertions(+), 90 deletions(-)

Thanks for putting this together! I have confirmed that this correctly 
enables AVIC by default on a Zen 5 system and does not do so on a Zen 1 
system.

For the series:
Tested-by: Naveen N Rao (AMD) <naveen@kernel.org>


- Naveen