Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STM32H7: Don't use DTCM memory for heap #396

Merged
merged 2 commits into from
Nov 26, 2024
Merged

Conversation

multiplemonomials
Copy link
Collaborator

@multiplemonomials multiplemonomials commented Nov 26, 2024

Summary of changes

The change that I made a month or two ago to use DTCM as the default heap on STM32H7 has caused some unforseen consequences. Basically, the DMA controller cannot access anything in DTCM in any shape or form, so using SDIO, Ethernet, or DMA SPI with heap-allocated buffers will cause a crash or unintended behavior.

This PR updates the STM32H7 linker scripts (soooo glad there are only 4 of them now!) to use main SRAM as heap 0 and SRAM_D2/SRAM_AXI as heap. This way, we still can take advantage of two of the memory banks for heap size, but we don't have to deal with the issues caused by DTCM. DTCM provides a big performance boost, make no mistake, but I think it's best left as a power user only option.

I also made one other change in this, which is that until now, targets which implemented split heap still had to provide the end and __HeapLimit symbols in the linker script in order for the mbed_heap_start/size variables to be correct, even though the actual heap allocator did not use them. This consolidates those so that only the "new style" symbols are needed if you have split heap, and only the "old style" symbols are needed if you don't. I like this because __end__ is a super vague name and while Mbed only uses it as the start of heap, it's hard to say if everyone agrees that's what it means, so I would rather move away from using it.

Impact of changes

  • Fixes crashes and broken stuff when using heap allocated buffers on STM32H7
  • For targets defining MBED_SPLIT_HEAP, the linker script only needs to define __mbed_sbrk_start/__mbed_sbrk_start_0 and __mbed_krbs_start/__mbed_krbs_start_0, not end and __HeapLimit.

Migration actions required

Documentation


Pull request type

[X] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[X] Tests / results supplied as part of this PR

The following tests passed on NUCLEO_H743ZI2:

        test-mbed-platform-stats-heap
        test-mbed-rtos-heap-and-stack
        test-mbed-storage-blockdevice-heap_block_device


@JojoS62
Copy link

JojoS62 commented Nov 26, 2024

building fails with

[build] /home/jojo/projects/mbed/mbed-ce-H7/mbed-os/platform/source/mbed_retarget.cpp:1532:(.text._sbrk+0x24): undefined reference to `__HeapLimit'

in the case of no split_heap

edit:
linker script is for use with split_heap, which is also default

@JojoS62 JojoS62 merged commit 2564b2c into master Nov 26, 2024
52 checks passed
@JojoS62 JojoS62 deleted the dev/stm32h7-dont-use-dtcm branch November 26, 2024 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants