From 2b0f623259f8af0797cf2a488b0e54762ca34a08 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 12 Jul 2017 10:25:13 +0800 Subject: [PATCH 1/2] bootloader/early boot: Error out if >192KB of static DRAM is allocated (temporary fix) Currently the last 128KB of DRAM is reserved for the bootloader & early boot stacks. This means if >192KB of static DRAM is allocated, the only available heap is this region - which is disabled until the scheduler starts. As a result, you get either heap corruption on early boot if the static data overlaps startup heap (leading to very weird errors), or FreeRTOS will fail to start when it can't malloc() anything. Long term fix is to move the stacks & bootloader data to the very end of RAM, and only reserve that part for early boot. This is a little fiddly because of also wanting to make sure this memory is not preemptively fragmented when it gets reintroduced to the heap. This will become more important if/when we have more static allocation options in the future. For now, these errors make it clear why the boot has failed. Ref TW13909 --- .../bootloader/src/main/bootloader_start.c | 7 +++++++ components/esp32/cpu_start.c | 19 ++++++++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/components/bootloader/src/main/bootloader_start.c b/components/bootloader/src/main/bootloader_start.c index fd062a5f9..123f67d8f 100644 --- a/components/bootloader/src/main/bootloader_start.c +++ b/components/bootloader/src/main/bootloader_start.c @@ -520,6 +520,13 @@ static void unpack_load_app(const esp_partition_pos_t* partition) bootloader RAM... */ if (end_addr < 0x40000000) { + if (end_addr > 0x3FFE0000) { + /* Temporary workaround for an ugly crash, until we allow >192KB of static DRAM */ + ESP_LOGE(TAG, "DRAM segment %d (start 0x%08x end 0x%08x) too large for IDF to boot", + segment, start_addr, end_addr); + return; + } + sp = (intptr_t)get_sp(); if (end_addr > sp) { ESP_LOGE(TAG, "Segment %d end address %08x overlaps bootloader stack %08x - can't load", diff --git a/components/esp32/cpu_start.c b/components/esp32/cpu_start.c index 858036c67..db32c4fdc 100644 --- a/components/esp32/cpu_start.c +++ b/components/esp32/cpu_start.c @@ -68,12 +68,12 @@ #define STRINGIFY(s) STRINGIFY2(s) #define STRINGIFY2(s) #s -void start_cpu0(void) __attribute__((weak, alias("start_cpu0_default"))); -void start_cpu0_default(void) IRAM_ATTR; +void start_cpu0(void) __attribute__((weak, alias("start_cpu0_default"))) __attribute__((noreturn)); +void start_cpu0_default(void) IRAM_ATTR __attribute__((noreturn)); #if !CONFIG_FREERTOS_UNICORE -static void IRAM_ATTR call_start_cpu1(); -void start_cpu1(void) __attribute__((weak, alias("start_cpu1_default"))); -void start_cpu1_default(void) IRAM_ATTR; +static void IRAM_ATTR call_start_cpu1() __attribute__((noreturn)); +void start_cpu1(void) __attribute__((weak, alias("start_cpu1_default"))) __attribute__((noreturn)); +void start_cpu1_default(void) IRAM_ATTR __attribute__((noreturn)); static bool app_cpu_started = false; #endif //!CONFIG_FREERTOS_UNICORE @@ -126,6 +126,13 @@ void IRAM_ATTR call_start_cpu0() esp_panic_wdt_stop(); } + // Temporary workaround for an ugly crash, until we allow > 192KB of static DRAM + if ((intptr_t)&_bss_end > 0x3FFE0000) { + // Can't use assert() or logging here because there's no .bss + ets_printf("ERROR: Static .bss section extends past 0x3FFE0000. IDF cannot boot.\n"); + abort(); + } + //Clear BSS. Please do not attempt to do any complex stuff (like early logging) before this. memset(&_bss_start, 0, (&_bss_end - &_bss_start) * sizeof(_bss_start)); @@ -287,6 +294,7 @@ void start_cpu0_default(void) ESP_TASK_MAIN_PRIO, NULL, 0); ESP_LOGI(TAG, "Starting scheduler on PRO CPU."); vTaskStartScheduler(); + abort(); /* Only get to here if not enough free heap to start scheduler */ } #if !CONFIG_FREERTOS_UNICORE @@ -313,6 +321,7 @@ void start_cpu1_default(void) ESP_EARLY_LOGI(TAG, "Starting scheduler on APP CPU."); xPortStartScheduler(); + abort(); /* Only get to here if FreeRTOS somehow very broken */ } #endif //!CONFIG_FREERTOS_UNICORE From 99fe61716c8cafe1778c19d55009a51bd7a85499 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 12 Jul 2017 11:33:51 +0800 Subject: [PATCH 2/2] startup: Add assertion checks around various initialisation sequences These may fail if close to 192KB of static RAM is allocated (remaining early heap RAM is too small.) --- components/esp32/cpu_start.c | 7 ++++--- components/esp32/dport_access.c | 3 ++- components/esp32/ipc.c | 5 +++-- components/esp32/task_wdt.c | 2 +- components/spi_flash/cache_utils.c | 1 + 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/components/esp32/cpu_start.c b/components/esp32/cpu_start.c index db32c4fdc..80942c2af 100644 --- a/components/esp32/cpu_start.c +++ b/components/esp32/cpu_start.c @@ -289,9 +289,10 @@ void start_cpu0_default(void) esp_core_dump_init(); #endif - xTaskCreatePinnedToCore(&main_task, "main", - ESP_TASK_MAIN_STACK, NULL, - ESP_TASK_MAIN_PRIO, NULL, 0); + portBASE_TYPE res = xTaskCreatePinnedToCore(&main_task, "main", + ESP_TASK_MAIN_STACK, NULL, + ESP_TASK_MAIN_PRIO, NULL, 0); + assert(res == pdTRUE); ESP_LOGI(TAG, "Starting scheduler on PRO CPU."); vTaskStartScheduler(); abort(); /* Only get to here if not enough free heap to start scheduler */ diff --git a/components/esp32/dport_access.c b/components/esp32/dport_access.c index 473c43325..49e6032d2 100644 --- a/components/esp32/dport_access.c +++ b/components/esp32/dport_access.c @@ -173,7 +173,8 @@ static void dport_access_init_core(void *arg) /* Defer initialisation until after scheduler is running */ void esp_dport_access_int_init(void) { - xTaskCreatePinnedToCore(&dport_access_init_core, "dport", configMINIMAL_STACK_SIZE, NULL, 5, NULL, xPortGetCoreID()); + portBASE_TYPE res = xTaskCreatePinnedToCore(&dport_access_init_core, "dport", configMINIMAL_STACK_SIZE, NULL, 5, NULL, xPortGetCoreID()); + assert(res == pdTRUE); } void esp_dport_access_int_deinit(void) diff --git a/components/esp32/ipc.c b/components/esp32/ipc.c index 41f7b8573..4f6398364 100644 --- a/components/esp32/ipc.c +++ b/components/esp32/ipc.c @@ -80,8 +80,9 @@ void esp_ipc_init() const char* task_names[2] = {"ipc0", "ipc1"}; for (int i = 0; i < portNUM_PROCESSORS; ++i) { s_ipc_sem[i] = xSemaphoreCreateBinary(); - xTaskCreatePinnedToCore(ipc_task, task_names[i], CONFIG_IPC_TASK_STACK_SIZE, (void*) i, - configMAX_PRIORITIES - 1, &s_ipc_tasks[i], i); + portBASE_TYPE res = xTaskCreatePinnedToCore(ipc_task, task_names[i], CONFIG_IPC_TASK_STACK_SIZE, (void*) i, + configMAX_PRIORITIES - 1, &s_ipc_tasks[i], i); + assert(res == pdTRUE); } } diff --git a/components/esp32/task_wdt.c b/components/esp32/task_wdt.c index 8585260e2..eaed32b4c 100644 --- a/components/esp32/task_wdt.c +++ b/components/esp32/task_wdt.c @@ -202,7 +202,7 @@ void esp_task_wdt_init() { #if CONFIG_TASK_WDT_CHECK_IDLE_TASK esp_register_freertos_idle_hook(idle_hook); #endif - esp_intr_alloc(ETS_TG0_WDT_LEVEL_INTR_SOURCE, 0, task_wdt_isr, NULL, NULL); + ESP_ERROR_CHECK( esp_intr_alloc(ETS_TG0_WDT_LEVEL_INTR_SOURCE, 0, task_wdt_isr, NULL, NULL) ); } diff --git a/components/spi_flash/cache_utils.c b/components/spi_flash/cache_utils.c index 3bb08fe9c..605c60115 100644 --- a/components/spi_flash/cache_utils.c +++ b/components/spi_flash/cache_utils.c @@ -48,6 +48,7 @@ static volatile int s_flash_op_cpu = -1; void spi_flash_init_lock() { s_flash_op_mutex = xSemaphoreCreateMutex(); + assert(s_flash_op_mutex != NULL); } void spi_flash_op_lock()