Enable Zephyr to handle shared interrupts

[sambhurst]

Introduction

Allow up to three Interrupt Service Routines (ISRs) to share one interrupt line.

Problem description

Many MCUs have peripherals that share interrupt lines. For example, the USB, UCPD1, and UCPD2 peripherals on the STM32G0B0x1 share interrupt 8. Currently no practical application can use those peripherals together without circumventing the Zephyr interrupt system or writing a custom device driver.

Proposed change

Interrupts are registered with a macro similar to IRQ_DIRECT_CONNECT that’s named IRQ_SHARE_CONNECT.
Allow up to three ISRs to share one interrupt line.
Only one of the user ISRs should service the interrupt, unless the interrupt is pending due to more than one source.

Detailed RFC

Create a new flag, ISR_FLAG_SHARE, that’s passed to Z_ISR_DECLARE.

/** This interrupt gets put directly in the vector table */
#define ISR_FLAG_DIRECT BIT(0)

/**
 * This interrupt gets put directly in the vector table and can
 * be shared with up to three ISRs
 */
#define ISR_FLAG_SHARE  BIT(1)

/* Create an instance of struct _isr_list which gets put in the .intList
 * section. This gets consumed by gen_isr_tables.py which creates the vector
 * and/or SW ISR tables.
 */
#define Z_ISR_DECLARE(irq, flags, func, param) \
        static Z_DECL_ALIGN(struct _isr_list) Z_GENERIC_SECTION(.intList) \
                __used _MK_ISR_NAME(func, __COUNTER__) = \
                        {irq, flags, (void *)&func, (const void *)param}

Add a new section to the linker script:

        *(.text)
        *(".text.*")
        *(".TEXT.*")
#if defined (CONFIG_SHARED_INTERRUPTS)
        *(".shared_isr")
#endif

When the gen_isr_tables.py script executes and detects that the ISR_FLAG_SHARE flag is set, it generates a shared ISR that calls the user ISRs in sequence. The generated ISR is placed in the .shared_isr section.

For example:

void usb_isr(void)
{
/* Test if interrupt should be serviced */
}

void ucpd_isr1(void)
{
/* Test if interrupt should be serviced */
}

void ucpd_isr2(void)
{
/* Test if interrupt should be serviced */
}

IRQ_SHARE_CONNECT(IRQ_8, PRI_1, usb_isr, ARCH_FLAG);
IRQ_SHARE_CONNECT(IRQ_8, PRI_1, ucpd_isr1, ARCH_FLAG);
IRQ_SHARE_CONNECT(IRQ_8, PRI_1, ucpd_isr2, ARCH_FLAG);

Resulting build/zephyr/isr_tables.c generated by gen_isr_tables.py:

/* AUTO-GENERATED by gen_isr_tables.py, do not edit! */

#include <zephyr/toolchain.h>
#include <zephyr/linker/sections.h>
#include <zephyr/sw_isr_table.h>
#include <zephyr/arch/cpu.h>

typedef void (* ISR)(const void *);
typedef void (* SHARE_ISR)(void);
...
void __attribute__((section(".shared_isr")))shared_isr_8(void);
...

uintptr_t __irq_vector_table _irq_vector_table[102] = {
        ((uintptr_t)&_isr_wrapper),
        ((uintptr_t)&_isr_wrapper),
        ((uintptr_t)&_isr_wrapper),
        ((uintptr_t)&_isr_wrapper),
        ((uintptr_t)&_isr_wrapper),
        ((uintptr_t)&_isr_wrapper),
        ((uintptr_t)&_isr_wrapper),
        ((uintptr_t)&_isr_wrapper),
        ((uintptr_t)&shared_isr_8),
...
};

... 

void __attribute__((section(".shared_isr"))) shared_isr_8(void){
        ((SHARE_ISR)0x80004fd)();
        ((SHARE_ISR)0x80004d9)();
        ((SHARE_ISR)0x80004b5)();
}

I have only tested this on the Cortex M architecture.

[gmarull]

Would really like to have this feature, STM32/GD32 now need to rely on hacks to use shared IRQs. What is the reason for Allow up to three ISRs to share one interrupt line?

[sambhurst]

Would really like to have this feature, STM32/GD32 now need to rely on hacks to use shared IRQs. What is the reason for Allow up to three ISRs to share one interrupt line?

I was thinking that calling too many functions in an ISR would bog down the system and I haven't seen very many MCUs that have more than three peripherals sharing one interrupt line but the short answer is, the number was chosen arbitrarily.

[str4t0m]

I like that approach of calling all the registered handlers from the isr in the shared_isr section, without requiring a separate driver.
In the case of STM32G0 quite a lot interrupts can overlap(UART, EXTI, DMA) so I wouldn't limit it to 3.

Do you plan to open a PR implementing this proposal or would you rather gather more feedback before.

