diff mbox series

[SPARC] question on LEON IRQMP interrupt controller.

Message ID 4b0a136a-7017-0890-6dd9-33bd8ddf3e3d@tribudubois.net
State New
Headers show
Series [SPARC] question on LEON IRQMP interrupt controller. | expand

Commit Message

Jean-Christophe DUBOIS Jan. 2, 2018, 11:13 a.m. UTC
Hi Mark, Artyom,

I am wondering if the IRQMP code in hw/intc/grlib_irqmp.c is correct 
when it comes to acknowledging interrupts.

With the actual code an interrupt can be lowered/acked only by an "ack" 
from the processor which means that the trap handler related to this 
external interrupt needs to be run for the ack to happen.

In particular this means that the interrupt cannot be acked only by 
software. Even if the software clears the "pending" interrupts (by 
writing to the CLEAR_OFFSET register before the interrupt handler is 
run) this does not clear the interrupt to the processor (which is kept 
asserted until the handler is run and the interrupt acked by the 
processor). Do you know if this is indeed the intended behavior (I 
understand that for most operating system the interrupt handler will be 
run at last and this does not make a difference)?

I would expect that clearing interrupt through software (by writing to 
the CLEAR_OFFSET register) would have the same effect as the processor 
acknowledgment (and could avoid to run the interrupt handler if things 
have already been taken care of by software).

Unfortunately the documentation I got (on the web) on the IRQMP is not 
very clear on the topic.

Anyway you can find below the patch I'd like to provide for IRQMP.

Thanks

JC

Comments

Fabien Chouteau Jan. 2, 2018, 6:58 p.m. UTC | #1
Hello Jean-Christophe,

I'm the original author of this patch and I add in copy my colleague
Frederic.

On 02/01/2018 12:13, Jean-Christophe DUBOIS wrote:
> I am wondering if the IRQMP code in hw/intc/grlib_irqmp.c is correct
> when it comes to acknowledging interrupts.
>
> With the actual code an interrupt can be lowered/acked only by an
> "ack" from the processor which means that the trap handler related to
> this external interrupt needs to be run for the ack to happen.
>
> In particular this means that the interrupt cannot be acked only by
> software. Even if the software clears the "pending" interrupts (by
> writing to the CLEAR_OFFSET register before the interrupt handler is
> run) this does not clear the interrupt to the processor (which is kept
> asserted until the handler is run and the interrupt acked by the
> processor). Do you know if this is indeed the intended behavior (I
> understand that for most operating system the interrupt handler will
> be run at last and this does not make a difference)?
>
> I would expect that clearing interrupt through software (by writing to
> the CLEAR_OFFSET register) would have the same effect as the processor
> acknowledgment (and could avoid to run the interrupt handler if things
> have already been taken care of by software).
>
> Unfortunately the documentation I got (on the web) on the IRQMP is not
> very clear on the topic.
>

I don't remember all the details of this CPU on top of my head, I worked
on this years ago.

If you have access to a real board the best would be to compare the
behavior of the CPU on it. There's also a cycle accurate simulator of
Leon3, you can download an evaluation version here:
http://www.gaisler.com/index.php/downloads/simulators


> Anyway you can find below the patch I'd like to provide for IRQMP.
>

Can you explain the reason for this change? Why can't you use the
current interrupt handling?

Regards,
Jean-Christophe DUBOIS Jan. 3, 2018, 9:23 a.m. UTC | #2
Le 2018-01-02 19:58, Fabien Chouteau a écrit :
> Hello Jean-Christophe,
> 
> I'm the original author of this patch and I add in copy my colleague
> Frederic.
> 
> On 02/01/2018 12:13, Jean-Christophe DUBOIS wrote:
>> I am wondering if the IRQMP code in hw/intc/grlib_irqmp.c is correct
>> when it comes to acknowledging interrupts.
>> 
>> With the actual code an interrupt can be lowered/acked only by an
>> "ack" from the processor which means that the trap handler related to
>> this external interrupt needs to be run for the ack to happen.
>> 
>> In particular this means that the interrupt cannot be acked only by
>> software. Even if the software clears the "pending" interrupts (by
>> writing to the CLEAR_OFFSET register before the interrupt handler is
>> run) this does not clear the interrupt to the processor (which is kept
>> asserted until the handler is run and the interrupt acked by the
>> processor). Do you know if this is indeed the intended behavior (I
>> understand that for most operating system the interrupt handler will
>> be run at last and this does not make a difference)?
>> 
>> I would expect that clearing interrupt through software (by writing to
>> the CLEAR_OFFSET register) would have the same effect as the processor
>> acknowledgment (and could avoid to run the interrupt handler if things
>> have already been taken care of by software).
>> 
>> Unfortunately the documentation I got (on the web) on the IRQMP is not
>> very clear on the topic.
>> 
> 
> I don't remember all the details of this CPU on top of my head, I 
> worked
> on this years ago.
> 
> If you have access to a real board the best would be to compare the
> behavior of the CPU on it.

Unfortunately I don't have a real board (yet).

