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 <[email protected]>
>
> ---
>
> 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
[email protected] http://www.aurel32.net