[Nuvoton] Fix crypto AC management

ccli8 · ccli8 · commit ca4467508642 · 2018-12-19T10:17:39.000+08:00
1. For SHA AC, use atomic flag to manage its ownership.
   (1) Nuvoton SHA AC doesn't support SHA context save &amp; restore, so S/W
       SHA fallback has been supported before. To make non-blocking 'acquire'
       semantics clearer, introduce 'try_acquire' to substitute for 'acquire'.
   (2) No biting CPU due to mechanism above.
   (3) No deadlock due to mechanism above.
2. For AES/DES/ECC AC, change to mutex to manage their ownership.
   (1) Change crypto-misc.c to crypto-misc.cpp to utilize C++ SingletonPtr
       which guarantees thread-safe mutex construct-on-first-use.
   (2) With change to crypto-misc.cpp, add 'extern "C"' modifier to CRYPTO_IRQHandler()
       to avoid name mangling in C++.
   (3) No priority inversion because mutex has osMutexPrioInherit attribute
       bit set.
   (4) No deadlock because these AC are all locked for a short sequence
       of operations rather than the whole lifetime of mbedtls context.
   (5) For double mbedtls_internal_ecp_init() issue, it has been fixed in upper
       mbedtls layer. So no need to change ecc init/free flow.
diff --git a/features/mbedtls/targets/TARGET_NUVOTON/TARGET_M480/aes/aes_alt.c b/features/mbedtls/targets/TARGET_NUVOTON/TARGET_M480/aes/aes_alt.c
@@ -144,10 +144,9 @@ static void __nvt_aes_crypt( mbedtls_aes_context *ctx,
         error("Buffer for AES alter. DMA requires to be word-aligned and located in 0x20000000-0x2FFFFFFF region.");
     }
 
-    /* TODO: Change busy-wait to other means to release CPU */
     /* Acquire ownership of AES H/W */
-    while (! crypto_aes_acquire());
-    
+    crypto_aes_acquire();
+
     /* Init crypto module */
     crypto_init();
     /* Enable AES interrupt */
diff --git a/features/mbedtls/targets/TARGET_NUVOTON/TARGET_M480/des/des_alt.c b/features/mbedtls/targets/TARGET_NUVOTON/TARGET_M480/des/des_alt.c
@@ -349,10 +349,9 @@ static int mbedtls_des_docrypt(uint16_t keyopt, uint8_t key[3][MBEDTLS_DES_KEY_S
         error("Buffer for DES alter. DMA requires to be word-aligned and located in 0x20000000-0x2FFFFFFF region.");
     }
     
-    /* TODO: Change busy-wait to other means to release CPU */
     /* Acquire ownership of DES H/W */
-    while (! crypto_des_acquire());
-    
+    crypto_des_acquire();
+
     /* Init crypto module */
     crypto_init();
     /* Enable DES interrupt */
diff --git a/features/mbedtls/targets/TARGET_NUVOTON/TARGET_M480/ecp/ecp_internal_alt.c b/features/mbedtls/targets/TARGET_NUVOTON/TARGET_M480/ecp/ecp_internal_alt.c
@@ -223,12 +223,23 @@ unsigned char mbedtls_internal_ecp_grp_capable( const mbedtls_ecp_group *grp )
 
 int mbedtls_internal_ecp_init( const mbedtls_ecp_group *grp )
 {
-    /* TODO: Change busy-wait with other means to release CPU */
+    /* Behavior of mbedtls_internal_ecp_init()/mbedtls_internal_ecp_free()
+     *
+     * mbedtls_internal_ecp_init()/mbedtls_internal_ecp_free() are like pre-op/post-op calls
+     * and they guarantee:
+     *
+     * 1. Paired
+     * 2. No overlapping
+     * 3. Upper public function cannot return when ECP alter. is still activated.
+     */
+    
     /* Acquire ownership of ECC accelerator */
-    while (! crypto_ecc_acquire());
+    crypto_ecc_acquire();
     
-    /* Init crypto module */
+    /* Initialize crypto module */
     crypto_init();
+    
+    /* Enable ECC interrupt */
     ECC_ENABLE_INT();
 
     return 0;
@@ -238,9 +249,10 @@ void mbedtls_internal_ecp_free( const mbedtls_ecp_group *grp )
 {
     /* Disable ECC interrupt */
     ECC_DISABLE_INT();
+
     /* Uninit crypto module */
     crypto_uninit();
-    
+
     /* Release ownership of ECC accelerator */
     crypto_ecc_release();
 }
@@ -590,7 +602,7 @@ NU_STATIC int internal_run_eccop(const mbedtls_ecp_group *grp,
         ret = MBEDTLS_ERR_ECP_BAD_INPUT_DATA;
         goto cleanup;
     }
-    
+
     /* Configure ECC curve coefficients A/B */
     /* Special case for A = -3 */
     if (grp->A.p == NULL) {
@@ -644,7 +656,7 @@ NU_STATIC int internal_run_eccop(const mbedtls_ecp_group *grp,
 cleanup:
 
     mbedtls_mpi_free(&N_);
-    
+
     return ret;
 }
 
@@ -698,7 +710,7 @@ NU_STATIC int internal_run_modop(mbedtls_mpi *r,
     const mbedtls_mpi *Np;
     
     mbedtls_mpi_init(&N_);
-    
+
     /* Use INTERNAL_MPI_NORM(Np, N1, N_, P) to get normalized MPI
      *
      * N_: Holds normalized MPI if the passed-in MPI N1 is not
diff --git a/features/mbedtls/targets/TARGET_NUVOTON/TARGET_M480/sha/sha1_alt.c b/features/mbedtls/targets/TARGET_NUVOTON/TARGET_M480/sha/sha1_alt.c
@@ -31,7 +31,7 @@
  */
 static void mbedtls_sha1_init_internal(mbedtls_sha1_context *ctx, int try_hw)
 {
-    if (try_hw && crypto_sha_acquire()) {
+    if (try_hw && crypto_sha_try_acquire()) {
         ctx->active_ctx = &ctx->hw_ctx;
         mbedtls_sha1_hw_init(&ctx->hw_ctx);
     } else {
diff --git a/features/mbedtls/targets/TARGET_NUVOTON/TARGET_M480/sha/sha256_alt.c b/features/mbedtls/targets/TARGET_NUVOTON/TARGET_M480/sha/sha256_alt.c
@@ -31,7 +31,7 @@
  */
 static void mbedtls_sha256_init_internal(mbedtls_sha256_context *ctx, int try_hw)
 {
-    if (try_hw && crypto_sha_acquire()) {
+    if (try_hw && crypto_sha_try_acquire()) {
         ctx->active_ctx = &ctx->hw_ctx;
         mbedtls_sha256_hw_init(&ctx->hw_ctx);
     } else {
diff --git a/features/mbedtls/targets/TARGET_NUVOTON/TARGET_M480/sha/sha512_alt.c b/features/mbedtls/targets/TARGET_NUVOTON/TARGET_M480/sha/sha512_alt.c
@@ -31,7 +31,7 @@
  */
 static void mbedtls_sha512_init_internal(mbedtls_sha512_context *ctx, int try_hw)
 {
-    if (try_hw && crypto_sha_acquire()) {
+    if (try_hw && crypto_sha_try_acquire()) {
         ctx->active_ctx = &ctx->hw_ctx;
         mbedtls_sha512_hw_init(&ctx->hw_ctx);
     } else {
diff --git a/features/mbedtls/targets/TARGET_NUVOTON/TARGET_NUC472/aes/aes_alt.c b/features/mbedtls/targets/TARGET_NUVOTON/TARGET_NUC472/aes/aes_alt.c
@@ -144,10 +144,9 @@ static void __nvt_aes_crypt( mbedtls_aes_context *ctx,
         error("Buffer for AES alter. DMA requires to be word-aligned and located in 0x20000000-0x2FFFFFFF region.");
     }
 
-    /* TODO: Change busy-wait to other means to release CPU */
     /* Acquire ownership of AES H/W */
-    while (! crypto_aes_acquire());
-    
+    crypto_aes_acquire();
+
     /* Init crypto module */
     crypto_init();
     /* Enable AES interrupt */
diff --git a/features/mbedtls/targets/TARGET_NUVOTON/TARGET_NUC472/des/des_alt.c b/features/mbedtls/targets/TARGET_NUVOTON/TARGET_NUC472/des/des_alt.c
@@ -348,11 +348,10 @@ static int mbedtls_des_docrypt(uint16_t keyopt, uint8_t key[3][MBEDTLS_DES_KEY_S
         (! crypto_dma_buff_compat(dmabuf_out, MAXSIZE_DMABUF, 8))) {
         error("Buffer for DES alter. DMA requires to be word-aligned and located in 0x20000000-0x2FFFFFFF region.");
     }
-    
-    /* TODO: Change busy-wait to other means to release CPU */
+
     /* Acquire ownership of DES H/W */
-    while (! crypto_des_acquire());
-    
+    crypto_des_acquire();
+
     /* Init crypto module */
     crypto_init();
     /* Enable DES interrupt */
diff --git a/features/mbedtls/targets/TARGET_NUVOTON/TARGET_NUC472/sha/sha1_alt.c b/features/mbedtls/targets/TARGET_NUVOTON/TARGET_NUC472/sha/sha1_alt.c
@@ -31,7 +31,7 @@
  */
 static void mbedtls_sha1_init_internal(mbedtls_sha1_context *ctx, int try_hw)
 {
-    if (try_hw && crypto_sha_acquire()) {
+    if (try_hw && crypto_sha_try_acquire()) {
         ctx->active_ctx = &ctx->hw_ctx;
         mbedtls_sha1_hw_init(&ctx->hw_ctx);
     } else {
diff --git a/features/mbedtls/targets/TARGET_NUVOTON/TARGET_NUC472/sha/sha256_alt.c b/features/mbedtls/targets/TARGET_NUVOTON/TARGET_NUC472/sha/sha256_alt.c
@@ -31,7 +31,7 @@
  */
 static void mbedtls_sha256_init_internal(mbedtls_sha256_context *ctx, int try_hw)
 {
-    if (try_hw && crypto_sha_acquire()) {
+    if (try_hw && crypto_sha_try_acquire()) {
         ctx->active_ctx = &ctx->hw_ctx;
         mbedtls_sha256_hw_init(&ctx->hw_ctx);
     } else {
diff --git a/targets/TARGET_NUVOTON/TARGET_M480/crypto/crypto-misc.cpp b/targets/TARGET_NUVOTON/TARGET_M480/crypto/crypto-misc.cpp
@@ -19,26 +19,48 @@
 #include "mbed_assert.h"
 #include "mbed_critical.h"
 #include "mbed_error.h"
+#include "cmsis_os2.h"
+#include "mbed_rtos_storage.h"
+#include <string.h>
 #include <limits.h>
 #include "nu_modutil.h"
 #include "nu_bitutil.h"
 #include "crypto-misc.h"
+#include "SingletonPtr.h"
+#include "Mutex.h"
+
+/* 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<rtos::Mutex> crypto_aes_mutex;
+
+/* Mutex for crypto DES AC management */
+static SingletonPtr<rtos::Mutex> crypto_des_mutex;
+
+/* Mutex for crypto ECC AC management */
+static SingletonPtr<rtos::Mutex> 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 */
@@ -119,44 +141,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)
@@ -240,18 +270,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;
@@ -285,7 +303,7 @@ static bool crypto_submodule_wait(volatile uint16_t *submodule_done)
 }
 
 /* Crypto interrupt handler */
-void CRYPTO_IRQHandler()
+extern "C" void CRYPTO_IRQHandler()
 {
     uint32_t intsts;
     
diff --git a/targets/TARGET_NUVOTON/TARGET_M480/crypto/crypto-misc.h b/targets/TARGET_NUVOTON/TARGET_M480/crypto/crypto-misc.h
@@ -32,26 +32,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.
diff --git a/targets/TARGET_NUVOTON/TARGET_NUC472/crypto/crypto-misc.cpp b/targets/TARGET_NUVOTON/TARGET_NUC472/crypto/crypto-misc.cpp
diff --git a/targets/TARGET_NUVOTON/TARGET_NUC472/crypto/crypto-misc.h b/targets/TARGET_NUVOTON/TARGET_NUC472/crypto/crypto-misc.h

Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@`
`31`	`31`	`*/`
`32`	`32`	`static void mbedtls_sha1_init_internal(mbedtls_sha1_context *ctx, int try_hw)`
`33`	`33`	`{`
`34`		`- if (try_hw && crypto_sha_acquire()) {`
	`34`	`+ if (try_hw && crypto_sha_try_acquire()) {`
`35`	`35`	`ctx->active_ctx = &ctx->hw_ctx;`
`36`	`36`	`mbedtls_sha1_hw_init(&ctx->hw_ctx);`
`37`	`37`	`} else {`