[PATCH] KVM: IOAPIC: Fix level-triggered irq injection hang

[PATCH] KVM: IOAPIC: Fix level-triggered irq injection hang

[Mark McLoughlin]

The "remote_irr" variable is used to indicate an interrupt
which has been received by the LAPIC, but not acked.

In our EOI handler, we unset remote_irr and re-inject the
interrupt if the interrupt line is still asserted.

However, we do not set remote_irr here, leading to a
situation where if kvm_ioapic_set_irq() is called, then we go
ahead and call ioapic_service(). This means that IRR is
re-asserted even though the interrupt is currently in service
(i.e. LAPIC IRR is cleared and ISR/TMR set)

The issue with this is that when the currently executing
interrupt handler finishes and writes LAPIC EOI, then TMR is
unset and EOI sent to the IOAPIC. Since IRR is now asserted,
but TMR is not, then when the second interrupt is handled,
no EOI is sent and if there is any pending interrupt, it is
not re-injected.

This fixes a hang only seen while running mke2fs -j on an
8Gb virtio disk backed by a fully sparse raw file, with
aliguori "avoid fragmented virtio-blk transfers by copying"
changes.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
Acked-by: Marcelo Tosatti <mtosatti@redhat.com>
---
 virt/kvm/ioapic.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index 1dcf9f3..4458908 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -278,7 +278,7 @@ static void __kvm_ioapic_update_eoi(struct kvm_ioapic *ioapic, int gsi)
 
 	ent->fields.remote_irr = 0;
 	if (!ent->fields.mask && (ioapic->irr & (1 << gsi)))
-		ioapic_deliver(ioapic, gsi);
+		ioapic_service(ioapic, gsi);
 }
 
 void kvm_ioapic_update_eoi(struct kvm *kvm, int vector)
-- 
1.5.4.1

Re: [PATCH] KVM: IOAPIC: Fix level-triggered irq injection hang

[Avi Kivity]

Mark McLoughlin wrote:
> The "remote_irr" variable is used to indicate an interrupt
> which has been received by the LAPIC, but not acked.
>
> In our EOI handler, we unset remote_irr and re-inject the
> interrupt if the interrupt line is still asserted.
>
> However, we do not set remote_irr here, leading to a
> situation where if kvm_ioapic_set_irq() is called, then we go
> ahead and call ioapic_service(). This means that IRR is
> re-asserted even though the interrupt is currently in service
> (i.e. LAPIC IRR is cleared and ISR/TMR set)
>
> The issue with this is that when the currently executing
> interrupt handler finishes and writes LAPIC EOI, then TMR is
> unset and EOI sent to the IOAPIC. Since IRR is now asserted,
> but TMR is not, then when the second interrupt is handled,
> no EOI is sent and if there is any pending interrupt, it is
> not re-injected.
>
> This fixes a hang only seen while running mke2fs -j on an
> 8Gb virtio disk backed by a fully sparse raw file, with
> aliguori "avoid fragmented virtio-blk transfers by copying"
> changes.
>
>   

Good catch indeed; applied.  I think it also fixes the case where the 
ioapic entry is masked when the eoi occurs.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.