From 2f16dddf25ba3acabd253118f05a38e9edb6091d Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Mon, 4 Jul 2022 10:41:53 +0200 Subject: [PATCH 1/7] Add a page flusher trait. --- src/page/flush.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/page/flush.rs b/src/page/flush.rs index 9aae51f..5dedf6a 100644 --- a/src/page/flush.rs +++ b/src/page/flush.rs @@ -8,6 +8,10 @@ use crate::{ VirtualAddress, }; +pub trait Flusher { + fn consume(&mut self, flush: PageFlush); +} + #[must_use = "The page table must be flushed, or the changes unsafely ignored"] pub struct PageFlush { virt: VirtualAddress, @@ -43,10 +47,6 @@ impl PageFlushAll { } } - pub fn consume(&self, flush: PageFlush) { - unsafe { flush.ignore(); } - } - pub fn flush(self) { unsafe { A::invalidate_all(); } } @@ -55,3 +55,16 @@ impl PageFlushAll { mem::forget(self); } } +impl Flusher for PageFlushAll { + fn consume(&mut self, flush: PageFlush) { + unsafe { flush.ignore(); } + } +} +impl + ?Sized> Flusher for &mut T { + fn consume(&mut self, flush: PageFlush) { + >::consume(self, flush) + } +} +impl Flusher for () { + fn consume(&mut self, _: PageFlush) {} +} -- GitLab From c847b1e2a88dee08ad8813d334d6cf0e4df6ab94 Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Sun, 17 Jul 2022 13:25:34 +0200 Subject: [PATCH 2/7] Implement FrameAllocator for mutable refs of impls. --- src/allocator/frame/mod.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/allocator/frame/mod.rs b/src/allocator/frame/mod.rs index 5b6898a..99c9d32 100644 --- a/src/allocator/frame/mod.rs +++ b/src/allocator/frame/mod.rs @@ -20,6 +20,7 @@ impl FrameCount { } } +#[derive(Debug)] pub struct FrameUsage { used: FrameCount, total: FrameCount, @@ -58,3 +59,21 @@ pub trait FrameAllocator { unsafe fn usage(&self) -> FrameUsage; } + +impl FrameAllocator for &mut T where T: FrameAllocator { + unsafe fn allocate(&mut self, count: FrameCount) -> Option { + T::allocate(self, count) + } + unsafe fn free(&mut self, address: PhysicalAddress, count: FrameCount) { + T::free(self, address, count) + } + unsafe fn allocate_one(&mut self) -> Option { + T::allocate_one(self) + } + unsafe fn free_one(&mut self, address: PhysicalAddress) { + T::free_one(self, address) + } + unsafe fn usage(&self) -> FrameUsage { + T::usage(self) + } +} -- GitLab From 7a209c83c9af82108d8853b88dab4bf30c650572 Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Sun, 17 Jul 2022 13:26:10 +0200 Subject: [PATCH 3/7] Implement Debug for X8664Arch. --- src/arch/x86_64.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/arch/x86_64.rs b/src/arch/x86_64.rs index 74c3754..1008df8 100644 --- a/src/arch/x86_64.rs +++ b/src/arch/x86_64.rs @@ -7,7 +7,7 @@ use crate::{ VirtualAddress, }; -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug)] pub struct X8664Arch; impl Arch for X8664Arch { -- GitLab From fad6afa7d8486e1b54f186a0bdd718113f353685 Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Sun, 17 Jul 2022 13:26:56 +0200 Subject: [PATCH 4/7] Flush on Drop, unless explicitly ignored. --- src/page/flush.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/page/flush.rs b/src/page/flush.rs index 5dedf6a..1114c33 100644 --- a/src/page/flush.rs +++ b/src/page/flush.rs @@ -35,9 +35,10 @@ impl PageFlush { } } -#[must_use = "The page table must be flushed, or the changes unsafely ignored"] -pub struct PageFlushAll { - phantom: PhantomData, +// TODO: Might remove Drop and add #[must_use] again, but ergonomically I prefer being able to pass +// a flusher, and have it dropped by the end of the function it is passed to, in order to flush. +pub struct PageFlushAll { + phantom: PhantomData A>, } impl PageFlushAll { @@ -47,14 +48,17 @@ impl PageFlushAll { } } - pub fn flush(self) { - unsafe { A::invalidate_all(); } - } + pub fn flush(self) {} pub unsafe fn ignore(self) { mem::forget(self); } } +impl Drop for PageFlushAll { + fn drop(&mut self) { + unsafe { A::invalidate_all(); } + } +} impl Flusher for PageFlushAll { fn consume(&mut self, flush: PageFlush) { unsafe { flush.ignore(); } -- GitLab From efc67a20127c60886fa6c0643164804e91a32e70 Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Sun, 17 Jul 2022 13:30:44 +0200 Subject: [PATCH 5/7] Use wrapped flags for PageEntry. --- src/page/entry.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/page/entry.rs b/src/page/entry.rs index 6732da4..0f593a2 100644 --- a/src/page/entry.rs +++ b/src/page/entry.rs @@ -2,6 +2,7 @@ use core::marker::PhantomData; use crate::{ Arch, + PageFlags, PhysicalAddress, }; @@ -28,8 +29,13 @@ impl PageEntry { } #[inline(always)] - pub fn flags(&self) -> usize { - self.data & A::ENTRY_FLAGS_MASK + pub fn flags(&self) -> PageFlags { + unsafe { PageFlags::from_data(self.data & A::ENTRY_FLAGS_MASK) } + } + #[inline(always)] + pub fn set_flags(&mut self, flags: PageFlags) { + self.data &= !A::ENTRY_FLAGS_MASK; + self.data |= flags.data(); } #[inline(always)] -- GitLab From 229ae5da40803e57a037571aa387cf9d235d7ed6 Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Sun, 17 Jul 2022 14:05:33 +0200 Subject: [PATCH 6/7] Add remaining interfaces to user RMM for user mem. --- src/page/entry.rs | 10 ++++-- src/page/flags.rs | 8 +++-- src/page/mapper.rs | 87 +++++++++++++++++++++++++++++++++++----------- src/page/table.rs | 18 +++++----- 4 files changed, 89 insertions(+), 34 deletions(-) diff --git a/src/page/entry.rs b/src/page/entry.rs index 0f593a2..800482c 100644 --- a/src/page/entry.rs +++ b/src/page/entry.rs @@ -24,8 +24,14 @@ impl PageEntry { } #[inline(always)] - pub fn address(&self) -> PhysicalAddress { - PhysicalAddress(self.data & A::ENTRY_ADDRESS_MASK) + pub fn address(&self) -> Result { + let addr = PhysicalAddress(self.data & A::ENTRY_ADDRESS_MASK); + + if self.present() { + Ok(addr) + } else { + Err(addr) + } } #[inline(always)] diff --git a/src/page/flags.rs b/src/page/flags.rs index 6058637..22b8780 100644 --- a/src/page/flags.rs +++ b/src/page/flags.rs @@ -111,7 +111,11 @@ impl PageFlags { impl fmt::Debug for PageFlags { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("PageFlags") - .field("data", &self.data) + .field("present", &self.has_present()) + .field("write", &self.has_write()) + .field("executable", &self.has_execute()) + .field("user", &self.has_user()) + .field("bits", &format_args!("{:#0x}", self.data)) .finish() } -} \ No newline at end of file +} diff --git a/src/page/mapper.rs b/src/page/mapper.rs index 6617547..d4f478b 100644 --- a/src/page/mapper.rs +++ b/src/page/mapper.rs @@ -8,44 +8,61 @@ use crate::{ PageFlush, PageTable, PhysicalAddress, + TableKind, VirtualAddress, }; -pub struct PageMapper<'f, A, F> { +pub struct PageMapper { table_addr: PhysicalAddress, - allocator: &'f mut F, - phantom: PhantomData, + allocator: F, + _phantom: PhantomData A>, } -impl<'f, A: Arch, F: FrameAllocator> PageMapper<'f, A, F> { - pub unsafe fn new(table_addr: PhysicalAddress, allocator: &'f mut F) -> Self { +impl PageMapper { + pub unsafe fn new(table_addr: PhysicalAddress, allocator: F) -> Self { Self { table_addr, allocator, - phantom: PhantomData, + _phantom: PhantomData, } } - pub unsafe fn create(allocator: &'f mut F) -> Option { + pub unsafe fn create(mut allocator: F) -> Option { let table_addr = allocator.allocate_one()?; Some(Self::new(table_addr, allocator)) } - pub unsafe fn current(allocator: &'f mut F) -> Self { + pub unsafe fn current(allocator: F) -> Self { let table_addr = A::table(); Self::new(table_addr, allocator) } + pub fn is_current(&self) -> bool { + unsafe { self.table().phys() == A::table() } + } - pub unsafe fn make_current(&mut self) { + pub unsafe fn make_current(&self) { A::set_table(self.table_addr); } - pub unsafe fn table(&self) -> PageTable { - PageTable::new( - VirtualAddress::new(0), - self.table_addr, - A::PAGE_LEVELS - 1 - ) + pub fn table(&self) -> PageTable { + // SAFETY: The only way to initialize a PageMapper is via new(), and we assume it upholds + // all necessary invariants for this to be safe. + unsafe { + PageTable::new( + VirtualAddress::new(0), + self.table_addr, + A::PAGE_LEVELS - 1 + ) + } + } + + pub unsafe fn remap(&mut self, virt: VirtualAddress, flags: PageFlags) -> Option> { + self.visit(virt, |p1, i| { + let mut entry = p1.entry(i)?; + entry.set_flags(flags); + p1.set_entry(i, entry); + Some(PageFlush::new(virt)) + }).flatten() } pub unsafe fn map(&mut self, virt: VirtualAddress, flags: PageFlags) -> Option> { @@ -71,7 +88,8 @@ impl<'f, A: Arch, F: FrameAllocator> PageMapper<'f, A, F> { None => { let next_phys = self.allocator.allocate_one()?; //TODO: correct flags? - table.set_entry(i, PageEntry::new(next_phys.data() | A::ENTRY_FLAG_READWRITE | A::ENTRY_FLAG_DEFAULT_TABLE)); + let flags = A::ENTRY_FLAG_READWRITE | A::ENTRY_FLAG_DEFAULT_TABLE | if virt.kind() == TableKind::User { A::ENTRY_FLAG_USER } else { 0 }; + table.set_entry(i, PageEntry::new(next_phys.data() | flags)); table.next(i)? } }; @@ -79,14 +97,35 @@ impl<'f, A: Arch, F: FrameAllocator> PageMapper<'f, A, F> { } } } + pub unsafe fn map_linearly(&mut self, phys: PhysicalAddress, flags: PageFlags) -> Option<(VirtualAddress, PageFlush)> { + let virt = A::phys_to_virt(phys); + self.map_phys(virt, phys, flags).map(|flush| (virt, flush)) + } + fn visit(&self, virt: VirtualAddress, 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)?; + } + } + } + } + pub fn translate(&self, virt: VirtualAddress) -> Option<(PhysicalAddress, PageFlags)> { + let entry = self.visit(virt, |p1, i| unsafe { p1.entry(i) })??; + Some((entry.address().ok()?, entry.flags())) + } pub unsafe fn unmap(&mut self, virt: VirtualAddress) -> Option> { - let (old, flush) = self.unmap_phys(virt)?; - self.allocator.free_one(old.address()); + let (old, _, flush) = self.unmap_phys(virt)?; + self.allocator.free_one(old); Some(flush) } - pub unsafe fn unmap_phys(&mut self, virt: VirtualAddress) -> Option<(PageEntry, PageFlush)> { + pub unsafe fn unmap_phys(&mut self, virt: VirtualAddress) -> Option<(PhysicalAddress, PageFlags, PageFlush)> { //TODO: verify virt is aligned let mut table = self.table(); //TODO: unmap parents @@ -96,10 +135,18 @@ impl<'f, A: Arch, F: FrameAllocator> PageMapper<'f, A, F> { let entry_opt = table.entry(i); table.set_entry(i, PageEntry::new(0)); let entry = entry_opt?; - return Some((entry, PageFlush::new(virt))); + return Some((entry.address().ok()?, entry.flags(), PageFlush::new(virt))); } else { table = table.next(i)?; } } } } +impl core::fmt::Debug for PageMapper { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_struct("PageMapper") + .field("frame", &self.table_addr) + .field("allocator", &self.allocator) + .finish() + } +} diff --git a/src/page/table.rs b/src/page/table.rs index 23a8fa1..0abc784 100644 --- a/src/page/table.rs +++ b/src/page/table.rs @@ -93,16 +93,14 @@ impl PageTable { } pub unsafe fn next(&self, i: usize) -> Option { - if self.level > 0 { - let entry = self.entry(i)?; - if entry.present() { - return Some(PageTable::new( - self.entry_base(i)?, - entry.address(), - self.level - 1 - )); - } + if self.level == 0 { + return None; } - None + + Some(PageTable::new( + self.entry_base(i)?, + self.entry(i)?.address().ok()?, + self.level - 1, + )) } } -- GitLab From 9462df03e786312b6ce197cf56113d411412cbb2 Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Sat, 23 Jul 2022 00:18:56 +0200 Subject: [PATCH 7/7] Add support for unmapping parents. Currently, this uses a relatively naive method of simply scanning the 512 entries for the PRESENT flag. But, unless the optimizer cannot, it can be reduced to calculating the bitwise OR of every entry and then checking that. If this turns out to be too slow, which it might be when unmapping lots of pages, then we can (1) either fall back to using a counter like the old paging code did, or even better (2) use the now-1:1 grants tree to check if it became empty. Putting the grants code in RMM might be suboptimal, so instead we can add "unmap_range" and have the kernel paging code take the offset of the next grant, if any, and then possibly unmap entire P1s/P2s/P3s -- whatever is in the page tables within that range. Note that I am fairly certain that method (1) was the cause of the visually notorious orbital memory corruption bug. --- src/page/mapper.rs | 42 ++++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/src/page/mapper.rs b/src/page/mapper.rs index d4f478b..17d4b11 100644 --- a/src/page/mapper.rs +++ b/src/page/mapper.rs @@ -119,27 +119,45 @@ impl PageMapper { Some((entry.address().ok()?, entry.flags())) } - pub unsafe fn unmap(&mut self, virt: VirtualAddress) -> Option> { - let (old, _, flush) = self.unmap_phys(virt)?; + pub unsafe fn unmap(&mut self, virt: VirtualAddress, unmap_parents: bool) -> Option> { + let (old, _, flush) = self.unmap_phys(virt, unmap_parents)?; self.allocator.free_one(old); Some(flush) } - pub unsafe fn unmap_phys(&mut self, virt: VirtualAddress) -> Option<(PhysicalAddress, PageFlags, PageFlush)> { + pub unsafe fn unmap_phys(&mut self, virt: VirtualAddress, unmap_parents: bool) -> Option<(PhysicalAddress, PageFlags, PageFlush)> { //TODO: verify virt is aligned let mut table = self.table(); - //TODO: unmap parents - loop { - let i = table.index_of(virt)?; - if table.level() == 0 { - let entry_opt = table.entry(i); + let level = table.level(); + unmap_phys_inner(virt, &mut table, level, false, &mut self.allocator).map(|(pa, pf)| (pa, pf, PageFlush::new(virt))) + } +} +unsafe fn unmap_phys_inner(virt: VirtualAddress, table: &mut PageTable, initial_level: usize, unmap_parents: bool, allocator: &mut impl FrameAllocator) -> Option<(PhysicalAddress, PageFlags)> { + let i = table.index_of(virt)?; + + if table.level() == 0 { + let entry_opt = table.entry(i); + table.set_entry(i, PageEntry::new(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)?; + + 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(table.phys()); table.set_entry(i, PageEntry::new(0)); - let entry = entry_opt?; - return Some((entry.address().ok()?, entry.flags(), PageFlush::new(virt))); - } else { - table = table.next(i)?; } } + + Some(res) } } impl core::fmt::Debug for PageMapper { -- GitLab