From d02312055ed202ffc213da8b19c415fb36c48145 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 28 Mar 2026 22:35:26 +0100 Subject: [PATCH] rmm: Couple of minor cleanups --- rmm/src/page/mapper.rs | 96 ++++++++++++++++++++---------------------- rmm/src/page/table.rs | 27 ++---------- 2 files changed, 49 insertions(+), 74 deletions(-) diff --git a/rmm/src/page/mapper.rs b/rmm/src/page/mapper.rs index b97d21543f..703262b5b9 100644 --- a/rmm/src/page/mapper.rs +++ b/rmm/src/page/mapper.rs @@ -13,7 +13,7 @@ pub struct PageMapper { } impl PageMapper { - pub unsafe fn new(table_kind: TableKind, table_addr: PhysicalAddress, allocator: F) -> Self { + unsafe fn new(table_kind: TableKind, table_addr: PhysicalAddress, allocator: F) -> Self { Self { table_kind, table_addr, @@ -128,25 +128,24 @@ impl PageMapper { //TODO: check for overwriting entry table.set_entry(i, entry); return Some(PageFlush::new(virt)); - } else { - let next_opt = table.next(i); - let next = match next_opt { - Some(some) => some, - None => { - let next_phys = self.allocator.allocate_one()?; - //TODO: correct flags? - let flags = A::ENTRY_FLAG_DEFAULT_TABLE - | if virt.kind() == TableKind::User { - A::ENTRY_FLAG_TABLE_USER - } else { - 0 - }; - table.set_entry(i, PageEntry::new(next_phys.data(), flags)); - table.next(i)? - } - }; - table = next; } + + let next = match table.next(i) { + Some(some) => some, + None => { + let next_phys = self.allocator.allocate_one()?; + //TODO: correct flags? + let flags = A::ENTRY_FLAG_DEFAULT_TABLE + | if virt.kind() == TableKind::User { + A::ENTRY_FLAG_TABLE_USER + } else { + 0 + }; + table.set_entry(i, PageEntry::new(next_phys.data(), flags)); + table.next(i)? + } + }; + table = next; } } } @@ -166,14 +165,12 @@ impl PageMapper { f: impl FnOnce(&mut PageTable, usize) -> T, ) -> Option { let mut table = self.table(); - unsafe { - loop { - let i = table.index_of(virt)?; - if table.level() == 0 { - return Some(f(&mut table, i)); - } else { - table = table.next(i)?; - } + loop { + let i = table.index_of(virt)?; + if table.level() == 0 { + return Some(f(&mut table, i)); + } else { + table = unsafe { table.next(i)? }; } } } @@ -223,30 +220,29 @@ unsafe fn unmap_phys_inner( table.set_entry(i, PageEntry::new(0, 0)); let entry = entry_opt?; - Some((entry.address().ok()?, entry.flags())) - } else { - let mut subtable = table.next(i)?; - - let res = - unmap_phys_inner(virt, &mut subtable, initial_level, unmap_parents, allocator)?; - - //TODO: This is a bad idea for architectures where the kernel mappings are done in the process tables, - // as these mappings may become out of sync - if unmap_parents { - // TODO: Use a counter? This would reduce the remaining number of available bits, but could be - // faster (benchmark is needed). - let is_still_populated = (0..A::PAGE_ENTRIES) - .map(|j| subtable.entry(j).expect("must be within bounds")) - .any(|e| e.present()); - - if !is_still_populated { - allocator.free_one(subtable.phys()); - table.set_entry(i, PageEntry::new(0, 0)); - } - } - - Some(res) + return Some((entry.address().ok()?, entry.flags())); } + + let mut subtable = table.next(i)?; + + let res = unmap_phys_inner(virt, &mut subtable, initial_level, unmap_parents, allocator)?; + + //TODO: This is a bad idea for architectures where the kernel mappings are done in the process tables, + // as these mappings may become out of sync + if unmap_parents { + // TODO: Use a counter? This would reduce the remaining number of available bits, but could be + // faster (benchmark is needed). + let is_still_populated = (0..A::PAGE_ENTRIES) + .map(|j| subtable.entry(j).expect("must be within bounds")) + .any(|e| e.present()); + + if !is_still_populated { + allocator.free_one(subtable.phys()); + table.set_entry(i, PageEntry::new(0, 0)); + } + } + + Some(res) } } impl core::fmt::Debug for PageMapper { diff --git a/rmm/src/page/table.rs b/rmm/src/page/table.rs index c6d377a28e..f3dad5e225 100644 --- a/rmm/src/page/table.rs +++ b/rmm/src/page/table.rs @@ -1,7 +1,7 @@ use core::marker::PhantomData; use super::PageEntry; -use crate::{Arch, PhysicalAddress, TableKind, VirtualAddress}; +use crate::{Arch, PhysicalAddress, VirtualAddress}; pub struct PageTable { base: VirtualAddress, @@ -20,16 +20,6 @@ impl PageTable { } } - pub unsafe fn top(table_kind: TableKind) -> Self { - unsafe { - Self::new( - VirtualAddress::new(0), - A::table(table_kind), - A::PAGE_LEVELS - 1, - ) - } - } - pub fn base(&self) -> VirtualAddress { self.base } @@ -43,18 +33,7 @@ impl PageTable { } pub unsafe fn virt(&self) -> VirtualAddress { - unsafe { - A::phys_to_virt(self.phys) - - // Recursive mapping - // let mut addr = 0xFFFF_FFFF_FFFF_F000; - // for level in (self.level + 1 .. A::PAGE_LEVELS).rev() { - // let index = (self.base.0 >> (level * A::PAGE_ENTRY_SHIFT + A::PAGE_SHIFT)) & A::PAGE_ENTRY_MASK; - // addr <<= A::PAGE_ENTRY_SHIFT; - // addr |= index << A::PAGE_SHIFT; - // } - // VirtualAddress::new(addr) - } + unsafe { A::phys_to_virt(self.phys) } } pub fn entry_base(&self, i: usize) -> Option { @@ -91,7 +70,7 @@ impl PageTable { } } - pub unsafe fn index_of(&self, address: VirtualAddress) -> Option { + pub fn index_of(&self, address: VirtualAddress) -> Option { // Canonicalize address first let address = VirtualAddress::new(address.data() & A::PAGE_ADDRESS_MASK); let level_shift = self.level * A::PAGE_ENTRY_SHIFT + A::PAGE_SHIFT;