Test: Refactor the os_wrapper layer and NS test
This patch refactors the os_wrapper layer and NS test.
- Use consistent naming of functions in the OS wrapper.
- Code style fixes.
- Rename the implementation to highlight that they are
CMSIS-RTOS2 specific examples.
- Reduce stack size used by SST test threads in regression.
- Reorganise the SST test to avoid the usage of thread
join operation.
- Remove un-needed functions.
- Disallow LOG_MSG(...) from Handler mode.
Signed-off-by: Antonio de Angelis <antonio.deangelis@arm.com>
Change-Id: Ifbdb3429f006cdf1a97090ed0c5e0db195777969
diff --git a/app/CMakeLists.txt b/app/CMakeLists.txt
index ac33cbb..4d9c953 100644
--- a/app/CMakeLists.txt
+++ b/app/CMakeLists.txt
@@ -50,11 +50,11 @@
"${CMSIS_5_DIR}/CMSIS/RTOS2/RTX/Source/rtx_lib.c"
"${APP_DIR}/main_ns.c"
"${APP_DIR}/tfm_integ_test.c"
- "${APP_DIR}/os_wrapper_rtx.c"
+ "${APP_DIR}/os_wrapper_cmsis_rtos.c"
"${INTERFACE_DIR}/src/tfm_sst_api.c"
"${INTERFACE_DIR}/src/tfm_crypto_api.c"
"${INTERFACE_DIR}/src/tfm_initial_attestation_api.c"
- "${INTERFACE_DIR}/src/tfm_ns_lock_rtx.c"
+ "${INTERFACE_DIR}/src/tfm_ns_lock_cmsis_rtos.c"
)
if (TFM_PARTITION_AUDIT_LOG)
@@ -126,7 +126,8 @@
config_setting_shared_compiler_flags(${PROJECT_OBJ_LIB})
#Set macro definitions
-target_compile_definitions(${PROJECT_OBJ_LIB} PRIVATE __thumb2__ __DOMAIN_NS=1 __ARM_FEATURE_CMSE=3 LOG_MSG_HANDLER_MODE_PRINTF_ENABLED)
+set(TARGET_COMPILE_DEFINITIONS __thumb2__ __DOMAIN_NS=1 __ARM_FEATURE_CMSE=3)
+target_compile_definitions(${PROJECT_OBJ_LIB} PRIVATE ${TARGET_COMPILE_DEFINITIONS})
#Set include directories.
embedded_target_include_directories(TARGET ${PROJECT_OBJ_LIB} PATH ${TEST_INTERFACE_DIR}/include ABSOLUTE APPEND)
diff --git a/app/main_ns.c b/app/main_ns.c
index c648dfe..8b4e271 100644
--- a/app/main_ns.c
+++ b/app/main_ns.c
@@ -90,21 +90,12 @@
/**
* \brief List of RTOS thread attributes
*/
-#ifdef TEST_FRAMEWORK_NS
-/* Allocate dedicated stack for test executor thread.
- * It must be 64 bit aligned.
- */
+#if defined(TEST_FRAMEWORK_NS) || defined(PSA_API_TEST_NS)
static uint64_t test_app_stack[(3u * 1024u) / (sizeof(uint64_t))]; /* 3KB */
-
-static const osThreadAttr_t tserv_test = {
- .name = "test_app",
- .stack_size = sizeof(test_app_stack),
+static const osThreadAttr_t thread_attr = {
+ .name = "test_thread",
.stack_mem = test_app_stack,
-};
-#elif PSA_API_TEST_NS
-static const osThreadAttr_t psa_api_test_attr = {
- .name = "psa_api_test",
- .stack_size = 3072U
+ .stack_size = sizeof(test_app_stack),
};
#endif
@@ -112,8 +103,9 @@
* \brief Static globals to hold RTOS related quantities,
* main thread
*/
-static osStatus_t status;
-static osThreadId_t thread_id;
+static osStatus_t status;
+static osThreadId_t thread_id;
+static osThreadFunc_t thread_func;
/**
* \brief main() function
@@ -131,12 +123,17 @@
/* Initialize the TFM NS lock */
tfm_ns_lock_init();
-#ifdef TEST_FRAMEWORK_NS
- thread_id = osThreadNew(test_app, NULL, &tserv_test);
-#elif PSA_API_TEST_NS
- thread_id = osThreadNew(psa_api_test, NULL, &psa_api_test_attr);
+#if defined(TEST_FRAMEWORK_NS)
+ thread_func = test_app;
+#elif defined(PSA_API_TEST_NS)
+ thread_func = psa_api_test;
+#endif
+
+#if defined(TEST_FRAMEWORK_NS) || defined(PSA_API_TEST_NS)
+ thread_id = osThreadNew(thread_func, NULL, &thread_attr);
#else
UNUSED_VARIABLE(thread_id);
+ UNUSED_VARIABLE(thread_func);
#endif
status = osKernelStart();
diff --git a/app/os_wrapper_rtx.c b/app/os_wrapper_cmsis_rtos.c
similarity index 69%
rename from app/os_wrapper_rtx.c
rename to app/os_wrapper_cmsis_rtos.c
index 9a9a887..3f5af0d 100644
--- a/app/os_wrapper_rtx.c
+++ b/app/os_wrapper_cmsis_rtos.c
@@ -10,18 +10,21 @@
#include <string.h>
#include "cmsis_os2.h"
-/* This is an example OS abstraction layer rtx RTOS for non-secure test
- * environment */
+/* This is an example OS abstraction layer for CMSIS-RTOS for non-secure test
+ * environment
+ */
-uint32_t os_wrapper_new_thread(const char* name, uint32_t stack_size,
+uint32_t os_wrapper_thread_new(const char *name, int32_t stack_size,
os_wrapper_thread_func func, void *arg,
uint32_t priority)
{
osThreadAttr_t task_attribs = {.tz_module = 1};
osThreadId_t thread_id;
- task_attribs.attr_bits = osThreadJoinable;
- task_attribs.stack_size = stack_size;
+ /* By default, the thread starts as osThreadDetached */
+ if (stack_size != OS_WRAPPER_DEFAULT_STACK_SIZE) {
+ task_attribs.stack_size = stack_size;
+ }
task_attribs.name = name;
task_attribs.priority = (osPriority_t) priority;
@@ -35,7 +38,7 @@
uint32_t os_wrapper_semaphore_create(uint32_t max_count, uint32_t initial_count,
- const char* name)
+ const char *name)
{
osSemaphoreAttr_t sema_attrib = {0};
osSemaphoreId_t semaphore;
@@ -54,7 +57,9 @@
{
osStatus_t status;
- status = osSemaphoreAcquire((osSemaphoreId_t)semaphore_id, timeout);
+ status = osSemaphoreAcquire((osSemaphoreId_t)semaphore_id,
+ (timeout == OS_WRAPPER_WAIT_FOREVER) ?
+ osWaitForever : timeout);
if (status != osOK) {
return OS_WRAPPER_ERROR;
}
@@ -86,19 +91,19 @@
return 0;
}
-uint32_t os_wrapper_get_thread_id(void)
+uint32_t os_wrapper_thread_get_id(void)
{
osThreadId_t thread_id;
thread_id = osThreadGetId();
- if(thread_id == NULL) {
+ if (thread_id == NULL) {
return OS_WRAPPER_ERROR;
}
return (uint32_t)thread_id;
}
-uint32_t os_wrapper_get_thread_priority(uint32_t id)
+uint32_t os_wrapper_thread_get_priority(uint32_t id)
{
osPriority_t prio;
@@ -110,19 +115,7 @@
return prio;
}
-uint32_t os_wrapper_join_thread(uint32_t id)
+void os_wrapper_thread_exit(void)
{
- osStatus_t status;
-
- /* Wait for the thread to terminate */
- status = osThreadJoin((osThreadId_t)id);
- if (status != osOK) {
- return OS_WRAPPER_ERROR;
- }
-
- /* RTX handles thread deletion automatically. So, no action is required in
- * this function to delete the thread.
- */
-
- return 0;
+ osThreadExit();
}
diff --git a/app/tfm_integ_test.h b/app/tfm_integ_test.h
index 1e419d2..8b52ba0 100644
--- a/app/tfm_integ_test.h
+++ b/app/tfm_integ_test.h
@@ -47,14 +47,10 @@
*/
__attribute__((always_inline)) __STATIC_INLINE void LOG_MSG(const char *MSG)
{
-#ifndef LOG_MSG_HANDLER_MODE_PRINTF_ENABLED
/* if IPSR is non-zero, exception is active. NOT banked S/NS */
if (!__get_IPSR()) {
printf("\t\033[1;32m[Non-Sec] %s\033[0m\r\n", MSG);
}
-#else
- printf("\t\033[1;32m[Non-Sec] %s\033[0m\r\n", MSG);
-#endif
}
#ifdef __cplusplus
diff --git a/interface/include/tfm_ns_lock.h b/interface/include/tfm_ns_lock.h
index d9acd00..f9a47f0 100644
--- a/interface/include/tfm_ns_lock.h
+++ b/interface/include/tfm_ns_lock.h
@@ -33,7 +33,7 @@
* \details Needs to be called during non-secure app init
* to initialize the TFM NS lock object
*/
-enum tfm_status_e tfm_ns_lock_init();
+enum tfm_status_e tfm_ns_lock_init(void);
#ifdef __cplusplus
}
diff --git a/interface/src/tfm_ns_lock_rtx.c b/interface/src/tfm_ns_lock_cmsis_rtos.c
similarity index 83%
rename from interface/src/tfm_ns_lock_rtx.c
rename to interface/src/tfm_ns_lock_cmsis_rtos.c
index 14fd76a..a81f6e1 100644
--- a/interface/src/tfm_ns_lock_rtx.c
+++ b/interface/src/tfm_ns_lock_cmsis_rtos.c
@@ -15,8 +15,7 @@
/**
* \brief struct ns_lock_state type
*/
-struct ns_lock_state
-{
+struct ns_lock_state {
bool init;
osMutexId_t id;
};
@@ -24,7 +23,7 @@
/**
* \brief ns_lock status
*/
-static struct ns_lock_state ns_lock = {.init=false, .id=NULL};
+static struct ns_lock_state ns_lock = {.init = false, .id = NULL};
/**
* \brief Mutex properties, NS lock
@@ -51,7 +50,7 @@
}
/* TFM request protected by NS lock */
- if (osMutexAcquire(ns_lock.id,osWaitForever) != osOK) {
+ if (osMutexAcquire(ns_lock.id, osWaitForever) != osOK) {
return TFM_ERROR_GENERIC;
}
@@ -67,20 +66,13 @@
/**
* \brief NS world, Init NS lock
*/
-enum tfm_status_e tfm_ns_lock_init()
+enum tfm_status_e tfm_ns_lock_init(void)
{
if (ns_lock.init == false) {
ns_lock.id = osMutexNew(&ns_lock_attrib);
ns_lock.init = true;
return TFM_SUCCESS;
- }
- else {
+ } else {
return TFM_ERROR_GENERIC;
}
}
-
-bool tfm_ns_lock_get_init_state()
-{
- return ns_lock.init;
-}
-
diff --git a/test/suites/sst/non_secure/ns_test_helpers.c b/test/suites/sst/non_secure/ns_test_helpers.c
index d86a8cd..e1f8f7e 100644
--- a/test/suites/sst/non_secure/ns_test_helpers.c
+++ b/test/suites/sst/non_secure/ns_test_helpers.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2018, Arm Limited. All rights reserved.
+ * Copyright (c) 2018-2019, Arm Limited. All rights reserved.
*
* SPDX-License-Identifier: BSD-3-Clause
*
@@ -11,7 +11,7 @@
#include "tfm_nspm_api.h"
-#define SST_TEST_TASK_STACK_SIZE 2048
+#define SST_TEST_TASK_STACK_SIZE (512)
struct test_task_t {
test_func_t *func;
@@ -39,6 +39,9 @@
/* Release the semaphore to unblock the parent thread */
os_wrapper_semaphore_release(test_semaphore);
+
+ /* Signal to the RTOS that the thread is finished */
+ os_wrapper_thread_exit();
}
void tfm_sst_run_test(const char *thread_name, struct test_result_t *ret,
@@ -50,27 +53,29 @@
uint32_t thread;
struct test_task_t test_task = { .func = test_func, .ret = ret };
- test_semaphore = os_wrapper_semaphore_create(1, 0, "sst_tests_mutex");
+ /* Create a binary semaphore with initial count of 0 tokens available */
+ test_semaphore = os_wrapper_semaphore_create(1, 0, "sst_tests_sema");
if (test_semaphore == OS_WRAPPER_ERROR) {
TEST_FAIL("Semaphore creation failed");
return;
}
- current_thread_id = os_wrapper_get_thread_id();
+ current_thread_id = os_wrapper_thread_get_id();
if (current_thread_id == OS_WRAPPER_ERROR) {
os_wrapper_semaphore_delete(test_semaphore);
TEST_FAIL("Failed to get current thread ID");
return;
}
- current_thread_priority = os_wrapper_get_thread_priority(current_thread_id);
+ current_thread_priority = os_wrapper_thread_get_priority(current_thread_id);
if (current_thread_priority == OS_WRAPPER_ERROR) {
os_wrapper_semaphore_delete(test_semaphore);
TEST_FAIL("Failed to get current thread priority");
return;
}
- thread = os_wrapper_new_thread(thread_name, SST_TEST_TASK_STACK_SIZE,
+ /* Start test thread */
+ thread = os_wrapper_thread_new(thread_name, SST_TEST_TASK_STACK_SIZE,
test_task_runner, &test_task,
current_thread_priority);
if (thread == OS_WRAPPER_ERROR) {
@@ -79,12 +84,14 @@
return;
}
- /* Wait indefinitely for the test to finish and release the semaphore */
- err = os_wrapper_semaphore_acquire(test_semaphore, 0xFFFFFFFF);
- err |= os_wrapper_join_thread(thread);
- if (err == OS_WRAPPER_ERROR) {
- TEST_FAIL("Failed while waiting for test thread to end");
- }
+ /* Signal semaphore, wait indefinitely until unblocked by child thread */
+ err = os_wrapper_semaphore_acquire(test_semaphore, OS_WRAPPER_WAIT_FOREVER);
+
+ /* At this point, it means the binary semaphore has been released by the
+ * test and re-acquired by this thread, so just finally release it and
+ * delete it
+ */
+ os_wrapper_semaphore_release(test_semaphore);
os_wrapper_semaphore_delete(test_semaphore);
}
diff --git a/test/suites/sst/non_secure/os_wrapper.h b/test/suites/sst/non_secure/os_wrapper.h
index 2df3532..eab121e 100644
--- a/test/suites/sst/non_secure/os_wrapper.h
+++ b/test/suites/sst/non_secure/os_wrapper.h
@@ -14,7 +14,9 @@
#include <stdint.h>
-#define OS_WRAPPER_ERROR 0xFFFFFFFF
+#define OS_WRAPPER_ERROR (0xFFFFFFFFU)
+#define OS_WRAPPER_WAIT_FOREVER (0xFFFFFFFFU)
+#define OS_WRAPPER_DEFAULT_STACK_SIZE (-1)
/* prototype for the thread entry function */
typedef void (*os_wrapper_thread_func) (void *argument);
@@ -65,14 +67,16 @@
* \brief Creates a new thread
*
* \param[in] name Name of the thread
- * \param[in] stack_size Size of stack to be allocated for this thread
+ * \param[in] stack_size Size of stack to be allocated for this thread. It can
+ * be OS_WRAPPER_DEFAULT_STACK_SIZE to use the default
+ * value provided by the underlying RTOS
* \param[in] func Pointer to the function invoked by thread
* \param[in] arg Argument to pass to the function invoked by thread
* \param[in] priority Initial thread priority
*
* \return Returns thread ID, or OS_WRAPPER_ERROR in case of error
*/
-uint32_t os_wrapper_new_thread(const char *name, uint32_t stack_size,
+uint32_t os_wrapper_thread_new(const char *name, int32_t stack_size,
os_wrapper_thread_func func, void *arg,
uint32_t priority);
/**
@@ -80,7 +84,7 @@
*
* \return Returns thread ID, or OS_WRAPPER_ERROR in case of error
*/
-uint32_t os_wrapper_get_thread_id(void);
+uint32_t os_wrapper_thread_get_id(void);
/**
* \brief Gets thread priority
@@ -89,16 +93,14 @@
*
* \return Returns thread priority value, or OS_WRAPPER_ERROR in case of error
*/
-uint32_t os_wrapper_get_thread_priority(uint32_t id);
+uint32_t os_wrapper_thread_get_priority(uint32_t id);
/**
- * \brief Waits for the thread to terminate
+ * \brief Exits the calling thread
*
- * \param[in] id Thread ID
- *
- * \return 0 in case of successful join or OS_WRAPPER_ERROR in case of error.
+ * \return void
*/
-uint32_t os_wrapper_join_thread(uint32_t id);
+void os_wrapper_thread_exit(void);
#ifdef __cplusplus
}