From 9f5332f44edaee66aefa536bc599ddd5e8cadeff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rom=C3=A1n=20C=C3=A1rdenas=20Rodr=C3=ADguez?= Date: Fri, 4 Jul 2025 10:36:56 +0200 Subject: [PATCH] Use callee-saved registers for preserving arguments --- riscv-rt/CHANGELOG.md | 3 +++ riscv-rt/src/asm.rs | 45 +++++++++++++------------------- riscv-rt/src/lib.rs | 60 +++++++++++++++++++++++++++++++++++++++---- 3 files changed, 76 insertions(+), 32 deletions(-) diff --git a/riscv-rt/CHANGELOG.md b/riscv-rt/CHANGELOG.md index de94d400..2abd567d 100644 --- a/riscv-rt/CHANGELOG.md +++ b/riscv-rt/CHANGELOG.md @@ -37,6 +37,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - `clippy` fixes - Merged `cfg_global_asm!` macro invocations to guarantee contiguous code generation. - Use `.balign` instead of `.align` in `_default_abort` +- Use `s0`, `s1`, `s2` (in RVI), and `a5` (in RVE) to preserve the input arguments of + `_start_rust`. The previous implementation used the stack. This was unsound, as in + the `.init` section RAM is still uninitialized. ## [v0.15.0] - 2025-06-10 diff --git a/riscv-rt/src/asm.rs b/riscv-rt/src/asm.rs index fe2c9c07..1b67be14 100644 --- a/riscv-rt/src/asm.rs +++ b/riscv-rt/src/asm.rs @@ -87,7 +87,7 @@ _abs_start: jr t0 1:", // only valid harts reach this point -// INITIALIZE GLOBAL POINTER, STACK POINTER, AND FRAME POINTER +// INITIALIZE GLOBAL POINTER (x3/gp) AND STACK POINTER (x2/sp) ".option push .option norelax la gp, __global_pointer$ @@ -111,20 +111,14 @@ _abs_start: "la t1, _stack_start", #[cfg(not(feature = "single-hart"))] "sub t1, t1, t0", - "andi sp, t1, -16 // align stack to 16-bytes - add s0, sp, zero", -// STORE A0..A2 IN THE STACK, AS THEY WILL BE NEEDED LATER BY _start_rust - #[cfg(target_arch = "riscv32")] - "addi sp, sp, -4 * 4 // we must keep stack aligned to 16-bytes - sw a0, 4 * 0(sp) - sw a1, 4 * 1(sp) - sw a2, 4 * 2(sp)", - #[cfg(target_arch = "riscv64")] - "addi sp, sp, -8 * 4 // we must keep stack aligned to 16-bytes - sd a0, 8 * 0(sp) - sd a1, 8 * 1(sp) - sd a2, 8 * 2(sp)", - + "andi sp, t1, -16", // align stack to 16-bytes + // PRESERVE VALUES OF a0..a2, AS THEY WILL BE NEEDED LATER BY _start_rust + "mv s0, a0 + mv s1, a1", + #[cfg(riscvi)] + "mv s2, a2", + #[cfg(not(riscvi))] + "mv a5, a2", // IN RISCVE TARGETS, s2 does not exist, so we use a5 instead // CALL __pre_init (IF ENABLED) AND INITIALIZE RAM #[cfg(not(feature = "single-hart"))] // Skip RAM initialization if current hart is not the boot hart @@ -165,7 +159,7 @@ _abs_start: addi t0, t0, 8 bltu t0, t2, 3b", " -4: // RAM initialized", +4:", // From this point, RAM is initialized // INITIALIZE FLOATING POINT UNIT #[cfg(any(riscvf, riscvd))] @@ -183,17 +177,14 @@ _abs_start: "fscsr x0", } -// RESTORE a0..a2, AND JUMP TO _start_rust FUNCTION - #[cfg(target_arch = "riscv32")] - "lw a0, 4 * 0(sp) - lw a1, 4 * 1(sp) - lw a2, 4 * 2(sp) - addi sp, sp, 4 * 4", - #[cfg(target_arch = "riscv64")] - "ld a0, 8 * 0(sp) - ld a1, 8 * 1(sp) - ld a2, 8 * 2(sp) - addi sp, sp, 8 * 4", +// RESTORE a0..a2, INITIALIZE FRAME POINTER (x8/s0/fp), AND JUMP TO _start_rust FUNCTION + "mv a0, s0 + mv a1, s1", + #[cfg(riscvi)] + "mv a2, s2", + #[cfg(not(riscvi))] + "mv a2, a5", + "mv s0, sp", // From this point, it is safe to execute Rust code "la t0, _start_rust jr t0 .cfi_endproc", diff --git a/riscv-rt/src/lib.rs b/riscv-rt/src/lib.rs index 934dbd21..5af6deb0 100644 --- a/riscv-rt/src/lib.rs +++ b/riscv-rt/src/lib.rs @@ -5,7 +5,7 @@ //! //! # Features //! -//! This crates takes care of: +//! This crate takes care of: //! //! - The memory layout of the program. //! @@ -337,10 +337,17 @@ //! This function can be redefined in the following way: //! //! ``` no_run -//! #[export_name = "_mp_hook"] -//! pub extern "Rust" fn mp_hook(hartid: usize) -> bool { -//! // ... -//! } +//! core::arch::global_asm!( +//! r#".section .init.mp_hook, "ax" +//! .global _mp_hook +//! _mp_hook: +//! beqz a0, 2f // check if hartid is 0 +//! 1: wfi // If not, wait for interrupt in a loop +//! j 1b +//! 2: li a0, 1 // Otherwise, return true +//! ret +//! "# +//! ); //! ``` //! //! Default implementation of this function wakes hart 0 and busy-loops all the other harts. @@ -350,6 +357,11 @@ //! `_mp_hook` is only necessary in multi-core targets. If the `single-hart` feature is enabled, //! `_mp_hook` is not included in the binary. //! +//! ### Important +//! +//! `_mp_hook` is a startup function that is called before RAM initialization. It must comply +//! with all the [considerations for startup functions](#important-considerations-for-startup-functions). +//! //! ## `_setup_interrupts` //! //! This function is called right before the main function and is responsible for setting up @@ -513,6 +525,11 @@ //! Note that using this macro is discouraged, as it may lead to undefined behavior. //! We left this option for backwards compatibility, but it is subject to removal in the future. //! +//! ### Important +//! +//! `__pre_init` is a startup function that is called before RAM initialization. It must comply +//! with all the [considerations for startup functions](#important-considerations-for-startup-functions). +//! //! ## `single-hart` //! //! Saves a little code size if there is only one hart on the target. @@ -555,6 +572,39 @@ //! [attr-external-interrupt]: attr.external_interrupt.html //! [attr-core-interrupt]: attr.core_interrupt.html //! [attr-pre-init]: attr.pre_init.html +//! +//! # Important considerations for startup functions +//! +//! Currently, there are two startup functions that are called during the startup sequence before +//! jumping to the Rust entry point ([`start_rust`]): +//! +//! - `_mp_hook` +//! - `__pre_init` +//! +//! These functions **MUST**: +//! +//! - Be implemented in assembly. +//! - Be placed within the `.init` section. +//! - Follow specific calling conventions and register usage. +//! +//! Before calling these functions, the runtime uses callee-saved registers to preserve +//! the initial argument values passed to `_start_rust`. Thus, the following registers are +//! reserved and **MUST NOT** be used in any startup function: +//! +//! - `s0`: Contains the original value of `a0` (hart ID). +//! - `s1`: Contains the original value of `a1` (usually a pointer to the device tree blob). +//! - **IN RISCVI TARGETS `s2`**: Contains the original value of `a2` (usually reserved for future use). +//! - **IN RISCVE TARGETS `a5`**: There are no more callee-saved registers, so `a2` is preserved in `a5`. +//! +//! As a general rule, when developing startup functions, use only these registers: +//! +//! - `**IN RISCVI TARGETS**: t0-t6` and `a0-a7`. +//! - `**IN RISCVE TARGETS**: t0-t2` and `a0-a4` (note that `a5` is reserved). +//! +//! If possible, use only temporary registers, as they are always safe to modify. +//! Note that, in `_mp_hook`, you are expected to modify `a0`, as it must hold the return value. +//! +//! **Violating these constraints will result in incorrect arguments being passed to `main()`.** // NOTE: Adapted from cortex-m/src/lib.rs #![no_std]