From 1991aa6da033e2deb9cff4b980199f14294295bb Mon Sep 17 00:00:00 2001 From: Erhan Kurubas Date: Wed, 5 Nov 2025 14:15:55 +0100 Subject: [PATCH 1/2] fix(espcoredump): prevent double exception during int_wdt panic with custom stack --- components/espcoredump/CMakeLists.txt | 5 +- .../include_core_dump/esp_core_dump_port.h | 19 +- .../port/riscv/esp_core_dump_port_impl.h | 64 ------- .../port/xtensa/esp_core_dump_port_impl.h | 99 ---------- components/espcoredump/src/core_dump_common.c | 166 ++++++++--------- components/espcoredump/src/core_dump_elf.c | 1 - .../src/port/riscv/core_dump_port.c | 87 ++++++++- .../src/port/xtensa/core_dump_port.c | 16 +- .../src/port/xtensa/core_dump_stack_switch.S | 170 ++++++++++++++++++ components/xtensa/include/xt_instr_macros.h | 10 +- tools/test_apps/system/panic/pytest_panic.py | 4 +- 11 files changed, 371 insertions(+), 270 deletions(-) delete mode 100644 components/espcoredump/include_core_dump/port/riscv/esp_core_dump_port_impl.h delete mode 100644 components/espcoredump/include_core_dump/port/xtensa/esp_core_dump_port_impl.h create mode 100644 components/espcoredump/src/port/xtensa/core_dump_stack_switch.S diff --git a/components/espcoredump/CMakeLists.txt b/components/espcoredump/CMakeLists.txt index cc4cd78977..e28d52878c 100644 --- a/components/espcoredump/CMakeLists.txt +++ b/components/espcoredump/CMakeLists.txt @@ -19,13 +19,12 @@ set(priv_includes "include_core_dump") idf_build_get_property(target IDF_TARGET) if(CONFIG_IDF_TARGET_ARCH_XTENSA) - list(APPEND srcs "src/port/xtensa/core_dump_port.c") + list(APPEND srcs "src/port/xtensa/core_dump_port.c" + "src/port/xtensa/core_dump_stack_switch.S") list(APPEND includes "include/port/xtensa") - list(APPEND priv_includes "include_core_dump/port/xtensa") elseif(CONFIG_IDF_TARGET_ARCH_RISCV) list(APPEND srcs "src/port/riscv/core_dump_port.c") list(APPEND includes "include/port/riscv") - list(APPEND priv_includes "include_core_dump/port/riscv") endif() idf_component_register(SRCS ${srcs} diff --git a/components/espcoredump/include_core_dump/esp_core_dump_port.h b/components/espcoredump/include_core_dump/esp_core_dump_port.h index 39658b29dd..c5bdd62a9d 100644 --- a/components/espcoredump/include_core_dump/esp_core_dump_port.h +++ b/components/espcoredump/include_core_dump/esp_core_dump_port.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -20,7 +20,6 @@ #include "freertos/FreeRTOS.h" #include "esp_app_format.h" #include "esp_core_dump_types.h" -#include "esp_core_dump_port_impl.h" #include "esp_core_dump.h" #ifdef __cplusplus @@ -106,10 +105,10 @@ uint32_t esp_core_dump_get_stack(core_dump_task_header_t* task_snapshot, * @note The goal of this function is to check whether the task passed is the * task that crashed or not. If this is the case and if it didn't crash * within an ISR, its stack pointer will be set to the panic frame, - * containing all the registers values when the error occured. This + * containing all the registers values when the error occurred. This * function also checks if the TCB address is sane or not. * - * @param task Pointer to the frame exception generated when the panic occured. + * @param task Pointer to the frame exception generated when the panic occurred. * * @return True if the TCB is sane, false else. */ @@ -120,7 +119,7 @@ bool esp_core_dump_check_task(core_dump_task_header_t *task); * * @note In practice, this function is used to fill the ELF file with the * PR_STATUS sections for all the existing tasks. This structure - * contains the CPU registers value when the exception occured. + * contains the CPU registers value when the exception occurred. * * @param task Task to dump the registers from. * @param reg_dump Pointer that will be filled with the registers dump. @@ -182,6 +181,16 @@ void esp_core_dump_summary_parse_backtrace_info(esp_core_dump_bt_info_t *bt_info const void *paddr, uint32_t stack_size); #endif /* CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH && CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF */ +/** + * @brief Stack switching implemented in C for RISC-V, Assembly for Xtensa + * + * This function switches to the coredump stack, writes the ELF core dump, + * reports stack usage, and restores the original stack. + * @param new_stack new stack buffer address + * @param new_sp aligned stack pointer address + */ +void esp_core_dump_port_write(uint32_t new_stack, uint32_t new_sp); + #ifdef __cplusplus } #endif diff --git a/components/espcoredump/include_core_dump/port/riscv/esp_core_dump_port_impl.h b/components/espcoredump/include_core_dump/port/riscv/esp_core_dump_port_impl.h deleted file mode 100644 index fa2db73afb..0000000000 --- a/components/espcoredump/include_core_dump/port/riscv/esp_core_dump_port_impl.h +++ /dev/null @@ -1,64 +0,0 @@ -/* - * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD - * - * SPDX-License-Identifier: Apache-2.0 - */ -#ifndef ESP_CORE_DUMP_PORT_IMPL_H_ -#define ESP_CORE_DUMP_PORT_IMPL_H_ - -/** - * @file - * @brief Core dump port interface implementation for RISC-V. - */ - -#include "sdkconfig.h" -#include "esp_core_dump_types.h" -#include "esp_app_format.h" - -#ifdef __cplusplus -extern "C" { -#endif - -/** - * @brief Define the type that will be used to describe the current context when - * doing a backup of the current stack. This same structure is used to restore the stack. - */ -typedef struct { - uint32_t sp; -#if CONFIG_ESP_SYSTEM_HW_STACK_GUARD - uint32_t sp_min; - uint32_t sp_max; -#endif // CONFIG_ESP_SYSTEM_HW_STACK_GUARD -} core_dump_stack_context_t; - -/** - * @brief Set the stack pointer to the address passed as a parameter. - * @note This function must be inlined. - * - * @param new_sp New stack pointer to set in sp register. - * @param old_ctx CPU context, related to SP, to fill. It will be given back when restoring SP. - */ -FORCE_INLINE_ATTR void esp_core_dump_replace_sp(void* new_sp, core_dump_stack_context_t* old_ctx) -{ - asm volatile("mv %0, sp \n\t\ - mv sp, %1 \n\t\ - " - : "=&r"(old_ctx->sp) - : "r"(new_sp)); -} - -/** - * @brief Restore the stack pointer that was returned when calling `esp_core_dump_replace_sp()` function. - * - * @param ctx CPU context, related to SP, to restore. - */ -FORCE_INLINE_ATTR void esp_core_dump_restore_sp(core_dump_stack_context_t* old_ctx) -{ - asm volatile("mv sp, %0 \n\t" :: "r"(old_ctx->sp)); -} - -#ifdef __cplusplus -} -#endif - -#endif diff --git a/components/espcoredump/include_core_dump/port/xtensa/esp_core_dump_port_impl.h b/components/espcoredump/include_core_dump/port/xtensa/esp_core_dump_port_impl.h deleted file mode 100644 index 6d62f59495..0000000000 --- a/components/espcoredump/include_core_dump/port/xtensa/esp_core_dump_port_impl.h +++ /dev/null @@ -1,99 +0,0 @@ -/* - * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD - * - * SPDX-License-Identifier: Apache-2.0 - */ -#ifndef ESP_CORE_DUMP_PORT_IMPL_H_ -#define ESP_CORE_DUMP_PORT_IMPL_H_ - -/** - * @file - * @brief Core dump port interface implementation for Xtensa boards. - */ -#include "esp_core_dump_types.h" -#include "esp_app_format.h" -/** - * Included for SET_STACK macro - */ -#include -#include - -#ifdef __cplusplus -extern "C" { -#endif - -/** - * @brief Define the type that will be used to describe the current context when - * doing a backup of the current stack. This same structure is used to restore the stack. - */ -typedef struct { - uint32_t sp; - uint32_t a0; - uint32_t ps; - uint32_t windowbase; - uint32_t windowstart; -} core_dump_stack_context_t; - -/** - * @brief Set the stack pointer to the address passed as a parameter. - * @note This function must be inlined. - * - * @param new_sp New stack pointer to set in sp register. - * @param old_ctx CPU context, related to SP, to fill. It will be given back when restoring SP. - */ -FORCE_INLINE_ATTR void esp_core_dump_replace_sp(void* new_sp, core_dump_stack_context_t* old_ctx) -{ - /* We have to spill all the windows to the stack first as the new stack pointer - * represents a clean new environment. */ - xthal_window_spill(); - - /* Backup the special registers PS, WindowBase and WindowStart. We will need to restore them later */ - asm volatile("mov %0, sp \n" \ - "mov %1, a0 \n" \ - "rsr.ps %2 \n"\ - "rsr.windowbase %3 \n"\ - "rsr.windowstart %4 \n"\ - : "=r"(old_ctx->sp), - "=r"(old_ctx->a0), - "=r"(old_ctx->ps), - "=r"(old_ctx->windowbase), - "=r"(old_ctx->windowstart) :); - - /* Set the new stack */ - SET_STACK(new_sp); -} - -/** - * @brief Restore the stack pointer that was returned when calling `esp_core_dump_replace_sp()` function. - * - * @param ctx CPU context, related to SP, to restore. - */ -FORCE_INLINE_ATTR void esp_core_dump_restore_sp(core_dump_stack_context_t* old_ctx) -{ - /* Start by disabling WindowOverflowEnable bit from PS to make sure we won't get a Window Overflow exception - * restoring WindowBase and WindowStart registers */ - const uint32_t ps_woe = old_ctx->ps & ~(PS_WOE_MASK); - asm volatile(\ - "wsr.ps %0 \n"\ - "rsync \n"\ - "wsr.windowbase %1 \n"\ - "rsync \n"\ - "wsr.windowstart %2 \n"\ - "rsync \n"\ - "mov sp, %3 \n" \ - "mov a0, %4 \n" \ - "wsr.ps %5 \n"\ - "rsync \n"\ - :: "r"(ps_woe), - "r"(old_ctx->windowbase), - "r"(old_ctx->windowstart), - "r"(old_ctx->sp), - "r"(old_ctx->a0), - "r"(old_ctx->ps)); -} - -#ifdef __cplusplus -} -#endif - -#endif diff --git a/components/espcoredump/src/core_dump_common.c b/components/espcoredump/src/core_dump_common.c index 7442d85688..14aad492b7 100644 --- a/components/espcoredump/src/core_dump_common.c +++ b/components/espcoredump/src/core_dump_common.c @@ -1,8 +1,11 @@ /* - * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ +#include "sdkconfig.h" + +#include #include #include #include "sdkconfig.h" @@ -12,9 +15,7 @@ #include "esp_rom_sys.h" #include "esp_core_dump_port.h" #include "esp_core_dump_common.h" -#if CONFIG_ESP_SYSTEM_HW_STACK_GUARD -#include "esp_private/hw_stack_guard.h" -#endif // CONFIG_ESP_SYSTEM_HW_STACK_GUARD +#include "esp_cpu.h" const static char TAG[] __attribute__((unused)) = "esp_core_dump_common"; @@ -55,9 +56,34 @@ extern int _coredump_rtc_fast_start; extern int _coredump_rtc_fast_end; #endif +static void* s_exc_frame = NULL; +static uint32_t s_coredump_sp = 0; + /** - * @brief In the menconfig, it is possible to specify a specific stack size for - * core dump generation. + * @brief Configuration validation: If USE_STACK_SIZE is enabled, STACK_SIZE must be > 0 + */ +#if CONFIG_ESP_COREDUMP_USE_STACK_SIZE && CONFIG_ESP_COREDUMP_STACK_SIZE == 0 +#error "CONFIG_ESP_COREDUMP_STACK_SIZE must not be 0 when CONFIG_ESP_COREDUMP_USE_STACK_SIZE is enabled" +#endif + +/** + * @brief Write ELF core dump and log any errors. + * + * This function wraps esp_core_dump_write_elf() and handles error logging. + * Can be called from both C and assembly code. + * + * @return esp_err_t ESP_OK on success, error code otherwise + */ +void esp_core_dump_write_elf_and_check(void) +{ + esp_err_t err = esp_core_dump_store(); + if (err != ESP_OK) { + ESP_COREDUMP_LOGE("Core dump write failed with error=%d", err); + } +} + +/** + * @brief In the menconfig, it is possible to specify a specific stack size for core dump generation. */ #if CONFIG_ESP_COREDUMP_STACK_SIZE > 0 @@ -67,7 +93,7 @@ extern int _coredump_rtc_fast_end; */ #if LOG_LOCAL_LEVEL >= ESP_LOG_DEBUG /* Increase stack size in verbose mode */ -#define ESP_COREDUMP_STACK_SIZE (CONFIG_ESP_COREDUMP_STACK_SIZE+100) +#define ESP_COREDUMP_STACK_SIZE (CONFIG_ESP_COREDUMP_STACK_SIZE + 100) #else #define ESP_COREDUMP_STACK_SIZE CONFIG_ESP_COREDUMP_STACK_SIZE #endif @@ -75,43 +101,15 @@ extern int _coredump_rtc_fast_end; #define COREDUMP_STACK_FILL_BYTE (0xa5U) static uint8_t s_coredump_stack[ESP_COREDUMP_STACK_SIZE]; -static uint8_t* s_core_dump_sp = NULL; -static core_dump_stack_context_t s_stack_context; -/** - * @brief Function setting up the core dump stack. - * - * @note This function **must** be aligned as it modifies the - * stack pointer register. - */ -FORCE_INLINE_ATTR void esp_core_dump_setup_stack(void) +void esp_core_dump_setup_stack(void) { - s_core_dump_sp = (uint8_t *)((uint32_t)(s_coredump_stack + ESP_COREDUMP_STACK_SIZE - 1) & ~0xf); + s_coredump_sp = (uint32_t)(s_coredump_stack + ESP_COREDUMP_STACK_SIZE - 1) & ~0xf; memset(s_coredump_stack, COREDUMP_STACK_FILL_BYTE, ESP_COREDUMP_STACK_SIZE); - /* watchpoint 1 can be used for task stack overflow detection, reuse it, it is no more necessary */ - //esp_cpu_clear_watchpoint(1); - //esp_cpu_set_watchpoint(1, s_coredump_stack, 1, ESP_WATCHPOINT_STORE); - -#if CONFIG_ESP_SYSTEM_HW_STACK_GUARD - /* Save the current area we are watching to restore it later */ - esp_hw_stack_guard_get_bounds(xPortGetCoreID(), &s_stack_context.sp_min, &s_stack_context.sp_max); - /* Since the stack is going to change, make sure we disable protection or an exception would be triggered */ - esp_hw_stack_guard_monitor_stop(); -#endif // CONFIG_ESP_SYSTEM_HW_STACK_GUARD - - /* Replace the stack pointer depending on the architecture, but save the - * current stack pointer, in order to be able too restore it later. - * This function must be inlined. */ - esp_core_dump_replace_sp(s_core_dump_sp, &s_stack_context); - ESP_COREDUMP_LOGI("Backing up stack @ 0x%" PRIx32 " and use core dump stack @ %p", - s_stack_context.sp, esp_cpu_get_sp()); - -#if CONFIG_ESP_SYSTEM_HW_STACK_GUARD - /* Re-enable the stack guard to check if the stack is big enough for coredump generation */ - esp_hw_stack_guard_set_bounds((uint32_t) s_coredump_stack, (uint32_t) s_core_dump_sp); - esp_hw_stack_guard_monitor_start(); -#endif // CONFIG_ESP_SYSTEM_HW_STACK_GUARD + /* Stack overflow detection. Watchpoint is set to the end of the core dump stack */ + esp_cpu_clear_watchpoint(1); + esp_cpu_set_watchpoint(1, s_coredump_stack, 1, ESP_CPU_WATCHPOINT_STORE); } /** @@ -132,73 +130,60 @@ FORCE_INLINE_ATTR uint32_t esp_core_dump_free_stack_space(const uint8_t *pucStac } /** - * @brief Print how many bytes have been used on the stack to create the core - * dump. + * @brief Print how many bytes have been used on the stack to create the core dump. + * */ -FORCE_INLINE_ATTR void esp_core_dump_report_stack_usage(void) +void esp_core_dump_report_stack_usage(uint32_t new_sp) { -#if CONFIG_ESP_COREDUMP_LOGS - uint32_t bytes_free = esp_core_dump_free_stack_space(s_coredump_stack); + uint32_t __attribute__((unused)) bytes_free = esp_core_dump_free_stack_space(s_coredump_stack); ESP_COREDUMP_LOGI("Core dump used %" PRIu32 " bytes on stack. %" PRIu32 " bytes left free.", - s_core_dump_sp - s_coredump_stack - bytes_free, bytes_free); -#endif + new_sp - (uint32_t)s_coredump_stack - bytes_free, bytes_free); +} - /* Restore the stack pointer. */ - ESP_COREDUMP_LOGI("Restoring stack @ 0x%" PRIx32, s_stack_context.sp); -#if CONFIG_ESP_SYSTEM_HW_STACK_GUARD - esp_hw_stack_guard_monitor_stop(); -#endif // CONFIG_ESP_SYSTEM_HW_STACK_GUARD - esp_core_dump_restore_sp(&s_stack_context); -#if CONFIG_ESP_SYSTEM_HW_STACK_GUARD - /* Monitor the same stack area that was set before replacing the stack pointer */ - esp_hw_stack_guard_set_bounds(s_stack_context.sp_min, s_stack_context.sp_max); - esp_hw_stack_guard_monitor_start(); -#endif // CONFIG_ESP_SYSTEM_HW_STACK_GUARD +void esp_core_dump_report_backup_stack(uint32_t old_sp) +{ + ESP_COREDUMP_LOGI("Backing up stack @ 0x%" PRIx32 " and use core dump stack @ %p", + old_sp, esp_cpu_get_sp()); +} + +void esp_core_dump_report_restore_stack(uint32_t old_sp) +{ + ESP_COREDUMP_LOGI("Restoring stack @ 0x%" PRIx32, old_sp); } #else // CONFIG_ESP_COREDUMP_STACK_SIZE == 0 -/* Here, we are not going to use a custom stack for coredump. Make sure the current configuration doesn't require one. */ -#if CONFIG_ESP_COREDUMP_USE_STACK_SIZE -#pragma error "CONFIG_ESP_COREDUMP_STACK_SIZE must not be 0 in the current configuration" -#endif // ESP_COREDUMP_USE_STACK_SIZE +static uint8_t *s_coredump_stack = NULL; -FORCE_INLINE_ATTR void esp_core_dump_setup_stack(void) +void esp_core_dump_setup_stack(void) { /* No stack setup is needed */ } + +/** + * @brief Setup watchpoint for stack overflow detection when no custom stack is configured. + * + * If we are in ISR context, sets up a watchpoint at the end of the ISR stack. + * For tasks, stack overflow detection should be enabled in menuconfig. + * TODO: If not enabled in menuconfig enable it ourselves. + */ +static void esp_core_dump_enable_stack_overflow_detection(void) { - /* If we are in ISR set watchpoint to the end of ISR stack */ if (esp_core_dump_in_isr_context()) { - uint8_t* topStack = esp_core_dump_get_isr_stack_top(); + uint8_t *topStack = esp_core_dump_get_isr_stack_top(); esp_cpu_clear_watchpoint(1); - esp_cpu_set_watchpoint(1, topStack + xPortGetCoreID()*configISR_STACK_SIZE, 1, ESP_CPU_WATCHPOINT_STORE); - } else { - /* for tasks user should enable stack overflow detection in menuconfig - TODO: if not enabled in menuconfig enable it ourselves */ + esp_cpu_set_watchpoint(1, topStack + xPortGetCoreID() * configISR_STACK_SIZE, 1, ESP_CPU_WATCHPOINT_STORE); } } -FORCE_INLINE_ATTR void esp_core_dump_report_stack_usage(void) +void esp_core_dump_port_write(uint32_t new_stack, uint32_t new_sp) { + (void)new_stack; + (void)new_sp; + + esp_core_dump_enable_stack_overflow_detection(); + esp_core_dump_write_elf_and_check(); } #endif // CONFIG_ESP_COREDUMP_STACK_SIZE > 0 -static void* s_exc_frame = NULL; - -inline static void esp_core_dump_write_internal(panic_info_t *info) -{ - bool isr_context = esp_core_dump_in_isr_context(); - - s_exc_frame = (void *)info->frame; - - esp_core_dump_setup_stack(); - esp_core_dump_port_init(info, isr_context); - esp_err_t err = esp_core_dump_store(); - if (err != ESP_OK) { - ESP_COREDUMP_LOGE("Core dump write failed with error=%d", err); - } - esp_core_dump_report_stack_usage(); -} - void __attribute__((weak)) esp_core_dump_init(void) { /* do nothing by default */ @@ -340,8 +325,13 @@ void esp_core_dump_write(panic_info_t *info) return; #endif + s_exc_frame = (void *)info->frame; esp_core_dump_print_write_start(); - esp_core_dump_write_internal(info); + + esp_core_dump_port_init(info, esp_core_dump_in_isr_context()); + esp_core_dump_setup_stack(); + esp_core_dump_port_write((uint32_t)s_coredump_stack, s_coredump_sp); + esp_core_dump_print_write_end(); } diff --git a/components/espcoredump/src/core_dump_elf.c b/components/espcoredump/src/core_dump_elf.c index 9d0037249e..5f74e890e4 100644 --- a/components/espcoredump/src/core_dump_elf.c +++ b/components/espcoredump/src/core_dump_elf.c @@ -10,7 +10,6 @@ #include "sdkconfig.h" #include "core_dump_checksum.h" #include "esp_core_dump_port.h" -#include "esp_core_dump_port_impl.h" #include "esp_core_dump_common.h" #include "hal/efuse_hal.h" #include "esp_task_wdt.h" diff --git a/components/espcoredump/src/port/riscv/core_dump_port.c b/components/espcoredump/src/port/riscv/core_dump_port.c index 7d872ce62e..38c6d74910 100644 --- a/components/espcoredump/src/port/riscv/core_dump_port.c +++ b/components/espcoredump/src/port/riscv/core_dump_port.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -447,4 +447,89 @@ void esp_core_dump_summary_parse_backtrace_info(esp_core_dump_bt_info_t *bt_info #endif /* #if CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH && CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF */ +#if CONFIG_ESP_COREDUMP_STACK_SIZE > 0 + +#if CONFIG_ESP_SYSTEM_HW_STACK_GUARD +#include "esp_private/hw_stack_guard.h" +#endif + +/** + * @brief Define the type that will be used to describe the current context when + * doing a backup of the current stack. This same structure is used to restore the stack. + */ +typedef struct { + uint32_t sp; +#if CONFIG_ESP_SYSTEM_HW_STACK_GUARD + uint32_t sp_min; + uint32_t sp_max; +#endif // CONFIG_ESP_SYSTEM_HW_STACK_GUARD +} core_dump_stack_context_t; + +static core_dump_stack_context_t s_stack_context; + +/* Helper functions defined in core_dump_common.c */ +void esp_core_dump_report_backup_stack(uint32_t old_sp); +void esp_core_dump_report_restore_stack(uint32_t old_sp); +void esp_core_dump_write_elf_and_check(void); +void esp_core_dump_report_stack_usage(uint32_t new_sp); + +/** + * @brief Function setting up the core dump stack. + * + * @note This function **must** be inlined as it modifies the stack pointer. + */ +FORCE_INLINE_ATTR void esp_core_dump_replace_stack(uint32_t new_stack, uint32_t new_sp) +{ +#if CONFIG_ESP_SYSTEM_HW_STACK_GUARD + /* Save the current area we are watching to restore it later */ + esp_hw_stack_guard_get_bounds(xPortGetCoreID(), &s_stack_context.sp_min, &s_stack_context.sp_max); + /* Since the stack is going to change, make sure we disable protection or an exception would be triggered */ + esp_hw_stack_guard_monitor_stop(); +#endif + + /* Save the current stack pointer and replace it with the core dump stack pointer */ + asm volatile("mv %0, sp \n\t\ + mv sp, %1 \n\t\ + " + : "=&r"(s_stack_context.sp) + : "r"(new_sp)); + + esp_core_dump_report_backup_stack(s_stack_context.sp); + +#if CONFIG_ESP_SYSTEM_HW_STACK_GUARD + /* Re-enable the stack guard to check if the stack is big enough for coredump generation */ + esp_hw_stack_guard_set_bounds(new_stack, new_sp); + esp_hw_stack_guard_monitor_start(); +#endif +} + +FORCE_INLINE_ATTR void esp_core_dump_restore_stack(void) +{ + /* Restore the stack pointer. */ + esp_core_dump_report_restore_stack(s_stack_context.sp); + +#if CONFIG_ESP_SYSTEM_HW_STACK_GUARD + esp_hw_stack_guard_monitor_stop(); +#endif + + /* Restore the stack pointer to the original value */ + asm volatile("mv sp, %0 \n\t" :: "r"(s_stack_context.sp)); + +#if CONFIG_ESP_SYSTEM_HW_STACK_GUARD + /* Monitor the same stack area that was set before replacing the stack pointer */ + esp_hw_stack_guard_set_bounds(s_stack_context.sp_min, s_stack_context.sp_max); + esp_hw_stack_guard_monitor_start(); +#endif +} + +void esp_core_dump_port_write(uint32_t new_stack, uint32_t new_sp) +{ + esp_core_dump_replace_stack(new_stack, new_sp); + esp_core_dump_write_elf_and_check(); + esp_core_dump_report_stack_usage(new_sp); + esp_core_dump_restore_stack(); +} + +#endif // CONFIG_ESP_COREDUMP_STACK_SIZE > 0 + #endif diff --git a/components/espcoredump/src/port/xtensa/core_dump_port.c b/components/espcoredump/src/port/xtensa/core_dump_port.c index 3aff19c537..0e6a54dcf3 100644 --- a/components/espcoredump/src/port/xtensa/core_dump_port.c +++ b/components/espcoredump/src/port/xtensa/core_dump_port.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -565,4 +565,16 @@ void esp_core_dump_summary_parse_backtrace_info(esp_core_dump_bt_info_t *bt_info #endif /* #if CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH && CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF */ -#endif +#if CONFIG_ESP_COREDUMP_STACK_SIZE > 0 + +/* Helper function defined in core_dump_stack_switch.S */ +void esp_core_dump_port_write_on_new_stack(uint32_t new_sp); + +void esp_core_dump_port_write(uint32_t new_stack, uint32_t new_sp) +{ + (void)new_stack; + esp_core_dump_port_write_on_new_stack(new_sp); +} +#endif // CONFIG_ESP_COREDUMP_STACK_SIZE > 0 + +#endif // CONFIG_ESP_COREDUMP_ENABLE diff --git a/components/espcoredump/src/port/xtensa/core_dump_stack_switch.S b/components/espcoredump/src/port/xtensa/core_dump_stack_switch.S new file mode 100644 index 0000000000..063227fa54 --- /dev/null +++ b/components/espcoredump/src/port/xtensa/core_dump_stack_switch.S @@ -0,0 +1,170 @@ +/* + * SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * @file + * @brief Assembly implementation for switching to a new stack during coredump. + * + * This implementation properly handles window spilling without lowering the + * interrupt level, making it safe to use from high-priority interrupt contexts + * (e.g., interrupt watchdog at level 4). + */ + +#include +#include +#include +#include "xt_asm_utils.h" +#include "xt_instr_macros.h" /* For SET_STACK macro */ +#include "sdkconfig.h" + + .extern esp_core_dump_write_elf_and_check + .extern esp_core_dump_report_stack_usage + .extern esp_core_dump_report_backup_stack + .extern esp_core_dump_report_restore_stack + +/** + * @brief Stack context structure layout (offsets in bytes) + * + * This structure defines the layout of s_stack_context used for saving/restoring + * the original stack context when switching to the coredump stack. + */ + .equ STACK_CTX_SP, 0 + .equ STACK_CTX_A0, 4 + .equ STACK_CTX_PS, 8 + .equ STACK_CTX_WINDOWBASE, 12 + .equ STACK_CTX_WINDOWSTART, 16 + .equ STACK_CTX_SIZE, 20 + + .section .bss + .align 4 + .global s_stack_context + .type s_stack_context,@object +s_stack_context: + .space STACK_CTX_SIZE + .size s_stack_context, STACK_CTX_SIZE + + .text + +/** + * @brief Execute core dump write on a new stack + * + * This function spills register windows and switches to new stack. + * Calls C functions and restores original stack and context. + */ +#if CONFIG_ESP_COREDUMP_STACK_SIZE > 0 + + .global esp_core_dump_port_write_on_new_stack + .type esp_core_dump_port_write_on_new_stack,@function + .align 4 +esp_core_dump_port_write_on_new_stack: + +#ifndef __XTENSA_CALL0_ABI__ + entry a1, 32 + + /* Save necessary parameters on stack before window spilling */ + s32i a0, a1, 0 /* Save return address */ + s32i a2, a1, 4 /* Save coredump_sp parameter */ + + /* Prepare for window spilling: + * - Clear PS.EXCM and set PS.WOE to enable window overflow exceptions + * - Raise INTLEVEL to at least XCHAL_EXCM_LEVEL (to maintain interrupt masking) + * - Save/restore EPC1 (clobbered by window overflow exceptions) + * - Only use a0-a3 here (a4-a15 may hold live data from previous windows) + */ + rsr a2, PS /* Save PS to be restored after SPILL_ALL_WINDOWS */ + movi a0, PS_INTLEVEL_MASK + and a3, a2, a0 /* Extract current INTLEVEL */ + bgeui a3, XCHAL_EXCM_LEVEL, 1f /* Ensure INTLEVEL >= XCHAL_EXCM_LEVEL */ + movi a3, XCHAL_EXCM_LEVEL +1: + movi a0, PS_UM | PS_WOE /* Clear EXCM, enable WOE */ + or a3, a3, a0 + wsr a3, PS + rsr a0, EPC1 /* Save EPC1 to be restored after SPILL_ALL_WINDOWS */ + + SPILL_ALL_WINDOWS + + /* Restore PS and EPC1 */ + wsr a2, PS + rsync + wsr a0, EPC1 + + /* Restore return address and save context to s_stack_context */ + mov a6, a1 /* a6 = SP */ + l32i a7, a1, 0 /* a7 = return address (from stack) */ + l32i a12, a1, 4 /* a12 = coredump_sp (from stack) */ + rsr a8, PS + rsr a9, WINDOWBASE + rsr a10, WINDOWSTART + movi a11, s_stack_context + s32i a6, a11, STACK_CTX_SP /* Save SP */ + s32i a7, a11, STACK_CTX_A0 /* Save A0 */ + s32i a8, a11, STACK_CTX_PS /* Save PS */ + s32i a9, a11, STACK_CTX_WINDOWBASE /* Save WindowBase */ + s32i a10, a11, STACK_CTX_WINDOWSTART /* Save WindowStart */ + + /* Switch to coredump stack (use saved coredump_sp from stack) */ + mov a6, a12 /* a6 = coredump_sp */ + SET_STACK a6, a7, a8 + + /* Report backup stack */ + l32i a10, a11, STACK_CTX_SP /* a10 = old SP */ + movi a14, esp_core_dump_report_backup_stack + callx8 a14 + + /* Write ELF core dump */ + movi a14, esp_core_dump_write_elf_and_check + callx8 a14 + + /* Report stack usage */ + mov a10, a12 /* a10 = coredump_sp */ + movi a14, esp_core_dump_report_stack_usage + callx8 a14 + + /* Report restore stack */ + l32i a10, a11, STACK_CTX_SP /* a10 = old SP */ + movi a14, esp_core_dump_report_restore_stack + callx8 a14 + + /* Load original context from s_stack_context */ + movi a11, s_stack_context + l32i a2, a11, STACK_CTX_SP /* Load SP */ + l32i a3, a11, STACK_CTX_A0 /* Load A0 */ + l32i a4, a11, STACK_CTX_PS /* Load PS */ + l32i a5, a11, STACK_CTX_WINDOWBASE /* Load WindowBase */ + l32i a6, a11, STACK_CTX_WINDOWSTART /* Load WindowStart */ + + /* Write PS with WOE disabled */ + movi a8, ~PS_WOE_MASK + and a7, a4, a8 + wsr a7, PS + rsync + + /* Restore WindowBase */ + wsr a5, WINDOWBASE + rsync + + /* Restore WindowStart */ + wsr a6, WINDOWSTART + rsync + + /* Restore SP */ + mov a1, a2 + + /* Restore A0 */ + mov a0, a3 + + /* Restore PS (final with original WOE setting) */ + wsr a4, PS + rsync + + retw + +#else + #error "this code is written for Window ABI" +#endif + +#endif // CONFIG_ESP_COREDUMP_STACK_SIZE > 0 diff --git a/components/xtensa/include/xt_instr_macros.h b/components/xtensa/include/xt_instr_macros.h index e3a1199020..773fd009e4 100644 --- a/components/xtensa/include/xt_instr_macros.h +++ b/components/xtensa/include/xt_instr_macros.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -53,10 +53,10 @@ */ #ifdef __ASSEMBLER__ .macro SET_STACK new_sp tmp1 tmp2 - addi tmp1, new_sp, -SAVE_AREA_OFFSET - addi tmp2, tmp1, -BASE_AREA_SP_OFFSET - s32i new_sp, tmp2, 0 - addi new_sp, tmp1, 0 + addi \tmp1, \new_sp, -SAVE_AREA_OFFSET + addi \tmp2, \tmp1, -BASE_AREA_SP_OFFSET + s32i \new_sp, \tmp2, 0 + addi \new_sp, \tmp1, 0 rsr.ps \tmp1 movi \tmp2, ~(PS_WOE_MASK | PS_OWB_MASK | PS_CALLINC_MASK) and \tmp1, \tmp1, \tmp2 diff --git a/tools/test_apps/system/panic/pytest_panic.py b/tools/test_apps/system/panic/pytest_panic.py index 51d4d00677..6d0281fce1 100644 --- a/tools/test_apps/system/panic/pytest_panic.py +++ b/tools/test_apps/system/panic/pytest_panic.py @@ -39,12 +39,12 @@ CONFIGS = list( 'coredump_flash_elf_soft_sha', 'coredump_uart_bin_crc', 'coredump_uart_elf_crc', + 'coredump_flash_custom_stack', 'gdbstub', 'panic', ], TARGETS_ALL, - ), - itertools.product(['coredump_flash_custom_stack'], TARGETS_RISCV), + ) ) ) From 2f63b7bc000893c369d0f4fc5b419fb6a9f703d3 Mon Sep 17 00:00:00 2001 From: Erhan Kurubas Date: Fri, 12 Dec 2025 11:08:13 +0100 Subject: [PATCH 2/2] fix(xtensa): Fix clang assembler errors in STRUCT_AFIELD_A macro --- components/xtensa/include/xtensa/xtruntime-frames.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/components/xtensa/include/xtensa/xtruntime-frames.h b/components/xtensa/include/xtensa/xtruntime-frames.h index b78095e0dd..3557e0c17b 100644 --- a/components/xtensa/include/xtensa/xtruntime-frames.h +++ b/components/xtensa/include/xtensa/xtruntime-frames.h @@ -37,8 +37,10 @@ #define STRUCT_FIELD(ctype,size,pre,name) .set pre##name, XT_STRUCT_OFFSET; .set XT_STRUCT_OFFSET, pre##name + size #define STRUCT_AFIELD(ctype,size,pre,name,n) .set pre##name, XT_STRUCT_OFFSET;\ .set XT_STRUCT_OFFSET, pre##name + (size)*(n); -#define STRUCT_AFIELD_A(ctype,size,align,pre,name,n) .set pre##name, XT_STRUCT_OFFSET\ - .ifgt (align-1); .set pre##name, XT_STRUCT_OFFSET + (align - (XT_STRUCT_OFFSET & (align-1))); .endif\ +#define STRUCT_AFIELD_A(ctype,size,align,pre,name,n) .set pre##name, XT_STRUCT_OFFSET; \ + .ifgt (align-1); \ + .set pre##name, XT_STRUCT_OFFSET + (align - (XT_STRUCT_OFFSET & (align-1))); \ + .endif; \ .set XT_STRUCT_OFFSET, pre##name + (size)*(n); #define STRUCT_END(sname) .set sname##Size, XT_STRUCT_OFFSET; #else /* __clang__ */ @@ -67,7 +69,7 @@ * stack frame is limited to 128 bytes (currently at 64). */ STRUCT_BEGIN -STRUCT_FIELD (long,4,KEXC_,pc) /* "parm" */ +STRUCT_FIELD (long,4,KEXC_,pc) /* "param" */ STRUCT_FIELD (long,4,KEXC_,ps) STRUCT_AFIELD(long,4,KEXC_,areg, 4) /* a12 .. a15 */ STRUCT_FIELD (long,4,KEXC_,sar) /* "save" */