From 124ec431e6eaf4fb05f7a417515d775510a48f88 Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Tue, 31 Mar 2020 13:58:02 -0300 Subject: [PATCH 1/2] test/shared_stack_printf: improved printf with shared stack function test --- .../newlib/test/test_shared_stack_printf.c | 40 +++++++++++++------ .../xtensa/expression_with_stack_xtensa_asm.S | 25 +++++++----- docs/en/api-reference/system/index.rst | 6 +-- 3 files changed, 44 insertions(+), 27 deletions(-) diff --git a/components/newlib/test/test_shared_stack_printf.c b/components/newlib/test/test_shared_stack_printf.c index 81997a5a9a..12ca948d43 100644 --- a/components/newlib/test/test_shared_stack_printf.c +++ b/components/newlib/test/test_shared_stack_printf.c @@ -8,25 +8,39 @@ #include "test_utils.h" #include "esp_expression_with_stack.h" -//makes sure this is not the task stack... +#define SHARED_STACK_SIZE 8192 + +static StackType_t *shared_stack_sp = NULL; + +void external_stack_function(void) +{ + printf("Executing this printf from external stack! sp=%p\n", get_sp()); + shared_stack_sp = (StackType_t *)get_sp(); +} + void another_external_stack_function(void) { //We can even use Freertos resources inside of this context. vTaskDelay(100); - printf("Executing this another printf inside a function with external stack"); -} - -TEST_CASE("test printf using shared buffer stack", "[newlib]") -{ - portSTACK_TYPE *shared_stack = malloc(8192 * sizeof(portSTACK_TYPE)); - - TEST_ASSERT(shared_stack != NULL); - - SemaphoreHandle_t printf_lock = xSemaphoreCreateMutex(); + printf("Done!, sp=%p\n", get_sp()); TEST_ASSERT_NOT_NULL(printf_lock); - ESP_EXECUTE_EXPRESSION_WITH_STACK(printf_lock, shared_stack,8192,printf("Executing this printf from external stack! \n")); - ESP_EXECUTE_EXPRESSION_WITH_STACK(printf_lock, shared_stack,8192,another_external_stack_function()); + esp_execute_shared_stack_function(printf_lock, + shared_stack, + SHARED_STACK_SIZE, + external_stack_function); + + TEST_ASSERT(((shared_stack_sp >= shared_stack_sp) && + (shared_stack_sp < (shared_stack + SHARED_STACK_SIZE)))); + + esp_execute_shared_stack_function(printf_lock, + shared_stack, + SHARED_STACK_SIZE, + another_external_stack_function); + + TEST_ASSERT(((shared_stack_sp >= shared_stack_sp) && + (shared_stack_sp < (shared_stack + SHARED_STACK_SIZE)))); + vSemaphoreDelete(printf_lock); free(shared_stack); } diff --git a/components/xtensa/expression_with_stack_xtensa_asm.S b/components/xtensa/expression_with_stack_xtensa_asm.S index f868131ba0..dc6579628b 100644 --- a/components/xtensa/expression_with_stack_xtensa_asm.S +++ b/components/xtensa/expression_with_stack_xtensa_asm.S @@ -45,17 +45,20 @@ esp_switch_stack_enter: esp_switch_stack_exit: #ifndef __XTENSA_CALL0_ABI__ - entry sp, 0x10 - -#if CONFIG_FREERTOS_WATCHPOINT_END_OF_STACK - movi a6, 2 - movi a4, esp_clear_watchpoint - callx4 a4 /* clear the watchpoint before releasing stack */ -#endif - - l32i a4, a2, 0 /* recover the original task stack */ - mov a1, a4 /* put it on sp register again */ - retw + movi a0, 0 /* no need to rotate window, it will be destroyed anyway */ + movi a6, shared_stack + l32i sp, a6, 0 /* load shared stack pointer */ + movi a12, shared_stack_callback + l32i a12, a12, 0 + callx4 a12 /* call user function */ + movi a6, shared_stack_function_done + movi a7, 1 + s32i a7, a6, 0 /* hint the function was finished */ + movi a6, shared_stack_env + movi a7, 0 + movi a12, longjmp + callx4 a12 /* jump to last clean state previously saved */ + ret #else #error "this code is written for Window ABI" #endif diff --git a/docs/en/api-reference/system/index.rst b/docs/en/api-reference/system/index.rst index dab87a1278..e15d1de207 100644 --- a/docs/en/api-reference/system/index.rst +++ b/docs/en/api-reference/system/index.rst @@ -16,9 +16,9 @@ System API Heap Memory Allocation Heap Memory Debugging High Resolution Timer - Himem (large external SPI RAM) API - Inter-Processor Call - Call function with external stack + :esp32: Himem (large external SPI RAM) API + :esp32: Inter-Processor Call + Call function with external stack Interrupt Allocation Logging Miscellaneous System APIs From 10c498ae7df09e78278f84943704dddcd10c51ae Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Wed, 1 Apr 2020 12:10:13 -0300 Subject: [PATCH 2/2] expression_with_stack: added a tweak on TCB stackpointers to avoid false trigger of stack overflow --- .../include/esp_expression_with_stack.h | 53 +++++------------ .../newlib/test/test_shared_stack_printf.c | 22 +++++-- .../xtensa/expression_with_stack_xtensa.c | 58 ++++++++++++++++-- .../xtensa/expression_with_stack_xtensa_asm.S | 59 ++++++++----------- ...rst => esp_function_with_shared_stack.rst} | 33 +++++++---- docs/en/api-reference/system/index.rst | 4 +- .../system/esp_expression_with_stack.rst | 1 - .../system/esp_function_with_shared_stack.rst | 1 + 8 files changed, 132 insertions(+), 99 deletions(-) rename docs/en/api-reference/system/{esp_expression_with_stack.rst => esp_function_with_shared_stack.rst} (56%) delete mode 100644 docs/zh_CN/api-reference/system/esp_expression_with_stack.rst create mode 100644 docs/zh_CN/api-reference/system/esp_function_with_shared_stack.rst diff --git a/components/esp_common/include/esp_expression_with_stack.h b/components/esp_common/include/esp_expression_with_stack.h index cd1e4765a8..456e5db16b 100644 --- a/components/esp_common/include/esp_expression_with_stack.h +++ b/components/esp_common/include/esp_expression_with_stack.h @@ -13,8 +13,10 @@ // limitations under the License. #pragma once +#include #include "freertos/FreeRTOS.h" #include "freertos/semphr.h" +#include "freertos/task.h" #include "esp_debug_helpers.h" #include "esp_log.h" @@ -22,53 +24,26 @@ extern "C" { #endif +typedef void (*shared_stack_function)(void); + +#define ESP_EXECUTE_EXPRESSION_WITH_STACK(lock, stack, stack_size, expression) \ + esp_execute_shared_stack_function(lock, stack, stack_size, expression) + /** - * @brief Executes a 1-line expression with a application alocated stack + * @brief Calls user defined shared stack space function * @param lock Mutex object to protect in case of shared stack * @param stack Pointer to user alocated stack * @param stack_size Size of current stack in bytes - * @param expression Expression or function to be executed using the stack + * @param function pointer to the shared stack function to be executed * @note if either lock, stack or stack size is invalid, the expression will * be called using the current stack. */ -#define ESP_EXECUTE_EXPRESSION_WITH_STACK(lock, stack, stack_size, expression) \ -({ \ - assert(lock && stack && (stack_size >= CONFIG_ESP_MINIMAL_SHARED_STACK_SIZE)); \ - uint32_t backup; \ - xSemaphoreTake(lock, portMAX_DELAY); \ - StackType_t *top_of_stack = esp_switch_stack_setup(stack, stack_size); \ - esp_switch_stack_enter(top_of_stack, &backup); \ - { \ - expression; \ - } \ - esp_switch_stack_exit(&backup); \ - xSemaphoreGive(lock); \ -}) +void esp_execute_shared_stack_function(SemaphoreHandle_t lock, + void *stack, + size_t stack_size, + shared_stack_function function); -/** - * @brief Fill stack frame with CPU-specifics value before use - * @param stack Caller allocated stack pointer - * @param stack_size Size of stack in bytes - * @return New pointer to the top of stack - * @note Application must not call this function directly - */ -StackType_t * esp_switch_stack_setup(StackType_t *stack, size_t stack_size); - -/** - * @brief Changes CPU sp-register to use another stack space and save the previous one - * @param stack Caller allocated stack pointer - * @param backup_stack Pointer to a place to save the current stack - * @note Application must not call this function directly - */ -extern void esp_switch_stack_enter(StackType_t *stack, uint32_t *backup_stack); - -/** - * @brief Restores the previous CPU sp-register - * @param backup_stack Pointer to the place where stack was saved - * @note Application must not call this function directly - */ -extern void esp_switch_stack_exit(uint32_t *backup_stack); #ifdef __cplusplus } -#endif \ No newline at end of file +#endif diff --git a/components/newlib/test/test_shared_stack_printf.c b/components/newlib/test/test_shared_stack_printf.c index 12ca948d43..98928871db 100644 --- a/components/newlib/test/test_shared_stack_printf.c +++ b/components/newlib/test/test_shared_stack_printf.c @@ -21,16 +21,30 @@ void external_stack_function(void) void another_external_stack_function(void) { //We can even use Freertos resources inside of this context. - vTaskDelay(100); - printf("Done!, sp=%p\n", get_sp()); + printf("We can even use FreeRTOS resources... yielding, sp=%p\n", get_sp()); + taskYIELD(); + shared_stack_sp = (StackType_t *)get_sp(); +} + +TEST_CASE("test printf using shared buffer stack", "[newlib]") +{ + portSTACK_TYPE *shared_stack = malloc(SHARED_STACK_SIZE); + + TEST_ASSERT_NOT_NULL(shared_stack); + + SemaphoreHandle_t printf_lock = xSemaphoreCreateMutex(); TEST_ASSERT_NOT_NULL(printf_lock); + printf("current task sp: %p\n", get_sp()); + printf("shared_stack: %p\n", (void *)shared_stack); + printf("shared_stack expected top: %p\n", (void *)(shared_stack + SHARED_STACK_SIZE)); + esp_execute_shared_stack_function(printf_lock, shared_stack, SHARED_STACK_SIZE, external_stack_function); - TEST_ASSERT(((shared_stack_sp >= shared_stack_sp) && + TEST_ASSERT(((shared_stack_sp >= shared_stack) && (shared_stack_sp < (shared_stack + SHARED_STACK_SIZE)))); esp_execute_shared_stack_function(printf_lock, @@ -38,7 +52,7 @@ void another_external_stack_function(void) SHARED_STACK_SIZE, another_external_stack_function); - TEST_ASSERT(((shared_stack_sp >= shared_stack_sp) && + TEST_ASSERT(((shared_stack_sp >= shared_stack) && (shared_stack_sp < (shared_stack + SHARED_STACK_SIZE)))); vSemaphoreDelete(printf_lock); diff --git a/components/xtensa/expression_with_stack_xtensa.c b/components/xtensa/expression_with_stack_xtensa.c index 74829d7e8b..e505ebec9c 100644 --- a/components/xtensa/expression_with_stack_xtensa.c +++ b/components/xtensa/expression_with_stack_xtensa.c @@ -15,14 +15,33 @@ #include #include #include +#include +#include -StackType_t * esp_switch_stack_setup(StackType_t *stack, size_t stack_size) +StackType_t *xtensa_shared_stack; +shared_stack_function xtensa_shared_stack_callback; +jmp_buf xtensa_shared_stack_env; +bool xtensa_shared_stack_function_done = false; +static portMUX_TYPE xtensa_shared_stack_spinlock = portMUX_INITIALIZER_UNLOCKED; +static void *current_task_stack = NULL; + +extern void esp_shared_stack_invoke_function(void); + +static void esp_switch_stack_setup(StackType_t *stack, size_t stack_size) { #if CONFIG_FREERTOS_WATCHPOINT_END_OF_STACK int watchpoint_place = (((int)stack + 31) & ~31); #endif - StackType_t *top_of_stack = (StackType_t *)&stack[0] + - ((stack_size * sizeof(StackType_t)) / sizeof(StackType_t)); + //We need also to tweak current task stackpointer to avoid erroneous + //stack overflow indication, so fills the stack with freertos known pattern: + memset(stack, 0xa5U, stack_size * sizeof(StackType_t)); + + StaticTask_t *current = (StaticTask_t *)xTaskGetCurrentTaskHandle(); + //Then put the fake stack inside of TCB: + current_task_stack = current->pxDummy6; + current->pxDummy6 = (void *)stack; + + StackType_t *top_of_stack = stack + stack_size; //Align stack to a 16byte boundary, as required by CPU specific: top_of_stack = (StackType_t *)(((UBaseType_t)(top_of_stack - 31) - @@ -38,5 +57,36 @@ StackType_t * esp_switch_stack_setup(StackType_t *stack, size_t stack_size) esp_set_watchpoint(2, (char*)watchpoint_place, 32, ESP_WATCHPOINT_STORE); #endif - return top_of_stack; + xtensa_shared_stack = top_of_stack; +} + + +void esp_execute_shared_stack_function(SemaphoreHandle_t lock, void *stack, size_t stack_size, shared_stack_function function) +{ + assert(lock); + assert(stack); + assert(stack_size > 0 && stack_size >= CONFIG_ESP_MINIMAL_SHARED_STACK_SIZE); + assert(function); + + xSemaphoreTake(lock, portMAX_DELAY); + portENTER_CRITICAL(&xtensa_shared_stack_spinlock); + xtensa_shared_stack_function_done = false; + esp_switch_stack_setup(stack, stack_size); + xtensa_shared_stack_callback = function; + portEXIT_CRITICAL(&xtensa_shared_stack_spinlock); + + setjmp(xtensa_shared_stack_env); + if(!xtensa_shared_stack_function_done) { + esp_shared_stack_invoke_function(); + } + + portENTER_CRITICAL(&xtensa_shared_stack_spinlock); + StaticTask_t *current = (StaticTask_t *)xTaskGetCurrentTaskHandle(); + + //Restore current task stack: + current->pxDummy6 = (StackType_t *)current_task_stack; + vPortSetStackWatchpoint(current->pxDummy6); + portEXIT_CRITICAL(&xtensa_shared_stack_spinlock); + + xSemaphoreGive(lock); } \ No newline at end of file diff --git a/components/xtensa/expression_with_stack_xtensa_asm.S b/components/xtensa/expression_with_stack_xtensa_asm.S index dc6579628b..38c0dd998c 100644 --- a/components/xtensa/expression_with_stack_xtensa_asm.S +++ b/components/xtensa/expression_with_stack_xtensa_asm.S @@ -13,52 +13,39 @@ // limitations under the License. #include - - .extern esp_clear_watchpoint + + .extern xtensa_shared_stack + .extern xtensa_shared_stack_callback + .extern xtensa_shared_stack_function_done + .extern xtensa_shared_stack_env + .extern longjmp .text -/** - * extern void switch_stack_enter(portSTACK_TYPE *stack, portSTACK_TYPE *backup_stack); - */ - .globl esp_switch_stack_enter - .type esp_switch_stack_enter,@function + +/* extern void esp_shared_stack_invoke_function(void) */ + + .globl esp_shared_stack_invoke_function + .type esp_shared_stack_invoke_function,@function .align 4 -esp_switch_stack_enter: +esp_shared_stack_invoke_function: #ifndef __XTENSA_CALL0_ABI__ - entry sp, 0x10 - mov a4, a1 - s32i a4, a3, 0 /* on a3 there is a safe place to save the current stack */ - l32i a4, a2, 0 /* obtains the user allocated stack buffer */ - mov a1, a4 /* sp register now contains caller specified stack */ - retw - #else - #error "this code is written for Window ABI" - #endif - -/** - * extern void switch_stack_exit(portSTACK_TYPE *backup_stack); - */ - .globl esp_switch_stack_exit - .type esp_switch_stack_exit,@function - .align 4 -esp_switch_stack_exit: - - #ifndef __XTENSA_CALL0_ABI__ - movi a0, 0 /* no need to rotate window, it will be destroyed anyway */ - movi a6, shared_stack + movi a0, 0 /* must not rotate the window here, */ + /* the state of execution for shared stack */ + /* functions will be completely destroyed at end */ + movi a6, xtensa_shared_stack l32i sp, a6, 0 /* load shared stack pointer */ - movi a12, shared_stack_callback + movi a12, xtensa_shared_stack_callback l32i a12, a12, 0 callx4 a12 /* call user function */ - movi a6, shared_stack_function_done - movi a7, 1 + movi a6, xtensa_shared_stack_function_done + movi a7, 1 s32i a7, a6, 0 /* hint the function was finished */ - movi a6, shared_stack_env + movi a6, xtensa_shared_stack_env movi a7, 0 - movi a12, longjmp + movi a12, longjmp callx4 a12 /* jump to last clean state previously saved */ ret - #else - #error "this code is written for Window ABI" + #else + #error "this code is written for Window ABI" #endif diff --git a/docs/en/api-reference/system/esp_expression_with_stack.rst b/docs/en/api-reference/system/esp_function_with_shared_stack.rst similarity index 56% rename from docs/en/api-reference/system/esp_expression_with_stack.rst rename to docs/en/api-reference/system/esp_function_with_shared_stack.rst index dd7b994f25..4e3dd7e809 100644 --- a/docs/en/api-reference/system/esp_expression_with_stack.rst +++ b/docs/en/api-reference/system/esp_function_with_shared_stack.rst @@ -8,24 +8,31 @@ A given function can be executed with a user allocated stack space which is independent of current task stack, this mechanism can be used to save stack space wasted by tasks which call a common function with intensive stack usage such as `printf`. The given function can -be called inside the macro :cpp:func:`ESP_EXECUTE_EXPRESSION_WITH_STACK` -it will redirect the target function to be executed using the space -allocated by the user. +be called inside the shared stack space which is a callback function +deferred by calling :cpp:func:`esp_execute_shared_stack_function`, +passing that function as parameter Usage ----- -:cpp:func:`ESP_EXECUTE_EXPRESSION_WITH_STACK` takes three arguments, +:cpp:func:`esp_execute_shared_stack_function` takes four arguments, a mutex object allocated by the caller, which is used to protect if the same function shares its allocated stack, a pointer to the top -of stack used to that fuction, and the function itself, note the -function is passed exactly in the same way as do when you call -it on a regular way. +of stack used to that fuction, the size in bytes of stack and, a pointer +to a user function where the shared stack space will reside, after calling +the function, the user defined function will be deferred as a callback +where functions can be called using the user allocated space without +taking space from current task stack. The usage may looks like the code below: .. code-block:: c + void external_stack_function(void) + { + printf("Executing this printf from external stack! \n"); + } + //Let's suppose we wanting to call printf using a separated stack space //allowing app to reduce its stack size. void app_main() @@ -39,9 +46,11 @@ The usage may looks like the code below: assert(printf_lock != NULL); //Call the desired function using the macro helper: - ESP_EXECUTE_EXPRESSION_WITH_STACK(printf_lock, - shared_stack, - printf("Executing this from external stack! \n")); + esp_execute_shared_stack_function(printf_lock, + shared_stack, + 8192, + external_stack_function); + vSemaphoreDelete(printf_lock); free(shared_stack); } @@ -51,6 +60,4 @@ The usage may looks like the code below: API Reference ------------- -.. include:: /_build/inc/esp_expression_with_stack.inc - - +.. include:: /_build/inc/esp_expression_with_stack.inc \ No newline at end of file diff --git a/docs/en/api-reference/system/index.rst b/docs/en/api-reference/system/index.rst index e15d1de207..cedee5901a 100644 --- a/docs/en/api-reference/system/index.rst +++ b/docs/en/api-reference/system/index.rst @@ -16,8 +16,8 @@ System API Heap Memory Allocation Heap Memory Debugging High Resolution Timer - :esp32: Himem (large external SPI RAM) API - :esp32: Inter-Processor Call + Himem (large external SPI RAM) API + Inter-Processor Call Call function with external stack Interrupt Allocation Logging diff --git a/docs/zh_CN/api-reference/system/esp_expression_with_stack.rst b/docs/zh_CN/api-reference/system/esp_expression_with_stack.rst deleted file mode 100644 index ee9cb17b22..0000000000 --- a/docs/zh_CN/api-reference/system/esp_expression_with_stack.rst +++ /dev/null @@ -1 +0,0 @@ -.. include:: ../../../en/api-reference/system/esp_expression_with_stack.rst \ No newline at end of file diff --git a/docs/zh_CN/api-reference/system/esp_function_with_shared_stack.rst b/docs/zh_CN/api-reference/system/esp_function_with_shared_stack.rst new file mode 100644 index 0000000000..22ff5f93e3 --- /dev/null +++ b/docs/zh_CN/api-reference/system/esp_function_with_shared_stack.rst @@ -0,0 +1 @@ +.. include:: ../../../en/api-reference/system/esp_function_with_shared_stack.rst \ No newline at end of file