diff mbox

KVM: IOAPIC: Do not check polarity

Message ID loom.20090313T105104-837@post.gmane.org (mailing list archive)
State Not Applicable
Headers show

Commit Message

Bernhard Kohl March 13, 2009, 10:53 a.m. UTC
NSN's proprietary OS DMX heavily depends on a correct BIOS MP table.
From there it also gets the information for initialization of PCI
devices. We defined the polarity and trigger mode as for standard
PCI devices "low level". The OS programs the ioapic the same way.

Nothing else in QEMU's and KVM's IRQ, PIC, IOAPIC and LAPIC code
cares about interrupt polarity. The "level" is always used as a
logical value. So we must not change the level here. If we do so,
PCI low level interrupts are not fired if the guest raises the
interrupt (level=1), but first if it is reset again (level=0). This
can result in infinite waiting for a single interupt. For example
the DMX guest waits for the link up interupt of the E1000
ethernet device before enabling the TX and RX interupts of
this device. E1000 emulation sets level=1, no interrupt is fired,
deadlock.

This does not happen if we use the -no-kvm-irqchip option.
This means that qemu's ioapic does not care about the polarity and
kvm's ioapic shouldn't do either.

Signed-off-by: Bernhard Kohl <bernhard.kohl@nsn.com>
---
 virt/kvm/ioapic.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

 		else {

Comments

Avi Kivity March 15, 2009, 10:38 a.m. UTC | #1
Bernhard Kohl wrote:
> NSN's proprietary OS DMX heavily depends on a correct BIOS MP table.
> From there it also gets the information for initialization of PCI
> devices. We defined the polarity and trigger mode as for standard
> PCI devices "low level". The OS programs the ioapic the same way.
>
> Nothing else in QEMU's and KVM's IRQ, PIC, IOAPIC and LAPIC code
> cares about interrupt polarity. The "level" is always used as a
> logical value. So we must not change the level here. If we do so,
> PCI low level interrupts are not fired if the guest raises the
> interrupt (level=1), but first if it is reset again (level=0). This
> can result in infinite waiting for a single interupt. For example
> the DMX guest waits for the link up interupt of the E1000
> ethernet device before enabling the TX and RX interupts of
> this device. E1000 emulation sets level=1, no interrupt is fired,
> deadlock.
>
> This does not happen if we use the -no-kvm-irqchip option.
> This means that qemu's ioapic does not care about the polarity and
> kvm's ioapic shouldn't do either.
>   

While qemu uses active-high interrupts throughout, kvm is meant to be 
useful with other userspace (or for emulating other chipsets) and thus 
needs correct polarity emulation.

If the guest programs polarity to active-low, this indicates either a 
guest bug or a bios bug.  Is it possible the bios does not communicate 
the fact that pci interrupts are active high correctly?
Bernhard Kohl March 16, 2009, 10:30 p.m. UTC | #2
Avi Kivity <avi <at> redhat.com> writes:
> 
> While qemu uses active-high interrupts throughout, kvm is meant to be 
> useful with other userspace (or for emulating other chipsets) and thus 
> needs correct polarity emulation.
> 
> If the guest programs polarity to active-low, this indicates either a 
> guest bug or a bios bug.  Is it possible the bios does not communicate 
> the fact that pci interrupts are active high correctly?
> 

The guest gets the polarity from the BIOS MP table, which I added. I defined
the polarity and trigger mode "low level". I will change that to "high level".
Need to wait for testing until next week.


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index 1eddae9..4deb8e1 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -180,7 +180,6 @@  int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq,
int level)
 
 	if (irq >= 0 && irq < IOAPIC_NUM_PINS) {
 		entry = ioapic->redirtbl[irq];
-		level ^= entry.fields.polarity;
 		if (!level)
 			ioapic->irr &= ~mask;