Merge pull request #9656 from OpenNuvoton/nuvoton_m2351_fix-crypto-ac-mgmt

M2351: Fix crypto AC management

Author: Cruz Monrreal

targets/TARGET_NUVOTON/TARGET_M2351/crypto/crypto-misc.c renamed to targets/TARGET_NUVOTON/TARGET_M2351/crypto/crypto-misc.cpp

@@ -23,29 +23,43 @@
 #include "nu_modutil.h"
 #include "nu_bitutil.h"
 #include "crypto-misc.h"
+#include "platform/SingletonPtr.h"
+#include "platform/PlatformMutex.h"
 
 #if DEVICE_TRNG || defined(MBEDTLS_CONFIG_HW_SUPPORT)
 
-/* NOTE: There's inconsistency in cryptography related naming, Crpt or Crypto. For example, cryptography IRQ
- *       handler could be CRPT_IRQHandler or CRYPTO_IRQHandler. To override default cryptography IRQ handler, see
- *       device/startup_{CHIP}.c for its name or call NVIC_SetVector regardless of its name. */
-void CRPT_IRQHandler();
+/* Consideration for choosing proper synchronization mechanism
+ *
+ * 1. We choose mutex to synchronize access to crypto non-SHA AC. We can guarantee:
+ *    (1) No deadlock
+ *        We just lock mutex for a short sequence of operations rather than the whole lifetime
+ *        of crypto context.
+ *    (2) No priority inversion
+ *        Mutex supports priority inheritance and it is enabled.
+ * 2. We choose atomic flag to synchronize access to crypto SHA AC. We can guarantee:
+ *    (1) No deadlock
+ *        With SHA AC not supporting context save & restore, we provide SHA S/W fallback when
+ *        SHA AC is not available.
+ *    (2) No biting CPU
+ *        Same reason as above.
+ */
+
+/* Mutex for crypto AES AC management */
+static SingletonPtr<PlatformMutex> crypto_aes_mutex;
+
+/* Mutex for crypto DES AC management */
+static SingletonPtr<PlatformMutex> crypto_des_mutex;
+
+/* Mutex for crypto ECC AC management */
+static SingletonPtr<PlatformMutex> crypto_ecc_mutex;
+
+/* Atomic flag for crypto SHA AC management */
+static core_util_atomic_flag crypto_sha_atomic_flag = CORE_UTIL_ATOMIC_FLAG_INIT;
 
-/* Track if AES H/W is available */
-static uint16_t crypto_aes_avail = 1;
-/* Track if DES H/W is available */
-static uint16_t crypto_des_avail = 1;
-/* Track if SHA H/W is available */
-static uint16_t crypto_sha_avail = 1;
-/* Track if ECC H/W is available */
-static uint16_t crypto_ecc_avail = 1;
 
 /* Crypto (AES, DES, SHA, etc.) init counter. Crypto's keeps active as it is non-zero. */
 static uint16_t crypto_init_counter = 0U;
 
-static bool crypto_submodule_acquire(uint16_t *submodule_avail);
-static void crypto_submodule_release(uint16_t *submodule_avail);
-
 /* Crypto done flags */
 #define CRYPTO_DONE_OK              BIT0    /* Done with OK */
 #define CRYPTO_DONE_ERR             BIT1    /* Done with error */
@@ -81,8 +95,7 @@ void crypto_init(void)
          * NOTE: We must call secure version (from non-secure domain) because SYS/CLK regions are secure.
          */
         CLK_EnableModuleClock_S(CRPT_MODULE);
-        
-        NVIC_SetVector(CRPT_IRQn, (uint32_t) CRPT_IRQHandler);
+
         NVIC_EnableIRQ(CRPT_IRQn);
     }
     core_util_critical_section_exit();
@@ -131,44 +144,52 @@ void crypto_zeroize32(uint32_t *v, size_t n)
     }
 }
 
-bool crypto_aes_acquire(void)
+void crypto_aes_acquire(void)
 {
-    return crypto_submodule_acquire(&crypto_aes_avail);
+    /* Don't check return code of Mutex::lock(void)
+     *
+     * This function treats RTOS errors as fatal system errors, so it can only return osOK.
+     * Use of the return value is deprecated, as the return is expected to become void in
+     * the future.
+     */
+    crypto_aes_mutex->lock();
 }
 
 void crypto_aes_release(void)
 {
-    crypto_submodule_release(&crypto_aes_avail);
+    crypto_aes_mutex->unlock();
 }
 
-bool crypto_des_acquire(void)
+void crypto_des_acquire(void)
 {
-    return crypto_submodule_acquire(&crypto_des_avail);
+    /* Don't check return code of Mutex::lock(void) */
+    crypto_des_mutex->lock();
 }
 
 void crypto_des_release(void)
 {
-    crypto_submodule_release(&crypto_des_avail);
+    crypto_des_mutex->unlock();
 }
 
-bool crypto_sha_acquire(void)
+void crypto_ecc_acquire(void)
 {
-    return crypto_submodule_acquire(&crypto_sha_avail);
+    /* Don't check return code of Mutex::lock(void) */
+    crypto_ecc_mutex->lock();
 }
 
-void crypto_sha_release(void)
+void crypto_ecc_release(void)
 {
-    crypto_submodule_release(&crypto_sha_avail);
+    crypto_ecc_mutex->unlock();
 }
 
-bool crypto_ecc_acquire(void)
+bool crypto_sha_try_acquire(void)
 {
-    return crypto_submodule_acquire(&crypto_ecc_avail);
+    return !core_util_atomic_flag_test_and_set(&crypto_sha_atomic_flag);
 }
 
-void crypto_ecc_release(void)
+void crypto_sha_release(void)
 {
-    crypto_submodule_release(&crypto_ecc_avail);
+    core_util_atomic_flag_clear(&crypto_sha_atomic_flag);
 }
 
 void crypto_prng_prestart(void)
@@ -252,18 +273,6 @@ bool crypto_dma_buffs_overlap(const void *in_buff, size_t in_buff_size, const vo
     return overlap;
 }
 
-static bool crypto_submodule_acquire(uint16_t *submodule_avail)
-{
-    uint16_t expectedCurrentValue = 1;
-    return core_util_atomic_cas_u16(submodule_avail, &expectedCurrentValue, 0);
-}
-
-static void crypto_submodule_release(uint16_t *submodule_avail)
-{
-    uint16_t expectedCurrentValue = 0;
-    while (! core_util_atomic_cas_u16(submodule_avail, &expectedCurrentValue, 1));
-}
-
 static void crypto_submodule_prestart(volatile uint16_t *submodule_done)
 {
     *submodule_done = 0;
@@ -296,8 +305,13 @@ static bool crypto_submodule_wait(volatile uint16_t *submodule_done)
     return false;
 }
 
-/* Crypto interrupt handler */
-void CRPT_IRQHandler()
+/* Crypto interrupt handler
+ * 
+ * There's inconsistency in cryptography related naming, Crpt or Crypto. For example,
+ * cryptography IRQ handler could be CRPT_IRQHandler or CRYPTO_IRQHandler. To override
+ * default cryptography IRQ handler, see device/startup_{CHIP}.c for its correct name
+ * or call NVIC_SetVector() in crypto_init() regardless of its name. */
+extern "C" void CRPT_IRQHandler()
 {
     uint32_t intsts;
 

targets/TARGET_NUVOTON/TARGET_M2351/crypto/crypto-misc.h

@@ -66,26 +66,33 @@ void crypto_uninit(void);
 void crypto_zeroize(void *v, size_t n);
 void crypto_zeroize32(uint32_t *v, size_t n);
 
-/* Acquire/release ownership of AES H/W */
-/* NOTE: If "acquire" succeeds, "release" must be done to pair it. */
-bool crypto_aes_acquire(void);
+/* Acquire/release ownership of crypto sub-module
+ * 
+ * \note            "acquire" is blocking until ownership is acquired
+ *
+ * \note            "acquire"/"release" must be paired.
+ *
+ * \note            Recursive "acquire" is allowed because the underlying synchronization
+ *                  primitive mutex supports it.
+ */
+void crypto_aes_acquire(void);
 void crypto_aes_release(void);
-
-/* Acquire/release ownership of DES H/W */
-/* NOTE: If "acquire" succeeds, "release" must be done to pair it. */
-bool crypto_des_acquire(void);
+void crypto_des_acquire(void);
 void crypto_des_release(void);
+void crypto_ecc_acquire(void);
+void crypto_ecc_release(void);
 
-/* Acquire/release ownership of SHA H/W */
-/* NOTE: If "acquire" succeeds, "release" must be done to pair it. */
-bool crypto_sha_acquire(void);
+/* Acquire/release ownership of crypto sub-module
+ * 
+ * \return          false if crytpo sub-module is held by another thread or
+ *                  another mbedtls context.
+ *                  true if successful
+ *
+ * \note            Successful "try_acquire" and "release" must be paired.
+ */
+bool crypto_sha_try_acquire(void);
 void crypto_sha_release(void);
 
-/* Acquire/release ownership of ECC H/W */
-/* NOTE: If "acquire" succeeds, "release" must be done to pair it. */
-bool crypto_ecc_acquire(void);
-void crypto_ecc_release(void);
-
 /* Flow control between crypto/xxx start and crypto/xxx ISR 
  *
  * crypto_xxx_prestart/crypto_xxx_wait encapsulate control flow between crypto/xxx start and crypto/xxx ISR.

targets/TARGET_NUVOTON/TARGET_M480/crypto/crypto-misc.cpp

@@ -19,10 +19,6 @@
 #include "mbed_assert.h"
 #include "mbed_critical.h"
 #include "mbed_error.h"
-#if MBED_CONF_RTOS_PRESENT
-#include "cmsis_os2.h"
-#endif
-#include <string.h>
 #include <limits.h>
 #include "nu_modutil.h"
 #include "nu_bitutil.h"

targets/TARGET_NUVOTON/TARGET_NUC472/crypto/crypto-misc.cpp

@@ -19,10 +19,6 @@
 #include "mbed_assert.h"
 #include "mbed_critical.h"
 #include "mbed_error.h"
-#if MBED_CONF_RTOS_PRESENT
-#include "cmsis_os2.h"
-#endif
-#include <string.h>
 #include <limits.h>
 #include "nu_modutil.h"
 #include "nu_bitutil.h"