mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
To: Sean Christopherson <seanjc@google.com>
Cc: Partha Sarathi Satapathy <partha.satapathy@oracle.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Thomas Gleixner <tglx@kernel.org>, Ingo Molnar <mingo@redhat.com>,
	Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org, "H . Peter Anvin" <hpa@zytor.com>,
	Shuah Khan <shuah@kernel.org>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-kselftest@vger.kernel.org, wim.ten.have@oracle.com
Subject: Re: [PATCH 1/2] KVM: x86: Select lowest vCPU ID for aliased xAPIC IDs
Date: Wed, 28 Oct 2026 18:34:07 -0400	[thread overview]
Message-ID: <62e09b20-b839-448c-90ad-0c63233b523e@oracle.com> (raw)
In-Reply-To: <auEI0B7QkIhg-lLG@google.com>



On 10/27/26 4:25 PM, Sean Christopherson wrote:
> 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>
> 
> 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

The problem we are trying to solve is an INIT/SIPI being delivered to the
wrong vCPU during hotplug.

e.g. we create/hotplug vCPU 531 (0x213) before vCPU 19 (0x13), and
KVM_X2APIC_API_USE_32BIT_IDS is enabled. The first vCPU 531 will populate
two entries in the phys_map:
phys_map[x2apic_id] --> phys_map[0x213] = vCPU 531 apic
phys_map[xapic_id] --> phys_map[0x013] = vCPU 531 apic

Side note: I think that the dual entry behavior is needed since the higher
"x2apic_id" entry is required for the KVM hotplug hack, and the
lower/truncated "xapic_id" entry is needed so the vCPU can still receive
interrupts addressed to its xAPIC ID (is this actually needed/used behavior?)

When vCPU 19 (0x13) is created/hotplugged after vCPU 531, the APIC map
rebuild will not replace the phys_map[0x13] entry with one corresponding to
this new/lower VCPU 19 apic. So INIT/SIPI addressed to "lower" vCPU 19
(0x13) are instead delivered to vCPU 531 (0x213).

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

Sorry, I don't follow. vCPU 531 would not receive an INIT/SIPI addressed to
the aliased 0x13 APIC ID in my example, but it will be reachable using its
full x2APIC ID of 0x213, for which an entry in the phys_map was installed in:

        if (vcpu->kvm->arch.x2apic_format) {

                /* See also kvm_apic_match_physical_addr(). */

                if (apic_x2apic_mode(apic) || x2apic_id > 0xff)

                        new->phys_map[x2apic_id] = apic;


My interpretation is that behavior is "the KVM hotplug hack", and that
behavior is not changed in the proposed patch.

My example was meant to show the conflict of your proposed change with the
"deliver to the lowest ID" rule, which you clarified is not a rule after
all :) (but perhaps it should be since it has been working so far..?)

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

But then we would also have to disable the optimized map for INIT as well
(APIC_DM_INIT), since those would still be routed to the wrong vCPU otherwise?

And if both INIT & SIPI are using the slow path, they will be delivered to
all matching vCPU IDs, which is likely to cause problems e.g. hot plugging
one vCPU will potentially initialize and start all of its aliases too?

I need to test this with proper instrumented kernel so I can "bring
receipts" (and I think Partha is doing it already), but hopefully my
concern above makes sense.

In short, I think the original proposal of allowing a replacement of a
phys_map[xapic_id] entry only with a new "low" entry (i.e. one with vcpu_id
== x2apic_id == xapic_id) fixes the creation order issue while preserving
the existing behavior for the typical cases.

Thank you,
Alejandro

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

      reply	other threads:[~2026-10-28 22:34 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-27  5:18 Partha Sarathi Satapathy
2026-10-27  5:18 ` [PATCH 2/2] KVM: selftests: Cover aliased xAPIC physical IDs Partha Sarathi Satapathy
2026-10-27 15:40 ` [PATCH 1/2] KVM: x86: Select lowest vCPU ID for aliased xAPIC IDs Sean Christopherson
2026-10-27 19:42   ` Alejandro Jimenez
2026-10-27 20:25     ` Sean Christopherson
2026-10-28 22:34       ` Alejandro Jimenez [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=62e09b20-b839-448c-90ad-0c63233b523e@oracle.com \
    --to=alejandro.j.jimenez@oracle.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=partha.satapathy@oracle.com \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=shuah@kernel.org \
    --cc=tglx@kernel.org \
    --cc=wim.ten.have@oracle.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®