re-building esp-riscv-rt on top of riscv-rt 0.13.0

[romancardenas]

One of the main fresh features of riscv-rt 0.13.0 is that, together with riscv 0.12.0, now it should be possible to support targets that do not completely fulfill the RISC-V interrupt standard (e.g., ESP32-C3 and C6).

I think it would be very interesting to adopt this new feature with esp-riscv-rt. It would help the RISC-V team to detect some deficiencies that still need our attention to support a wider range of targets. Additionally, new features in riscv-rt would be easily adopted in ESP32-Cx targets.

Let me know what you think, and if you are interested in exploring this option!

[jessebraham]

Thank you for opening this issue! I've been meaning to investigate better utilizing the packages provided by rust-embedded for RISC-V support, I just unfortunately have not yet gotten around to this.

So I am definitely interested in exploring this option, and will try to make some time for it soon :)

[bjoernQ]

The only problem currently might be that we need to store/restore the full context (not only caller saved registers) as the trap-frame since that is what the scheduler in esp-wifi is using - I wanted to explore to change the way the scheduler works anyways so this might be a good reason to look into that

[romancardenas]

We can also add a feature flag to modify the trap frame in riscv-rt. I had in mind these kinds of potential issues while re-designing riscv-rt. For example, RISC-V E targets will need also a special version of the trap frame. It should not be too difficult to implement this (I hope).

Let me know what you think and if I can help you with anything.

[bjoernQ]

I thought about this a bit more and I realized we actually added a bit more functionality over time. Maybe the functionality is already available in riscv-rt (need to check)

