Skip to content

Commit a78d904

Browse files
committed
KVM: x86: Move "KVM no-APIC vCPU" key management into local APIC code
Move incrementing and decrementing of kvm_has_noapic_vcpu into kvm_create_lapic() and kvm_free_lapic() respectively to fix a benign bug where KVM fails to decrement the count if vCPU creation ultimately fails, e.g. due to a memory allocation failing. Note, the bug is benign as kvm_has_noapic_vcpu is used purely to optimize lapic_in_kernel() checks, and that optimization is quite dubious. That, and practically speaking no setup that cares at all about performance runs with a userspace local APIC. Reported-by: Li RongQing <lirongqing@baidu.com> Cc: Maxim Levitsky <mlevitsk@redhat.com> Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com> Link: https://lore.kernel.org/r/20240209222047.394389-2-seanjc@google.com Signed-off-by: Sean Christopherson <seanjc@google.com>
1 parent 0ec3d6d commit a78d904

File tree

2 files changed

+29
-27
lines changed

2 files changed

+29
-27
lines changed

arch/x86/kvm/lapic.c

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,9 @@ static inline int __apic_test_and_clear_vector(int vec, void *bitmap)
124124
return __test_and_clear_bit(VEC_POS(vec), (bitmap) + REG_POS(vec));
125125
}
126126

127+
__read_mostly DEFINE_STATIC_KEY_FALSE(kvm_has_noapic_vcpu);
128+
EXPORT_SYMBOL_GPL(kvm_has_noapic_vcpu);
129+
127130
__read_mostly DEFINE_STATIC_KEY_DEFERRED_FALSE(apic_hw_disabled, HZ);
128131
__read_mostly DEFINE_STATIC_KEY_DEFERRED_FALSE(apic_sw_disabled, HZ);
129132

@@ -2466,8 +2469,10 @@ void kvm_free_lapic(struct kvm_vcpu *vcpu)
24662469
{
24672470
struct kvm_lapic *apic = vcpu->arch.apic;
24682471

2469-
if (!vcpu->arch.apic)
2472+
if (!vcpu->arch.apic) {
2473+
static_branch_dec(&kvm_has_noapic_vcpu);
24702474
return;
2475+
}
24712476

24722477
hrtimer_cancel(&apic->lapic_timer.timer);
24732478

@@ -2809,6 +2814,11 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int timer_advance_ns)
28092814

28102815
ASSERT(vcpu != NULL);
28112816

2817+
if (!irqchip_in_kernel(vcpu->kvm)) {
2818+
static_branch_inc(&kvm_has_noapic_vcpu);
2819+
return 0;
2820+
}
2821+
28122822
apic = kzalloc(sizeof(*apic), GFP_KERNEL_ACCOUNT);
28132823
if (!apic)
28142824
goto nomem;
@@ -2844,6 +2854,21 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int timer_advance_ns)
28442854
static_branch_inc(&apic_sw_disabled.key); /* sw disabled at reset */
28452855
kvm_iodevice_init(&apic->dev, &apic_mmio_ops);
28462856

2857+
/*
2858+
* Defer evaluating inhibits until the vCPU is first run, as this vCPU
2859+
* will not get notified of any changes until this vCPU is visible to
2860+
* other vCPUs (marked online and added to the set of vCPUs).
2861+
*
2862+
* Opportunistically mark APICv active as VMX in particularly is highly
2863+
* unlikely to have inhibits. Ignore the current per-VM APICv state so
2864+
* that vCPU creation is guaranteed to run with a deterministic value,
2865+
* the request will ensure the vCPU gets the correct state before VM-Entry.
2866+
*/
2867+
if (enable_apicv) {
2868+
apic->apicv_active = true;
2869+
kvm_make_request(KVM_REQ_APICV_UPDATE, vcpu);
2870+
}
2871+
28472872
return 0;
28482873
nomem_free_apic:
28492874
kfree(apic);

arch/x86/kvm/x86.c

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -12046,27 +12046,9 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
1204612046
if (r < 0)
1204712047
return r;
1204812048

12049-
if (irqchip_in_kernel(vcpu->kvm)) {
12050-
r = kvm_create_lapic(vcpu, lapic_timer_advance_ns);
12051-
if (r < 0)
12052-
goto fail_mmu_destroy;
12053-
12054-
/*
12055-
* Defer evaluating inhibits until the vCPU is first run, as
12056-
* this vCPU will not get notified of any changes until this
12057-
* vCPU is visible to other vCPUs (marked online and added to
12058-
* the set of vCPUs). Opportunistically mark APICv active as
12059-
* VMX in particularly is highly unlikely to have inhibits.
12060-
* Ignore the current per-VM APICv state so that vCPU creation
12061-
* is guaranteed to run with a deterministic value, the request
12062-
* will ensure the vCPU gets the correct state before VM-Entry.
12063-
*/
12064-
if (enable_apicv) {
12065-
vcpu->arch.apic->apicv_active = true;
12066-
kvm_make_request(KVM_REQ_APICV_UPDATE, vcpu);
12067-
}
12068-
} else
12069-
static_branch_inc(&kvm_has_noapic_vcpu);
12049+
r = kvm_create_lapic(vcpu, lapic_timer_advance_ns);
12050+
if (r < 0)
12051+
goto fail_mmu_destroy;
1207012052

1207112053
r = -ENOMEM;
1207212054

@@ -12187,8 +12169,6 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
1218712169
srcu_read_unlock(&vcpu->kvm->srcu, idx);
1218812170
free_page((unsigned long)vcpu->arch.pio_data);
1218912171
kvfree(vcpu->arch.cpuid_entries);
12190-
if (!lapic_in_kernel(vcpu))
12191-
static_branch_dec(&kvm_has_noapic_vcpu);
1219212172
}
1219312173

1219412174
void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
@@ -12465,9 +12445,6 @@ bool kvm_vcpu_is_bsp(struct kvm_vcpu *vcpu)
1246512445
return (vcpu->arch.apic_base & MSR_IA32_APICBASE_BSP) != 0;
1246612446
}
1246712447

12468-
__read_mostly DEFINE_STATIC_KEY_FALSE(kvm_has_noapic_vcpu);
12469-
EXPORT_SYMBOL_GPL(kvm_has_noapic_vcpu);
12470-
1247112448
void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu)
1247212449
{
1247312450
struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);

0 commit comments

Comments
 (0)