public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Marcelo Tosatti <mtosatti@redhat.com>
To: Avi Kivity <avi@qumranet.com>, Gerd von Egidy <lists@egidy.de>
Cc: kvm-devel <kvm@vger.kernel.org>
Subject: KVM: IOAPIC: don't clear remote_irr if IRQ is reinjected from EOI
Date: Thu, 5 Jun 2008 00:08:11 -0300	[thread overview]
Message-ID: <20080605030811.GA10631@dmt.cnet> (raw)


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 <mtosatti@redhat.com>


diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index 99a1736..92ed191 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 void ioapic_inj_nmi(struct kvm_vcpu *vcpu)
@@ -191,7 +191,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;
@@ -200,7 +200,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",
@@ -209,7 +209,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) {
@@ -221,7 +221,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: "
@@ -239,7 +239,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);
 			}
 		}
@@ -262,6 +262,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)

             reply	other threads:[~2008-06-05  3:08 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-05  3:08 Marcelo Tosatti [this message]
2008-06-05  3:13 ` KVM: IOAPIC: don't clear remote_irr if IRQ is reinjected from EOI Marcelo Tosatti
2008-06-05  7:55 ` Avi Kivity

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080605030811.GA10631@dmt.cnet \
    --to=mtosatti@redhat.com \
    --cc=avi@qumranet.com \
    --cc=kvm@vger.kernel.org \
    --cc=lists@egidy.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox