Skip to content

Commit c018c51

Browse files
ludfjigludfjig
committed
Clean up shared PT iterators: restore sealed TableMovability, export arch constants, remove PTE_SHIFT, restore comments
Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
1 parent 48b9af6 commit c018c51

5 files changed

Lines changed: 158 additions & 98 deletions

File tree

‎src/hyperlight_common/src/arch/aarch64/vmem.rs‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ use crate::vmem::{Mapping, TableOps, TableReadOps, Void};
2121
pub const PAGE_SIZE: usize = 4096;
2222
pub const PAGE_TABLE_SIZE: usize = 4096;
2323
pub const PAGE_PRESENT: u64 = 1; // AArch64: bit 0 is the "valid" bit
24+
pub const PTE_ADDR_MASK: u64 = 0x0000_FFFF_FFFF_F000; // bits [47:12]
2425
pub type PageTableEntry = u64;
2526
pub type VirtAddr = u64;
2627
pub type PhysAddr = u64;
@@ -45,17 +46,9 @@ pub unsafe fn virt_to_phys<'a, Op: TableReadOps + 'a>(
4546
core::iter::empty()
4647
}
4748

48-
impl<Op: TableOps<TableMovability = crate::vmem::MayMoveTable>> crate::vmem::TableMovability<Op>
49+
pub trait TableMovability<Op: TableReadOps + ?Sized, TableMoveInfo> {}
50+
impl<Op: TableOps<TableMovability = crate::vmem::MayMoveTable>> TableMovability<Op, Op::TableAddr>
4951
for crate::vmem::MayMoveTable
5052
{
51-
type RootUpdateParent = crate::vmem::UpdateParentRoot;
52-
fn root_update_parent() -> Self::RootUpdateParent {
53-
crate::vmem::UpdateParentRoot {}
54-
}
55-
}
56-
impl<Op: TableReadOps> crate::vmem::TableMovability<Op> for crate::vmem::MayNotMoveTable {
57-
type RootUpdateParent = crate::vmem::UpdateParentNone;
58-
fn root_update_parent() -> Self::RootUpdateParent {
59-
crate::vmem::UpdateParentNone {}
60-
}
6153
}
54+
impl<Op: TableReadOps> TableMovability<Op, Void> for crate::vmem::MayNotMoveTable {}

‎src/hyperlight_common/src/arch/amd64/vmem.rs‎

Lines changed: 28 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ limitations under the License.
2626
//! allocating intermediate tables as needed and setting appropriate flags on leaf PTEs
2727
2828
use crate::vmem::{
29-
BasicMapping, CowMapping, MapRequest, MapResponse, Mapping, MappingKind, TableMovability as _,
30-
TableMovabilityBase, TableOps, TableReadOps, UpdateParent, UpdateParentNone, UpdateParentRoot,
31-
UpdateParentTable, modify_ptes, read_pte_if_present, require_pte_exist, write_entry_updating,
29+
BasicMapping, CowMapping, MapRequest, MapResponse, Mapping, MappingKind, TableMovabilityBase,
30+
TableOps, TableReadOps, UpdateParent, UpdateParentNone, UpdateParentRoot, UpdateParentTable,
31+
modify_ptes, read_pte_if_present, require_pte_exist, write_entry_updating,
3232
};
3333

3434
// Paging Flags
@@ -58,7 +58,7 @@ const PAGE_RW: u64 = 1 << 1;
5858
const PAGE_NX: u64 = 1 << 63;
5959
/// Mask to extract the physical address from a PTE (bits 51:12)
6060
/// This masks out the lower 12 flag bits AND the upper bits including NX (bit 63)
61-
const PTE_ADDR_MASK: u64 = 0x000F_FFFF_FFFF_F000;
61+
pub const PTE_ADDR_MASK: u64 = 0x000F_FFFF_FFFF_F000;
6262
const PAGE_USER_ACCESS_DISABLED: u64 = 0 << 2; // U/S bit not set - supervisor mode only (no code runs in user mode for now)
6363
const PAGE_DIRTY_SET: u64 = 1 << 6; // D - dirty bit
6464
const PAGE_ACCESSED_SET: u64 = 1 << 5; // A - accessed bit
@@ -96,17 +96,22 @@ fn pte_for_table<Op: TableOps>(table_addr: Op::TableAddr) -> u64 {
9696
PAGE_PRESENT // P - this entry is present
9797
}
9898

