8000 Update .apps section by alexandruradovici · Pull Request #4191 · tock/tock · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Update .apps section #4191

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 11, 2024
Merged

Update .apps section #4191

merged 2 commits into from
Oct 11, 2024

Conversation

alexandruradovici
Copy link
Contributor
@alexandruradovici alexandruradovici commented Oct 9, 2024 •
edited
Loading

Pull Request Overview

This pull request fixes a the linker script to allow the writing of applications into kernel's elf file.

While rebasing #4099, @inesmaria08 got the following error while adding apps into the kernel's elf file errors with:

arm-none-eabi-objcopy --set-section-flags .apps=LOAD --update-section .apps=../../../../../libtock-c/examples/tests/screen/build/cortex-m0/cortex-m0.tbf /Users/ines_maria/tock_wyliodrin/clona/tock/target/thumbv6m-none-eabi/release/pico_explorer_base.elf /Users/ines_maria/tock_wyliodrin/clona/tock//target/thumbv6m-none-eabi/release/pico_explorer_base-app.elf
arm-none-eabi-objcopy: /Users/ines_maria/tock_wyliodrin/clona/tock//target/thumbv6m-none-eabi/release/pico_explorer_base-app.elf: section `.relocate' can't be allocated in segment 2
LOAD: .ARM.exidx .storage .relocate
arm-none-eabi-objcopy: /Users/ines_maria/tock_wyliodrin/clona/tock//target/thumbv6m-none-eabi/release/pico_explorer_base-app.elf[.apps]: section has no contents

This seems to be a regression bug due the merge of #4169 which sets the .apps section to NOLOAD.

After further inquiry, it seems that the linker is not able to relocate the .relocate section. I am not an expert in liker files, and I think this should not happen as the 8000 .relocate section seems to be loaded in RAM.

Putting .apps after .relocate seems to fix the issue.

Furthermore, it seems that gdb and probe-rs refuse to load sections which are not tagged as ALLOC.

Testing Strategy

This pull request was tested by @inesmaria08 using a Pico Explorer Base.

TODO or Help Wanted

Feedback regarding the linker script.

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make prepush.

bradjc
bradjc previously approved these changes Oct 9, 2024
View reviewed changes
Copy link
Contributor
@bradjc bradjc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't explain why this works but it seems like an improvement over what we have.

ppannuto
ppannuto requested changes Oct 9, 2024
View reviewed changes
Copy link
Member
@ppannuto ppannuto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also have no explanation for why this should make a difference, but working code > not-working code.

Can you update the other boards Makefiles and READMEs that need it to include ,ALLOC, i.e. these:

$ git grep '\.apps=LOAD'
esp32-c3-devkitM-1/Makefile:    $(RISC_PREFIX)-objcopy --set-section-flags .apps=LOAD --update-section .apps=$(APP) $^ $(TOCK_ROOT_DIRECTORY)target/$(TARGET)/release/$(PLATFORM)-app.elf
imxrt1050-evkb/Makefile:        arm-none-eabi-objcopy --set-section-flags .apps=LOAD --update-section .apps=$(APP) $< \
imxrt1050-evkb/README.md:    --set-section-flags .apps=LOAD \
nano_rp2040_connect/Makefile:   arm-none-eabi-objcopy --set-section-flags .apps=LOAD --update-section .apps=$(APP) $(KERNEL) $(KERNEL_WITH_APP)
nucleo_f429zi/README.md:    --set-section-flags .apps=LOAD \
nucleo_f429zi/README.md:        arm-none-eabi-objcopy --set-section-flags .apps=LOAD --update-section .apps=$(APP) $(KERNEL) $(KERNEL_WITH_APP)
nucleo_f446re/README.md:    --set-section-flags .apps=LOAD \
nucleo_f446re/README.md:        arm-none-eabi-objcopy --set-section-flags .apps=LOAD --update-section .apps=$(APP) $(KERNEL) $(KERNEL_WITH_APP)
opentitan/earlgrey-cw310/Makefile:      $(RISC_PREFIX)-objcopy --set-section-flags .apps=LOAD --update-section .apps=$(APP) $^ $(TOCK_ROOT_DIRECTORY)target/$(TARGET)/release/$(PLATFORM)-app.elf
opentitan/earlgrey-cw310/Makefile:              $(RISC_PREFIX)-objcopy --set-section-flags .apps=LOAD --update-section .apps=$(APP) $(TOCK_ROOT_DIRECTORY)target/$(TARGET)/release/$(PLATFORM)_verilator.elf $(TOCK_ROOT_DIRECTORY)target/$(TARGET)/release/$(PLATFORM)_verilator.elf
pico_explorer_base/Makefile:    arm-none-eabi-objcopy --set-section-flags .apps=LOAD --update-section .apps=$(APP) $(KERNEL) $(KERNEL_WITH_APP)
raspberry_pi_pico/Makefile:     arm-none-eabi-objcopy --set-section-flags .apps=LOAD --update-section .apps=$(APP) $(KERNEL) $(KERNEL_WITH_APP)
teensy40/Makefile:      $(Q)arm-none-eabi-objcopy --set-section-flags .apps=LOAD --update-section .apps=$(APP) $< $(TOCK_ROOT_DIRECTORY)target/$(TARGET)/release/$(PLATFORM)-apps.elf
teensy40/README.md:    --set-section-flags .apps=LOAD \
teensy40/README.md:     arm-none-eabi-objcopy --set-section-flags .apps=LOAD --update-section .apps=$(APP) $(KERNEL) $(KERNEL_WITH_APP)
weact_f401ccu6/Makefile:        arm-none-eabi-objcopy --set-section-flags .apps=LOAD --update-section .apps=$(APP) $< $(KERNEL_WITH_APP)
weact_f401ccu6/README.md:    --set-section-flags .apps=LOAD \

