Merge branch 'master' into crs-write-unsafe

romancardenas · web-flow · commit 08abb733ed65 · 2025-01-27T18:07:41.000+01:00
diff --git a/.github/ISSUE_TEMPLATE/-riscv---nominate-csr-to-be--write--safe.md b/.github/ISSUE_TEMPLATE/-riscv---nominate-csr-to-be--write--safe.md
@@ -0,0 +1,23 @@
+---
+name: "`riscv`: Nominate CSR to be `write` safe"
+about: Suggest to make writes of a given CSR safe
+title: "`riscv`: make [CSR] write-safe"
+labels: ''
+assignees: ''
+
+---
+
+**Which CSR do you want to nominate as `write` safe?**
+Indicate which CSR you want to be `write` safe. Ex. `mepc`
+
+**Does a CSR write introduce potential memory safety issues in safe code? Please describe.**
+A clear and concise justification on why writing to this CSR will **never** introduce memory safety issues in safe code.
+
+**Does a CSR write introduce potential undefined behavior in safe code? Please describe.**
+A clear and concise justification on why writing to this CSR will **never** lead to undefined behavior in safe code.
+
+**Does a CSR write invalidate invariants in safe code?**
+A clear and concise justification on why writing to this CSR will **never** invalidate invariants in safe code.
+
+**Additional context**
+Please feel free to add any other context or screenshots about your request here.
diff --git a/riscv-rt/CHANGELOG.md b/riscv-rt/CHANGELOG.md
@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Changed
 
+- Use `RISCV_MTVEC_ALIGN` to control the alignment constraint of the vector table.
+- Ensure the `.heap` section is 4-byte aligned.
 - Limit rustc cfg flags to `riscvi`, `riscvm`, `riscvf`, and `riscvd`.
 - Temporary use of `RISCV_RT_LLVM_ARCH_PATCH` environment variable to include the
   temporary patch required for avoid LLVM spurious errors.
diff --git a/riscv-rt/build.rs b/riscv-rt/build.rs
@@ -47,6 +47,13 @@ fn main() {
         // make sure that these env variables are not changed without notice.
         println!("cargo:rerun-if-env-changed=RISCV_RT_BASE_ISA");
         println!("cargo:rerun-if-env-changed=RISCV_RT_LLVM_ARCH_PATCH");
+        if env::var_os("CARGO_FEATURE_V_TRAP").is_some()
+            && env::var_os("CARGO_FEATURE_NO_INTERRUPTS").is_none()
+        {
+            // This environment variable is used by the `#[riscv::pac_enum()]` call in
+            // `src/interrupts.rs` (when `v-trap` is enabled and `no-interrupts` disabled).
+            println!("cargo:rerun-if-env-changed=RISCV_MTVEC_ALIGN");
+        }
 
         for flag in target.rustc_flags() {
             // Required until target_feature risc-v is stable and in-use
diff --git a/riscv-rt/link.x.in b/riscv-rt/link.x.in
@@ -168,7 +168,7 @@ SECTIONS
   __ebss = .;
 
   /* fictitious region that represents the memory available for the heap */
-  .heap (NOLOAD) :
+  .heap (NOLOAD) : ALIGN(4)
   {
     __sheap = .;
     . += _heap_size;
diff --git a/riscv-rt/src/interrupts.rs b/riscv-rt/src/interrupts.rs
@@ -7,97 +7,24 @@
 //!
 //! In vectored mode (i.e., `v-trap` feature enabled), interrupt dispatching is handled by hardware.
 //! To support this mode, we provide inline assembly code that defines the interrupt vector table.
+//! Since the alignment constraint of this vector table is implementation-specific, it can be
+//! changed by setting the `RISCV_MTVEC_ALIGN` environment variable (the default is 4).
 //!
 //! # Note
 //!
-//! If your target has custom core interrupt sources, the target PAC might provide equivalent
-//! code to adapt for the target needs. In this case, you may need to opt out this module.
-//! To do so, activate the `no-interrupts` feature of the `riscv-rt` crate.
-
-#[cfg(not(feature = "v-trap"))]
-extern "C" {
-    fn SupervisorSoft();
-    fn MachineSoft();
-    fn SupervisorTimer();
-    fn MachineTimer();
-    fn SupervisorExternal();
-    fn MachineExternal();
-}
-
-/// Array with all the core interrupt handlers sorted according to their interrupt source code.
-///
-/// # Note
-///
-/// This array is necessary only in direct mode (i.e., `v-trap` feature disabled).
-#[cfg(not(feature = "v-trap"))]
-#[no_mangle]
-pub static __CORE_INTERRUPTS: [Option<unsafe extern "C" fn()>; 12] = [
-    None,
-    Some(SupervisorSoft),
-    None,
-    Some(MachineSoft),
-    None,
-    Some(SupervisorTimer),
-    None,
-    Some(MachineTimer),
-    None,
-    Some(SupervisorExternal),
-    None,
-    Some(MachineExternal),
-];
-
-/// It calls the corresponding interrupt handler depending on the interrupt source code.
-///
-/// # Note
-///
-/// This function is only required in direct mode (i.e., `v-trap` feature disabled).
-/// In vectored mode, interrupt handler dispatching is performed directly by hardware.
-///
-/// # Safety
-///
-/// This function must be called only from the [`crate::start_trap_rust`] function.
-/// Do **NOT** call this function directly.
-#[cfg(not(feature = "v-trap"))]
-#[inline]
-#[no_mangle]
-pub unsafe extern "C" fn _dispatch_core_interrupt(code: usize) {
-    extern "C" {
-        fn DefaultHandler();
-    }
-    match __CORE_INTERRUPTS.get(code) {
-        Some(Some(handler)) => handler(),
-        _ => DefaultHandler(),
-    }
-}
+//! If your target has custom core interrupt sources, the target PAC might provide equivalent code
+//! to adapt for the target needs (and is responsible for any alignment constraint). In this case,
+//! you may need to opt out this module. To do so, activate the `no-interrupts` feature of the
+//! `riscv-rt` crate.
 
 // In vectored mode, we also must provide a vector table
-#[cfg(all(
-    any(target_arch = "riscv32", target_arch = "riscv64"),
-    feature = "v-trap"
-))]
-core::arch::global_asm!(
-    r#" .section .trap, "ax"
-        .weak _vector_table
-        .type _vector_table, @function
-        
-        .option push
-        .balign 0x4 // TODO check if this is the correct alignment
-        .option norelax
-        .option norvc
-        
-        _vector_table:
-            j _start_trap                     // Interrupt 0 is used for exceptions
-            j _start_SupervisorSoft_trap
-            j _start_DefaultHandler_trap      // Interrupt 2 is reserved
-            j _start_MachineSoft_trap
-            j _start_DefaultHandler_trap      // Interrupt 4 is reserved
-            j _start_SupervisorTimer_trap
-            j _start_DefaultHandler_trap      // Interrupt 6 is reserved
-            j _start_MachineTimer_trap
-            j _start_DefaultHandler_trap      // Interrupt 8 is reserved
-            j _start_SupervisorExternal_trap
-            j _start_DefaultHandler_trap      // Interrupt 10 is reserved
-            j _start_MachineExternal_trap
-        
-        .option pop"#
-);
+#[riscv::pac_enum(unsafe CoreInterruptNumber)]
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+enum Interrupt {
+    SupervisorSoft = 1,
+    MachineSoft = 3,
+    SupervisorTimer = 5,
+    MachineTimer = 7,
+    SupervisorExternal = 9,
+    MachineExternal = 11,
+}
diff --git a/riscv/CHANGELOG.md b/riscv/CHANGELOG.md
@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Changed
 
 - Make all CSR writes `unsafe` by default (#209)
+- Use `RISCV_MTVEC_ALIGN` to control the alignment constraint of the vector table
 - Simplify register macros with `cfg` field
 - Align assembly functions with `cortex-m`
 - Use CSR helper macros to define `marchid` register
@@ -27,6 +28,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Use CSR helper macros to define `misa` register
 - Use CSR helper macros to define `mip` register
 - Use CSR helper macros to define `mstatus` register
+- Use CSR helper macros to define `mstatush` register
+- Use CSR helper macros to define `mtvec` register
+- Use CSR helper macros to define `mtvendorid` register
+- Use CSR helper macros to define `satp` register
 
 ## [v0.12.1] - 2024-10-20
 
diff --git a/riscv/macros/src/lib.rs b/riscv/macros/src/lib.rs
@@ -265,7 +265,20 @@ impl PacEnumItem {
     }
 
     fn vector_table(&self) -> TokenStream2 {
-        let mut asm = String::from(
+        let align = match std::env::var("RISCV_MTVEC_ALIGN") {
+            Ok(x) => x.parse::<u32>().ok(),
+            Err(std::env::VarError::NotPresent) => Some(4),
+            Err(std::env::VarError::NotUnicode(_)) => None,
+        };
+        let align = match align {
+            Some(x) if x.is_power_of_two() && 4 <= x => x,
+            _ => {
+                return quote!(compile_error!(
+                    "RISCV_MTVEC_ALIGN is not a power of 2 (minimum 4)"
+                ))
+            }
+        };
+        let mut asm = format!(
             r#"
 #[cfg(all(feature = "v-trap", any(target_arch = "riscv32", target_arch = "riscv64")))]
 core::arch::global_asm!("
@@ -274,7 +287,7 @@ core::arch::global_asm!("
     .type _vector_table, @function
     
     .option push
-    .balign 0x4 // TODO check if this is the correct alignment
+    .balign {align}
     .option norelax
     .option norvc
     
@@ -315,6 +328,8 @@ core::arch::global_asm!("
         let max_discriminant = self.max_number;
         let valid_matches = self.valid_matches();
 
+        let is_core_interrupt = matches!(attr, PacTrait::Interrupt(InterruptType::Core));
+
         // Push the trait implementation
         res.push(quote! {
             unsafe impl riscv::#trait_name for #name {
@@ -350,19 +365,26 @@ core::arch::global_asm!("
 
             let handlers = self.handlers(&trap_config);
             let interrupt_array = self.handlers_array();
+            let cfg_v_trap = match is_core_interrupt {
+                true => Some(quote!(#[cfg(not(feature = "v-trap"))])),
+                false => None,
+            };
 
             // Push the interrupt handler functions and the interrupt array
             res.push(quote! {
+                #cfg_v_trap
                 extern "C" {
                     #(#handlers;)*
                 }
 
+                #cfg_v_trap
                 #[doc(hidden)]
                 #[no_mangle]
                 pub static #vector_table: [Option<unsafe extern "C" fn(#(#array_signature),*)>; #max_discriminant + 1] = [
                     #(#interrupt_array),*
                 ];
 
+                #cfg_v_trap
                 #[inline]
                 #[no_mangle]
                 unsafe extern "C" fn #dispatch_fn_name(#(#dispatch_fn_args),*) {
@@ -378,7 +400,7 @@ core::arch::global_asm!("
             });
         }
 
-        if let PacTrait::Interrupt(InterruptType::Core) = attr {
+        if is_core_interrupt {
             res.push(self.vector_table());
         }
 
diff --git a/riscv/src/lib.rs b/riscv/src/lib.rs
@@ -35,6 +35,7 @@
 #![no_std]
 #![allow(clippy::missing_safety_doc)]
 #![allow(clippy::eq_op)]
+#![allow(clippy::identity_op)]
 
 pub use paste::paste;
 
diff --git a/riscv/src/register/macros.rs b/riscv/src/register/macros.rs
@@ -544,7 +544,7 @@ macro_rules! clear_pmp {
 macro_rules! csr {
     ($(#[$doc:meta])*
      $ty:ident,
-     $mask:literal) => {
+     $mask:expr) => {
         #[repr(C)]
         $(#[$doc])*
         #[derive(Clone, Copy, Debug, Eq, PartialEq)]
@@ -647,8 +647,8 @@ macro_rules! csr_field_enum {
 #[macro_export]
 macro_rules! read_write_csr {
     ($(#[$doc:meta])+
-     $ty:ident: $csr:tt,
-     mask: $mask:tt$(,)?
+     $ty:ident: $csr:expr,
+     mask: $mask:expr$(,)?
     ) => {
         $crate::csr!($(#[$doc])+ $ty, $mask);
 
@@ -663,17 +663,17 @@ macro_rules! read_write_csr {
 #[macro_export]
 macro_rules! read_only_csr {
     ($(#[$doc:meta])+
-     $ty:ident: $csr:tt,
-     mask: $mask:tt$(,)?
+     $ty:ident: $csr:expr,
+     mask: $mask:expr$(,)?
     ) => {
         $crate::csr! { $(#[$doc])+ $ty, $mask }
 
         $crate::read_csr_as!($ty, $csr);
     };
 
     ($(#[$doc:meta])+
-     $ty:ident: $csr:tt,
-     mask: $mask:tt,
+     $ty:ident: $csr:expr,
+     mask: $mask:expr,
      sentinel: $sentinel:tt$(,)?,
     ) => {
         $crate::csr! { $(#[$doc])+ $ty, $mask }
@@ -688,8 +688,8 @@ macro_rules! read_only_csr {
 #[macro_export]
 macro_rules! write_only_csr {
     ($(#[$doc:meta])+
-     $ty:ident: $csr:literal,
-     mask: $mask:literal$(,)?
+     $ty:ident: $csr:expr,
+     mask: $mask:expr$(,)?
     ) => {
         $crate::csr! { $(#[$doc])+ $ty, $mask }
 
diff --git a/riscv/src/register/mstatush.rs b/riscv/src/register/mstatush.rs
@@ -2,28 +2,26 @@
 
 pub use super::mstatus::Endianness;
 
-/// mstatus register
-#[derive(Clone, Copy, Debug)]
-pub struct Mstatush {
-    bits: usize,
+read_write_csr! {
+    /// mstatus register
+    Mstatush: 0x310,
+    mask: 0x30,
 }
 
-impl Mstatush {
+read_write_csr_field! {
+    Mstatush,
     /// S-mode non-instruction-fetch memory endianness
-    #[inline]
-    pub fn sbe(&self) -> Endianness {
-        Endianness::from(self.bits & (1 << 4) != 0)
-    }
+    sbe,
+    Endianness: [4:4],
+}
 
+read_write_csr_field! {
+    Mstatush,
     /// M-mode non-instruction-fetch memory endianness
-    #[inline]
-    pub fn mbe(&self) -> Endianness {
-        Endianness::from(self.bits & (1 << 5) != 0)
-    }
+    mbe,
+    Endianness: [5:5],
 }
 
-read_csr_as_rv32!(Mstatush, 0x310);
-write_csr_rv32!(0x310);
 set_rv32!(0x310);
 clear_rv32!(0x310);
 
@@ -44,3 +42,20 @@ pub unsafe fn set_mbe(endianness: Endianness) {
         Endianness::LittleEndian => _clear(1 << 5),
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_mstatush() {
+        let mut m = Mstatush::from_bits(0);
+
+        [Endianness::LittleEndian, Endianness::BigEndian]
+            .into_iter()
+            .for_each(|endianness| {
+                test_csr_field!(m, sbe: endianness);
+                test_csr_field!(m, mbe: endianness);
+            });
+    }
+}
diff --git a/riscv/src/register/mtvec.rs b/riscv/src/register/mtvec.rs
diff --git a/riscv/src/register/mvendorid.rs b/riscv/src/register/mvendorid.rs
diff --git a/riscv/src/register/satp.rs b/riscv/src/register/satp.rs

Original file line number	Diff line number	Diff line change
`@@ -168,7 +168,7 @@ SECTIONS`
`168`	`168`	`__ebss = .;`
`169`	`169`
`170`	`170`	`/* fictitious region that represents the memory available for the heap */`
`171`		`- .heap (NOLOAD) :`
	`171`	`+ .heap (NOLOAD) : ALIGN(4)`
`172`	`172`	`{`
`173`	`173`	`__sheap = .;`
`174`	`174`	`. += _heap_size;`