-
Notifications
You must be signed in to change notification settings - Fork 58
Move Note and GDT definitions into Rust #37
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
1c47247
layout: Improve development workflow
josephlr 0f813a5
layout: Add explict .stack section
josephlr 70ee5b7
pvh: Move note definition/declaration to Rust
josephlr f7f5c6d
gdt: Move GDT and GDT Definitions to Rust
josephlr ae04b87
Fix typos and add comments
josephlr File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,3 @@ | ||
global_asm!(include_str!("note.s")); | ||
global_asm!(include_str!("ram32.s")); | ||
global_asm!(include_str!("ram64.s")); | ||
global_asm!(include_str!("gdt64.s")); |
This file was deleted.
Oops, something went wrong.
This file contains hidden or 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly when it was 4 bytes that forced a change in the Cloud Hypervisor PVH code to support it. It's ambiguous to me what it should be. But if it works... :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The size of Desc seems to depend on how the code in question is compiled. Linux sets a field of type
_ASM_PTR
, which is 8 bytes (at least on my kernel). The PVH Specification seems to indicate the type should be.long
aka 4 bytes. I think it's fine for hypervisors to just support any size <= 8 for this field. QEMU just does a raw read of asize_t
without checkingdescsz
.There type of
namesz
,descsz
, andkind
seems to usually beu32
(even for 64-bit ELF). That's what Linux does, NetBSD does, and what Oracle says to do. However, the gABI says they should beu64
s but that seems to get ignored. See this mailing list discussion for more info.The final complexity is alignment of the structure (and hence the containing phdr's alignment). Some loaders expect this to always be 4, others allow for other alignments but compute the structure padding incorrectly:
linux-loader
crate used bycloud-hypervisor
also does it wrong.Using an alignment of 4 prevents any issues (as there will just not be any padding). This is what Linux does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a comment about alignment to
pvh.rs
.