From abf710b4a200fc17fc93d3f72a4bcae0a3e9c658 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 29 Mar 2026 12:06:23 +0200 Subject: [PATCH] Move handling of kernel page table entry copying to RMM This way it can ensure those page table entries never get unmapped, ensuring they are kept in sync between all processes. --- rmm/src/arch/aarch64.rs | 2 + rmm/src/arch/emulate.rs | 2 + rmm/src/arch/mod.rs | 7 +++ rmm/src/arch/riscv64/sv39.rs | 2 + rmm/src/arch/riscv64/sv48.rs | 2 + rmm/src/arch/x86.rs | 2 + rmm/src/arch/x86_64.rs | 2 + rmm/src/main.rs | 2 +- rmm/src/page/mapper.rs | 58 +++++++++++++++++------- src/acpi/madt/arch/x86.rs | 2 +- src/arch/riscv64/paging/mod.rs | 3 -- src/arch/x86_shared/device/local_apic.rs | 2 +- src/arch/x86_shared/paging/mod.rs | 2 - src/context/arch/aarch64.rs | 15 +----- src/context/arch/riscv64.rs | 29 ++---------- src/context/arch/x86.rs | 38 +--------------- src/context/arch/x86_64.rs | 32 +------------ src/context/memory.rs | 18 ++++---- src/startup/memory.rs | 17 ------- 19 files changed, 83 insertions(+), 154 deletions(-) diff --git a/rmm/src/arch/aarch64.rs b/rmm/src/arch/aarch64.rs index 626c45088b..43ee0610a2 100644 --- a/rmm/src/arch/aarch64.rs +++ b/rmm/src/arch/aarch64.rs @@ -6,6 +6,8 @@ use crate::{Arch, PhysicalAddress, TableKind, VirtualAddress}; pub struct AArch64Arch; impl Arch for AArch64Arch { + const KERNEL_SEPARATE_TABLE: bool = true; + const PAGE_SHIFT: usize = 12; // 4096 bytes const PAGE_ENTRY_SHIFT: usize = 9; // 512 entries, 8 bytes each const PAGE_LEVELS: usize = 4; // L0, L1, L2, L3 diff --git a/rmm/src/arch/emulate.rs b/rmm/src/arch/emulate.rs index d92bee0c9b..c388a82fa1 100644 --- a/rmm/src/arch/emulate.rs +++ b/rmm/src/arch/emulate.rs @@ -53,6 +53,8 @@ impl EmulateArch { } impl Arch for EmulateArch { + const KERNEL_SEPARATE_TABLE: bool = false; + const PAGE_SHIFT: usize = X8664Arch::PAGE_SHIFT; const PAGE_ENTRY_SHIFT: usize = X8664Arch::PAGE_ENTRY_SHIFT; const PAGE_LEVELS: usize = X8664Arch::PAGE_LEVELS; diff --git a/rmm/src/arch/mod.rs b/rmm/src/arch/mod.rs index b2405c741b..78eb9895fa 100644 --- a/rmm/src/arch/mod.rs +++ b/rmm/src/arch/mod.rs @@ -26,6 +26,13 @@ mod x86; mod x86_64; pub trait Arch: Clone + Copy { + /// Does the architecture use a separate page table for the kernel. + /// + /// If false, the page table entries corresponding to the top half of the + /// address space will be copied into the top level of every page table + /// and will never be unmapped when unmapping pages. + const KERNEL_SEPARATE_TABLE: bool; + const PAGE_SHIFT: usize; const PAGE_ENTRY_SHIFT: usize; const PAGE_LEVELS: usize; diff --git a/rmm/src/arch/riscv64/sv39.rs b/rmm/src/arch/riscv64/sv39.rs index 6eb78f9ef8..9e8e3c0384 100644 --- a/rmm/src/arch/riscv64/sv39.rs +++ b/rmm/src/arch/riscv64/sv39.rs @@ -9,6 +9,8 @@ pub const ACCESSED: usize = 1 << 6; pub const DIRTY: usize = 1 << 7; impl Arch for RiscV64Sv39Arch { + const KERNEL_SEPARATE_TABLE: bool = false; + const PAGE_SHIFT: usize = 12; // 4096 bytes const PAGE_ENTRY_SHIFT: usize = 9; // 512 entries, 8 bytes each const PAGE_LEVELS: usize = 3; // L0, L1, L2 diff --git a/rmm/src/arch/riscv64/sv48.rs b/rmm/src/arch/riscv64/sv48.rs index 72ff27c32a..f9040ef78c 100644 --- a/rmm/src/arch/riscv64/sv48.rs +++ b/rmm/src/arch/riscv64/sv48.rs @@ -6,6 +6,8 @@ use crate::{Arch, PhysicalAddress, TableKind, VirtualAddress}; pub struct RiscV64Sv48Arch; impl Arch for RiscV64Sv48Arch { + const KERNEL_SEPARATE_TABLE: bool = false; + const PAGE_SHIFT: usize = 12; // 4096 bytes const PAGE_ENTRY_SHIFT: usize = 9; // 512 entries, 8 bytes each const PAGE_LEVELS: usize = 4; // L0, L1, L2, L3 diff --git a/rmm/src/arch/x86.rs b/rmm/src/arch/x86.rs index 486ced56d7..2a9cff3e85 100644 --- a/rmm/src/arch/x86.rs +++ b/rmm/src/arch/x86.rs @@ -7,6 +7,8 @@ use crate::{Arch, PhysicalAddress, TableKind, VirtualAddress}; pub struct X86Arch; impl Arch for X86Arch { + const KERNEL_SEPARATE_TABLE: bool = false; + const PAGE_SHIFT: usize = 12; // 4096 bytes const PAGE_ENTRY_SHIFT: usize = 10; // 1024 entries, 4 bytes each const PAGE_LEVELS: usize = 2; // PD, PT diff --git a/rmm/src/arch/x86_64.rs b/rmm/src/arch/x86_64.rs index 7059b7381a..824d20ab1e 100644 --- a/rmm/src/arch/x86_64.rs +++ b/rmm/src/arch/x86_64.rs @@ -6,6 +6,8 @@ use crate::{Arch, PhysicalAddress, TableKind, VirtualAddress}; pub struct X8664Arch; impl Arch for X8664Arch { + const KERNEL_SEPARATE_TABLE: bool = false; + const PAGE_SHIFT: usize = 12; // 4096 bytes const PAGE_ENTRY_SHIFT: usize = 9; // 512 entries, 8 bytes each const PAGE_LEVELS: usize = 4; // PML4, PDP, PD, PT diff --git a/rmm/src/main.rs b/rmm/src/main.rs index f115f9ac7e..00fee6a997 100644 --- a/rmm/src/main.rs +++ b/rmm/src/main.rs @@ -245,7 +245,7 @@ unsafe fn new_tables(areas: &'static [MemoryArea]) { let mut flush_all = PageFlushAll::new(); for i in 0..16 { let virt = VirtualAddress::new(MEGABYTE + i * A::PAGE_SIZE); - let flush = mapper.unmap(virt, false).expect("failed to unmap page"); + let flush = mapper.unmap(virt).expect("failed to unmap page"); flush_all.consume(flush); } flush_all.flush(); diff --git a/rmm/src/page/mapper.rs b/rmm/src/page/mapper.rs index 248f1f8a1b..7af3bfd6ca 100644 --- a/rmm/src/page/mapper.rs +++ b/rmm/src/page/mapper.rs @@ -121,7 +121,39 @@ impl PageMapper { pub unsafe fn create(table_kind: TableKind, mut allocator: F) -> Option { unsafe { let table_addr = allocator.allocate_one()?; - Some(Self::new(table_kind, table_addr, allocator)) + let mut table = Self::new(table_kind, table_addr, allocator); + + match (table_kind, A::KERNEL_SEPARATE_TABLE) { + (TableKind::Kernel, false) => { + // Pre-allocate all kernel top-level page table entries so that when + // the page table is copied, these entries are synced between processes. + for i in A::PAGE_ENTRIES / 2..A::PAGE_ENTRIES { + let phys = table + .allocator + .allocate_one() + .expect("failed to map page table"); + let flags = A::ENTRY_FLAG_DEFAULT_TABLE; + table + .table() + .set_entry(i, PageEntry::new(phys.data(), flags)); + } + } + (TableKind::User, false) => { + // Copy higher half (kernel) mappings + let active_ktable = PageMapper::current(TableKind::Kernel, ()); + for i in A::PAGE_ENTRIES / 2..A::PAGE_ENTRIES { + if let Some(entry) = active_ktable.table().entry(i) { + table.table().set_entry(i, entry); + } + } + } + (_, true) => { + // There is a separate page table for the kernel. No need to copy the kernel + // mappings to the user page table. + } + } + + Some(table) } } @@ -186,16 +218,12 @@ impl PageMapper { } } - pub unsafe fn unmap( - &mut self, - virt: VirtualAddress, - unmap_parents: bool, - ) -> Option> + pub unsafe fn unmap(&mut self, virt: VirtualAddress) -> Option> where F: FrameAllocator, { unsafe { - let (old, _, flush) = self.unmap_phys(virt, unmap_parents)?; + let (old, _, flush) = self.unmap_phys(virt)?; self.allocator.free_one(old); Some(flush) } @@ -204,13 +232,14 @@ impl PageMapper { 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(); + + let unmap_parents = A::KERNEL_SEPARATE_TABLE || table.index_of(virt)? < A::PAGE_ENTRIES / 2; // Is a userspace mapping + unsafe { - //TODO: verify virt is aligned - let mut table = self.table(); - let level = table.level(); - unmap_phys_inner(virt, &mut table, level, unmap_parents, &mut self.allocator) + unmap_phys_inner(virt, &mut table, unmap_parents, &mut self.allocator) .map(|(pa, pf)| (pa, pf, PageFlush::new(virt))) } } @@ -219,7 +248,6 @@ impl PageMapper { unsafe fn unmap_phys_inner( virt: VirtualAddress, table: &mut PageTable, - initial_level: usize, unmap_parents: bool, allocator: &mut impl FrameAllocator, ) -> Option<(PhysicalAddress, PageFlags)> { @@ -236,10 +264,8 @@ unsafe fn unmap_phys_inner( let mut subtable = table.next(i)?; - let res = unmap_phys_inner(virt, &mut subtable, initial_level, unmap_parents, allocator)?; + let res = unmap_phys_inner(virt, &mut subtable, 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). diff --git a/src/acpi/madt/arch/x86.rs b/src/acpi/madt/arch/x86.rs index 88959fdac2..4b900b011d 100644 --- a/src/acpi/madt/arch/x86.rs +++ b/src/acpi/madt/arch/x86.rs @@ -144,7 +144,7 @@ pub(super) fn init(madt: Madt) { // Unmap trampoline let (_frame, _, flush) = unsafe { KernelMapper::lock_rw() - .unmap_phys(trampoline_page.start_address(), true) + .unmap_phys(trampoline_page.start_address()) .expect("failed to unmap trampoline page") }; flush.flush(); diff --git a/src/arch/riscv64/paging/mod.rs b/src/arch/riscv64/paging/mod.rs index 1fa656f275..194cf7f043 100644 --- a/src/arch/riscv64/paging/mod.rs +++ b/src/arch/riscv64/paging/mod.rs @@ -9,9 +9,6 @@ pub use crate::rmm::KernelMapper; pub mod entry; pub mod mapper; -/// Number of entries per page table -pub const ENTRY_COUNT: usize = RmmA::PAGE_ENTRIES; - /// Size of pages pub const PAGE_SIZE: usize = RmmA::PAGE_SIZE; pub const PAGE_MASK: usize = RmmA::PAGE_OFFSET_MASK; diff --git a/src/arch/x86_shared/device/local_apic.rs b/src/arch/x86_shared/device/local_apic.rs index 4d1bf965e9..7c45143b81 100644 --- a/src/arch/x86_shared/device/local_apic.rs +++ b/src/arch/x86_shared/device/local_apic.rs @@ -70,7 +70,7 @@ impl LocalApic { if !self.x2 { debug!("Detected xAPIC at {:#x}", physaddr.data()); - if let Some((_entry, _, flush)) = mapper.unmap_phys(virtaddr, true) { + if let Some((_entry, _, flush)) = mapper.unmap_phys(virtaddr) { // Unmap xAPIC page if already mapped flush.flush(); } diff --git a/src/arch/x86_shared/paging/mod.rs b/src/arch/x86_shared/paging/mod.rs index 9b0481f849..18f1e69652 100644 --- a/src/arch/x86_shared/paging/mod.rs +++ b/src/arch/x86_shared/paging/mod.rs @@ -23,8 +23,6 @@ pub mod entry { pub mod mapper; -pub const ENTRY_COUNT: usize = RmmA::PAGE_ENTRIES; - /// Size of pages pub const PAGE_SIZE: usize = RmmA::PAGE_SIZE; pub const PAGE_MASK: usize = RmmA::PAGE_OFFSET_MASK; diff --git a/src/context/arch/aarch64.rs b/src/context/arch/aarch64.rs index bf580d4b9b..80b3a2a970 100644 --- a/src/context/arch/aarch64.rs +++ b/src/context/arch/aarch64.rs @@ -1,11 +1,10 @@ use crate::{ - arch::{device::cpu::registers::control_regs, interrupt::InterruptStack, paging::PageMapper}, - context::{context::Kstack, memory::Table}, + arch::{device::cpu::registers::control_regs, interrupt::InterruptStack}, + context::context::Kstack, percpu::PercpuBlock, syscall::FloatRegisters, }; use core::{mem, mem::offset_of, ptr, sync::atomic::AtomicBool}; -use rmm::TableKind; use spin::Once; use syscall::{EnvRegisters, Error, Result, ENOMEM}; @@ -390,13 +389,3 @@ unsafe extern "C" fn switch_to_inner(_prev: &mut Context, _next: &mut Context) { switch_hook = sym crate::context::switch_finish_hook, ); } - -/// Allocates a new empty utable -pub fn setup_new_utable() -> Result { - let utable = unsafe { - PageMapper::create(TableKind::User, crate::memory::TheFrameAllocator) - .ok_or(Error::new(ENOMEM))? - }; - - Ok(Table { utable }) -} diff --git a/src/context/arch/riscv64.rs b/src/context/arch/riscv64.rs index 152af5c444..41fe45ff89 100644 --- a/src/context/arch/riscv64.rs +++ b/src/context/arch/riscv64.rs @@ -1,15 +1,12 @@ use crate::{ - arch::{ - interrupt::InterruptStack, - paging::{PageMapper, ENTRY_COUNT}, - }, - context::{context::Kstack, memory::Table}, + arch::interrupt::InterruptStack, + context::context::Kstack, memory::{KernelMapper, RmmA}, percpu::PercpuBlock, syscall::FloatRegisters, }; use core::{mem::offset_of, sync::atomic::AtomicBool}; -use rmm::{Arch, TableKind, VirtualAddress}; +use rmm::{Arch, VirtualAddress}; use spin::Once; use syscall::{error::*, EnvRegisters}; @@ -228,23 +225,3 @@ unsafe extern "C" fn switch_to_inner(prev: &mut Context, next: &mut Context) { switch_hook = sym crate::context::switch_finish_hook, ); } - -/// Allocates a new empty utable -pub fn setup_new_utable() -> Result
{ - let utable = unsafe { - PageMapper::create(TableKind::User, crate::memory::TheFrameAllocator) - .ok_or(Error::new(ENOMEM))? - }; - - // Copy higher half (kernel) mappings - unsafe { - let active_ktable = KernelMapper::lock_ro(); - for pde_no in ENTRY_COUNT / 2..ENTRY_COUNT { - if let Some(entry) = active_ktable.table().entry(pde_no) { - utable.table().set_entry(pde_no, entry); - } - } - } - - Ok(Table { utable }) -} diff --git a/src/context/arch/x86.rs b/src/context/arch/x86.rs index cc212f58cf..f859d13c49 100644 --- a/src/context/arch/x86.rs +++ b/src/context/arch/x86.rs @@ -6,13 +6,9 @@ use crate::{ syscall::FloatRegisters, }; -use crate::{ - arch::{interrupt::InterruptStack, paging::PageMapper}, - context::{context::Kstack, memory::Table}, - memory::RmmA, -}; +use crate::{arch::interrupt::InterruptStack, context::context::Kstack, memory::RmmA}; use core::mem::offset_of; -use rmm::{Arch, TableKind, VirtualAddress}; +use rmm::{Arch, VirtualAddress}; use spin::Once; use syscall::{error::*, EnvRegisters}; @@ -315,33 +311,3 @@ unsafe extern "cdecl" fn switch_to_inner() { switch_hook = sym crate::context::switch_finish_hook, ); } - -/// Allocates a new identically mapped ktable and empty utable (same memory on x86) -pub fn setup_new_utable() -> Result
{ - use crate::memory::KernelMapper; - - let utable = unsafe { - PageMapper::create(TableKind::User, crate::memory::TheFrameAllocator) - .ok_or(Error::new(ENOMEM))? - }; - - { - let active_ktable = KernelMapper::lock_ro(); - - let copy_mapping = |p4_no| unsafe { - let entry = active_ktable - .table() - .entry(p4_no) - .unwrap_or_else(|| panic!("expected kernel PML {} to be mapped", p4_no)); - - utable.table().set_entry(p4_no, entry) - }; - - // Copy higher half (kernel) mappings - for i in 512..1024 { - copy_mapping(i); - } - } - - Ok(Table { utable }) -} diff --git a/src/context/arch/x86_64.rs b/src/context/arch/x86_64.rs index 5e5b73a392..e9523d4bfc 100644 --- a/src/context/arch/x86_64.rs +++ b/src/context/arch/x86_64.rs @@ -5,16 +5,9 @@ use core::{ use crate::syscall::FloatRegisters; -use crate::{ - arch::{ - interrupt::InterruptStack, - paging::{PageMapper, ENTRY_COUNT}, - }, - context::{context::Kstack, memory::Table}, - memory::RmmA, -}; +use crate::{arch::interrupt::InterruptStack, context::context::Kstack, memory::RmmA}; use core::mem::offset_of; -use rmm::{Arch, TableKind, VirtualAddress}; +use rmm::{Arch, VirtualAddress}; use spin::Once; use syscall::{error::*, EnvRegisters}; use x86::msr; @@ -394,24 +387,3 @@ unsafe extern "sysv64" fn switch_to_inner(_prev: &mut Context, _next: &mut Conte switch_hook = sym crate::context::switch_finish_hook, ); } - -/// Allocates a new identically mapped ktable and empty utable (same memory on x86_64). -pub fn setup_new_utable() -> Result
{ - use crate::memory::{KernelMapper, TheFrameAllocator}; - - let utable = unsafe { - PageMapper::create(TableKind::User, TheFrameAllocator).ok_or(Error::new(ENOMEM))? - }; - - // Copy higher half (kernel) mappings - unsafe { - let active_ktable = KernelMapper::lock_ro(); - for pde_no in ENTRY_COUNT / 2..ENTRY_COUNT { - if let Some(entry) = active_ktable.table().entry(pde_no) { - utable.table().set_entry(pde_no, entry); - } - } - } - - Ok(Table { utable }) -} diff --git a/src/context/memory.rs b/src/context/memory.rs index 77d260a73e..0c3f462c7a 100644 --- a/src/context/memory.rs +++ b/src/context/memory.rs @@ -14,7 +14,7 @@ use syscall::{error::*, flag::MapFlags, GrantFlags, MunmapFlags}; use crate::{ arch::paging::PAGE_SIZE, - context::{arch::setup_new_utable, file::LockedFileDescription}, + context::file::LockedFileDescription, cpu_set::LogicalCpuSet, memory::{ deallocate_frame, get_page_info, init_frame, the_zeroed_frame, AddRefError, Enomem, Frame, @@ -566,9 +566,14 @@ impl AddrSpace { } pub fn new() -> Result { + let utable = unsafe { + PageMapper::create(TableKind::User, crate::memory::TheFrameAllocator) + .ok_or(Error::new(ENOMEM))? + }; + Ok(Self { grants: UserGrants::new(), - table: setup_new_utable()?, + table: Table { utable }, mmap_min: MMAP_MIN_DEFAULT, used_by: LogicalCpuSet::empty(), }) @@ -1822,11 +1827,9 @@ impl Grant { for src_page in self.span().pages() { let dst_page = dst_base.next_by(src_page.offset_from(self.base)); - let unmap_parents = true; - // TODO: Validate flags? let Some((phys, _flags, flush)) = - (unsafe { src_mapper.unmap_phys(src_page.start_address(), unmap_parents) }) + (unsafe { src_mapper.unmap_phys(src_page.start_address()) }) else { continue; }; @@ -1953,7 +1956,7 @@ impl Grant { for i in 0..self.info.page_count { unsafe { let (phys, _, flush) = mapper - .unmap_phys(self.base.next_by(i).start_address(), true) + .unmap_phys(self.base.next_by(i).start_address()) .expect("all physborrowed grants must be fully Present in the page tables"); flush.ignore(); @@ -1969,8 +1972,7 @@ impl Grant { } else { for page in self.span().pages() { // Lazy mappings do not need to be unmapped. - let Some((phys, _, flush)) = - (unsafe { mapper.unmap_phys(page.start_address(), true) }) + let Some((phys, _, flush)) = (unsafe { mapper.unmap_phys(page.start_address()) }) else { continue; }; diff --git a/src/startup/memory.rs b/src/startup/memory.rs index 058ec5265a..77b502def4 100644 --- a/src/startup/memory.rs +++ b/src/startup/memory.rs @@ -295,23 +295,6 @@ unsafe fn map_memory(areas: &[MemoryArea], mut bump_allocator: &mut Bum let mut mapper = PageMapper::::create(TableKind::Kernel, &mut bump_allocator) .expect("failed to create Mapper"); - if cfg!(target_arch = "x86") { - // Pre-allocate all kernel PD entries so that when the page table is copied, - // these entries are synced between processes - for i in 512..1024 { - use rmm::{FrameAllocator, PageEntry}; - - let phys = mapper - .allocator_mut() - .allocate_one() - .expect("failed to map page table"); - let flags = A::ENTRY_FLAG_READWRITE | A::ENTRY_FLAG_DEFAULT_TABLE; - mapper - .table() - .set_entry(i, PageEntry::new(phys.data(), flags)); - } - } - // Map all physical areas at PHYS_OFFSET for area in areas.iter() { for i in 0..area.size / PAGE_SIZE {