[PATCH] bsps/riscv: Clear interrupt complete before disable

[Sebastian Huber]

On 14.03.23 14:37, Padmarao.Begari at microchip.com wrote:
>> On Tue, 2023-03-14 at 13:37 +0100, Sebastian Huber wrote:
>>
>> On 14.03.23 13:31,Padmarao.Begari at microchip.com  wrote:
>>> Hi Sebastian,
>>>
>>>> On Mon, 2023-03-06 at 14:11 +0100, Sebastian Huber wrote:
>>>>
>>>>
>>>> On 06.03.23 13:00,Padmarao.Begari at microchip.com   wrote:
>>>>>> On Mon, 2023-03-06 at 11:19 +0100, Sebastian Huber wrote:
>>>>>>
>>>>>> On 06.03.23 10:24,Padmarao.Begari at microchip.com    wrote:
>>>>>>> Hi Sebastian,
>>>>>>>
>>>>>>>> On Mon, 2023-03-06 at 09:41 +0100, Sebastian Huber wrote:
>>>>>>>>
>>>>>>>> On 06.03.23 09:37,Padmarao.Begari at microchip.com    wrote:
>>>>>>>>>> Is
>>>>>>>>>> the claim complete message ignored if the interrupt
>>>>>>>>>> is
>>>>>>>>>> disabled?
>>>>>>>>>>
>>>>>>>>> Yes.
>>>>>>>> Is this an intended and documented behaviour of the PLIC?
>>>>>>> Not documented
>>>>>> Is this a common PLIC behaviour or just the case for the
>>>>>> MicroChip
>>>>>> implementation?
>>>>>>
>>>>> It's a common PLIC behaviour.
>>>> It is not implemented in the Qemu PLIC emulation:
>>>>
>>>> https://github.com/qemu/qemu/blob/master/hw/intc/sifive_plic.c#L242
>>>>
>>>> Where do I see this behaviour in a PLIC implementation, for
>>>> example:
>>>>
>>>> https://github.com/lowRISC/opentitan/tree/master/hw/ip_templates/rv_plic
>>>>
>>>> That the interrupt completion depends on the interrupt
>>>> enable/disable
>>>> status is quite unusual.
>>>>
>>> https://github.com/riscv/riscv-plic-spec/blob/master/riscv-plic.adoc#8-interrupt-completion
>>> The interrupt is completed only when the interrupt is enabled but
>>> not
>>> when disabled.
>>>
>>> Can we clear the interrupt before calling the interrupt handler?
>>> like
>>> below
>>> https://github.com/RTEMS/rtems/blob/master/bsps/riscv/riscv/irq/irq.c#L90
>>>
>>>       while ((interrupt_index = plic_hart_regs->claim_complete) !=
>>> 0) {
>>> +     plic_hart_regs->claim_complete = interrupt_index;
>>>         bsp_interrupt_handler_dispatch(
>>>           RISCV_INTERRUPT_VECTOR_EXTERNAL(interrupt_index)
>>>         );
>>>
>>> -      plic_hart_regs->claim_complete = interrupt_index;
>>>
>>> Also refer
>>> https://github.com/psherman42/Demonstrating-MTVEC/blob/main/start.s#L307
>> At some point I would like to enable support for nested interrupts. I
>> guess in this case it is important that you complete the interrupt
>> after
>> processing.
>>
> The Exception(trap) handler is called when an interrupt occurs and the
> global interrupt(mie) is disabled automatically at the top-most
> level–the MIE bit in mstatus and re-enabled after return from machine
> interrupt instruction "mret".
> 
> I think, we will not get any other interrupt while servicing the
> interrupt handler, we can use the interrupt complete before processing.

Currently yes, however, if we support nested interrupts then it would 
look like this:

   } else if (mcause == (RISCV_INTERRUPT_EXTERNAL_MACHINE << 1)) {
     volatile RISCV_PLIC_hart_regs *plic_hart_regs;
     uint32_t interrupt_index;

     plic_hart_regs = cpu_self->cpu_per_cpu.plic_hart_regs;

     while ((interrupt_index = plic_hart_regs->claim_complete) != 0) {

ENABLE MIE

       bsp_interrupt_handler_dispatch(
         RISCV_INTERRUPT_VECTOR_EXTERNAL(interrupt_index)
       );

DISABLE MIE

       plic_hart_regs->claim_complete = interrupt_index;

       /*
        * FIXME: It is not clear which fence is necessary here or if a 
fence is
        * necessary at all.  The goal is that the complete signal is somehow
        * recognized by the PLIC before the next claim is issued.
        */
       __asm__ volatile ("fence o, i" : : : "memory");
     }

The PLIC/CLINT combination is not really well suited for systems which 
want to use interrupt priorities since the software and timer interrupts 
bypass the PLIC interrupt priority handling. I don't know why hardware 
developers can't stick with the best existing solutions instead of 
reinventing the next second best solution.

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber at embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/