Patch merge origin 240405 (#5)

* Add pre_init_trap

* Use weak symbols

* Fix sip::{set,clear}_ssoft

* Fix previously incorrect changelog

* impl From<usize> for Mcause

This is useful unit tests that want test trap handlers.

* Implement Mstatus.bits()

* make abort weak

---------

Co-authored-by: Román Cárdenas Rodríguez <rcardenas.rod@gmail.com>
Co-authored-by: Property404 <git@dagans.dev>
Co-authored-by: Aleš Katona <ales@katona.me>
Co-authored-by: Jesse Braham <jessebraham@users.noreply.github.com>
Co-authored-by: David Reiss <dreiss@meta.com>

riscv-rt/CHANGELOG.md

@@ -7,13 +7,20 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+### Added
+
+- Add `pre_init_trap` to detect early errors during the boot process.
+
 ### Changed
 
 - Moved all the assembly code to `asm.rs`
+- Use `weak` symbols for functions such as `_mp_hook` or `_start_trap`
+- `abort` is now `weak`, so it is possible to link third-party libraries including this symbol.
 
 ### Removed
 
 - `start_rust` is no longer needed, as it is now written in assembly
+- `default_*` symbols are no longer needed, as we use `weak` symbols now.
 
 ## [v0.12.2] - 2024-02-15
 

riscv-rt/link.x.in

@@ -7,7 +7,7 @@
   static mut _heap_size }`).
 
 - `EXTERN` forces the linker to keep a symbol in the final binary. We use this to make sure a
-  symbol if not dropped if it appears in or near the front of the linker arguments and "it's not
+  symbol is not dropped if it appears in or near the front of the linker arguments and "it's not
   needed" by any of the preceding objects (linker arguments)
 
 - `PROVIDE` is used to provide default values that can be overridden by a user linker script
@@ -28,6 +28,15 @@ PROVIDE(_max_hart_id = 0);
 PROVIDE(_hart_stack_size = 2K);
 PROVIDE(_heap_size = 0);
 
+/** EXCEPTION HANDLERS **/
+
+/* Default exception handler. The riscv-rt crate provides a weak alias of this function,
+   which is a busy loop. Users can override this alias by defining the symbol themselves */
+EXTERN(ExceptionHandler);
+
+/* It is possible to define a special handler for each exception type.
+   By default, all exceptions are handled by ExceptionHandler. However, users can
+   override these alias by defining the symbol themselves */
 PROVIDE(InstructionMisaligned = ExceptionHandler);
 PROVIDE(InstructionFault = ExceptionHandler);
 PROVIDE(IllegalInstruction = ExceptionHandler);
@@ -43,6 +52,15 @@ PROVIDE(InstructionPageFault = ExceptionHandler);
 PROVIDE(LoadPageFault = ExceptionHandler);
 PROVIDE(StorePageFault = ExceptionHandler);
 
+/** INTERRUPT HANDLERS **/
+
+/* Default interrupt handler. The riscv-rt crate provides a weak alias of this function,
+   which is a busy loop. Users can override this alias by defining the symbol themselves */
+EXTERN(DefaultHandler);
+
+/* It is possible to define a special handler for each interrupt type.
+   By default, all interrupts are handled by DefaultHandler. However, users can
+   override these alias by defining the symbol themselves */
 PROVIDE(SupervisorSoft = DefaultHandler);
 PROVIDE(MachineSoft = DefaultHandler);
 PROVIDE(SupervisorTimer = DefaultHandler);
@@ -50,32 +68,6 @@ PROVIDE(MachineTimer = DefaultHandler);
 PROVIDE(SupervisorExternal = DefaultHandler);
 PROVIDE(MachineExternal = DefaultHandler);
 
-PROVIDE(DefaultHandler = DefaultInterruptHandler);
-PROVIDE(ExceptionHandler = DefaultExceptionHandler);
-
-/* # Pre-initialization function */
-/* If the user overrides this using the `#[pre_init]` attribute or by creating a `__pre_init` function,
-   then the function this points to will be called before the RAM is initialized. */
-PROVIDE(__pre_init = default_pre_init);
-
-/* A PAC/HAL defined routine that should initialize custom interrupt controller if needed. */
-PROVIDE(_setup_interrupts = default_setup_interrupts);
-
-/* # Multi-processing hook function
-   fn _mp_hook() -> bool;
-
-   This function is called from all the harts and must return true only for one hart,
-   which will perform memory initialization. For other harts it must return false
-   and implement wake-up in platform-dependent way (e.g. after waiting for a user interrupt).
-*/
-PROVIDE(_mp_hook = default_mp_hook);
-
-/* # Start trap function override
-  By default uses the riscv crates default trap handler
-  but by providing the `_start_trap` symbol external crates can override.
-*/
-PROVIDE(_start_trap = default_start_trap);
-
 SECTIONS
 {
   .text.dummy (NOLOAD) :
@@ -91,6 +83,8 @@ SECTIONS
     KEEP(*(.init));
     KEEP(*(.init.rust));
     . = ALIGN(4);
+    KEEP(*(.init.trap));
+    . = ALIGN(4);
     *(.trap);
     *(.trap.rust);
     *(.text.abort);

riscv-rt/src/asm.rs

@@ -66,12 +66,19 @@ _abs_start:
     .option norelax
     .cfi_startproc
     .cfi_undefined ra",
+    // Disable interrupts
     #[cfg(feature = "s-mode")]
     "csrw sie, 0
     csrw sip, 0",
     #[cfg(not(feature = "s-mode"))]
     "csrw mie, 0
     csrw mip, 0",
+    // Set pre-init trap vector
+    "la t0, _pre_init_trap",
+    #[cfg(feature = "s-mode")]
+    "csrw stvec, t0",
+    #[cfg(not(feature = "s-mode"))]
+    "csrw mtvec, t0",
 );
 
 // ZERO OUT GENERAL-PURPOSE REGISTERS
@@ -220,6 +227,53 @@ cfg_global_asm!(
     .cfi_endproc",
 );
 
+cfg_global_asm!(
+    // Default implementation of `__pre_init` does nothing.
+    // Users can override this function with the [`#[pre_init]`] macro.
+    ".weak __pre_init
+__pre_init:
+    ret",
+    #[cfg(not(feature = "single-hart"))]
+    // Default implementation of `_mp_hook` wakes hart 0 and busy-loops all the other harts.
+    // Users can override this function by defining their own `_mp_hook`.
+    // This function is only used when the `single-hart` feature is not enabled.
+    ".weak _mp_hook
+_mp_hook:
+    beqz a0, 2f // if hartid is 0, return true
+1:  wfi // Otherwise, wait for interrupt in a loop
+    j 1b
+2:  li a0, 1
+    ret",
+    // Default implementation of `_setup_interrupts` sets the trap vector to `_start_trap`.
+    // Trap mode is set to `Direct` by default.
+    // Users can override this function by defining their own `_setup_interrupts`
+    ".weak _setup_interrupts
+_setup_interrupts:
+    la t0, _start_trap", // _start_trap is 16-byte aligned, so it corresponds to the Direct trap mode
+    #[cfg(feature = "s-mode")]
+    "csrw stvec, t0",
+    #[cfg(not(feature = "s-mode"))]
+    "csrw mtvec, t0",
+    "ret",
+    // Default implementation of `ExceptionHandler` is an infinite loop.
+    // Users can override this function by defining their own `ExceptionHandler`
+    ".weak ExceptionHandler
+ExceptionHandler:
+    j ExceptionHandler",
+    // Default implementation of `DefaultHandler` is an infinite loop.
+    // Users can override this function by defining their own `DefaultHandler`
+    ".weak DefaultHandler
+DefaultHandler:
+    j DefaultHandler",
+    // Default implementation of `_pre_init_trap` is an infinite loop.
+    // Users can override this function by defining their own `_pre_init_trap`
+    // If the execution reaches this point, it means that there is a bug in the boot code.
+    ".section .init.trap, \"ax\"
+    .weak _pre_init_trap
+_pre_init_trap:
+    j _pre_init_trap",
+);
+
 /// Trap entry point (_start_trap). It saves caller saved registers, calls
 /// _start_trap_rust, restores caller saved registers and then returns.
 ///
@@ -241,8 +295,8 @@ macro_rules! trap_handler {
         global_asm!(
         "
             .section .trap, \"ax\"
-            .global default_start_trap
-        default_start_trap:",
+            .weak _start_trap
+            _start_trap:",
             // save space for trap handler in stack
             concat!("addi sp, sp, -", stringify!($TRAP_SIZE * $BYTES)),
             // save registers in the desired order
@@ -280,10 +334,10 @@ trap_handler!(
      (a0, 8), (a1, 9), (a2, 10), (a3, 11), (a4, 12), (a5, 13), (a6, 14), (a7, 15)]
 );
 
-// Make sure there is an abort when linking
+#[rustfmt::skip]
 global_asm!(
     ".section .text.abort
-     .globl abort
-abort:
+.weak abort
+abort:  // make sure there is an abort symbol when linking
     j abort"
 );

riscv-rt/src/lib.rs

@@ -228,6 +228,16 @@
 //! }
 //! ```
 //!
+//! ## `_pre_init_trap`
+//!
+//! This function is set as a provisional trap handler for the early trap handling.
+//! If either an exception or an interrupt occurs during the boot process, this
+//! function is triggered. The default implementation of this function is a busy-loop.
+//! While this function can be redefined, it is not recommended to do so, as it is
+//! intended to be a temporary trap handler to detect bugs in the early boot process.
+//! Recall that this trap is triggered before the `.bss` and `.data` sections are
+//! initialized, so it is not safe to use any global variables in this function.
+//!
 //! ### `_mp_hook`
 //!
 //! This function is called from all the harts and must return true only for one hart,
@@ -247,7 +257,15 @@
 //! Default implementation of this function wakes hart 0 and busy-loops all the other harts.
 //!
 //! `_mp_hook` is only necessary in multi-core targets. If the `single-hart` feature is enabled,
-//! `_mp_hook` is not called, as it is assumed that there is only one hart on the target.
+//! `_mp_hook` is not included in the binary.
+//!
+//! ### `_setup_interrupts`
+//!
+//! This function is called right before the main function and is responsible for setting up
+//! the interrupt controller.
+//!
+//! Default implementation sets the trap vector to `_start_trap` in direct mode.
+//! Users can override this function by defining their own `_setup_interrupts`
 //!
 //! ### Core exception handlers
 //!
@@ -404,10 +422,10 @@
 mod asm;
 
 #[cfg(feature = "s-mode")]
-use riscv::register::{scause as xcause, stvec as xtvec, stvec::TrapMode as xTrapMode};
+use riscv::register::scause as xcause;
 
 #[cfg(not(feature = "s-mode"))]
-use riscv::register::{mcause as xcause, mtvec as xtvec, mtvec::TrapMode as xTrapMode};
+use riscv::register::mcause as xcause;
 
 pub use riscv_rt_macros::{entry, pre_init};
 
@@ -487,28 +505,6 @@ pub unsafe extern "C" fn start_trap_rust(trap_frame: *const TrapFrame) {
     }
 }
 
