openmrn icon indicating copy to clipboard operation
openmrn copied to clipboard

Should we be using IRAM_ATTR on all ESP32 Interrupt Handlers and functions they call

Open alex-shepherd-nv opened this issue 6 years ago • 4 comments

Hi Guys, I've recently had to investigate causes for an ESP8266 application crash and along the way found out about the need to add the "ICACHE_FLASH_ATTR" attribute to all interrupt handlers and any functions they call, to ensure the code is in IRAM to avoid crash/conflict issues when accessing files stored on the local SPIFFS. The same thing applies to the ESP32 but the attribute is "IRAM_ATTR". Doing a "grep -r IRAM_ATTR * at the top level of the project only shows 1 file (ESP32CanLoadTest.ino) that has the attribute present. Unless we're avoiding/handling the issue another way, (using a special wildcard rule in the linker script to move the functions into iram0) this is potentially a problem waiting to happen if/when we enable a WebServer using the SPIFFS to store/host the web app files or some other operation that would cause the same SPI resource contention. Here are some links that explain the problem: https://bbs.espressif.com/viewtopic.php?t=1183#p3964 https://docs.espressif.com/projects/esp-idf/en/latest/api-guides/general-notes.html#iram-instruction-ram

alex-shepherd-nv avatar Apr 15 '19 02:04 alex-shepherd-nv

Yes, I agree, it is an issue and poorly implemented by Espressif for some of our use cases. We have run into this before, and it is on our radar.

Thanks, Stuart

On Apr 14, 2019, at 9:03 PM, Alex Shepherd [email protected] wrote:

Hi Guys, I've recently had to investigate causes for an ESP8266 application crash and along the way found out about the need to add the "ICACHE_FLASH_ATTR" attribute to all interrupt handlers and any functions they call, to ensure the code is in IRAM to avoid crash/conflict issues when accessing files stored on the local SPIFFS. The same thing applies to the ESP32 but the attribute is "IRAM_ATTR". Doing a "grep -r IRAM_ATTR * at the top level of the project only shows 1 file (ESP32CanLoadTest.ino) that has the attribute present. Unless we're avoiding/handling the issue another way, (using a special wildcard rule in the linker script to move the functions into iram0) this is potentially a problem waiting to happen if/when we enable a WebServer using the SPIFFS to store/host the web app files or some other operation that would cause the same SPI resource contention. Here are some links that explain the problem: https://bbs.espressif.com/viewtopic.php?t=1183#p3964 https://bbs.espressif.com/viewtopic.php?t=1183#p3964 https://docs.espressif.com/projects/esp-idf/en/latest/api-guides/general-notes.html#iram-instruction-ram https://docs.espressif.com/projects/esp-idf/en/latest/api-guides/general-notes.html#iram-instruction-ram — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bakerstu/openmrn/issues/290, or mute the thread https://github.com/notifications/unsubscribe-auth/ACLFjqlrE7juTr02KODnQO4sT16tRr1nks5vg93xgaJpZM4cvAHs.

bakerstu avatar Apr 15 '19 02:04 bakerstu

IRAM is very limited and we need to be very selective as to what we tag as that. We shouldn't have any ISRs in OpenMRNLite code for the esp32, except in the load test example which uses a hardware timer which uses an ISR context for the callback invocation. Outside of this usage there shouldn't be any other ISR usages at this time.

One side note on the esp32, using the IRAM_ATTR tag does NOT make it safe to use SPIFFS! If you access spi_flash it will crash almost everytime with an error related to "cache accessed while cache disabled". For this reason, I would avoid ISRs/timers on the esp32 unless absolutely necessary.

atanisoft avatar Apr 15 '19 03:04 atanisoft

Also inside the ISR context, unless data is tagged as DRAM_ATTR it is possible to crash the esp32. If it is tagged it is read-only. The only data that can be accessed as read-write is inside IRAM and only accessible as 32bit access (byte access will cause a panic).

atanisoft avatar Apr 15 '19 03:04 atanisoft

@alex-shepherd-nv for the ESP32 we are usually not providing interrupt handlers. interrupt handlers are required by OS functions and hardware drivers, and all of those are coming from specific arduino libraries or the arduino esp32 core. No interrupt handlers -> no IRAM_ATTR needed.

balazsracz avatar Apr 18 '19 17:04 balazsracz