From e5c239cfd5b0ec22751c099dbf4d91f3c504a64f Mon Sep 17 00:00:00 2001 From: Marcelo Tosatti Date: Thu, 8 May 2008 19:47:01 -0300 Subject: KVM: Fix kvm_vcpu_block() task state race There's still a race in kvm_vcpu_block(), if a wake_up_interruptible() call happens before the task state is set to TASK_INTERRUPTIBLE: CPU0 CPU1 kvm_vcpu_block add_wait_queue kvm_cpu_has_interrupt = 0 set interrupt if (waitqueue_active()) wake_up_interruptible() kvm_cpu_has_pending_timer kvm_arch_vcpu_runnable signal_pending set_current_state(TASK_INTERRUPTIBLE) schedule() Can be fixed by using prepare_to_wait() which sets the task state before testing for the wait condition. Signed-off-by: Marcelo Tosatti Signed-off-by: Avi Kivity --- virt/kvm/kvm_main.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) (limited to 'virt') diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index f7ba099049ea..2d29e260da3d 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -758,25 +758,26 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn) */ void kvm_vcpu_block(struct kvm_vcpu *vcpu) { - DECLARE_WAITQUEUE(wait, current); - - add_wait_queue(&vcpu->wq, &wait); - - /* - * We will block until either an interrupt or a signal wakes us up - */ - while (!kvm_cpu_has_interrupt(vcpu) - && !kvm_cpu_has_pending_timer(vcpu) - && !signal_pending(current) - && !kvm_arch_vcpu_runnable(vcpu)) { - set_current_state(TASK_INTERRUPTIBLE); + DEFINE_WAIT(wait); + + for (;;) { + prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE); + + if (kvm_cpu_has_interrupt(vcpu)) + break; + if (kvm_cpu_has_pending_timer(vcpu)) + break; + if (kvm_arch_vcpu_runnable(vcpu)) + break; + if (signal_pending(current)) + break; + vcpu_put(vcpu); schedule(); vcpu_load(vcpu); } - __set_current_state(TASK_RUNNING); - remove_wait_queue(&vcpu->wq, &wait); + finish_wait(&vcpu->wq, &wait); } void kvm_resched(struct kvm_vcpu *vcpu) -- cgit v1.2.3 From ff4b9df877b30b8a371d706d3552999dee450738 Mon Sep 17 00:00:00 2001 From: Marcelo Tosatti Date: Thu, 5 Jun 2008 00:08:11 -0300 Subject: KVM: IOAPIC: only set remote_irr if interrupt was injected There's a bug in the IOAPIC code for level-triggered interrupts. Its relatively easy to trigger by sharing (virtio-blk + usbtablet was the testcase, initially reported by Gerd von Egidy). The "remote_irr" variable is used to indicate accepted but not yet acked interrupts. Its cleared from the EOI handler. Problem is that the EOI handler clears remote_irr unconditionally, even if it reinjected another pending interrupt. In that case, kvm_ioapic_set_irq() proceeds to ioapic_service() which sets remote_irr even if it failed to inject (since the IRR was high due to EOI reinjection). Since the TMR bit has been cleared by the first EOI, the second one fails to clear remote_irr. End result is interrupt line dead. Fix it by setting remote_irr only if a new pending interrupt has been generated (and the TMR bit for vector in question set). Signed-off-by: Marcelo Tosatti Signed-off-by: Avi Kivity --- virt/kvm/ioapic.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) (limited to 'virt') diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c index 4232fd75dd20..98778cb69c6e 100644 --- a/virt/kvm/ioapic.c +++ b/virt/kvm/ioapic.c @@ -45,7 +45,7 @@ #else #define ioapic_debug(fmt, arg...) #endif -static void ioapic_deliver(struct kvm_ioapic *vioapic, int irq); +static int ioapic_deliver(struct kvm_ioapic *vioapic, int irq); static unsigned long ioapic_read_indirect(struct kvm_ioapic *ioapic, unsigned long addr, @@ -89,8 +89,8 @@ static void ioapic_service(struct kvm_ioapic *ioapic, unsigned int idx) pent = &ioapic->redirtbl[idx]; if (!pent->fields.mask) { - ioapic_deliver(ioapic, idx); - if (pent->fields.trig_mode == IOAPIC_LEVEL_TRIG) + int injected = ioapic_deliver(ioapic, idx); + if (injected && pent->fields.trig_mode == IOAPIC_LEVEL_TRIG) pent->fields.remote_irr = 1; } if (!pent->fields.trig_mode) @@ -133,7 +133,7 @@ static void ioapic_write_indirect(struct kvm_ioapic *ioapic, u32 val) } } -static void ioapic_inj_irq(struct kvm_ioapic *ioapic, +static int ioapic_inj_irq(struct kvm_ioapic *ioapic, struct kvm_vcpu *vcpu, u8 vector, u8 trig_mode, u8 delivery_mode) { @@ -143,7 +143,7 @@ static void ioapic_inj_irq(struct kvm_ioapic *ioapic, ASSERT((delivery_mode == IOAPIC_FIXED) || (delivery_mode == IOAPIC_LOWEST_PRIORITY)); - kvm_apic_set_irq(vcpu, vector, trig_mode); + return kvm_apic_set_irq(vcpu, vector, trig_mode); } static u32 ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest, @@ -186,7 +186,7 @@ static u32 ioapic_get_delivery_bitmask(struct kvm_ioapic *ioapic, u8 dest, return mask; } -static void ioapic_deliver(struct kvm_ioapic *ioapic, int irq) +static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq) { u8 dest = ioapic->redirtbl[irq].fields.dest_id; u8 dest_mode = ioapic->redirtbl[irq].fields.dest_mode; @@ -195,7 +195,7 @@ static void ioapic_deliver(struct kvm_ioapic *ioapic, int irq) u8 trig_mode = ioapic->redirtbl[irq].fields.trig_mode; u32 deliver_bitmask; struct kvm_vcpu *vcpu; - int vcpu_id; + int vcpu_id, r = 0; ioapic_debug("dest=%x dest_mode=%x delivery_mode=%x " "vector=%x trig_mode=%x\n", @@ -204,7 +204,7 @@ static void ioapic_deliver(struct kvm_ioapic *ioapic, int irq) deliver_bitmask = ioapic_get_delivery_bitmask(ioapic, dest, dest_mode); if (!deliver_bitmask) { ioapic_debug("no target on destination\n"); - return; + return 0; } switch (delivery_mode) { @@ -216,7 +216,7 @@ static void ioapic_deliver(struct kvm_ioapic *ioapic, int irq) vcpu = ioapic->kvm->vcpus[0]; #endif if (vcpu != NULL) - ioapic_inj_irq(ioapic, vcpu, vector, + r = ioapic_inj_irq(ioapic, vcpu, vector, trig_mode, delivery_mode); else ioapic_debug("null lowest prio vcpu: " @@ -234,7 +234,7 @@ static void ioapic_deliver(struct kvm_ioapic *ioapic, int irq) deliver_bitmask &= ~(1 << vcpu_id); vcpu = ioapic->kvm->vcpus[vcpu_id]; if (vcpu) { - ioapic_inj_irq(ioapic, vcpu, vector, + r = ioapic_inj_irq(ioapic, vcpu, vector, trig_mode, delivery_mode); } } @@ -246,6 +246,7 @@ static void ioapic_deliver(struct kvm_ioapic *ioapic, int irq) delivery_mode); break; } + return r; } void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level) -- cgit v1.2.3