fix priority inversion issues

hifive1-test/examples/clint.rs

@@ -101,7 +101,13 @@ fn main() -> ! {
     }
     //let mut delay = CLINT::delay();
     loop {
+        //  read stack pointer
+        let pc: usize;
+        unsafe {
+            core::arch::asm!("mv {}, sp", out(reg) pc);
+        }
+        sprintln!("Program counter: {:x}", pc);
         sprintln!("Going to sleep!");
-        unsafe { riscv_slic::riscv::asm::wfi() };
+        riscv_slic::riscv::asm::wfi();
     }
 }

riscv-slic-macros/src/api.rs

@@ -19,18 +19,45 @@ pub fn api_mod() -> TokenStream {
         /// # Safety
         ///
         /// This function is only for `riscv-slic` internal use. Do not call it directly.
+        ///
+        /// Setting the priority threshold to a value lower than the current threshold
+        /// may lead to priority inversion. If you want to make sure that the threshold
+        /// is only raised, use the [`__riscv_slic_raise_threshold`] function instead.
         #[inline]
         #[no_mangle]
         pub unsafe fn __riscv_slic_set_threshold(thresh: u8) {
             critical_section::with(|cs| {
                 let mut slic = __SLIC.borrow_ref_mut(cs);
                 slic.set_threshold(thresh);
+                // trigger a software interrupt if the SLIC is still ready at this point
                 if slic.is_ready() {
                     __riscv_slic_swi_pend();
                 }
             });
         }
 
+        /// Raises the priority threshold of the SLIC only if the new threshold is higher than the current one.
+        ///
+        /// # Safety
+        ///
+        /// This function is only for `riscv-slic` internal use. Do not call it directly.
+        ///
+        /// This function is thought to be used as a way to temporarily raise the priority threshold.
+        /// You must return the previous threshold to the SLIC after you are done.
+        #[inline]
+        #[no_mangle]
+        pub unsafe fn __riscv_slic_raise_threshold(priority: u8) -> Result<u8, ()> {
+            critical_section::with(|cs| {
+                let mut slic = __SLIC.borrow_ref_mut(cs);
+                let res = slic.raise_threshold(priority);
+                // trigger a software interrupt if the SLIC is still ready at this point
+                if slic.is_ready() {
+                    __riscv_slic_swi_pend();
+                }
+                res
+            })
+        }
+
         /// Returns the interrupt priority of a given software interrupt source.
         ///
         /// # Safety
@@ -79,9 +106,24 @@ pub fn api_mod() -> TokenStream {
         /// This function is only for `riscv-slic` internal use. Do not call it directly.
         #[inline]
         #[no_mangle]
-        pub unsafe fn __riscv_slic_run() {
-            if let Some((pri, int)) = critical_section::with(|cs| __SLIC.borrow_ref_mut(cs).pop()) {
-                run(pri, || __SOFTWARE_INTERRUPTS[int as usize]());
+        pub unsafe fn __riscv_slic_pop() {
+            // We check if there are pending software interrupts and run them
+            // Note that we must raise the threshold within the same critical section
+            // to avoid corner cases where another interrupt is raised in between.
+            if let Some((prev, int)) = critical_section::with(|cs| {
+                let mut slic = __SLIC.borrow_ref_mut(cs);
+                match slic.pop() {
+                    Some((priority, interrupt)) => {
+                        // SAFETY: we restore the previous threshold after the function is done
+                        let previous = unsafe { slic.raise_threshold(priority).unwrap() }; // must be Ok if pop returned Some!
+                        Some((previous, interrupt))
+                    }
+                    None => None,
+                }
+            }) {
+                __SOFTWARE_INTERRUPTS[int as usize]();
+                // SAFETY: we restore the previous threshold after the function is done
+                unsafe { __riscv_slic_set_threshold(prev) };
             }
         }
     )

riscv-slic-macros/src/swi.rs

@@ -76,7 +76,7 @@ pub fn swi_mod(input: &CodegenInput) -> TokenStream {
         #[allow(non_snake_case)]
         #swi_handler_signature {
             __riscv_slic_swi_unpend();
-            riscv::interrupt::nested(|| unsafe { __riscv_slic_run() });
+            riscv::interrupt::nested(|| unsafe { __riscv_slic_pop() });
         }
     ));
     quote!(#(#res)*)

riscv-slic/src/api.rs

@@ -7,8 +7,9 @@ use riscv::register::sie::{clear_ssoft as disable_swi, set_ssoft as enable_swi};
 
 extern "Rust" {
     fn __riscv_slic_swi_unpend();
-    fn __riscv_slic_set_threshold(priority: u8);
     fn __riscv_slic_get_threshold() -> u8;
+    fn __riscv_slic_set_threshold(priority: u8);
+    fn __riscv_slic_raise_threshold(priority: u8) -> Result<u8, ()>;
     fn __riscv_slic_get_priority(interrupt: u16) -> u8;
     fn __riscv_slic_set_priority(interrupt: u16, priority: u8);
     fn __riscv_slic_pend(interrupt: u16);
@@ -27,7 +28,7 @@ pub fn clear_interrupts() {
     unsafe {
         disable_swi();
         __riscv_slic_swi_unpend();
-        set_threshold(u8::MAX);
+        __riscv_slic_set_threshold(u8::MAX);
     }
 }
 
@@ -44,32 +45,27 @@ pub fn clear_interrupts() {
 /// This function may break mask-based critical sections.
 #[inline]
 pub unsafe fn set_interrupts() {
-    set_threshold(0);
+    __riscv_slic_set_threshold(0);
     enable_swi();
 }
 
-/// Stabilized API for changing the threshold of the SLIC.
-///
-/// # Safety
-///
-/// Changing the priority threshold may break mask-based critical sections.
-#[inline]
-pub unsafe fn set_threshold(priority: u8) {
-    __riscv_slic_set_threshold(priority);
-}
-
 /// Stabilized API for getting the current threshold of the SLIC.
 #[inline]
 pub fn get_threshold() -> u8 {
     // SAFETY: this read has no side effects.
     unsafe { __riscv_slic_get_threshold() }
 }
 
-/// Stabilized API for getting the priority of a given software interrupt source.
+/// Stabilized API for setting the threshold of the SLIC.
+///
+/// # Safety
+///
+/// Setting the priority threshold to a value lower than the current threshold
+/// may lead to priority inversion. If you want to make sure that the threshold
+/// is only raised, use the [`raise_threshold`] function instead.
 #[inline]
-pub fn get_priority<I: crate::InterruptNumber>(interrupt: I) -> u8 {
-    // SAFETY: this read has no side effects.
-    unsafe { __riscv_slic_get_priority(interrupt.number()) }
+pub unsafe fn set_threshold(priority: u8) {
+    __riscv_slic_set_threshold(priority);
 }
 
 /// Stabilized API for setting the priority of a software interrupt of the SLIC.
@@ -85,37 +81,40 @@ pub unsafe fn set_priority<I: crate::InterruptNumber>(interrupt: I, priority: u8
 /// Stabilized API for pending a software interrupt on the SLIC.
 #[inline]
 pub fn pend<I: crate::InterruptNumber>(interrupt: I) {
-    // SAFETY: TODO
+    // SAFETY: it is safe to pend a software interrupt
     unsafe { __riscv_slic_pend(interrupt.number()) };
 }
 
 /// Runs a function with priority mask.
-///
-/// # Safety
-///
-/// If new priority is less than current priority, priority inversion may occur.
 #[inline]
-pub unsafe fn run<F: FnOnce()>(priority: u8, f: F) {
-    let current = get_threshold();
-    set_threshold(priority);
+pub fn run<F: FnOnce()>(priority: u8, f: F) {
+    // SAFETY: we restore the previous threshold after the function is done
+    let previous = unsafe { __riscv_slic_raise_threshold(priority) };
     f();
-    set_threshold(current);
+    if let Ok(prev) = previous {
+        // SAFETY: we restore the previous threshold after the function is done
+        unsafe { __riscv_slic_set_threshold(prev) };
+    }
 }
 
 /// Runs a function that takes a shared resource with a priority ceiling.
 /// This function returns the return value of the target function.
 ///
 /// # Safety
 ///
-/// If ceiling is less than current priority, priority inversion may occur.
+/// Input argument `ptr` must be a valid pointer to a shared resource.
 #[inline]
 pub unsafe fn lock<F, T, R>(ptr: *mut T, ceiling: u8, f: F) -> R
 where
     F: FnOnce(&mut T) -> R,
 {
-    let current = get_threshold();
-    set_threshold(ceiling);
+    // SAFETY: we restore the previous threshold after the function is done
+    let previous = unsafe { __riscv_slic_raise_threshold(ceiling) };
+    // SAFETY: provided that caller respects the safety requirements, this is safe
     let r = f(&mut *ptr);
-    set_threshold(current);
+    if let Ok(prev) = previous {
+        // SAFETY: we restore the previous threshold after the function is done
+        unsafe { __riscv_slic_set_threshold(prev) };
+    }
     r
 }