Merge branch 'bugfix/panic_fixes' into 'master'

panic handling fixes

See merge request espressif/esp-idf!8597

components/esp32s2/cache_err_int.c

@@ -45,14 +45,8 @@ void esp_cache_err_int_init(void)
     intr_matrix_set(core_id, ETS_CACHE_IA_INTR_SOURCE, ETS_MEMACCESS_ERR_INUM);
 
     // Enable invalid cache access interrupt when the cache is disabled.
-    // When the interrupt happens, we can not determine the CPU where the
+    // The status bits are cleared first, in case we are restarting after
-    // invalid cache access has occurred. We enable the interrupt to catch
+    // a cache error has triggered.
-    // invalid access on both CPUs, but the interrupt is connected to the
-    // CPU which happens to call this function.
-    // For this reason, panic handler backtrace will not be correct if the
-    // interrupt is connected to PRO CPU and invalid access happens on the APP
-    // CPU.
-
     DPORT_SET_PERI_REG_MASK(EXTMEM_CACHE_DBG_INT_CLR_REG,
                             EXTMEM_MMU_ENTRY_FAULT_INT_CLR |
                             EXTMEM_DCACHE_REJECT_INT_CLR |
@@ -78,7 +72,9 @@ void esp_cache_err_int_init(void)
 
 int IRAM_ATTR esp_cache_err_get_cpuid(void)
 {
-    // TODO: The description for this seem to indicate that when cache is not in error
+    if (REG_READ(EXTMEM_CACHE_DBG_STATUS0_REG) != 0 ||
-    // state, return -1.
+        REG_READ(EXTMEM_CACHE_DBG_STATUS1_REG) != 0) {
-    return PRO_CPU_NUM;
+            return PRO_CPU_NUM;
+    }
+    return -1;
 }

components/esp_common/include/esp_private/system_internal.h

@@ -36,6 +36,11 @@ extern "C" {
  */
 void esp_restart_noos(void) __attribute__ ((noreturn));
 
+/**
+ * @brief Similar to esp_restart_noos, but resets all the digital peripherals.
+ */
+void esp_restart_noos_dig(void) __attribute__ ((noreturn));
+
 /**
  * @brief  Internal function to set reset reason hint
  *

components/esp_system/panic.c

@@ -340,4 +340,17 @@ void __attribute__((noreturn)) panic_abort(const char *details)
 
     *((int *) 0) = 0; // NOLINT(clang-analyzer-core.NullDereference) should be an invalid operation on targets
     while(1);
-}
+}
+
+/* Weak versions of reset reason hint functions.
+ * If these weren't provided, reset reason code would be linked into the app
+ * even if the app never called esp_reset_reason().
+ */
+void IRAM_ATTR __attribute__((weak)) esp_reset_reason_set_hint(esp_reset_reason_t hint)
+{
+}
+
+esp_reset_reason_t IRAM_ATTR  __attribute__((weak)) esp_reset_reason_get_hint(void)
+{
+    return ESP_RST_UNKNOWN;
+}

components/esp_system/port/panic_handler.c

@@ -486,7 +486,13 @@ static void panic_handler(XtExcFrame *frame, bool pseudo_excause)
         BUSY_WAIT_IF_TRUE(frame->exccause == PANIC_RSN_INTWDT_CPU1 && core_id == 0);
 
         // For cache error, pause the non-offending core - offending core handles panic
-        BUSY_WAIT_IF_TRUE(frame->exccause == PANIC_RSN_CACHEERR && core_id != esp_cache_err_get_cpuid());
+        if (frame->exccause == PANIC_RSN_CACHEERR && core_id != esp_cache_err_get_cpuid()) {
+            // Only print the backtrace for the offending core in case of the cache error
+            xt_exc_frames[core_id] = NULL;
+            while (1) {
+                ;
+            }
+        }
     }
 
     ets_delay_us(1);
@@ -536,42 +542,22 @@ void xt_unhandled_exception(XtExcFrame *frame)
     panic_handler(frame, false);
 }
 
-static __attribute__((noreturn)) void esp_digital_reset(void)
-{
-    // make sure all the panic handler output is sent from UART FIFO
-    uart_tx_wait_idle(CONFIG_ESP_CONSOLE_UART_NUM);
-    // switch to XTAL (otherwise we will keep running from the PLL)
-
-    rtc_clk_cpu_freq_set_xtal();
-
-#if CONFIG_IDF_TARGET_ESP32
-    esp_cpu_unstall(PRO_CPU_NUM);
-#endif
-
-    // reset the digital part
-    SET_PERI_REG_MASK(RTC_CNTL_OPTIONS0_REG, RTC_CNTL_SW_SYS_RST);
-    while (true) {
-        ;
-    }
-}
-
 void __attribute__((noreturn)) panic_restart(void)
 {
-    // If resetting because of a cache error, reset the digital part
-    // Make sure that the reset reason is not a generic panic reason as well on ESP32S2,
-    // as esp_cache_err_get_cpuid always returns PRO_CPU_NUM
-
     bool digital_reset_needed = false;
-    if ( esp_cache_err_get_cpuid() != -1 && esp_reset_reason_get_hint() != ESP_RST_PANIC ) {
+#ifdef CONFIG_IDF_TARGET_ESP32
-        digital_reset_needed = true;
+    // On the ESP32, cache error status can only be cleared by system reset
-    }
+    if (esp_cache_err_get_cpuid() != -1) {
-#if CONFIG_IDF_TARGET_ESP32S2
-    if ( esp_memprot_is_intr_ena_any() || esp_memprot_is_locked_any() ) {
         digital_reset_needed = true;
     }
 #endif
-    if ( digital_reset_needed ) {
+#if CONFIG_IDF_TARGET_ESP32S2
-        esp_digital_reset();
+    if (esp_memprot_is_intr_ena_any() || esp_memprot_is_locked_any()) {
+        digital_reset_needed = true;
+    }
+#endif
+    if (digital_reset_needed) {
+        esp_restart_noos_dig();
     }
     esp_restart_noos();
 }

components/esp_system/system_api.c

@@ -3,16 +3,16 @@
 #include "esp_heap_caps.h"
 #include "freertos/FreeRTOS.h"
 #include "freertos/task.h"
-
-#if CONFIG_IDF_TARGET_ESP32S2
 #include "soc/rtc.h"
 #include "soc/rtc_cntl_reg.h"
+#include "panic_internal.h"
+#if CONFIG_IDF_TARGET_ESP32
+#include "esp32/rom/uart.h"
+#elif CONFIG_IDF_TARGET_ESP32S2
 #include "esp32s2/rom/uart.h"
 #include "esp32s2/memprot.h"
 #endif
 
-#include "esp_system.h"
-#include "panic_internal.h"
 
 #define SHUTDOWN_HANDLERS_NO 2
 static shutdown_handler_t shutdown_handlers[SHUTDOWN_HANDLERS_NO];
@@ -41,22 +41,25 @@ esp_err_t esp_unregister_shutdown_handler(shutdown_handler_t handler)
     return ESP_ERR_INVALID_STATE;
 }
 
-#if CONFIG_IDF_TARGET_ESP32S2
+void IRAM_ATTR esp_restart_noos_dig(void)
-static __attribute__((noreturn)) void esp_digital_reset(void)
 {
     // make sure all the panic handler output is sent from UART FIFO
-    uart_tx_wait_idle(CONFIG_ESP_CONSOLE_UART_NUM);
+    if (CONFIG_ESP_CONSOLE_UART_NUM >= 0) {
-    // switch to XTAL (otherwise we will keep running from the PLL)
+        uart_tx_wait_idle(CONFIG_ESP_CONSOLE_UART_NUM);
+    }
 
+    // switch to XTAL (otherwise we will keep running from the PLL)
     rtc_clk_cpu_freq_set_xtal();
 
+#if CONFIG_IDF_TARGET_ESP32
+    esp_cpu_unstall(PRO_CPU_NUM);
+#endif
     // reset the digital part
     SET_PERI_REG_MASK(RTC_CNTL_OPTIONS0_REG, RTC_CNTL_SW_SYS_RST);
     while (true) {
         ;
     }
 }
-#endif
 
 void IRAM_ATTR esp_restart(void)
 {
@@ -70,8 +73,8 @@ void IRAM_ATTR esp_restart(void)
     vTaskSuspendAll();
 
 #if CONFIG_IDF_TARGET_ESP32S2
-    if ( esp_memprot_is_intr_ena_any() || esp_memprot_is_locked_any()) {
+    if (esp_memprot_is_intr_ena_any() || esp_memprot_is_locked_any()) {
-        esp_digital_reset();
+        esp_restart_noos_dig();
     }
 #endif
     esp_restart_noos();
@@ -95,4 +98,4 @@ const char *esp_get_idf_version(void)
 void __attribute__((noreturn)) esp_system_abort(const char *details)
 {
     panic_abort(details);
-}
+}