Closed romancardenas closed 3 months ago
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.
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
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.
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.
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.
TO DO LIST
_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)_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:
_start_rust_trap
. When an interrupt is detected, it relies on _handle_interrupts
function, which, by default, behaves as this part._vector_table
Essentially, with these changes, we would start moving towards supporting target-specific interrupt sources (see #146)
New changes to riscv-rt
:
_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).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
._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.riscv_rt::interrupt
macro also generates the _start_INTERRUPT_trap
routine.The PR is good to go. @rust-embedded/riscv please take a look.
Sorry for not taking a look at this sooner. I will hopefully have some time to review at the weekend :)
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:
v-trap
feature to enable vectored mode. If this feature is enabled, the assembly code will add a trap vector withj
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.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, ifv-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