riscv icon indicating copy to clipboard operation
riscv copied to clipboard

`riscv-rt`: Support for vectored mode interrupt handling

Open romancardenas opened this issue 1 year ago • 8 comments

I am working on providing support for interrupt dispatching in vectored mode. This is a first draft, and I could not test yet if it works. However, I wanted to share it with you so I can start getting some feedback and suggestions about different design decisions. So, the main changes are:

  • New v-trap feature to enable vectored mode. If this feature is enabled, the assembly code will add a trap vector with j instructions to a different trap per interrupt source. Some targets do not implement the standard interrupt convention (e.g., ESP32C3). That is why I left the table as weak. PACs will be able to replace this table accordingly.
  • New interrupt procedural macro to implement interrupt handlers. Currently, in direct mode, this macro is quite dummy. However, I plan to add some parameters to the macro that will, eventually, allow us to "check" at compile time that the interrupt source name is an interrupt source of the target. Additionally, if v-trap feature is enabled, this macro will generate the trap handler to make sure that user registers are properly restored after executing the interrupt handler.

Also, I've been careful regarding how traps are generated to ease shortly the compatibility with the E extension (see #178).

This PR is still WIP, and I appreciate the feedback. Especially from the esp-rs folks (@MabezDev , @jessebraham) and @hegza , who already implemented a solution for this.

Closes #158

romancardenas avatar Apr 11 '24 09:04 romancardenas

For sure I can at least test this on our HW & sims, compare it with mine and see how it fits into the current workflow. I'll be onto this.

hegza avatar Apr 11 '24 13:04 hegza

I successfully ran an example on QEMU using MachineTimer and MachineSoft interrupts in vectored mode.

For each interrupt source, an independent trap handler is added in the final binary:

20010150 <_start_MachineSoft_trap>:
20010150:      	addi	sp, sp, -64                           // allocate space for trap frame
20010152:      	sw	a0, 32(sp)                            // store trap partially (only register a0)
20010154:      	auipc	a0, 1
20010158:      	addi	a0, a0, -476                          // load MachineSoft address into a0
2001015c:      	j	0x20010210 <_continue_interrupt_trap> // jump to common interrupt handler

20010160 <_start_MachineTimer_trap>:
20010160:      	addi	sp, sp, -64                           // allocate space for trap frame
20010162:      	sw	a0, 32(sp)                            // store trap partially (only register a0)
20010164:      	auipc	a0, 1
20010168:      	addi	a0, a0, 1250                          // load MachineTimer address into a0
2001016c:      	j	0x20010210 <_continue_interrupt_trap> // jump to common interrupt handler

Additionally, a common interrupt trap does the rest of the work and jumps to the corresponding handler:

20010210 <_continue_interrupt_trap>:
// store all the registers of the trap excluding a0 (which points to interrupt handler)
20010210:      	sw	ra, 0(sp)
20010214:      	sw	t0, 4(sp)
20010218:      	sw	t1, 8(sp)
2001021c:      	sw	t2, 12(sp)
20010220:      	sw	t3, 16(sp)
20010224:      	sw	t4, 20(sp)
20010228:      	sw	t5, 24(sp)
2001022c:      	sw	t6, 28(sp)
20010230:      	sw	a1, 36(sp)  // skip a0, already stored
20010234:      	sw	a2, 40(sp)
20010238:      	sw	a3, 44(sp)
2001023c:      	sw	a4, 48(sp)
20010240:      	sw	a5, 52(sp)
20010244:      	sw	a6, 56(sp)
20010248:      	sw	a7, 60(sp)
2001024c:      	jalr	a0           // jump to interrupt handler in a0
// load registers from trap
20010250:      	lw	ra, 0(sp)
20010254:      	lw	t0, 4(sp)
20010258:      	lw	t1, 8(sp)
2001025c:      	lw	t2, 12(sp)
20010260:      	lw	t3, 16(sp)
20010264:      	lw	t4, 20(sp)
20010268:      	lw	t5, 24(sp)
2001026c:      	lw	t6, 28(sp)
20010270:      	lw	a0, 32(sp) // we now include a0
20010274:      	lw	a1, 36(sp)
20010278:      	lw	a2, 40(sp)
2001027c:      	lw	a3, 44(sp)
20010280:      	lw	a4, 48(sp)
20010284:      	lw	a5, 52(sp)
20010288:      	lw	a6, 56(sp)
2001028c:      	lw	a7, 60(sp)
20010290:      	addi	sp, sp, 64 // free stack and return from interrupt
20010294:      	mret

We also preserve the original _start_trap, as it is the default implementation for all those interrupts that are not defined using the macro. Also, it is required for dealing with exceptions:

20010200 <_start_trap>:
20010200:      	addi	sp, sp, -64
20010204:      	sw	ra, 0(sp)
20010208:      	sw	t0, 4(sp)
2001020c:      	sw	t1, 8(sp)
20010210:      	sw	t2, 12(sp)
20010214:      	sw	t3, 16(sp)
20010218:      	sw	t4, 20(sp)
2001021c:      	sw	t5, 24(sp)
20010220:      	sw	t6, 28(sp)
20010224:      	sw	a0, 32(sp)
20010228:      	sw	a1, 36(sp)
2001022c:      	sw	a2, 40(sp)
20010230:      	sw	a3, 44(sp)
20010234:      	sw	a4, 48(sp)
20010238:      	sw	a5, 52(sp)
2001023c:      	sw	a6, 56(sp)
20010240:      	sw	a7, 60(sp)
20010244:      	add	a0, sp, zero // store pointer to trap in a0
20010248:      	jal	0x2001042a <_start_trap_rust>
2001024c:      	lw	ra, 0(sp)
20010250:      	lw	t0, 4(sp)
20010254:      	lw	t1, 8(sp)
20010258:      	lw	t2, 12(sp)
2001025c:      	lw	t3, 16(sp)
20010260:      	lw	t4, 20(sp)
20010264:      	lw	t5, 24(sp)
20010268:      	lw	t6, 28(sp)
2001026c:      	lw	a0, 32(sp)
20010270:      	lw	a1, 36(sp)
20010274:      	lw	a2, 40(sp)
20010278:      	lw	a3, 44(sp)
2001027c:      	lw	a4, 48(sp)
20010280:      	lw	a5, 52(sp)
20010284:      	lw	a6, 56(sp)
20010288:      	lw	a7, 60(sp)
2001028c:      	addi	sp, sp, 64
20010290:      	mret

Next, the vector table is filled as expected:

20010300 <_vector_table>:
20010300:      	j	0x20010200 <_start_trap>
20010304:      	j	0x20010200 <_start_trap>
20010308:      	j	0x20010200 <_start_trap>
2001030c:      	j	0x20010150 <_start_MachineSoft_trap>
20010310:      	j	0x20010200 <_start_trap>
20010314:      	j	0x20010200 <_start_trap>
20010318:      	j	0x20010200 <_start_trap>
2001031c:      	j	0x200101a0 <_start_MachineTimer_trap>
20010320:      	j	0x20010200 <_start_trap>
20010324:      	j	0x20010200 <_start_trap>
20010328:      	j	0x20010200 <_start_trap>
2001032c:      	j	0x20010200 <_start_trap>

I used GDB to monitor which trap handler is used and works as expected. The program does not use the _start_trap handler at all :D

romancardenas avatar Apr 12 '24 09:04 romancardenas

The proc-macro trap handler is somewhat more intimidating than the previous trap_handler macro. I guess it becomes worth it as the vectored handler needs a more customizable code flow anyway.

Looks OK other than that. I'll try out the user experience on a sim.

hegza avatar Apr 19 '24 09:04 hegza

Works as expected on Renode simulated generic 64-bit RISC-V using a CLINT for interrupt dispatch. I tested MachineSoft & MachineTimer as well. GDB shows the same vectoring behavior. Renode VP example for reference: https://github.com/soc-hub-fi/headsail-vp/compare/main...tmp/vectored-rt

I wonder how it meshes with the nested interrupts we use on the FPGA implementation of the real-time RVE. I'll try adapting that runtime to this one and see if it raises any issue.

hegza avatar Apr 19 '24 11:04 hegza

I tested this with my SLIC crate, which exploits nested MachineSoft interrupts to have an interrupt-driven program (e.g., RTIC). So hopefully you will face no issues.

romancardenas avatar Apr 19 '24 13:04 romancardenas

TO DO LIST

  • [x] I would love to skip _start_trap at all and leave it only for exceptions. We could use the DefaultHandler to do this: https://github.com/rust-embedded/riscv/blob/aca641040c5bd3ef856bee94f2db0e235d6bb317/riscv-rt/src/lib.rs#L478)
  • [ ] use cargo flags to simplify _start_rust_trap if vectored mode is enabled? maybe the array of __INTERRUPTS is not necessary in this mode, as only exceptions are expected to fall here.

Next, another challenge is supporting heterogeneous interrupt sources (e.g., target-specific additional interrupt sources or non-standard interrupt sources. In order to achieve this, I would do as follows:

  • [ ] Modify _start_rust_trap. When an interrupt is detected, it relies on _handle_interrupts function, which, by default, behaves as this part.
  • [ ] Provide a new macro to 1) define core interrupt enums 2) define the appropriate _vector_table

Essentially, with these changes, we would start moving towards supporting target-specific interrupt sources (see #146)

romancardenas avatar Apr 19 '24 14:04 romancardenas

New changes to riscv-rt:

  • Now, when an interrupt is detected, _start_trap_rust calls _dispatch_interrupt(interrupt_number). This does not really change anything from previous versions, but we are now closer to accepting custom interrupt sources (I will leave this to a future PR, it needs additional changes).
  • When v-trap feature is enabled, the binary will contain a _vector_table with j instructions to trap handlers. The first j instruction points to _start_trap, as it is reserved to exceptions. The rest points to _start_INTERRUPT_trap, where INTERRUPT is the ID of the corresponding interrupt name (e.g., MachineSoft). By default, all _start_INTERRUPT_trap are weak aliases of _start_DefaultHandler_trap.
  • Interrupt handlers are divided into two stages:
    • _start_INTERRUPT_trap: saves space in stack, stores a0 register, loads the interrupt handler in a0, and jumps to _continue_interrupt_trap.
    • _continue_interrupt_trap: saves the rest of the registers, calls the interrupt handler stored in a0, restores all registers, and returns.
  • To help users to define interrupt traps together with their interrupt handler, the new riscv_rt::interrupt macro also generates the _start_INTERRUPT_trap routine.

romancardenas avatar May 04 '24 16:05 romancardenas

The PR is good to go. @rust-embedded/riscv please take a look.

romancardenas avatar May 04 '24 16:05 romancardenas

Sorry for not taking a look at this sooner. I will hopefully have some time to review at the weekend :)

MabezDev avatar May 09 '24 11:05 MabezDev