fix(openeth): fix cache-off panic in IRAM ISR handler (IDFGH-17705)#18645
Open
vvzvlad wants to merge 1 commit into
Open
fix(openeth): fix cache-off panic in IRAM ISR handler (IDFGH-17705)#18645vvzvlad wants to merge 1 commit into
vvzvlad wants to merge 1 commit into
Conversation
The openeth MAC ISR (emac_opencores_isr_handler) is registered with ESP_INTR_FLAG_IRAM and marked IRAM_ATTR. This means it must not access flash-backed memory (constants in .rodata) when called while the flash cache is disabled. Two issues were present: 1. TAG was declared as 'static const char *TAG', placing the string literal in flash (.rodata). Accessing it from IRAM ISR context with cache disabled causes a 'Cache disabled but cached memory region accessed' panic (EXCCAUSE 7). 2. ESP_EARLY_LOGW was used with TAG and __func__ (also a flash string), which is unsafe in cache-off ISR context. Fix: - Declare TAG as 'static const DRAM_ATTR char TAG[]' to place the string in DRAM, making it accessible regardless of cache state. - Replace ESP_EARLY_LOGW with ESP_DRAM_LOGW, which is designed for use when cache may be disabled. - Remove __func__ argument (flash string) from the log call. Reproducer and crash logs available in openeth_repro/ directory.
|
vvzvlad seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
emac_opencores_isr_handler is registered with ESP_INTR_FLAG_IRAM and marked IRAM_ATTR, meaning it must not access flash-backed memory while the cache is disabled. However, the handler contained:
ESP_EARLY_LOGW(TAG, "%s: RX frame dropped (0x%" PRIx32 ")", func, status);
This is not cache-off-safe:
TAG (static const char *TAG = "opencores.emac") sits in .rodata → flash
func is also a flash-resident string
ESP_EARLY_LOGW eventually calls uart_hal_write_txfifo, which lives in flash (IROM) unless CONFIG_UART_ISR_IN_IRAM=y
When OPENETH_INT_BUSY fires while another task has the cache disabled (e.g. during esp_partition_read), the firmware panics:
Guru Meditation Error: Core 0 panic'ed (Cache disabled but cached memory region accessed).
EXCCAUSE: 0x00000007
Fix:
Declare TAG as static const DRAM_ATTR char TAG[] to place the string in DRAM
Replace ESP_EARLY_LOGW with ESP_DRAM_LOGW (the correct macro for cache-off ISR context)
Remove the func argument (flash string) from the log call
Related
Reproducer (synthetic, no networking, deterministic): openeth_repro/ — registers an ESP_INTR_FLAG_IRAM SW interrupt mirroring the openeth ISR body, and concurrently hammers esp_partition_read to disable the cache. Crashes within seconds on unpatched code; runs indefinitely with this fix.
Measurements (ESP32, IDF v5.4, qemu-system-xtensa):
Build Crashes / 30 s ISR fires before crash
Upstream (buggy) 6 ~191
This patch 0 27 636+
Issue: #18644