Merge branch 'fix/dualcore_branch_predictor_cache_race_v6.0' into 'release/v6.0'

fix(spi_flash): disable branch prediction on the parked core during flash ops (v6.0)

See merge request espressif/esp-idf!52202
This commit is contained in:
morris
2026-09-04 17:57:09 +08:00
7 changed files with 94 additions and 23 deletions
@@ -22,6 +22,7 @@
#include "hal/cache_hal.h" #include "hal/cache_hal.h"
#endif #endif
#include "esp_private/cache_utils.h" #include "esp_private/cache_utils.h"
#include "esp_cpu.h"
#include "esp_private/mspi_timing_tuning.h" #include "esp_private/mspi_timing_tuning.h"
#include "esp_private/mspi_timing_config.h" #include "esp_private/mspi_timing_config.h"
#include "esp_private/mspi_timing_by_mspi_delay.h" #include "esp_private/mspi_timing_by_mspi_delay.h"
@@ -633,8 +634,21 @@ static void restore_cache(uint32_t cpuid, uint32_t saved_state)
} }
#else // ESP_TEE_BUILD #else // ESP_TEE_BUILD
#define disable_cache(cpuid, saved_state) spi_flash_disable_cache(cpuid, saved_state) static void disable_cache(uint32_t cpuid, uint32_t *saved_state)
#define restore_cache(cpuid, saved_state) spi_flash_restore_cache(cpuid, 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
}
#endif #endif
+7
View File
@@ -11,6 +11,7 @@
#include <inttypes.h> #include <inttypes.h>
#include "esp_attr.h" #include "esp_attr.h"
#include "esp_cpu.h"
#include "esp_rom_caps.h" #include "esp_rom_caps.h"
#include "esp_macros.h" #include "esp_macros.h"
#include "esp_memory_utils.h" #include "esp_memory_utils.h"
@@ -573,6 +574,9 @@ static void FORCE_IRAM_ATTR suspend_cache(void) {
// If the access to external memory hits in the cache, it will not trigger a cache error. So in order to // If the access to external memory hits in the cache, it will not trigger a cache error. So in order to
// fully check the access to external memory, writeback & invalidate is needed here. // fully check the access to external memory, writeback & invalidate is needed here.
Cache_WriteBack_Invalidate_All(CACHE_MAP_MASK); Cache_WriteBack_Invalidate_All(CACHE_MAP_MASK);
#endif
#if SOC_BRANCH_PREDICTOR_SUPPORTED
esp_cpu_branch_prediction_disable();
#endif #endif
spi_flash_disable_cache(esp_cpu_get_core_id(), &s_cache_state); spi_flash_disable_cache(esp_cpu_get_core_id(), &s_cache_state);
} }
@@ -584,6 +588,9 @@ static void FORCE_IRAM_ATTR resume_cache(void) {
assert(s_cache_suspend_cnt >= 0 && DRAM_STR("cache resume doesn't match suspend ops")); assert(s_cache_suspend_cnt >= 0 && DRAM_STR("cache resume doesn't match suspend ops"));
if (s_cache_suspend_cnt == 0) { if (s_cache_suspend_cnt == 0) {
spi_flash_restore_cache(esp_cpu_get_core_id(), s_cache_state); spi_flash_restore_cache(esp_cpu_get_core_id(), s_cache_state);
#if SOC_BRANCH_PREDICTOR_SUPPORTED
esp_cpu_branch_prediction_enable();
#endif
} }
} }
@@ -13,9 +13,6 @@
#include "soc/spi_mem_reg.h" #include "soc/spi_mem_reg.h"
#endif #endif
#if SOC_BRANCH_PREDICTOR_SUPPORTED
#include "riscv/rv_utils.h"
#endif
#include "esp_rom_spiflash.h" #include "esp_rom_spiflash.h"
#if CONFIG_IDF_TARGET_ESP32 #if CONFIG_IDF_TARGET_ESP32
#include "esp32/rom/spi_flash.h" #include "esp32/rom/spi_flash.h"
@@ -817,10 +814,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); 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) 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); rom_spi_flash_disable_cache(cpuid, saved_state);
} }
@@ -828,8 +821,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) void spi_flash_restore_cache(uint32_t cpuid, uint32_t saved_state)
{ {
rom_spi_flash_restore_cache(cpuid, saved_state); rom_spi_flash_restore_cache(cpuid, saved_state);
#if SOC_BRANCH_PREDICTOR_SUPPORTED
rv_utils_en_branch_predictor();
#endif
} }
#endif #endif
@@ -7,6 +7,7 @@
#include "stdint.h" #include "stdint.h"
#include "soc/interrupt_reg.h" #include "soc/interrupt_reg.h"
#include "soc/soc_caps.h" #include "soc/soc_caps.h"
#include "esp_cpu.h"
#include "esp_ipc_isr.h" #include "esp_ipc_isr.h"
#include "esp_private/esp_ipc_isr.h" #include "esp_private/esp_ipc_isr.h"
#include "esp_private/esp_system_attr.h" #include "esp_private/esp_system_attr.h"
@@ -46,6 +47,15 @@ void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_record_interrupted_context(void)
void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_waiting_for_finish_cmd(void* arg) void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_waiting_for_finish_cmd(void* arg)
{ {
#if SOC_BRANCH_PREDICTOR_SUPPORTED
/* The branch predictor keeps issuing speculative instruction fetches while
* this core spins here. Callers of esp_ipc_isr_stall_other_cpu() may
* suspend the external memory cache during the stall (e.g. sleep flows
* powering down flash), in which case a speculative fetch into cached
* address space raises a cache access-fail interrupt. Keep the predictor
* disabled until the stall is released. */
esp_cpu_branch_prediction_disable();
#endif
esp_ipc_isr_stall_fl = 1; esp_ipc_isr_stall_fl = 1;
while (esp_ipc_isr_stall_args.cmd == ESP_IPC_ISR_CMD_RESET_STATE) { while (esp_ipc_isr_stall_args.cmd == ESP_IPC_ISR_CMD_RESET_STATE) {
if (esp_ipc_isr_stall_args.func != NULL) { if (esp_ipc_isr_stall_args.func != NULL) {
@@ -53,4 +63,7 @@ void ESP_SYSTEM_IRAM_ATTR esp_ipc_isr_waiting_for_finish_cmd(void* arg)
esp_ipc_isr_stall_args.func = NULL; esp_ipc_isr_stall_args.func = NULL;
} }
} }
#if SOC_BRANCH_PREDICTOR_SUPPORTED
esp_cpu_branch_prediction_enable();
#endif
} }
@@ -120,6 +120,15 @@ static void frame_to_panic_info(void *frame, panic_info_t *info, bool pseudo_exc
FORCE_INLINE_ATTR __attribute__((__noreturn__)) FORCE_INLINE_ATTR __attribute__((__noreturn__))
void busy_wait(void) 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(); ESP_INFINITE_LOOP();
} }
#endif // !CONFIG_ESP_SYSTEM_SINGLE_CORE_MODE #endif // !CONFIG_ESP_SYSTEM_SINGLE_CORE_MODE
+39 -8
View File
@@ -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 * SPDX-License-Identifier: Apache-2.0
*/ */
@@ -30,6 +30,7 @@
#include "esp_private/esp_ipc.h" #include "esp_private/esp_ipc.h"
#endif #endif
#include "esp_attr.h" #include "esp_attr.h"
#include "esp_cpu.h"
#include "esp_memory_utils.h" #include "esp_memory_utils.h"
#include "esp_intr_alloc.h" #include "esp_intr_alloc.h"
#include "esp_private/esp_cache_private.h" #include "esp_private/esp_cache_private.h"
@@ -100,6 +101,13 @@ void IRAM_ATTR spi_flash_op_block_func(void *arg)
// Restore interrupts that aren't located in IRAM // Restore interrupts that aren't located in IRAM
esp_intr_noniram_disable(); esp_intr_noniram_disable();
uint32_t cpuid = (uint32_t) arg; 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 // 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 // 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) // (if it is preempted by an ISR taking non-trivial amount of time)
@@ -110,6 +118,9 @@ void IRAM_ATTR spi_flash_op_block_func(void *arg)
} }
// Flash operation is complete, re-enable cache // Flash operation is complete, re-enable cache
spi_flash_restore_cache(cpuid, s_flash_op_cache_state[cpuid]); 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 // Restore interrupts that aren't located in IRAM
esp_intr_noniram_enable(); esp_intr_noniram_enable();
#if ( ( CONFIG_FREERTOS_SMP ) && ( !CONFIG_FREERTOS_UNICORE ) ) #if ( ( CONFIG_FREERTOS_SMP ) && ( !CONFIG_FREERTOS_UNICORE ) )
@@ -180,6 +191,9 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu(void)
// Kill interrupts that aren't located in IRAM // Kill interrupts that aren't located in IRAM
esp_intr_noniram_disable(); 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 // 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 // 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 // with non-iram interrupts and the scheduler disabled. None of these CPUs will
@@ -216,6 +230,9 @@ void IRAM_ATTR spi_flash_enable_interrupts_caches_and_other_cpu(void)
s_flash_op_complete = true; s_flash_op_complete = true;
} }
#if SOC_BRANCH_PREDICTOR_SUPPORTED
esp_cpu_branch_prediction_enable();
#endif
// Re-enable non-iram interrupts // Re-enable non-iram interrupts
esp_intr_noniram_enable(); esp_intr_noniram_enable();
@@ -242,6 +259,12 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu_no_os(void)
const uint32_t cpuid = xPortGetCoreID(); const uint32_t cpuid = xPortGetCoreID();
const uint32_t other_cpuid = (cpuid == 0) ? 1 : 0; 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 // 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]); spi_flash_disable_cache(other_cpuid, &s_flash_op_cache_state[other_cpuid]);
// Kill interrupts that aren't located in IRAM // Kill interrupts that aren't located in IRAM
@@ -256,6 +279,9 @@ void IRAM_ATTR spi_flash_enable_interrupts_caches_no_os(void)
// Re-enable cache on this CPU // Re-enable cache on this CPU
spi_flash_restore_cache(cpuid, s_flash_op_cache_state[cpuid]); 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 // Re-enable non-iram interrupts
esp_intr_noniram_enable(); esp_intr_noniram_enable();
} }
@@ -295,12 +321,18 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu(void)
{ {
spi_flash_op_lock(); spi_flash_op_lock();
esp_intr_noniram_disable(); 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]); spi_flash_disable_cache(0, &s_flash_op_cache_state[0]);
} }
void IRAM_ATTR spi_flash_enable_interrupts_caches_and_other_cpu(void) void IRAM_ATTR spi_flash_enable_interrupts_caches_and_other_cpu(void)
{ {
spi_flash_restore_cache(0, s_flash_op_cache_state[0]); 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(); esp_intr_noniram_enable();
spi_flash_op_unlock(); spi_flash_op_unlock();
} }
@@ -309,6 +341,9 @@ void IRAM_ATTR spi_flash_disable_interrupts_caches_and_other_cpu_no_os(void)
{ {
// Kill interrupts that aren't located in IRAM // Kill interrupts that aren't located in IRAM
esp_intr_noniram_disable(); esp_intr_noniram_disable();
#if SOC_BRANCH_PREDICTOR_SUPPORTED
esp_cpu_branch_prediction_disable();
#endif
// Disable cache on this CPU as well // Disable cache on this CPU as well
spi_flash_disable_cache(0, &s_flash_op_cache_state[0]); spi_flash_disable_cache(0, &s_flash_op_cache_state[0]);
} }
@@ -317,6 +352,9 @@ void IRAM_ATTR spi_flash_enable_interrupts_caches_no_os(void)
{ {
// Re-enable cache on this CPU // Re-enable cache on this CPU
spi_flash_restore_cache(0, s_flash_op_cache_state[0]); 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 // Re-enable non-iram interrupts
esp_intr_noniram_enable(); esp_intr_noniram_enable();
} }
@@ -339,19 +377,12 @@ void IRAM_ATTR spi_flash_enable_cache(uint32_t cpuid)
#if !CONFIG_SPI_FLASH_ROM_IMPL #if !CONFIG_SPI_FLASH_ROM_IMPL
void IRAM_ATTR spi_flash_disable_cache(uint32_t cpuid, uint32_t *saved_state) 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(); esp_cache_suspend_ext_mem_cache();
} }
void IRAM_ATTR spi_flash_restore_cache(uint32_t cpuid, uint32_t saved_state) void IRAM_ATTR spi_flash_restore_cache(uint32_t cpuid, uint32_t saved_state)
{ {
esp_cache_resume_ext_mem_cache(); 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) bool IRAM_ATTR spi_flash_cache_enabled(void)
@@ -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 * SPDX-License-Identifier: Apache-2.0
*/ */
@@ -97,7 +97,11 @@ bool spi_flash_cache_enabled(void);
void spi_flash_enable_cache(uint32_t cpuid); 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 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) * @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); 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 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) * @param saved_state Cache status hold by hal (Used only on ROM impl. in idf, this param unused)