Merge branch 'fix/dualcore_branch_predictor_cache_race_v5.4' into 'release/v5.4'

fix(spi_flash): disable branch prediction on the parked core during flash ops (v5.4) See merge request espressif/esp-idf!52199
2026-09-22 13:01:16 +03:00 · 2026-09-01 18:22:06 +08:00
parent 21f8271c12 a76e14afe7
commit 7080d6ce36
7 changed files with 98 additions and 26 deletions
--- a/components/esp_hw_support/mspi_timing_tuning.c
+++ b/components/esp_hw_support/mspi_timing_tuning.c
@@ -16,6 +16,7 @@
 #include "hal/spi_flash_hal.h"
 #include "esp_private/esp_cache_private.h"
 #include "esp_private/cache_utils.h"
+#include "esp_cpu.h"
 #include "esp_private/mspi_timing_tuning.h"
 #include "esp_private/mspi_timing_config.h"
 #include "mspi_timing_by_mspi_delay.h"
@@ -589,20 +590,36 @@ void mspi_timing_change_speed_mode_cache_safe(bool switch_down)
 /*------------------------------------------------------------------------------
 * Early-init MSPI speed switch (see mspi_timing_tuning.h)
 *----------------------------------------------------------------------------*/
+static void disable_cache(uint32_t cpuid, uint32_t *saved_state)
+{
+#if SOC_BRANCH_PREDICTOR_SUPPORTED
+    esp_cpu_branch_prediction_disable();
+#endif
+    spi_flash_disable_cache(cpuid, saved_state);
+}
+
+static void restore_cache(uint32_t cpuid, uint32_t saved_state)
+{
+    spi_flash_restore_cache(cpuid, saved_state);
+#if SOC_BRANCH_PREDICTOR_SUPPORTED
+    esp_cpu_branch_prediction_enable();
+#endif
+}
+
 void mspi_timing_enter_low_speed_early(void)
 {
    uint32_t cache_state = 0;
-    spi_flash_disable_cache(0, &cache_state);
+    disable_cache(0, &cache_state);
    mspi_timing_enter_low_speed_mode(true);
-    spi_flash_restore_cache(0, cache_state);
+    restore_cache(0, cache_state);
 }

 void mspi_timing_enter_high_speed_early(void)
 {
    uint32_t cache_state = 0;
-    spi_flash_disable_cache(0, &cache_state);
+    disable_cache(0, &cache_state);
    mspi_timing_enter_high_speed_mode(true);
-    spi_flash_restore_cache(0, cache_state);
+    restore_cache(0, cache_state);
 }

 /*------------------------------------------------------------------------------
--- a/components/esp_hw_support/sleep_modes.c
+++ b/components/esp_hw_support/sleep_modes.c
@@ -11,6 +11,7 @@
 #include <inttypes.h>

 #include "esp_attr.h"
+#include "esp_cpu.h"
 #include "esp_rom_caps.h"
 #include "esp_macros.h"
 #include "esp_memory_utils.h"
@@ -510,6 +511,9 @@ static uint32_t s_cache_state = 0;
 static void IRAM_ATTR suspend_cache(void) {
    s_cache_suspend_cnt++;
    if (s_cache_suspend_cnt == 1) {
+#if SOC_BRANCH_PREDICTOR_SUPPORTED
+        esp_cpu_branch_prediction_disable();
+#endif
        spi_flash_disable_cache(esp_cpu_get_core_id(), &s_cache_state);
    }
 }
@@ -520,6 +524,9 @@ static void IRAM_ATTR resume_cache(void) {
    assert(s_cache_suspend_cnt >= 0 && DRAM_STR("cache resume doesn't match suspend ops"));
    if (s_cache_suspend_cnt == 0) {
        spi_flash_restore_cache(esp_cpu_get_core_id(), s_cache_state);
+#if SOC_BRANCH_PREDICTOR_SUPPORTED
+        esp_cpu_branch_prediction_enable();
+#endif
    }
 }

--- a/components/esp_rom/patches/esp_rom_spiflash.c
+++ b/components/esp_rom/patches/esp_rom_spiflash.c
@@ -19,10 +19,6 @@
 #include "esp32p4/rom/opi_flash.h"
 #endif

-#if SOC_BRANCH_PREDICTOR_SUPPORTED
-#include "riscv/rv_utils.h"
-#endif
-
 #define SPI_IDX   1

 #if CONFIG_IDF_TARGET_ESP32
@@ -774,10 +770,6 @@ void esp_rom_opiflash_cache_mode_config(esp_rom_spiflash_read_mode_t mode, const
 extern void rom_spi_flash_disable_cache(uint32_t cpuid, uint32_t *saved_state);
 void spi_flash_disable_cache(uint32_t cpuid, uint32_t *saved_state)
 {
-#if SOC_BRANCH_PREDICTOR_SUPPORTED
-    //branch predictor will start cache request as well
-    rv_utils_dis_branch_predictor();
-#endif
    rom_spi_flash_disable_cache(cpuid, saved_state);
 }

@@ -785,8 +777,5 @@ extern void rom_spi_flash_restore_cache(uint32_t cpuid, uint32_t saved_state);
 void spi_flash_restore_cache(uint32_t cpuid, uint32_t saved_state)
 {
    rom_spi_flash_restore_cache(cpuid, saved_state);
-#if SOC_BRANCH_PREDICTOR_SUPPORTED
-    rv_utils_en_branch_predictor();
-#endif
 }
 #endif
--- a/components/esp_system/port/arch/riscv/esp_ipc_isr_routines.c
+++ b/components/esp_system/port/arch/riscv/esp_ipc_isr_routines.c
@@ -5,9 +5,21 @@
 */

 #include "stdint.h"
