Skip to content

Commit 1d518c6

Browse files
Wanpeng Libonzini
authored andcommitted
KVM: LAPIC: Fix reentrancy issues with preempt notifiers
Preempt can occur in the preemption timer expiration handler: CPU0 CPU1 preemption timer vmexit handle_preemption_timer(vCPU0) kvm_lapic_expired_hv_timer hv_timer_is_use == true sched_out sched_in kvm_arch_vcpu_load kvm_lapic_restart_hv_timer restart_apic_timer start_hv_timer already-expired timer or sw timer triggerd in the window start_sw_timer cancel_hv_timer /* back in kvm_lapic_expired_hv_timer */ cancel_hv_timer WARN_ON(!apic->lapic_timer.hv_timer_in_use); ==> Oops This can be reproduced if CONFIG_PREEMPT is enabled. ------------[ cut here ]------------ WARNING: CPU: 4 PID: 2972 at /home/kernel/linux/arch/x86/kvm//lapic.c:1563 kvm_lapic_expired_hv_timer+0x9e/0xb0 [kvm] CPU: 4 PID: 2972 Comm: qemu-system-x86 Tainted: G OE 4.13.0-rc2+ #16 RIP: 0010:kvm_lapic_expired_hv_timer+0x9e/0xb0 [kvm] Call Trace: handle_preemption_timer+0xe/0x20 [kvm_intel] vmx_handle_exit+0xb8/0xd70 [kvm_intel] kvm_arch_vcpu_ioctl_run+0xdd1/0x1be0 [kvm] ? kvm_arch_vcpu_load+0x47/0x230 [kvm] ? kvm_arch_vcpu_load+0x62/0x230 [kvm] kvm_vcpu_ioctl+0x340/0x700 [kvm] ? kvm_vcpu_ioctl+0x340/0x700 [kvm] ? __fget+0xfc/0x210 do_vfs_ioctl+0xa4/0x6a0 ? __fget+0x11d/0x210 SyS_ioctl+0x79/0x90 do_syscall_64+0x81/0x220 entry_SYSCALL64_slow_path+0x25/0x25 ------------[ cut here ]------------ WARNING: CPU: 4 PID: 2972 at /home/kernel/linux/arch/x86/kvm//lapic.c:1498 cancel_hv_timer.isra.40+0x4f/0x60 [kvm] CPU: 4 PID: 2972 Comm: qemu-system-x86 Tainted: G W OE 4.13.0-rc2+ #16 RIP: 0010:cancel_hv_timer.isra.40+0x4f/0x60 [kvm] Call Trace: kvm_lapic_expired_hv_timer+0x3e/0xb0 [kvm] handle_preemption_timer+0xe/0x20 [kvm_intel] vmx_handle_exit+0xb8/0xd70 [kvm_intel] kvm_arch_vcpu_ioctl_run+0xdd1/0x1be0 [kvm] ? kvm_arch_vcpu_load+0x47/0x230 [kvm] ? kvm_arch_vcpu_load+0x62/0x230 [kvm] kvm_vcpu_ioctl+0x340/0x700 [kvm] ? kvm_vcpu_ioctl+0x340/0x700 [kvm] ? __fget+0xfc/0x210 do_vfs_ioctl+0xa4/0x6a0 ? __fget+0x11d/0x210 SyS_ioctl+0x79/0x90 do_syscall_64+0x81/0x220 entry_SYSCALL64_slow_path+0x25/0x25 This patch fixes it by making the caller of cancel_hv_timer, start_hv_timer and start_sw_timer be in preemption-disabled regions, which trivially avoid any reentrancy issue with preempt notifier. Cc: Paolo Bonzini <[email protected]> Cc: Radim Krčmář <[email protected]> Signed-off-by: Wanpeng Li <[email protected]> [Add more WARNs. - Paolo] Signed-off-by: Paolo Bonzini <[email protected]>
1 parent 67fbcd6 commit 1d518c6

File tree

1 file changed

+14
-3
lines changed

1 file changed

+14
-3
lines changed

arch/x86/kvm/lapic.c

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1495,18 +1495,18 @@ EXPORT_SYMBOL_GPL(kvm_lapic_hv_timer_in_use);
14951495

14961496
static void cancel_hv_timer(struct kvm_lapic *apic)
14971497
{
1498+
WARN_ON(preemptible());
14981499
WARN_ON(!apic->lapic_timer.hv_timer_in_use);
1499-
preempt_disable();
15001500
kvm_x86_ops->cancel_hv_timer(apic->vcpu);
15011501
apic->lapic_timer.hv_timer_in_use = false;
1502-
preempt_enable();
15031502
}
15041503

15051504
static bool start_hv_timer(struct kvm_lapic *apic)
15061505
{
15071506
struct kvm_timer *ktimer = &apic->lapic_timer;
15081507
int r;
15091508

1509+
WARN_ON(preemptible());
15101510
if (!kvm_x86_ops->set_hv_timer)
15111511
return false;
15121512

@@ -1538,6 +1538,8 @@ static bool start_hv_timer(struct kvm_lapic *apic)
15381538
static void start_sw_timer(struct kvm_lapic *apic)
15391539
{
15401540
struct kvm_timer *ktimer = &apic->lapic_timer;
1541+
1542+
WARN_ON(preemptible());
15411543
if (apic->lapic_timer.hv_timer_in_use)
15421544
cancel_hv_timer(apic);
15431545
if (!apic_lvtt_period(apic) && atomic_read(&ktimer->pending))
@@ -1552,15 +1554,20 @@ static void start_sw_timer(struct kvm_lapic *apic)
15521554

15531555
static void restart_apic_timer(struct kvm_lapic *apic)
15541556
{
1557+
preempt_disable();
15551558
if (!start_hv_timer(apic))
15561559
start_sw_timer(apic);
1560+
preempt_enable();
15571561
}
15581562

15591563
void kvm_lapic_expired_hv_timer(struct kvm_vcpu *vcpu)
15601564
{
15611565
struct kvm_lapic *apic = vcpu->arch.apic;
15621566

1563-
WARN_ON(!apic->lapic_timer.hv_timer_in_use);
1567+
preempt_disable();
1568+
/* If the preempt notifier has already run, it also called apic_timer_expired */
1569+
if (!apic->lapic_timer.hv_timer_in_use)
1570+
goto out;
15641571
WARN_ON(swait_active(&vcpu->wq));
15651572
cancel_hv_timer(apic);
15661573
apic_timer_expired(apic);
@@ -1569,6 +1576,8 @@ void kvm_lapic_expired_hv_timer(struct kvm_vcpu *vcpu)
15691576
advance_periodic_target_expiration(apic);
15701577
restart_apic_timer(apic);
15711578
}
1579+
out:
1580+
preempt_enable();
15721581
}
15731582
EXPORT_SYMBOL_GPL(kvm_lapic_expired_hv_timer);
15741583

@@ -1582,9 +1591,11 @@ void kvm_lapic_switch_to_sw_timer(struct kvm_vcpu *vcpu)
15821591
{
15831592
struct kvm_lapic *apic = vcpu->arch.apic;
15841593

1594+
preempt_disable();
15851595
/* Possibly the TSC deadline timer is not enabled yet */
15861596
if (apic->lapic_timer.hv_timer_in_use)
15871597
start_sw_timer(apic);
1598+
preempt_enable();
15881599
}
15891600
EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_sw_timer);
15901601

0 commit comments

Comments
 (0)