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

libs/modlib: Load data using up_textheap_data_address #14600

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

tmedicci
Copy link
Contributor

@tmedicci tmedicci commented Nov 1, 2024

Summary

  • libs/modlib: Load data using up_textheap_data_address

Some chips have different memory addressing spaces for the same region. This is true, for instance, for ESP32-S3: the same memory region can be accessed using the data bus or the data bus using different address ranges. The instruction bus, however, requires word-aligned access. That being said, it is recommended to use the data bus while copying sections to the text heap to avoid any illegal access using the instruction bus address which will be later used to run the program.

Impact

Fix #14487

Testing

Internal CI testing + ESP32-S3-DevKitC-1 v1.0:

Build steps:

make -j distclean && ./tools/configure.sh esp32s3-devkit:elf && make flash ESPTOOL_PORT=/dev/ttyUSB0 -s -j$(nproc) && minicom -D /dev/ttyUSB0

And, then, run elf example:

nsh> elf

@github-actions github-actions bot added Area: OS Components OS Components issues Size: XS The size of the change in this PR is very small labels Nov 1, 2024
@xiaoxiang781216
Copy link
Contributor

@tmedicci some chip need provide the dummy up_textheap_data_address implementation

@acassis
Copy link
Contributor

acassis commented Nov 2, 2024

@tmedicci I think it will break ELF for some arch that doesn't use CONFIG_ARCH_USE_TEXT_HEAP see:

#if defined(CONFIG_ARCH_USE_TEXT_HEAP)
#if defined(CONFIG_ARCH_HAVE_TEXT_HEAP_SEPARATE_DATA_ADDRESS)
FAR void *up_textheap_data_address(FAR void *p);
#else
#define up_textheap_data_address(p) ((FAR void *)p)
#endif
#endif

Note that up_textheap_data_address() doesn't exist case CONFIG_ARCH_USE_TEXT_HEAP is not defined

Some chips have different memory addressing spaces for the same
region. This is true, for instance, for ESP32-S3: the same memory
region can be accessed using the data bus or the data bus using
different address ranges. The instruction bus, however, requires
word-aligned access. That being said, it is recommended to use the
data bus while copying sections to the text heap to avoid any
illegal access using the instruction bus address which will be
later used to run the program.
@github-actions github-actions bot added Size: S The size of the change in this PR is small and removed Size: XS The size of the change in this PR is very small labels Nov 4, 2024
@tmedicci tmedicci requested review from anjiahao1 and acassis and removed request for anjiahao1 November 4, 2024 18:39
@xiaoxiang781216 xiaoxiang781216 merged commit f6a72ad into apache:master Nov 4, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: OS Components OS Components issues Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] ELF loader on ESP32-S3 broken after #14100
4 participants