+#include "soc/soc_caps.h"
 #include "esp_attr.h"
+#include "esp_cpu.h"

 void IRAM_ATTR esp_ipc_isr_waiting_for_finish_cmd(void* ipc_isr_finish_cmd)
 {
+#if SOC_BRANCH_PREDICTOR_SUPPORTED
+    /* The branch predictor keeps issuing speculative instruction fetches while
+     * this core spins here. The other core may suspend the external memory
+     * cache during the stall, in which case a speculative fetch into cached
+     * address space raises a cache access-fail interrupt. */
+    esp_cpu_branch_prediction_disable();
+#endif
    while (*(volatile uint32_t *)ipc_isr_finish_cmd == 0) { };
+#if SOC_BRANCH_PREDICTOR_SUPPORTED
+    esp_cpu_branch_prediction_enable();
+#endif
 }
--- a/components/esp_system/port/panic_handler.c
+++ b/components/esp_system/port/panic_handler.c
@@ -130,6 +130,15 @@ static void frame_to_panic_info(void *frame, panic_info_t *info, bool pseudo_exc
 FORCE_INLINE_ATTR __attribute__((__noreturn__))
 void busy_wait(void)
 {
+#if SOC_BRANCH_PREDICTOR_SUPPORTED
+    /* This core parks here while the offending core handles the panic, which
+     * may include flash accesses with the cache suspended (e.g. writing a core
+     * dump). Stop the branch predictor so its speculative fetches cannot latch
+     * spurious cache access-fail errors that would corrupt the cache error
+     * status of the panic being reported. This core never resumes, so the
+     * predictor is not re-enabled. */
+    esp_cpu_branch_prediction_disable();
+#endif
    ESP_INFINITE_LOOP();
 }
 #endif // !CONFIG_ESP_SYSTEM_SINGLE_CORE_MODE
--- a/components/spi_flash/cache_utils.c
+++ b/components/spi_flash/cache_utils.c
@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD
 *
 * SPDX-License-Identifier: Apache-2.0
 */
@@ -55,6 +55,7 @@
 #include "esp_private/esp_ipc.h"
 #endif
 #include "esp_attr.h"
+#include "esp_cpu.h"
 #include "esp_memory_utils.h"
 #include "esp_intr_alloc.h"
 #include "spi_flash_override.h"
@@ -126,6 +127,13 @@ void IRAM_ATTR spi_flash_op_block_func(void *arg)
    // Restore interrupts that aren't located in IRAM
    esp_intr_noniram_disable();
    uint32_t cpuid = (uint32_t) arg;
+#if SOC_BRANCH_PREDICTOR_SUPPORTED
+    /* The branch predictor issues speculative cache requests while this core
+     * spins in IRAM. The flash-op core is about to suspend the (shared) cache,
+     * so speculative fetches into flash would raise a cache access-fail on
+     * this core. */
+    esp_cpu_branch_prediction_disable();
+#endif
    // s_flash_op_complete flag is cleared on *this* CPU, otherwise the other
    // CPU may reset the flag back to false before IPC task has a chance to check it
    // (if it is preempted by an ISR taking non-trivial amount of time)
@@ -136,6 +144,9 @@ void IRAM_ATTR spi_flash_op_block_func(void *arg)
    }
    // Flash operation is complete, re-enable cache
    spi_flash_restore_cache(cpuid, s_flash_op_cache_state[cpuid]);
