All of lore.kernel.org
 help / color / mirror / Atom feed
* [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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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
  2026-08-19 14:02   ` Dongli Zhang
  2 siblings, 1 reply; 6+ 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] 6+ messages in thread

* Re: [PATCH 1/1] KVM: x86: lapic: update apic_base and APIC ID together
  2026-08-19  5:48 ` Chao Gao
@ 2026-08-19 14:02   ` Dongli Zhang
  0 siblings, 0 replies; 6+ messages in thread
From: Dongli Zhang @ 2026-08-19 14:02 UTC (permalink / raw)
  To: Chao Gao; +Cc: kvm, seanjc, pbonzini, joe.jin



On Tue, Aug 18, 2026 10:48:18PM -0700, Chao Gao wrote:
> 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.

I was trying to explain the history behind the change.

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

Yes.

> 
> If so, please spell out in the changelog why it's worth fixing.

Regarding "why it's worth fixing," the primary motivation is to avoid
unnecessary APICv inhibition and reactivation, ideally throughout the entire
lifecycle of a VM.

Another motivation came from reading the AMD SDM. As mentioned in commit
052c3b99cbc8 ("KVM: x86: Reinitialize xAPIC ID when
userspace forces x2APIC => xAPIC"), when x2APIC is enabled or disabled, the APIC
ID is expected to change according to the APIC mode. On real hardware, I assume
this operation is performed atomically in a single transaction when the APIC
mode is changed. This change makes the behavior more consistent with the SDM,
with APIC map recalculation being the major user-visible impact.

I would leave it to the maintainers and reviewers to decide whether this change
is needed. From a production perspective, zero APICv inhibition is appreciated.

Thank you very much!

Dongli Zhang


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] KVM: x86: lapic: update apic_base and APIC ID together
@ 2026-08-23 12:29 kernel test robot
  0 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2026-08-23 12:29 UTC (permalink / raw)
  To: oe-kbuild; +Cc: lkp, Julia Lawall

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20260818234238.618158-1-dongli.zhang@oracle.com>
References: <20260818234238.618158-1-dongli.zhang@oracle.com>
TO: Dongli Zhang <dongli.zhang@oracle.com>
TO: kvm@vger.kernel.org
CC: seanjc@google.com
CC: pbonzini@redhat.com
CC: joe.jin@oracle.com

Hi Dongli,

kernel test robot noticed the following build warnings:

[auto build test WARNING on kvm/queue]
[also build test WARNING on kvm/next mst-vhost/linux-next linus/master v7.2 next-20260821]
[cannot apply to kvm/linux-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Dongli-Zhang/KVM-x86-lapic-update-apic_base-and-APIC-ID-together/20260818-164238
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git queue
patch link:    https://lore.kernel.org/r/20260818234238.618158-1-dongli.zhang%40oracle.com
patch subject: [PATCH 1/1] KVM: x86: lapic: update apic_base and APIC ID together
:::::: branch date: 2 days ago
:::::: commit date: 2 days ago
config: i386-randconfig-054-20260822 (https://download.01.org/0day-ci/archive/20260822/202608222154.KilWZhsr-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Julia Lawall <julia.lawall@inria.fr>
| Closes: https://lore.kernel.org/r/202608222154.KilWZhsr-lkp@intel.com/

cocci warnings: (new ones prefixed by >>)
>> arch/x86/kvm/lapic.c:2824:2-8: preceding lock on line 2816

vim +2824 arch/x86/kvm/lapic.c

c7722e5e1daeea arch/x86/kvm/lapic.c Sean Christopherson        2026-05-29  2800  
7d1cb7cee94ffd arch/x86/kvm/lapic.c Sean Christopherson        2024-11-01  2801  static void __kvm_apic_set_base(struct kvm_vcpu *vcpu, u64 value)
97222cc8316328 drivers/kvm/lapic.c  Eddie Dong                 2007-09-12  2802  {
8d14695f9542e9 arch/x86/kvm/lapic.c Yang Zhang                 2013-01-25  2803  	u64 old_value = vcpu->arch.apic_base;
ad312c7c79f781 drivers/kvm/lapic.c  Zhang Xiantao              2007-12-13  2804  	struct kvm_lapic *apic = vcpu->arch.apic;
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18  2805  	u64 changed = old_value ^ value;
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18  2806  	bool apicbase_enable_changed = changed & MSR_IA32_APICBASE_ENABLE;
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18  2807  	bool x2apic_enable_changed = changed & X2APIC_ENABLE;
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18  2808  	bool apic_mode_changed = apicbase_enable_changed || x2apic_enable_changed;
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18  2809  	bool need_lock = apic && apic_mode_changed;
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18  2810  
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18  2811  	/*
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18  2812  	 * Serialize apic_base and APIC ID updates with APIC map
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18  2813  	 * recalculation.
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18  2814  	 */
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18  2815  	if (need_lock)
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18 @2816  		mutex_lock(&vcpu->kvm->arch.apic_map_lock);
97222cc8316328 drivers/kvm/lapic.c  Eddie Dong                 2007-09-12  2817  
e66d2ae7c67bd9 arch/x86/kvm/lapic.c Jan Kiszka                 2013-12-29  2818  	vcpu->arch.apic_base = value;
e66d2ae7c67bd9 arch/x86/kvm/lapic.c Jan Kiszka                 2013-12-29  2819  
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18  2820  	if (apicbase_enable_changed)
93da6af3ae563b arch/x86/kvm/lapic.c Sean Christopherson        2024-12-10  2821  		vcpu->arch.cpuid_dynamic_bits_dirty = true;
c7dd15b33707e9 arch/x86/kvm/lapic.c Jim Mattson                2016-11-09  2822  
c7dd15b33707e9 arch/x86/kvm/lapic.c Jim Mattson                2016-11-09  2823  	if (!apic)
c7dd15b33707e9 arch/x86/kvm/lapic.c Jim Mattson                2016-11-09 @2824  		return;
c7dd15b33707e9 arch/x86/kvm/lapic.c Jim Mattson                2016-11-09  2825  
c5cc421ba3219b arch/x86/kvm/lapic.c Gleb Natapov               2012-08-05  2826  	/* update jump label if enable bit changes */
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18  2827  	if (apicbase_enable_changed) {
49bd29ba1dbd57 arch/x86/kvm/lapic.c Radim Krčmář               2016-07-12  2828  		if (value & MSR_IA32_APICBASE_ENABLE) {
49bd29ba1dbd57 arch/x86/kvm/lapic.c Radim Krčmář               2016-07-12  2829  			kvm_apic_set_xapic_id(apic, vcpu->vcpu_id);
6e4e3b4df4e31e arch/x86/kvm/lapic.c Cun Li                     2021-01-11  2830  			static_branch_slow_dec_deferred(&apic_hw_disabled);
2f15d027c05fac arch/x86/kvm/lapic.c Vitaly Kuznetsov           2021-04-22  2831  			/* Check if there are APF page ready requests pending */
2f15d027c05fac arch/x86/kvm/lapic.c Vitaly Kuznetsov           2021-04-22  2832  			kvm_make_request(KVM_REQ_APF_READY, vcpu);
187ca84b4b02d8 arch/x86/kvm/lapic.c Wanpeng Li                 2016-08-03  2833  		} else {
6e4e3b4df4e31e arch/x86/kvm/lapic.c Cun Li                     2021-01-11  2834  			static_branch_inc(&apic_hw_disabled.key);
44d527170731c7 arch/x86/kvm/lapic.c Paolo Bonzini              2020-06-22  2835  			atomic_set_release(&apic->vcpu->kvm->arch.apic_map_dirty, DIRTY);
c5cc421ba3219b arch/x86/kvm/lapic.c Gleb Natapov               2012-08-05  2836  		}
187ca84b4b02d8 arch/x86/kvm/lapic.c Wanpeng Li                 2016-08-03  2837  	}
c5cc421ba3219b arch/x86/kvm/lapic.c Gleb Natapov               2012-08-05  2838  
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18  2839  	if (x2apic_enable_changed) {
052c3b99cbc8d2 arch/x86/kvm/lapic.c Emanuele Giuseppe Esposito 2023-01-10  2840  		if (value & X2APIC_ENABLE)
257b9a5faab584 arch/x86/kvm/lapic.c Radim Krčmář               2015-05-22  2841  			kvm_apic_set_x2apic_id(apic, vcpu->vcpu_id);
052c3b99cbc8d2 arch/x86/kvm/lapic.c Emanuele Giuseppe Esposito 2023-01-10  2842  		else if (value & MSR_IA32_APICBASE_ENABLE)
052c3b99cbc8d2 arch/x86/kvm/lapic.c Emanuele Giuseppe Esposito 2023-01-10  2843  			kvm_apic_set_xapic_id(apic, vcpu->vcpu_id);
052c3b99cbc8d2 arch/x86/kvm/lapic.c Emanuele Giuseppe Esposito 2023-01-10  2844  	}
8d860bbeedef97 arch/x86/kvm/lapic.c Jim Mattson                2018-05-09  2845  
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18  2846  	if (need_lock)
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18  2847  		mutex_unlock(&vcpu->kvm->arch.apic_map_lock);
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18  2848  
a184b36a549d15 arch/x86/kvm/lapic.c Dongli Zhang               2026-08-18  2849  	if (apic_mode_changed) {
1459f5c6b8b8df arch/x86/kvm/lapic.c Sean Christopherson        2023-01-06  2850  		kvm_make_request(KVM_REQ_APICV_UPDATE, vcpu);
896046474f8d2e arch/x86/kvm/lapic.c Wei Wang                   2024-05-07  2851  		kvm_x86_call(set_virtual_apic_mode)(vcpu);
8fc9c7a3079e2d arch/x86/kvm/lapic.c Suravee Suthikulpanit      2022-05-19  2852  	}
8d14695f9542e9 arch/x86/kvm/lapic.c Yang Zhang                 2013-01-25  2853  
ad312c7c79f781 drivers/kvm/lapic.c  Zhang Xiantao              2007-12-13  2854  	apic->base_address = apic->vcpu->arch.apic_base &
97222cc8316328 drivers/kvm/lapic.c  Eddie Dong                 2007-09-12  2855  			     MSR_IA32_APICBASE_BASE;
97222cc8316328 drivers/kvm/lapic.c  Eddie Dong                 2007-09-12  2856  
db324fe6f20b0a arch/x86/kvm/lapic.c Nadav Amit                 2014-11-02  2857  	if ((value & MSR_IA32_APICBASE_ENABLE) &&
3743c2f0251743 arch/x86/kvm/lapic.c Maxim Levitsky             2022-06-06  2858  	     apic->base_address != APIC_DEFAULT_PHYS_BASE) {
3743c2f0251743 arch/x86/kvm/lapic.c Maxim Levitsky             2022-06-06  2859  		kvm_set_apicv_inhibit(apic->vcpu->kvm,
3743c2f0251743 arch/x86/kvm/lapic.c Maxim Levitsky             2022-06-06  2860  				      APICV_INHIBIT_REASON_APIC_BASE_MODIFIED);
3743c2f0251743 arch/x86/kvm/lapic.c Maxim Levitsky             2022-06-06  2861  	}
97222cc8316328 drivers/kvm/lapic.c  Eddie Dong                 2007-09-12  2862  }
97222cc8316328 drivers/kvm/lapic.c  Eddie Dong                 2007-09-12  2863  

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-08-23 12:30 UTC | newest]

Thread overview: 6+ 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
2026-08-19 14:02   ` Dongli Zhang
  -- strict thread matches above, loose matches on Subject: below --
2026-08-23 12:29 kernel test robot

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.