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

@@ -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

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;

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;

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

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

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

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

rmm/src/main.rs

@@ -245,7 +245,7 @@ unsafe fn new_tables<A: Arch>(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();

rmm/src/page/mapper.rs

@@ -121,7 +121,39 @@ impl<A: Arch, F: FrameAllocator> PageMapper<A, F> {
     pub unsafe fn create(table_kind: TableKind, mut allocator: F) -> Option<Self> {
         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<A: Arch, F: FrameAllocator> PageMapper<A, F> {
         }
     }
 
-    pub unsafe fn unmap(
-        &mut self,
-        virt: VirtualAddress,
-        unmap_parents: bool,
-    ) -> Option<PageFlush<A>>
+    pub unsafe fn unmap(&mut self, virt: VirtualAddress) -> Option<PageFlush<A>>
     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<A: Arch, F: FrameAllocator> PageMapper<A, F> {
     pub unsafe fn unmap_phys(
         &mut self,
         virt: VirtualAddress,
-        unmap_parents: bool,
     ) -> Option<(PhysicalAddress, PageFlags<A>, PageFlush<A>)> {
+        //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<A: Arch, F: FrameAllocator> PageMapper<A, F> {
 unsafe fn unmap_phys_inner<A: Arch>(
     virt: VirtualAddress,
     table: &mut PageTable<A>,
-    initial_level: usize,
     unmap_parents: bool,
     allocator: &mut impl FrameAllocator,
 ) -> Option<(PhysicalAddress, PageFlags<A>)> {
@@ -236,10 +264,8 @@ unsafe fn unmap_phys_inner<A: Arch>(
 
         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).

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();

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;

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();
                 }

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;

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<Table> {
-    let utable = unsafe {
-        PageMapper::create(TableKind::User, crate::memory::TheFrameAllocator)
-            .ok_or(Error::new(ENOMEM))?
-    };
-
-    Ok(Table { utable })
-}

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<Table> {
-    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 })
-}

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<Table> {
-    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 })
-}

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<Table> {
-    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 })
-}

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<Self> {
+        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;
                 };

src/startup/memory.rs

@@ -295,23 +295,6 @@ unsafe fn map_memory<A: Arch>(areas: &[MemoryArea], mut bump_allocator: &mut Bum
         let mut mapper = PageMapper::<A, _>::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 {