Re: [PATCH 3/3] KVM: x86: Decouple APICv activation state from apicv_inhibit_reasons

[Paolo Bonzini <pbonzini@redhat.com>]

On 2/3/25 19:45, Sean Christopherson wrote:
> Unless there's a very, very good reason to support a use case that generates
> ExtInts during boot, but _only_ during boot, and otherwise doesn't have any APICv
> ihibits, I'm leaning towards making SVM's IRQ window inhibit sticky, i.e. never
> clear it.

BIOS tends to use PIT, so that may be too much.  With respect to Naveen's report
of contention on apicv_update_lock, I would go with the sticky-bit idea but apply
it to APICV_INHIBIT_REASON_PIT_REINJ.

Plus, to avoid crazy ExtINT configurations, something like this:

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 3ec6197b1386..3e358d55b676 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1295,6 +1295,11 @@ enum kvm_apicv_inhibit {
  	 */
  	APICV_INHIBIT_REASON_LOGICAL_ID_ALIASED,
  
+	/*
+	 * AVIC is disabled because more than one vCPU has extint unmasked
+	 */
+	APICV_INHIBIT_REASON_EXTINT,
+
  	/*********************************************************/
  	/* INHIBITs that are relevant only to the Intel's APICv. */
  	/*********************************************************/
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 71544b0f6301..33a5f4ef42bd 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -377,6 +377,7 @@ void kvm_recalculate_apic_map(struct kvm *kvm)
  	struct kvm_apic_map *new, *old = NULL;
  	struct kvm_vcpu *vcpu;
  	unsigned long i;
+	int extint_cnt = 0;
  	u32 max_id = 255; /* enough space for any xAPIC ID */
  	bool xapic_id_mismatch;
  	int r;
@@ -432,6 +433,8 @@ void kvm_recalculate_apic_map(struct kvm *kvm)
  		if (!kvm_apic_present(vcpu))
  			continue;
  
+		extint_cnt += kvm_apic_accept_pic_intr(vcpu);
+
  		r = kvm_recalculate_phys_map(new, vcpu, &xapic_id_mismatch);
  		if (r) {
  			kvfree(new);
@@ -457,6 +460,11 @@ void kvm_recalculate_apic_map(struct kvm *kvm)
  	else
  		kvm_clear_apicv_inhibit(kvm, APICV_INHIBIT_REASON_PHYSICAL_ID_ALIASED);
  
+	if (extint_cnt > 1)
+		kvm_set_apicv_inhibit(kvm, APICV_INHIBIT_REASON_EXTINT);
+	else
+		kvm_clear_apicv_inhibit(kvm, APICV_INHIBIT_REASON_EXTINT);
+
  	if (!new || new->logical_mode == KVM_APIC_MODE_MAP_DISABLED)
  		kvm_set_apicv_inhibit(kvm, APICV_INHIBIT_REASON_LOGICAL_ID_ALIASED);
  	else
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 57ff79bc02a4..ba2fc7dd8ca2 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -676,6 +676,7 @@ extern struct kvm_x86_nested_ops svm_nested_ops;
  	BIT(APICV_INHIBIT_REASON_HYPERV) |		\
  	BIT(APICV_INHIBIT_REASON_NESTED) |		\
  	BIT(APICV_INHIBIT_REASON_IRQWIN) |		\
+	BIT(APICV_INHIBIT_REASON_EXTINT) |		\
  	BIT(APICV_INHIBIT_REASON_PIT_REINJ) |		\
  	BIT(APICV_INHIBIT_REASON_BLOCKIRQ) |		\
  	BIT(APICV_INHIBIT_REASON_SEV)      |		\


I don't love adding another inhibit reason but, together, these two should
remove the contention on apicv_update_lock.  Another idea could be to move
IRQWIN to per-vCPU reason but Maxim tells me that it's not so easy.

Paolo