SPE: Fix up atomic usage

PSA SPE code was using atomics, but not consistently. On the assumption
that the atomics are needed, correct the code to be properly atomic.

* Tighten up table full case - new_handle was left with a bogus value,
  and surrounding code (as a result?) used loop index to assert success.
  Make handle the sole output of the loop, and correct and use it.
* Ensure handle in table is cleared last, with atomic store to release.
* Make skipping of the invalid handle in handle generator loop atomic.
* Use atomic load on state assert, and don't re-read on failure.
* Tighten up types, and avoid casts by using new signed atomics.

Author: kjbracey

components/TARGET_PSA/TARGET_MBED_SPM/COMPONENT_SPE/handles_manager.c

@@ -47,35 +47,30 @@ psa_handle_t psa_hndl_mgr_handle_create(psa_handle_manager_t *handle_mgr, void *
     // Get active partition id - Needed for requester identification
     spm_partition_t *curr_part_ptr = get_active_partition();
     int32_t          current_pid   = ((curr_part_ptr != NULL) ? curr_part_ptr->partition_id : PSA_NSPE_IDENTIFIER);
-    uint32_t         expected      = UINT16_MAX;
-
-    // Avoid passing UINT16_MAX. Start again from 0 if reached.
-    // The reason for this is that we use the 16 upper bits to store the handle's index in the handles pool (for performance reasons)
-    core_util_atomic_cas_u32((uint32_t *)(&(handle_mgr->handle_generator)),
-                             &expected,
-                             PSA_HANDLE_MGR_INVALID_HANDLE
-                            );
 
     // Generate a new handle identifier
-    uint32_t tmp_handle = core_util_atomic_incr_u32(&(handle_mgr->handle_generator), 1);
-    uint32_t new_handle = PSA_HANDLE_MGR_INVALID_HANDLE;
-    uint32_t pool_ix    = 0;
+    uint32_t tmp_handle;
+    do {
+        tmp_handle = core_util_atomic_incr_u16(&(handle_mgr->handle_generator), 1);
+    } while (tmp_handle == PSA_HANDLE_MGR_INVALID_HANDLE);
+    psa_handle_t new_handle = PSA_NULL_HANDLE;
 
     // Look for a vacant space in handles pool for the generated handle
-    for (pool_ix = 0; pool_ix < handle_mgr->pool_size; pool_ix++) {
+    for (uint32_t pool_ix = 0; pool_ix < handle_mgr->pool_size; pool_ix++) {
 
-        expected = PSA_HANDLE_MGR_INVALID_HANDLE;
+        psa_handle_t expected = PSA_NULL_HANDLE;
 
         // Write the handles pool index in the upper 16 bits of the handle
-        new_handle = ((pool_ix << PSA_HANDLE_MGR_HANDLE_INDEX_POS) | tmp_handle);
+        psa_handle_t desired_handle = ((pool_ix << PSA_HANDLE_MGR_HANDLE_INDEX_POS) | tmp_handle);
 
         // Store the generated handle in the handles pool
-        if (core_util_atomic_cas_u32((uint32_t *)(&(handle_mgr->handles_pool[pool_ix].handle)),
+        if (core_util_atomic_cas_s32(&(handle_mgr->handles_pool[pool_ix].handle),
                                      &expected,
-                                     new_handle
+                                     desired_handle
                                     )) {
 
             // Handle is successfully stored in handles pool
+            new_handle = desired_handle;
 
             // Store the handle memory in the handles pool, "coupled" with the stored handle
             handle_mgr->handles_pool[pool_ix].handle_mem    = handle_mem;
@@ -90,7 +85,7 @@ psa_handle_t psa_hndl_mgr_handle_create(psa_handle_manager_t *handle_mgr, void *
 
     // Handle creation should only occur after a successful memory allocation
     // and is not expected to fail.
-    SPM_ASSERT(pool_ix != handle_mgr->pool_size);
+    SPM_ASSERT(new_handle != PSA_NULL_HANDLE);
 
     return new_handle;
 }
@@ -123,9 +118,9 @@ void psa_hndl_mgr_handle_destroy(psa_handle_manager_t *handle_mgr, psa_handle_t
         SPM_PANIC("[ERROR] Request for destroy by non-owner or friend!\n");
     }
 
-    handle_mgr->handles_pool[pool_ix].handle        = PSA_NULL_HANDLE;
     handle_mgr->handles_pool[pool_ix].handle_owner  = PSA_HANDLE_MGR_INVALID_FRIEND_OWNER;
     handle_mgr->handles_pool[pool_ix].handle_friend = PSA_HANDLE_MGR_INVALID_FRIEND_OWNER;
+    core_util_atomic_store_s32(&(handle_mgr->handles_pool[pool_ix].handle), PSA_NULL_HANDLE);
 }
 
 

components/TARGET_PSA/TARGET_MBED_SPM/COMPONENT_SPE/handles_manager.h

@@ -55,7 +55,7 @@ extern "C" {
 
 /* ------------------------------------ Definitions ---------------------------------- */
 
-#define PSA_HANDLE_MGR_INVALID_HANDLE           ((uint32_t)PSA_NULL_HANDLE)
+#define PSA_HANDLE_MGR_INVALID_HANDLE           ((uint16_t)PSA_NULL_HANDLE)
 
 #define PSA_HANDLE_MGR_INVALID_FRIEND_OWNER     0       // Denoting invalid friend or invalid owner
 
@@ -80,8 +80,10 @@ typedef struct psa_handle_item_t {
 
 typedef struct psa_handle_manager_t {
 
-    uint32_t           handle_generator;    /* A counter supplying handle numbers.  */
-    uint32_t           pool_size;           /* The maximum number of handles that pool can contain. */
+    // Handle generator uses only 16 bits, and wraps.
+    // The reason for this is that we use the 16 upper bits to store the handle's index in the handles pool (for performance reasons)
+    uint16_t           handle_generator;    /* A counter supplying handle numbers.  */
+    uint16_t           pool_size;           /* The maximum number of handles that pool can contain. */
     psa_handle_item_t *handles_pool;        /* Holds couples of handles and their memory "blocks". */
 
 } psa_handle_manager_t;

components/TARGET_PSA/TARGET_MBED_SPM/COMPONENT_SPE/spm_common.c

@@ -16,12 +16,11 @@
  */
 
 #include "cmsis_os2.h"
+#include "mbed_critical.h"
 #include "psa_defs.h"
 #include "spm_internal.h"
 #include "spm_panic.h"
 
-bool core_util_atomic_cas_u8(volatile uint8_t *ptr, uint8_t *expectedCurrentValue, uint8_t desiredValue);
-
 inline void validate_iovec(
     const void *in_vec,
     const uint32_t in_len,
@@ -42,18 +41,19 @@ inline void validate_iovec(
 
 inline void channel_state_switch(uint8_t *current_state, uint8_t expected_state, uint8_t new_state)
 {
-    uint8_t backup_expected = expected_state;
-    if (!core_util_atomic_cas_u8(current_state, &expected_state, new_state)) {
+    uint8_t actual_state = expected_state;
+    if (!core_util_atomic_cas_u8(current_state, &actual_state, new_state)) {
         SPM_PANIC("channel in incorrect processing state: %d while %d is expected!\n",
-                  expected_state, backup_expected);
+                  actual_state, expected_state);
     }
 }
 
-inline void channel_state_assert(uint8_t *current_state, uint8_t expected_state)
+inline void channel_state_assert(const uint8_t *current_state, uint8_t expected_state)
 {
-    if (*current_state != expected_state) {
+    uint8_t actual_state = core_util_atomic_load_u8(current_state);
+    if (actual_state != expected_state) {
         SPM_PANIC("channel in incorrect processing state: %d while %d is expected!\n",
-                  *current_state, expected_state);
+                  actual_state, expected_state);
     }
 }
 

components/TARGET_PSA/TARGET_MBED_SPM/COMPONENT_SPE/spm_internal.h

@@ -242,7 +242,7 @@ void channel_state_switch(uint8_t *current_state, uint8_t expected_state, uint8_
  * @param[in] current_state - current state
  * @param[in] expected_state - expected state
 */
-void channel_state_assert(uint8_t *current_state, uint8_t expected_state);
+void channel_state_assert(const uint8_t *current_state, uint8_t expected_state);
 
 #ifdef __cplusplus
 }