Merge pull request #9081 from OpenNuvoton/nuvoton_refine_crypto_ac

Cruz Monrreal · web-flow · commit 9edbcd77dff6 · 2018-12-19T13:09:47.000-06:00
Nuvoton: Fix crypto AC management
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
@@ -53,6 +53,7 @@
  *        would be defined in mbedtls/ecp.h from ecp.c for our inclusion */
 #define ECP_SHORTWEIERSTRASS
 
+#include "mbedtls/platform.h"
 #include "mbedtls/ecp_internal.h"
 #include "mbed_toolchain.h"
 #include "mbed_assert.h"
@@ -222,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;
@@ -237,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();
 }
@@ -589,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) {
@@ -632,10 +645,9 @@ NU_STATIC int internal_run_eccop(const mbedtls_ecp_group *grp,
     crypto_ecc_prestart();
     CRPT->ECC_CTL = (grp->pbits << CRPT_ECC_CTL_CURVEM_Pos) | eccop | CRPT_ECC_CTL_FSEL_Msk | CRPT_ECC_CTL_START_Msk;
     ecc_done = crypto_ecc_wait();
-    
-    /* FIXME: Better error code for ECC accelerator error */
-    MBEDTLS_MPI_CHK(ecc_done ? 0 : -1);
-    
+
+    MBEDTLS_MPI_CHK(ecc_done ? 0 : MBEDTLS_ERR_PLATFORM_HW_ACCEL_FAILED);
+
     /* (X1, Y1) hold the normalized result. */
     MBEDTLS_MPI_CHK(internal_mpi_read_eccreg(&R->X, (uint32_t *) CRPT->ECC_X1, NU_ECC_BIGNUM_MAXWORD));
     MBEDTLS_MPI_CHK(internal_mpi_read_eccreg(&R->Y, (uint32_t *) CRPT->ECC_Y1, NU_ECC_BIGNUM_MAXWORD));
@@ -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
@@ -726,10 +738,9 @@ NU_STATIC int internal_run_modop(mbedtls_mpi *r,
     crypto_ecc_prestart();
     CRPT->ECC_CTL = (pbits << CRPT_ECC_CTL_CURVEM_Pos) | (ECCOP_MODULE | modop) | CRPT_ECC_CTL_FSEL_Msk | CRPT_ECC_CTL_START_Msk;
     ecc_done = crypto_ecc_wait();
-    
-    /* FIXME: Better error code for ECC accelerator error */
-    MBEDTLS_MPI_CHK(ecc_done ? 0 : -1);
-    
+
+    MBEDTLS_MPI_CHK(ecc_done ? 0 : MBEDTLS_ERR_PLATFORM_HW_ACCEL_FAILED);
+
     /* X1 holds the result. */
     MBEDTLS_MPI_CHK(internal_mpi_read_eccreg(r, (uint32_t *) CRPT->ECC_X1, NU_ECC_BIGNUM_MAXWORD));
 
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
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 {`