From b605404b062fdead349d5441da730914a6fbf8d5 Mon Sep 17 00:00:00 2001 From: KonstantinKondrashov Date: Thu, 4 May 2023 17:14:46 +0800 Subject: [PATCH] esp_app_format: IRAM space optimization --- components/app_update/esp_ota_app_desc.c | 2 +- components/esp_app_format/Kconfig.projbuild | 12 ++-- components/esp_app_format/esp_app_desc.c | 67 +++++++++---------- .../esp_app_format/include/esp_app_desc.h | 18 ++++- .../test_apps/main/test_app_desc.c | 36 +++++++--- components/esp_system/CMakeLists.txt | 4 +- components/esp_system/panic.c | 8 +-- components/espcoredump/src/core_dump_elf.c | 6 +- .../system/panic/test_panic_util/panic_dut.py | 2 +- 9 files changed, 95 insertions(+), 60 deletions(-) diff --git a/components/app_update/esp_ota_app_desc.c b/components/app_update/esp_ota_app_desc.c index 70774d50e0..220b07071b 100644 --- a/components/app_update/esp_ota_app_desc.c +++ b/components/app_update/esp_ota_app_desc.c @@ -15,7 +15,7 @@ const esp_app_desc_t *esp_ota_get_app_description(void) return esp_app_get_description(); } -int IRAM_ATTR esp_ota_get_app_elf_sha256(char* dst, size_t size) +int esp_ota_get_app_elf_sha256(char* dst, size_t size) { return esp_app_get_elf_sha256(dst, size); } diff --git a/components/esp_app_format/Kconfig.projbuild b/components/esp_app_format/Kconfig.projbuild index 8471162c6d..635a49784e 100644 --- a/components/esp_app_format/Kconfig.projbuild +++ b/components/esp_app_format/Kconfig.projbuild @@ -39,12 +39,14 @@ menu "Application manager" config APP_RETRIEVE_LEN_ELF_SHA int "The length of APP ELF SHA is stored in RAM(chars)" - default 16 + default 9 range 8 64 help - At startup, the app will read this many hex characters from the embedded APP ELF SHA-256 hash value - and store it in static RAM. This ensures the app ELF SHA-256 value is always available - if it needs to be printed by the panic handler code. - Changing this value will change the size of a static buffer, in bytes. + At startup, the app will read the embedded APP ELF SHA-256 hash value from flash + and convert it into a string and store it in a RAM buffer. + This ensures the panic handler and core dump will be able to print this string + even when cache is disabled. + The size of the buffer is APP_RETRIEVE_LEN_ELF_SHA plus the null terminator. + Changing this value will change the size of this buffer, in bytes. endmenu # "Application manager" diff --git a/components/esp_app_format/esp_app_desc.c b/components/esp_app_format/esp_app_desc.c index 4ce5726448..d13b2c8733 100644 --- a/components/esp_app_format/esp_app_desc.c +++ b/components/esp_app_format/esp_app_desc.c @@ -6,8 +6,8 @@ #include #include +#include #include "esp_app_desc.h" -#include "esp_attr.h" #include "sdkconfig.h" @@ -56,48 +56,43 @@ const esp_app_desc_t *esp_app_get_description(void) return &esp_app_desc; } -/* The following two functions may be called from the panic handler - * or core dump, hence IRAM_ATTR. - */ +char app_elf_sha256_str[CONFIG_APP_RETRIEVE_LEN_ELF_SHA + 1] = { 0 }; -static inline char IRAM_ATTR to_hex_digit(unsigned val) -{ - return (val < 10) ? ('0' + val) : ('a' + val - 10); -} - -__attribute__((constructor)) void esp_init_app_elf_sha256(void) -{ - esp_app_get_elf_sha256(NULL, 0); -} - -/* The esp_app_desc.app_elf_sha256 should be possible to print in panic handler during cache is disabled. +/* The esp_app_desc.app_elf_sha256 should be possible to print in panic handler or core dump during cache is disabled. * But because the cache is disabled the reading esp_app_desc.app_elf_sha256 is not right and * can lead to a complete lock-up of the CPU. - * For this reason we do a reading of esp_app_desc.app_elf_sha256 while start up in esp_init_app_elf_sha256() - * and keep it in the static s_app_elf_sha256 value. + * For this reason we do a reading of esp_app_desc.app_elf_sha256 and convert to string while start up in esp_system_init_app_elf_sha256() + * and keep it in the static app_elf_sha256_str variable. */ -int IRAM_ATTR esp_app_get_elf_sha256(char* dst, size_t size) +__attribute__((constructor)) void esp_app_format_init_elf_sha256(void) { - static char s_app_elf_sha256[CONFIG_APP_RETRIEVE_LEN_ELF_SHA / 2]; - static bool first_call = true; - if (first_call) { - first_call = false; - // At -O2 optimization level, GCC optimizes out the copying of the first byte of the app_elf_sha256, - // because it is zero at compile time, and only modified afterwards by esptool. - // Casting to volatile disables the optimization. - const volatile uint8_t* src = (const volatile uint8_t*)esp_app_desc.app_elf_sha256; - for (size_t i = 0; i < sizeof(s_app_elf_sha256); ++i) { - s_app_elf_sha256[i] = src[i]; + if (*((int *)&app_elf_sha256_str) != 0) { + // app_elf_sha256_str is already set + return; + } + // At -O2 optimization level, GCC optimizes out the copying of the first byte of the app_elf_sha256, + // because it is zero at compile time, and only modified afterwards by esptool. + // Casting to volatile disables the optimization. + const volatile char* src = (const volatile char*)esp_app_desc.app_elf_sha256; + for (size_t i = 0; i < sizeof(app_elf_sha256_str) / 2; ++i) { + char c = src[i]; + for (size_t s = 0; s < 2; ++s) { + char val = (c >> 4) & 0xF; + app_elf_sha256_str[2 * i + s] = (val < 10) ? ('0' + val) : ('a' + val - 10); + c <<= 4; } } - if (dst == NULL || size == 0) { + app_elf_sha256_str[sizeof(app_elf_sha256_str) - 1] = 0; +} + +int esp_app_get_elf_sha256(char* dst, size_t size) +{ + if (dst == NULL || size < 2) { return 0; } - size_t n = MIN((size - 1) / 2, sizeof(s_app_elf_sha256)); - for (size_t i = 0; i < n; ++i) { - dst[2*i] = to_hex_digit(s_app_elf_sha256[i] >> 4); - dst[2*i + 1] = to_hex_digit(s_app_elf_sha256[i] & 0xf); - } - dst[2*n] = 0; - return 2*n + 1; + esp_app_format_init_elf_sha256(); + size_t n = MIN(size, sizeof(app_elf_sha256_str)); + memcpy(dst, app_elf_sha256_str, n); + dst[n - 1] = 0; + return n; } diff --git a/components/esp_app_format/include/esp_app_desc.h b/components/esp_app_format/include/esp_app_desc.h index fd7582be79..f4c7d04b1a 100644 --- a/components/esp_app_format/include/esp_app_desc.h +++ b/components/esp_app_format/include/esp_app_desc.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -11,6 +11,7 @@ #include #include "esp_err.h" #include "esp_assert.h" +#include "esp_attr.h" #ifdef __cplusplus extern "C" @@ -57,6 +58,21 @@ const esp_app_desc_t *esp_app_get_description(void); */ int esp_app_get_elf_sha256(char* dst, size_t size); +/** @cond */ +extern char app_elf_sha256_str[]; +/** @endcond */ + +/** + * @brief Return SHA256 of the ELF file which is already formatted as hexadecimal, null-terminated included. + * Can be used in panic handler or core dump during when cache is disabled. + * The length is defined by CONFIG_APP_RETRIEVE_LEN_ELF_SHA option. + * @return Hexadecimal SHA256 string + */ +FORCE_INLINE_ATTR char *esp_app_get_elf_sha256_str(void) +{ + return app_elf_sha256_str; +} + #ifdef __cplusplus } #endif diff --git a/components/esp_app_format/test_apps/main/test_app_desc.c b/components/esp_app_format/test_apps/main/test_app_desc.c index 99992702f0..2d729afb63 100644 --- a/components/esp_app_format/test_apps/main/test_app_desc.c +++ b/components/esp_app_format/test_apps/main/test_app_desc.c @@ -55,16 +55,32 @@ TEST(esp_app_format, esp_app_get_elf_sha256_test) TEST_ASSERT_EQUAL_HEX(0, dst[8]); TEST_ASSERT_EQUAL_HEX(fill, dst[9]); - memset(dst, fill, sizeof(dst)); - len = 8; - res = esp_app_get_elf_sha256(dst, len); - printf("%d: %s (%d)\n", len, dst, res); - // should output even number of characters plus '\0' - TEST_ASSERT_EQUAL(7, res); - TEST_ASSERT_EQUAL(0, memcmp(dst, ref_sha256, res - 1)); - TEST_ASSERT_EQUAL_HEX(0, dst[6]); - TEST_ASSERT_EQUAL_HEX(fill, dst[7]); - TEST_ASSERT_EQUAL_HEX(fill, dst[8]); + memset(ref_sha256, 0xCC, sizeof(ref_sha256)); + strncpy(ref_sha256, esp_app_get_elf_sha256_str(), sizeof(ref_sha256)); + len = strlen(ref_sha256); + TEST_ASSERT_EQUAL(CONFIG_APP_RETRIEVE_LEN_ELF_SHA, len); + printf("\n_Ref: %s (len=%d with null)\n", ref_sha256, len); + + TEST_ASSERT_EQUAL(0, esp_app_get_elf_sha256(dst, 0)); + TEST_ASSERT_EQUAL(0, esp_app_get_elf_sha256(dst, 1)); + TEST_ASSERT_EQUAL(0, esp_app_get_elf_sha256(NULL, 99)); + + for (size_t req_len = 2; req_len <= CONFIG_APP_RETRIEVE_LEN_ELF_SHA + 1; req_len++) { + memset(dst, 0xCC, sizeof(dst)); + TEST_ASSERT_EQUAL(req_len, esp_app_get_elf_sha256(dst, req_len)); + len = strlen(dst) + 1; // + 1 for the null terminator + printf("_%02d_: %-15s (len=%d with null)\n", req_len, dst, len); + TEST_ASSERT_EQUAL(req_len, len); + TEST_ASSERT_EQUAL_STRING_LEN(ref_sha256, dst, len - 1); // -1 without null terminator + } + + memset(dst, 0xCC, sizeof(dst)); + size_t max_len = CONFIG_APP_RETRIEVE_LEN_ELF_SHA + 1; // + 1 for the null terminator + TEST_ASSERT_EQUAL(max_len, esp_app_get_elf_sha256(dst, 99)); + len = strlen(dst) + 1; // + 1 for the null terminator + printf("_99_: %-15s (len=%d with null)\n", dst, len); + TEST_ASSERT_EQUAL(max_len, len); + TEST_ASSERT_EQUAL_STRING_LEN(ref_sha256, dst, len - 1); // -1 without null terminator } TEST_GROUP_RUNNER(esp_app_format) diff --git a/components/esp_system/CMakeLists.txt b/components/esp_system/CMakeLists.txt index ab1b4db1d9..fb8f798ee6 100644 --- a/components/esp_system/CMakeLists.txt +++ b/components/esp_system/CMakeLists.txt @@ -110,7 +110,9 @@ endif() # need to introduce panic "event" concept to remove this dependency (IDF-2194) idf_component_optional_requires(PRIVATE esp_gdbstub) -idf_component_optional_requires(PRIVATE esp_app_format) +if(NOT CONFIG_ESP_SYSTEM_PANIC_SILENT_REBOOT) + idf_component_optional_requires(PRIVATE esp_app_format) +endif() if(CONFIG_PM_ENABLE) idf_component_optional_requires(PRIVATE pm) diff --git a/components/esp_system/panic.c b/components/esp_system/panic.c index 1804e84f0f..f709d812fc 100644 --- a/components/esp_system/panic.c +++ b/components/esp_system/panic.c @@ -26,10 +26,12 @@ #include "sdkconfig.h" +#if !CONFIG_ESP_SYSTEM_PANIC_SILENT_REBOOT #if __has_include("esp_app_desc.h") #define WITH_ELF_SHA256 #include "esp_app_desc.h" #endif +#endif // CONFIG_ESP_SYSTEM_PANIC_SILENT_REBOOT #if CONFIG_ESP_COREDUMP_ENABLE #include "esp_core_dump.h" @@ -332,11 +334,9 @@ void esp_panic_handler(panic_info_t *info) #ifdef WITH_ELF_SHA256 panic_print_str("\r\nELF file SHA256: "); - char sha256_buf[65]; - esp_app_get_elf_sha256(sha256_buf, sizeof(sha256_buf)); - panic_print_str(sha256_buf); + panic_print_str(esp_app_get_elf_sha256_str()); panic_print_str("\r\n"); -#endif +#endif // WITH_ELF_SHA256 panic_print_str("\r\n"); diff --git a/components/espcoredump/src/core_dump_elf.c b/components/espcoredump/src/core_dump_elf.c index 35be9347a1..59e5e51149 100644 --- a/components/espcoredump/src/core_dump_elf.c +++ b/components/espcoredump/src/core_dump_elf.c @@ -498,7 +498,11 @@ static int elf_write_core_dump_info(core_dump_elf_t *self) ESP_COREDUMP_LOG_PROCESS("================ Processing coredump info ================"); int data_len = (int)sizeof(self->elf_version_info.app_elf_sha256); - data_len = esp_app_get_elf_sha256((char*)self->elf_version_info.app_elf_sha256, (size_t)data_len); + // This dump function can be called when the cache is diable which requires putting + // esp_app_get_elf_sha256() into IRAM. But we want to reduce IRAM usage, + // so we use a function that returns an already formatted string. + strncpy((char*)self->elf_version_info.app_elf_sha256, esp_app_get_elf_sha256_str(), (size_t)data_len); + data_len = strlen((char*)self->elf_version_info.app_elf_sha256) + 1; // + 1 for the null terminator ESP_COREDUMP_LOG_PROCESS("Application SHA256='%s', length=%d.", self->elf_version_info.app_elf_sha256, data_len); self->elf_version_info.version = esp_core_dump_elf_version(); diff --git a/tools/test_apps/system/panic/test_panic_util/panic_dut.py b/tools/test_apps/system/panic/test_panic_util/panic_dut.py index 4afa79855e..e9511b043e 100644 --- a/tools/test_apps/system/panic/test_panic_util/panic_dut.py +++ b/tools/test_apps/system/panic/test_panic_util/panic_dut.py @@ -92,7 +92,7 @@ class PanicTestDut(IdfDut): """Expect method for ELF SHA256 line""" elf_sha256 = sha256(self.app.elf_file) elf_sha256_len = int( - self.app.sdkconfig.get('CONFIG_APP_RETRIEVE_LEN_ELF_SHA', '16') + self.app.sdkconfig.get('CONFIG_APP_RETRIEVE_LEN_ELF_SHA', '9') ) self.expect_exact('ELF file SHA256: ' + elf_sha256[0:elf_sha256_len])