+#if SOC_BRANCH_PREDICTOR_SUPPORTED
+    esp_cpu_branch_prediction_enable();
+#endif
    // Restore interrupts that aren't located in IRAM
    esp_intr_noniram_enable();
 #if ( ( CONFIG_FREERTOS_SMP ) && ( !CONFIG_FREERTOS_UNICORE ) )
@@ -206,6 +217,9 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu(void)

    // Kill interrupts that aren't located in IRAM
    esp_intr_noniram_disable();
+#if SOC_BRANCH_PREDICTOR_SUPPORTED
+    esp_cpu_branch_prediction_disable();
+#endif
    // This CPU executes this routine, with non-IRAM interrupts and the scheduler
    // disabled. The other CPU is spinning in the spi_flash_op_block_func task, also
    // with non-iram interrupts and the scheduler disabled. None of these CPUs will
@@ -242,6 +256,9 @@ void IRAM_ATTR spi_flash_enable_interrupts_caches_and_other_cpu(void)
        s_flash_op_complete = true;
    }

+#if SOC_BRANCH_PREDICTOR_SUPPORTED
+    esp_cpu_branch_prediction_enable();
+#endif
    // Re-enable non-iram interrupts
    esp_intr_noniram_enable();

@@ -268,6 +285,12 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu_no_os(void)
    const uint32_t cpuid = xPortGetCoreID();
    const uint32_t other_cpuid = (cpuid == 0) ? 1 : 0;

+#if SOC_BRANCH_PREDICTOR_SUPPORTED
+    /* Disable BP before the first disable_cache(): on shared-cache chips that
+     * call suspends external memory for all cores, so speculative fetches must
+     * already be stopped. */
+    esp_cpu_branch_prediction_disable();
+#endif
    // do not care about other CPU, it was halted upon entering panic handler
    spi_flash_disable_cache(other_cpuid, &s_flash_op_cache_state[other_cpuid]);
    // Kill interrupts that aren't located in IRAM
@@ -282,6 +305,9 @@ void IRAM_ATTR spi_flash_enable_interrupts_caches_no_os(void)

    // Re-enable cache on this CPU
    spi_flash_restore_cache(cpuid, s_flash_op_cache_state[cpuid]);
+#if SOC_BRANCH_PREDICTOR_SUPPORTED
+    esp_cpu_branch_prediction_enable();
+#endif
    // Re-enable non-iram interrupts
    esp_intr_noniram_enable();
 }
