[PATCH 1/2] KVM: x86: Select lowest vCPU ID for aliased xAPIC IDs

[PATCH 1/2] KVM: x86: Select lowest vCPU ID for aliased xAPIC IDs

[Partha Sarathi Satapathy]

From: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>

When KVM_X2APIC_API_USE_32BIT_IDS is enabled, KVM's x2APIC hotplug
quirk allows events to target a vCPU by its x2APIC ID while the vCPU
is still in xAPIC mode.

In xAPIC mode, APIC IDs are truncated to eight bits. Consequently,
vCPUs with distinct x2APIC IDs can share an effective physical APIC ID.
KVM's x2APIC erratum specifies that an event targeting an aliased
physical APIC ID is delivered only to the vCPU with the lowest vCPU ID.

However, the optimized physical APIC map does not guarantee lowest-ID
selection. When rebuilding the map, KVM retains the first xAPIC-mode
vCPU encountered for an aliased 8-bit xAPIC ID. Since vCPUs can be
added in any order, that vCPU need not have the lowest vCPU ID.

For example, vCPU 19 has x2APIC ID 0x013 and vCPU 275 has x2APIC ID
0x113. Both expose xAPIC ID 0x13 while in xAPIC mode. If vCPU 275 is
created before vCPU 19, the map retains:

  phys_map[0x13] = vCPU 275

An INIT/SIPI targeting physical destination 0x13 then starts vCPU 275
instead of vCPU 19. vCPU 275 executes the secondary startup path for
the firmware CPU entry with APIC ID 0x013. After vCPU 275 enables
x2APIC, Linux observes APIC ID 0x113 and reports:

  Firmware: 13 APIC: 113

KVM hardcodes a vCPU's x2APIC ID to its vCPU ID. When populating an
aliased legacy xAPIC entry, retain the vCPU with the lowest vCPU ID.
This makes optimized-map selection independent of vCPU-add order. The
separate full-ID entry remains unaffected, and destination 0x113
continues to select vCPU 275.

Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
Signed-off-by: Partha Sarathi Satapathy <partha.satapathy@oracle.com>
---
 arch/x86/kvm/lapic.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 1597dd0b0cc6..5dedffddebec 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -245,7 +245,9 @@ static int kvm_recalculate_phys_map(struct kvm_apic_map *new,
 		if (apic_x2apic_mode(apic) || x2apic_id > 0xff)
 			new->phys_map[x2apic_id] = apic;
 
-		if (!apic_x2apic_mode(apic) && !new->phys_map[xapic_id])
+		if (!apic_x2apic_mode(apic) &&
+		    (!new->phys_map[xapic_id] ||
+		     x2apic_id < new->phys_map[xapic_id]->vcpu->vcpu_id))
 			new->phys_map[xapic_id] = apic;
 	} else {
 		/*
-- 
2.43.7

[PATCH 2/2] KVM: selftests: Cover aliased xAPIC physical IDs

[Partha Sarathi Satapathy]

From: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>

The optimized physical APIC map must select the lowest vCPU ID
when multiple xAPIC-mode vCPUs share an effective physical APIC ID.

Extend recalc_apic_map_test with a regression test that enables
KVM_X2APIC_API_USE_32BIT_IDS and creates vCPU 285 before vCPU 29.
Keep both LAPICs in xAPIC mode, where their physical APIC IDs alias
to 0x1d.

Inject an MSI to destination 0x1d and verify that its vector is
pending only in vCPU 29's IRR. This checks lowest-ID selection when
the higher-ID alias is created first.

Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
Tested-by: Partha Sarathi Satapathy <partha.satapathy@oracle.com>
Signed-off-by: Partha Sarathi Satapathy <partha.satapathy@oracle.com>
---
 .../selftests/kvm/x86/recalc_apic_map_test.c  | 98 +++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/tools/testing/selftests/kvm/x86/recalc_apic_map_test.c b/tools/testing/selftests/kvm/x86/recalc_apic_map_test.c
index cbc92a862ea9..88959adde14d 100644
--- a/tools/testing/selftests/kvm/x86/recalc_apic_map_test.c
+++ b/tools/testing/selftests/kvm/x86/recalc_apic_map_test.c
@@ -18,6 +18,100 @@
 #define LAPIC_X2APIC	(MSR_IA32_APICBASE_ENABLE | X2APIC_ENABLE)
 #define MAX_XAPIC_ID	0xff
 
+#define ALIAS_LOW_VCPU_ID	29
+#define ALIAS_HIGH_VCPU_ID	(ALIAS_LOW_VCPU_ID | 0x100)
+#define ALIAS_VECTOR		0xe1
+#define X86_MSI_BASE_ADDRESS	0xfee00000
+
+static void assert_lapic_xapic_mode(struct kvm_vcpu *vcpu)
+{
+	u64 apicbase = vcpu_get_msr(vcpu, MSR_IA32_APICBASE);
+
+	TEST_ASSERT(apicbase & MSR_IA32_APICBASE_ENABLE,
+		    "vCPU %u local APIC is not enabled, APICBASE=%#lx",
+		    vcpu->id, (unsigned long)apicbase);
+	TEST_ASSERT(!(apicbase & X2APIC_ENABLE),
+		    "vCPU %u unexpectedly entered x2APIC mode, APICBASE=%#lx",
+		    vcpu->id, (unsigned long)apicbase);
+}
+
+static void lapic_sw_enable(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic_state lapic;
+
+	vcpu_ioctl(vcpu, KVM_GET_LAPIC, &lapic);
+	*((u32 *)&lapic.regs[APIC_SPIV]) |= APIC_SPIV_APIC_ENABLED;
+	vcpu_ioctl(vcpu, KVM_SET_LAPIC, &lapic);
+}
+
+static bool lapic_irr_has_vector(struct kvm_vcpu *vcpu, u32 vector)
+{
+	struct kvm_lapic_state lapic;
+	u32 irr;
+
+	vcpu_ioctl(vcpu, KVM_GET_LAPIC, &lapic);
+	irr = *((u32 *)&lapic.regs[APIC_IRR + APIC_VECTOR_TO_REG_OFFSET(vector)]);
+
+	return irr & (1u << APIC_VECTOR_TO_BIT_NUMBER(vector));
+}
+
+static void signal_msi(struct kvm_vm *vm, u32 dest_id, u8 vector)
+{
+	struct kvm_msi msi = {
+		.address_lo	= X86_MSI_BASE_ADDRESS | (dest_id << 12),
+		.address_hi	= 0,
+		.data		= vector,
+	};
+	int r;
+
+	r = __vm_ioctl(vm, KVM_SIGNAL_MSI, &msi);
+	TEST_ASSERT(r == 1, "KVM_SIGNAL_MSI should deliver 1 IRQ, got %d", r);
+}
+
+static void test_xapic_alias_lowest_vcpu_id(void)
+{
+	struct kvm_vcpu *high, *low;
+	struct kvm_vm *vm;
+	bool high_has_irq;
+	bool low_has_irq;
+
+	TEST_REQUIRE(kvm_check_cap(KVM_CAP_X2APIC_API) &
+		     KVM_X2APIC_API_USE_32BIT_IDS);
+	TEST_REQUIRE(kvm_has_cap(KVM_CAP_SIGNAL_MSI));
+	TEST_REQUIRE(kvm_check_cap(KVM_CAP_MAX_VCPU_ID) > ALIAS_HIGH_VCPU_ID);
+
+	vm = vm_create(2);
+	vm_enable_cap(vm, KVM_CAP_X2APIC_API, KVM_X2APIC_API_USE_32BIT_IDS);
+
+	/*
+	 * Reproduce hotplug ordering where the vCPU with x2APIC ID 0x11d is
+	 * created before the vCPU with x2APIC ID 0x1d.  In xAPIC mode they
+	 * alias to physical APIC ID 0x1d.  With KVM_X2APIC_API_USE_32BIT_IDS,
+	 * KVM's ABI says aliased physical APIC events are delivered only to
+	 * the lowest vCPU ID.
+	 */
+	high = vm_vcpu_add(vm, ALIAS_HIGH_VCPU_ID, NULL);
+	low = vm_vcpu_add(vm, ALIAS_LOW_VCPU_ID, NULL);
+
+	assert_lapic_xapic_mode(high);
+	assert_lapic_xapic_mode(low);
+
+	lapic_sw_enable(high);
+	lapic_sw_enable(low);
+
+	signal_msi(vm, ALIAS_LOW_VCPU_ID, ALIAS_VECTOR);
+
+	high_has_irq = lapic_irr_has_vector(high, ALIAS_VECTOR);
+	low_has_irq = lapic_irr_has_vector(low, ALIAS_VECTOR);
+
+	TEST_ASSERT(!high_has_irq && low_has_irq,
+		    "Aliased xAPIC IRQ delivered to wrong vCPU: high vCPU %u IRR=%u, low vCPU %u IRR=%u",
+		    ALIAS_HIGH_VCPU_ID, high_has_irq,
+		    ALIAS_LOW_VCPU_ID, low_has_irq);
+
+	kvm_vm_free(vm);
+}
+
 static void *race(void *arg)
 {
 	struct kvm_lapic_state lapic = {};
@@ -42,6 +136,10 @@ int main(void)
 	int i;
 
 	kvm_static_assert(KVM_MAX_VCPUS > MAX_XAPIC_ID);
+	kvm_static_assert(ALIAS_HIGH_VCPU_ID > MAX_XAPIC_ID);
+	kvm_static_assert((ALIAS_HIGH_VCPU_ID & MAX_XAPIC_ID) == ALIAS_LOW_VCPU_ID);
+
+	test_xapic_alias_lowest_vcpu_id();
 
 	/*
 	 * Create the max number of vCPUs supported by selftests so that KVM
-- 
2.43.7

Re: [PATCH 1/2] KVM: x86: Select lowest vCPU ID for aliased xAPIC IDs

[Sean Christopherson]

On Tue, Oct 27, 2026, Partha Sarathi Satapathy wrote:
> From: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> 
> When KVM_X2APIC_API_USE_32BIT_IDS is enabled, KVM's x2APIC hotplug
> quirk allows events to target a vCPU by its x2APIC ID while the vCPU
> is still in xAPIC mode.
> 
> In xAPIC mode, APIC IDs are truncated to eight bits. Consequently,
> vCPUs with distinct x2APIC IDs can share an effective physical APIC ID.
> KVM's x2APIC erratum specifies that an event targeting an aliased
> physical APIC ID is delivered only to the vCPU with the lowest vCPU ID.
> 
> However, the optimized physical APIC map does not guarantee lowest-ID
> selection. When rebuilding the map, KVM retains the first xAPIC-mode
> vCPU encountered for an aliased 8-bit xAPIC ID. Since vCPUs can be
> added in any order, that vCPU need not have the lowest vCPU ID.
> 
> For example, vCPU 19 has x2APIC ID 0x013 and vCPU 275 has x2APIC ID
> 0x113. Both expose xAPIC ID 0x13 while in xAPIC mode. If vCPU 275 is
> created before vCPU 19, the map retains:
> 
>   phys_map[0x13] = vCPU 275
> 
> An INIT/SIPI targeting physical destination 0x13 then starts vCPU 275
> instead of vCPU 19. vCPU 275 executes the secondary startup path for
> the firmware CPU entry with APIC ID 0x013. After vCPU 275 enables
> x2APIC, Linux observes APIC ID 0x113 and reports:
> 
>   Firmware: 13 APIC: 113
> 
> KVM hardcodes a vCPU's x2APIC ID to its vCPU ID. When populating an
> aliased legacy xAPIC entry, retain the vCPU with the lowest vCPU ID.
> This makes optimized-map selection independent of vCPU-add order.

I'm genuinely curious.  What VMM adds vCPUs in a "random" order?  I agree a fix
is needed, as KVM should be internally consistent (kvm_apic_match_physical_addr()
will always try to match the truncated ID).  But this also makes me wonder exactly
what userspace is doing, and why.

> The separate full-ID entry remains unaffected, and destination 0x113
> continues to select vCPU 275.
> 
> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> Signed-off-by: Partha Sarathi Satapathy <partha.satapathy@oracle.com>
> ---
>  arch/x86/kvm/lapic.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 1597dd0b0cc6..5dedffddebec 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -245,7 +245,9 @@ static int kvm_recalculate_phys_map(struct kvm_apic_map *new,
>  		if (apic_x2apic_mode(apic) || x2apic_id > 0xff)
>  			new->phys_map[x2apic_id] = apic;
>  
> -		if (!apic_x2apic_mode(apic) && !new->phys_map[xapic_id])
> +		if (!apic_x2apic_mode(apic) &&
> +		    (!new->phys_map[xapic_id] ||
> +		     x2apic_id < new->phys_map[xapic_id]->vcpu->vcpu_id))

Hmm, I don't think KVM should always override a smaller ID.  It should be a moot
point, since xapic_id_mismatch will be set if "xapic_id != (u8)vcpu->vcpu_id",
i.e. if the incoming vCPU has a smaller vCPU ID and a true xAPIC ID collision
(not due to truncation).  But to match kvm_apic_match_physical_addr(), I think
it would be this?

		if (!apic_x2apic_mode(apic) &&
		    (!new->phys_map[xapic_id] ||
		     (u8)new->phys_map[xapic_id]->vcpu->vcpu_id == vcpu->vcpu_id))

>  			new->phys_map[xapic_id] = apic;
>  	} else {
>  		/*
> -- 
> 2.43.7
> 
> 

Re: [PATCH 1/2] KVM: x86: Select lowest vCPU ID for aliased xAPIC IDs

[Alejandro Jimenez]

On 10/27/26 11:40 AM, Sean Christopherson wrote:
> On Tue, Oct 27, 2026, Partha Sarathi Satapathy wrote: > From: Alejandro
> Jimenez <alejandro. j. jimenez@ oracle. com> > > When
> KVM_X2APIC_API_USE_32BIT_IDS is enabled, KVM's x2APIC hotplug > quirk
> allows events to target a vCPU
> 
> On Tue, Oct 27, 2026, Partha Sarathi Satapathy wrote:
>> From: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
>> 
>> When KVM_X2APIC_API_USE_32BIT_IDS is enabled, KVM's x2APIC hotplug
>> quirk allows events to target a vCPU by its x2APIC ID while the vCPU
>> is still in xAPIC mode.
>> 
>> In xAPIC mode, APIC IDs are truncated to eight bits. Consequently,
>> vCPUs with distinct x2APIC IDs can share an effective physical APIC ID.
>> KVM's x2APIC erratum specifies that an event targeting an aliased
>> physical APIC ID is delivered only to the vCPU with the lowest vCPU ID.
>> 
>> However, the optimized physical APIC map does not guarantee lowest-ID
>> selection. When rebuilding the map, KVM retains the first xAPIC-mode
>> vCPU encountered for an aliased 8-bit xAPIC ID. Since vCPUs can be
>> added in any order, that vCPU need not have the lowest vCPU ID.
>> 
>> For example, vCPU 19 has x2APIC ID 0x013 and vCPU 275 has x2APIC ID
>> 0x113. Both expose xAPIC ID 0x13 while in xAPIC mode. If vCPU 275 is
>> created before vCPU 19, the map retains:
>> 
>>   phys_map[0x13] = vCPU 275
>> 
>> An INIT/SIPI targeting physical destination 0x13 then starts vCPU 275
>> instead of vCPU 19. vCPU 275 executes the secondary startup path for
>> the firmware CPU entry with APIC ID 0x013. After vCPU 275 enables
>> x2APIC, Linux observes APIC ID 0x113 and reports:
>> 
>>   Firmware: 13 APIC: 113
>> 
>> KVM hardcodes a vCPU's x2APIC ID to its vCPU ID. When populating an
>> aliased legacy xAPIC entry, retain the vCPU with the lowest vCPU ID.
>> This makes optimized-map selection independent of vCPU-add order.
> 

First of all, apologies for the confusion since this is an earlier WIP
version and due to a miscommunication it was sent as v1. The selftest also
has changes that are not included in Patch 2. I will send a v2 after the
discussion on this thread.

For this patch, the intended commit message is phrased/focused on
maintaining the stated KVM hotplug hack behavior i.e. the commit should be
more like:
--
When KVM_X2APIC_API_USE_32BIT_IDS is enabled, events targeting an aliased
xAPIC ID must be delivered to the vCPU with the lowest vCPU ID. However,
the optimized physical APIC map does not guarantee lowest-ID
selection. When rebuilding the map, KVM retains the first xAPIC-mode
vCPU encountered for an aliased xAPIC ID. Since vCPUs can be added in any
order, the initial entry does not necessarily map to the APIC of the vCPU
with lowest vCPU ID.

Compare vCPU IDs when populating aliased xAPIC entries and retain the
lowest one, making destination selection independent of vCPU creation
order.
--

> I'm genuinely curious.  What VMM adds vCPUs in a "random" order?  I agree a fix
> is needed, as KVM should be internally consistent (kvm_apic_match_physical_addr()
> will always try to match the truncated ID).  But this also makes me wonder exactly
> what userspace is doing, and why.
> 

The issue was triggered with QEMU and a tool that uses libvirt API for
resizing guests and balancing their vCPU allocations across NUMA nodes. I
am not sure if similar work has been published upstream, so I'll defer to
Wim (CC'd) if you want more details.

A basic reproducer (besides the selftest) simply uses QEMU to issue
'device_add' QMP commands and starts with a vCPU placed on socket 1,
followed by its "alias" on socket 0. e.g.

{ "execute": "device_add", "arguments": { "driver": "host-x86_64-cpu",
"id": "cpu117", "socket-id": 1, "die-id": 0, "core-id": 11, "thread-id": 1 } }
{ "execute": "device_add", "arguments": { "driver": "host-x86_64-cpu",
"id": "cpu017", "socket-id": 0, "die-id": 0, "core-id": 11, "thread-id": 1 } }

I wondered initially if this approach was breaking some convention, but I
have not found anything stating even a "random order" is disallowed. So
here we are...
>> The separate full-ID entry remains unaffected, and destination 0x113
>> continues to select vCPU 275.
>> 
>> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
>> Signed-off-by: Partha Sarathi Satapathy <partha.satapathy@oracle.com>
>> ---
>>  arch/x86/kvm/lapic.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> index 1597dd0b0cc6..5dedffddebec 100644
>> --- a/arch/x86/kvm/lapic.c
>> +++ b/arch/x86/kvm/lapic.c
>> @@ -245,7 +245,9 @@ static int kvm_recalculate_phys_map(struct kvm_apic_map *new,
>>  		if (apic_x2apic_mode(apic) || x2apic_id > 0xff)
>>  			new->phys_map[x2apic_id] = apic;
>>  
>> -		if (!apic_x2apic_mode(apic) && !new->phys_map[xapic_id])
>> +		if (!apic_x2apic_mode(apic) &&
>> +		    (!new->phys_map[xapic_id] ||
>> +		     x2apic_id < new->phys_map[xapic_id]->vcpu->vcpu_id))
> 
> Hmm, I don't think KVM should always override a smaller ID.  It should be a moot

Why should it not always override the smaller ID (in cases where
KVM_X2APIC_API_USE_32BIT_IDS is enabled)? I interpret the wording from
x2APIC section in:
https://www.kernel.org/doc/Documentation/virt/kvm/x86/errata.rst

"KVM will deliver events targeting that APIC ID only to the vCPU with the
lowest vCPU ID"

as having the consequence that we should always replace an existing entry
in the map with an incoming lower vCPU ID, but the potential corner cases
in this space make my head hurt :).
I think xAPIC ID is unfortunately writable, so perhaps that is the sort of
scenario you are thinking about?

> point, since xapic_id_mismatch will be set if "xapic_id != (u8)vcpu->vcpu_id",
> i.e. if the incoming vCPU has a smaller vCPU ID and a true xAPIC ID collision
> (not due to truncation).  But to match kvm_apic_match_physical_addr(), I think
> it would be this?
> 
> 		if (!apic_x2apic_mode(apic) &&
> 		    (!new->phys_map[xapic_id] ||
> 		     (u8)new->phys_map[xapic_id]->vcpu->vcpu_id == vcpu->vcpu_id))
> 

The above works for the case described in the commit, but going one step
further, I think it doesn't behave correctly in this case:

vCPU 531 (0x213) is created first
vCPU 275 (0x113) is created after

the map is not updated and will still point to the lapic of vCPU 531.
Reversing the creation order above leaves the map with vCPU 275.
If my reading of errata.rst is accurate, then vCPU 275 (i.e. the lowest
vCPU ID) should always be selected regardless of creation order.

Thank you,
Alejandro

>>  			new->phys_map[xapic_id] = apic;
>>  	} else {
>>  		/*
>> -- 
>> 2.43.7
>> 
>> 
> 

Re: [PATCH 1/2] KVM: x86: Select lowest vCPU ID for aliased xAPIC IDs

[Sean Christopherson]

On Tue, Oct 27, 2026, Alejandro Jimenez wrote:
> On 10/27/26 11:40 AM, Sean Christopherson wrote:
> > On Tue, Oct 27, 2026, Partha Sarathi Satapathy wrote: > From: Alejandro
> > Jimenez <alejandro. j. jimenez@ oracle. com> > > When
> > KVM_X2APIC_API_USE_32BIT_IDS is enabled, KVM's x2APIC hotplug > quirk
> > allows events to target a vCPU
> > 
> > On Tue, Oct 27, 2026, Partha Sarathi Satapathy wrote:
> >> From: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> >> 
> >> When KVM_X2APIC_API_USE_32BIT_IDS is enabled, KVM's x2APIC hotplug
> >> quirk allows events to target a vCPU by its x2APIC ID while the vCPU
> >> is still in xAPIC mode.
> >> 
> >> In xAPIC mode, APIC IDs are truncated to eight bits. Consequently,
> >> vCPUs with distinct x2APIC IDs can share an effective physical APIC ID.
> >> KVM's x2APIC erratum specifies that an event targeting an aliased
> >> physical APIC ID is delivered only to the vCPU with the lowest vCPU ID.
> >> 
> >> However, the optimized physical APIC map does not guarantee lowest-ID
> >> selection. When rebuilding the map, KVM retains the first xAPIC-mode
> >> vCPU encountered for an aliased 8-bit xAPIC ID. Since vCPUs can be
> >> added in any order, that vCPU need not have the lowest vCPU ID.
> >> 
> >> For example, vCPU 19 has x2APIC ID 0x013 and vCPU 275 has x2APIC ID
> >> 0x113. Both expose xAPIC ID 0x13 while in xAPIC mode. If vCPU 275 is
> >> created before vCPU 19, the map retains:
> >> 
> >>   phys_map[0x13] = vCPU 275
> >> 
> >> An INIT/SIPI targeting physical destination 0x13 then starts vCPU 275
> >> instead of vCPU 19. vCPU 275 executes the secondary startup path for
> >> the firmware CPU entry with APIC ID 0x013. After vCPU 275 enables
> >> x2APIC, Linux observes APIC ID 0x113 and reports:
> >> 
> >>   Firmware: 13 APIC: 113
> >> 
> >> KVM hardcodes a vCPU's x2APIC ID to its vCPU ID. When populating an
> >> aliased legacy xAPIC entry, retain the vCPU with the lowest vCPU ID.
> >> This makes optimized-map selection independent of vCPU-add order.
> > 
> 
> First of all, apologies for the confusion since this is an earlier WIP
> version and due to a miscommunication it was sent as v1. The selftest also
> has changes that are not included in Patch 2. I will send a v2 after the
> discussion on this thread.
> 
> For this patch, the intended commit message is phrased/focused on
> maintaining the stated KVM hotplug hack behavior i.e. the commit should be
> more like:
> --
> When KVM_X2APIC_API_USE_32BIT_IDS is enabled, events targeting an aliased
> xAPIC ID must be delivered to the vCPU with the lowest vCPU ID. However,
> the optimized physical APIC map does not guarantee lowest-ID
> selection. When rebuilding the map, KVM retains the first xAPIC-mode
> vCPU encountered for an aliased xAPIC ID. Since vCPUs can be added in any
> order, the initial entry does not necessarily map to the APIC of the vCPU
> with lowest vCPU ID.
> 
> Compare vCPU IDs when populating aliased xAPIC entries and retain the
> lowest one, making destination selection independent of vCPU creation
> order.
> --
> 
> > I'm genuinely curious.  What VMM adds vCPUs in a "random" order?  I agree a fix
> > is needed, as KVM should be internally consistent (kvm_apic_match_physical_addr()
> > will always try to match the truncated ID).  But this also makes me wonder exactly
> > what userspace is doing, and why.
> > 
> 
> The issue was triggered with QEMU and a tool that uses libvirt API for
> resizing guests and balancing their vCPU allocations across NUMA nodes. I
> am not sure if similar work has been published upstream, so I'll defer to
> Wim (CC'd) if you want more details.

Oooh, actual hotplug of vCPUs.  That makes sense.
> 
> A basic reproducer (besides the selftest) simply uses QEMU to issue
> 'device_add' QMP commands and starts with a vCPU placed on socket 1,
> followed by its "alias" on socket 0. e.g.
> 
> { "execute": "device_add", "arguments": { "driver": "host-x86_64-cpu",
> "id": "cpu117", "socket-id": 1, "die-id": 0, "core-id": 11, "thread-id": 1 } }
> { "execute": "device_add", "arguments": { "driver": "host-x86_64-cpu",
> "id": "cpu017", "socket-id": 0, "die-id": 0, "core-id": 11, "thread-id": 1 } }
> 
> I wondered initially if this approach was breaking some convention, but I
> have not found anything stating even a "random order" is disallowed. So
> here we are...

It's not disallowed, just unusual, obviously.

> >> The separate full-ID entry remains unaffected, and destination 0x113
> >> continues to select vCPU 275.
> >> 
> >> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
> >> Signed-off-by: Partha Sarathi Satapathy <partha.satapathy@oracle.com>
> >> ---
> >>  arch/x86/kvm/lapic.c | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> >> index 1597dd0b0cc6..5dedffddebec 100644
> >> --- a/arch/x86/kvm/lapic.c
> >> +++ b/arch/x86/kvm/lapic.c
> >> @@ -245,7 +245,9 @@ static int kvm_recalculate_phys_map(struct kvm_apic_map *new,
> >>  		if (apic_x2apic_mode(apic) || x2apic_id > 0xff)
> >>  			new->phys_map[x2apic_id] = apic;
> >>  
> >> -		if (!apic_x2apic_mode(apic) && !new->phys_map[xapic_id])
> >> +		if (!apic_x2apic_mode(apic) &&
> >> +		    (!new->phys_map[xapic_id] ||
> >> +		     x2apic_id < new->phys_map[xapic_id]->vcpu->vcpu_id))
> > 
> > Hmm, I don't think KVM should always override a smaller ID.  It should be a moot
> 
> Why should it not always override the smaller ID (in cases where
> KVM_X2APIC_API_USE_32BIT_IDS is enabled)? I interpret the wording from
> x2APIC section in:
> https://www.kernel.org/doc/Documentation/virt/kvm/x86/errata.rst
> 
> "KVM will deliver events targeting that APIC ID only to the vCPU with the
> lowest vCPU ID"

Gah, foiled by my own documentation.  I had this wrong in my head.  It's not the
aliasing logic that matters, it's literally the behavior of the optimized map that
I was documenting.

> as having the consequence that we should always replace an existing entry
> in the map with an incoming lower vCPU ID, but the potential corner cases
> in this space make my head hurt :).
> I think xAPIC ID is unfortunately writable, so perhaps that is the sort of
> scenario you are thinking about?
> 
> > point, since xapic_id_mismatch will be set if "xapic_id != (u8)vcpu->vcpu_id",
> > i.e. if the incoming vCPU has a smaller vCPU ID and a true xAPIC ID collision
> > (not due to truncation).  But to match kvm_apic_match_physical_addr(), I think
> > it would be this?
> > 
> > 		if (!apic_x2apic_mode(apic) &&
> > 		    (!new->phys_map[xapic_id] ||
> > 		     (u8)new->phys_map[xapic_id]->vcpu->vcpu_id == vcpu->vcpu_id))
> > 
> 
> The above works for the case described in the commit, but going one step
> further, I think it doesn't behave correctly in this case:
> 
> vCPU 531 (0x213) is created first
> vCPU 275 (0x113) is created after
> 
> the map is not updated and will still point to the lapic of vCPU 531.
> Reversing the creation order above leaves the map with vCPU 275.
> If my reading of errata.rst is accurate, then vCPU 275 (i.e. the lowest
> vCPU ID) should always be selected regardless of creation order.

Yeah, ignore that.

Backing up a step, what exactly is the problem you're trying to solve?  Because
I suspect there's no solution that will magically fix everything.  E.g. if you're
trying to resolve an issue where vCPUs aren't recognizing SIPIs, then the example
you provided means vCPU_531 will never wakeup, i.e. which vCPU is  "buggy" changes,
but the bug itself is still there.

If this is about SIPIs hitting the all CPUs, then I think to have a robust (well,
as robust as this mess can get) implementation, we'd have to disable use of the
optimized map when processing SIPIs with PHYSICAL delivery, e.g.

diff --git a/Documentation/virt/kvm/x86/errata.rst b/Documentation/virt/kvm/x86/errata.rst
index a9cf0e004651..65b8a612bfff 100644
--- a/Documentation/virt/kvm/x86/errata.rst
+++ b/Documentation/virt/kvm/x86/errata.rst
@@ -63,10 +63,10 @@ When KVM_X2APIC_API_USE_32BIT_IDS is enabled, KVM activates a hack/quirk that
 allows sending events to a single vCPU using its x2APIC ID even if the target
 vCPU has legacy xAPIC enabled, e.g. to bring up hotplugged vCPUs via INIT-SIPI
 on VMs with > 255 vCPUs.  A side effect of the quirk is that, if multiple vCPUs
-have the same physical APIC ID, KVM will deliver events targeting that APIC ID
-only to the vCPU with the lowest vCPU ID.  If KVM_X2APIC_API_USE_32BIT_IDS is
-not enabled, KVM follows x86 architecture when processing interrupts (all vCPUs
-matching the target APIC ID receive the interrupt).
+have the same physical APIC ID, KVM *may* deliver events targeting that APIC ID
+to only one of the vCPUs.  If KVM_X2APIC_API_USE_32BIT_IDS is not enabled, KVM
+follows x86 architecture when processing interrupts (all vCPUs matching the
+target APIC ID receive the interrupt).
 
 MTRRs
 -----
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index ecf611845fb1..255c8dc00e0a 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1181,6 +1181,10 @@ static inline bool kvm_apic_map_get_dest_lapic(struct kvm *kvm,
 		return false;
 
 	if (irq->dest_mode == APIC_DEST_PHYSICAL) {
+		if (kvm->arch.x2apic_format &&
+		    irq->delivery_mode == APIC_DM_STARTUP)
+			return false;
+
 		if (irq->dest_id > map->max_apic_id) {
 			*bitmap = 0;
 		} else {