[Crash][v4.4.3][ESP32-S3] 16KB Data Cache KConfig does not work. It's half implemented. (IDFGH-8841)

chipweinberger commented 1 year ago

Answers checklist.

[X] I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
[X] I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
[X] I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

v4.4.3-215-g9f3f82f54b

Operating System used.

Windows

How did you build your project?

VS Code IDE

If you are using Windows, please specify command line type.

No response

Development Kit.

ESP32-S3 Dev Kit C

Power Supply used.

USB

What is the expected behavior?

I should save 16KB of Internal RAM by setting Data Cache Size to 16KB, versus 32KB.

What is the actual behavior?

Shortly after boot, I hit this assert:

assert failed: spi_flash_disable_interrupts_caches_and_other_cpu cache_utils.c:137 (esp_task_stack_is_sane_cache_disabled())

which checks that the stack ptr is in dram:

inline static bool IRAM_ATTR esp_ptr_in_dram(const void *p) {
    return ((intptr_t)p >= SOC_DRAM_LOW && (intptr_t)p < SOC_DRAM_HIGH);
}

Steps to reproduce.

Set CONFIG_ESP32S3_DATA_CACHE_16KB=y
try to initialize wifi
(possibly other unknown steps)
crash

Debug Logs.

I (9407) pd wifi: esp_event_loop_create_default()
I (9417) pd wifi: esp_netif_create_default_wifi_ap()
I (9417) pd wifi: esp_wifi_init()
I (9427) pp: pp rom version: e7ae62f
I (9427) net80211: net80211 rom version: e7ae62f

assert failed: spi_flash_disable_interrupts_caches_and_other_cpu cache_utils.c:137 (esp_task_stack_is_sane_cache_disabled())

Backtrace: 0x40376507:0x3c002680 |<-CORRUPTED
0x40376507: panic_abort at /esp-idf/components/esp_system/panic.c:429

More Information.

sdkconfig.txt

chipweinberger commented 1 year ago

Inmemory_layout.c, this code looks fishy.

#if CONFIG_ESP32S3_DATA_CACHE_16KB || CONFIG_ESP32S3_DATA_CACHE_32KB
    { 0x3FCF0000,           0x8000,                                     SOC_MEMORY_TYPE_DRAM,       0}, //Level 9, DRAM, DMA is accessible but retention DMA is inaccessible
#endif
#if CONFIG_ESP32S3_DATA_CACHE_16KB
    { 0x3C000000,           0x4000,                                     SOC_MEMORY_TYPE_DRAM,       0}, //Level 10, DRAM, DMA is accessible but retention DMA is inaccessible
#endif

I would have expected 0x3C000000 to be 0x3FCF0000 + 0x8000, or something. I don't really understand this map.

chipweinberger commented 1 year ago

Also this code looks fishy. There are 3 options (64KB, 32KB, 16KB), not 2. Shouldnt there be 3 #ifdefs?

void esp_config_data_cache_mode() {
  ...
  #if CONFIG_ESP32S3_DATA_CACHE_32KB
      Cache_Occupy_DCache_MEMORY(CACHE_MEMORY_DBANK1, CACHE_MEMORY_INVALID);
      cache_size = CACHE_SIZE_HALF;
  #else
      Cache_Occupy_DCache_MEMORY(CACHE_MEMORY_DBANK0, CACHE_MEMORY_DBANK1);
      cache_size = CACHE_SIZE_FULL;
  #endif
  ...
}

chipweinberger commented 1 year ago

To be honest, I dont understand why 0x3C000000, which is the ESP32-S3 SOC_DROM_LOW address is being mapped to SOC_MEMORY_TYPE_DRAM. This makes no sense to me. DROM is in flash memory, meant for string constants, etc, not the heap.

But in cpu_start.c I see this code, which seems to confirm the that 'Data Cache' uses DROM? Very confusing.

#if CONFIG_ESP32S3_DATA_CACHE_16KB
    Cache_Invalidate_DCache_All();
    Cache_Occupy_Addr(SOC_DROM_LOW, 0x4000);
#endif
}

But I believe these might be typos.

chipweinberger commented 1 year ago

The more I learn using the technical reference manual, the more I realize 16KB Data Cache is just not supported.

Cache_Set_DCache_Mode only supports "FULL(64KB)" and "HALF(32KB)", but AFAICT not "QUARTER(16KB)". This function is implemented in ROM, so I don't know for sure.

chipweinberger commented 1 year ago

this was fixed here: https://github.com/espressif/esp-idf/commit/3118120659869ea79ecdb33c45f9d4e20fded563

esp_hw_support: Update memory ptr location/property checks
- to acknowledge the unused DCACHE added to DRAM for ESP32-S3

- For ESP32-S3, when the DCACHE size is set to 16 kB, the unused 48 kB is added to
  the heap in 2 blocks of 32 kB (from 0x3FCF0000) and 16 kB (from 0x3C000000).
- But, if we try allocating memory from the 16 kB block and run an `esp_ptr_internal`
  check on that memory pointer, it fails as the address block from 0x3C000000
  corresponds to the external memory symbols SOC_DROM_LOW and SOC_EXTRAM_DATA_LOW.
  (E.g. freertos - If the IDLE task stack buffer gets allocated from this region,
  the firmware will abort due to this failure).
- Thus, the checks `esp_ptr_internal`, `esp_ptr_in_drom` and `esp_ptr_byte_accessible`
  have been updated to acknowledge this memory as a part of the DRAM.

Co-authored-by: Mahavir Jain <mahavir@espressif.com>

chipweinberger commented 1 year ago

@mahavirj can you look at this comment: https://github.com/espressif/esp-idf/issues/10268#issuecomment-1330125529 Does this look okay to you?

Edit: It appears esp_config_data_cache_mode(); is not called for ESP32S3. See call_start_cpu0

#if CONFIG_IDF_TARGET_ESP32S2
    ...
    Cache_Enable_DCache(0);
#endif

but instead S3 calls:

rom_config_data_cache_mode(CONFIG_ESP32S3_DATA_CACHE_SIZE, CONFIG_ESP32S3_DCACHE_ASSOCIATED_WAYS, CONFIG_ESP32S3_DATA_CACHE_LINE_SIZE);

All of this code is fishy. @mahavirj can you please search the code base for CONFIG_ESP32S3_DATA_CACHE_ and audit it =)

Sherry616 commented 8 months ago

Hi @chipweinberger, let me introduce the confusing map.

In esp32s3, our cache supports cache occupy feature, that is force occupy some cache lines in the cache data memory. The occupied cache lines cannot be replaced, in such ways the cache lines can be treat as internal memory. But, we should must use cache virtual address to access these "internal memory", or we can call it "FAKE dram"

The issue is obvious clear for the assert log: esp_task_stack_is_sane_cache_disabled() I think this is because somehow your task's stack is located in the "FAKE dram", but IDF does not support good of the address region. Thanks.

espressif / esp-idf