On 8/2/24 22:29, Sean Christopherson wrote:
> [...]
> Making the x2APIC ID fully readonly fixes a WARN in KVM's optimized map
> calculation, which expects the LDR to align with the x2APIC ID.
>
> WARNING: CPU: 2 PID: 958 at arch/x86/kvm/lapic.c:331 kvm_recalculate_apic_map+0x609/0xa00 [kvm]
> CPU: 2 PID: 958 Comm: recalc_apic_map Not tainted 6.4.0-rc3-vanilla+ #35
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux 1.16.2-1-1 04/01/2014
> RIP: 0010:kvm_recalculate_apic_map+0x609/0xa00 [kvm]
> Call Trace:
> <TASK>
> kvm_apic_set_state+0x1cf/0x5b0 [kvm]
> kvm_arch_vcpu_ioctl+0x1806/0x2100 [kvm]
> kvm_vcpu_ioctl+0x663/0x8a0 [kvm]
> __x64_sys_ioctl+0xb8/0xf0
> do_syscall_64+0x56/0x80
> entry_SYSCALL_64_after_hwframe+0x46/0xb0
> RIP: 0033:0x7fade8b9dd6f
Isn't this WARN_ON_ONCE() inherently racy, though? With your patch applied,
it can still be hit by juggling the APIC modes.
[ 53.882945] WARNING: CPU: 13 PID: 1181 at arch/x86/kvm/lapic.c:355 kvm_recalculate_apic_map+0x335/0x650 [kvm]
[ 53.883007] CPU: 13 UID: 1000 PID: 1181 Comm: recalc_logical_ Not tainted 6.11.0-rc1nokasan+ #18
[ 53.883009] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux 1.16.3-1-1 04/01/2014
[ 53.883010] RIP: 0010:kvm_recalculate_apic_map+0x335/0x650 [kvm]
[ 53.883057] Call Trace:
[ 53.883058] <TASK>
[ 53.883169] kvm_apic_set_state+0x105/0x3d0 [kvm]
[ 53.883201] kvm_arch_vcpu_ioctl+0xf09/0x19c0 [kvm]
[ 53.883285] kvm_vcpu_ioctl+0x6cc/0x920 [kvm]
[ 53.883310] __x64_sys_ioctl+0x90/0xd0
[ 53.883313] do_syscall_64+0x93/0x180
[ 53.883623] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 53.883625] RIP: 0033:0x7fd90fee0d2d
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 48d32c5aa3eb..3344f1478230 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -129,6 +129,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/amx_test
TEST_GEN_PROGS_x86_64 += x86_64/max_vcpuid_cap_test
TEST_GEN_PROGS_x86_64 += x86_64/triple_fault_event_test
TEST_GEN_PROGS_x86_64 += x86_64/recalc_apic_map_test
+TEST_GEN_PROGS_x86_64 += x86_64/recalc_logical_map_warn
TEST_GEN_PROGS_x86_64 += access_tracking_perf_test
TEST_GEN_PROGS_x86_64 += demand_paging_test
TEST_GEN_PROGS_x86_64 += dirty_log_test
diff --git a/tools/testing/selftests/kvm/x86_64/recalc_logical_map_warn.c b/tools/testing/selftests/kvm/x86_64/recalc_logical_map_warn.c
new file mode 100644
index 000000000000..ad3ae0433230
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86_64/recalc_logical_map_warn.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * WARN_ON_ONCE(ldr != kvm_apic_calc_x2apic_ldr(kvm_x2apic_id(apic)))
+ * in arch/x86/kvm/lapic.c:kvm_recalculate_logical_map()
+ */
+
+#include <pthread.h>
+
+#include "processor.h"
+#include "kvm_util.h"
+#include "apic.h"
+
+#define LAPIC_XAPIC MSR_IA32_APICBASE_ENABLE
+#define LAPIC_X2APIC (MSR_IA32_APICBASE_ENABLE | X2APIC_ENABLE)
+
+static void *race(void *arg)
+{
+ struct kvm_lapic_state state = {};
+ struct kvm_vcpu *vcpu0 = arg;
+
+ /* Trigger kvm_recalculate_apic_map(). */
+ for (;;)
+ __vcpu_ioctl(vcpu0, KVM_SET_LAPIC, &state);
+
+ return NULL;
+}
+
+int main(void)
+{
+ struct kvm_vcpu *vcpus[2];
+ struct kvm_vm *vm;
+ pthread_t thread;
+
+ vm = vm_create_with_vcpus(2, NULL, vcpus);
+
+ vcpu_set_msr(vcpus[1], MSR_IA32_APICBASE, LAPIC_X2APIC);
+ vcpu_set_msr(vcpus[1], APIC_BASE_MSR + (APIC_SPIV >> 4), APIC_SPIV_APIC_ENABLED);
+
+ TEST_ASSERT_EQ(pthread_create(&thread, NULL, race, vcpus[0]), 0);
+
+ for (;;) {
+ _vcpu_set_msr(vcpus[1], MSR_IA32_APICBASE, LAPIC_XAPIC);
+ _vcpu_set_msr(vcpus[1], MSR_IA32_APICBASE, LAPIC_X2APIC);
+ }
+
+ kvm_vm_free(vm);
+
+ return 0;
+}
