From fa0348b512381629d80d8d5237bc662f096cff82 Mon Sep 17 00:00:00 2001 From: Li Shuai Date: Wed, 25 Mar 2020 16:16:42 +0800 Subject: [PATCH] Fixed interrupt watchdog error caused by live lock --- components/esp32/Kconfig | 7 +- components/esp32/cpu_start.c | 4 +- components/esp32/dport_panic_highint_hdl.S | 201 +++++++++++++++++---- components/esp32/int_wdt.c | 13 +- components/esp32/panic.c | 9 - 5 files changed, 176 insertions(+), 58 deletions(-) diff --git a/components/esp32/Kconfig b/components/esp32/Kconfig index 0d01a3bc12..739f4e6340 100644 --- a/components/esp32/Kconfig +++ b/components/esp32/Kconfig @@ -6,6 +6,11 @@ menu "ESP32-specific" default "y" if IDF_TARGET="esp32" default "n" + config ESP32_ECO3_CACHE_LOCK_FIX + bool + default y + depends on !FREERTOS_UNICORE && SPIRAM_SUPPORT + choice ESP32_REV_MIN prompt "Minimum Supported ESP32 Revision" default ESP32_REV_MIN_0 @@ -21,7 +26,7 @@ menu "ESP32-specific" bool "Rev 2" config ESP32_REV_MIN_3 bool "Rev 3" - select INT_WDT if !FREERTOS_UNICORE && SPIRAM_SUPPORT + select INT_WDT if ESP32_ECO3_CACHE_LOCK_FIX endchoice config ESP32_REV_MIN diff --git a/components/esp32/cpu_start.c b/components/esp32/cpu_start.c index 156ec4662c..61fefd5b17 100644 --- a/components/esp32/cpu_start.c +++ b/components/esp32/cpu_start.c @@ -389,8 +389,8 @@ void start_cpu0_default(void) //Initialize the interrupt watch dog for CPU0. esp_int_wdt_cpu_init(); #else -#if !defined(CONFIG_FREERTOS_UNICORE) && defined(CONFIG_SPIRAM_SUPPORT) - assert(!soc_has_cache_lock_bug() && "Minimum Supported ESP32 Revision requires Rev 3"); +#if CONFIG_ESP32_ECO3_CACHE_LOCK_FIX + assert(!soc_has_cache_lock_bug() && "ESP32 Rev 3 + Dual Core + PSRAM requires INT WDT enabled in project config!"); #endif #endif esp_cache_err_int_init(); diff --git a/components/esp32/dport_panic_highint_hdl.S b/components/esp32/dport_panic_highint_hdl.S index b5576aaebf..5e6a271097 100644 --- a/components/esp32/dport_panic_highint_hdl.S +++ b/components/esp32/dport_panic_highint_hdl.S @@ -39,9 +39,9 @@ Interrupt , a high-priority interrupt, is used for several things: #define L5_INTR_A4_OFFSET 8 .data _l5_intr_stack: - .space L5_INTR_STACK_SIZE + .space L5_INTR_STACK_SIZE*portNUM_PROCESSORS -#if !defined(CONFIG_FREERTOS_UNICORE) && defined(CONFIG_SPIRAM_SUPPORT) +#if CONFIG_ESP32_ECO3_CACHE_LOCK_FIX .global _l4_intr_livelock_counter .global _l4_intr_livelock_max .align 16 @@ -49,6 +49,10 @@ _l4_intr_livelock_counter: .word 0 _l4_intr_livelock_max: .word 0 +_l4_intr_livelock_sync: + .word 0, 0 +_l4_intr_livelock_app: + .word 0 #endif .section .iram1,"ax" @@ -64,15 +68,12 @@ xt_highint5: bnez a0, .handle_dport_access_int #endif // CONFIG_FREERTOS_UNICORE -#if !defined(CONFIG_FREERTOS_UNICORE) && defined(CONFIG_SPIRAM_SUPPORT) +#if CONFIG_ESP32_ECO3_CACHE_LOCK_FIX /* See if we're here for the tg1 watchdog interrupt */ rsr a0, INTERRUPT extui a0, a0, ETS_T1_WDT_INUM, 1 beqz a0, 1f - getcoreid a0 - bnez a0, 1f /* App cpu (Core 1) jump bypass */ - /* Pro cpu (Core 0) can execute to here. */ wsr a5, depc /* use DEPC as temp storage */ movi a0, _l4_intr_livelock_counter @@ -161,19 +162,115 @@ xt_highint5: rfi 5 -#if !defined(CONFIG_FREERTOS_UNICORE) && defined(CONFIG_SPIRAM_SUPPORT) +#if CONFIG_ESP32_ECO3_CACHE_LOCK_FIX + +/* +-------------------------------------------------------------------------------- + Macro intr_matrix_map - Attach an CPU interrupt to a hardware source. + + Input : "addr" - Interrupt map configuration base address + Input : "src" - Interrupt source. + Input : "inum" - Interrupt number. +-------------------------------------------------------------------------------- +*/ + .macro intr_matrix_map addr src inum + movi a2, \src + slli a2, a2, 2 + movi a3, \addr + add a3, a3, a2 + movi a2, \inum + s32i a2, a3, 0 + .endm + +/* +-------------------------------------------------------------------------------- + Macro wdt_clr_intr_status - Clear the WDT interrupt status. + Macro wdt_feed - Feed the WDT. + + Input : "dev" - Beginning address of the peripheral registers +-------------------------------------------------------------------------------- +*/ + .macro wdt_clr_intr_status dev + movi a2, \dev + movi a3, TIMG_WDT_WKEY_VALUE + memw + s32i a3, a2, 100 /* disable write protect */ + l32i a4, a2, 164 + movi a3, 4 + or a3, a4, a3 + memw + s32i a3, a2, 164 /* clear 1st stage timeout interrupt */ + movi a3, 0 + s32i a3, a2, 100 /* enable write protect */ + .endm + + .macro wdt_feed dev + movi a2, \dev + movi a3, TIMG_WDT_WKEY_VALUE + memw + s32i a3, a2, 100 /* disable write protect */ + movi a4, _l4_intr_livelock_max + l32i a4, a4, 0 + addi a4, a4, 1 + movi a3, (CONFIG_INT_WDT_TIMEOUT_MS<<1) + quou a3, a3, a4 + memw + s32i a3, a2, 80 /* set timeout before interrupt */ + movi a3, (CONFIG_INT_WDT_TIMEOUT_MS<<2) + memw + s32i a3, a2, 84 /* set timeout before system reset */ + movi a3, 1 + s32i a3, a2, 96 /* feed wdt */ + memw + movi a3, 0 + s32i a3, a2, 100 /* enable write protect */ + memw + .endm .align 4 .handle_livelock_int: + getcoreid a5 + /* Save A2, A3, A4 so we can use those registers */ + movi a0, L4_INTR_STACK_SIZE + mull a5, a5, a0 movi a0, _l4_intr_stack + add a0, a0, a5 s32i a2, a0, L4_INTR_A2_OFFSET s32i a3, a0, L4_INTR_A3_OFFSET s32i a4, a0, L4_INTR_A4_OFFSET - rsil a0, CONFIG_ESP32_DPORT_DIS_INTERRUPT_LVL /* disable nested iterrupt */ + /* Here, we can use a0, a2, a3, a4, a5 registers */ + getcoreid a5 + beqz a5, 1f + movi a2, _l4_intr_livelock_app + l32i a3, a2, 0 + addi a3, a3, 1 + s32i a3, a2, 0 + + /* Dual core synchronization, ensuring that both cores enter interrupts */ +1: movi a4, 0x1 + movi a2, _l4_intr_livelock_sync + addx4 a3, a5, a2 + s32i a4, a3, 0 + +1: movi a2, _l4_intr_livelock_sync + movi a3, 1 + addx4 a3, a3, a2 + l32i a2, a2, 0 + l32i a3, a3, 0 + and a2, a2, a3 + beqz a2, 1b + + rsil a0, CONFIG_ESP32_DPORT_DIS_INTERRUPT_LVL /* disable nested interrupt */ + + beqz a5, 1f /* Pro cpu (Core 0) jump bypass */ + + movi a2, _l4_intr_livelock_app + l32i a2, a2, 0 + bnei a2, 2, 1f movi a2, _l4_intr_livelock_counter /* _l4_intr_livelock_counter++ */ l32i a3, a2, 0 addi a3, a3, 1 @@ -192,8 +289,10 @@ xt_highint5: When flash is DOUT/DIO read, N = 2. When flash is QOUT/QIO read, N = 4. */ - rsr.ccount a2 - movi a4, g_ticks_per_us_pro +1: rsr.ccount a2 + movi a3, g_ticks_per_us_pro + movi a4, g_ticks_per_us_app + moveqz a4, a3, a5 l32i a4, a4, 0 #if defined(CONFIG_FLASHMODE_QIO) || defined(CONFIG_FLASHMODE_QOUT) # if defined(CONFIG_ESPTOOLPY_FLASHFREQ_80M) && defined(CONFIG_SPIRAM_SPEED_80M) @@ -221,43 +320,71 @@ xt_highint5: # endif #endif mull a3, a3, a4 -1: rsr.ccount a4 /* delay_us(N) */ +2: rsr.ccount a4 /* delay_us(N) */ sub a4, a4, a2 - bltu a4, a3, 1b + bltu a4, a3, 2b - /* Feed watchdog and clear tg1 1st stage timeout interrupt */ - movi a2, TIMERG1 - movi a3, TIMG_WDT_WKEY_VALUE - memw - s32i a3, a2, 100 /* disable tg1 write protect */ - movi a3, 40 - memw - s32i a3, a2, 80 /* set timeout before interrupt */ - movi a3, 4000 - memw - s32i a3, a2, 84 /* set timeout before system reset */ - movi a3, 1 - s32i a3, a2, 96 /* feed wdt */ - memw + beqz a5, 2f + movi a2, _l4_intr_livelock_app + l32i a2, a2, 0 + beqi a2, 2, 8f + +2: movi a2, _l4_intr_livelock_sync + movi a4, 1 + addx4 a3, a4, a2 + l32i a2, a2, 0 + l32i a3, a3, 0 + and a2, a2, a3 + beqz a2, 4f + + beqz a5, 3f + /* + Pro cpu (Core 0) jump bypass, continue waiting, App cpu (Core 1) + can execute to here, unmap itself tg1 1st stage timeout interrupt + then restore registers and exit highint4. + */ + intr_matrix_map DPORT_APP_MAC_INTR_MAP_REG, ETS_TG1_WDT_LEVEL_INTR_SOURCE, 16 + j 9f +3: j 1b /* - The vector number of the interrupt watchdog is ETS_T1_WDT_INUM (24), which is a - Level-Triggered interrupt and needs to be cleared at the peripheral. + Here, App cpu (Core 1) has exited isr, Pro cpu (Core 0) help the + App cpu map tg1 1st stage timeout interrupt clear tg1 interrupt. */ - l32i a4, a2, 164 - movi a3, 4 - or a3, a4, a3 - memw - s32i a3, a2, 164 /* clear tg1 1st stage timeout interrupt */ +4: intr_matrix_map DPORT_APP_MAC_INTR_MAP_REG, ETS_TG1_WDT_LEVEL_INTR_SOURCE, ETS_T1_WDT_INUM - movi a3, 0 - s32i a3, a2, 100 /* enable tg1 write protect */ - memw +1: movi a2, _l4_intr_livelock_sync + movi a4, 1 + addx4 a3, a4, a2 + l32i a2, a2, 0 + l32i a3, a3, 0 + and a2, a2, a3 + beqz a2, 1b /* Wait for App cpu to enter highint4 again */ - wsr a0, PS /* restore iterrupt level */ + wdt_clr_intr_status TIMERG1 + j 9f + + /* Feed watchdog */ +8: wdt_feed TIMERG1 + +9: wsr a0, PS /* restore iterrupt level */ + + movi a0, 0 + beqz a5, 1f + movi a2, _l4_intr_livelock_app + l32i a3, a2, 0 + bnei a3, 2, 1f + s32i a0, a2, 0 + +1: movi a2, _l4_intr_livelock_sync + addx4 a2, a5, a2 + s32i a0, a2, 0 /* Done. Restore registers and return. */ + movi a0, L4_INTR_STACK_SIZE + mull a5, a5, a0 movi a0, _l4_intr_stack + add a0, a0, a5 l32i a2, a0, L4_INTR_A2_OFFSET l32i a3, a0, L4_INTR_A3_OFFSET l32i a4, a0, L4_INTR_A4_OFFSET diff --git a/components/esp32/int_wdt.c b/components/esp32/int_wdt.c index 34a0f6ab70..b0e6bcadf0 100644 --- a/components/esp32/int_wdt.c +++ b/components/esp32/int_wdt.c @@ -39,7 +39,7 @@ // #define WDT_INT_NUM 24 #define WDT_INT_NUM ETS_T1_WDT_INUM -#if !defined(CONFIG_FREERTOS_UNICORE) && defined(CONFIG_SPIRAM_SUPPORT) +#if CONFIG_ESP32_ECO3_CACHE_LOCK_FIX /* * This parameter is indicates the response time of tg1 watchdog to identify the * live lock, Too large values may affect BT and Wifi modules. @@ -61,7 +61,7 @@ static void IRAM_ATTR tick_hook(void) { //Only feed wdt if app cpu also ticked. if (int_wdt_app_cpu_ticked) { TIMERG1.wdt_wprotect=TIMG_WDT_WKEY_VALUE; -#if !defined(CONFIG_FREERTOS_UNICORE) && defined(CONFIG_SPIRAM_SUPPORT) +#if CONFIG_ESP32_ECO3_CACHE_LOCK_FIX _l4_intr_livelock_counter = 0; TIMERG1.wdt_config2=CONFIG_INT_WDT_TIMEOUT_MS*2/(_l4_intr_livelock_max+1); //Set timeout before interrupt #else @@ -115,13 +115,9 @@ void esp_int_wdt_cpu_init() * We found a live lock issue on ESP32 ECO3, This problem will cause the cache busy and then * the CPU to stop executing instructions. In order to solve this problem, we need to use * tg1 1st stage timeout interrupt to interrupt the cache busy state of the live lock. - * Here we only bind this interrupt to the Pro cpu (Core 0), when the tg1 1st stage timeout - * interrupt caused by the live lock occurs, only the Pro cpu (Core 0) execution path switched - * to level 4 ISR to unlock the cache busy status and resume system. */ - if (xPortGetCoreID() == PRO_CPU_NUM) { - intr_matrix_set(xPortGetCoreID(), ETS_TG1_WDT_LEVEL_INTR_SOURCE, WDT_INT_NUM); -#if !defined(CONFIG_FREERTOS_UNICORE) && defined(CONFIG_SPIRAM_SUPPORT) + intr_matrix_set(xPortGetCoreID(), ETS_TG1_WDT_LEVEL_INTR_SOURCE, WDT_INT_NUM); +#if CONFIG_ESP32_ECO3_CACHE_LOCK_FIX _l4_intr_livelock_max = 0; if (soc_has_cache_lock_bug()) { assert(((1000/CONFIG_FREERTOS_HZ)<<1) <= TG1_WDT_LIVELOCK_TIMEOUT_MS); @@ -129,7 +125,6 @@ void esp_int_wdt_cpu_init() _l4_intr_livelock_max = CONFIG_INT_WDT_TIMEOUT_MS/TG1_WDT_LIVELOCK_TIMEOUT_MS - 1; } #endif - } //We do not register a handler for the interrupt because it is interrupt level 4 which //is not servicable from C. Instead, xtensa_vectors.S has a call to the panic handler for //this interrupt. diff --git a/components/esp32/panic.c b/components/esp32/panic.c index ca89b5858a..a3dd0c532a 100644 --- a/components/esp32/panic.c +++ b/components/esp32/panic.c @@ -240,15 +240,6 @@ void panicHandler(XtExcFrame *frame) } #if !CONFIG_FREERTOS_UNICORE - /* - * When the real Interrupt watchdog occurs (_l4_intr_livelock_counter >= _l4_intr_livelock_max), - * do not clear the wdt interrupt, help the App cpu (Core 1) map tg1 1st stage timeout - * interrupt, trigger the App cpu (Core 1) to respond to the wdt interrupt. - */ - if (core_id == PRO_CPU_NUM) { - intr_matrix_set(APP_CPU_NUM, ETS_TG1_WDT_LEVEL_INTR_SOURCE, ETS_T1_WDT_INUM); - } - //Save frame for other core. if ((frame->exccause == PANIC_RSN_INTWDT_CPU0 && core_id == 1) || (frame->exccause == PANIC_RSN_INTWDT_CPU1 && core_id == 0)) { other_core_frame = frame;