All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Alejandro Jimenez <alejandro.j.jimenez@oracle.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: Tue, 27 Oct 2026 13:25:52 -0700	[thread overview]
Message-ID: <auEI0B7QkIhg-lLG@google.com> (raw)
In-Reply-To: <85b0e3aa-0b84-4a00-852a-821aed4a0649@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
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 {


  reply	other threads:[~2026-10-27 20:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-27  5:18 [PATCH 1/2] KVM: x86: Select lowest vCPU ID for aliased xAPIC IDs 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 [this message]
2026-10-28 22:34       ` Alejandro Jimenez

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=auEI0B7QkIhg-lLG@google.com \
    --to=seanjc@google.com \
    --cc=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=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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.