Zephyr Shared ISRs (alternative) #57146

aurel32 · 2023-04-21T19:16:14Z

This is a continuation from #49427 and should be seen as an alternative proof of concept. The goal is to see if the strategy is working, and check if CI is happy. The details of naming and moving code can be discussed in a second time if this alternative solution is acceptable.

The first two commits are from the original PR, reusing the great of work of @sambhurst. The third commit changes the per shared interrupt handler to a common interrupt handler which is fixed and thus already present in the first linking stage. It is marked as KEEP in the linker file as it is not used before the second stage. It reads both argument and function from a per interrupt table, with a NULL pointer function to mark the end of the list. I reused the existing _isr_table_entry struct, but that can be changed later if it proves to be a bad idea.

So far I have only tested that on Cortex-M CPU.

Adds a feature to gen_isr_tables.py that enables it to detect and generate shared IRQs when IRQ_CONNECT is used. Signed-off-by: Sam Hurst <[email protected]>

Add test case of shared interrupts. This tests that IRQs are shared when IRQ_CONNECT is called multiple times on the same IRQ with different ISR handlers. Signed-off-by: Sam Hurst <[email protected]>

aurel32 · 2023-04-21T20:39:47Z

It appears that the tests now pass on all the enabled platforms, that is:
qemu_cortex_m0 qemu_cortex_m3 qemu_cortex_a9 qemu_cortex_a53 qemu_cortex_a53_smp

cmake/linker_script/arm/linker.cmake

include/zephyr/arch/arm/aarch32/cortex_a_r/scripts/linker.ld

scripts/build/gen_isr_tables.py

tests/kernel/interrupt/testcase.yaml

aurel32 · 2023-04-23T17:20:26Z

@carlocaione Thanks for the review. Do you think this PR is acceptable with the issues your reported fixed? I don't plan to spend time on it if it is likely going to be rejected like the original PR.

carlocaione · 2023-04-24T06:54:31Z

@carlocaione Thanks for the review. Do you think this PR is acceptable with the issues your reported fixed? I don't plan to spend time on it if it is likely going to be rejected like the original PR.

Yes, but other reviewers will chime in for sure. So please, rework the commits and push this out of draft state.

aurel32 · 2023-04-25T19:30:12Z

I have just pushed a new version with the comments from @carlocaione fixed. I also simplified a bit the gen_isr_tables.py script now that only irq data need to be generated. Finally I dropped the .shared_irq_arg section, as it is now just fine to have the corresponding data in the rodata section.

The strategy is to not generate code depending on the enabled shared interrupts, but only data. For that a generic shared_irq_handler is added and called with a list of functions and args to be called, ended with 0. Signed-off-by: Aurelien Jarno <[email protected]>

aurel32 · 2023-04-25T19:49:22Z

The compliance checks should be fixed by this PR: #57261

andyross

Refresh +1 from the last time this came around. This seems very reasonable to me. One nitpick in the test code.

And I'll repeat that I still think it would be simpler to use a linked-list or array representation for the ISRs at build time (even though it adds a few cycles to the case where there is only one ISR registered) instead of doing all the work here to create a special "shared" record.

andyross · 2023-04-25T19:59:30Z

tests/kernel/interrupt/src/shared_irq.c

