From 42b216954260cb3c48eafd5d87612c8cf6c2a5b2 Mon Sep 17 00:00:00 2001 From: Omar Chebib Date: Fri, 11 Nov 2022 14:47:58 +0800 Subject: [PATCH 1/2] coredump: add support for stacks in external RAM Tasks having their stacks in SPIRAM can now be part of the coredump written to flash --- components/espcoredump/Kconfig | 11 ++++++++++- components/espcoredump/src/core_dump_common.c | 10 ++++++++-- .../espcoredump/src/port/riscv/core_dump_port.c | 2 +- .../espcoredump/src/port/xtensa/core_dump_port.c | 2 +- components/spi_flash/spi_flash_os_func_noos.c | 2 -- 5 files changed, 20 insertions(+), 7 deletions(-) diff --git a/components/espcoredump/Kconfig b/components/espcoredump/Kconfig index 07f419e8ebd8..199d4e0bc89e 100644 --- a/components/espcoredump/Kconfig +++ b/components/espcoredump/Kconfig @@ -14,7 +14,6 @@ menu "Core dump" config ESP_COREDUMP_ENABLE_TO_FLASH bool "Flash" - depends on !SPIRAM_ALLOW_STACK_EXTERNAL_MEMORY select FREERTOS_ENABLE_TASK_SNAPSHOT select ESP_COREDUMP_ENABLE config ESP_COREDUMP_ENABLE_TO_UART @@ -79,9 +78,19 @@ menu "Core dump" Config delay (in ms) before printing core dump to UART. Delay can be interrupted by pressing Enter key. + + config ESP_COREDUMP_USE_STACK_SIZE + bool + default y if ESP_COREDUMP_ENABLE_TO_FLASH && SPIRAM_ALLOW_STACK_EXTERNAL_MEMORY + default n + help + Force the use of a custom DRAM stack for coredump when Task stacks can be in PSRAM. + config ESP_COREDUMP_STACK_SIZE int "Reserved stack size" depends on ESP_COREDUMP_ENABLE + range 0 4096 if !ESP_COREDUMP_USE_STACK_SIZE + range 1280 4096 if ESP_COREDUMP_USE_STACK_SIZE default 0 help Size of the memory to be reserved for core dump stack. If 0 core dump process will run on diff --git a/components/espcoredump/src/core_dump_common.c b/components/espcoredump/src/core_dump_common.c index 241065fa84ae..e387c3fbc113 100644 --- a/components/espcoredump/src/core_dump_common.c +++ b/components/espcoredump/src/core_dump_common.c @@ -115,7 +115,13 @@ FORCE_INLINE_ATTR void esp_core_dump_report_stack_usage(void) esp_core_dump_restore_sp(&s_stack_context); } -#else +#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 + FORCE_INLINE_ATTR void esp_core_dump_setup_stack(void) { /* If we are in ISR set watchpoint to the end of ISR stack */ @@ -133,7 +139,7 @@ FORCE_INLINE_ATTR void esp_core_dump_setup_stack(void) FORCE_INLINE_ATTR void esp_core_dump_report_stack_usage(void) { } -#endif +#endif // CONFIG_ESP_COREDUMP_STACK_SIZE > 0 static void* s_exc_frame = NULL; diff --git a/components/espcoredump/src/port/riscv/core_dump_port.c b/components/espcoredump/src/port/riscv/core_dump_port.c index 19d901770503..aeb36aafb368 100644 --- a/components/espcoredump/src/port/riscv/core_dump_port.c +++ b/components/espcoredump/src/port/riscv/core_dump_port.c @@ -306,10 +306,10 @@ bool esp_core_dump_check_task(core_dump_task_header_t *task) */ bool esp_core_dump_mem_seg_is_sane(uint32_t addr, uint32_t sz) { - //TODO: external SRAM not supported yet return (esp_ptr_in_dram((void *)addr) && esp_ptr_in_dram((void *)(addr+sz-1))) || (esp_ptr_in_rtc_slow((void *)addr) && esp_ptr_in_rtc_slow((void *)(addr+sz-1))) || (esp_ptr_in_rtc_dram_fast((void *)addr) && esp_ptr_in_rtc_dram_fast((void *)(addr+sz-1))) + || (esp_ptr_external_ram((void *)addr) && esp_ptr_external_ram((void *)(addr+sz-1))) || (esp_ptr_in_iram((void *)addr) && esp_ptr_in_iram((void *)(addr+sz-1))); } diff --git a/components/espcoredump/src/port/xtensa/core_dump_port.c b/components/espcoredump/src/port/xtensa/core_dump_port.c index e0f4cddc3276..fb4704b3e327 100644 --- a/components/espcoredump/src/port/xtensa/core_dump_port.c +++ b/components/espcoredump/src/port/xtensa/core_dump_port.c @@ -336,10 +336,10 @@ bool esp_core_dump_check_stack(core_dump_task_header_t *task) */ bool esp_core_dump_mem_seg_is_sane(uint32_t addr, uint32_t sz) { - //TODO: external SRAM not supported yet return (esp_ptr_in_dram((void *)addr) && esp_ptr_in_dram((void *)(addr+sz-1))) || (esp_ptr_in_rtc_slow((void *)addr) && esp_ptr_in_rtc_slow((void *)(addr+sz-1))) || (esp_ptr_in_rtc_dram_fast((void *)addr) && esp_ptr_in_rtc_dram_fast((void *)(addr+sz-1))) + || (esp_ptr_external_ram((void *)addr) && esp_ptr_external_ram((void *)(addr+sz-1))) || (esp_ptr_in_iram((void *)addr) && esp_ptr_in_iram((void *)(addr+sz-1))); } diff --git a/components/spi_flash/spi_flash_os_func_noos.c b/components/spi_flash/spi_flash_os_func_noos.c index bc3ca523b488..1c594dbd19d5 100644 --- a/components/spi_flash/spi_flash_os_func_noos.c +++ b/components/spi_flash/spi_flash_os_func_noos.c @@ -67,8 +67,6 @@ static IRAM_ATTR esp_err_t start(void *arg) static IRAM_ATTR esp_err_t end(void *arg) { #if CONFIG_IDF_TARGET_ESP32 - Cache_Flush(0); - Cache_Flush(1); Cache_Read_Enable(0); Cache_Read_Enable(1); #elif CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3 From 835263e50a65f572595b53dced7342b32c47f17a Mon Sep 17 00:00:00 2001 From: Omar Chebib Date: Mon, 19 Dec 2022 17:38:17 +0100 Subject: [PATCH 2/2] Coredump: add a test to check that coredump supports stacks in SPIRAM --- components/espcoredump/Kconfig | 3 +- .../system/panic/main/CMakeLists.txt | 2 +- .../system/panic/main/test_panic_main.c | 34 +++++++++++++++++++ tools/test_apps/system/panic/pytest_panic.py | 25 ++++++++++++++ .../panic/sdkconfig.ci.coredump_extram_stack | 11 ++++++ 5 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 tools/test_apps/system/panic/sdkconfig.ci.coredump_extram_stack diff --git a/components/espcoredump/Kconfig b/components/espcoredump/Kconfig index 199d4e0bc89e..623fc059ac1a 100644 --- a/components/espcoredump/Kconfig +++ b/components/espcoredump/Kconfig @@ -91,7 +91,8 @@ menu "Core dump" depends on ESP_COREDUMP_ENABLE range 0 4096 if !ESP_COREDUMP_USE_STACK_SIZE range 1280 4096 if ESP_COREDUMP_USE_STACK_SIZE - default 0 + default 0 if !ESP_COREDUMP_USE_STACK_SIZE + default 1280 if ESP_COREDUMP_USE_STACK_SIZE help Size of the memory to be reserved for core dump stack. If 0 core dump process will run on the stack of crashed task/ISR, otherwise special stack will be allocated. diff --git a/tools/test_apps/system/panic/main/CMakeLists.txt b/tools/test_apps/system/panic/main/CMakeLists.txt index 83427066bf15..92dc34634124 100644 --- a/tools/test_apps/system/panic/main/CMakeLists.txt +++ b/tools/test_apps/system/panic/main/CMakeLists.txt @@ -1,3 +1,3 @@ idf_component_register(SRCS "test_panic_main.c" INCLUDE_DIRS "." - REQUIRES spi_flash esp_system esp_partition) + REQUIRES spi_flash esp_psram esp_system esp_partition) diff --git a/tools/test_apps/system/panic/main/test_panic_main.c b/tools/test_apps/system/panic/main/test_panic_main.c index 2b4c4512ef49..5d9211e52f8c 100644 --- a/tools/test_apps/system/panic/main/test_panic_main.c +++ b/tools/test_apps/system/panic/main/test_panic_main.c @@ -17,6 +17,9 @@ static void test_abort(void); static void test_abort_cache_disabled(void); static void test_int_wdt(void); static void test_task_wdt_cpu0(void); +#if CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH && CONFIG_SPIRAM_ALLOW_STACK_EXTERNAL_MEMORY +static void test_panic_extram_stack(void); +#endif #if !CONFIG_FREERTOS_UNICORE static void test_task_wdt_cpu1(void); static void test_task_wdt_both_cpus(void); @@ -55,6 +58,9 @@ void app_main(void) HANDLE_TEST(test_abort_cache_disabled); HANDLE_TEST(test_int_wdt); HANDLE_TEST(test_task_wdt_cpu0); +#if CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH && CONFIG_SPIRAM_ALLOW_STACK_EXTERNAL_MEMORY + HANDLE_TEST(test_panic_extram_stack); +#endif #if !CONFIG_FREERTOS_UNICORE HANDLE_TEST(test_task_wdt_cpu1); HANDLE_TEST(test_task_wdt_both_cpus); @@ -102,6 +108,34 @@ static void test_task_wdt_cpu0(void) } } +#if CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH && CONFIG_SPIRAM_ALLOW_STACK_EXTERNAL_MEMORY + +static void stack_in_extram(void* arg) { + (void) arg; + /* Abort instead of using a load/store prohibited to prevent a sanitize error */ + abort(); +} + +static void test_panic_extram_stack(void) { + /* Start by initializing a Task which has a stack in external RAM */ + StaticTask_t handle; + const uint32_t stack_size = 8192; + void* stack = heap_caps_malloc(stack_size, MALLOC_CAP_SPIRAM); + + /* Make sure the stack is in external RAM */ + if (!esp_ptr_external_ram(stack)) { + die("Allocated stack is not in external RAM!\n"); + } + + xTaskCreateStatic(stack_in_extram, "Task_stack_extram", stack_size, NULL, 4, (StackType_t*) stack, &handle); + + vTaskDelay(1000); +} + + +#endif // ESP_COREDUMP_ENABLE_TO_FLASH && SPIRAM_ALLOW_STACK_EXTERNAL_MEMORY + + #if !CONFIG_FREERTOS_UNICORE static void infinite_loop(void* arg) { (void) arg; diff --git a/tools/test_apps/system/panic/pytest_panic.py b/tools/test_apps/system/panic/pytest_panic.py index a923f8cf4e85..ff7a64e76ccb 100644 --- a/tools/test_apps/system/panic/pytest_panic.py +++ b/tools/test_apps/system/panic/pytest_panic.py @@ -28,6 +28,14 @@ pytest.param('panic', marks=[pytest.mark.esp32]), ] +# IDF-5692: Uncomment the marks related to ESP32-S3 and quad_psram once ESP32-S3 runners are available +CONFIG_EXTRAM_STACK = [ + pytest.param('coredump_extram_stack', + marks=[pytest.mark.esp32, pytest.mark.esp32s2, pytest.mark.psram, + # pytest.mark.esp32s3, pytest.mark.quad_psram + ]) +] + def get_default_backtrace(config: str) -> List[str]: return [config, 'app_main', 'main_task', 'vPortTaskWrapper'] @@ -142,6 +150,23 @@ def test_task_wdt_both_cpus(dut: PanicTestDut, config: str, test_func_name: str) common_test(dut, config) +@pytest.mark.parametrize('config', CONFIG_EXTRAM_STACK, indirect=True) +def test_panic_extram_stack(dut: PanicTestDut, config: str, test_func_name: str) -> None: + dut.expect_test_func_name(test_func_name) + dut.expect_none('Allocated stack is not in external RAM') + dut.expect_none('Guru Meditation') + dut.expect_backtrace() + dut.expect_elf_sha256() + # Check that coredump is getting written to flash + dut.expect_exact('Save core dump to flash...') + # And that the stack is replaced and restored + dut.expect_exact('Backing up stack @') + dut.expect_exact('Restoring stack') + # The caller must be accessible after restoring the stack + dut.expect_exact('Core dump has been saved to flash.') + common_test(dut, config) + + @pytest.mark.parametrize('config', CONFIGS, indirect=True) @pytest.mark.generic def test_int_wdt( diff --git a/tools/test_apps/system/panic/sdkconfig.ci.coredump_extram_stack b/tools/test_apps/system/panic/sdkconfig.ci.coredump_extram_stack new file mode 100644 index 000000000000..06ac9fcc8eaa --- /dev/null +++ b/tools/test_apps/system/panic/sdkconfig.ci.coredump_extram_stack @@ -0,0 +1,11 @@ +CONFIG_IDF_TARGET="esp32" +CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH=y +CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF=y +CONFIG_ESP_COREDUMP_CHECKSUM_SHA256=y +# CONFIG_ESP_SYSTEM_PANIC_GDBSTUB is not set +CONFIG_ESP_SYSTEM_PANIC_PRINT_HALT=y +# We need to have the coredump info log +CONFIG_LOG_DEFAULT_LEVEL_INFO=y +CONFIG_SPIRAM=y +CONFIG_SPIRAM_ALLOW_STACK_EXTERNAL_MEMORY=y +CONFIG_ESP_COREDUMP_USE_STACK_SIZE=y