> There's also a cycle accurate simulator of
> Leon3, you can download an evaluation version here:
> http://www.gaisler.com/index.php/downloads/simulators

OK, I will try the tsim simulator from Gaisler as a reference.

> 
>> Anyway you can find below the patch I'd like to provide for IRQMP.
>> 
> 
> Can you explain the reason for this change? Why can't you use the
> current interrupt handling?

I am working on a cooperative multitasking kernel (with no preemption). 
So the kernel is not handling interrupt related traps (actually the 
kernel is not handling the interrupt controller). All interrupts are 
masked at all time for all application or kernel so no interrupt trap 
handler is ever going to trigger (except for IRQ 15 which is not 
maskable).

When the tasks have nothing to do the kernel goes to sleep using ASR19 
on LEON. So the system is awaken by next interrupt and the kernel will 
schedule the task handling the interrupt controller.

On LEON, I can go to sleep until the first interrupt. Once the first 
interrupt has been raised the LEON will never be able to get to 
sleep/idle again through ASR19 (it exists immediately) even if the 
interrupt controller handling task clears the interrupt (writing to 
CLEAR_OFFSET register). And this is because, in the actual Qemu 
implementation, the interrupt can only be acknowledged in the interrupt 
controller by the CPU through the triggering of the related trap 
handler.

So I am wondering if this is indeed the expected behavior in real 
hardware/life (interrupts can only be acked by processor and not by 
software).

Note: On a related subject I am wondering if the system (put in 
idle/sleep through ASR19) would woke up on interrupt if the interrupts 
are all masked through PSR (PIL) (it does wake up in Qemu for now). I 
will also test this on tsim before trying it on real hardware someday.

> 
> Regards,
Jean-Christophe DUBOIS Jan. 6, 2018, 3:52 p.m. UTC | #3
Hi,

So after trying my code on tsim, I can confirm that the software is 
indeed able to clear/ack the interrupt without requiring the ack from 
the processor.

Things are a bit strange with tsim as the simulator doesn't seem to 
respect time delay when the processor is in sleep/idle mode and jump 
straight to the next timer expiration/interrupt (incrementing the 
required clock cycles in the process) ...

And as the evaluation version of tsim stops the program after 2^32 clock 
cycles the program could not run more than 86 seconds (with the LEON 
clock @ 50 MHz).

But still the interrupts are acknowledged by software only without 
requiring the trap handler to run or the processor to ack the interrupt 
on a hardware way.

So I believe that my proposed modification is correct.

On a related note, tsim is using different interrupts (than Qemu) for 
timer and all. So what is the reference LEON3 platform that Qemu is 
emulating with leon3_generic?

JC

Le 03/01/2018 à 10:23, jcd@tribudubois.net a écrit :
> Le 2018-01-02 19:58, Fabien Chouteau a écrit :
>> Hello Jean-Christophe,
>>
>> I'm the original author of this patch and I add in copy my colleague
>> Frederic.
>>
>> On 02/01/2018 12:13, Jean-Christophe DUBOIS wrote:
>>> I am wondering if the IRQMP code in hw/intc/grlib_irqmp.c is correct
>>> when it comes to acknowledging interrupts.
>>>
>>> With the actual code an interrupt can be lowered/acked only by an
>>> "ack" from the processor which means that the trap handler related to
>>> this external interrupt needs to be run for the ack to happen.
>>>
>>> In particular this means that the interrupt cannot be acked only by
>>> software. Even if the software clears the "pending" interrupts (by
>>> writing to the CLEAR_OFFSET register before the interrupt handler is
>>> run) this does not clear the interrupt to the processor (which is kept
>>> asserted until the handler is run and the interrupt acked by the
>>> processor). Do you know if this is indeed the intended behavior (I
>>> understand that for most operating system the interrupt handler will
>>> be run at last and this does not make a difference)?
>>>
>>> I would expect that clearing interrupt through software (by writing to
>>> the CLEAR_OFFSET register) would have the same effect as the processor
>>> acknowledgment (and could avoid to run the interrupt handler if things
>>> have already been taken care of by software).
>>>
>>> Unfortunately the documentation I got (on the web) on the IRQMP is not
>>> very clear on the topic.
>>>
>>
>> I don't remember all the details of this CPU on top of my head, I worked
>> on this years ago.
>>
>> If you have access to a real board the best would be to compare the
>> behavior of the CPU on it.
>
> Unfortunately I don't have a real board (yet).
>
>> There's also a cycle accurate simulator of
>> Leon3, you can download an evaluation version here:
>> http://www.gaisler.com/index.php/downloads/simulators
>
> OK, I will try the tsim simulator from Gaisler as a reference.
>
>>
>>> Anyway you can find below the patch I'd like to provide for IRQMP.
>>>
>>
>> Can you explain the reason for this change? Why can't you use the
>> current interrupt handling?
>
> I am working on a cooperative multitasking kernel (with no 
> preemption). So the kernel is not handling interrupt related traps 
> (actually the kernel is not handling the interrupt controller). All 
> interrupts are masked at all time for all application or kernel so no 
> interrupt trap handler is ever going to trigger (except for IRQ 15 
> which is not maskable).
>
> When the tasks have nothing to do the kernel goes to sleep using ASR19 
> on LEON. So the system is awaken by next interrupt and the kernel will 
> schedule the task handling the interrupt controller.
>
> On LEON, I can go to sleep until the first interrupt. Once the first 
> interrupt has been raised the LEON will never be able to get to 
> sleep/idle again through ASR19 (it exists immediately) even if the 
> interrupt controller handling task clears the interrupt (writing to 
> CLEAR_OFFSET register). And this is because, in the actual Qemu 
> implementation, the interrupt can only be acknowledged in the 
> interrupt controller by the CPU through the triggering of the related 
> trap handler.
>
> So I am wondering if this is indeed the expected behavior in real 
> hardware/life (interrupts can only be acked by processor and not by 
> software).
>
> Note: On a related subject I am wondering if the system (put in 
> idle/sleep through ASR19) would woke up on interrupt if the interrupts 
> are all masked through PSR (PIL) (it does wake up in Qemu for now). I 
> will also test this on tsim before trying it on real hardware someday.
>
>>
>> Regards,
>
>
>
Mark Cave-Ayland Jan. 8, 2018, 7:56 p.m. UTC | #4
On 02/01/18 11:13, Jean-Christophe DUBOIS wrote:

