From d17755489d5498d4d8303da8342331fcb668c27e Mon Sep 17 00:00:00 2001 From: corigan01 Date: Mon, 20 Jan 2025 14:01:37 -0600 Subject: [PATCH] Mem: New vm objects map pages when fault to correct permissions --- crates/mem/src/paging.rs | 32 ++++++++++-- crates/mem/src/vm.rs | 109 +++++++++++++++++++++++++++++++++++---- kernel/src/main.rs | 7 +-- 3 files changed, 129 insertions(+), 19 deletions(-) diff --git a/crates/mem/src/paging.rs b/crates/mem/src/paging.rs index 531c2c85..2f0da50b 100644 --- a/crates/mem/src/paging.rs +++ b/crates/mem/src/paging.rs @@ -821,6 +821,8 @@ impl Virt2PhysMapping { field(RW, 4, pub reduce_perm), /// Just change permissions, dont change page mapping field(RW, 5, pub only_commit_permissions), + /// Force set permissions, regardless if they are higher or lower for the bottom most table only + field(RW, 5, pub force_permissions_on_page), )] #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub struct VmOptions(usize); @@ -904,12 +906,29 @@ impl BitOrAssign for VmPermissions { impl PartialOrd for VmPermissions { fn partial_cmp(&self, other: &Self) -> Option { - self.0.count_ones().partial_cmp(&other.0.count_ones()) + if self.is_exec_set() && !other.is_exec_set() { + return Some(core::cmp::Ordering::Less); + } + if self.is_read_set() && !other.is_read_set() { + return Some(core::cmp::Ordering::Less); + } + if self.is_write_set() && !other.is_write_set() { + return Some(core::cmp::Ordering::Less); + } + if self.is_user_set() && !other.is_user_set() { + return Some(core::cmp::Ordering::Less); + } + + if self.0 == other.0 { + return Some(core::cmp::Ordering::Equal); + } + + Some(core::cmp::Ordering::Greater) } } impl Ord for VmPermissions { fn cmp(&self, other: &Self) -> core::cmp::Ordering { - self.0.count_ones().cmp(&other.0.count_ones()) + self.partial_cmp(other).unwrap() } } @@ -1162,12 +1181,17 @@ impl SafePageMapLvl1 { } // Unless we are told to reduce permissions, fail - if prev_permissions > permissions && !options.is_reduce_perm_set() { + if prev_permissions > permissions + && !options.is_reduce_perm_set() + && !options.is_force_permissions_on_page_set() + { return Err(PageCorrelationError::ExistingPermissionsPermissive { table_perms: entry.get_permissions(), requested_perms: permissions, }); - } else if prev_permissions < permissions && options.is_reduce_perm_set() { + } else if prev_permissions < permissions && options.is_reduce_perm_set() + || options.is_force_permissions_on_page_set() + { entry.reduce_permissions_to(permissions); } } diff --git a/crates/mem/src/vm.rs b/crates/mem/src/vm.rs index b8a04c39..ac1bd29e 100644 --- a/crates/mem/src/vm.rs +++ b/crates/mem/src/vm.rs @@ -24,12 +24,12 @@ OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWA */ extern crate alloc; -use core::error::Error; +use core::{error::Error, fmt::Display}; use crate::{ MemoryError, addr::{AlignedTo, VirtAddr}, - page::{PhysPage, VirtPage}, + page::{Page4K, PhysPage, VirtPage}, paging::{PageCorrelationError, Virt2PhysMapping, VmOptions, VmPermissions}, pmm::SharedPhysPage, }; @@ -72,12 +72,12 @@ impl VmRegion { /// Is this virtual address contained within this VmRegion pub fn does_contain_addr(&self, addr: VirtAddr) -> bool { - self.start.addr() >= addr && (self.end.addr().offset(PAGE_4K - 1)) <= addr + self.start.addr() <= addr && (self.end.addr().offset(PAGE_4K - 1)) >= addr } /// Is this page contained within this VmRegion pub fn does_contain_page(&self, page: VirtPage) -> bool { - self.start >= page && self.end <= page + self.start <= page && self.end >= page } /// Get an iterator of the pages contained within this region @@ -135,7 +135,7 @@ pub trait VmInjectFillAction: core::fmt::Debug { /// What to do when this region gets a page fault (if anything) #[allow(unused_variables)] fn page_fault_handler( - &mut self, + &self, parent_object: &VmObject, process: &VmProcess, info: PageFaultInfo, @@ -218,14 +218,17 @@ impl VmInjectFillAction for VmFillAction { } fn page_fault_handler( - &mut self, + &self, parent_object: &VmObject, process: &VmProcess, info: PageFaultInfo, ) -> PageFaultReponse { match self { - VmFillAction::Nothing => PageFaultReponse::NotAttachedHandler, - VmFillAction::Scrub(_) => PageFaultReponse::NotAttachedHandler, + // If we return with 'Handled' we will later receive a call to map that page + // + // We should not do the mapping of the page in the page fault handler! + VmFillAction::Nothing => PageFaultReponse::Handled, + VmFillAction::Scrub(_) => PageFaultReponse::Handled, VmFillAction::InjectWith(rw_lock) => { rw_lock .write() @@ -275,6 +278,13 @@ pub enum VmObjectMappingError { }, } +impl Error for VmObjectMappingError {} +impl Display for VmObjectMappingError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + writeln!(f, "{:#?}", self) + } +} + /// Options to apply to a page when populating it const KERNEL_POPULATE_OPT: VmOptions = VmOptions::none() .set_overwrite_flag(true) @@ -315,7 +325,8 @@ impl VmObject { .map_err(|err| NewVmObjectError::MappingErr(err))?; } } - todo!() + + Ok(Arc::new(RwLock::new(new_self))) } /// Do the mapping of a virtual page @@ -372,6 +383,7 @@ impl VmObject { VmOptions::none() .set_only_commit_permissions_flag(true) .set_increase_perm_flag(true) + .set_force_permissions_on_page_flag(true) .set_overwrite_flag(true), self.permissions, ) @@ -379,6 +391,30 @@ impl VmObject { Ok(()) } + + /// The page fault handler for this VmObject + pub fn page_fault_handler( + &mut self, + vm_process: &VmProcess, + info: PageFaultInfo, + ) -> PageFaultReponse { + match self + .fill_action + .write() + .page_fault_handler(self, vm_process, info) + { + PageFaultReponse::Handled => (), + err => return err, + } + + // If the FillAction returned 'Handled' we should call map_new_page() to let it allocate that page + match self.map_new_page(vm_process, VirtPage::containing_addr(info.vaddr)) { + Ok(_) => PageFaultReponse::Handled, + Err(page_mapping_err) => { + return PageFaultReponse::CriticalFault(Box::new(page_mapping_err)); + } + } + } } /// A possible reponse to inserting a VmObject into a VmProcess @@ -417,6 +453,14 @@ impl VmProcess { } } + /// Inhearit the page tables from 'page_tables' + pub fn inhearit_page_tables(page_tables: &Virt2PhysMapping) -> Self { + Self { + objects: RwLock::new(Vec::new()), + page_tables: Virt2PhysMapping::inhearit_from(page_tables), + } + } + /// Does this VmRegion overlap with any of the VmObjects in this Process? /// /// If it returns the region that is overlapping. @@ -483,6 +527,23 @@ impl VmProcess { Ok(obj) } + + /// The page fault handler for this VmProcess + pub fn page_fault_handler(&self, info: PageFaultInfo) -> PageFaultReponse { + let lock = self.objects.read(); + let Some(object) = lock + .iter() + .find(|object| object.read().region.does_contain_addr(info.vaddr)) + else { + logln!( + "Called PageFaultHandler, but could not find a region with that addr! {:#?}", + info + ); + return PageFaultReponse::NotAttachedHandler; + }; + + object.write().page_fault_handler(self, info) + } } /// Possible scenarios for a page fault to occur @@ -551,3 +612,33 @@ pub fn set_page_fault_handler(handler: SystemAttachedPageFaultFn) { pub fn remove_page_fault_handler() { *MAIN_PAGE_FAULT_HANDLER.write() = None; } + +// TODO: REMOVE THIS FUNCTION, its just for testing :) +pub fn test() { + let proc = VmProcess::new(); + + proc.inplace_new_vmobject( + VmRegion::new(VirtPage::new(10), VirtPage::new(20)), + VmPermissions::none() + .set_read_flag(true) + .set_write_flag(true) + .set_user_flag(true), + VmFillAction::Nothing, + ) + .unwrap(); + + logln!( + "{:#?}", + proc.page_fault_handler(PageFaultInfo { + is_present: false, + write_read_access: false, + execute_fault: false, + user_fault: false, + vaddr: VirtPage::::new(15).addr(), + }) + ); + + logln!("{}", proc.page_tables); + + todo!("Test Done!"); +} diff --git a/kernel/src/main.rs b/kernel/src/main.rs index 955194f9..56b66864 100644 --- a/kernel/src/main.rs +++ b/kernel/src/main.rs @@ -103,12 +103,7 @@ fn main(kbh: &KernelBootHeader) { logln!("Attached virt2phys provider!"); init_virt2phys_provider(); - let new_tables = unsafe { Virt2PhysMapping::inhearit_bootloader() }.unwrap(); - unsafe { new_tables.clone().load() }.unwrap(); - logln!("Page tables:\n{new_tables:#}"); - - let mut proc = mem::vm::VmProcess::new(); - proc.test(); + mem::vm::test(); logln!("Init VirtMemoryManager"); let kernel_process = unsafe { VmProcess::new_from_bootloader() };