@@ -321,12 +347,18 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu(void)
 {
    spi_flash_op_lock();
    esp_intr_noniram_disable();
+#if SOC_BRANCH_PREDICTOR_SUPPORTED
+    esp_cpu_branch_prediction_disable();
+#endif
    spi_flash_disable_cache(0, &s_flash_op_cache_state[0]);
 }

 void IRAM_ATTR spi_flash_enable_interrupts_caches_and_other_cpu(void)
 {
    spi_flash_restore_cache(0, s_flash_op_cache_state[0]);
+#if SOC_BRANCH_PREDICTOR_SUPPORTED
+    esp_cpu_branch_prediction_enable();
+#endif
    esp_intr_noniram_enable();
    spi_flash_op_unlock();
 }
@@ -335,6 +367,9 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu_no_os(void)
 {
    // Kill interrupts that aren't located in IRAM
    esp_intr_noniram_disable();
+#if SOC_BRANCH_PREDICTOR_SUPPORTED
+    esp_cpu_branch_prediction_disable();
+#endif
    // Disable cache on this CPU as well
    spi_flash_disable_cache(0, &s_flash_op_cache_state[0]);
 }
@@ -343,6 +378,9 @@ void IRAM_ATTR spi_flash_enable_interrupts_caches_no_os(void)
 {
    // Re-enable cache on this CPU
    spi_flash_restore_cache(0, s_flash_op_cache_state[0]);
+#if SOC_BRANCH_PREDICTOR_SUPPORTED
+    esp_cpu_branch_prediction_enable();
+#endif
    // Re-enable non-iram interrupts
    esp_intr_noniram_enable();
 }
@@ -365,19 +403,12 @@ void IRAM_ATTR spi_flash_enable_cache(uint32_t cpuid)
 #if !CONFIG_SPI_FLASH_ROM_IMPL
 void IRAM_ATTR spi_flash_disable_cache(uint32_t cpuid, uint32_t *saved_state)
 {
-#if SOC_BRANCH_PREDICTOR_SUPPORTED
-    //branch predictor will start cache request as well
-    esp_cpu_branch_prediction_disable();
-#endif
    esp_cache_suspend_ext_mem_cache();
 }

 void IRAM_ATTR spi_flash_restore_cache(uint32_t cpuid, uint32_t saved_state)
 {
    esp_cache_resume_ext_mem_cache();
-#if SOC_BRANCH_PREDICTOR_SUPPORTED
-    esp_cpu_branch_prediction_enable();
-#endif
 }

 bool IRAM_ATTR spi_flash_cache_enabled(void)
--- a/components/spi_flash/include/esp_private/cache_utils.h
+++ b/components/spi_flash/include/esp_private/cache_utils.h
@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD
 *
 * SPDX-License-Identifier: Apache-2.0
 */
@@ -97,7 +97,11 @@ bool spi_flash_cache_enabled(void);
 void spi_flash_enable_cache(uint32_t cpuid);

 /**
- * @brief Suspend the Cache access to external memory, will disable branch predictor if supported.
+ * @brief Suspend the Cache access to external memory.
+ *
+ * @note Callers must disable branch prediction around this window when
+ *       SOC_BRANCH_PREDICTOR_SUPPORTED, otherwise speculative fetches can
+ *       raise cache access-fail errors while the cache is suspended.
 *
 * @param cpuid       the core number to enable the cache for, meaning less on shared cache.
 * @param saved_state Cache status hold by hal (Used only on ROM impl. in idf, this param unused)
@@ -105,7 +109,10 @@ void spi_flash_enable_cache(uint32_t cpuid);
 void spi_flash_disable_cache(uint32_t cpuid, uint32_t *saved_state);

 /**
- * @brief Resume the Cache access to external memory, will enable branch predictor if supported.
+ * @brief Resume the Cache access to external memory.
+ *
+ * @note Callers that disabled branch prediction for the suspend window must
+ *       re-enable it after this call when SOC_BRANCH_PREDICTOR_SUPPORTED.
 *
 * @param cpuid       the core number to enable the cache for, meaning less on shared cache.
 * @param saved_state Cache status hold by hal (Used only on ROM impl. in idf, this param unused)