* [PATCH 1/1] KVM: x86: lapic: update apic_base and APIC ID together
@ 2026-08-18 23:42 Dongli Zhang
2026-08-18 23:49 ` Dongli Zhang
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Dongli Zhang @ 2026-08-18 23:42 UTC (permalink / raw)
To: kvm; +Cc: seanjc, pbonzini, joe.jin
The commit b2849bec936b ("KVM: VMX: Update SVI during runtime APICv
activation") resolved the loss of EOI issue when apicv is activated after
being inhibited at runtime. However, it does not resolve the cause of the
runtime apicv inhibition.
The inhibition occurs because apic_base and the APIC ID are not updated
together.
Although commit 052c3b99cbc8 ("KVM: x86: Reinitialize xAPIC ID when
userspace forces x2APIC => xAPIC") reinitializes the xAPIC ID to the
vCPU ID when userspace forces the APIC to transition directly from x2APIC
to xAPIC mode, the updates are not performed in a single transaction.
If another thread calls kvm_recalculate_apic_map() during the window
between updating apic_base and the APIC ID, kvm_recalculate_phys_map() may
set xapic_id_mismatch and temporarily inhibit APICv.
Thread A Thread B
__kvm_apic_set_base()
-> vcpu->arch.apic_base = value;
(disable x2apic)
kvm_recalculate_apic_map()
-> kvm_recalculate_phys_map()
*xapic_id_mismatch = true;
-> kvm_apic_set_xapic_id(apic, vcpu->vcpu_id);
It is easier to reproduce without the commit 052c3b99cbc8 ("KVM: x86:
Reinitialize xAPIC ID when userspace forces x2APIC => xAPIC").
In the QEMU scenario, when a vCPU is removed, QEMU parks the KVM vCPU fd
and reuses it later. When the same vCPU fd is reused for another hot-add
operation, its x2APIC is still enabled. Once QEMU tries to disable x2APIC,
we may encounter the race windows described above.
Add conditional lock protection within __kvm_apic_set_base() so that
vcpu->arch.apic_base and the APIC ID are updated atomically. The same lock
is also used in kvm_recalculate_apic_map().
Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
---
arch/x86/kvm/lapic.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 48b019114c19..1d858c259ab8 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2793,17 +2793,29 @@ static void __kvm_apic_set_base(struct kvm_vcpu *vcpu, u64 value)
{
u64 old_value = vcpu->arch.apic_base;
struct kvm_lapic *apic = vcpu->arch.apic;
+ u64 changed = old_value ^ value;
+ bool apicbase_enable_changed = changed & MSR_IA32_APICBASE_ENABLE;
+ bool x2apic_enable_changed = changed & X2APIC_ENABLE;
+ bool apic_mode_changed = apicbase_enable_changed || x2apic_enable_changed;
+ bool need_lock = apic && apic_mode_changed;
+
+ /*
+ * Serialize apic_base and APIC ID updates with APIC map
+ * recalculation.
+ */
+ if (need_lock)
+ mutex_lock(&vcpu->kvm->arch.apic_map_lock);
vcpu->arch.apic_base = value;
- if ((old_value ^ value) & MSR_IA32_APICBASE_ENABLE)
+ if (apicbase_enable_changed)
vcpu->arch.cpuid_dynamic_bits_dirty = true;
if (!apic)
return;
/* update jump label if enable bit changes */
- if ((old_value ^ value) & MSR_IA32_APICBASE_ENABLE) {
+ if (apicbase_enable_changed) {
if (value & MSR_IA32_APICBASE_ENABLE) {
kvm_apic_set_xapic_id(apic, vcpu->vcpu_id);
static_branch_slow_dec_deferred(&apic_hw_disabled);
@@ -2815,14 +2827,17 @@ static void __kvm_apic_set_base(struct kvm_vcpu *vcpu, u64 value)
}
}
- if ((old_value ^ value) & X2APIC_ENABLE) {
+ if (x2apic_enable_changed) {
if (value & X2APIC_ENABLE)
kvm_apic_set_x2apic_id(apic, vcpu->vcpu_id);
else if (value & MSR_IA32_APICBASE_ENABLE)
kvm_apic_set_xapic_id(apic, vcpu->vcpu_id);
}
- if ((old_value ^ value) & (MSR_IA32_APICBASE_ENABLE | X2APIC_ENABLE)) {
+ if (need_lock)
+ mutex_unlock(&vcpu->kvm->arch.apic_map_lock);
+
+ if (apic_mode_changed) {
kvm_make_request(KVM_REQ_APICV_UPDATE, vcpu);
kvm_x86_call(set_virtual_apic_mode)(vcpu);
}
--
2.43.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] KVM: x86: lapic: update apic_base and APIC ID together
2026-08-18 23:42 [PATCH 1/1] KVM: x86: lapic: update apic_base and APIC ID together Dongli Zhang
@ 2026-08-18 23:49 ` Dongli Zhang
2026-08-19 0:03 ` sashiko-bot
2026-08-19 5:48 ` Chao Gao
2 siblings, 0 replies; 4+ messages in thread
From: Dongli Zhang @ 2026-08-18 23:49 UTC (permalink / raw)
To: kvm; +Cc: seanjc, pbonzini, joe.jin
Here is how I reproduce the issue. Apply the KVM reproducer patch at the end of
this email. The patch intentionally adds mdelay here and there to reproduce the
race window. Install it on an AMD KVM host.
1. Create QEMU instance:
qemu-system-x86_64 \
-machine q35,kernel_irqchip=split,dump-guest-core=off \
-hda ol89.qcow2 \
-m 8G -smp 4,maxcpus=6 \
-enable-kvm -cpu host \
-net nic -net user,hostfwd=tcp::5028-:22 \
-monitor stdio -vnc :8
2. Add and remove two vCPUs.
(qemu) device_add host-x86_64-cpu,id=core4,socket-id=0,core-id=4,thread-id=0
(qemu) device_add host-x86_64-cpu,id=core5,socket-id=0,core-id=5,thread-id=0
(qemu) device_del core4
(qemu) device_del core5
3. Enable ftrace.
hv# echo "kvm_apicv_inhibit_changed" > /sys/kernel/debug/tracing/set_event
hv# cat /sys/kernel/debug/tracing/trace_pipe
4. Re-add the same vCPUs again.
(qemu) device_add host-x86_64-cpu,id=core4,socket-id=0,core-id=4,thread-id=0
(qemu) device_add host-x86_64-cpu,id=core5,socket-id=0,core-id=5,thread-id=0
Here are host dmesg are ftrace output.
[ 435.984063] kvm: debug: kvm_apic_set_base() vcpu=4 wait ...
[ 436.111019] kvm: debug: __kvm_apic_set_base() vcpu=5 race window start
[ 437.991539] kvm: debug: kvm_apic_set_base() vcpu=4 done!
[ 437.997618] kvm: debugbug: kvm_recalculate_apic_map() mismatch detected for
vcpu=5
[ 441.121122] kvm: debug: __kvm_apic_set_base() vcpu=5 race window end
CPU 4/KVM-12339 [422] ..... 440.424414: kvm_apicv_inhibit_changed: cleared
reason=5, inhibits=0x0
CPU 4/KVM-12339 [422] ..... 440.424414: kvm_apicv_inhibit_changed: cleared
reason=8, inhibits=0x0
CPU 4/KVM-12339 [155] ..... 442.448155: kvm_apicv_inhibit_changed: set
reason=4, inhibits=0x10 PHYSICAL_ID_ALIASED
CPU 4/KVM-12339 [155] ..... 442.448162: kvm_apicv_inhibit_changed: cleared
reason=8, inhibits=0x10 PHYSICAL_ID_ALIASED
CPU 4/KVM-12339 [155] ..... 442.448171: kvm_apicv_inhibit_changed: set
reason=11, inhibits=0x810 PHYSICAL_ID_ALIASED|LOGICAL_ID_ALIASED
CPU 4/KVM-12339 [155] ..... 442.448171: kvm_apicv_inhibit_changed: cleared
reason=8, inhibits=0x810 PHYSICAL_ID_ALIASED|LOGICAL_ID_ALIASED
CPU 4/KVM-12339 [155] ..... 442.448171: kvm_apicv_inhibit_changed: set
reason=5, inhibits=0x830 PHYSICAL_ID_ALIASED|APIC_ID_MODIFIED|LOGICAL_ID_ALIASED
CPU 4/KVM-12339 [155] ..... 442.448171: kvm_apicv_inhibit_changed: cleared
reason=8, inhibits=0x830 PHYSICAL_ID_ALIASED|APIC_ID_MODIFIED|LOGICAL_ID_ALIASED
CPU 5/KVM-12340 [153] ..... 445.570271: kvm_apicv_inhibit_changed: cleared
reason=4, inhibits=0x820 APIC_ID_MODIFIED|LOGICAL_ID_ALIASED
CPU 5/KVM-12340 [153] ..... 445.570276: kvm_apicv_inhibit_changed: cleared
reason=8, inhibits=0x820 APIC_ID_MODIFIED|LOGICAL_ID_ALIASED
CPU 5/KVM-12340 [153] ..... 445.570276: kvm_apicv_inhibit_changed: cleared
reason=11, inhibits=0x20 APIC_ID_MODIFIED
CPU 5/KVM-12340 [153] ..... 445.570276: kvm_apicv_inhibit_changed: cleared
reason=8, inhibits=0x20 APIC_ID_MODIFIED
CPU 5/KVM-12340 [153] ..... 445.570276: kvm_apicv_inhibit_changed: cleared
reason=5, inhibits=0x0
This is reproducer KVM patch.
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 48b019114c19..6db820edef38 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -46,6 +46,7 @@
#include "cpuid.h"
#include "hyperv.h"
#include "smm.h"
+#include <linux/delay.h>
#ifndef CONFIG_X86_64
#define mod_64(x, y) ((x) - (y) * div64_u64(x, y))
@@ -280,8 +281,11 @@ static int kvm_recalculate_phys_map(struct kvm_apic_map *new,
* 32-bit value. Any unwanted aliasing due to truncation results will
* be detected below.
*/
- if (!apic_x2apic_mode(apic) && xapic_id != (u8)vcpu->vcpu_id)
+ if (!apic_x2apic_mode(apic) && xapic_id != (u8)vcpu->vcpu_id) {
*xapic_id_mismatch = true;
+ pr_alert("debugbug: kvm_recalculate_apic_map() mismatch detected for vcpu=%d\n",
+ vcpu->vcpu_id);
+ }
/*
* Apply KVM's hotplug hack if userspace has enable 32-bit APIC IDs.
@@ -2796,6 +2800,15 @@ static void __kvm_apic_set_base(struct kvm_vcpu *vcpu,
u64 value)
vcpu->arch.apic_base = value;
+ if (vcpu->vcpu_id == 5 &&
+ kvm_apic_present(vcpu) &&
+ !(vcpu->arch.apic_base & X2APIC_ENABLE) &&
+ kvm_xapic_id(apic) != (u8)vcpu->vcpu_id) {
+ pr_alert("debug: __kvm_apic_set_base() vcpu=5 race window start\n");
+ mdelay(5000);
+ pr_alert("debug: __kvm_apic_set_base() vcpu=5 race window end\n");
+ }
+
if ((old_value ^ value) & MSR_IA32_APICBASE_ENABLE)
vcpu->arch.cpuid_dynamic_bits_dirty = true;
@@ -2858,6 +2871,13 @@ int kvm_apic_set_base(struct kvm_vcpu *vcpu, u64 value,
bool host_initiated)
}
__kvm_apic_set_base(vcpu, value);
+
+ if (vcpu->vcpu_id == 4 && !host_initiated) {
+ pr_alert("debug: kvm_apic_set_base() vcpu=4 wait ...\n");
+ mdelay(2000);
+ pr_alert("debug: kvm_apic_set_base() vcpu=4 done!\n");
+ }
+
kvm_recalculate_apic_map(vcpu->kvm);
return 0;
}
--
2.43.7
Thank you very much!
Dongli Zhang
On Tue, Aug 18, 2026 4:42:38PM -0700, Dongli Zhang wrote:
> The commit b2849bec936b ("KVM: VMX: Update SVI during runtime APICv
> activation") resolved the loss of EOI issue when apicv is activated after
> being inhibited at runtime. However, it does not resolve the cause of the
> runtime apicv inhibition.
>
> The inhibition occurs because apic_base and the APIC ID are not updated
> together.
>
> Although commit 052c3b99cbc8 ("KVM: x86: Reinitialize xAPIC ID when
> userspace forces x2APIC => xAPIC") reinitializes the xAPIC ID to the
> vCPU ID when userspace forces the APIC to transition directly from x2APIC
> to xAPIC mode, the updates are not performed in a single transaction.
>
> If another thread calls kvm_recalculate_apic_map() during the window
> between updating apic_base and the APIC ID, kvm_recalculate_phys_map() may
> set xapic_id_mismatch and temporarily inhibit APICv.
>
> Thread A Thread B
>
> __kvm_apic_set_base()
>
> -> vcpu->arch.apic_base = value;
> (disable x2apic)
> kvm_recalculate_apic_map()
> -> kvm_recalculate_phys_map()
> *xapic_id_mismatch = true;
>
> -> kvm_apic_set_xapic_id(apic, vcpu->vcpu_id);
>
>
> It is easier to reproduce without the commit 052c3b99cbc8 ("KVM: x86:
> Reinitialize xAPIC ID when userspace forces x2APIC => xAPIC").
>
> In the QEMU scenario, when a vCPU is removed, QEMU parks the KVM vCPU fd
> and reuses it later. When the same vCPU fd is reused for another hot-add
> operation, its x2APIC is still enabled. Once QEMU tries to disable x2APIC,
> we may encounter the race windows described above.
>
> Add conditional lock protection within __kvm_apic_set_base() so that
> vcpu->arch.apic_base and the APIC ID are updated atomically. The same lock
> is also used in kvm_recalculate_apic_map().
>
> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
> ---
> arch/x86/kvm/lapic.c | 23 +++++++++++++++++++----
> 1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 48b019114c19..1d858c259ab8 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -2793,17 +2793,29 @@ static void __kvm_apic_set_base(struct kvm_vcpu *vcpu, u64 value)
> {
> u64 old_value = vcpu->arch.apic_base;
> struct kvm_lapic *apic = vcpu->arch.apic;
> + u64 changed = old_value ^ value;
> + bool apicbase_enable_changed = changed & MSR_IA32_APICBASE_ENABLE;
> + bool x2apic_enable_changed = changed & X2APIC_ENABLE;
> + bool apic_mode_changed = apicbase_enable_changed || x2apic_enable_changed;
> + bool need_lock = apic && apic_mode_changed;
> +
> + /*
> + * Serialize apic_base and APIC ID updates with APIC map
> + * recalculation.
> + */
> + if (need_lock)
> + mutex_lock(&vcpu->kvm->arch.apic_map_lock);
>
> vcpu->arch.apic_base = value;
>
> - if ((old_value ^ value) & MSR_IA32_APICBASE_ENABLE)
> + if (apicbase_enable_changed)
> vcpu->arch.cpuid_dynamic_bits_dirty = true;
>
> if (!apic)
> return;
>
> /* update jump label if enable bit changes */
> - if ((old_value ^ value) & MSR_IA32_APICBASE_ENABLE) {
> + if (apicbase_enable_changed) {
> if (value & MSR_IA32_APICBASE_ENABLE) {
> kvm_apic_set_xapic_id(apic, vcpu->vcpu_id);
> static_branch_slow_dec_deferred(&apic_hw_disabled);
> @@ -2815,14 +2827,17 @@ static void __kvm_apic_set_base(struct kvm_vcpu *vcpu, u64 value)
> }
> }
>
> - if ((old_value ^ value) & X2APIC_ENABLE) {
> + if (x2apic_enable_changed) {
> if (value & X2APIC_ENABLE)
> kvm_apic_set_x2apic_id(apic, vcpu->vcpu_id);
> else if (value & MSR_IA32_APICBASE_ENABLE)
> kvm_apic_set_xapic_id(apic, vcpu->vcpu_id);
> }
>
> - if ((old_value ^ value) & (MSR_IA32_APICBASE_ENABLE | X2APIC_ENABLE)) {
> + if (need_lock)
> + mutex_unlock(&vcpu->kvm->arch.apic_map_lock);
> +
> + if (apic_mode_changed) {
> kvm_make_request(KVM_REQ_APICV_UPDATE, vcpu);
> kvm_x86_call(set_virtual_apic_mode)(vcpu);
> }
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] KVM: x86: lapic: update apic_base and APIC ID together
2026-08-18 23:42 [PATCH 1/1] KVM: x86: lapic: update apic_base and APIC ID together Dongli Zhang
2026-08-18 23:49 ` Dongli Zhang
@ 2026-08-19 0:03 ` sashiko-bot
2026-08-19 5:48 ` Chao Gao
2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-19 0:03 UTC (permalink / raw)
To: Dongli Zhang; +Cc: kvm
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Lock inversion (ABBA deadlock) between `cpus_read_lock()` and `apic_map_lock`.
--
--- Patch [1]: [PATCH 1/1] KVM: x86: lapic: update apic_base and APIC ID together ---
commit 01695608c13ffe0ad80fe486aaf7e61b96936d9e
Author: Dongli Zhang <dongli.zhang@oracle.com>
KVM: x86: lapic: update apic_base and APIC ID together
This commit aims to resolve a runtime apicv inhibition issue by atomically
updating apic_base and the APIC ID. It introduces conditional lock protection
using apic_map_lock within __kvm_apic_set_base() to serialize the updates
with APIC map recalculation.
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -2793,17 +2793,29 @@ static void __kvm_apic_set_base(struct kvm_vcpu *vcpu, u64 value)
> {
> u64 old_value = vcpu->arch.apic_base;
> struct kvm_lapic *apic = vcpu->arch.apic;
> + u64 changed = old_value ^ value;
> + bool apicbase_enable_changed = changed & MSR_IA32_APICBASE_ENABLE;
> + bool x2apic_enable_changed = changed & X2APIC_ENABLE;
> + bool apic_mode_changed = apicbase_enable_changed || x2apic_enable_changed;
> + bool need_lock = apic && apic_mode_changed;
> +
> + /*
> + * Serialize apic_base and APIC ID updates with APIC map
> + * recalculation.
> + */
> + if (need_lock)
> + mutex_lock(&vcpu->kvm->arch.apic_map_lock);
>
> vcpu->arch.apic_base = value;
>
> [ ... ]
>
> /* update jump label if enable bit changes */
> - if ((old_value ^ value) & MSR_IA32_APICBASE_ENABLE) {
> + if (apicbase_enable_changed) {
> if (value & MSR_IA32_APICBASE_ENABLE) {
> kvm_apic_set_xapic_id(apic, vcpu->vcpu_id);
> static_branch_slow_dec_deferred(&apic_hw_disabled);
> /* Check if there are APF page ready requests pending */
> kvm_make_request(KVM_REQ_APF_READY, vcpu);
> } else {
> static_branch_inc(&apic_hw_disabled.key);
[Severity: High]
Does this introduce a lock inversion between apic_map_lock and
cpus_read_lock()?
By acquiring apic_map_lock at the beginning of __kvm_apic_set_base() and
calling static_branch_inc() here, we might be taking the global
cpus_read_lock() while holding a per-VM lock.
Because static_branch_inc() internally calls static_key_slow_inc(), which
unconditionally acquires cpus_read_lock(), this establishes a dependency of:
apic_map_lock -> cpus_read_lock()
However, CPU hotplug operations (like kvm_online_cpu()) take KVM locks under
cpus_write_lock(). Since KVM commonly acquires vcpu->mutex under kvm_lock,
and apic_map_lock is nested further inside vcpu->mutex, this appears to
create a circular dependency:
cpus_read_lock() -> kvm_lock -> vcpu->mutex -> apic_map_lock
Can a guest triggering this path by clearing the APIC enable bit via WRMSR,
while racing with a host CPU hotplug operation, cause an ABBA deadlock?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260818234238.618158-1-dongli.zhang@oracle.com?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] KVM: x86: lapic: update apic_base and APIC ID together
2026-08-18 23:42 [PATCH 1/1] KVM: x86: lapic: update apic_base and APIC ID together Dongli Zhang
2026-08-18 23:49 ` Dongli Zhang
2026-08-19 0:03 ` sashiko-bot
@ 2026-08-19 5:48 ` Chao Gao
2 siblings, 0 replies; 4+ messages in thread
From: Chao Gao @ 2026-08-19 5:48 UTC (permalink / raw)
To: Dongli Zhang; +Cc: kvm, seanjc, pbonzini, joe.jin
On Tue, Aug 18, 2026 at 04:42:38PM -0700, Dongli Zhang wrote:
>The commit b2849bec936b ("KVM: VMX: Update SVI during runtime APICv
>activation") resolved the loss of EOI issue when apicv is activated after
>being inhibited at runtime. However, it does not resolve the cause of the
>runtime apicv inhibition.
Why lead with commit b2849bec936b? APICv is inhibited and re-activated for
other reasons regardless of the race below, so that fix is needed anyway. The
race is at best an orthogonal issue.
>
>The inhibition occurs because apic_base and the APIC ID are not updated
>together.
>
>Although commit 052c3b99cbc8 ("KVM: x86: Reinitialize xAPIC ID when
>userspace forces x2APIC => xAPIC") reinitializes the xAPIC ID to the
>vCPU ID when userspace forces the APIC to transition directly from x2APIC
>to xAPIC mode, the updates are not performed in a single transaction.
>
>If another thread calls kvm_recalculate_apic_map() during the window
>between updating apic_base and the APIC ID, kvm_recalculate_phys_map() may
>set xapic_id_mismatch and temporarily inhibit APICv.
So the goal is to avoid a _transient_ APICv inhibit in a corner case?
If so, please spell out in the changelog why it's worth fixing.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-08-19 5:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-18 23:42 [PATCH 1/1] KVM: x86: lapic: update apic_base and APIC ID together Dongli Zhang
2026-08-18 23:49 ` Dongli Zhang
2026-08-19 0:03 ` sashiko-bot
2026-08-19 5:48 ` Chao Gao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox