Skip to content

Commit 267f62e

Browse files
committed
[M2351] Fix crypto AC mgmt
Port of NUC472/M487 crypto AC mgmt work to M2351: 1. Choose mutex to synchronize access to crypto non-SHA AC 2. Choose atomic flag to synchronize access to crypto SHA AC
1 parent 3252530 commit 267f62e

File tree

2 files changed

+78
-52
lines changed

2 files changed

+78
-52
lines changed

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

Lines changed: 56 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -19,33 +19,56 @@
1919
#include "mbed_assert.h"
2020
#include "mbed_critical.h"
2121
#include "mbed_error.h"
22+
#if MBED_CONF_RTOS_PRESENT
23+
#include "cmsis_os2.h"
24+
#endif
25+
#include <string.h>
2226
#include <limits.h>
2327
#include "nu_modutil.h"
2428
#include "nu_bitutil.h"
2529
#include "crypto-misc.h"
30+
#include "platform/SingletonPtr.h"
31+
#include "platform/PlatformMutex.h"
2632

2733
#if DEVICE_TRNG || defined(MBEDTLS_CONFIG_HW_SUPPORT)
2834

35+
/* Consideration for choosing proper synchronization mechanism
36+
*
37+
* 1. We choose mutex to synchronize access to crypto non-SHA AC. We can guarantee:
38+
* (1) No deadlock
39+
* We just lock mutex for a short sequence of operations rather than the whole lifetime
40+
* of crypto context.
41+
* (2) No priority inversion
42+
* Mutex supports priority inheritance and it is enabled.
43+
* 2. We choose atomic flag to synchronize access to crypto SHA AC. We can guarantee:
44+
* (1) No deadlock
45+
* With SHA AC not supporting context save & restore, we provide SHA S/W fallback when
46+
* SHA AC is not available.
47+
* (2) No biting CPU
48+
* Same reason as above.
49+
*/
50+
51+
/* Mutex for crypto AES AC management */
52+
static SingletonPtr<PlatformMutex> crypto_aes_mutex;
53+
54+
/* Mutex for crypto DES AC management */
55+
static SingletonPtr<PlatformMutex> crypto_des_mutex;
56+
57+
/* Mutex for crypto ECC AC management */
58+
static SingletonPtr<PlatformMutex> crypto_ecc_mutex;
59+
60+
/* Atomic flag for crypto SHA AC management */
61+
static core_util_atomic_flag crypto_sha_atomic_flag = CORE_UTIL_ATOMIC_FLAG_INIT;
62+
2963
/* NOTE: There's inconsistency in cryptography related naming, Crpt or Crypto. For example, cryptography IRQ
3064
* handler could be CRPT_IRQHandler or CRYPTO_IRQHandler. To override default cryptography IRQ handler, see
3165
* device/startup_{CHIP}.c for its name or call NVIC_SetVector regardless of its name. */
3266
void CRPT_IRQHandler();
3367

34-
/* Track if AES H/W is available */
35-
static uint16_t crypto_aes_avail = 1;
36-
/* Track if DES H/W is available */
37-
static uint16_t crypto_des_avail = 1;
38-
/* Track if SHA H/W is available */
39-
static uint16_t crypto_sha_avail = 1;
40-
/* Track if ECC H/W is available */
41-
static uint16_t crypto_ecc_avail = 1;
4268

4369
/* Crypto (AES, DES, SHA, etc.) init counter. Crypto's keeps active as it is non-zero. */
4470
static uint16_t crypto_init_counter = 0U;
4571

46-
static bool crypto_submodule_acquire(uint16_t *submodule_avail);
47-
static void crypto_submodule_release(uint16_t *submodule_avail);
48-
4972
/* Crypto done flags */
5073
#define CRYPTO_DONE_OK BIT0 /* Done with OK */
5174
#define CRYPTO_DONE_ERR BIT1 /* Done with error */
@@ -131,44 +154,52 @@ void crypto_zeroize32(uint32_t *v, size_t n)
131154
}
132155
}
133156

134-
bool crypto_aes_acquire(void)
157+
void crypto_aes_acquire(void)
135158
{
136-
return crypto_submodule_acquire(&crypto_aes_avail);
159+
/* Don't check return code of Mutex::lock(void)
160+
*
161+
* This function treats RTOS errors as fatal system errors, so it can only return osOK.
162+
* Use of the return value is deprecated, as the return is expected to become void in
163+
* the future.
164+
*/
165+
crypto_aes_mutex->lock();
137166
}
138167

139168
void crypto_aes_release(void)
140169
{
141-
crypto_submodule_release(&crypto_aes_avail);
170+
crypto_aes_mutex->unlock();
142171
}
143172

