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

[Alexander Graf]

Kevin Wolf wrote:
> 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.
>
>   

Ok, done :)


Alex