On Mon, Nov 16, 2009 at 7:19 PM, Artyom Tarasenko
<atar4q...@googlemail.com> wrote:
> 2009/11/16 Blue Swirl <blauwir...@gmail.com>:
>> On Mon, Nov 16, 2009 at 1:47 PM, Artyom Tarasenko
>> <atar4q...@googlemail.com> wrote:
>>> 2009/11/15 Blue Swirl <blauwir...@gmail.com>:
>>>> On Sun, Nov 15, 2009 at 1:15 AM, Artyom Tarasenko
>>>> <atar4q...@googlemail.com> wrote:
>>>>> 2009/11/14 Blue Swirl <blauwir...@gmail.com>:
>>>>>> On Sat, Nov 14, 2009 at 3:03 AM, Artyom Tarasenko
>>>>>> <atar4q...@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.
From bf8c381c4c6f7140ddc73865eba64640f4c75352 Mon Sep 17 00:00:00 2001
From: Blue Swirl <blauwirbel@gmail.com>
Date: Mon, 16 Nov 2009 21:48:53 +0000
Subject: [PATCH] Sparc32: latch incoming interrupts
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
hw/slavio_intctl.c | 11 ++---------
1 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/hw/slavio_intctl.c b/hw/slavio_intctl.c
index 9aff892..a67d5c9 100644
--- a/hw/slavio_intctl.c
+++ b/hw/slavio_intctl.c
@@ -188,8 +188,8 @@ static void slavio_intctlm_mem_writel(void *opaque, target_phys_addr_t addr,
case 3: // set (disable, clear pending)
// Force clear unused bits
val &= MASTER_IRQ_MASK;
- s->intregm_disabled |= val;
s->intregm_pending &= ~val;
+ s->intregm_disabled |= val;
slavio_check_interrupts(s, 1);
DPRINTF("Disabled master irq mask %x, curmask %x\n", val,
s->intregm_disabled);
@@ -338,15 +338,8 @@ static void slavio_set_irq(void *opaque, int irq, int level)
s->slaves[i].intreg_pending |= 1 << pil;
}
}
- } else {
- s->intregm_pending &= ~mask;
- if (pil == 15) {
- for (i = 0; i < MAX_CPUS; i++) {
- s->slaves[i].intreg_pending &= ~(1 << pil);
- }
- }
+ slavio_check_interrupts(s, 1);
}
- slavio_check_interrupts(s, 1);
}
}
--
1.5.6.5
