On Tue, Feb 01, 2011 at 07:35:01PM +0100, Alexander Graf wrote: > When using level based interrupts, the interrupt is treated the same as an > edge triggered one: leaving the line up does not retrigger the interrupt. > > In fact, when not lowering the line, we won't ever get a new interrupt inside > the guest. So let's always retrigger an interrupt as soon as the OS ack'ed > something on the device. This way we're sure the guest doesn't starve on > interrupts until someone fixes the actual interrupt path. > > Signed-off-by: Alexander Graf <ag...@suse.de> > > --- > > v2 -> v3: > > - add comment about the interrupt hack > --- > hw/ide/ahci.c | 8 ++++++-- > 1 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c > index 98bdf70..95e1cf7 100644 > --- a/hw/ide/ahci.c > +++ b/hw/ide/ahci.c > @@ -152,11 +152,15 @@ static void ahci_check_irq(AHCIState *s) > } > } > > + /* XXX We lower the interrupt line here because of a bug with interrupt > + handling in Qemu. When leaving a line up, the interrupt does > + not get retriggered automatically currently. Once that bug is > fixed, > + this workaround is not necessary anymore and we only need to lower > + in the else branch. */ > + ahci_irq_lower(s, NULL); > if (s->control_regs.irqstatus && > (s->control_regs.ghc & HOST_CTL_IRQ_EN)) { > ahci_irq_raise(s, NULL); > - } else { > - ahci_irq_lower(s, NULL); > } > } >
It's a lot better that way, however I think we should still keep the correct code. Also given this problem only concerns the i386 target (ppc code is actually a bit strange, but at the end does the correct thing), it's something we should probably mention. What about something like that? diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 671b4df..0b153f0 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -489,12 +489,25 @@ static void ahci_check_irq(AHCIState *s) } } +#if 0 if (s->control_regs.irqstatus && (s->control_regs.ghc & HOST_CTL_IRQ_EN)) { ahci_irq_raise(s, NULL); } else { ahci_irq_lower(s, NULL); } +#else + /* XXX We lower the interrupt line here because of a bug with interrupt + handling in Qemu on the i386 target. When leaving a line up, the + interrupt does not get retriggered automatically currently. Once + that bug is fixed, this workaround is not necessary anymore and + we only need to lower in the else branch. */ + ahci_irq_lower(s, NULL); + if (s->control_regs.irqstatus && + (s->control_regs.ghc & HOST_CTL_IRQ_EN)) { + ahci_irq_raise(s, NULL); + } +#endif } static void ahci_trigger_irq(AHCIState *s, AHCIDevice *d, -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net