144-
bool crypto_des_acquire(void)
173+
void crypto_des_acquire(void)
145174
{
146-
return crypto_submodule_acquire(&crypto_des_avail);
175+
/* Don't check return code of Mutex::lock(void) */
176+
crypto_des_mutex->lock();
147177
}
148178

149179
void crypto_des_release(void)
150180
{
151-
crypto_submodule_release(&crypto_des_avail);
181+
crypto_des_mutex->unlock();
152182
}
153183

154-
bool crypto_sha_acquire(void)
184+
void crypto_ecc_acquire(void)
155185
{
156-
return crypto_submodule_acquire(&crypto_sha_avail);
186+
/* Don't check return code of Mutex::lock(void) */
187+
crypto_ecc_mutex->lock();
157188
}
158189

159-
void crypto_sha_release(void)
190+
void crypto_ecc_release(void)
160191
{
161-
crypto_submodule_release(&crypto_sha_avail);
192+
crypto_ecc_mutex->unlock();
162193
}
163194

164-
bool crypto_ecc_acquire(void)
195+
bool crypto_sha_try_acquire(void)
165196
{
166-
return crypto_submodule_acquire(&crypto_ecc_avail);
197+
return !core_util_atomic_flag_test_and_set(&crypto_sha_atomic_flag);
167198
}
168199

169-
void crypto_ecc_release(void)
200+
void crypto_sha_release(void)
170201
{
171-
crypto_submodule_release(&crypto_ecc_avail);
202+
core_util_atomic_flag_clear(&crypto_sha_atomic_flag);
172203
}
173204

174205
void crypto_prng_prestart(void)
@@ -252,18 +283,6 @@ bool crypto_dma_buffs_overlap(const void *in_buff, size_t in_buff_size, const vo
252283
return overlap;
253284
}
254285

255-
static bool crypto_submodule_acquire(uint16_t *submodule_avail)
256-
{
257-
uint16_t expectedCurrentValue = 1;
258-
return core_util_atomic_cas_u16(submodule_avail, &expectedCurrentValue, 0);
259-
}
260-
261-
static void crypto_submodule_release(uint16_t *submodule_avail)
262-
{
263-
uint16_t expectedCurrentValue = 0;
264-
while (! core_util_atomic_cas_u16(submodule_avail, &expectedCurrentValue, 1));
265-
}
266-
267286
static void crypto_submodule_prestart(volatile uint16_t *submodule_done)
268287
{
269288
*submodule_done = 0;

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

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -66,26 +66,33 @@ void crypto_uninit(void);
6666
void crypto_zeroize(void *v, size_t n);
6767
void crypto_zeroize32(uint32_t *v, size_t n);
6868

69-
/* Acquire/release ownership of AES H/W */
70-
/* NOTE: If "acquire" succeeds, "release" must be done to pair it. */
71-
bool crypto_aes_acquire(void);
69+
/* Acquire/release ownership of crypto sub-module
70+
*
71+
* \note "acquire" is blocking until ownership is acquired
72+
*
73+
* \note "acquire"/"release" must be paired.
74+
*
75+
* \note Recursive "acquire" is allowed because the underlying synchronization
76+
* primitive mutex supports it.
77+
*/
78+
void crypto_aes_acquire(void);
7279
void crypto_aes_release(void);
73-
74-
/* Acquire/release ownership of DES H/W */
75-
/* NOTE: If "acquire" succeeds, "release" must be done to pair it. */
76-
bool crypto_des_acquire(void);
80+
void crypto_des_acquire(void);
7781
void crypto_des_release(void);
82+
void crypto_ecc_acquire(void);
83+
void crypto_ecc_release(void);
7884

79-
/* Acquire/release ownership of SHA H/W */
80-
/* NOTE: If "acquire" succeeds, "release" must be done to pair it. */
81-
bool crypto_sha_acquire(void);
85+
/* Acquire/release ownership of crypto sub-module
86+
*
87+
* \return false if crytpo sub-module is held by another thread or
88+
* another mbedtls context.
89+
* true if successful
90+
*
91+
* \note Successful "try_acquire" and "release" must be paired.
92+
*/
93+
bool crypto_sha_try_acquire(void);
8294
void crypto_sha_release(void);
8395

84-
/* Acquire/release ownership of ECC H/W */
85-
/* NOTE: If "acquire" succeeds, "release" must be done to pair it. */
86-
bool crypto_ecc_acquire(void);
87-
void crypto_ecc_release(void);
88-
8996
/* Flow control between crypto/xxx start and crypto/xxx ISR
9097
*
9198
* crypto_xxx_prestart/crypto_xxx_wait encapsulate control flow between crypto/xxx start and crypto/xxx ISR.

0 commit comments

Comments
 (0)