• (already mentioned) there is the problem that we currently need the full context, not just the caller-saved registers in the trap-frame - that's only an issue for esp-wifi and I think it would be good if we wouldn't require that (i.e. rewrite the scheduler in esp-wifi .... I think that is a viable option but I haven't tried it)

  esp-hal/esp-riscv-rt/src/lib.rs

  Lines 76 to 161 in ccb3a1b

  	pub struct TrapFrame {
  	/// Return address, stores the address to return to after a function call or
  	/// interrupt.
  	pub ra: usize,
  	/// Temporary register t0, used for intermediate values.
  	pub t0: usize,
  	/// Temporary register t1, used for intermediate values.
  	pub t1: usize,
  	/// Temporary register t2, used for intermediate values.
  	pub t2: usize,
  	/// Temporary register t3, used for intermediate values.
  	pub t3: usize,
  	/// Temporary register t4, used for intermediate values.
  	pub t4: usize,
  	/// Temporary register t5, used for intermediate values.
  	pub t5: usize,
  	/// Temporary register t6, used for intermediate values.
  	pub t6: usize,
  	/// Argument register a0, typically used to pass the first argument to a
  	/// function.
  	pub a0: usize,
  	/// Argument register a1, typically used to pass the second argument to a
  	/// function.
  	pub a1: usize,
  	/// Argument register a2, typically used to pass the third argument to a
  	/// function.
  	pub a2: usize,
  	/// Argument register a3, typically used to pass the fourth argument to a
  	/// function.
  	pub a3: usize,
  	/// Argument register a4, typically used to pass the fifth argument to a
  	/// function.
  	pub a4: usize,
  	/// Argument register a5, typically used to pass the sixth argument to a
  	/// function.
  	pub a5: usize,
  	/// Argument register a6, typically used to pass the seventh argument to a
  	/// function.
  	pub a6: usize,
  	/// Argument register a7, typically used to pass the eighth argument to a
  	/// function.
  	pub a7: usize,
  	/// Saved register s0, used to hold values across function calls.
  	pub s0: usize,
  	/// Saved register s1, used to hold values across function calls.
  	pub s1: usize,
  	/// Saved register s2, used to hold values across function calls.
  	pub s2: usize,
  	/// Saved register s3, used to hold values across function calls.
  	pub s3: usize,
  	/// Saved register s4, used to hold values across function calls.
  	pub s4: usize,
  	/// Saved register s5, used to hold values across function calls.
  	pub s5: usize,
  	/// Saved register s6, used to hold values across function calls.
  	pub s6: usize,
  	/// Saved register s7, used to hold values across function calls.
  	pub s7: usize,
  	/// Saved register s8, used to hold values across function calls.
  	pub s8: usize,
  	/// Saved register s9, used to hold values across function calls.
  	pub s9: usize,
  	/// Saved register s10, used to hold values across function calls.
  	pub s10: usize,
  	/// Saved register s11, used to hold values across function calls.
  	pub s11: usize,
  	/// Global pointer register, holds the address of the global data area.
  	pub gp: usize,
  	/// Thread pointer register, holds the address of the thread-local storage
  	/// area.
  	pub tp: usize,
  	/// Stack pointer register, holds the address of the top of the stack.
  	pub sp: usize,
  	/// Program counter, stores the address of the next instruction to be
  	/// executed.
  	pub pc: usize,
  	/// Machine status register, holds the current status of the processor,
  	/// including interrupt enable bits and privilege mode.
  	pub mstatus: usize,
  	/// Machine cause register, contains the reason for the trap (e.g.,
  	/// exception or interrupt number).
  	pub mcause: usize,
  	/// Machine trap value register, contains additional information about the
  	/// trap (e.g., faulting address).
  	pub mtval: usize,
  	}

  but because of our interrupt-nesting we most probably at least need MEPC (and other CSRs?) in addition to the caller-saved-registers
• our crate allows nested interrupts

  esp-hal/esp-riscv-rt/src/lib.rs

  Lines 723 to 730 in ccb3a1b

  	r#"
  	addi sp, sp, -16 #build stack
  	sw ra, 0(sp)
  	jal ra, _handle_priority
  	lw ra, 0(sp)
  	sw a0, 0(sp) #reuse old stack, a0 is return of _handle_priority
  	addi a0, sp, 16 #the proper stack pointer is an argument to the HAL handler
  	"#,

  and

  esp-hal/esp-riscv-rt/src/lib.rs

  Lines 735 to 740 in ccb3a1b

  	// restore threshold
  	r#"
  	lw a0, 0(sp) #load stored priority
  	jal ra, _restore_priority
  	addi sp, sp, 16 #pop
  	"#,

  ... the real work is done by the HAL however - but the rt-crate calls into the HAL for that
• we can optionally make the SP point to valid RAM (used for flip-link, we (ab)use exception 14 for that):

  esp-hal/esp-riscv-rt/src/lib.rs

  Lines 489 to 514 in ccb3a1b

  	#[cfg(feature="fix-sp")]
  	r#"
  	// move SP to some save place if it's pointing below the RAM
  	// otherwise we won't be able to do anything reasonable
  	// (since we don't have a working stack)
  	//
  	// most probably we will just print something and halt in this case
  	// we actually can't do anything else
  	csrw mscratch, t0
  	la t0, _stack_end
  	bge sp, t0, 1f
  	// use the reserved exception cause 14 to signal we detected a stack overflow
  	li t0, 14
  	csrw mcause, t0
  	// set SP to the start of the stack
  	la sp, _stack_start
  	li t0, 4 // make sure stack start is in RAM
  	sub sp, sp, t0
  	andi sp, sp, -16 // Force 16-byte alignment
  	1:
  	csrr t0, mscratch
  	// now SP is in RAM - continue
  	"#,

• I think riscv-rt supports vectored-mode (our chips only support vectored-mode mtvec)

Maybe we just need to start and see where we hit the show-stoppers

[romancardenas]

• I'm open to adding a full-trap-frame feature to capture all these registers.
• While riscv-rt does not support nested interrupts directly, I typically use the riscv::interrupt::nested function, which allows you to do this. In any case, we can also figure out a way for esp-riscv-rt to inject this enriched functionality.
• Probably the flip-link feature could be useful for people using riscv-rt for other chips.
• We can also use of the _pre_init (or a new symbol) that allows crates such as esp-riscv-rt to inject additional features (in assembly) to base riscv-rt. cortex-m-rt has deprecated the pre_init attribute, as running Rust code before the initialization is finished might be unsound.
  The new version of riscv-rt allows you to use direct or vectored mode, so that shouldn't be a blocker.

It is great to start finding out what we have and what kind of problem we are facing!

[romancardenas]

@bjoernQ can you point out where you use the trap frame at esp-wifi?

[bjoernQ]

Sure, basically it's just a timer ISR where we call

esp-hal/esp-wifi/src/preempt/mod.rs

Line 135 in 2c14e59

pub(crate) fn task_switch(trap_frame: &mut TrapFrame) {

There we just swap all the registers and a few CSRs ... That's all the magic :)

[romancardenas]

A possibility for extending the trap frame is letting esp-riscv-rt overwrite _start_trap_rust so you can push all the additional registers. Then, you could jump to a target-specific _start_esp_trap_rust, which input parameter would be EspTrapFrame:

// in esp-riscv-rt
#[repr(C)]
struct EspTrapFrame {
   /* ... all required extra registers ... */
   frame: riscv_rt::TrapFrame,
}

So the sequence would be:

1. Interrupt INTERRUPT triggers.
2. We jump to _start_INTERRUPT_trap in vectored mode. This just pushes the regular trap frame and adapts the code to work like in direct mode.
3. We jump to _start_trap_rust. Regular targets use riscv-rt's implementation. ESP targets have a custom assembly routine that pushes all the additional fields to the trap, activate interrupt nesting?, and jump to _start_esp_trap_rust (or something like that).
4. Now things work as esp-riscv-rt needs

[romancardenas]

A potential minor issue I see here, especially for ESP32C6 trying to use the CLINT backend in RTIC:

The CLINT backend uses riscv::interrupt::nested to allow nested interrupts. If esp-riscv-rt forces interrupt nesting, then there would be an unnecessary overhead. It would be great if esp-hal could use the standard nesting mechanism to shrink a bit the trap frame and make it possible to opt-out interrupt nesting.

In any case, I would advance here and then polish these issues.