From 523aacd4133a9e0a4b48a9fb2cffa39a30cade02 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 17 May 2022 23:42:54 +0200 Subject: [PATCH] esp_system: allow defining priorities for startup functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Some components have initialization dependencies. To account for them, simple numeric priority values are introduced. * esp_system_init_fn_array moved into Flash from DRAM * System init functions defined using ESP_SYSTEM_INIT_FN now return an error code. This enables simpler and more consistent error handling in the init functions. Returning an error from an init function is now a valid approach — the startup code will print an error and abort. --- .../include/esp_private/startup_internal.h | 49 +++++++++++++------ components/esp_system/ld/esp32/sections.ld.in | 8 +-- .../esp_system/ld/esp32c2/sections.ld.in | 8 +-- .../esp_system/ld/esp32c3/sections.ld.in | 8 +-- .../esp_system/ld/esp32h2/sections.ld.in | 8 +-- .../esp_system/ld/esp32s2/sections.ld.in | 8 +-- .../esp_system/ld/esp32s3/sections.ld.in | 8 +-- components/esp_system/startup.c | 33 ++++++++++--- 8 files changed, 85 insertions(+), 45 deletions(-) diff --git a/components/esp_system/include/esp_private/startup_internal.h b/components/esp_system/include/esp_private/startup_internal.h index 3e59429c2a..da39b4b1fa 100644 --- a/components/esp_system/include/esp_private/startup_internal.h +++ b/components/esp_system/include/esp_private/startup_internal.h @@ -7,6 +7,8 @@ #pragma once #include "esp_attr.h" +#include "esp_err.h" +#include "esp_bit_defs.h" #include "soc/soc_caps.h" #include "hal/cpu_hal.h" @@ -36,25 +38,44 @@ extern sys_startup_fn_t const g_startup_fn[1]; void startup_resume_other_cores(void); #endif +/** + * Internal structure describing ESP_SYSTEM_INIT_FN startup functions + */ typedef struct { - void (*fn)(void); - uint32_t cores; + esp_err_t (*fn)(void); /*!< Pointer to the startup function */ + uint32_t cores; /*!< Bit map of cores where the function has to be called */ } esp_system_init_fn_t; -/* - * Declare an component initialization function that will execute on the specified cores (ex. if BIT0 == 1, will execute - * on CORE0, CORE1 if BIT1 and so on). +/** + * @brief Define a system initialization function which will be executed on the specified cores * - * @note Initialization functions should be placed in a compilation unit where at least one other - * symbol is referenced 'meaningfully' in another compilation unit, otherwise this gets discarded during linking. (By - * 'meaningfully' we mean the reference should not itself get optimized out by the compiler/discarded by the linker). + * @param f function name (identifier) + * @param c bit mask of cores to execute the function on (ex. if BIT0 is set, the function + * will be executed on CPU 0, if BIT1 is set - on CPU 1, and so on) + * @param priority integer, priority of the initialization function. Higher values mean that + * the function will be executed later in the process. + * @param (varargs) optional, additional attributes for the function declaration (such as IRAM_ATTR) + * + * The function defined using this macro must return ESP_OK on success. Any other value will be + * logged and the startup process will abort. + * + * Initialization functions should be placed in a compilation unit where at least one other + * symbol is referenced in another compilation unit. This means that the reference should not itself + * get optimized out by the compiler or discarded by the linker if the related feature is used. + * It is, on the other hand, a good practice to make sure the initialization function does get + * discarded if the related feature is not used. */ -#define ESP_SYSTEM_INIT_FN(f, c, ...) \ -static void __attribute__((used)) __VA_ARGS__ __esp_system_init_fn_##f(void); \ -static __attribute__((used)) esp_system_init_fn_t _SECTION_ATTR_IMPL(".esp_system_init_fn", f) \ - esp_system_init_fn_##f = { .fn = ( __esp_system_init_fn_##f), .cores = (c) }; \ -static __attribute__((used)) __VA_ARGS__ void __esp_system_init_fn_##f(void) // [refactor-todo] this can be made public API if we allow components to declare init functions, - // instead of calling them explicitly +#define ESP_SYSTEM_INIT_FN(f, c, priority, ...) \ + static esp_err_t __VA_ARGS__ __esp_system_init_fn_##f(void); \ + static __attribute__((used)) _SECTION_ATTR_IMPL(".esp_system_init_fn", priority) \ + esp_system_init_fn_t esp_system_init_fn_##f = { .fn = ( __esp_system_init_fn_##f), .cores = (c) }; \ + static esp_err_t __esp_system_init_fn_##f(void) + +#ifdef CONFIG_ESP_SYSTEM_SINGLE_CORE_MODE +#define ESP_SYSTEM_INIT_ALL_CORES BIT(0) +#else +#define ESP_SYSTEM_INIT_ALL_CORES (BIT(SOC_CPU_CORES_NUM) - 1) +#endif extern uint64_t g_startup_time; // Startup time that serves as the point of origin for system time. Should be set by the entry // function in the port layer. May be 0 as well if this is not backed by a persistent counter, in which case diff --git a/components/esp_system/ld/esp32/sections.ld.in b/components/esp_system/ld/esp32/sections.ld.in index 1de48ab1e5..29f581ef8e 100644 --- a/components/esp_system/ld/esp32/sections.ld.in +++ b/components/esp_system/ld/esp32/sections.ld.in @@ -185,10 +185,6 @@ SECTIONS *(.gnu.linkonce.s2.*) *(.jcr) - _esp_system_init_fn_array_start = ABSOLUTE(.); - KEEP (*(SORT(.esp_system_init_fn) SORT(.esp_system_init_fn.*))) - _esp_system_init_fn_array_end = ABSOLUTE(.); - mapping[dram0_data] _data_end = ABSOLUTE(.); @@ -304,6 +300,10 @@ SECTIONS soc_reserved_memory_region_start = ABSOLUTE(.); KEEP (*(.reserved_memory_address)) soc_reserved_memory_region_end = ABSOLUTE(.); + /* System init functions registered via ESP_SYSTEM_INIT_FN */ + _esp_system_init_fn_array_start = ABSOLUTE(.); + KEEP (*(SORT_BY_INIT_PRIORITY(.esp_system_init_fn.*))) + _esp_system_init_fn_array_end = ABSOLUTE(.); _rodata_end = ABSOLUTE(.); /* Literals are also RO data. */ _lit4_start = ABSOLUTE(.); diff --git a/components/esp_system/ld/esp32c2/sections.ld.in b/components/esp_system/ld/esp32c2/sections.ld.in index 6b7b6a7417..66431103de 100644 --- a/components/esp_system/ld/esp32c2/sections.ld.in +++ b/components/esp_system/ld/esp32c2/sections.ld.in @@ -47,10 +47,6 @@ SECTIONS *(.gnu.linkonce.s2.*) *(.jcr) - _esp_system_init_fn_array_start = ABSOLUTE(.); - KEEP (*(SORT(.esp_system_init_fn) SORT(.esp_system_init_fn.*))) - _esp_system_init_fn_array_end = ABSOLUTE(.); - mapping[dram0_data] _data_end = ABSOLUTE(.); @@ -208,6 +204,10 @@ SECTIONS soc_reserved_memory_region_start = ABSOLUTE(.); KEEP (*(.reserved_memory_address)) soc_reserved_memory_region_end = ABSOLUTE(.); + /* System init functions registered via ESP_SYSTEM_INIT_FN */ + _esp_system_init_fn_array_start = ABSOLUTE(.); + KEEP (*(SORT_BY_INIT_PRIORITY(.esp_system_init_fn.*))) + _esp_system_init_fn_array_end = ABSOLUTE(.); _rodata_end = ABSOLUTE(.); /* Literals are also RO data. */ _lit4_start = ABSOLUTE(.); diff --git a/components/esp_system/ld/esp32c3/sections.ld.in b/components/esp_system/ld/esp32c3/sections.ld.in index 8803f0254d..d6670bf759 100644 --- a/components/esp_system/ld/esp32c3/sections.ld.in +++ b/components/esp_system/ld/esp32c3/sections.ld.in @@ -157,10 +157,6 @@ SECTIONS *(.gnu.linkonce.s2.*) *(.jcr) - _esp_system_init_fn_array_start = ABSOLUTE(.); - KEEP (*(SORT(.esp_system_init_fn) SORT(.esp_system_init_fn.*))) - _esp_system_init_fn_array_end = ABSOLUTE(.); - mapping[dram0_data] _data_end = ABSOLUTE(.); @@ -318,6 +314,10 @@ SECTIONS soc_reserved_memory_region_start = ABSOLUTE(.); KEEP (*(.reserved_memory_address)) soc_reserved_memory_region_end = ABSOLUTE(.); + /* System init functions registered via ESP_SYSTEM_INIT_FN */ + _esp_system_init_fn_array_start = ABSOLUTE(.); + KEEP (*(SORT_BY_INIT_PRIORITY(.esp_system_init_fn.*))) + _esp_system_init_fn_array_end = ABSOLUTE(.); _rodata_end = ABSOLUTE(.); /* Literals are also RO data. */ _lit4_start = ABSOLUTE(.); diff --git a/components/esp_system/ld/esp32h2/sections.ld.in b/components/esp_system/ld/esp32h2/sections.ld.in index a49f22e70e..93239b0dec 100644 --- a/components/esp_system/ld/esp32h2/sections.ld.in +++ b/components/esp_system/ld/esp32h2/sections.ld.in @@ -160,10 +160,6 @@ SECTIONS *(.gnu.linkonce.s2.*) *(.jcr) - _esp_system_init_fn_array_start = ABSOLUTE(.); - KEEP (*(SORT(.esp_system_init_fn) SORT(.esp_system_init_fn.*))) - _esp_system_init_fn_array_end = ABSOLUTE(.); - mapping[dram0_data] _data_end = ABSOLUTE(.); @@ -321,6 +317,10 @@ SECTIONS soc_reserved_memory_region_start = ABSOLUTE(.); KEEP (*(.reserved_memory_address)) soc_reserved_memory_region_end = ABSOLUTE(.); + /* System init functions registered via ESP_SYSTEM_INIT_FN */ + _esp_system_init_fn_array_start = ABSOLUTE(.); + KEEP (*(SORT_BY_INIT_PRIORITY(.esp_system_init_fn.*))) + _esp_system_init_fn_array_end = ABSOLUTE(.); _rodata_end = ABSOLUTE(.); /* Literals are also RO data. */ _lit4_start = ABSOLUTE(.); diff --git a/components/esp_system/ld/esp32s2/sections.ld.in b/components/esp_system/ld/esp32s2/sections.ld.in index 4e1ccc83b2..16b9104edd 100644 --- a/components/esp_system/ld/esp32s2/sections.ld.in +++ b/components/esp_system/ld/esp32s2/sections.ld.in @@ -207,10 +207,6 @@ SECTIONS *(.gnu.linkonce.s2.*) *(.jcr) - _esp_system_init_fn_array_start = ABSOLUTE(.); - KEEP (*(SORT(.esp_system_init_fn) SORT(.esp_system_init_fn.*))) - _esp_system_init_fn_array_end = ABSOLUTE(.); - mapping[dram0_data] _data_end = ABSOLUTE(.); @@ -322,6 +318,10 @@ SECTIONS soc_reserved_memory_region_start = ABSOLUTE(.); KEEP (*(.reserved_memory_address)) soc_reserved_memory_region_end = ABSOLUTE(.); + /* System init functions registered via ESP_SYSTEM_INIT_FN */ + _esp_system_init_fn_array_start = ABSOLUTE(.); + KEEP (*(SORT_BY_INIT_PRIORITY(.esp_system_init_fn.*))) + _esp_system_init_fn_array_end = ABSOLUTE(.); _rodata_end = ABSOLUTE(.); /* Literals are also RO data. */ _lit4_start = ABSOLUTE(.); diff --git a/components/esp_system/ld/esp32s3/sections.ld.in b/components/esp_system/ld/esp32s3/sections.ld.in index 5c0d41b1ca..13741b45ef 100644 --- a/components/esp_system/ld/esp32s3/sections.ld.in +++ b/components/esp_system/ld/esp32s3/sections.ld.in @@ -194,10 +194,6 @@ SECTIONS *(.gnu.linkonce.s2.*) *(.jcr) - _esp_system_init_fn_array_start = ABSOLUTE(.); - KEEP (*(SORT(.esp_system_init_fn) SORT(.esp_system_init_fn.*))) - _esp_system_init_fn_array_end = ABSOLUTE(.); - mapping[dram0_data] _data_end = ABSOLUTE(.); @@ -347,6 +343,10 @@ SECTIONS soc_reserved_memory_region_start = ABSOLUTE(.); KEEP (*(.reserved_memory_address)) soc_reserved_memory_region_end = ABSOLUTE(.); + /* System init functions registered via ESP_SYSTEM_INIT_FN */ + _esp_system_init_fn_array_start = ABSOLUTE(.); + KEEP (*(SORT_BY_INIT_PRIORITY(.esp_system_init_fn.*))) + _esp_system_init_fn_array_end = ABSOLUTE(.); _rodata_end = ABSOLUTE(.); /* Literals are also RO data. */ _lit4_start = ABSOLUTE(.); diff --git a/components/esp_system/startup.c b/components/esp_system/startup.c index e1c00d8fcd..dfb7e75c4b 100644 --- a/components/esp_system/startup.c +++ b/components/esp_system/startup.c @@ -179,17 +179,25 @@ static void do_global_ctors(void) #if __riscv for (p = &__init_priority_array_start; p < &__init_priority_array_end; ++p) { - ESP_EARLY_LOGD(TAG, "calling init function: %p", *p); + ESP_LOGD(TAG, "calling init function: %p", *p); (*p)(); } #endif for (p = &__init_array_end - 1; p >= &__init_array_start; --p) { - ESP_EARLY_LOGD(TAG, "calling init function: %p", *p); + ESP_LOGD(TAG, "calling init function: %p", *p); (*p)(); } } +/** + * @brief Call component init functions defined using ESP_SYSTEM_INIT_Fn macros. + * The esp_system_init_fn_t structures describing these functions are collected into + * an array [_esp_system_init_fn_array_start, _esp_system_init_fn_array_end) by the + * linker. The functions are sorted by their priority value. + * The sequence of the init function calls (sorted by priority) is documented in + * system_init_fn.txt file. + */ static void do_system_init_fn(void) { extern esp_system_init_fn_t _esp_system_init_fn_array_start; @@ -197,14 +205,20 @@ static void do_system_init_fn(void) esp_system_init_fn_t *p; - for (p = &_esp_system_init_fn_array_end - 1; p >= &_esp_system_init_fn_array_start; --p) { - if (p->cores & BIT(cpu_hal_get_core_id())) { - (*(p->fn))(); + int core_id = cpu_hal_get_core_id(); + for (p = &_esp_system_init_fn_array_start; p < &_esp_system_init_fn_array_end; ++p) { + if (p->cores & BIT(core_id)) { + ESP_LOGD(TAG, "calling init function: %p on core: %d", p->fn, core_id); + esp_err_t err = (*(p->fn))(); + if (err != ESP_OK) { + ESP_LOGE(TAG, "init function %p has failed (0x%x), aborting", p->fn, err); + abort(); + } } } #if !CONFIG_ESP_SYSTEM_SINGLE_CORE_MODE - s_system_inited[cpu_hal_get_core_id()] = true; + s_system_inited[core_id] = true; #endif } @@ -216,6 +230,9 @@ static void esp_startup_start_app_other_cores_default(void) } } +/* This function has to be in IRAM, as while it is running on CPU1, CPU0 may do some flash operations + * (e.g. initialize the core dump), which means that cache will be disabled. + */ static void IRAM_ATTR start_cpu_other_cores_default(void) { do_system_init_fn(); @@ -428,7 +445,7 @@ static void start_cpu0_default(void) while (1); } -IRAM_ATTR ESP_SYSTEM_INIT_FN(init_components0, BIT(0)) +ESP_SYSTEM_INIT_FN(init_components0, BIT(0), 200) { esp_timer_init(); @@ -474,4 +491,6 @@ IRAM_ATTR ESP_SYSTEM_INIT_FN(init_components0, BIT(0)) _Unwind_SetNoFunctionContextInstall(1); _Unwind_SetEnableExceptionFdeSorting(0); #endif // CONFIG_COMPILER_CXX_EXCEPTIONS + + return ESP_OK; }