arch/arm/arm64: Fix stxr's used in spinlocks register size
#907
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Our `spinlock`s have a size of 4-bytes and an alignment of 8, which, in our `semaphore` implementation, causes a padding of 4 bytes. This is fine, but it is an inconsistency. With `QEMU` direct kernel boot, all of our free memory is zeroed out. However, when using a previous boot phase such as `UEFI`, this inconsistency shows itself through an always locked `spinlock`. `UEFI` firmware "poisons" all of its free memory with `0xaf` bytes which leads to our `stxr`, whose second register argument is 64-bit instead of 32-bit, also atomically storing the `poisoned` padding. Thus, make sure that our `stxr` uses 32-bit register for the to be transferred register. Signed-off-by: Sergiu Moga <sergiu.moga@protonmail.com> GitHub-Fixes: unikraft#289
This was referenced May 25, 2023
Closed
razvand
requested review from
eduardvintila and
mariasfiraiala
and removed request for
a team
razvand
added
area/arch
mogasergiu
added a commit
to mogasergiu/unikraft
that referenced
this pull request
Jun 22, 2023
This was referenced Jun 22, 2023
mariasfiraiala
approved these changes
Jul 5, 2023
There was a problem hiding this comment.
This looks good, thanks @mogasergiu!
Reviewed-by: Maria Sfiraiala maria.sfiraiala@gmail.com
|
@eduardvintila please let me know if you're planning to have a look at this PR, otherwise I'm ready to merge. |
eduardvintila
approved these changes
Jul 5, 2023
There was a problem hiding this comment.
All good. Nice catch, @mogasergiu!
Reviewed-by: Eduard Vintilă eduard.vintila47@gmail.com
|
Approved-by: Michalis Pappas michalis@unikraft.io |
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Our
spinlocks have a size of 4-bytes and an alignment of 8, which, in oursemaphoreimplementation, causes a padding of 4 bytes. This is fine, but it is an inconsistency.With
QEMUdirect kernel boot, all of our free memory is zeroed out. However, when using a previous boot phase such asUEFI, this inconsistency shows itself through an always lockedspinlock.UEFIfirmware "poisons" all of its free memory with0xafbytes which leads to ourstxr, whose second register argument is 64-bit instead of 32-bit, also atomically storing thepoisonedpadding.Thus, make sure that our
stxruses 32-bit register for the to be transferred register.GitHub-Fixes: #289
Prerequisite checklist
checkpatch.plon your commit series before opening this PR;Base target
arm64kvmAdditional configuration
Description of changes