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

[Blue Swirl <blauwirbel@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3963 bytes --]

On Mon, Nov 16, 2009 at 7:19 PM, Artyom Tarasenko
<atar4qemu@googlemail.com> wrote:
> 2009/11/16 Blue Swirl <blauwirbel@gmail.com>:
>> On Mon, Nov 16, 2009 at 1:47 PM, Artyom Tarasenko
>> <atar4qemu@googlemail.com> wrote:
>>> 2009/11/15 Blue Swirl <blauwirbel@gmail.com>:
>>>> On Sun, Nov 15, 2009 at 1:15 AM, Artyom Tarasenko
>>>> <atar4qemu@googlemail.com> wrote:
>>>>> 2009/11/14 Blue Swirl <blauwirbel@gmail.com>:
>>>>>> On Sat, Nov 14, 2009 at 3:03 AM, Artyom Tarasenko
>>>>>> <atar4qemu@googlemail.com> 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.

[-- Attachment #2: 0001-Sparc32-latch-incoming-interrupts.patch --]
[-- Type: application/x-patch, Size: 1483 bytes --]