Merge branch 'bugfix/cache_writeback_bug' into 'master'

esp_rom: patch Cache_WriteBack_Addr,  avoid accessing cachelines that are being writebacked

Closes AUD-4678

See merge request espressif/esp-idf!24663

components/esp_mm/esp_cache.c

@@ -9,85 +9,52 @@
 #include "sdkconfig.h"
 #include "esp_check.h"
 #include "esp_log.h"
+#include "esp_rom_caps.h"
 #include "soc/soc_caps.h"
 #include "hal/mmu_hal.h"
 #include "hal/cache_hal.h"
 #include "esp_cache.h"
 #include "esp_private/critical_section.h"
 
-
 static const char *TAG = "cache";
 DEFINE_CRIT_SECTION_LOCK_STATIC(s_spinlock);
 
-void s_cache_freeze(void)
-{
-#if SOC_CACHE_FREEZE_SUPPORTED
- cache_hal_freeze(CACHE_TYPE_DATA | CACHE_TYPE_INSTRUCTION);
-#endif
-
- /**
- * For writeback supported, but the freeze not supported chip (Now only S2),
- * as it's single core, the critical section is enough to prevent preemption from an non-IRAM ISR
- */
-}
-
-void s_cache_unfreeze(void)
-{
-#if SOC_CACHE_FREEZE_SUPPORTED
- cache_hal_unfreeze(CACHE_TYPE_DATA | CACHE_TYPE_INSTRUCTION);
-#endif
-
- /**
- * Similarly, for writeback supported, but the freeze not supported chip (Now only S2),
- * we don't need to do more
- */
-}
-
 
 esp_err_t esp_cache_msync(void *addr, size_t size, int flags)
 {
  ESP_RETURN_ON_FALSE_ISR(addr, ESP_ERR_INVALID_ARG, TAG, "null pointer");
  ESP_RETURN_ON_FALSE_ISR(mmu_hal_check_valid_ext_vaddr_region(0, (uint32_t)addr, size, MMU_VADDR_DATA), ESP_ERR_INVALID_ARG, TAG, "invalid address");
  bool both_dir = (flags & ESP_CACHE_MSYNC_FLAG_DIR_C2M) && (flags & ESP_CACHE_MSYNC_FLAG_DIR_M2C);
  ESP_RETURN_ON_FALSE_ISR(!both_dir, ESP_ERR_INVALID_ARG, TAG, "both C2M and M2C directions are selected, you should only select one");
+ uint32_t data_cache_line_size = cache_hal_get_cache_line_size(CACHE_TYPE_DATA);
+ if ((flags & ESP_CACHE_MSYNC_FLAG_UNALIGNED) == 0) {
+ bool aligned_addr = (((uint32_t)addr % data_cache_line_size) == 0) && ((size % data_cache_line_size) == 0);
+ ESP_RETURN_ON_FALSE_ISR(aligned_addr, ESP_ERR_INVALID_ARG, TAG, "start address, end address or the size is(are) not aligned with the data cache line size (%d)B", data_cache_line_size);
+ }
 
  uint32_t vaddr = (uint32_t)addr;
-
  if (flags & ESP_CACHE_MSYNC_FLAG_DIR_M2C) {
  ESP_EARLY_LOGD(TAG, "M2C DIR");
 
  esp_os_enter_critical_safe(&s_spinlock);
- s_cache_freeze();
-
  //Add preload feature / flag here, IDF-7800
  cache_hal_invalidate_addr(vaddr, size);
-
- s_cache_unfreeze();
  esp_os_exit_critical_safe(&s_spinlock);
 
  } else {
  ESP_EARLY_LOGD(TAG, "C2M DIR");
 
 #if SOC_CACHE_WRITEBACK_SUPPORTED
- esp_os_enter_critical_safe(&s_spinlock);
- uint32_t data_cache_line_size = cache_hal_get_cache_line_size(CACHE_TYPE_DATA);
- esp_os_exit_critical_safe(&s_spinlock);
-
- if ((flags & ESP_CACHE_MSYNC_FLAG_UNALIGNED) == 0) {
- bool aligned_addr = (((uint32_t)addr % data_cache_line_size) == 0) && ((size % data_cache_line_size) == 0);
- ESP_RETURN_ON_FALSE_ISR(aligned_addr, ESP_ERR_INVALID_ARG, TAG, "start address, end address or the size is(are) not aligned with the data cache line size (%d)B", data_cache_line_size);
- }
 
  esp_os_enter_critical_safe(&s_spinlock);
- s_cache_freeze();
-
  cache_hal_writeback_addr(vaddr, size);
+ esp_os_exit_critical_safe(&s_spinlock);
+
  if (flags & ESP_CACHE_MSYNC_FLAG_INVALIDATE) {
+ esp_os_enter_critical_safe(&s_spinlock);
  cache_hal_invalidate_addr(vaddr, size);
+ esp_os_exit_critical_safe(&s_spinlock);
  }
-
- s_cache_unfreeze();
- esp_os_exit_critical_safe(&s_spinlock);
 #endif
  }
 

components/esp_rom/CMakeLists.txt

@@ -46,10 +46,14 @@ if(CONFIG_HAL_WDT_USE_ROM_IMPL)
  list(APPEND sources "patches/esp_rom_wdt.c")
 endif()
 
-if(CONFIG_ESP_ROM_HAS_FLASH_COUNT_PAGES_BUG)
+if(CONFIG_ESP_ROM_HAS_FLASH_COUNT_PAGES_BUG OR CONFIG_ESP_ROM_HAS_CACHE_WRITEBACK_BUG)
  list(APPEND sources "patches/esp_rom_cache_esp32s2_esp32s3.c")
 endif()
 
+if(CONFIG_ESP_ROM_HAS_CACHE_WRITEBACK_BUG)
+ list(APPEND sources "patches/esp_rom_cache_writeback_esp32s3.S")
+endif()
+
 idf_component_register(SRCS ${sources}
  INCLUDE_DIRS ${include_dirs}
  PRIV_REQUIRES ${private_required_comp}

components/esp_rom/esp32s3/Kconfig.soc_caps.in

@@ -82,3 +82,7 @@ config ESP_ROM_HAS_FLASH_COUNT_PAGES_BUG
 config ESP_ROM_HAS_CACHE_SUSPEND_WAITI_BUG
  bool
  default y
+
+config ESP_ROM_HAS_CACHE_WRITEBACK_BUG
+ bool
+ default y

components/esp_rom/esp32s3/esp_rom_caps.h

@@ -26,3 +26,4 @@
 #define ESP_ROM_RAM_APP_NEEDS_MMU_INIT (1) // ROM doesn't init cache MMU when it's a RAM APP, needs MMU hal to init
 #define ESP_ROM_HAS_FLASH_COUNT_PAGES_BUG (1) // ROM api Cache_Count_Flash_Pages will return unexpected value
 #define ESP_ROM_HAS_CACHE_SUSPEND_WAITI_BUG (1) // ROM api Cache_Suspend_I/DCache and Cache_Freeze_I/DCache_Enable does not waiti
+#define ESP_ROM_HAS_CACHE_WRITEBACK_BUG (1) // ROM api Cache_WriteBack_Addr address or size misalignment may cause cache hit with wrong value.

components/esp_rom/esp32s3/ld/esp32s3.rom.ld

@@ -381,7 +381,7 @@ PROVIDE( Cache_WriteBack_Items = 0x40001698 );
 PROVIDE( Cache_Op_Addr = 0x400016a4 );
 PROVIDE( Cache_Invalidate_Addr = 0x400016b0 );
 PROVIDE( Cache_Clean_Addr = 0x400016bc );
-PROVIDE( Cache_WriteBack_Addr = 0x400016c8 );
+PROVIDE( rom_Cache_WriteBack_Addr = 0x400016c8 );
 PROVIDE( Cache_Invalidate_ICache_All = 0x400016d4 );
 PROVIDE( Cache_Invalidate_DCache_All = 0x400016e0 );
 PROVIDE( Cache_Clean_All = 0x400016ec );

components/esp_rom/linker.lf

@@ -4,6 +4,8 @@ entries:
  esp_rom_spiflash (noflash)
  if ESP_ROM_HAS_FLASH_COUNT_PAGES_BUG = y:
  esp_rom_cache_esp32s2_esp32s3 (noflash)
+ if ESP_ROM_HAS_CACHE_WRITEBACK_BUG = y:
+ esp_rom_cache_writeback_esp32s3 (noflash)
  if HEAP_TLSF_USE_ROM_IMPL = y && ESP_ROM_TLSF_CHECK_PATCH = y:
  esp_rom_tlsf (noflash)
  if SOC_SYSTIMER_SUPPORTED = y:

components/esp_rom/patches/esp_rom_cache_esp32s2_esp32s3.c

@@ -7,11 +7,16 @@
 #include "sdkconfig.h"
 #include <stdint.h>
 #include "soc/soc_caps.h"
+#include "esp_rom_caps.h"
 #include "soc/extmem_reg.h"
+#include "xtensa/xtruntime.h"
 #if CONFIG_IDF_TARGET_ESP32S3
 #include "esp32s3/rom/cache.h"
 #endif
 
+#define ALIGN_UP(addr, align) (((addr) + (align)-1) & ~((align)-1))
+#define ALIGN_DOWN(addr, align) ((addr) & ~((align) - 1))
+
 // this api is renamed for patch
 extern uint32_t rom_Cache_Count_Flash_Pages(uint32_t bus, uint32_t * page0_mapped);
 uint32_t Cache_Count_Flash_Pages(uint32_t bus, uint32_t * page0_mapped)
@@ -30,7 +35,7 @@ uint32_t Cache_Count_Flash_Pages(uint32_t bus, uint32_t * page0_mapped)
 }
 extern uint32_t Cache_Count_Flash_Pages(uint32_t bus, uint32_t * page0_mapped);
 
-#if CONFIG_ESP_ROM_HAS_CACHE_SUSPEND_WAITI_BUG
+#if ESP_ROM_HAS_CACHE_SUSPEND_WAITI_BUG
 static inline void Cache_Wait_Idle(int icache)
 {
  if (icache) {
@@ -81,5 +86,72 @@ void Cache_Freeze_DCache_Enable(cache_freeze_mode_t mode)
  Cache_Wait_Idle(0);
 }
 extern void Cache_Freeze_DCache_Enable(cache_freeze_mode_t mode);
-#endif
-#endif
+#endif //#if SOC_CACHE_FREEZE_SUPPORTED
+#endif //#if ESP_ROM_HAS_CACHE_SUSPEND_WAITI_BUG
+
+#if ESP_ROM_HAS_CACHE_WRITEBACK_BUG
+/* Defined in esp_rom_cache_writeback_esp32s3.S */
+extern void cache_writeback_items_freeze(uint32_t addr, uint32_t items);
+// renamed for patch
+extern int rom_Cache_WriteBack_Addr(uint32_t addr, uint32_t size);
+int Cache_WriteBack_Addr(uint32_t addr, uint32_t size)
+{
+ /* Do special processing for unaligned memory at the start and end of the cache writeback memory.
+ * 1. Disable the interrupt to prevent the current CPU accessing the same cacheline.
+ * 2. Enable dcache freeze to prevent the another CPU accessing the same cacheline.
+ */
+ uint32_t irq_status;
+ uint32_t start_len, end_len;
+ uint32_t start, end;
+ uint32_t dcache_line_size;
+ uint32_t autoload;
+ int ret = 0;
+ start = addr;
+ end = addr + size;
+ dcache_line_size = Cache_Get_DCache_Line_Size();
+
+ if (size == 0) {
+ return 0;
+ }
+
+ /*the start address is unaligned*/
+ if (start & (dcache_line_size -1)) {
+ addr = ALIGN_UP(start, dcache_line_size);
+ start_len = addr - start;
+ size = (size < start_len) ? 0 : (size - start_len);
+
+ /*writeback start unaligned mem, one cacheline*/
+ irq_status = XTOS_SET_INTLEVEL(XCHAL_NMILEVEL);//mask all interrupts
+ cache_writeback_items_freeze(start, 1);
+ XTOS_RESTORE_INTLEVEL(irq_status);
+
+ if (size == 0) {
+ return 0;
+ }
+ }
+
+ /*the end address is unaligned*/
+ if (end & (dcache_line_size -1)) {
+ end = ALIGN_DOWN(end, dcache_line_size);
+ end_len = addr + size - end;
+ size = (size - end_len);
+
+ /*writeback end unaligned mem, one cacheline*/
+ irq_status = XTOS_SET_INTLEVEL(XCHAL_NMILEVEL);//mask all interrupts
+ cache_writeback_items_freeze(end, 1);
+ XTOS_RESTORE_INTLEVEL(irq_status);
+
+ if (size == 0) {
+ return 0;
+ }
+ }
+
+ /*suspend autoload, avoid load cachelines being written back*/
+ autoload = Cache_Suspend_DCache_Autoload();
+ ret = rom_Cache_WriteBack_Addr(addr, size);
+ Cache_Resume_DCache_Autoload(autoload);
+
+ return ret;
+}
+extern int Cache_WriteBack_Addr(uint32_t addr, uint32_t size);
+#endif //#if ESP_ROM_HAS_CACHE_WRITEBACK_BUG

components/esp_rom/patches/esp_rom_cache_writeback_esp32s3.S

@@ -0,0 +1,132 @@
+/*
+ * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include <xtensa/corebits.h>
+#include <sdkconfig.h>
+#include "soc/extmem_reg.h"
+
+/**
+ * @brief Write back the cache items of DCache, enable cache freeze during writeback.
+ * Operation will be done CACHE_LINE_SIZE aligned.
+ * If the region is not in DCache addr room, nothing will be done.
+ * Please do not call this function in your SDK application.
+ * @param uint32_t addr: start address to write back
+ * @param uint32_t items: cache lines to invalidate, items * cache_line_size should
+ * not exceed the bus address size(4MB)
+ *
+ * void cache_writeback_items_freeze(uint32_t addr, uint32_t items)
+*/
+
+/*******************************************************************************
+
+This function is a cache write-back function that works around the following
+hardware errata on the ESP32-S3:
+
+- Core X manually triggers (via the EXTMEM_DCACHE_SYNC_CTRL_REG register) the
+write-back of one or more cache lines.
+- While the write-back is in progress, there are two scenarios that may cause
+cache hit error.
+ - Core X enters the interrupt handler and access the same cache line
+ being written back.
+ - Core Y access the same cache line being written back.
+
+To workaround this errata, the following steps must be taken when manually
+triggering a cache write-back:
+
+- Core X must disable interrupts so that it cannot be preempted
+- Core X must freeze the cache (via the EXTMEM_DCACHE_FREEZE_REG register) to
+prevent Core Y from accessing the same cache lines that are about to be written
+back.
+- Core X now triggers the cache write-back. During the write-back...
+ - If Core Y attempts the access any address in the cache region, Core Y will
+ busy wait until the cache is unfrozen.
+ - Core X must ensure that it does not access any address in the cache region,
+ otherwise Core X will busy wait thus causing a deadlock.
+- After the write-back is complete, Core X unfreezes the cache, and reenables
+interrupts.
+
+Notes:
+
+- Please do not modify this function, it must strictly follow the current execution
+sequence, otherwise it may cause unexpected errors.
+- This function is written in assmebly to ensure that the function itself never
+accesses any cache address while the cache is frozen. Unexpected cache access
+could occur if...
+ - the function triggers an window overflow onto a stack placed in PSRAM.
+ Thus, we only use two window panes (a0 to a8) in this function and trigger
+ all window overflows before freezing the cache.
+ - the function accesses literals/read-only variables placed in Flash.
+
+*******************************************************************************/
+
+ .align 4
+ /*
+ Create dedicated literal pool for this function. Mostly used to store out
+ of range movi transformations.
+ */
+ .literal_position
+ .global cache_writeback_items_freeze
+ .type cache_writeback_items_freeze, @function
+cache_writeback_items_freeze:
+ entry sp, 32
+
+ /* REG_WRITE(EXTMEM_DCACHE_SYNC_ADDR_REG, addr); */
+ movi a4, EXTMEM_DCACHE_SYNC_ADDR_REG
+ s32i a2, a4, 0
+ /* REG_WRITE(EXTMEM_DCACHE_SYNC_SIZE_REG, items); */
+ movi a4, EXTMEM_DCACHE_SYNC_SIZE_REG
+ s32i a3, a4, 0
+ memw /* About to freeze the cache. Ensure all previous memory R/W are completed */
+
+ movi a2, EXTMEM_DCACHE_FREEZE_REG
+ movi a3, EXTMEM_DCACHE_SYNC_CTRL_REG
+
+ /*
+ REG_CLR_BIT(EXTMEM_DCACHE_FREEZE_REG, EXTMEM_DCACHE_FREEZE_MODE);
+ REG_SET_BIT(EXTMEM_DCACHE_FREEZE_REG, EXTMEM_DCACHE_FREEZE_ENA);
+ */
+ l32i a4, a2, 0 /* a4 = *(EXTMEM_DCACHE_FREEZE_REG) */
+ movi a5, ~(EXTMEM_DCACHE_FREEZE_MODE_M)
+ and a4, a4, a5
+ movi a5, EXTMEM_DCACHE_FREEZE_ENA_M
+ or a4, a4, a5
+ s32i a4, a2, 0 /* *(EXTMEM_DCACHE_FREEZE_REG) = a4 */
+
+ /* while (!REG_GET_BIT(EXTMEM_DCACHE_FREEZE_REG, EXTMEM_DCACHE_FREEZE_DONE)); */
+ movi a5, EXTMEM_DCACHE_FREEZE_DONE_M
+_wait_freeze_done:
+ l32i a4, a2, 0 /* a4 = *(EXTMEM_DCACHE_FREEZE_REG) */
+ memw
+ bnone a4, a5, _wait_freeze_done
+
+ /* REG_SET_BIT(EXTMEM_DCACHE_SYNC_CTRL_REG, EXTMEM_DCACHE_WRITEBACK_ENA); */
+ l32i a4, a3, 0 /* a4 = *(EXTMEM_DCACHE_SYNC_CTRL_REG) */
+ movi a5, EXTMEM_DCACHE_WRITEBACK_ENA_M
+ or a4, a4, a5
+ s32i a4, a3, 0 /* *(EXTMEM_DCACHE_SYNC_CTRL_REG) = a4 */
+
+ /* while(!REG_GET_BIT(EXTMEM_DCACHE_SYNC_CTRL_REG, EXTMEM_DCACHE_SYNC_DONE)); */
+ movi a5, EXTMEM_DCACHE_SYNC_DONE_M
+_wait_writeback_done:
+ l32i a4, a3, 0 /* a4 = *(EXTMEM_DCACHE_SYNC_CTRL_REG) */
+ memw
+ bnone a4, a5, _wait_writeback_done
+
+ /* REG_CLR_BIT(EXTMEM_DCACHE_FREEZE_REG, EXTMEM_DCACHE_FREEZE_ENA); */
+ l32i a4, a2, 0 /* a4 = *(EXTMEM_DCACHE_FREEZE_REG) */
+ movi a5, ~(EXTMEM_DCACHE_FREEZE_ENA_M)
+ and a4, a4, a5
+ s32i a4, a2, 0 /* *(EXTMEM_DCACHE_FREEZE_REG) = a4 */
+
+ /* while (REG_GET_BIT(EXTMEM_DCACHE_FREEZE_REG, EXTMEM_DCACHE_FREEZE_DONE)); */
+ movi a5, EXTMEM_DCACHE_FREEZE_DONE_M
+_wait_unfreeze_done:
+ l32i a4, a2, 0 /* a4 = *(EXTMEM_DCACHE_FREEZE_REG) */
+ memw
+ bany a4, a5, _wait_unfreeze_done
+
+ retw
+ .size cache_writeback_items_freeze, . - cache_writeback_items_freeze