rustsbi: fix a bug on hart mask

Legacy and new mode of hart masking have different mechanisms.
Under legacy mode, it requires visiting supervisor memory; but under hart mask more, it does not.

Signed-off-by: luojia65 <[email protected]>

src/ecall/ipi.rs

@@ -1,6 +1,6 @@
 use super::SbiRet;
 use crate::hart_mask::HartMask;
-use crate::ipi::{max_hart_id, send_ipi_many};
+use crate::ipi::send_ipi_many;
 
 const FUNCTION_IPI_SEND_IPI: usize = 0x0;
 
@@ -14,11 +14,6 @@ pub fn handle_ecall_ipi(function: usize, param0: usize, param1: usize) -> SbiRet
 
 #[inline]
 fn send_ipi(hart_mask: usize, hart_mask_base: usize) -> SbiRet {
-    let max_hart_id = if let Some(id) = max_hart_id() {
-        id
-    } else {
-        return SbiRet::not_supported();
-    };
-    let hart_mask = unsafe { HartMask::from_addr(hart_mask, hart_mask_base, max_hart_id) };
+    let hart_mask = HartMask::from_mask_base(hart_mask, hart_mask_base);
     send_ipi_many(hart_mask)
 }

src/ecall/legacy.rs

@@ -1,6 +1,6 @@
 use super::SbiRet;
 use crate::hart_mask::HartMask;
-use crate::ipi::{max_hart_id, send_ipi_many};
+use crate::ipi::send_ipi_many;
 use crate::legacy_stdio::{legacy_stdio_getchar, legacy_stdio_putchar};
 use riscv::register::{mie, mip};
 
@@ -20,14 +20,9 @@ pub fn console_getchar() -> SbiRet {
 
 #[inline]
 pub fn send_ipi(hart_mask_addr: usize) -> SbiRet {
-    let max_hart_id = if let Some(id) = max_hart_id() {
-        id
-    } else {
-        return SbiRet::not_supported();
-    };
-    // note(unsafe): if any load fault, should be handled by user or supervisor
-    // base hart should be 0 on legacy
-    let hart_mask = unsafe { HartMask::from_addr(hart_mask_addr, 0, max_hart_id) };
+    // note(unsafe): if any load fault, should be handled by machine level;
+    // see docs at `legacy_from_addr`.
+    let hart_mask = unsafe { HartMask::legacy_from_addr(hart_mask_addr) };
     send_ipi_many(hart_mask)
 }
 

src/ecall/rfence.rs

@@ -1,6 +1,5 @@
 use super::SbiRet;
 use crate::hart_mask::HartMask;
-use crate::ipi::max_hart_id;
 use crate::rfence;
 
 const FUNCTION_RFENCE_REMOTE_FENCE_I: usize = 0x0;
@@ -38,19 +37,9 @@ pub fn handle_ecall_rfence(
     }
 }
 
-// If None = max_hart_id(), that means IPI extension is not supported.
-// In RustSBI, RFENCE support requires an IPI support is implemented.
-// If platform does not provide IPI support, RustSBI will disable RFENCE
-// interface access from SBI environment caller.
-
 #[inline]
 fn remote_fence_i(hart_mask: usize, hart_mask_base: usize) -> SbiRet {
-    let max_hart_id = if let Some(id) = max_hart_id() {
-        id
-    } else {
-        return SbiRet::not_supported();
-    };
-    let hart_mask = unsafe { HartMask::from_addr(hart_mask, hart_mask_base, max_hart_id) };
+    let hart_mask = HartMask::from_mask_base(hart_mask, hart_mask_base);
     rfence::remote_fence_i(hart_mask)
 }
 
@@ -61,12 +50,7 @@ fn remote_sfence_vma(
     start_addr: usize,
     size: usize,
 ) -> SbiRet {
-    let max_hart_id = if let Some(id) = max_hart_id() {
-        id
-    } else {
-        return SbiRet::not_supported();
-    };
-    let hart_mask = unsafe { HartMask::from_addr(hart_mask, hart_mask_base, max_hart_id) };
+    let hart_mask = HartMask::from_mask_base(hart_mask, hart_mask_base);
     rfence::remote_sfence_vma(hart_mask, start_addr, size)
 }
 
@@ -78,12 +62,7 @@ fn remote_sfence_vma_asid(
     size: usize,
     asid: usize,
 ) -> SbiRet {
-    let max_hart_id = if let Some(id) = max_hart_id() {
-        id
-    } else {
-        return SbiRet::not_supported();
-    };
-    let hart_mask = unsafe { HartMask::from_addr(hart_mask, hart_mask_base, max_hart_id) };
+    let hart_mask = HartMask::from_mask_base(hart_mask, hart_mask_base);
     rfence::remote_sfence_vma_asid(hart_mask, start_addr, size, asid)
 }
 
@@ -95,12 +74,7 @@ fn remote_hfence_gvma_vmid(
     size: usize,
     vmid: usize,
 ) -> SbiRet {
-    let max_hart_id = if let Some(id) = max_hart_id() {
-        id
-    } else {
-        return SbiRet::not_supported();
-    };
-    let hart_mask = unsafe { HartMask::from_addr(hart_mask, hart_mask_base, max_hart_id) };
+    let hart_mask = HartMask::from_mask_base(hart_mask, hart_mask_base);
     rfence::remote_hfence_gvma_vmid(hart_mask, start_addr, size, vmid)
 }
 
@@ -111,12 +85,7 @@ fn remote_hfence_gvma(
     start_addr: usize,
     size: usize,
 ) -> SbiRet {
-    let max_hart_id = if let Some(id) = max_hart_id() {
-        id
-    } else {
-        return SbiRet::not_supported();
-    };
-    let hart_mask = unsafe { HartMask::from_addr(hart_mask, hart_mask_base, max_hart_id) };
+    let hart_mask = HartMask::from_mask_base(hart_mask, hart_mask_base);
     rfence::remote_hfence_gvma(hart_mask, start_addr, size)
 }
 
@@ -128,12 +97,7 @@ fn remote_hfence_vvma_asid(
     size: usize,
     asid: usize,
 ) -> SbiRet {
-    let max_hart_id = if let Some(id) = max_hart_id() {
-        id
-    } else {
-        return SbiRet::not_supported();
-    };
-    let hart_mask = unsafe { HartMask::from_addr(hart_mask, hart_mask_base, max_hart_id) };
+    let hart_mask = HartMask::from_mask_base(hart_mask, hart_mask_base);
     rfence::remote_hfence_vvma_asid(hart_mask, start_addr, size, asid)
 }
 
@@ -144,11 +108,6 @@ fn remote_hfence_vvma(
     start_addr: usize,
     size: usize,
 ) -> SbiRet {
-    let max_hart_id = if let Some(id) = max_hart_id() {
-        id
-    } else {
-        return SbiRet::not_supported();
-    };
-    let hart_mask = unsafe { HartMask::from_addr(hart_mask, hart_mask_base, max_hart_id) };
+    let hart_mask = HartMask::from_mask_base(hart_mask, hart_mask_base);
     rfence::remote_hfence_vvma(hart_mask, start_addr, size)
 }

src/hart_mask.rs

@@ -1,54 +1,78 @@
+use core::mem::size_of;
+
 /// Hart mask structure reference
 #[derive(Debug, Clone)]
 pub struct HartMask {
-    bit_vector: *const usize,
-    base: usize,
-    max_hart_id: usize,
+    inner: MaskInner,
 }
 
 impl HartMask {
-    /// Construct a reference to hart mask from bit vector and starting hartid.
-    ///
-    /// # Parameters
-    ///
-    /// - The `vaddr` is a scalar bit-vector containing hartids.
-    ///   Should return address from supervisor level.
-    /// - The `base` is the starting hartid from which bit-vector must be computed.
-    ///   If `base` equals `usize::MAX`, that means `vaddr` is ignored and all available harts must be considered.
-    /// - The `max_hart_id` should be returned by SBI implementation for maximum hart id this hart mask supports.
-    ///
-    /// # Safety
-    ///
-    /// Caller must ensure all usize values in the bit vector is accessible.
-    pub unsafe fn from_addr(vaddr: usize, base: usize, max_hart_id: usize) -> HartMask {
+    /// Construct a hart mask from mask value and base hart id.
+    #[inline]
+    pub fn from_mask_base(hart_mask: usize, hart_mask_base: usize) -> HartMask {
         HartMask {
-            bit_vector: vaddr as *const usize,
-            base,
-            max_hart_id,
+            inner: MaskInner::BitVector {
+                hart_mask,
+                hart_mask_base,
+            }
         }
     }
-
     /// Check if the `hart_id` is included in this hart mask structure.
     pub fn has_bit(&self, hart_id: usize) -> bool {
-        assert!(hart_id <= self.max_hart_id);
-        if self.base == usize::MAX {
-            // If `base` equals `usize::MAX`, that means `vaddr` is ignored and all available harts must be considered.
-            return true;
+        match self.inner {
+            MaskInner::BitVector { hart_mask, hart_mask_base } => {
+                if hart_mask_base == usize::MAX {
+                    // If `hart_mask_base` equals `usize::MAX`, that means `hart_mask` is ignored
+                    // and all available harts must be considered.
+                    return true;
+                }
+                let idx = if let Some(idx) = hart_id.checked_sub(hart_mask_base) {
+                    idx
+                } else {
+                    // hart_id < hart_mask_base, not in current mask range
+                    return false;
+                };
+                if idx > size_of::<usize>() {
+                    // hart_idx >= hart_mask_base + XLEN, not in current mask range
+                    return false;
+                }
+                hart_mask & (1 << idx) != 0
+            },
+            MaskInner::Legacy { legacy_bit_vector } => {
+                fn split_index_usize(index: usize) -> (usize, usize) {
+                    let bits_in_usize = usize::BITS as usize;
+                    (index / bits_in_usize, index % bits_in_usize)
+                }
+                let (i, j) = split_index_usize(hart_id);
+                let cur_vector = unsafe { get_vaddr_usize(legacy_bit_vector.add(i)) };
+                cur_vector & (1 << j) != 0
+            },
         }
-        if hart_id < self.base {
-            // `base` if the starting hartid
-            return false;
+    }
+
+    /// *This is a legacy function; it should not be used in newer designs. If `vaddr` is invalid
+    /// from S level, it would result in machine level load access or load misaligned exception.*
+    ///
+    /// Construct a hart mask from legacy bit vector and number of harts in current platform.
+    #[inline]
+    pub unsafe fn legacy_from_addr(vaddr: usize) -> HartMask {
+        HartMask {
+            inner: MaskInner::Legacy {
+                legacy_bit_vector: vaddr as *const _,
+            }
         }
-        let (i, j) = split_index_usize(hart_id - self.base);
-        let cur_vector = unsafe { get_vaddr_usize(self.bit_vector.add(i)) };
-        cur_vector & (1 << j) != 0
     }
 }
 
-#[inline]
-fn split_index_usize(index: usize) -> (usize, usize) {
-    let bits_in_usize = usize::BITS as usize;
-    (index / bits_in_usize, index % bits_in_usize)
+#[derive(Debug, Clone)]
+enum MaskInner {
+    BitVector {
+        hart_mask: usize,
+        hart_mask_base: usize,
+    },
+    Legacy {
+        legacy_bit_vector: *const usize,
+    },
 }
 
 #[inline]
@@ -76,17 +100,6 @@ unsafe fn get_vaddr_usize(vaddr_ptr: *const usize) -> usize {
             ", ans = lateout(reg) ans, vmem = in(reg) vaddr_ptr, tmp = out(reg) _);
             ans
         }
-        #[cfg(target_arch = "riscv128")]
-        () => {
-            let mut ans: usize;
-            core::arch::asm!("
-                li      {tmp}, (1 << 17)
-                csrrs   {tmp}, mstatus, {tmp}
-                lq      {ans}, 0({vmem})
-                csrw    mstatus, {tmp}
-            ", ans = lateout(reg) ans, vmem = in(reg) vaddr_ptr, tmp = out(reg) _);
-            ans
-        }
         #[cfg(not(any(target_arch = "riscv32", target_arch = "riscv64")))]
         () => {
             drop(vaddr_ptr);

src/ipi.rs

@@ -5,8 +5,6 @@ use alloc::boxed::Box;
 
 /// Inter-processor interrupt support
 pub trait Ipi: Send {
-    /// Get the maximum hart id available by this IPI support module
-    fn max_hart_id(&self) -> usize;
     /// Send an inter-processor interrupt to all the harts defined in `hart_mask`.
     ///
     /// Inter-processor interrupts manifest at the receiving harts as the supervisor software interrupts.
@@ -15,6 +13,11 @@ pub trait Ipi: Send {
     ///
     /// Should return error code `SBI_SUCCESS` if IPI was sent to all the targeted harts successfully.
     fn send_ipi_many(&self, hart_mask: HartMask) -> SbiRet;
+    #[doc(hidden)]
+    /// Get the maximum hart id available by this IPI support module
+    fn max_hart_id(&self) -> usize {
+        unimplemented!("remained for compatibility, should remove in 0.3.0")
+    }
 }
 
 static IPI: OnceFatBox<dyn Ipi + Sync + 'static> = OnceFatBox::new();
@@ -39,8 +42,3 @@ pub(crate) fn send_ipi_many(hart_mask: HartMask) -> SbiRet {
         SbiRet::not_supported()
     }
 }
-
-// Returns maximum hart id if IPI presents, or None if absent.
-pub(crate) fn max_hart_id() -> Option<usize> {
-    IPI.get().map(|ipi| ipi.max_hart_id())
-}

src/rfence.rs

@@ -5,10 +5,6 @@ use alloc::boxed::Box;
 
 /// Remote fence support
 ///
-/// In RustSBI, RFENCE support requires an IPI support is implemented.
-/// If your platform does not provide IPI support, RustSBI will disable RFENCE
-/// interface access from supervisor level.
-///
 /// The remote fence function acts as a full TLB flush if
 /// - `start_addr` and `size` are both 0, and
 /// - `size` is equal to `usize::MAX`.