Skip to content

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 acassis and anjiahao1 and removed request for anjiahao1 November 4, 2024 18:39
@xiaoxiang781216 xiaoxiang781216 merged commit f6a72ad into apache:master Nov 4, 2024
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