Skip to content
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

Nested interrupt test is broken for RISC-V #23593

Closed
stephanosio opened this issue Mar 19, 2020 · 5 comments · Fixed by #23614
Closed

Nested interrupt test is broken for RISC-V #23593

stephanosio opened this issue Mar 19, 2020 · 5 comments · Fixed by #23614
Labels
area: Kernel area: RISCV RISCV Architecture (32-bit & 64-bit) area: Tests Issues related to a particular existing or missing test bug The issue is a bug, or the PR is fixing a bug

Comments

@stephanosio
Copy link
Member

Describe the bug
The test_nested_isr test in tests/kernel/interrupt does not execute the designated interrupt service routine on qemu_riscv32 and qemu_risv64.

To Reproduce
Run the aforementioned test on qemu_riscv32 or qemu_riscv64

Expected behavior
We should see the messages printed by the ISRs, like below (from qemu_cortex_m3):

starting test - test_nested_isr
isr0 running !!
Triggering irq : 41
isr1 ran !!
isr0 execution completed !!
Triggering irq : 42
isr0 running !!
isr0 execution completed !!
PASS - test_nested_isr

Screenshots or console output
The following output from qemu_riscv32 suggests that the designated ISRs are not being executed at all.

starting test - test_nested_isr
Triggering irq : 62
PASS - test_nested_isr

Environment (please complete the following information):

  • OS: Ubuntu 18.04
  • Toolchain: Zephyr SDK 0.11.2
  • Commit SHA: c0f7a1a

Additional context
The test should not succeed, but it does due to a bad design; I am planning to rework this test.

@stephanosio stephanosio added bug The issue is a bug, or the PR is fixing a bug area: Kernel area: RISCV RISCV Architecture (32-bit & 64-bit) area: Tests Issues related to a particular existing or missing test labels Mar 19, 2020
@stephanosio
Copy link
Member Author

I did a quick analysis on this and the following is what I found:

  1. qemu_riscv32 and qemu_riscv64 emulate the SiFive FE310 SoC.
  2. The current RISC-V trigger_irq implementation writes to the MIP register supposedly to trigger software interrupts. While setting some interrupt bits in this register will trigger the corresponding interrupts according to the RISC-V arch spec, the SiFive FE310 SoC does not implement this -- according to the SoC docs, this register is entirely read-only and the directly trigger-able interrupts are not present/supported.
  3. Only one software generated interrupt is supported on the SiFive FE310 -- machine software interrupt (MSI), which must be triggered through the CLINT MSIP register.

@stephanosio
Copy link
Member Author

stephanosio commented Mar 20, 2020

It seems the RISC-V arch port (in particular, arch/riscv/core/isr.S) does not properly support interrupt nesting, and the interrupt nesting/preemption support on the RISC-V architecture seems to be a rather messy business.

After experimenting with the Machine Timer Interrupt (MTI) and Machine Software Interrupt (MSI) -- triggering MSI from the MTI ISR, I can confirm that preemption is not happening.

At this point, this is not simply that the test is broken; interrupt nesting is simply not supported on the current RISC-V arch port.

Could RISC-V arch maintainers confirm/comment? (cc @npitre @kgugala @pgielda @nategraff-sifive)

stephanosio added a commit to stephanosio/zephyr that referenced this issue Mar 20, 2020
This commit disables the nested interrupt test for the RISC-V platform,
as interrupt nesting is not supported on the current RISV-C
architecture port.

Furthermore, the current `trigger_irq` implementation for RISC-V is
mostly incorrect and cannot be used, so there is no point in leaving
that in the codebase (see zephyrproject-rtos#23593).

Signed-off-by: Stephanos Ioannidis <root@stephanos.io>
nashif pushed a commit that referenced this issue Mar 27, 2020
This commit disables the nested interrupt test for the RISC-V platform,
as interrupt nesting is not supported on the current RISV-C
architecture port.

Furthermore, the current `trigger_irq` implementation for RISC-V is
mostly incorrect and cannot be used, so there is no point in leaving
that in the codebase (see #23593).

Signed-off-by: Stephanos Ioannidis <root@stephanos.io>
@karstenkoenig
Copy link
Collaborator

Is just disabling the test the resolution? Does the RISC-V port not support interrupt nesting at all or might weird things happen if an interrupt happens during an interrupt?

@stephanosio
Copy link
Member Author

Is just disabling the test the resolution? Does the RISC-V port not support interrupt nesting at all or might weird things happen if an interrupt happens during an interrupt?

@karstenkoenig #23593 (comment)

As far as I can see, the RISC-V arch port does not support interrupt nesting and, even if it does, there is currently no viable way of testing it in software because CLINT does not support priority-based nested interrupts and PLIC does not support generating interrupts from software (and that is pretty much all we have in terms of interrupt handling in qemu_riscv{32,64}).

@karstenkoenig
Copy link
Collaborator

Ok, thanks for clearing it up for me!

hakehuang pushed a commit to hakehuang/zephyr that referenced this issue Apr 30, 2020
This commit disables the nested interrupt test for the RISC-V platform,
as interrupt nesting is not supported on the current RISV-C
architecture port.

Furthermore, the current `trigger_irq` implementation for RISC-V is
mostly incorrect and cannot be used, so there is no point in leaving
that in the codebase (see zephyrproject-rtos#23593).

Signed-off-by: Stephanos Ioannidis <root@stephanos.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Kernel area: RISCV RISCV Architecture (32-bit & 64-bit) area: Tests Issues related to a particular existing or missing test bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants