Revise usage of mutexes

firmware/acid-firmware/src/console.rs

@@ -18,11 +18,11 @@ impl embedded_io::ErrorType for Writer {
 
 impl embedded_io::Write for Writer {
     fn write(&mut self, buf: &[u8]) -> Result<usize, Self::Error> {
-        with_uart_tx(|_, uart| uart.write(buf))
+        with_uart_tx(|uart| uart.write(buf))
     }
 
     fn flush(&mut self) -> Result<(), Self::Error> {
-        with_uart_tx(|_, uart| uart.flush())
+        with_uart_tx(|uart| uart.flush())
     }
 }
 

firmware/acid-firmware/src/ffi/crypto.rs

@@ -3,7 +3,6 @@ use core::{
     ffi::{c_char, c_int, c_size_t, c_uchar, c_ulonglong},
 };
 
-use critical_section::Mutex;
 use embassy_sync::blocking_mutex::{self, raw::CriticalSectionRawMutex};
 use hmac::digest::{FixedOutput, KeyInit, Update};
 use password_hash::Key;
@@ -39,7 +38,8 @@ unsafe extern "C" fn __spre_crypto_hash_sha256(
 
 /// This is the encrypted user key currently being used in the key derivation function of spectre.
 /// It decrypts using the user's password into the key that would be derived with the original password hashing function.
-pub static ACTIVE_ENCRYPTED_USER_KEY: Mutex<Cell<Key>> = Mutex::new(Cell::new([0; _]));
+pub static ACTIVE_ENCRYPTED_USER_KEY: blocking_mutex::Mutex<CriticalSectionRawMutex, Cell<Key>> =
+    blocking_mutex::Mutex::new(Cell::new([0; _]));
 
 #[unsafe(no_mangle)]
 #[must_use]
@@ -65,7 +65,7 @@ unsafe extern "C" fn __spre_crypto_pwhash_scryptsalsa208sha256_ll(
     };
 
     let output: &mut [u8] = unsafe { core::slice::from_raw_parts_mut(output, output_len) };
-    let mut user_key = critical_section::with(|cs| ACTIVE_ENCRYPTED_USER_KEY.borrow(cs).get());
+    let mut user_key = ACTIVE_ENCRYPTED_USER_KEY.lock(|user_key| user_key.get());
 
     password_hash::decrypt_with(&mut user_key, &encryption_key);
     output.copy_from_slice(&user_key);

firmware/acid-firmware/src/logging.rs

@@ -69,28 +69,29 @@ pub mod usb {
 #[cfg(feature = "alt-log")]
 #[macro_use]
 pub mod uart {
+    use crate::util::MutexExt;
+
     use super::*;
 
     use core::{cell::RefCell, fmt::Write};
-    use critical_section::{CriticalSection, Mutex};
+    use embassy_sync::{blocking_mutex::raw::CriticalSectionRawMutex, mutex::Mutex};
-    use esp_hal::{
+    use esp_hal::{Blocking, uart::UartTx};
-        Blocking,
-        uart::UartTx,
-    };
     use log::{Log, info};
 
-    static ALT_LOGGER_UART: Mutex<RefCell<Option<UartTx<'static, Blocking>>>> =
+    static ALT_LOGGER_UART: Mutex<
-        Mutex::new(RefCell::new(None));
+        CriticalSectionRawMutex,
+        RefCell<Option<UartTx<'static, Blocking>>>,
+    > = Mutex::new(RefCell::new(None));
 
-    pub fn with_uart_tx<R>(
+    pub fn with_uart_tx<R>(f: impl FnOnce(&'_ mut UartTx<'static, Blocking>) -> R) -> R {
-        f: impl FnOnce(CriticalSection<'_>, &'_ mut UartTx<'static, Blocking>) -> R,
+        // Safety:
-    ) -> R {
+        // * The guard is not held across yield points.
-        critical_section::with(|cs| {
+        // * **CARE MUST BE TAKEN NOT TO INVOKE THIS FUNCTION FROM AN INTERRUPT HANDLER.**
-            let mut uart = ALT_LOGGER_UART.borrow(cs).borrow_mut();
+        let uart = unsafe { ALT_LOGGER_UART.lock_blocking() };
+        let mut uart = uart.borrow_mut();
         let uart = uart.as_mut().unwrap();
 
-            (f)(cs, uart)
+        (f)(uart)
-        })
     }
 
     #[allow(unused)]
@@ -106,7 +107,7 @@ pub mod uart {
 
     #[allow(unused)]
     fn do_print(args: core::fmt::Arguments<'_>) {
-        with_uart_tx(|_, uart| {
+        with_uart_tx(|uart| {
             uart.write_fmt(format_args!("{}\n", args)).unwrap();
             uart.flush().unwrap();
         })
@@ -123,7 +124,7 @@ pub mod uart {
 
         #[allow(unused)]
         fn log(&self, record: &log::Record) {
-            with_uart_tx(|cs, uart| {
+            with_uart_tx(|uart| {
                 with_formatted_log_record(record, |args| uart.write_fmt(args)).unwrap();
                 uart.flush().unwrap();
             })
@@ -154,9 +155,13 @@ pub mod uart {
     }
 
     pub fn setup_logging(uart_tx: UartTx<'static, Blocking>) {
-        critical_section::with(|cs| {
+        {
-            *ALT_LOGGER_UART.borrow(cs).borrow_mut() = Some(uart_tx);
+            // Safety:
-        });
+            // * The guard is not held across yield points.
+            // * This function is not invoked from an interrupt handler.
+            let uart = unsafe { ALT_LOGGER_UART.lock_blocking() };
+            *uart.borrow_mut() = Some(uart_tx);
+        }
 
         unsafe {
             log::set_logger_racy(&UartLogger).unwrap();

firmware/acid-firmware/src/main.rs

@@ -30,6 +30,7 @@ use alloc::vec::Vec;
 use embassy_embedded_hal::adapter::BlockingAsync;
 use embassy_embedded_hal::flash::partition::Partition;
 use embassy_executor::Spawner;
+use embassy_sync::blocking_mutex;
 use embassy_sync::blocking_mutex::raw::CriticalSectionRawMutex;
 use embassy_sync::channel::Channel;
 use embassy_sync::mutex::Mutex;
@@ -753,7 +754,7 @@ async fn main_task(peripherals: MainPeripherals) {
                     window: RefCell::new(None),
                     swapchain: RefCell::new(swapchain_writer),
                     quit_event_loop: Default::default(),
-                    events: Arc::new(critical_section::Mutex::new(RefCell::new(VecDeque::new()))),
+                    events: Arc::new(blocking_mutex::Mutex::new(RefCell::new(VecDeque::new()))),
                 };
                 spawner.must_spawn(ui::run_renderer_task(slint_backend, flash_part_acid));
             });

firmware/acid-firmware/src/peripherals/st7701s/mod.rs

@@ -1276,7 +1276,7 @@ where
             //
             // Adafruit would use 11 MHz.
             // I had lowered the frequency, so that `DmaBounce` could keep up.
-            .with_frequency(Rate::from_mhz(5)) // From Adafruit
+            .with_frequency(Rate::from_mhz(6)) // From Adafruit
             .with_clock_mode(ClockMode {
                 polarity: Polarity::IdleLow, // From Adafruit
                 phase: Phase::ShiftHigh,     // From Adafruit

firmware/acid-firmware/src/ui/backend.rs

@@ -7,7 +7,7 @@ use core::{
 use alloc::{
     boxed::Box, collections::vec_deque::VecDeque, rc::Rc, string::ToString, sync::Arc, vec::Vec,
 };
-use critical_section::Mutex;
+use embassy_sync::blocking_mutex::{self, raw::CriticalSectionRawMutex};
 use esp_hal::time::Instant;
 use esp_hal_bounce_buffers::SwapchainWriter;
 use log::{debug, info};
@@ -29,7 +29,9 @@ pub struct SlintBackend {
     pub window: RefCell<Option<Rc<SoftwareWindowAdapter>>>,
     pub swapchain: RefCell<SwapchainWriter>,
     pub quit_event_loop: Arc<AtomicBool>,
-    pub events: Arc<Mutex<RefCell<VecDeque<Box<dyn FnOnce() + Send>>>>>,
+    pub events: Arc<
+        blocking_mutex::Mutex<CriticalSectionRawMutex, RefCell<VecDeque<Box<dyn FnOnce() + Send>>>>,
+    >,
 }
 
 impl slint::platform::Platform for SlintBackend {
@@ -66,13 +68,9 @@ impl slint::platform::Platform for SlintBackend {
 
         /* loop */
         {
-            let drained_events = critical_section::with(|cs| {
+            let drained_events = self
-                self.events
+                .events
-                    .borrow(cs)
+                .lock(|events| events.borrow_mut().drain(..).collect::<Vec<_>>());
-                    .borrow_mut()
-                    .drain(..)
-                    .collect::<Vec<_>>()
-            });
 
             for event in drained_events {
                 (event)();
@@ -146,7 +144,9 @@ impl slint::platform::Platform for SlintBackend {
 
 struct AcidEventLoopProxy {
     pub quit_event_loop: Arc<AtomicBool>,
-    pub events: Arc<Mutex<RefCell<VecDeque<Box<dyn FnOnce() + Send>>>>>,
+    pub events: Arc<
+        blocking_mutex::Mutex<CriticalSectionRawMutex, RefCell<VecDeque<Box<dyn FnOnce() + Send>>>>,
+    >,
 }
 
 impl EventLoopProxy for AcidEventLoopProxy {
@@ -159,8 +159,8 @@ impl EventLoopProxy for AcidEventLoopProxy {
         &self,
         event: Box<dyn FnOnce() + Send>,
     ) -> Result<(), EventLoopError> {
-        critical_section::with(|cs| {
+        self.events.lock(|events| {
-            self.events.borrow(cs).borrow_mut().push_back(event);
+            events.borrow_mut().push_back(event);
         });
         Ok(())
     }

firmware/acid-firmware/src/ui/mod.rs

@@ -61,8 +61,8 @@ fn spectre_derive_user_key(
     encrypted_key: Option<Key>,
 ) -> SpectreUserKey {
     if let Some(encrypted_key) = encrypted_key {
-        critical_section::with(|cs| {
+        ACTIVE_ENCRYPTED_USER_KEY.lock(|user_key| {
-            ACTIVE_ENCRYPTED_USER_KEY.borrow(cs).set(encrypted_key);
+            user_key.set(encrypted_key);
         });
     }
 

firmware/acid-firmware/src/util.rs

@@ -1,5 +1,9 @@
 use core::fmt::Display;
 
+use embassy_sync::{
+    blocking_mutex::raw::RawMutex,
+    mutex::{Mutex, MutexGuard, TryLockError},
+};
 use embassy_time::Duration;
 
 pub struct FormattedDuration<'a>(&'a Duration);
@@ -46,3 +50,36 @@ pub const fn get_file_name(path: &str) -> &str {
         Err(_) => panic!("Failed to extract the file name from a path."),
     }
 }
+
+pub trait MutexExt<M, T> {
+    type Guard<'a>
+    where
+        M: 'a,
+        T: 'a,
+        Self: 'a;
+
+    /// Locks the mutex, while entering a critical section only during locking and unlocking.
+    ///
+    /// # Safety:
+    /// * The guard must not be kept across async `yield` points.
+    /// * This must not be called from within interrupt handlers.
+    /// Otherwise, a deadlock might occur.
+    unsafe fn lock_blocking(&self) -> Self::Guard<'_>;
+}
+
+impl<M: RawMutex, T> MutexExt<M, T> for Mutex<M, T> {
+    type Guard<'a>
+        = MutexGuard<'a, M, T>
+    where
+        M: 'a,
+        T: 'a,
+        Self: 'a;
+
+    unsafe fn lock_blocking(&self) -> Self::Guard<'_> {
+        loop {
+            if let Ok(guard) = self.try_lock() {
+                return guard;
+            }
+        }
+    }
+}