From 2a6733579d292c706e3933258c0049e013e3d98f Mon Sep 17 00:00:00 2001 From: YanWQ-monad Date: Tue, 23 Jul 2024 10:09:02 +0800 Subject: [PATCH] Refactor architecture-specific page fault handling --- kernel/src/arch/x86/cpu.rs | 32 +++++++- kernel/src/thread/exception.rs | 73 ++++++++--------- kernel/src/vm/page_fault_handler.rs | 13 ++- kernel/src/vm/vmar/dyn_cap.rs | 20 ++--- kernel/src/vm/vmar/mod.rs | 57 +++++++------ kernel/src/vm/vmar/options.rs | 16 +++- kernel/src/vm/vmar/static_cap.rs | 20 ++--- kernel/src/vm/vmar/vm_mapping.rs | 122 ++++++++++++++-------------- ostd/src/arch/x86/cpu/mod.rs | 11 ++- 9 files changed, 190 insertions(+), 174 deletions(-) diff --git a/kernel/src/arch/x86/cpu.rs b/kernel/src/arch/x86/cpu.rs index 4eda6f996..f90067df2 100644 --- a/kernel/src/arch/x86/cpu.rs +++ b/kernel/src/arch/x86/cpu.rs @@ -1,11 +1,11 @@ // SPDX-License-Identifier: MPL-2.0 use ostd::{ - cpu::{RawGeneralRegs, UserContext}, + cpu::{CpuExceptionInfo, RawGeneralRegs, UserContext, PAGE_FAULT}, Pod, }; -use crate::cpu::LinuxAbi; +use crate::{cpu::LinuxAbi, thread::exception::PageFaultInfo, vm::perms::VmPerms}; impl LinuxAbi for UserContext { fn syscall_num(&self) -> usize { @@ -100,3 +100,31 @@ impl GpRegs { copy_gp_regs!(src, self); } } + +impl TryFrom<&CpuExceptionInfo> for PageFaultInfo { + // [`Err`] indicates that the [`CpuExceptionInfo`] is not a page fault, + // with no additional error information. + type Error = (); + + fn try_from(value: &CpuExceptionInfo) -> Result { + if value.cpu_exception() != PAGE_FAULT { + return Err(()); + } + + const WRITE_ACCESS_MASK: usize = 0x1 << 1; + const INSTRUCTION_FETCH_MASK: usize = 0x1 << 4; + + let required_perms = if value.error_code & INSTRUCTION_FETCH_MASK != 0 { + VmPerms::EXEC + } else if value.error_code & WRITE_ACCESS_MASK != 0 { + VmPerms::WRITE + } else { + VmPerms::READ + }; + + Ok(PageFaultInfo { + address: value.page_fault_addr, + required_perms, + }) + } +} diff --git a/kernel/src/thread/exception.rs b/kernel/src/thread/exception.rs index 6b6317150..cb899d5ae 100644 --- a/kernel/src/thread/exception.rs +++ b/kernel/src/thread/exception.rs @@ -8,32 +8,41 @@ use ostd::{cpu::*, mm::VmSpace}; use crate::{ prelude::*, process::signal::signals::fault::FaultSignal, - vm::{page_fault_handler::PageFaultHandler, vmar::Vmar}, + vm::{page_fault_handler::PageFaultHandler, perms::VmPerms, vmar::Vmar}, }; +/// Page fault information converted from [`CpuExceptionInfo`]. +/// +/// `From` should be implemented for this struct. +/// If `CpuExceptionInfo` is a page fault, `try_from` should return `Ok(PageFaultInfo)`, +/// or `Err(())` (no error information) otherwise. +pub struct PageFaultInfo { + /// The virtual address where a page fault occurred. + pub address: Vaddr, + + /// The [`VmPerms`] required by the memory operation that causes page fault. + /// For example, a "store" operation may require `VmPerms::WRITE`. + pub required_perms: VmPerms, +} + /// We can't handle most exceptions, just send self a fault signal before return to user space. pub fn handle_exception(ctx: &Context, context: &UserContext) { let trap_info = context.trap_information(); - let exception = CpuException::to_cpu_exception(trap_info.id as u16).unwrap(); - log_trap_info(exception, trap_info); + log_trap_info(trap_info); - match *exception { - PAGE_FAULT => { - if handle_page_fault_from_vmar(ctx.process.root_vmar(), trap_info).is_err() { - generate_fault_signal(trap_info, ctx); - } - } - _ => { - // We current do nothing about other exceptions - generate_fault_signal(trap_info, ctx); + if let Ok(page_fault_info) = PageFaultInfo::try_from(trap_info) { + if handle_page_fault_from_vmar(ctx.process.root_vmar(), &page_fault_info).is_ok() { + return; } } + + generate_fault_signal(trap_info, ctx); } /// Handles the page fault occurs in the input `VmSpace`. pub(crate) fn handle_page_fault_from_vm_space( vm_space: &VmSpace, - trap_info: &CpuExceptionInfo, + page_fault_info: &PageFaultInfo, ) -> core::result::Result<(), ()> { let current = current!(); let root_vmar = current.root_vmar(); @@ -44,38 +53,22 @@ pub(crate) fn handle_page_fault_from_vm_space( vm_space as *const VmSpace ); - handle_page_fault_from_vmar(root_vmar, trap_info) + handle_page_fault_from_vmar(root_vmar, page_fault_info) } /// Handles the page fault occurs in the input `Vmar`. pub(crate) fn handle_page_fault_from_vmar( root_vmar: &Vmar, - trap_info: &CpuExceptionInfo, + page_fault_info: &PageFaultInfo, ) -> core::result::Result<(), ()> { - const PAGE_NOT_PRESENT_ERROR_MASK: usize = 0x1 << 0; - const WRITE_ACCESS_MASK: usize = 0x1 << 1; - let page_fault_addr = trap_info.page_fault_addr as Vaddr; - trace!( - "page fault error code: 0x{:x}, Page fault address: 0x{:x}", - trap_info.error_code, - page_fault_addr - ); - - let not_present = trap_info.error_code & PAGE_NOT_PRESENT_ERROR_MASK == 0; - let write = trap_info.error_code & WRITE_ACCESS_MASK != 0; - if not_present || write { - if let Err(e) = root_vmar.handle_page_fault(page_fault_addr, not_present, write) { - warn!( - "page fault handler failed: addr: 0x{:x}, err: {:?}", - page_fault_addr, e - ); - return Err(()); - } - Ok(()) - } else { - // Otherwise, the page fault cannot be handled - Err(()) + if let Err(e) = root_vmar.handle_page_fault(page_fault_info) { + warn!( + "page fault handler failed: addr: 0x{:x}, err: {:?}", + page_fault_info.address, e + ); + return Err(()); } + Ok(()) } /// generate a fault signal for current process. @@ -94,8 +87,8 @@ macro_rules! log_trap_common { }; } -fn log_trap_info(exception: &CpuException, trap_info: &CpuExceptionInfo) { - match *exception { +fn log_trap_info(trap_info: &CpuExceptionInfo) { + match trap_info.cpu_exception() { DIVIDE_BY_ZERO => log_trap_common!(DIVIDE_BY_ZERO, trap_info), DEBUG => log_trap_common!(DEBUG, trap_info), NON_MASKABLE_INTERRUPT => log_trap_common!(NON_MASKABLE_INTERRUPT, trap_info), diff --git a/kernel/src/vm/page_fault_handler.rs b/kernel/src/vm/page_fault_handler.rs index 9686637d8..6d2a9f92a 100644 --- a/kernel/src/vm/page_fault_handler.rs +++ b/kernel/src/vm/page_fault_handler.rs @@ -1,14 +1,11 @@ // SPDX-License-Identifier: MPL-2.0 -use crate::prelude::*; +use crate::{prelude::*, thread::exception::PageFaultInfo}; /// This trait is implemented by structs which can handle a user space page fault. pub trait PageFaultHandler { - /// Handle a page fault at a specific addr. if not_present is true, the page fault is caused by page not present. - /// Otherwise, it's caused by page protection error. - /// if write is true, the page fault is caused by a write access, - /// otherwise, the page fault is caused by a read access. - /// If the page fault can be handled successfully, this function will return Ok(()). - /// Otherwise, this function will return Err. - fn handle_page_fault(&self, offset: Vaddr, not_present: bool, write: bool) -> Result<()>; + /// Handle a page fault, whose information is provided in `page_fault_info`. + /// + /// Returns `Ok` if the page fault is handled successfully, `Err` otherwise. + fn handle_page_fault(&self, page_fault_info: &PageFaultInfo) -> Result<()>; } diff --git a/kernel/src/vm/vmar/dyn_cap.rs b/kernel/src/vm/vmar/dyn_cap.rs index a660cf2bc..3e5ff837b 100644 --- a/kernel/src/vm/vmar/dyn_cap.rs +++ b/kernel/src/vm/vmar/dyn_cap.rs @@ -7,7 +7,9 @@ use aster_rights::Rights; use super::{ options::VmarChildOptions, vm_mapping::VmarMapOptions, VmPerms, Vmar, VmarRightsOp, Vmar_, }; -use crate::{prelude::*, vm::page_fault_handler::PageFaultHandler}; +use crate::{ + prelude::*, thread::exception::PageFaultInfo, vm::page_fault_handler::PageFaultHandler, +}; impl Vmar { /// Creates a root VMAR. @@ -144,19 +146,9 @@ impl Vmar { } impl PageFaultHandler for Vmar { - fn handle_page_fault( - &self, - page_fault_addr: Vaddr, - not_present: bool, - write: bool, - ) -> Result<()> { - if write { - self.check_rights(Rights::WRITE)?; - } else { - self.check_rights(Rights::READ)?; - } - self.0 - .handle_page_fault(page_fault_addr, not_present, write) + fn handle_page_fault(&self, page_fault_info: &PageFaultInfo) -> Result<()> { + self.check_rights(page_fault_info.required_perms.into())?; + self.0.handle_page_fault(page_fault_info) } } diff --git a/kernel/src/vm/vmar/mod.rs b/kernel/src/vm/vmar/mod.rs index c2089611e..f691a4045 100644 --- a/kernel/src/vm/vmar/mod.rs +++ b/kernel/src/vm/vmar/mod.rs @@ -14,14 +14,21 @@ use core::ops::Range; use align_ext::AlignExt; use aster_rights::Rights; -use ostd::mm::{VmSpace, MAX_USERSPACE_VADDR}; +use ostd::{ + cpu::CpuExceptionInfo, + mm::{VmSpace, MAX_USERSPACE_VADDR}, +}; use self::{ interval::{Interval, IntervalSet}, vm_mapping::VmMapping, }; use super::page_fault_handler::PageFaultHandler; -use crate::{prelude::*, thread::exception::handle_page_fault_from_vm_space, vm::perms::VmPerms}; +use crate::{ + prelude::*, + thread::exception::{handle_page_fault_from_vm_space, PageFaultInfo}, + vm::perms::VmPerms, +}; /// Virtual Memory Address Regions (VMARs) are a type of capability that manages /// user address spaces. @@ -72,12 +79,7 @@ impl VmarRightsOp for Vmar { // TODO: how page faults can be delivered to and handled by the current VMAR. impl PageFaultHandler for Vmar { - default fn handle_page_fault( - &self, - page_fault_addr: Vaddr, - not_present: bool, - write: bool, - ) -> Result<()> { + default fn handle_page_fault(&self, _page_fault_info: &PageFaultInfo) -> Result<()> { unimplemented!() } } @@ -218,6 +220,13 @@ impl Vmar_ { } fn new_root() -> Arc { + fn handle_page_fault_wrapper( + vm_space: &VmSpace, + trap_info: &CpuExceptionInfo, + ) -> core::result::Result<(), ()> { + handle_page_fault_from_vm_space(vm_space, &trap_info.try_into().unwrap()) + } + let mut free_regions = BTreeMap::new(); let root_region = FreeRegion::new(ROOT_VMAR_LOWEST_ADDR..ROOT_VMAR_CAP_ADDR); free_regions.insert(root_region.start(), root_region); @@ -228,7 +237,7 @@ impl Vmar_ { free_regions, }; let vm_space = VmSpace::new(); - vm_space.register_page_fault_handler(handle_page_fault_from_vm_space); + vm_space.register_page_fault_handler(handle_page_fault_wrapper); Vmar_::new(vmar_inner, Arc::new(vm_space), 0, ROOT_VMAR_CAP_ADDR, None) } @@ -298,33 +307,23 @@ impl Vmar_ { Ok(()) } - /// Handles user space page fault, if the page fault is successfully handled ,return Ok(()). - fn handle_page_fault( - &self, - page_fault_addr: Vaddr, - not_present: bool, - write: bool, - ) -> Result<()> { - if page_fault_addr < self.base || page_fault_addr >= self.base + self.size { + /// Handles user space page fault, if the page fault is successfully handled, return Ok(()). + pub fn handle_page_fault(&self, page_fault_info: &PageFaultInfo) -> Result<()> { + let address = page_fault_info.address; + if !(self.base..self.base + self.size).contains(&address) { return_errno_with_message!(Errno::EACCES, "page fault addr is not in current vmar"); } let inner = self.inner.lock(); - if let Some(child_vmar) = inner.child_vmar_s.find_one(&page_fault_addr) { - debug_assert!(is_intersected( - &child_vmar.range(), - &(page_fault_addr..page_fault_addr + 1) - )); - return child_vmar.handle_page_fault(page_fault_addr, not_present, write); + if let Some(child_vmar) = inner.child_vmar_s.find_one(&address) { + debug_assert!(child_vmar.range().contains(&address)); + return child_vmar.handle_page_fault(page_fault_info); } // FIXME: If multiple VMOs are mapped to the addr, should we allow all VMOs to handle page fault? - if let Some(vm_mapping) = inner.vm_mappings.find_one(&page_fault_addr) { - debug_assert!(is_intersected( - &vm_mapping.range(), - &(page_fault_addr..page_fault_addr + 1) - )); - return vm_mapping.handle_page_fault(page_fault_addr, not_present, write); + if let Some(vm_mapping) = inner.vm_mappings.find_one(&address) { + debug_assert!(vm_mapping.range().contains(&address)); + return vm_mapping.handle_page_fault(page_fault_info); } return_errno_with_message!(Errno::EACCES, "page fault addr is not in current vmar"); diff --git a/kernel/src/vm/vmar/options.rs b/kernel/src/vm/vmar/options.rs index 8af45336f..935e5928c 100644 --- a/kernel/src/vm/vmar/options.rs +++ b/kernel/src/vm/vmar/options.rs @@ -142,7 +142,7 @@ mod test { use crate::vm::{ page_fault_handler::PageFaultHandler, perms::VmPerms, - vmar::ROOT_VMAR_CAP_ADDR, + vmar::{PageFaultInfo, ROOT_VMAR_CAP_ADDR}, vmo::{VmoOptions, VmoRightsOp}, }; @@ -192,7 +192,12 @@ mod test { const OFFSET: usize = 0x1000_0000; let root_vmar = Vmar::::new_root(); // the page is not mapped by a vmo - assert!(root_vmar.handle_page_fault(OFFSET, true, true).is_err()); + assert!(root_vmar + .handle_page_fault(&PageFaultInfo { + address: OFFSET, + required_perms: VmPerms::WRITE, + }) + .is_err()); // the page is mapped READ let vmo = VmoOptions::::new(PAGE_SIZE).alloc().unwrap().to_dyn(); let perms = VmPerms::READ; @@ -204,6 +209,11 @@ mod test { .offset(OFFSET) .build() .unwrap(); - root_vmar.handle_page_fault(OFFSET, true, false).unwrap(); + root_vmar + .handle_page_fault(&PageFaultInfo { + address: OFFSET, + required_perms: VmPerms::READ, + }) + .unwrap(); } } diff --git a/kernel/src/vm/vmar/static_cap.rs b/kernel/src/vm/vmar/static_cap.rs index 25653fb8f..be1a44115 100644 --- a/kernel/src/vm/vmar/static_cap.rs +++ b/kernel/src/vm/vmar/static_cap.rs @@ -8,7 +8,9 @@ use aster_rights_proc::require; use super::{ options::VmarChildOptions, vm_mapping::VmarMapOptions, VmPerms, Vmar, VmarRightsOp, Vmar_, }; -use crate::{prelude::*, vm::page_fault_handler::PageFaultHandler}; +use crate::{ + prelude::*, thread::exception::PageFaultInfo, vm::page_fault_handler::PageFaultHandler, +}; impl Vmar> { /// Creates a root VMAR. @@ -169,19 +171,9 @@ impl Vmar> { } impl PageFaultHandler for Vmar> { - fn handle_page_fault( - &self, - page_fault_addr: Vaddr, - not_present: bool, - write: bool, - ) -> Result<()> { - if write { - self.check_rights(Rights::WRITE)?; - } else { - self.check_rights(Rights::READ)?; - } - self.0 - .handle_page_fault(page_fault_addr, not_present, write) + fn handle_page_fault(&self, page_fault_info: &PageFaultInfo) -> Result<()> { + self.check_rights(page_fault_info.required_perms.into())?; + self.0.handle_page_fault(page_fault_info) } } diff --git a/kernel/src/vm/vmar/vm_mapping.rs b/kernel/src/vm/vmar/vm_mapping.rs index 7fa18d152..b5d818e66 100644 --- a/kernel/src/vm/vmar/vm_mapping.rs +++ b/kernel/src/vm/vmar/vm_mapping.rs @@ -17,6 +17,7 @@ use ostd::mm::{ use super::{interval::Interval, is_intersected, Vmar, Vmar_}; use crate::{ prelude::*, + thread::exception::PageFaultInfo, vm::{ perms::VmPerms, util::duplicate_frame, @@ -203,83 +204,80 @@ impl VmMapping { self.inner.lock().map_size += extra_size; } - pub fn handle_page_fault( - &self, - page_fault_addr: Vaddr, - not_present: bool, - write: bool, - ) -> Result<()> { - let required_perm = if write { VmPerms::WRITE } else { VmPerms::READ }; - self.check_perms(&required_perm)?; + pub fn handle_page_fault(&self, page_fault_info: &PageFaultInfo) -> Result<()> { + self.check_perms(&page_fault_info.required_perms)?; - if !write && self.vmo.is_some() && self.handle_page_faults_around { - self.handle_page_faults_around(page_fault_addr)?; + let address = page_fault_info.address; + + let page_aligned_addr = address.align_down(PAGE_SIZE); + let is_write = page_fault_info.required_perms.contains(VmPerms::WRITE); + + if !is_write && self.vmo.is_some() && self.handle_page_faults_around { + self.handle_page_faults_around(address)?; return Ok(()); } - let page_aligned_addr = page_fault_addr.align_down(PAGE_SIZE); - let root_vmar = self.parent.upgrade().unwrap(); let mut cursor = root_vmar .vm_space() .cursor_mut(&(page_aligned_addr..page_aligned_addr + PAGE_SIZE))?; - let current_mapping = cursor.query().unwrap(); - // Perform COW if it is a write access to a shared mapping. - if write && !not_present { - let VmItem::Mapped { + match cursor.query().unwrap() { + VmItem::Mapped { va: _, frame, mut prop, - } = current_mapping - else { - return Err(Error::new(Errno::EFAULT)); - }; + } if is_write => { + // Perform COW if it is a write access to a shared mapping. - // Skip if the page fault is already handled. - if prop.flags.contains(PageFlags::W) { - return Ok(()); - } - - // If the forked child or parent immediately unmaps the page after - // the fork without accessing it, we are the only reference to the - // frame. We can directly map the frame as writable without - // copying. In this case, the reference count of the frame is 2 ( - // one for the mapping and one for the frame handle itself). - let only_reference = frame.reference_count() == 2; - - if self.is_shared || only_reference { - cursor.protect(PAGE_SIZE, |p| p.flags |= PageFlags::W); - } else { - let new_frame = duplicate_frame(&frame)?; - prop.flags |= PageFlags::W | PageFlags::ACCESSED | PageFlags::DIRTY; - cursor.map(new_frame, prop); - } - return Ok(()); - } - - // Map a new frame to the page fault address. - // Skip if the page fault is already handled. - if let VmItem::NotMapped { .. } = current_mapping { - let inner_lock = self.inner.lock(); - let (frame, is_readonly) = self.prepare_page(&inner_lock, page_fault_addr, write)?; - - let vm_perms = { - let mut perms = inner_lock.perms; - if is_readonly { - // COW pages are forced to be read-only. - perms -= VmPerms::WRITE; + // Skip if the page fault is already handled. + if prop.flags.contains(PageFlags::W) { + return Ok(()); } - perms - }; - let mut page_flags = vm_perms.into(); - page_flags |= PageFlags::ACCESSED; - if write { - page_flags |= PageFlags::DIRTY; - } - let map_prop = PageProperty::new(page_flags, CachePolicy::Writeback); - cursor.map(frame, map_prop); + // If the forked child or parent immediately unmaps the page after + // the fork without accessing it, we are the only reference to the + // frame. We can directly map the frame as writable without + // copying. In this case, the reference count of the frame is 2 ( + // one for the mapping and one for the frame handle itself). + let only_reference = frame.reference_count() == 2; + + let new_flags = PageFlags::W | PageFlags::ACCESSED | PageFlags::DIRTY; + + if self.is_shared || only_reference { + cursor.protect(PAGE_SIZE, |p| p.flags |= new_flags); + } else { + let new_frame = duplicate_frame(&frame)?; + prop.flags |= new_flags; + cursor.map(new_frame, prop); + } + } + VmItem::Mapped { .. } => { + panic!("non-COW page fault should not happen on mapped address") + } + VmItem::NotMapped { .. } => { + // Map a new frame to the page fault address. + + let inner_lock = self.inner.lock(); + let (frame, is_readonly) = self.prepare_page(&inner_lock, address, is_write)?; + + let vm_perms = { + let mut perms = inner_lock.perms; + if is_readonly { + // COW pages are forced to be read-only. + perms -= VmPerms::WRITE; + } + perms + }; + let mut page_flags = vm_perms.into(); + page_flags |= PageFlags::ACCESSED; + if is_write { + page_flags |= PageFlags::DIRTY; + } + let map_prop = PageProperty::new(page_flags, CachePolicy::Writeback); + + cursor.map(frame, map_prop); + } } Ok(()) diff --git a/ostd/src/arch/x86/cpu/mod.rs b/ostd/src/arch/x86/cpu/mod.rs index 1c2411a91..5eccb68ae 100644 --- a/ostd/src/arch/x86/cpu/mod.rs +++ b/ostd/src/arch/x86/cpu/mod.rs @@ -196,7 +196,7 @@ impl UserContextApiInternal for UserContext { /// /// But there exists some vector which are special. Vector 1 can be both fault or trap and vector 2 is interrupt. /// So here we also define FaultOrTrap and Interrupt -#[derive(PartialEq, Eq, Debug)] +#[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum CpuExceptionType { /// CPU faults. Faults can be corrected, and the program may continue as if nothing happened. Fault, @@ -213,7 +213,7 @@ pub enum CpuExceptionType { } /// CPU exception. -#[derive(Debug, Eq, PartialEq)] +#[derive(Debug, Copy, Clone, Eq, PartialEq)] pub struct CpuException { /// The ID of the CPU exception. pub number: u16, @@ -323,6 +323,13 @@ impl CpuException { } } +impl CpuExceptionInfo { + /// Get corresponding CPU exception + pub fn cpu_exception(&self) -> CpuException { + EXCEPTION_LIST[self.id] + } +} + impl UserContextApi for UserContext { fn trap_number(&self) -> usize { self.user_context.trap_num