-#[doc(hidden)]
-#[no_mangle]
-#[allow(unused_variables, non_snake_case)]
-pub fn DefaultExceptionHandler(trap_frame: &TrapFrame) -> ! {
-    loop {
-        // Prevent this from turning into a UDF instruction
-        // see rust-lang/rust#28728 for details
-        continue;
-    }
-}
-
-#[doc(hidden)]
-#[no_mangle]
-#[allow(non_snake_case)]
-pub fn DefaultInterruptHandler() {
-    loop {
-        // Prevent this from turning into a UDF instruction
-        // see rust-lang/rust#28728 for details
-        continue;
-    }
-}
-
 extern "C" {
     fn InstructionMisaligned(trap_frame: &TrapFrame);
     fn InstructionFault(trap_frame: &TrapFrame);
@@ -572,40 +568,3 @@ pub static __INTERRUPTS: [Option<unsafe extern "C" fn()>; 12] = [
     None,
     Some(MachineExternal),
 ];
-
-/// Default implementation of `_pre_init` does nothing.
-/// Users can override this function with the [`#[pre_init]`] macro.
-#[doc(hidden)]
-#[no_mangle]
-#[rustfmt::skip]
-pub extern "Rust" fn default_pre_init() {}
-
-/// Default implementation of `_mp_hook` wakes hart 0 and busy-loops all the other harts.
-/// Users can override this function by defining their own `_mp_hook`.
-/// 
-/// # Note
-/// 
-/// If the `single-hart` feature is enabled, `_mp_hook` is not called.
-#[doc(hidden)]
-#[no_mangle]
-#[rustfmt::skip]
-pub extern "Rust" fn default_mp_hook(hartid: usize) -> bool {
-    match hartid {
-        0 => true,
-        _ => loop {
-            riscv::asm::wfi();
-        },
-    }
-}
-
-/// Default implementation of `_setup_interrupts` sets `mtvec`/`stvec` to the address of `_start_trap`.
-#[doc(hidden)]
-#[no_mangle]
-#[rustfmt::skip]
-pub unsafe extern "Rust" fn default_setup_interrupts() {
-    extern "C" {
-        fn _start_trap();
-    }
-
-    xtvec::write(_start_trap as usize, xTrapMode::Direct);
-}

riscv/CHANGELOG.md

@@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+### Added
+
+- Add `Mcause::from(usize)` for use in unit tests
+- Add `Mstatus.bits()`
+
+### Fixed
+
+- Fixed `sip::set_ssoft` and `sip::clear_ssoft` using wrong address
+
 ## [v0.11.1] - 2024-02-15
 
 ### Changed

riscv/src/register/mcause.rs

@@ -6,6 +6,13 @@ pub struct Mcause {
     bits: usize,
 }
 
+impl From<usize> for Mcause {
+    #[inline]
+    fn from(bits: usize) -> Self {
+        Self { bits }
+    }
+}
+
 /// Trap Cause
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub enum Trap {

riscv/src/register/mstatus.rs

@@ -65,6 +65,12 @@ impl From<bool> for Endianness {
 }
 
 impl Mstatus {
+    /// Returns the contents of the register as raw bits
+    #[inline]
+    pub fn bits(&self) -> usize {
+        self.bits
+    }
+
     /// Supervisor Interrupt Enable
     #[inline]
     pub fn sie(&self) -> bool {

riscv/src/register/sip.rs

@@ -33,8 +33,8 @@ impl Sip {
 }
 
 read_csr_as!(Sip, 0x144);
-set!(0x344);
-clear!(0x104);
+set!(0x144);
+clear!(0x144);
 
 set_clear_csr!(
     /// Supervisor Software Interrupt Pending