[sambhurst]

I like that approach of calling all the registered handlers from the isr in the shared_isr section, without requiring a separate driver. In the case of STM32G0 quite a lot interrupts can overlap(UART, EXTI, DMA) so I wouldn't limit it to 3.

Do you plan to open a PR implementing this proposal or would you rather gather more feedback before.

I think the only issue is the number of shared interrupts allowed. I'll bump it up to 4 and open a PR. I thought about using a Kconfig but I didn't know how to pass the option to the python script.

[galak]

How does this differ from what is done in drivers/interrupt_controller/Kconfig.shared_irq & drivers/interrupt_controller/intc_shared_irq.c?

[sambhurst]

How does this differ from what is done in drivers/interrupt_controller/Kconfig.shared_irq & drivers/interrupt_controller/intc_shared_irq.c?

To be honest, I was unaware of this implementation and it seems that no projects are using it, other than drivers/ethernet/eth_smsc911x.c including the header. The shared_irq implementation doesn't seem very compatible with the current drivers and would require some code changes.

This pull request is more efficient when calling the shared ISR handlers.

[aurel32]

How does this differ from what is done in drivers/interrupt_controller/Kconfig.shared_irq & drivers/interrupt_controller/intc_shared_irq.c?

I have tried this approach in the past. The int_shared_irq controller is based on multilevel interrupts, which are not supported on aarch32.

[sambhurst]

I've received some useful comments on the PR but I think the discussion should move back to the issue. In the PR, I've implemented two different versions that each have pros and cons. I'd like to list them here so we can pick one.

1) Only ISRs created with IRQ_DIRECT_CONNECT can be shared.
Pros: Very easy to implement, no C or ASM code needs to change, and any number of shared ISRs.
Cons: No arguments can be passed to the ISRs.
Example:
Generated isr_tables.c looks as follows:

/* Shared Prototypes */
void attribute((section(".shared_isr")))shared_isr_20(void)

/* IRQ_DIRECT_CONNECT table */
uintptr_t __irq_vector_table _irq_vector_table[102] = {
...
((uintptr_t)&shared_isr_20),
....
};

/* IRQ_CONNECT table */
struct _isr_table_entry __sw_isr_table _sw_isr_table[102] = {
...
};

void attribute((section(".shared_isr")))shared_isr_20(void){
((DIRECT_ISR)0x8000549)();
((DIRECT_ISR)0x8000525)();
}

2) Allow ISRs created with IRQ_CONNECT to share IRQ lines.
Pros: Arguments can be passed to ISRs.
Cons: Not so easy to implement, C and ASM code needs to change, Flash memory will be wasted on unused shared ISR place holders. See below:
Example:
Generated isr_tables.c looks as follows:

/* IRQ_DIRECT_CONNECT table */
uintptr_t __irq_vector_table _irq_vector_table[102] = {
...
};

/* IRQ_CONNECT table */
struct _isr_table_entry __sw_isr_table _sw_isr_table[102][SOME_NUMBER_OF_SHARED_ISRS] = {
...
{ {(const void *)0x8009824, ((ISR)0x8000549)}, {(const void *)0x8009734, ((ISR)0x8000525)}, {0,0} },
...
};

Each entry in the __sw_isr_table is a NULL terminated list of shared interrupts with corresponding arguments. The
ASM code needs to iterate through the list, calling each ISR in turn, until the NULL terminator is reached. Or the
__isr_table_entry structure can be changed to include the number of shared ISRs to iterate over. This approach
wastes memory because this is a 2 dimensional array and not all entries will be used.

Or there is another way that I've yet to consider.

[gmarull]

I haven't gone into much detail on how ISR tables are generated, so please ignore the following if it doesn't make sense. In case of shared IRQs, would it be possible to autogenerate a function stub that calls the N attached routines with the right parameter? Then the table would just point to such function stub. Non-shared IRQs would just call the original function.

[sambhurst]

I haven't gone into much detail on how ISR tables are generated, so please ignore the following if it doesn't make sense. In case of shared IRQs, would it be possible to autogenerate a function stub that calls the N attached routines with the right parameter? Then the table would just point to such function stub. Non-shared IRQs would just call the original function.

@gmarull
That makes sense and was one of the approaches I tried. I stored the arguments in flash but then I needed to rewrite the arm/core/aarch32/isr_wrapper.S to access the arguments and call the ISRs. I thought this change would be to invasive and opted for the two approaches listed above. But maybe I should have listed this one too.

[sambhurst]

@gmarull
After second thought, maybe I can store the arguments contiguously in flash and use the isr_wrapper.S to call the stub ISR along with a pointer to the list of arguments. Then the stub ISR can pass the appropriate arguments to the N attached routines.

[henrikbrixandersen]

Zephyr now supports shared interrupts on the architecture level: #61422