Refactor architecture-specific page fault handling

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<Self, ()> {
+        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,
+        })
+    }
+}

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<CpuExceptionInfo>` 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(trap_info);
-    log_trap_info(exception, trap_info);
+
+    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;
+        }
+    }
 
-    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);
-        }
-    }
-}
 
 /// 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<Full>,
-    trap_info: &CpuExceptionInfo,
+    page_fault_info: &PageFaultInfo,
 ) -> core::result::Result<(), ()> {
-    const PAGE_NOT_PRESENT_ERROR_MASK: usize = 0x1 << 0;
+    if let Err(e) = root_vmar.handle_page_fault(page_fault_info) {
-    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
+            page_fault_info.address, e
         );
         return Err(());
     }
     Ok(())
-    } else {
-        // Otherwise, the page fault cannot be handled
-        Err(())
-    }
 }
 
 /// 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) {
+fn log_trap_info(trap_info: &CpuExceptionInfo) {
-    match *exception {
+    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),

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.
+    /// Handle a page fault, whose information is provided in `page_fault_info`.
-    /// Otherwise, it's caused by page protection error.
+    ///
-    /// if write is true, the page fault is caused by a write access,
+    /// Returns `Ok` if the page fault is handled successfully, `Err` otherwise.
-    /// otherwise, the page fault is caused by a read access.
+    fn handle_page_fault(&self, page_fault_info: &PageFaultInfo) -> Result<()>;
-    /// 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<()>;
 }

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<Rights> {
     /// Creates a root VMAR.
@@ -144,19 +146,9 @@ impl Vmar<Rights> {
 }
 
 impl PageFaultHandler for Vmar<Rights> {
-    fn handle_page_fault(
+    fn handle_page_fault(&self, page_fault_info: &PageFaultInfo) -> Result<()> {
-        &self,
+        self.check_rights(page_fault_info.required_perms.into())?;
-        page_fault_addr: Vaddr,
+        self.0.handle_page_fault(page_fault_info)
-        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)
     }
 }
 

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<R> VmarRightsOp for Vmar<R> {
 
 // TODO: how page faults can be delivered to and handled by the current VMAR.
 impl<R> PageFaultHandler for Vmar<R> {
-    default fn handle_page_fault(
+    default fn handle_page_fault(&self, _page_fault_info: &PageFaultInfo) -> Result<()> {
-        &self,
-        page_fault_addr: Vaddr,
-        not_present: bool,
-        write: bool,
-    ) -> Result<()> {
         unimplemented!()
     }
 }
@@ -218,6 +220,13 @@ impl Vmar_ {
     }
 
     fn new_root() -> Arc<Self> {
+        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)
     }
 
@@ -299,32 +308,22 @@ impl Vmar_ {
     }
 
     /// Handles user space page fault, if the page fault is successfully handled, return Ok(()).
-    fn handle_page_fault(
+    pub fn handle_page_fault(&self, page_fault_info: &PageFaultInfo) -> Result<()> {
-        &self,
+        let address = page_fault_info.address;
-        page_fault_addr: Vaddr,
+        if !(self.base..self.base + self.size).contains(&address) {
-        not_present: bool,
-        write: bool,
-    ) -> Result<()> {
-        if page_fault_addr < self.base || page_fault_addr >= self.base + self.size {
             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) {
+        if let Some(child_vmar) = inner.child_vmar_s.find_one(&address) {
-            debug_assert!(is_intersected(
+            debug_assert!(child_vmar.range().contains(&address));
-                &child_vmar.range(),
+            return child_vmar.handle_page_fault(page_fault_info);
-                &(page_fault_addr..page_fault_addr + 1)
-            ));
-            return child_vmar.handle_page_fault(page_fault_addr, not_present, write);
         }
 
         // 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) {
+        if let Some(vm_mapping) = inner.vm_mappings.find_one(&address) {
-            debug_assert!(is_intersected(
+            debug_assert!(vm_mapping.range().contains(&address));
-                &vm_mapping.range(),
+            return vm_mapping.handle_page_fault(page_fault_info);
-                &(page_fault_addr..page_fault_addr + 1)
-            ));
-            return vm_mapping.handle_page_fault(page_fault_addr, not_present, write);
         }
 
         return_errno_with_message!(Errno::EACCES, "page fault addr is not in current vmar");

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

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<R: TRights> Vmar<TRightSet<R>> {
     /// Creates a root VMAR.
@@ -169,19 +171,9 @@ impl<R: TRights> Vmar<TRightSet<R>> {
 }
 
 impl<R: TRights> PageFaultHandler for Vmar<TRightSet<R>> {
-    fn handle_page_fault(
+    fn handle_page_fault(&self, page_fault_info: &PageFaultInfo) -> Result<()> {
-        &self,
+        self.check_rights(page_fault_info.required_perms.into())?;
-        page_fault_addr: Vaddr,
+        self.0.handle_page_fault(page_fault_info)
-        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)
     }
 }
 

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,38 +204,31 @@ impl VmMapping {
         self.inner.lock().map_size += extra_size;
     }
 
-    pub fn handle_page_fault(
+    pub fn handle_page_fault(&self, page_fault_info: &PageFaultInfo) -> Result<()> {
-        &self,
+        self.check_perms(&page_fault_info.required_perms)?;
-        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)?;
 
-        if !write && self.vmo.is_some() && self.handle_page_faults_around {
+        let address = page_fault_info.address;
-            self.handle_page_faults_around(page_fault_addr)?;
+
+        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.
+        match cursor.query().unwrap() {
-        if write && !not_present {
+            VmItem::Mapped {
-            let VmItem::Mapped {
                 va: _,
                 frame,
                 mut prop,
-            } = current_mapping
+            } if is_write => {
-            else {
+                // Perform COW if it is a write access to a shared mapping.
-                return Err(Error::new(Errno::EFAULT));
-            };
 
                 // Skip if the page fault is already handled.
                 if prop.flags.contains(PageFlags::W) {
@@ -248,21 +242,24 @@ impl VmMapping {
                 // 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 |= PageFlags::W);
+                    cursor.protect(PAGE_SIZE, |p| p.flags |= new_flags);
                 } else {
                     let new_frame = duplicate_frame(&frame)?;
-                prop.flags |= PageFlags::W | PageFlags::ACCESSED | PageFlags::DIRTY;
+                    prop.flags |= new_flags;
                     cursor.map(new_frame, prop);
                 }
-            return Ok(());
             }
-
+            VmItem::Mapped { .. } => {
+                panic!("non-COW page fault should not happen on mapped address")
+            }
+            VmItem::NotMapped { .. } => {
                 // 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 (frame, is_readonly) = self.prepare_page(&inner_lock, address, is_write)?;
 
                 let vm_perms = {
                     let mut perms = inner_lock.perms;
@@ -274,13 +271,14 @@ impl VmMapping {
                 };
                 let mut page_flags = vm_perms.into();
                 page_flags |= PageFlags::ACCESSED;
-            if write {
+                if is_write {
                     page_flags |= PageFlags::DIRTY;
                 }
                 let map_prop = PageProperty::new(page_flags, CachePolicy::Writeback);
 
                 cursor.map(frame, map_prop);
             }
+        }
 
         Ok(())
     }

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