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

[Kevin Wolf]

Am 01.02.2011 21:10, schrieb Alexander Graf:
> 
> On 01.02.2011, at 20:58, Aurelien Jarno wrote:
> 
>> 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 <address@hidden>
>>>
>>> ---
>>>
>>> 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?
> 
> While I dislike #if 0s in released code in general, it's fine with me. I know 
> what I meant based on the comment, but for others this might make it more 
> explicit. How would we go about committing this? Kevin, will you just change 
> the code inside your tree?

I would prefer if you sent a new version of this patch.

Kevin