Skip to content

fix(riscv/irqc): align IMSIC handler to AIA specifications #209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

filippofontana
Copy link
Contributor

"The RISC-V Advanced Interrupt Architecture" specifications explain in Chapter 3 (pages 26-27) how interrupts should be handled with IMSIC, in particular, interrupt claiming has to be done right after reading the STOPEI register or in the same read instruction with the csrrw instruction.

This commit aligns the IMSIC interrupt handler to that specs. Without this change, if an interrupt with higher priority becomes pending while handling a lower one, writing to STOPEI after interrupts_handle() executes will claim the higher IRQ, freezing the execution.

@filippofontana filippofontana force-pushed the fix-imsic-handler branch 2 times, most recently from 9dc1917 to cc18264 Compare March 31, 2025 16:28
@filippofontana
Copy link
Contributor Author

Had to force push because of the failing format check, I wanted to keep the change small but clang-format doesn't like it 🥲.

@josecm
Copy link
Member

josecm commented Apr 4, 2025

Thanks again for fixing this @filippofontana!

Copy link
Member

@josecm josecm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd just maybe ask you to split the PR in two commits. One for the new csrs accessors the other for the interrupt handle fix.

@josecm josecm self-assigned this Apr 4, 2025
@filippofontana
Copy link
Contributor Author

Ok, sure!

This commit adds a "logical" swap operation to the CSR registers
accessors. It is implemented with the `csrrw` instruction under the hood.

Signed-off-by: Filippo Fontana <[email protected]>
"The RISC-V Advanced Interrupt Architecture" specifications explain
in Chapter 3 (pages 26-27) how interrupts should be handled with IMSIC,
in particular, interrupt claiming has to be done right after reading the
STOPEI register or in the same read instruction with the csrrw
instruction.

This commit aligns the IMSIC interrupt handler to that specs.
Without this change, if an interrupt with higher priority becomes
pending while handling a lower one, writing to STOPEI after
`interrupts_handle()` executes will claim the higher IRQ, freezing
the execution.

Signed-off-by: Filippo Fontana <[email protected]>
@filippofontana filippofontana force-pushed the fix-imsic-handler branch 2 times, most recently from 3cf36d6 to a6c0c1b Compare April 4, 2025 10:30
@filippofontana filippofontana requested a review from josecm April 11, 2025 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants