From eb5752c635b1d42a322093dc3717c38b4581d664 Mon Sep 17 00:00:00 2001
From: Ivan Grokhotkov <ivan@espressif.com>
Date: Thu, 26 Oct 2017 19:11:47 +0800
Subject: [PATCH] esp_restart: fix possible race while stalling other CPU,
 enable WDT early

Previously esp_restart would stall the other CPU before enabling RTC_WDT.
If the other CPU was executing an s32c1i instruction, the lock signal
from CPU to the arbiter would still be held after CPU was stalled. If
the CPU running esp_restart would then try to access the same locked
memory pool, it would be stuck, because lock signal would never be
released.

With this change, esp_restart resets the other CPU before stalling it.
Ideally, we would want to reset the CPU and keep it in reset, but the
hardware doesn't have such feature for PRO_CPU (it is possible to hold
APP_CPU in reset using DPORT register). Given that ROM code will not use
s32c1i in the first few hundred cycles, doing reset and then stall seems
to be safe.

In addition to than, RTC_WDT initialization is moved to the beginning of
the function, to prevent possible lock-up if CPU stalling still has any
issue.
---
 components/esp32/system_api.c          | 42 ++++++++++++++------------
 components/soc/esp32/cpu_util.c        |  6 ++++
 components/soc/esp32/include/soc/cpu.h |  7 +++++
 3 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/components/esp32/system_api.c b/components/esp32/system_api.c
index 37958db40..851d4a450 100644
--- a/components/esp32/system_api.c
+++ b/components/esp32/system_api.c
@@ -266,15 +266,10 @@ void IRAM_ATTR esp_restart(void)
 */
 void IRAM_ATTR esp_restart_noos()
 {
-    const uint32_t core_id = xPortGetCoreID();
-    const uint32_t other_core_id = core_id == 0 ? 1 : 0;
-    esp_cpu_stall(other_core_id);
+    // Disable interrupts
+    xt_ints_off(0xFFFFFFFF);
 
-    // other core is now stalled, can access DPORT registers directly
-    esp_dport_access_int_pause();
-
-    // We need to disable TG0/TG1 watchdogs
-    // First enable RTC watchdog for 1 second
+    // Enable RTC watchdog for 1 second
     REG_WRITE(RTC_CNTL_WDTWPROTECT_REG, RTC_CNTL_WDT_WKEY_VALUE);
     REG_WRITE(RTC_CNTL_WDTCONFIG0_REG,
             RTC_CNTL_WDT_FLASHBOOT_MOD_EN_M |
@@ -284,6 +279,18 @@ void IRAM_ATTR esp_restart_noos()
             (1 << RTC_CNTL_WDT_CPU_RESET_LENGTH_S) );
     REG_WRITE(RTC_CNTL_WDTCONFIG1_REG, rtc_clk_slow_freq_get_hz() * 1);
 
+    // Reset and stall the other CPU.
+    // CPU must be reset before stalling, in case it was running a s32c1i
+    // instruction. This would cause memory pool to be locked by arbiter
+    // to the stalled CPU, preventing current CPU from accessing this pool.
+    const uint32_t core_id = xPortGetCoreID();
+    const uint32_t other_core_id = (core_id == 0) ? 1 : 0;
+    esp_cpu_reset(other_core_id);
+    esp_cpu_stall(other_core_id);
+
+    // Other core is now stalled, can access DPORT registers directly
+    esp_dport_access_int_abort();
+
     // Disable TG0/TG1 watchdogs
     TIMERG0.wdt_wprotect=TIMG_WDT_WKEY_VALUE;
     TIMERG0.wdt_config0.en = 0;
@@ -292,8 +299,10 @@ void IRAM_ATTR esp_restart_noos()
     TIMERG1.wdt_config0.en = 0;
     TIMERG1.wdt_wprotect=0;
 
-    // Disable all interrupts
-    xt_ints_off(0xFFFFFFFF);
+    // Flush any data left in UART FIFOs
+    uart_tx_wait_idle(0);
+    uart_tx_wait_idle(1);
+    uart_tx_wait_idle(2);
 
     // Disable cache
     Cache_Read_Disable(0);
@@ -310,11 +319,6 @@ void IRAM_ATTR esp_restart_noos()
     WRITE_PERI_REG(GPIO_FUNC5_IN_SEL_CFG_REG, 0x30);
 #endif
 
-    // Flush any data left in UART FIFOs
-    uart_tx_wait_idle(0);
-    uart_tx_wait_idle(1);
-    uart_tx_wait_idle(2);
-
     // Reset wifi/bluetooth/ethernet/sdio (bb/mac)
     DPORT_SET_PERI_REG_MASK(DPORT_CORE_RST_EN_REG, 
          DPORT_BB_RST | DPORT_FE_RST | DPORT_MAC_RST |
@@ -337,14 +341,14 @@ void IRAM_ATTR esp_restart_noos()
     // Reset CPUs
     if (core_id == 0) {
         // Running on PRO CPU: APP CPU is stalled. Can reset both CPUs.
-        SET_PERI_REG_MASK(RTC_CNTL_OPTIONS0_REG,
-                RTC_CNTL_SW_PROCPU_RST_M | RTC_CNTL_SW_APPCPU_RST_M);
+        esp_cpu_reset(1);
+        esp_cpu_reset(0);
     } else {
         // Running on APP CPU: need to reset PRO CPU and unstall it,
         // then reset APP CPU
-        SET_PERI_REG_MASK(RTC_CNTL_OPTIONS0_REG, RTC_CNTL_SW_PROCPU_RST_M);
+        esp_cpu_reset(0);
         esp_cpu_unstall(0);
-        SET_PERI_REG_MASK(RTC_CNTL_OPTIONS0_REG, RTC_CNTL_SW_APPCPU_RST_M);
+        esp_cpu_reset(1);
     }
     while(true) {
         ;
diff --git a/components/soc/esp32/cpu_util.c b/components/soc/esp32/cpu_util.c
index ecfcab4ba..bc052af98 100644
--- a/components/soc/esp32/cpu_util.c
+++ b/components/soc/esp32/cpu_util.c
@@ -44,6 +44,12 @@ void IRAM_ATTR esp_cpu_unstall(int cpu_id)
     }
 }
 
+void IRAM_ATTR esp_cpu_reset(int cpu_id)
+{
+    SET_PERI_REG_MASK(RTC_CNTL_OPTIONS0_REG,
+            cpu_id == 0 ? RTC_CNTL_SW_PROCPU_RST_M : RTC_CNTL_SW_APPCPU_RST_M);
+}
+
 bool IRAM_ATTR esp_cpu_in_ocd_debug_mode()
 {
 #if CONFIG_ESP32_DEBUG_OCDAWARE
diff --git a/components/soc/esp32/include/soc/cpu.h b/components/soc/esp32/include/soc/cpu.h
index b56fb3dc8..05ec91776 100644
--- a/components/soc/esp32/include/soc/cpu.h
+++ b/components/soc/esp32/include/soc/cpu.h
@@ -85,6 +85,13 @@ void esp_cpu_stall(int cpu_id);
  */
 void esp_cpu_unstall(int cpu_id);
 
+/**
+ * @brief Reset CPU using RTC controller
+ * @param cpu_id ID of the CPU to reset (0 = PRO, 1 = APP)
+ */
+void esp_cpu_reset(int cpu_id);
+
+
 /**
  * @brief Returns true if a JTAG debugger is attached to CPU
  * OCD (on chip debug) port.