> Hi Mark, Artyom,
> 
> I am wondering if the IRQMP code in hw/intc/grlib_irqmp.c is correct 
> when it comes to acknowledging interrupts.
> 
> With the actual code an interrupt can be lowered/acked only by an "ack" 
> from the processor which means that the trap handler related to this 
> external interrupt needs to be run for the ack to happen.
> 
> In particular this means that the interrupt cannot be acked only by 
> software. Even if the software clears the "pending" interrupts (by 
> writing to the CLEAR_OFFSET register before the interrupt handler is 
> run) this does not clear the interrupt to the processor (which is kept 
> asserted until the handler is run and the interrupt acked by the 
> processor). Do you know if this is indeed the intended behavior (I 
> understand that for most operating system the interrupt handler will be 
> run at last and this does not make a difference)?
> 
> I would expect that clearing interrupt through software (by writing to 
> the CLEAR_OFFSET register) would have the same effect as the processor 
> acknowledgment (and could avoid to run the interrupt handler if things 
> have already been taken care of by software).
> 
> Unfortunately the documentation I got (on the web) on the IRQMP is not 
> very clear on the topic.
> 
> Anyway you can find below the patch I'd like to provide for IRQMP.
> 
> Thanks

Thanks for the patch! I'm afraid I don't really have any experience with 
LEON as my focus is sun4m/sun4u, however I'm happy to take patches 
Acked/Reviewed by Fabien as the current LEON maintainer if they don't 
cause any regressions in my own tests.


ATB,

Mark.
Jean-Christophe DUBOIS Jan. 8, 2018, 8:44 p.m. UTC | #5
Le 08/01/2018 à 20:56, Mark Cave-Ayland a écrit :
> Thanks for the patch! I'm afraid I don't really have any experience 
> with LEON as my focus is sun4m/sun4u, however I'm happy to take 
> patches Acked/Reviewed by Fabien as the current LEON maintainer

I am waiting for Fabien feedback after my experiment with tsim.

> if they don't cause any regressions in my own tests.

Of course.

JC

>
>
> ATB,
>
> Mark.
diff mbox series

Patch

diff --git a/hw/intc/grlib_irqmp.c b/hw/intc/grlib_irqmp.c
index 94659ee256..d6f9cb3692 100644
--- a/hw/intc/grlib_irqmp.c
+++ b/hw/intc/grlib_irqmp.c
@@ -106,6 +106,15 @@  static void grlib_irqmp_check_irqs(IRQMPState *state)
      }
  }

+static void grlib_irqmp_ack_mask(IRQMPState *state, uint32_t mask)
+{
+    /* Clear registers */
+    state->pending  &= ~mask;
+    state->force[0] &= ~mask; /* Only CPU 0 (No SMP support) */
+
+    grlib_irqmp_check_irqs(state);
+}
+
  void grlib_irqmp_ack(DeviceState *dev, int intno)
  {
      IRQMP        *irqmp = GRLIB_IRQMP(dev);
@@ -120,11 +129,7 @@  void grlib_irqmp_ack(DeviceState *dev, int intno)

      trace_grlib_irqmp_ack(intno);

-    /* Clear registers */
-    state->pending  &= ~mask;
-    state->force[0] &= ~mask; /* Only CPU 0 (No SMP support) */
-
-    grlib_irqmp_check_irqs(state);
+    grlib_irqmp_ack_mask(state, mask);
  }

  void grlib_irqmp_set_irq(void *opaque, int irq, int level)
@@ -251,7 +256,7 @@  static void grlib_irqmp_write(void *opaque, hwaddr addr,

      case CLEAR_OFFSET:
          value &= ~1; /* clean up the value */
-        state->pending &= ~value;
+        grlib_irqmp_ack_mask(state, value);
          return;

      case MP_STATUS_OFFSET: