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

[Alejandro Jimenez <alejandro.j.jimenez@oracle.com>]

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