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 <ag...@suse.de>
>>
>> ---
>>
>> 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
