From a76e14afe7ea35125b1460f8974d7e3614f3c01e Mon Sep 17 00:00:00 2001 From: wuzhenghui Date: Fri, 31 Jul 2026 17:48:43 +0800 Subject: [PATCH] fix(spi_flash): integrate branch prediction control during cache operations --- .../esp_hw_support/mspi_timing_tuning.c | 25 ++++++++++-- components/esp_hw_support/sleep_modes.c | 7 ++++ components/esp_rom/patches/esp_rom_spiflash.c | 11 ----- components/spi_flash/cache_utils.c | 40 +++++++++++++++---- .../include/esp_private/cache_utils.h | 13 ++++-- 5 files changed, 70 insertions(+), 26 deletions(-) diff --git a/components/esp_hw_support/mspi_timing_tuning.c b/components/esp_hw_support/mspi_timing_tuning.c index 490b6e9641c..d143c49fb24 100644 --- 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); } /*------------------------------------------------------------------------------ diff --git a/components/esp_hw_support/sleep_modes.c b/components/esp_hw_support/sleep_modes.c index 0c1d0b243d0..a87f3166889 100644 --- a/components/esp_hw_support/sleep_modes.c +++ b/components/esp_hw_support/sleep_modes.c @@ -11,6 +11,7 @@ #include #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 } } diff --git a/components/esp_rom/patches/esp_rom_spiflash.c b/components/esp_rom/patches/esp_rom_spiflash.c index 133d66e7507..b69f790a3ac 100644 --- 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 diff --git a/components/spi_flash/cache_utils.c b/components/spi_flash/cache_utils.c index 30321921654..6271dbcd622 100644 --- a/components/spi_flash/cache_utils.c +++ b/components/spi_flash/cache_utils.c @@ -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" @@ -130,7 +131,7 @@ void IRAM_ATTR spi_flash_op_block_func(void *arg) /* 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. spi_flash_restore_cache() below re-enables prediction. */ + * this core. */ esp_cpu_branch_prediction_disable(); #endif // s_flash_op_complete flag is cleared on *this* CPU, otherwise the other @@ -143,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 ) ) @@ -213,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 @@ -249,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(); @@ -275,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 @@ -289,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(); } @@ -328,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(); } @@ -342,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]); } @@ -350,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(); } @@ -372,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) diff --git a/components/spi_flash/include/esp_private/cache_utils.h b/components/spi_flash/include/esp_private/cache_utils.h index f90c3768e68..468cc7ce65f 100644 --- 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)