From 1fc288556cc097cc2d9b9edfd975254409e5a17c Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 24 Jun 2021 18:02:28 +1000 Subject: [PATCH] esp_system: Reconfigure the WDTs at the start of the panic handler This is mostly important on ESP32 ECO3 with the ESP32_ECO3_CACHE_LOCK_FIX, because when we stall the other CPU core before we disable the TG1 WDT then the first CPU can get stuck in WDT ISR handle_livelock_int routine waiting for the other CPU. --- components/esp_system/panic.c | 28 +++++++++++++++------- components/esp_system/port/panic_handler.c | 5 ++++ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/components/esp_system/panic.c b/components/esp_system/panic.c index 196e46ec6d..5c5b4f0d3d 100644 --- a/components/esp_system/panic.c +++ b/components/esp_system/panic.c @@ -64,8 +64,6 @@ bool g_panic_abort = false; static char *s_panic_abort_details = NULL; static wdt_hal_context_t rtc_wdt_ctx = {.inst = WDT_RWDT, .rwdt_dev = &RTCCNTL}; -static wdt_hal_context_t wdt0_context = {.inst = WDT_MWDT0, .mwdt_dev = &TIMERG0}; -static wdt_hal_context_t wdt1_context = {.inst = WDT_MWDT1, .mwdt_dev = &TIMERG1}; #if !CONFIG_ESP_SYSTEM_PANIC_SILENT_REBOOT @@ -140,9 +138,16 @@ void panic_print_dec(int d) the risk of somehow halting in the panic handler and not resetting. That is why this routine kills all watchdogs except the timer group 0 watchdog, and it reconfigures that to reset the chip after one second. + + We have to do this before we do anything that might cause issues in the WDT interrupt handlers, + for example stalling the other core on ESP32 may cause the ESP32_ECO3_CACHE_LOCK_FIX + handler to get stuck. */ -static void reconfigure_all_wdts(void) +void esp_panic_handler_reconfigure_wdts(void) { + wdt_hal_context_t wdt0_context = {.inst = WDT_MWDT0, .mwdt_dev = &TIMERG0}; + wdt_hal_context_t wdt1_context = {.inst = WDT_MWDT1, .mwdt_dev = &TIMERG1}; + //Todo: Refactor to use Interrupt or Task Watchdog API, and a system level WDT context //Reconfigure TWDT (Timer Group 0) wdt_hal_init(&wdt0_context, WDT_MWDT0, MWDT0_TICK_PRESCALER, false); //Prescaler: wdt counts in ticks of TG0_WDT_TICK_US @@ -162,6 +167,9 @@ static void reconfigure_all_wdts(void) */ static inline void disable_all_wdts(void) { + wdt_hal_context_t wdt0_context = {.inst = WDT_MWDT0, .mwdt_dev = &TIMERG0}; + wdt_hal_context_t wdt1_context = {.inst = WDT_MWDT1, .mwdt_dev = &TIMERG1}; + //Todo: Refactor to use Interrupt or Task Watchdog API, and a system level WDT context //Task WDT is the Main Watchdog Timer of Timer Group 0 wdt_hal_write_protect_disable(&wdt0_context); @@ -184,6 +192,10 @@ static void print_abort_details(const void *f) // already been done, and panic_info_t has been filled. void esp_panic_handler(panic_info_t *info) { + // The port-level panic handler has already called this, but call it again + // to reset the TG0WDT period + esp_panic_handler_reconfigure_wdts(); + // If the exception was due to an abort, override some of the panic info if (g_panic_abort) { info->description = NULL; @@ -267,8 +279,7 @@ void esp_panic_handler(panic_info_t *info) } - //Feed the watchdogs, so they will give us time to print out debug info - reconfigure_all_wdts(); + esp_panic_handler_reconfigure_wdts(); // Restart WDT again PANIC_INFO_DUMP(info, state); panic_print_str("\r\n"); @@ -289,8 +300,8 @@ void esp_panic_handler(panic_info_t *info) esp_apptrace_flush_nolock(ESP_APPTRACE_DEST_TRAX, CONFIG_APPTRACE_POSTMORTEM_FLUSH_THRESH, APPTRACE_ONPANIC_HOST_FLUSH_TMO); #endif - reconfigure_all_wdts(); -#endif + esp_panic_handler_reconfigure_wdts(); // restore WDT config +#endif // CONFIG_APPTRACE_ENABLE #if CONFIG_ESP_SYSTEM_PANIC_GDBSTUB disable_all_wdts(); @@ -314,7 +325,8 @@ void esp_panic_handler(panic_info_t *info) esp_core_dump_to_uart(info); #endif s_dumping_core = false; - reconfigure_all_wdts(); + + esp_panic_handler_reconfigure_wdts(); } #endif /* CONFIG_ESP_COREDUMP_ENABLE */ wdt_hal_write_protect_disable(&rtc_wdt_ctx); diff --git a/components/esp_system/port/panic_handler.c b/components/esp_system/port/panic_handler.c index a5fcd09a69..45a238acce 100644 --- a/components/esp_system/port/panic_handler.c +++ b/components/esp_system/port/panic_handler.c @@ -50,6 +50,8 @@ extern int _invalid_pc_placeholder; +extern void esp_panic_handler_reconfigure_wdts(void); + extern void esp_panic_handler(panic_info_t *); static wdt_hal_context_t wdt0_context = {.inst = WDT_MWDT0, .mwdt_dev = &TIMERG0}; @@ -156,6 +158,9 @@ static void panic_handler(void *frame, bool pseudo_excause) } } + // Need to reconfigure WDTs before we stall any other CPU + esp_panic_handler_reconfigure_wdts(); + esp_rom_delay_us(1); SOC_HAL_STALL_OTHER_CORES(); #endif