[v5.1] JTAG memory region collision when CONFIG_ESP_SYSTEM_ESP32_SRAM1_REGION_AS_IRAM is true (IDFGH-10220)

AntoineSX 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.

v5.1-dev-4900-g56677dabe8

Operating System used.

Linux

How did you build your project?

VS Code IDE

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

None

What is the expected behavior?

No assertions, no memory region collision

What is the actual behavior?

Panic abort caused by overlapping memory regions during heap init

Steps to reproduce.

Enable JTAG as Data Destination 1, keep TRAX tracing disabled
Enable Reserve parts of SRAM1 for app IRAM (WARNING, read help before enabling)
build and flash

Build or installation Logs.

I (395) cpu_start: App cpu up.
I (425) cpu_start: Pro cpu start user code
I (425) cpu_start: cpu freq: 240000000 Hz
I (425) cpu_start: Application information:
I (430) cpu_start: Project name:     [redacted]
I (434) cpu_start: App version:      4da13c1-dirty
I (440) cpu_start: Compile time:     May 23 2023 12:49:18
I (446) cpu_start: ELF file SHA256:  7d7c660b8281e1ae...
I (452) cpu_start: ESP-IDF:          v5.1-dev-4900-g56677dabe8-dirty
I (459) cpu_start: Min chip rev:     v3.0
I (464) cpu_start: Max chip rev:     v3.99 
I (468) cpu_start: Chip rev:         v3.0

assert failed: s_prepare_reserved_regions memory_layout_utils.c:94 (reserved[i + 1].start > reserved[i].start)

Backtrace: 0x40081c62:0x3ffe32f0 0x40095201:0x3ffe3310 0x4009bad5:0x3ffe3330 0x400d415a:0x3ffe3460 0x400d41f5:0x3ffe3490 0x400d3caf:0x3ffe3800 0x400d3037:0x3ffe3be0 0x400d337e:0x3ffe3c00 0x40081a6e:0x3ffe3c40 0x4007941e:0x3ffe3c90 |<-CORRUPTED

More Information.

I'm using an ESP32-WROVER-E module.

I did some digging around and found that in the linker script for the ESP32 family and the c file memory_layout.c contain some errors. The commit in question is 34fea0d38faec37df4492835e7d4ebf6aee55b8a

memory.ld.in:

#if CONFIG_ESP_SYSTEM_ESP32_SRAM1_REGION_AS_IRAM
#define SRAM1_IRAM_LEN 0xA000
#else
#define SRAM1_IRAM_LEN 0x0
#endif

MEMORY
{

  /* IRAM for PRO cpu. Not sure if happy with this, this is MMU area... */
  iram0_0_seg (RX) :                 org = 0x40080000, len = 0x20000 + SRAM1_IRAM_LEN

  . . .

  _sram1_iram_start = 0x400A0000;
  _sram1_iram_len = ( _iram_end > _sram1_iram_start) ? (_iram_end - _sram1_iram_start) : 0;
  _heap_end = ALIGN(0x40000000 - _sram1_iram_len - 3, 4);

  #if CONFIG_ESP32_TRACEMEM_RESERVE_DRAM != 0
  _heap_end = 0x40000000 - CONFIG_ESP32_TRACEMEM_RESERVE_DRAM;
  #endif

  . . .

}

if SRAM1_IRAM_LEN is 0xA000, _sram1_iram_len will be:
_sram1_iram_len = 0x40080000 + 0x20000 + 0xA000 - 0x400A0000
_sram1_iram_len = 0xA000

and so:
_heap_end = ALIGN(0x40000000 - 0xA000 - 3,4)
_heap_end = 0x3FFF5FFC

But when JTAG is enabled (without TRAX) tracemem_reserver_dram is 0x8000 bytes (32KB). which mean _heap_end will be redefined as:
_heap_end = 0x40000000 - 0x8000
_heap_end = 0x3FFF8000
It seems like that line doesn't take into account the offset caused by the _sram1_iram_len so I changed that part of the linker script to:

_sram1_iram_start = 0x400A0000;
_sram1_iram_len = ( _iram_end > _sram1_iram_start) ? (_iram_end - _sram1_iram_start) : 0;
_heap_end = 0x40000000 - _sram1_iram_len;

#if CONFIG_ESP32_TRACEMEM_RESERVE_DRAM != 0
_heap_end -= CONFIG_ESP32_TRACEMEM_RESERVE_DRAM;
#endif

I'm not sure why an ALIGN was added in that same commit, as both sram1_iram_len and 0x40000000 are already 32bit aligned, and I'm also not sure why it wasn't also used for the _heap_end assignment when CONFIG_ESP32_TRACEMEM_RESERVE_DRAM != 0. In my case I just didn't use ALIGN, but it might be a mistake on my part.

Then I had to redefine the memory regions for tracemem in the memory_layout.c file of the esp32 taking that offset into account.

Before:

#if CONFIG_ESP32_MEMMAP_TRACEMEM
#if CONFIG_ESP32_MEMMAP_TRACEMEM_TWOBANKS
SOC_RESERVE_MEMORY_REGION(0x3fff8000, 0x40000000, trace_mem); //Reserve trace mem region, 32K for both cpu
#else
SOC_RESERVE_MEMORY_REGION(0x3fffc000, 0x40000000, trace_mem); //Reserve trace mem region, 16K (upper-half) for pro cpu
#endif
#endif

After:

#if CONFIG_ESP32_MEMMAP_TRACEMEM
#if CONFIG_ESP32_MEMMAP_TRACEMEM_TWOBANKS
#if CONFIG_ESP_SYSTEM_ESP32_SRAM1_REGION_AS_IRAM
SOC_RESERVE_MEMORY_REGION(0x3ffee000, 0x3fff6000, trace_mem); //Reserve trace mem region, 32K for both cpu
#else
SOC_RESERVE_MEMORY_REGION(0x3fff8000, 0x40000000, trace_mem); //Reserve trace mem region, 32K for both cpu
#endif
#else
#if CONFIG_ESP_SYSTEM_ESP32_SRAM1_REGION_AS_IRAM
SOC_RESERVE_MEMORY_REGION(0x3fff2000, 0x3fff6000, trace_mem); //Reserve trace mem region, 16K (upper-half) for pro cpu
#else
SOC_RESERVE_MEMORY_REGION(0x3fffc000, 0x40000000, trace_mem); //Reserve trace mem region, 16K (upper-half) for pro cpu
#endif
#endif
#endif

Here's a diagram that helped me visualize the sram segments: Memory Map

ESP-Marius commented 1 year ago

@AntoineSX Thanks for reporting this!

We'll take a look at it.

AntoineSX commented 1 year ago

Any update ?

ESP-Marius commented 1 year ago

Sorry for the delay! We'll prioritize closing this one next week.

ESP-Marius commented 1 year ago

Unfortunately I dont think the proposed change will work, since as far as I can see the trace memory blocks on ESP32 are fixed, so just reserving a different memory are wont work.

I think the fix will be to not allow these to be used together (like we already did for ESP32_TRAX)

espressif / esp-idf