[Qemu-devel] Re: [PATCH 7/7] ahci: work around bug with level interrupts

[Alexander Graf]

On 03.02.2011, at 11:30, Jan Kiszka wrote:

> On 2011-02-02 15:39, 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 <address@hidden>
>> 
>> ---
>> 
>> v2 -> v3:
>> 
>>  - add comment about the interrupt hack
>> 
>> v3 -> v4:
>> 
>>  - embed non-workaround version in the code
>> ---
>> hw/ide/ahci.c |   13 +++++++++++++
>> 1 files changed, 13 insertions(+), 0 deletions(-)
>> 
>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>> index 98bdf70..10377ca 100644
>> --- a/hw/ide/ahci.c
>> +++ b/hw/ide/ahci.c
>> @@ -152,12 +152,25 @@ static void ahci_check_irq(AHCIState *s)
>>         }
>>     }
>> 
>> +    /* XXX We always 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. */
>> +#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
>> +    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,
> 
> Could you check if this quick-hack obsoletes the above hack?
> 
> I was hoping it has some influence on our 64-bit Windows issue, but it
> hasn't, or it's still buggy, or both. However, its intention is to
> reassert still pending level-triggered IRQs on APIC EOI. This logic is
> missing in the user space model but exists in KVM's kernel model (I was
> asking for a test against the latter but unfortunately did not receive
> an answer - I bet you already don't need your patch over qemu-kvm).

Oh, sorry for not replying there. I work on qemu.git, so the in-kernel apic is 
out of question for testing. I tried merging qemu-kvm.git and qemu.git several 
times and every time just wasted a few hours of my life, so I gave up on 
testing things on qemu-kvm.git.

The patch works though. If everybody agrees to take it, we can drop patch 7/7 
of my ahci set.


Alex