From 7ec19d4268ea9f6bc954e287b6b2dfda0dd37d76 Mon Sep 17 00:00:00 2001 From: Erhan Kurubas Date: Tue, 27 Feb 2024 15:42:09 +0100 Subject: [PATCH] refactor(espcoredump): simplify uart/flash write flow --- components/esp_system/panic.c | 10 +-- components/espcoredump/Kconfig | 4 +- .../espcoredump/include/esp_core_dump.h | 82 +++++++++---------- .../include_core_dump/esp_core_dump_common.h | 28 ++++++- .../include_core_dump/esp_core_dump_types.h | 11 --- components/espcoredump/src/core_dump_binary.c | 11 ++- components/espcoredump/src/core_dump_common.c | 30 +++---- components/espcoredump/src/core_dump_elf.c | 11 ++- components/espcoredump/src/core_dump_flash.c | 69 ++++++++++------ components/espcoredump/src/core_dump_uart.c | 20 ++++- 10 files changed, 161 insertions(+), 115 deletions(-) diff --git a/components/esp_system/panic.c b/components/esp_system/panic.c index ca492892e3..45351535cd 100644 --- a/components/esp_system/panic.c +++ b/components/esp_system/panic.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -378,14 +378,8 @@ void esp_panic_handler(panic_info_t *info) } else { disable_all_wdts(); s_dumping_core = true; -#if CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH - esp_core_dump_to_flash(info); -#endif -#if CONFIG_ESP_COREDUMP_ENABLE_TO_UART && !CONFIG_ESP_SYSTEM_PANIC_SILENT_REBOOT - esp_core_dump_to_uart(info); -#endif + esp_core_dump_write(info); s_dumping_core = false; - esp_panic_handler_reconfigure_wdts(1000); } #endif /* CONFIG_ESP_COREDUMP_ENABLE */ diff --git a/components/espcoredump/Kconfig b/components/espcoredump/Kconfig index f742c12bfc..c3e0f4ab7f 100644 --- a/components/espcoredump/Kconfig +++ b/components/espcoredump/Kconfig @@ -98,9 +98,9 @@ menu "Core dump" 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 + range 1792 4096 if ESP_COREDUMP_USE_STACK_SIZE default 0 if !ESP_COREDUMP_USE_STACK_SIZE - default 1280 if ESP_COREDUMP_USE_STACK_SIZE + default 1792 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/components/espcoredump/include/esp_core_dump.h b/components/espcoredump/include/esp_core_dump.h index 04b69f47c9..21b04c8159 100644 --- a/components/espcoredump/include/esp_core_dump.h +++ b/components/espcoredump/include/esp_core_dump.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -47,51 +47,49 @@ typedef struct { */ void esp_core_dump_init(void); -/** - * @brief Saves core dump to flash. - * - * The structure of data stored in flash is as follows: - * - * | TOTAL_LEN | VERSION | TASKS_NUM | TCB_SIZE | - * | TCB_ADDR_1 | STACK_TOP_1 | STACK_END_1 | TCB_1 | STACK_1 | - * . . . . - * . . . . - * | TCB_ADDR_N | STACK_TOP_N | STACK_END_N | TCB_N | STACK_N | - * | CHECKSUM | - * - * Core dump in flash consists of header and data for every task in the system at the moment of crash. - * For flash data integrity, a checksum is used at the end of core the dump data. - * The structure of core dump data is described below in details. - * 1) Core dump starts with header: - * 1.1) TOTAL_LEN is total length of core dump data in flash including the checksum. Size is 4 bytes. - * 1.2) VERSION field keeps 4 byte version of core dump. - * 1.2) TASKS_NUM is the number of tasks for which data are stored. Size is 4 bytes. - * 1.3) TCB_SIZE is the size of task's TCB structure. Size is 4 bytes. - * 2) Core dump header is followed by the data for every task in the system. - * Task data are started with task header: - * 2.1) TCB_ADDR is the address of TCB in memory. Size is 4 bytes. - * 2.2) STACK_TOP is the top of task's stack (address of the topmost stack item). Size is 4 bytes. - * 2.2) STACK_END is the end of task's stack (address from which task's stack starts). Size is 4 bytes. - * 3) Task header is followed by TCB data. Size is TCB_SIZE bytes. - * 4) Task's stack is placed after TCB data. Size is (STACK_END - STACK_TOP) bytes. - * 5) The checksum is placed at the end of the data. - */ -void esp_core_dump_to_flash(panic_info_t *info); - -/** - * @brief Print base64-encoded core dump to UART. - * - * The structure of core dump data is the same as for data stored in flash (@see esp_core_dump_to_flash) with some notes: - * 1) The checksum is not present in core dump printed to UART. - * 2) Since checksum is omitted TOTAL_LEN does not include its size. - * 3) Printed base64 data are surrounded with special messages to help user recognize the start and end of actual data. - */ -void esp_core_dump_to_uart(panic_info_t *info); - /**************************************************************************************/ /*********************************** USER MODE API ************************************/ /**************************************************************************************/ +/** + * Core dump file consists of header and data (in binary or ELF format) for every task in the system at the moment of crash. + * For the data integrity, a checksum is used at the end of core the dump data. + * The structure of core dump file is described below in details. + * 1) Core dump starts with header: + * 1.1) TOTAL_LEN is total length of core dump data in flash including the checksum. Size is 4 bytes. + * 1.2) VERSION field keeps 4 byte version of core dump. + * 1.2) TASKS_NUM is the number of tasks for which data are stored. Size is 4 bytes. Unused in ELF format + * 1.3) TCB_SIZE is the size of task's TCB structure. Size is 4 bytes. Unused in ELF format + * 1.4) MEM_SEG_NUM is the number of memory segment. Size is 4 bytes. Unused in ELF format + * 1.5) CHIP_REV is the revision of the chip. Size is 4 bytes. + * 2) Core dump header is followed by the data for every task in the system. Data part is differs for the binary + * and elf formats. + * 2.1) The core dump file uses a subset of the ELF structures to store the crash information. + * Loadable ELF segments and ELF notes (ELF.PT_NOTE) used with a special name and type (CORE, NT_PRSTATUS type) + * 2.2) In Binary format task data are started with the task header: + * 2.2.1) TCB_ADDR is the address of TCB in memory. Size is 4 bytes. + * 2.2.2) STACK_TOP is the top of task's stack (address of the topmost stack item). Size is 4 bytes. + * 2.2.3) STACK_END is the end of task's stack (address from which task's stack starts). Size is 4 bytes. + * 2.2.4) Task header is followed by TCB data. Size is TCB_SIZE bytes. + * 2.2.5) Task's stack is placed after TCB data. Size is (STACK_END - STACK_TOP) bytes. + * 2.3) The checksum is placed at the end of the data. + * 3) The structure of the uart data is the same as the data stored in flash + * 3.1) Uart data is printed in base64 format surrounded with special messages to help user recognize the start and + * end of actual data. + * + * For more information about the implementation please check api-guides/core_dump_internals.rst + * + */ + +/** + * @brief Print/store coredump data to the selected destination uart or flash. + * + * @param info Pointer to the panic information. It contains the execution frame. + * + * @return ESP_OK on success, otherwise \see esp_err_t + */ +void esp_core_dump_write(panic_info_t *info); + /** * @brief Check integrity of coredump data in flash. * This function reads the coredump data while calculating their checksum. If it diff --git a/components/espcoredump/include_core_dump/esp_core_dump_common.h b/components/espcoredump/include_core_dump/esp_core_dump_common.h index d5b233166a..c122750845 100644 --- a/components/espcoredump/include_core_dump/esp_core_dump_common.h +++ b/components/espcoredump/include_core_dump/esp_core_dump_common.h @@ -112,25 +112,45 @@ uint32_t esp_core_dump_get_user_ram_size(void); /** - * @brief Function called to prepare flash/uart for the data storage + * @brief Prints write start info string according to destination. + */ +void esp_core_dump_print_write_start(void); + +/** + * @brief Prints write end info string according to destination. + */ +void esp_core_dump_print_write_end(void); + +/** + * @brief Initializes the flash/UART hardware for data storage. + */ +esp_err_t esp_core_dump_write_init(void); + +/** + * @brief Prepares the flash/UART for data storage */ esp_err_t esp_core_dump_write_prepare(core_dump_write_data_t *wr_data, uint32_t *data_len); /** - * @brief Function called at the beginning of data writing + * @brief Initiates the beginning of data writing. */ esp_err_t esp_core_dump_write_start(core_dump_write_data_t *wr_data); /** - * @brief Function called to write data chunk + * @brief Writes a data chunk to the flash/UART */ esp_err_t esp_core_dump_write_data(core_dump_write_data_t *wr_data, void *data, uint32_t data_len); /** - * @brief Function called once all data have been written + * @brief Finalizes the data writing process */ esp_err_t esp_core_dump_write_end(core_dump_write_data_t *wr_data); +/** + * @brief Stores the core dump in either binary or ELF format. + */ +esp_err_t esp_core_dump_store(void); + /** * @brief Get TCB length, in bytes. * diff --git a/components/espcoredump/include_core_dump/esp_core_dump_types.h b/components/espcoredump/include_core_dump/esp_core_dump_types.h index c43ddaae7b..2884fca5ab 100644 --- a/components/espcoredump/include_core_dump/esp_core_dump_types.h +++ b/components/espcoredump/include_core_dump/esp_core_dump_types.h @@ -174,17 +174,6 @@ typedef struct _core_dump_mem_seg_header_t uint32_t size; /*!< Memory region size */ } core_dump_mem_seg_header_t; -/** - * @brief Core dump flash init function - */ -void esp_core_dump_flash_init(void); - - -/** - * @brief Common core dump write function - */ -void esp_core_dump_write(panic_info_t *info); - #ifdef __cplusplus } #endif diff --git a/components/espcoredump/src/core_dump_binary.c b/components/espcoredump/src/core_dump_binary.c index 4621d29af6..205c670e70 100644 --- a/components/espcoredump/src/core_dump_binary.c +++ b/components/espcoredump/src/core_dump_binary.c @@ -14,6 +14,8 @@ const static char TAG[] __attribute__((unused)) = "esp_core_dump_binary"; +esp_err_t esp_core_dump_store(void) __attribute__((alias("esp_core_dump_write_binary"))); + static esp_err_t esp_core_dump_save_task(core_dump_write_data_t *write_data, core_dump_task_header_t *task) { esp_err_t err = ESP_FAIL; @@ -78,9 +80,8 @@ static esp_err_t esp_core_dump_save_mem_segment(core_dump_write_data_t* write_da return ESP_OK; } -esp_err_t esp_core_dump_write_binary(void) +static esp_err_t esp_core_dump_write_binary(void) { - esp_err_t err = ESP_OK; uint32_t tcb_sz = esp_core_dump_get_tcb_len(); uint32_t data_len = 0; uint32_t bad_tasks_num = 0; @@ -91,6 +92,12 @@ esp_err_t esp_core_dump_write_binary(void) void *task = NULL; void *cur_task = NULL; + esp_err_t err = esp_core_dump_write_init(); + if (err != ESP_OK) { + ESP_COREDUMP_LOGE("Binary write init failed!"); + return ESP_FAIL; + } + // Verifies all tasks in the snapshot esp_core_dump_reset_tasks_snapshots_iter(); while ((task = esp_core_dump_get_next_task(task))) { diff --git a/components/espcoredump/src/core_dump_common.c b/components/espcoredump/src/core_dump_common.c index 5405449ee2..f4e44d48c2 100644 --- a/components/espcoredump/src/core_dump_common.c +++ b/components/espcoredump/src/core_dump_common.c @@ -143,28 +143,19 @@ FORCE_INLINE_ATTR void esp_core_dump_report_stack_usage(void) static void* s_exc_frame = NULL; -inline void esp_core_dump_write(panic_info_t *info) +inline static void esp_core_dump_write_internal(panic_info_t *info) { -#ifndef CONFIG_ESP_COREDUMP_ENABLE_TO_NONE - esp_err_t err = ESP_ERR_NOT_SUPPORTED; - s_exc_frame = (void*) info->frame; - 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); -#if CONFIG_ESP_COREDUMP_DATA_FORMAT_BIN - esp_err_t esp_core_dump_write_binary(void); - err = esp_core_dump_write_binary(); -#elif CONFIG_ESP_COREDUMP_DATA_FORMAT_ELF - esp_err_t esp_core_dump_write_elf(void); - err = esp_core_dump_write_elf(); -#endif + esp_err_t err = esp_core_dump_store(); if (err != ESP_OK) { - ESP_COREDUMP_LOGE("Core dump write binary failed with error=%d", err); + ESP_COREDUMP_LOGE("Core dump write failed with error=%d", err); } esp_core_dump_report_stack_usage(); -#endif } void __attribute__((weak)) esp_core_dump_init(void) @@ -324,4 +315,15 @@ inline core_dump_task_handle_t esp_core_dump_get_current_task_handle() return (core_dump_task_handle_t) xTaskGetCurrentTaskHandleForCPU(xPortGetCoreID()); } +void esp_core_dump_write(panic_info_t *info) +{ +#if CONFIG_ESP_COREDUMP_ENABLE_TO_UART && CONFIG_ESP_SYSTEM_PANIC_SILENT_REBOOT + return; +#endif + + esp_core_dump_print_write_start(); + esp_core_dump_write_internal(info); + esp_core_dump_print_write_end(); +} + #endif diff --git a/components/espcoredump/src/core_dump_elf.c b/components/espcoredump/src/core_dump_elf.c index 39cfa508d0..8a85c4d3f5 100644 --- a/components/espcoredump/src/core_dump_elf.c +++ b/components/espcoredump/src/core_dump_elf.c @@ -93,6 +93,8 @@ static inline uint32_t align(uint32_t width, uint32_t in) return (in + (width - 1)) & -width; } +esp_err_t esp_core_dump_store(void) __attribute__((alias("esp_core_dump_write_elf"))); + // Builds elf header and check all data offsets static int elf_write_file_header(core_dump_elf_t *self, uint32_t seg_count) { @@ -571,14 +573,19 @@ static int esp_core_dump_do_write_elf_pass(core_dump_elf_t *self) return tot_len; } -esp_err_t esp_core_dump_write_elf(void) +static esp_err_t esp_core_dump_write_elf(void) { core_dump_elf_t self = { 0 }; core_dump_header_t dump_hdr = { 0 }; - esp_err_t err = ESP_OK; int tot_len = sizeof(dump_hdr); int write_len = sizeof(dump_hdr); + esp_err_t err = esp_core_dump_write_init(); + if (err != ESP_OK) { + ESP_COREDUMP_LOGE("Elf write init failed!"); + return ESP_FAIL; + } + // On first pass (do not write actual data), but calculate data length needed to allocate memory self.elf_stage = ELF_STAGE_CALC_SPACE; ESP_COREDUMP_LOG_PROCESS("================= Calc data size ==============="); diff --git a/components/espcoredump/src/core_dump_flash.c b/components/espcoredump/src/core_dump_flash.c index 2e7fcef46b..b6194f83c5 100644 --- a/components/espcoredump/src/core_dump_flash.c +++ b/components/espcoredump/src/core_dump_flash.c @@ -39,6 +39,9 @@ typedef struct _core_dump_flash_config_t { /* Core dump flash data. */ static core_dump_flash_config_t s_core_flash_config; +void esp_core_dump_print_write_start(void) __attribute__((alias("esp_core_dump_flash_print_write_start"))); +void esp_core_dump_print_write_end(void) __attribute__((alias("esp_core_dump_flash_print_write_end"))); +esp_err_t esp_core_dump_write_init(void) __attribute__((alias("esp_core_dump_flash_hw_init"))); esp_err_t esp_core_dump_write_prepare(core_dump_write_data_t *wr_data, uint32_t *data_len) __attribute__((alias("esp_core_dump_flash_write_prepare"))); esp_err_t esp_core_dump_write_start(core_dump_write_data_t *wr_data) __attribute__((alias("esp_core_dump_flash_write_start"))); esp_err_t esp_core_dump_write_end(core_dump_write_data_t *wr_data) __attribute__((alias("esp_core_dump_flash_write_end"))); @@ -51,6 +54,16 @@ esp_err_t esp_core_dump_write_data(core_dump_write_data_t *wr_data, void *data, esp_err_t esp_core_dump_image_check(void); static esp_err_t esp_core_dump_partition_and_size_get(const esp_partition_t **partition, uint32_t* size); +static void esp_core_dump_flash_print_write_start(void) +{ + ESP_COREDUMP_LOGI("Save core dump to flash..."); +} + +static void esp_core_dump_flash_print_write_end(void) +{ + ESP_COREDUMP_LOGI("Core dump has been saved to flash."); +} + static esp_err_t esp_core_dump_flash_custom_write(uint32_t address, const void *buffer, uint32_t length) { esp_err_t err = ESP_OK; @@ -69,7 +82,35 @@ static inline core_dump_crc_t esp_core_dump_calc_flash_config_crc(void) return esp_rom_crc32_le(0, (uint8_t const *)&s_core_flash_config.partition, sizeof(s_core_flash_config.partition)); } -void esp_core_dump_flash_init(void) +static esp_err_t esp_core_dump_flash_hw_init(void) +{ + /* Check core dump partition configuration. */ + core_dump_crc_t crc = esp_core_dump_calc_flash_config_crc(); + if (s_core_flash_config.partition_config_crc != crc) { + ESP_COREDUMP_LOGE("Core dump flash config is corrupted! CRC=0x%x instead of 0x%x", crc, s_core_flash_config.partition_config_crc); + return ESP_FAIL; + } + + /* Make sure that the partition can at least hold the data length. */ + if (s_core_flash_config.partition.start == 0 || s_core_flash_config.partition.size < sizeof(uint32_t)) { + ESP_COREDUMP_LOGE("Invalid flash partition config!"); + return ESP_FAIL; + } + +#if CONFIG_ESP_COREDUMP_FLASH_NO_OVERWRITE + if (!s_core_flash_config.partition.empty) { + ESP_COREDUMP_LOGW("Core dump already exists in flash, will not overwrite it with a new core dump"); + return ESP_FAIL; + } +#endif + + /* Initialize non-OS flash access critical section. */ + spi_flash_guard_set(&g_flash_guard_no_os_ops); + esp_flash_app_disable_protect(true); + return ESP_OK; +} + +static void esp_core_dump_partition_init(void) { const esp_partition_t *core_part = NULL; @@ -298,33 +339,9 @@ static esp_err_t esp_core_dump_flash_write_end(core_dump_write_data_t *wr_data) return err; } -void esp_core_dump_to_flash(panic_info_t *info) -{ - /* Check core dump partition configuration. */ - core_dump_crc_t crc = esp_core_dump_calc_flash_config_crc(); - if (s_core_flash_config.partition_config_crc != crc) { - ESP_COREDUMP_LOGE("Core dump flash config is corrupted! CRC=0x%x instead of 0x%x", crc, s_core_flash_config.partition_config_crc); - return; - } - - /* Make sure that the partition can at least hold the data length. */ - if (s_core_flash_config.partition.start == 0 || s_core_flash_config.partition.size < sizeof(uint32_t)) { - ESP_COREDUMP_LOGE("Invalid flash partition config!"); - return; - } - - /* Initialize non-OS flash access critical section. */ - spi_flash_guard_set(&g_flash_guard_no_os_ops); - esp_flash_app_disable_protect(true); - - ESP_COREDUMP_LOGI("Save core dump to flash..."); - esp_core_dump_write(info); - ESP_COREDUMP_LOGI("Core dump has been saved to flash."); -} - void esp_core_dump_init(void) { - esp_core_dump_flash_init(); + esp_core_dump_partition_init(); #if CONFIG_ESP_COREDUMP_CHECK_BOOT const esp_partition_t *partition = 0; diff --git a/components/espcoredump/src/core_dump_uart.c b/components/espcoredump/src/core_dump_uart.c index c2e1f17887..3a3e5678fa 100644 --- a/components/espcoredump/src/core_dump_uart.c +++ b/components/espcoredump/src/core_dump_uart.c @@ -17,6 +17,9 @@ const static char TAG[] __attribute__((unused)) = "esp_core_dump_uart"; #if CONFIG_ESP_COREDUMP_ENABLE_TO_UART +void esp_core_dump_print_write_start(void) __attribute__((alias("esp_core_dump_uart_print_write_start"))); +void esp_core_dump_print_write_end(void) __attribute__((alias("esp_core_dump_uart_print_write_end"))); +esp_err_t esp_core_dump_write_init(void) __attribute__((alias("esp_core_dump_uart_hw_init"))); esp_err_t esp_core_dump_write_prepare(core_dump_write_data_t *wr_data, uint32_t *data_len) __attribute__((alias("esp_core_dump_uart_write_prepare"))); esp_err_t esp_core_dump_write_start(core_dump_write_data_t *wr_data) __attribute__((alias("esp_core_dump_uart_write_start"))); esp_err_t esp_core_dump_write_end(core_dump_write_data_t *wr_data) __attribute__((alias("esp_core_dump_uart_write_end"))); @@ -51,6 +54,16 @@ static void esp_core_dump_b64_encode(const uint8_t *src, uint32_t src_len, uint8 dst[j++] = '\0'; } +static void esp_core_dump_uart_print_write_start(void) +{ + ESP_COREDUMP_LOGI("Print core dump to uart..."); +} + +static void esp_core_dump_uart_print_write_end(void) +{ + ESP_COREDUMP_LOGI("Core dump has been written to uart."); +} + static esp_err_t esp_core_dump_uart_write_start(core_dump_write_data_t *wr_data) { esp_err_t err = ESP_OK; @@ -124,7 +137,7 @@ static int esp_core_dump_uart_get_char(void) { return i; } -void esp_core_dump_to_uart(panic_info_t *info) +static esp_err_t esp_core_dump_uart_hw_init(void) { uint32_t tm_end = 0; uint32_t tm_cur = 0; @@ -148,9 +161,8 @@ void esp_core_dump_to_uart(panic_info_t *info) } ch = esp_core_dump_uart_get_char(); } - ESP_COREDUMP_LOGI("Print core dump to uart..."); - esp_core_dump_write(info); - ESP_COREDUMP_LOGI("Core dump has been written to uart."); + + return ESP_OK; } void esp_core_dump_init(void)