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

TBF: Add "Fixed Addresses" TLV #1845

Merged
merged 8 commits into from May 19, 2020
Merged

TBF: Add "Fixed Addresses" TLV #1845

merged 8 commits into from May 19, 2020

Conversation

bradjc
Copy link
Contributor

@bradjc bradjc commented May 12, 2020

Pull Request Overview

Part of: #1283 and #1465

This adds a new TLV header (FixedAddresses) for processes to specify the addresses in flash and ram they need (if they require fixed addresses, i.e. are not position independent. The kernel then checks these addresses when creating processes, and throws errors if they are not met. Note: this does not change the kernel to try to give a process its requested memory address.

The main benefit of this is that non-PIC apps can use this header and the kernel can catch errors when loading processes. Right now misalignments cause other errors to pop up once a process is running, and these are difficult to debug.

I have related changes to elf2tab and tockloader in progress, but will wait to PR those until any changes are ironed out here.

Testing Strategy

Adding the headers to Hail and verifying that apps work when they match and an error is displayed when they do not.

TODO or Help Wanted

  • Is this the right TLV header structure to use?
  • Does this break any 1.0 promises?
    • I don't think so, as it doesn't change any existing headers and any existing apps won't change, so old apps will run just fine. In particular, ARM libtock-c apps won't change at all.

Documentation Updated

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

Formatting

  • Ran make formatall.

Unsafe not needed for parsing, make it more official and clear.
If the process included fixed addresses in its header, make sure those
are used, otherwise return an error.

Note, this does not yet actually try to use the correct address (i.e. in
ram) that the kernel could try to optimize for.
Allows processes to communicate to the kernel they need a very specific
address.
@bradjc bradjc added kernel P-Significant This is a substancial change that requires review from all core developers. labels May 12, 2020
doc/TockBinaryFormat.md Show resolved Hide resolved
kernel/src/process.rs Outdated Show resolved Hide resolved
kernel/src/process.rs Outdated Show resolved Hide resolved
kernel/src/tbfheader.rs Show resolved Hide resolved
kernel/src/tbfheader.rs Outdated Show resolved Hide resolved
@bradjc
Copy link
Contributor Author

bradjc commented May 13, 2020

Updated.

ppannuto
ppannuto previously approved these changes May 13, 2020
kernel/src/tbfheader.rs Outdated Show resolved Hide resolved
jrvanwhy
jrvanwhy previously approved these changes May 13, 2020
Co-authored-by: Pat Pannuto <pat.pannuto@gmail.com>
@bradjc bradjc dismissed stale reviews from jrvanwhy and ppannuto via 981ef6d May 13, 2020 16:10
ppannuto
ppannuto previously approved these changes May 13, 2020
@bradjc bradjc mentioned this pull request May 13, 2020
6 tasks
@bradjc
Copy link
Contributor Author

bradjc commented May 18, 2020

Any other comments @tock/core-wg?

@ppannuto ppannuto added the last-call Final review period for a pull request. label May 18, 2020
@ppannuto
Copy link
Member

bors r+

@bors bors bot merged commit c075d7b into master May 19, 2020
@bors bors bot deleted the tbf-header-locations branch May 19, 2020 14:20
bors bot added a commit to tock/libtock-rs that referenced this pull request Jul 7, 2020
208: linker: add sram origin symbol r=bradjc a=bradjc

This allows elf2tab to determine what the start of RAM address is when generating a fixed address header in the TBF.

See tock/elf2tab#23 and tock/tock#1845.

Co-authored-by: Brad Campbell <bradjc5@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kernel last-call Final review period for a pull request. P-Significant This is a substancial change that requires review from all core developers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants