Conversation
m4tx
force-pushed
the
stage-2
branch
22 times, most recently
from
d91e3dc to
32d3e8a
Compare
src/hypervisor.rs
Outdated
|
|
||
| fn setup_stage2() { | ||
| debug!("Setting up stage 2 page table"); | ||
| let idmap = Box::new(PlatformImpl::make_stage2_pagetable()); |
There was a problem hiding this comment.
You could avoid the heap allocation by putting it in a static spin::Once instead.
There was a problem hiding this comment.
aarch64-paging still does heap allocation internally (https://github.com/google/aarch64-paging/blob/e36465b1b557303aed5474ba6f840fda65275598/src/paging.rs#L869), but sure - it's good to remove as many heap allocations as possible.
EDIT: the current code actually causes a memory leak - as this function is invoked on every CPU_ON or return from CPU_SUSPEND. Even better reason to use Once.
src/main.rs
Outdated
| const HEAP_SIZE: usize = 40 * PAGE_SIZE; | ||
| static HEAP: SpinMutex<[u8; HEAP_SIZE]> = SpinMutex::new([0; HEAP_SIZE]); | ||
|
|
||
| const SHARED_HEAP_SIZE: usize = 16 * PAGE_SIZE; |
There was a problem hiding this comment.
Will different platforms need different heap sizes?
There was a problem hiding this comment.
Right, for real devices 16 * 4kB might be insufficient even to store the modified DTB. It doesn't hurt to move this constant to the Platform trait.
src/main.rs
Outdated
| /// Panics if the requested size is invalid or if the allocation fails. | ||
| pub fn shared_alloc(size: usize) -> &'static mut [u8] { | ||
| use core::alloc::Layout; | ||
| let layout = Layout::from_size_align(size, PAGE_SIZE).expect("invalid layout"); |
There was a problem hiding this comment.
Why is the alignment fixed at PAGE_SIZE?
There was a problem hiding this comment.
Sorry, a dirty leftover from the early versions of the code. Updated the function so that Layout can be passed instead. For the FDT itself, using 8 byte alignment, as per the docs: https://docs.kernel.org/arch/arm64/booting.html#setup-the-device-tree
This makes the RITM memory invisible from the guest.
|
|
||
| // Linux requires the device tree to be "placed on an 8-byte boundary": | ||
| // https://docs.kernel.org/arch/arm64/booting.html#setup-the-device-tree | ||
| const FDT_ALIGNMENT: usize = 8; |
There was a problem hiding this comment.
Maybe this constant should be in a common module, not platform-specific.
| .union(Stage2Attributes::MEMATTR_NORMAL) | ||
| .union(Stage2Attributes::S2AP_ACCESS_RW) | ||
| .union(Stage2Attributes::ACCESS_FLAG) | ||
| .union(Stage2Attributes::SH_INNER); |
There was a problem hiding this comment.
Do these memory attributes need to be platform-specific?
| // High MMIO | ||
| // We split this into two ranges to avoid mapping a full L0 entry (512 GiB) as a single | ||
| // block, which is not supported by the architecture (L0 blocks are not supported with 4 KiB | ||
| // pages without enabling FEAT_LPA2) |
There was a problem hiding this comment.
We should probably add some logic in aarch64-paging to avoid this, perhaps with a flag.
2 participants
This makes the RITM memory invisible from the guest.