99-
// ---- TableMovability + UpdateParent impls ----
100-
101-
impl<Op: TableOps<TableMovability = crate::vmem::MayMoveTable>> crate::vmem::TableMovability<Op>
99+
/// This trait is used to select appropriate implementations of
100+
/// [`UpdateParent`] to be used, depending on whether a particular
101+
/// implementation needs the ability to move tables.
102+
pub trait TableMovability<Op: TableReadOps + ?Sized, TableMoveInfo> {
103+
type RootUpdateParent: UpdateParent<Op, TableMoveInfo = TableMoveInfo>;
104+
fn root_update_parent() -> Self::RootUpdateParent;
105+
}
106+
impl<Op: TableOps<TableMovability = crate::vmem::MayMoveTable>> TableMovability<Op, Op::TableAddr>
102107
for crate::vmem::MayMoveTable
103108
{
104109
type RootUpdateParent = UpdateParentRoot;
105110
fn root_update_parent() -> Self::RootUpdateParent {
106111
UpdateParentRoot {}
107112
}
108113
}
109-
impl<Op: TableReadOps> crate::vmem::TableMovability<Op> for crate::vmem::MayNotMoveTable {
114+
impl<Op: TableReadOps> TableMovability<Op, crate::vmem::Void> for crate::vmem::MayNotMoveTable {
110115
type RootUpdateParent = UpdateParentNone;
111116
fn root_update_parent() -> Self::RootUpdateParent {
112117
UpdateParentNone {}
@@ -146,9 +151,6 @@ impl<Op: TableOps<TableMovability = crate::vmem::MayMoveTable>> crate::vmem::Upd
146151
}
147152
}
148153

149-
/// amd64 PTE shift: log2(8) = 3 for 8-byte entries
150-
const PTE_SHIFT: u8 = 3;
151-
152154
/// Page-mapping callback to allocate a next-level page table if necessary.
153155
/// # Safety
154156
/// This function modifies page table data structures, and should not be called concurrently
@@ -265,18 +267,18 @@ unsafe fn map_page<
265267
/// 4. PT (20:12) - write final PTE with physical address and flags
266268
#[allow(clippy::missing_safety_doc)]
267269
pub unsafe fn map<Op: TableOps>(op: &Op, mapping: Mapping) {
268-
modify_ptes::<47, 39, PTE_SHIFT, Op, _>(MapRequest {
270+
modify_ptes::<47, 39, Op, _>(MapRequest {
269271
table_base: op.root_table(),
270272
vmin: mapping.virt_base,
271273
len: mapping.len,
272274
update_parent: Op::TableMovability::root_update_parent(),
273275
})
274276
.map(|r| unsafe { alloc_pte_if_needed(op, r) })
275-
.flat_map(modify_ptes::<38, 30, PTE_SHIFT, Op, _>)
277+
.flat_map(modify_ptes::<38, 30, Op, _>)
276278
.map(|r| unsafe { alloc_pte_if_needed(op, r) })
277-
.flat_map(modify_ptes::<29, 21, PTE_SHIFT, Op, _>)
279+
.flat_map(modify_ptes::<29, 21, Op, _>)
278280
.map(|r| unsafe { alloc_pte_if_needed(op, r) })
279-
.flat_map(modify_ptes::<20, 12, PTE_SHIFT, Op, _>)
281+
.flat_map(modify_ptes::<20, 12, Op, _>)
280282
.map(|r| unsafe { map_page(op, &mapping, r) })
281283
.for_each(drop);
282284
}
@@ -311,18 +313,18 @@ pub unsafe fn virt_to_phys<'a, Op: TableReadOps + 'a>(
311313
// Calculate the maximum virtual address we need to look at based on the starting
312314
// address and length ensuring we don't go past the end of the address space
313315
let vmax = core::cmp::min(addr + len, 1u64 << VA_BITS);
314-
modify_ptes::<47, 39, PTE_SHIFT, Op, _>(MapRequest {
316+
modify_ptes::<47, 39, Op, _>(MapRequest {
315317
table_base: op.as_ref().root_table(),
316318
vmin,
317319
len: vmax - vmin,
318320
update_parent: UpdateParentNone {},
319321
})
320-
.filter_map(move |r| unsafe { require_pte_exist::<PTE_ADDR_MASK, _, _>(op.as_ref(), r) })
321-
.flat_map(modify_ptes::<38, 30, PTE_SHIFT, Op, _>)
322-
.filter_map(move |r| unsafe { require_pte_exist::<PTE_ADDR_MASK, _, _>(op.as_ref(), r) })
323-
.flat_map(modify_ptes::<29, 21, PTE_SHIFT, Op, _>)
324-
.filter_map(move |r| unsafe { require_pte_exist::<PTE_ADDR_MASK, _, _>(op.as_ref(), r) })
325-
.flat_map(modify_ptes::<20, 12, PTE_SHIFT, Op, _>)
322+
.filter_map(move |r| unsafe { require_pte_exist(op.as_ref(), r) })
323+
.flat_map(modify_ptes::<38, 30, Op, _>)
324+
.filter_map(move |r| unsafe { require_pte_exist(op.as_ref(), r) })
325+
.flat_map(modify_ptes::<29, 21, Op, _>)
326+
.filter_map(move |r| unsafe { require_pte_exist(op.as_ref(), r) })
327+
.flat_map(modify_ptes::<20, 12, Op, _>)
326328
.filter_map(move |r| {
327329
let pte = unsafe { read_pte_if_present(op.as_ref(), r.entry_ptr) }?;
328330
let phys_addr = pte & PTE_ADDR_MASK;
@@ -839,8 +841,7 @@ mod tests {
839841
update_parent: UpdateParentNone {},
840842
};
841843

842-
let responses: Vec<_> =
843-
modify_ptes::<20, 12, PTE_SHIFT, MockTableOps, _>(request).collect();
844+
let responses: Vec<_> = modify_ptes::<20, 12, MockTableOps, _>(request).collect();
844845
assert_eq!(responses.len(), 1, "Single page should yield one response");
845846
assert_eq!(responses[0].vmin, 0x1000);
846847
assert_eq!(responses[0].len, PAGE_SIZE as u64);
@@ -856,8 +857,7 @@ mod tests {
856857
update_parent: UpdateParentNone {},
857858
};
858859

859-
let responses: Vec<_> =
860-
modify_ptes::<20, 12, PTE_SHIFT, MockTableOps, _>(request).collect();
860+
let responses: Vec<_> = modify_ptes::<20, 12, MockTableOps, _>(request).collect();
861861
assert_eq!(responses.len(), 3, "3 pages should yield 3 responses");
862862
}
863863

@@ -871,8 +871,7 @@ mod tests {
871871
update_parent: UpdateParentNone {},
872872
};
873873

874-
let responses: Vec<_> =
875-
modify_ptes::<20, 12, PTE_SHIFT, MockTableOps, _>(request).collect();
874+
let responses: Vec<_> = modify_ptes::<20, 12, MockTableOps, _>(request).collect();
876875
assert_eq!(responses.len(), 0, "Zero length should yield no responses");
877876
}
878877

@@ -888,8 +887,7 @@ mod tests {
888887
update_parent: UpdateParentNone {},
889888
};
890889

891-
let responses: Vec<_> =
892-
modify_ptes::<20, 12, PTE_SHIFT, MockTableOps, _>(request).collect();
890+
let responses: Vec<_> = modify_ptes::<20, 12, MockTableOps, _>(request).collect();
893891
assert_eq!(
894892
responses.len(),
895893
2,

‎src/hyperlight_common/src/arch/i686/vmem.rs‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ limitations under the License.
2222
//! Entries are 4 bytes wide. There is no NX bit; all pages are executable.
2323
2424
use crate::vmem::{
25-
BasicMapping, CowMapping, MapRequest, MapResponse, Mapping, MappingKind, TableMovability as _,
26-
TableMovabilityBase, TableOps, TableReadOps, UpdateParent, UpdateParentNone, modify_ptes,
27-
read_pte_if_present, require_pte_exist, write_entry_updating,
25+
BasicMapping, CowMapping, MapRequest, MapResponse, Mapping, MappingKind, TableMovabilityBase,
26+
TableOps, TableReadOps, UpdateParent, UpdateParentNone, modify_ptes, read_pte_if_present,
27+
require_pte_exist, write_entry_updating,
2828
};
2929

3030
pub const PAGE_SIZE: usize = 4096;
@@ -38,15 +38,18 @@ pub const PAGE_PRESENT: u64 = 1;
3838
const PAGE_RW: u64 = 1 << 1;
3939
pub const PAGE_USER: u64 = 1 << 2;
4040
const PAGE_ACCESSED: u64 = 1 << 5;
41-
const PTE_ADDR_MASK: u64 = 0xFFFFF000;
41+
pub const PTE_ADDR_MASK: u64 = 0xFFFFF000;
4242
const PTE_AVL_MASK: u64 = 0x0E00;
4343
const PAGE_AVL_COW: u64 = 1 << 9;
4444

4545
const VA_BITS: usize = 32;
46-
/// log2(4) for 4-byte entries
47-
const PTE_SHIFT: u8 = 2;
4846

49-
impl<Op: TableReadOps> crate::vmem::TableMovability<Op> for crate::vmem::MayNotMoveTable {
47+
pub trait TableMovability<Op: TableReadOps + ?Sized, TableMoveInfo> {
48+
type RootUpdateParent: UpdateParent<Op, TableMoveInfo = TableMoveInfo>;
49+
fn root_update_parent() -> Self::RootUpdateParent;
50+
}
51+
52+
impl<Op: TableReadOps> TableMovability<Op, crate::vmem::Void> for crate::vmem::MayNotMoveTable {
5053
type RootUpdateParent = UpdateParentNone;
5154
fn root_update_parent() -> Self::RootUpdateParent {
5255
UpdateParentNone {}
@@ -156,14 +159,14 @@ unsafe fn map_page<
156159
/// See [`crate::vmem::map`].
157160
#[allow(clippy::missing_safety_doc)]
158161
pub unsafe fn map<Op: TableOps>(op: &Op, mapping: Mapping) {
159-
modify_ptes::<31, 22, PTE_SHIFT, Op, _>(MapRequest {
162+
modify_ptes::<31, 22, Op, _>(MapRequest {
160163
table_base: op.root_table(),
161164
vmin: mapping.virt_base,
162165
len: mapping.len,
163166
update_parent: Op::TableMovability::root_update_parent(),
164167
})
165168
.map(|r| unsafe { alloc_pte_if_needed(op, r) })
166-
.flat_map(modify_ptes::<21, 12, PTE_SHIFT, Op, _>)
169+
.flat_map(modify_ptes::<21, 12, Op, _>)
167170
.map(|r| unsafe { map_page(op, &mapping, r) })
168171
.for_each(drop);
169172
}
@@ -180,14 +183,14 @@ pub unsafe fn virt_to_phys<'a, Op: TableReadOps + 'a>(
180183
) -> impl Iterator<Item = Mapping> + 'a {
181184
let vmin = address & !(PAGE_SIZE as u64 - 1);
182185
let vmax = core::cmp::min(address + len, 1u64 << VA_BITS);
183-
modify_ptes::<31, 22, PTE_SHIFT, Op, _>(MapRequest {
186+
modify_ptes::<31, 22, Op, _>(MapRequest {
184187
table_base: op.as_ref().root_table(),
185188
vmin,
186189
len: vmax.saturating_sub(vmin),
187190
update_parent: UpdateParentNone {},
188191
})
189-
.filter_map(move |r| unsafe { require_pte_exist::<PTE_ADDR_MASK, _, _>(op.as_ref(), r) })
190-
.flat_map(modify_ptes::<21, 12, PTE_SHIFT, Op, _>)
192+
.filter_map(move |r| unsafe { require_pte_exist(op.as_ref(), r) })
193+
.flat_map(modify_ptes::<21, 12, Op, _>)
191194
.filter_map(move |r| {
192195
let pte = unsafe { read_pte_if_present(op.as_ref(), r.entry_ptr) }?;
193196
let phys_addr = pte & PTE_ADDR_MASK;

0 commit comments

Comments
 (0)