+	k_busy_wait(MS_TO_US(DURATION));
+
+	for (int i = 0; i < MAX_ISR_TESTS; i++) {
+		zassert_true(test_flag[i] == i + 1, "Test flag not set by ISR%d\n", i);


Seems like you want to have the test block after every interrupt, and clear the flags between them. It's important to verify that the right ISRs run, it's equally important to ensure that the wrong ones don't!

carlocaione

The third commit is fixing something introduced by the first one. Squash them.

carlocaione · 2023-04-26T07:38:39Z

arch/Kconfig

@@ -416,6 +419,13 @@ config GEN_IRQ_VECTOR_TABLE
 	  indexed by IRQ line. In the latter case, the vector table must be
 	  supplied by the application or architecture code.

+config ISR_SHARE_IRQ
+	bool "Allow IRQs to share multiple ISRs"
+	depends on GEN_ISR_TABLES


also depends on ARCH_HAS_SW_ISR_TABLE_SHARED_IRQ

carlocaione · 2023-05-23T10:12:22Z

@aurel32 do you plan to keep working on this?

Cherish-Gww · 2023-06-25T08:39:04Z

@aurel32
Looking forward to your PR merge.
I want use this patch on risc-v platform.

github-actions · 2023-08-25T00:29:34Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

sambhurst added 2 commits April 20, 2023 21:58

scripts: Enable gen_isr_tables.py to generate shared IRQs

fed423c

Adds a feature to gen_isr_tables.py that enables it to detect and generate shared IRQs when IRQ_CONNECT is used. Signed-off-by: Sam Hurst <[email protected]>

tests: interrupt: Add tests for shared irqs

aece3f2

Add test case of shared interrupts. This tests that IRQs are shared when IRQ_CONNECT is called multiple times on the same IRQ with different ISR handlers. Signed-off-by: Sam Hurst <[email protected]>

aurel32 requested review from dcpleung, andyross, nashif, julius-barendt, carlocaione, stephanosio, MaureenHelm, galak, ioannisg and tejlmand as code owners April 21, 2023 19:16

zephyrbot added area: Build System area: Kernel area: SPARC SPARC Architecture area: ARM ARM (32-bit) Architecture area: ARM64 ARM (64-bit) Architecture area: Architectures labels Apr 21, 2023

zephyrbot requested review from ceolin, jeremybettis, microbuilder, nordicjm and peter-mitsis April 21, 2023 19:16

zephyrbot assigned andyross Apr 21, 2023

aurel32 marked this pull request as draft April 21, 2023 19:16

aurel32 force-pushed the shared_irq branch 2 times, most recently from 6116977 to 9c0c372 Compare April 21, 2023 20:58

carlocaione reviewed Apr 23, 2023

View reviewed changes

aurel32 force-pushed the shared_irq branch from 9c0c372 to 6ae449f Compare April 25, 2023 19:28

aurel32 changed the title ~~RFC/WIP: Zephyr Shared ISRs (alternative)~~ Zephyr Shared ISRs (alternative) Apr 25, 2023

aurel32 marked this pull request as ready for review April 25, 2023 19:30

aurel32 force-pushed the shared_irq branch from 6ae449f to 1c73afa Compare April 25, 2023 19:32

andyross approved these changes Apr 25, 2023

View reviewed changes

carlocaione requested changes Apr 26, 2023

View reviewed changes

aurel32 mentioned this pull request Apr 26, 2023

RFC: Introduce a STM32 SYSCFG interrupt controller driver #57310

Closed

This was referenced Aug 10, 2023

Add architectural support for shared interrupts #61331

Closed

Add architectural support for shared interrupts (take 2) #61422

Merged

soburi mentioned this pull request Aug 21, 2023

rpi_pico: Added Generic PIO support and a PIO based UART driver #56678

Closed

github-actions bot added the Stale label Aug 25, 2023

github-actions bot closed this Sep 9, 2023

aurel32 deleted the shared_irq branch April 28, 2024 17:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Zephyr Shared ISRs (alternative) #57146

Zephyr Shared ISRs (alternative) #57146

aurel32 commented Apr 21, 2023

aurel32 commented Apr 21, 2023

aurel32 commented Apr 23, 2023

carlocaione commented Apr 24, 2023

aurel32 commented Apr 25, 2023 •

edited

Loading

aurel32 commented Apr 25, 2023

andyross left a comment

andyross Apr 25, 2023

carlocaione left a comment

carlocaione Apr 26, 2023

carlocaione commented May 23, 2023

Cherish-Gww commented Jun 25, 2023 •

edited

Loading

github-actions bot commented Aug 25, 2023

Zephyr Shared ISRs (alternative) #57146

Zephyr Shared ISRs (alternative) #57146

Conversation

aurel32 commented Apr 21, 2023

aurel32 commented Apr 21, 2023

aurel32 commented Apr 23, 2023

carlocaione commented Apr 24, 2023

aurel32 commented Apr 25, 2023 • edited Loading

aurel32 commented Apr 25, 2023

andyross left a comment

Choose a reason for hiding this comment

andyross Apr 25, 2023

Choose a reason for hiding this comment

carlocaione left a comment

Choose a reason for hiding this comment

carlocaione Apr 26, 2023

Choose a reason for hiding this comment

carlocaione commented May 23, 2023

Cherish-Gww commented Jun 25, 2023 • edited Loading

github-actions bot commented Aug 25, 2023

aurel32 commented Apr 25, 2023 •

edited

Loading

Cherish-Gww commented Jun 25, 2023 •

edited

Loading