Extract all layout related information into a MemoryLayout trait#1409
Extract all layout related information into a MemoryLayout trait#1409jounathaen wants to merge 3 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1409 +/- ##
==========================================
+ Coverage 79.50% 79.51% +0.01%
==========================================
Files 38 39 +1
Lines 5464 5536 +72
==========================================
+ Hits 4344 4402 +58
- Misses 1120 1134 +14 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
cabfc28 to
33c6e4d
Compare
jounathaen
changed the title
354b962 to
71859c1
Compare
fogti
left a comment
fogti
left a comment
There was a problem hiding this comment.
I'm a bit confused about where guest_address additions were removed and where they weren't, particularly in page table initialization code...
| let pt_size = 512 * size_of::<u64>(); | ||
| assert!(mem.guest_addr() + mem.size() >= layout.pgt_address() + pt_size); |
There was a problem hiding this comment.
| let pt_size = 512 * size_of::<u64>(); | |
| assert!(mem.guest_addr() + mem.size() >= layout.pgt_address() + pt_size); | |
| const PT_SIZE: usize = 512 * size_of::<u64>(); | |
| assert!(mem.guest_addr() + mem.size() >= layout.pgt_address() + PT_SIZE); |
| self.addr | ||
| } | ||
| // "no code using it on aarch64 (yet)" | ||
| pub fn end(&self) -> GuestPhysAddr { |
There was a problem hiding this comment.
This looks wrong for any underlying T with core::mem::size_of::<T>() != 1.
To ensure correct usage, Section should be tagged with the underlying slice type T using PhantomData, and use self.addr + self.length * core::mem::size_of::<T>() below.
|
|
||
| fn boot_info(&self) -> BootInfoSection; | ||
|
|
||
| /// The Starting position of the allocatable pagetables area. This doesn't need to include all pagetables. |
There was a problem hiding this comment.
What's the meaning of the length of the returned section? Otherwise, just return an address...
| }); | ||
|
|
||
| let legacy_mapping = if let Some(version) = hermit_version { | ||
| // actually, all versions that have the tag in the elf are valid, but an explicit check doesn't hurt |
There was a problem hiding this comment.
What does "valid" mean in his context?
| }; | ||
| trace!("Initialize guest memory"); | ||
| VirtBackend::init_guest_mem( | ||
| &mut mem, // slice only lives during this fn call |
2 participants
The impls. of
MemoryLayout(Aarch64MemoryLayoutandX86_64MemoryLayout) are basically duplicates, but semantically this makes more sense and also gives us more freedom to modify the layout for architecture-specific features.Closes #1303