On 2011-02-03 11:38, Alexander Graf wrote:
>
> 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.
Ah, I see. Marcelo recently merged back, resolving the tricky bits, and
now it should be much easier. Anyway.
>
> The patch works though. If everybody agrees to take it, we can drop patch 7/7
> of my ahci set.
Cool! I've a few more minor things to clean up here and will sent that
as a series later today, also for 0.14.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
