[Qemu-devel] Re: [PATCH] sparc32 irq clearing (guest Solaris performance+NetBSD) fix

[Artyom Tarasenko]

2009/11/16 Blue Swirl <address@hidden>:
> On Mon, Nov 16, 2009 at 7:19 PM, Artyom Tarasenko
> <address@hidden> wrote:
>> 2009/11/16 Blue Swirl <address@hidden>:
>>> On Mon, Nov 16, 2009 at 1:47 PM, Artyom Tarasenko
>>> <address@hidden> wrote:
>>>> 2009/11/15 Blue Swirl <address@hidden>:
>>>>> On Sun, Nov 15, 2009 at 1:15 AM, Artyom Tarasenko
>>>>> <address@hidden> wrote:
>>>>>> 2009/11/14 Blue Swirl <address@hidden>:
>>>>>>> On Sat, Nov 14, 2009 at 3:03 AM, Artyom Tarasenko
>>>>>>> <address@hidden> wrote:
>>>>>>>> According to NCR89C105 documentation
>>>>>>>> http://www.ibiblio.org/pub/historic-linux/early-ports/Sparc/NCR/NCR89C105.txt
>>>>>>>>
>>>>>>>> Interrupts are cleared by disabling and then re-enabling them.
>>>>>>>> This patch implements the specified behaviour. The most visible 
>>>>>>>> effects:
>>>>>>>
>>>>>>> I think the current version also implements this behaviour. The
>>>>>>> difference is that now we clear on disable, with your version, the
>>>>>>> interrupts are cleared when re-enabling them.
>>>>>>
>>>>>> Doesn't this imply that the current version does not implement this
>>>>>> ("Interrupts are cleared by disabling and then re-enabling them") 
>>>>>> behavior? ;-)
>>>>>
>>>>> The specification only says that the sequence disable-enable clears
>>>>> interrupts, but not which of these is true:
>>>>> - clearing happens in the moment of disabling (and interrupts after
>>>>> that are not cleared, current version)
>>>>> - clearing happens  in the moment of re-enabling (your version, sort of)
>>>>> - clearing happens in both cases (lose interrupts)
>>>>
>>>> English is not my native language, but fwiw I think "and then
>>>> re-enabling" can only be the second variant. Without "then" it could
>>>> be either one or three. And if the first variant is what they really
>>>> meant, the part with "and then" is totally redundant and misleading.
>>>
>>> Still, this is user documentation, not implementation specification.
>>> I'm open to both versions, if they work.
>>>
>>>>> It's also interesting to think what happens between the interrupt
>>>>> controller and the devices. Clearing an interrupt at controller level
>>>>> does not clear the interrupt condition at the device. Aren't the
>>>>> interrupts level triggered on Sparc, so the interrupt is still posted?
>>>>> Is the interrupt actually masked by clearing until the level is
>>>>> deactivated?
>>>>
>>>> Looks unlikely to me. In the book "Panic! Unix system crash dump
>>>> analysis" the authors write that the first thing interrupt handler has
>>>> to do is disable the interrupt, and yes wrting "unix" they mean
>>>> "SunOS/Solaris".
>>>>
>>>> That's also what I observe debugging the Solaris kernel code
>>>> (Solaris kernel debugger is a really powerful tool).
>>>> Looks like interrupts can be shared between devices, so the general
>>>> handler disables the interrupt and then calls multiple device-specific
>>>> handlers sequentially and checks if any of then claims the interrupt.
>>>> If no one does it writes the message "Spurious interrupt %d\n".
>>>>
>>>>
>>>>> Or maybe the controller latches the interrupt so that even after the
>>>>> device releases the interrupt line, interrupt is still active towards
>>>>> the CPU. Then the clearing would make sense.
>>>>
>>>> Looks very realistic to me. I think that's the way the interrupts are
>>>> handled at least under x86.
>>>
>>> It's a must on x86, because the interrupts are edge triggered.
>>
>> I don't know, how the real sun4m reacts in the case where irq stays
>> on, not being cleared.
>> It can not be though that it would try to process irq for every next
>> tick. The CPU must have some time to clear the pending irq, so it must
>> be edge triggered too, at least in a way.
>
> This patch makes the interrupts latch: ignore source clearing the
> interrupt. It seems be ~okay for my usual test setup, but does not
> help NetBSD 1.3.3. Some other NetBSD tests are changed, but they
> crashed before.

Makes no difference in my tests. Except that my broken patch really
clears too many interrupts when combined with it. Will play with it
further.