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

[romancardenas]

trafficstars

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) https://github.com/esp-rs/esp-hal/blob/ccb3a1ba40c0a4ae88abe6e449d25833432be3bb/esp-riscv-rt/src/lib.rs#L76-L161 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 https://github.com/esp-rs/esp-hal/blob/ccb3a1ba40c0a4ae88abe6e449d25833432be3bb/esp-riscv-rt/src/lib.rs#L723-L730 and https://github.com/esp-rs/esp-hal/blob/ccb3a1ba40c0a4ae88abe6e449d25833432be3bb/esp-riscv-rt/src/lib.rs#L735-L740 ... 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): https://github.com/esp-rs/esp-hal/blob/ccb3a1ba40c0a4ae88abe6e449d25833432be3bb/esp-riscv-rt/src/lib.rs#L489-L514
• 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 https://github.com/esp-rs/esp-hal/blob/2c14e595dbc853b23a085a48aff7105eaef71652/esp-wifi/src/preempt/mod.rs#L135

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.

[bjoernQ]

See #3857

See https://github.com/rust-embedded/riscv/pull/335#issuecomment-3140658097