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

arch/libcontext: arm64: Make TCB overlap configurable #720

Closed

Conversation

eduardvintila
Copy link
Member

Prerequisite checklist

  • Read the contribution guidelines regarding submitting new changes to the project;
  • Tested your changes against relevant architectures and platforms;
  • Ran the checkpatch.pl on your commit series before opening this PR;
  • Updated relevant documentation.

Base target

  • Architecture(s): [aarch64]
  • Platform(s): [kvm]
  • Application(s): [N/A]

Additional configuration

Description of changes

The Aarch64 ABI requires 16 bytes of space after the tlsp pointer as part of the TCB. However, this space is not expected by newer versions of the musl library.

As such, this PR introduces a new Makefile rule by which a library can register whether this reserved space is not necessary. The rule sets a macro by which we can then decide to assume a 16-byte block space is reserved at the end of the TCB or not.

This maintains backwards compatibility with previous versions of musl, since if the rule is not invoked, it is assumed that the library expects the AARCH64_RESERVED block of 16 bytes described by the ABI.

Signed-off-by: Eduard Vintilă eduard.vintila47@gmail.com

arch/arm/arm64/tls.c Show resolved Hide resolved
arch/Makefile.rules Outdated Show resolved Hide resolved
@eduardvintila
Copy link
Member Author

Hey, @kubanrob! Thanks a lot for pointing this out! I have updated my PR accordingly.

Copy link
Contributor

@kubanrob kubanrob left a comment

Choose a reason for hiding this comment

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

I stumbled over one more comment that might needs to be updated, otherwise I think the PR looks good!

arch/arm/arm64/tls.c Show resolved Hide resolved
The Aarch64 ABI requires 16 bytes of space after the `tlsp` pointer as
part of the TCB. However, this space is not expected by newer versions
of the `musl` library.

As such, this commit introduces a new Makefile rule by which a library
can register whether this reserved space is not necessary. The rule sets
a macro by which we can then decide to assume a 16-byte
block space is reserved at the end of the TCB or not.

This maintains backwards compatibility with previous versions of `musl`,
since if the rule is not invoked, it is assumed that the library expects
the `AARCH64_RESERVED` block of 16 bytes described by the ABI.

Signed-off-by: Eduard Vintilă <eduard.vintila47@gmail.com>
@eduardvintila
Copy link
Member Author

Hey, @kubanrob. Great catch, thanks! I've updated that comment, as well.

@unikraft-bot
Copy link
Member

Checkpatch passed

Beep boop! I ran Unikraft's checkpatch.pl support script on your pull request and it all looks good!

SHA commit checkpatch
b807df1 arch/libcontext: arm64: Make TCB overlap configurable

Copy link
Contributor

@kubanrob kubanrob left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you @eduardvintila!

Reviewed-by: Robert Kuban robert.kuban@opensynergy.com

Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

Approved-by: Razvan Deaconescu razvand@unikraft.io

@unikraft-bot unikraft-bot added the ci/merged Merged by CI label Jan 31, 2023
noureddine-taleb pushed a commit to noureddine-taleb/unikraft that referenced this pull request Mar 23, 2023
The Aarch64 ABI requires 16 bytes of space after the `tlsp` pointer as
part of the TCB. However, this space is not expected by newer versions
of the `musl` library.

As such, this commit introduces a new Makefile rule by which a library
can register whether this reserved space is not necessary. The rule sets
a macro by which we can then decide to assume a 16-byte
block space is reserved at the end of the TCB or not.

This maintains backwards compatibility with previous versions of `musl`,
since if the rule is not invoked, it is assumed that the library expects
the `AARCH64_RESERVED` block of 16 bytes described by the ABI.

Signed-off-by: Eduard Vintilă <eduard.vintila47@gmail.com>
Reviewed-by: Robert Kuban <robert.kuban@opensynergy.com>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: unikraft#720
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch/arm arch/arm64 area/arch Unikraft Architecture ci/merged Merged by CI lang/c Issues or PRs to do with C/C++
Projects
Status: Done!
Development

Successfully merging this pull request may close these issues.

4 participants