@alexandruradovici
Copy link
Contributor Author

Done.

@github-actions github-actions bot added the WG-OpenTitan In the purview of the OpenTitan working group. label Oct 9, 2024
@alexandruradovici
Copy link
Contributor Author

@inesmaria08 can you verify this PR one more time with the Pico and the STM32 board?
@lschuermann are you able to verify some of the rv32 boards?
8000

@inesmaria08
Copy link
Contributor

@inesmaria08 can you verify this PR one more time with the Pico and the STM32 board?

I tested the changes once more on both the Pico and the STM32, and it seems to work.

@lschuermann
Copy link
Member

@alexandruradovici I tried running the above commands on a QEMU rv32 virt kernel:

On current upstream master:

> riscv32-none-elf-objcopy \
       --set-section-flags .apps=LOAD \
       --update-section .apps=app.tbf \
       ../../target/riscv32imac-unknown-none-elf/release/qemu_rv32_virt.elf \
       kernel-with-app.elf
riscv32-none-elf-objcopy: kernel-with-app.elf[.apps]: section has no contents

With this PR:

> gh pr checkout 4191
Switched to branch 'appplication_section_update'
> make
f5048ed823ac172f7182d768b75dd6cc97f5ace8a8a87f50a8906fd874febc79  target/riscv32imac-unknown-none-elf/release/qemu_rv32_virt.bin
> riscv32-none-elf-objcopy \
       --set-section-flags .apps=LOAD \
       --update-section .apps=app.tbf \
       ../../target/riscv32imac-unknown-none-elf/release/qemu_rv32_virt.elf \
       kernel-with-app.elf
riscv32-none-elf-objcopy: kernel-with-app.elf[.apps]: section has no contents

Neither command manages to produce an output file.

That being said, the LiteX checks still work on this PR and I don't think that this method has many applications on RISC-V. Loading more than one app requires careful placement for matching the flash load address of a given app, and making sure that no two apps conflict in their RAM addresses. Tockloader's flash-file backend is a great way to load apps into kernel binaries while maintaining these constraints.

So, while this doesn't seem to change anything for RISC-V, I'm fine with merging this given it improves things over in the ARM world.

@lschuermann
Copy link
Member

Gotta love linkers.

@alexandruradovici
Copy link
Contributor Author
alexandruradovici commented Oct 10, 2024 •
edited
Loading

@lschuermann the command has to be split in two:

> riscv32-none-elf-objcopy \
       --set-section-flags .apps=LOAD,ALLOC \
       ../../target/riscv32imac-unknown-none-elf/release/qemu_rv32_virt.elf \
       kernel-with-app.elf
> riscv32-none-elf-objcopy \
       --update-section .apps=app.tbf
       kernel-with-app.elf

This should be updated in the commit that I sent yesterday.

@alexandruradovici
Copy link
Contributor Author
alexandruradovici commented Oct 10, 2024 •
edited
Loading

I am not sure why the size check fails.

@lschuermann
Copy link
Member

@lschuermann the command has to be split in two

Gotcha, I missed that change -- will try that again.

I'm wondering though whether it makes sense to guide users to this ELF hackery when we have the flash-file backend in Tockloader that can do all of this, and work well with multiple apps across both ARM and RISC-V. Independent of the fact that this is a good fix, should we update the READMEs to point to Tockloader instead?

@alexandruradovici
Copy link
Contributor Author
alexandruradovici commented Oct 10, 2024 •
edited
Loading

I'm wondering though whether it makes sense to guide users to this ELF hackery when we have the flash-file backend in Tockloader that can do all of this, and work well with multiple apps across both ARM and RISC-V. Independent of the fact that this is a good fix, should we update the READMEs to point to Tockloader instead?

You are mostly right, the problem is that we have a couple of boards like the STM32s and the Raspberry Pi Pico (without debugger) where tockloader does not work:

  • for STM32 works partially - only with bundle apps
  • for the Raspberry Pi Pico without debugger (which is more expensive than the board) the only way of flashing is using by copying the uf2 file to the boot loader's partition

@bradjc
Copy link
Contributor
bradjc commented Oct 10, 2024
  • for the Raspberry Pi Pico without debugger (which is more expensive than the board) the only way of flashing is using by copying the uf2 file to the boot loader's partition

This does seem like another good use of flash file, but we would need to add uf2 file support somehow.

@alexandruradovici alexandruradovici added the last-call Final review period for a pull request. label Oct 10, 2024
@alexandruradovici
Copy link
Contributor Author

This does seem like another good use of flash file, but we would need to add uf2 file support somehow.

Are you referring to generating a uf2 output? That is simple to do.

@alevy alevy added this pull request to the merge queue Oct 11, 2024
Merged via the queue into tock:master with commit 948bfe0 Oct 11, 2024
12 checks passed
@lschuermann lschuermann mentioned this pull request Oct 12, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alevy alevy alevy approved these changes

@ppannuto ppannuto ppannuto approved these changes

@bradjc bradjc bradjc approved these changes

@lschuermann lschuermann lschuermann approved these changes

Assignees

@jrvanwhy jrvanwhy

Labels
last-call Final review period for a pull request. WG-OpenTitan In the purview of the OpenTitan working group.
Projects
None yet
Milestone
No milestone
4432
Development

Successfully merging this pull request may close these issues.
7 participants
@alexandruradovici @inesmaria08 @lschuermann @bradjc @alevy @ppannuto @jrvanwhy
0