ESP32: RSA, SHA fixes

esp-hal/src/rsa/mod.rs

@@ -31,8 +31,8 @@ use crate::{
     asynch::AtomicWaker,
     interrupt::InterruptHandler,
     pac,
-    peripherals::{Interrupt, RSA},
-    system::{Cpu, GenericPeripheralGuard, Peripheral as PeripheralEnable},
+    peripherals::RSA,
+    system::{GenericPeripheralGuard, Peripheral as PeripheralEnable},
     trm_markdown_link,
     work_queue::{self, Status, VTable, WorkQueue, WorkQueueDriver, WorkQueueFrontend},
 };
@@ -41,9 +41,7 @@ use crate::{
 pub struct Rsa<'d, Dm: DriverMode> {
     rsa: RSA<'d>,
     phantom: PhantomData<Dm>,
-    #[cfg(not(rsa_version = "1"))]
-    _memory_guard: RsaMemoryPowerGuard,
-    _guard: GenericPeripheralGuard<{ PeripheralEnable::Rsa as u8 }>,
+    _guard: RsaGuard,
 }
 
 // There are two distinct peripheral versions: ESP32, and all else. There is a naming split in the
@@ -56,25 +54,26 @@ pub struct Rsa<'d, Dm: DriverMode> {
 /// bits, or 16 words.
 const WORDS_PER_INCREMENT: u32 = property!("rsa.size_increment") / 32;
 
-#[cfg(not(rsa_version = "1"))]
-struct RsaMemoryPowerGuard;
+struct RsaGuard {
+    _guard: GenericPeripheralGuard<{ PeripheralEnable::Rsa as u8 }>,
+}
 
-#[cfg(not(rsa_version = "1"))]
-impl RsaMemoryPowerGuard {
+impl RsaGuard {
     fn new() -> Self {
+        let _guard = GenericPeripheralGuard::new();
+        #[cfg(not(rsa_version = "1"))]
         crate::peripherals::SYSTEM::regs()
             .rsa_pd_ctrl()
             .modify(|_, w| {
                 w.rsa_mem_force_pd().clear_bit();
                 w.rsa_mem_force_pu().set_bit();
                 w.rsa_mem_pd().clear_bit()
             });
-        Self
+        Self { _guard }
     }
 }
 
-#[cfg(not(rsa_version = "1"))]
-impl Drop for RsaMemoryPowerGuard {
+impl Drop for RsaGuard {
     fn drop(&mut self) {
         unsafe {
             // Stopping the peripheral's clock source pends an interrupt. Since the clocks
@@ -83,6 +82,7 @@ impl Drop for RsaMemoryPowerGuard {
             // To prevent this, we disable interrupts manually before stopping the peripheral.
             crate::peripherals::RSA::steal().disable_peri_interrupt_on_all_cores();
         }
+        #[cfg(not(rsa_version = "1"))]
         crate::peripherals::SYSTEM::regs()
             .rsa_pd_ctrl()
             .modify(|_, w| {
@@ -98,14 +98,10 @@ impl<'d> Rsa<'d, Blocking> {
     ///
     /// Optionally an interrupt handler can be bound.
     pub fn new(rsa: RSA<'d>) -> Self {
-        let guard = GenericPeripheralGuard::new();
-
         let this = Self {
             rsa,
             phantom: PhantomData,
-            #[cfg(not(rsa_version = "1"))]
-            _memory_guard: RsaMemoryPowerGuard::new(),
-            _guard: guard,
+            _guard: RsaGuard::new(),
         };
 
         while !this.ready() {}
@@ -121,8 +117,6 @@ impl<'d> Rsa<'d, Blocking> {
         Rsa {
             rsa: self.rsa,
             phantom: PhantomData,
-            #[cfg(not(rsa_version = "1"))]
-            _memory_guard: self._memory_guard,
             _guard: self._guard,
         }
     }
@@ -161,12 +155,9 @@ impl<'d> Rsa<'d, Async> {
         self.internal_enable_disable_interrupt(false);
         self.rsa.disable_peri_interrupt_on_all_cores();
 
-        crate::interrupt::disable(Cpu::current(), Interrupt::RSA);
         Rsa {
             rsa: self.rsa,
             phantom: PhantomData,
-            #[cfg(not(rsa_version = "1"))]
-            _memory_guard: self._memory_guard,
             _guard: self._guard,
         }
     }
@@ -836,9 +827,7 @@ impl<'d> RsaBackend<'d> {
                 let driver = Rsa {
                     rsa: unsafe { self.peri.clone_unchecked() },
                     phantom: PhantomData,
-                    #[cfg(not(rsa_version = "1"))]
-                    _memory_guard: RsaMemoryPowerGuard::new(),
-                    _guard: GenericPeripheralGuard::new(),
+                    _guard: RsaGuard::new(),
                 };
                 self.state = RsaBackendState::Initializing(driver);
                 work_queue::Poll::Pending(true)

esp-hal/src/sha.rs

@@ -1105,6 +1105,16 @@ impl<'t, 'd> ShaWorkQueueDriver<'t, 'd> {
     }
 }
 
+// ESP32 can't save (or rather, restore) the hash context, so we need to fall back to software
+// if a context already uses the hardware. Also we must not power down the hardware when a
+// context uses it, as it corrupts state.
+#[cfg(esp32)]
+#[expect(clippy::large_enum_variant)]
+enum Esp32Hasher {
+    Hardware(GenericPeripheralGuard<{ crate::system::Peripheral::Sha as u8 }>),
+    Software(SoftwareHasher),
+}
+
 #[cfg(esp32)]
 enum SoftwareHasher {
     Sha1(sha1::Sha1),
@@ -1122,10 +1132,8 @@ struct ShaContext<const CHUNK_BYTES: usize, const DIGEST_WORDS: usize> {
     #[cfg(not(esp32))] // Saved H_MEM registers
     state: [u32; 16],
 
-    // ESP32 can't save (or rather, restore) the hash context, so we need to fall back to software
-    // if a context already uses the hardware.
     #[cfg(esp32)]
-    use_software: Option<SoftwareHasher>,
+    hasher: Esp32Hasher,
 }
 
 // ESP32 can't save the context, so it can't support interleaved operation. To support the digest
@@ -1143,16 +1151,16 @@ static ACCELERATOR_IN_USE: AtomicBool = AtomicBool::new(false);
 impl<const CHUNK_BYTES: usize, const DIGEST_WORDS: usize> ShaContext<CHUNK_BYTES, DIGEST_WORDS> {
     fn new(algorithm: ShaAlgorithmKind) -> Self {
         #[cfg(esp32)]
-        let use_software = if ACCELERATOR_IN_USE.swap(true, Ordering::SeqCst) {
+        let hasher = if ACCELERATOR_IN_USE.swap(true, Ordering::SeqCst) {
             let hasher = match algorithm {
                 ShaAlgorithmKind::Sha1 => SoftwareHasher::Sha1(sha1::Sha1::new()),
                 ShaAlgorithmKind::Sha256 => SoftwareHasher::Sha256(sha2::Sha256::new()),
                 ShaAlgorithmKind::Sha384 => SoftwareHasher::Sha384(sha2::Sha384::new()),
                 ShaAlgorithmKind::Sha512 => SoftwareHasher::Sha512(sha2::Sha512::new()),
             };
-            Some(hasher)
+            Esp32Hasher::Software(hasher)
         } else {
-            None
+            Esp32Hasher::Hardware(GenericPeripheralGuard::new())
         };
 
         Self {
@@ -1170,7 +1178,7 @@ impl<const CHUNK_BYTES: usize, const DIGEST_WORDS: usize> ShaContext<CHUNK_BYTES
             state: [0; 16],
 
             #[cfg(esp32)]
-            use_software,
+            hasher,
         }
     }
 
@@ -1181,7 +1189,7 @@ impl<const CHUNK_BYTES: usize, const DIGEST_WORDS: usize> ShaContext<CHUNK_BYTES
             data.len()
         );
         #[cfg(esp32)]
-        if let Some(hasher) = self.use_software.as_mut() {
+        if let Esp32Hasher::Software(ref mut hasher) = self.hasher {
             Self::update_using_software(hasher, data);
             return ShaHandle(self.frontend.post_completed(&SHA_WORK_QUEUE));
         }
@@ -1232,7 +1240,7 @@ impl<const CHUNK_BYTES: usize, const DIGEST_WORDS: usize> ShaContext<CHUNK_BYTES
             result.len()
         );
         #[cfg(esp32)]
-        if let Some(hasher) = self.use_software.as_mut() {
+        if let Esp32Hasher::Software(ref mut hasher) = self.hasher {
             Self::finalize_using_software(hasher, result);
             return ShaHandle(self.frontend.post_completed(&SHA_WORK_QUEUE));
         }
@@ -1290,7 +1298,9 @@ impl<const CHUNK_BYTES: usize, const DIGEST_WORDS: usize> Drop
     for ShaContext<CHUNK_BYTES, DIGEST_WORDS>
 {
     fn drop(&mut self) {
-        ACCELERATOR_IN_USE.store(false, Ordering::Release);
+        if matches!(self.hasher, Esp32Hasher::Hardware(_)) {
+            ACCELERATOR_IN_USE.store(false, Ordering::Release);
+        }
     }
 }
 

hil-test/src/bin/crypto/sha.rs

@@ -462,6 +462,9 @@ mod tests {
     /// interleaving. This specifically tests the SHA backend implementation
     #[test]
     fn test_for_digest_rolling_context_interleaved(_ctx: Context) {
+        // Drop the Sha driver so that the backend can release resources when it needs to.
+        core::mem::drop(_ctx.sha);
+
         let mut sha_backend = ShaBackend::new(unsafe { esp_hal::peripherals::SHA::steal() });
         let _sha_driver = sha_backend.start();