mm: various improvements to PageRef

fuzz/fuzz_targets/page_alloc.rs

@@ -11,9 +11,10 @@ use libfuzzer_sys::fuzz_target;
 use std::collections::BTreeSet;
 use svsm::address::VirtAddr;
 use svsm::mm::alloc::{
-    allocate_file_page, allocate_file_page_ref, allocate_page, allocate_pages, allocate_slab_page,
-    allocate_zeroed_page, free_page, get_order, TestRootMem,
+    allocate_file_page, allocate_page, allocate_pages, allocate_slab_page, allocate_zeroed_page,
+    free_page, get_order, TestRootMem,
 };
+use svsm::mm::PageRef;
 use svsm::types::PAGE_SIZE;
 
 const WRITE_BYTE: u8 = 0x66;
@@ -131,7 +132,7 @@ fuzz_target!(|inp: FuzzInput| {
                 }
             }
             Action::AllocateFilePageRef => {
-                if let Ok(pageref) = allocate_file_page_ref() {
+                if let Ok(pageref) = PageRef::new() {
                     pagerefs.push(pageref);
                 }
             }

kernel/src/cpu/mem.rs

@@ -0,0 +1,39 @@
+use core::arch::asm;
+
+/// Copy `size` bytes from `src` to `dst`.
+///
+/// # Safety
+///
+/// This function has all the safety requirements of `core::ptr::copy` except
+/// that data races (both on `src` and `dst`) are explicitly permitted.
+#[inline(always)]
+pub unsafe fn copy_bytes(src: usize, dst: usize, size: usize) {
+    unsafe {
+        asm!(
+            "rep movsb",
+            inout("rsi") src => _,
+            inout("rdi") dst => _,
+            inout("rcx") size => _,
+            options(nostack),
+        );
+    }
+}
+
+/// Set `size` bytes at `dst` to `val`.
+///
+/// # Safety
+///
+/// This function has all the safety requirements of `core::ptr::write_bytes` except
+/// that data races are explicitly permitted.
+#[inline(always)]
+pub unsafe fn write_bytes(dst: usize, size: usize, value: u8) {
+    unsafe {
+        asm!(
+            "rep stosb",
+            inout("rdi") dst => _,
+            inout("rcx") size => _,
+            in("al") value,
+            options(nostack),
+        );
+    }
+}

kernel/src/cpu/mod.rs

@@ -13,6 +13,7 @@ pub mod features;
 pub mod gdt;
 pub mod idt;
 pub mod irq_state;
+pub mod mem;
 pub mod msr;
 pub mod percpu;
 pub mod registers;

kernel/src/fs/ramfs.rs

@@ -8,9 +8,9 @@ use super::*;
 
 use crate::error::SvsmError;
 use crate::locking::RWLock;
-use crate::mm::{allocate_file_page_ref, PageRef};
+use crate::mm::PageRef;
 use crate::types::{PAGE_SHIFT, PAGE_SIZE};
-use crate::utils::{page_align_up, page_offset, zero_mem_region};
+use crate::utils::{page_align_up, page_offset};
 
 extern crate alloc;
 use alloc::sync::Arc;
@@ -47,7 +47,7 @@ impl RawRamFile {
     /// [`Result<(), SvsmError>`]: A [`Result`] containing empty
     /// value if successful, SvsvError otherwise.
     fn increase_capacity(&mut self) -> Result<(), SvsmError> {
-        let page_ref = allocate_file_page_ref()?;
+        let page_ref = PageRef::new()?;
         self.pages.push(page_ref);
         self.capacity += PAGE_SIZE;
         Ok(())
@@ -87,13 +87,7 @@ impl RawRamFile {
     fn read_from_page(&self, buf: &mut [u8], offset: usize) {
         let page_index = page_offset(offset);
         let index = offset / PAGE_SIZE;
-        let len = buf.len();
-        let page_end = page_index + len;
-
-        assert!(page_end <= PAGE_SIZE);
-
-        let page_buf = self.pages[index].as_ref();
-        buf.copy_from_slice(&page_buf[page_index..page_end]);
+        self.pages[index].read(page_index, buf);
     }
 
     /// Used to write contents to a page corresponding to
@@ -106,16 +100,10 @@ impl RawRamFile {
     /// # Assert
     ///
     /// Assert that write operation doesn't extend beyond a page.
-    fn write_to_page(&mut self, buf: &[u8], offset: usize) {
+    fn write_to_page(&self, buf: &[u8], offset: usize) {
         let page_index = page_offset(offset);
         let index = offset / PAGE_SIZE;
-        let len = buf.len();
-        let page_end = page_index + len;
-
-        assert!(page_end <= PAGE_SIZE);
-
-        let page_buf = self.pages[index].as_mut();
-        page_buf[page_index..page_end].copy_from_slice(buf);
+        self.pages[index].write(page_index, buf);
     }
 
     /// Used to read the file from a particular offset.
@@ -218,10 +206,8 @@ impl RawRamFile {
         };
 
         // Clear pages and remove them from the file
-        while self.pages.len() > new_pages {
-            let page_ref = self.pages.pop().unwrap();
-            let vaddr = page_ref.virt_addr();
-            zero_mem_region(vaddr, vaddr + PAGE_SIZE);
+        for page_ref in self.pages.drain(new_pages..) {
+            page_ref.fill(0, 0);
         }
 
         self.capacity = new_pages * PAGE_SIZE;
@@ -230,8 +216,7 @@ impl RawRamFile {
         if offset > 0 {
             // Clear the last page after new EOF
             let page_ref = self.pages.last().unwrap();
-            let vaddr = page_ref.virt_addr();
-            zero_mem_region(vaddr + offset, vaddr + PAGE_SIZE);
+            page_ref.fill(offset, 0);
         }
 
         Ok(size)

kernel/src/mm/alloc.rs

@@ -5,6 +5,7 @@
 // Author: Joerg Roedel <[email protected]>
 
 use crate::address::{Address, PhysAddr, VirtAddr};
+use crate::cpu::mem::{copy_bytes, write_bytes};
 use crate::error::SvsmError;
 use crate::locking::SpinLock;
 use crate::mm::virt_to_phys;
@@ -909,17 +910,15 @@ pub struct PageRef {
 }
 
 impl PageRef {
-    /// Creates a new [`PageRef`] instance with the given virtual and physical addresses.
-    ///
-    /// # Arguments
-    ///
-    /// * `virt_addr` - Virtual address of the memory page.
-    /// * `phys_addr` - Physical address of the memory page.
-    pub const fn new(virt_addr: VirtAddr, phys_addr: PhysAddr) -> Self {
-        Self {
+    /// Allocate a reference-counted file page.
+    pub fn new() -> Result<Self, SvsmError> {
+        let virt_addr = allocate_file_page()?;
+        let phys_addr = virt_to_phys(virt_addr);
+
+        Ok(Self {
             virt_addr,
             phys_addr,
-        }
+        })
     }
 
     /// Returns the virtual address of the memory page.
@@ -934,31 +933,53 @@ impl PageRef {
 
     pub fn try_copy_page(&self) -> Result<Self, SvsmError> {
         let virt_addr = allocate_file_page()?;
+
+        let src = self.virt_addr.bits();
+        let dst = virt_addr.bits();
+        let size = PAGE_SIZE;
         unsafe {
-            let src = self.virt_addr.as_ptr::<[u8; PAGE_SIZE]>();
-            let dst = virt_addr.as_mut_ptr::<[u8; PAGE_SIZE]>();
-            ptr::copy_nonoverlapping(src, dst, 1);
+            // SAFETY: `src` and `dst` are both valid.
+            copy_bytes(src, dst, size);
         }
+
         Ok(PageRef {
             virt_addr,
             phys_addr: virt_to_phys(virt_addr),
         })
     }
-}
 
-impl AsRef<[u8; PAGE_SIZE]> for PageRef {
-    /// Returns a reference to the underlying array representing the memory page.
-    fn as_ref(&self) -> &[u8; PAGE_SIZE] {
-        let ptr = self.virt_addr.as_ptr::<[u8; PAGE_SIZE]>();
-        unsafe { ptr.as_ref().unwrap() }
+    pub fn write(&self, offset: usize, buf: &[u8]) {
+        assert!(offset.checked_add(buf.len()).unwrap() <= PAGE_SIZE);
+
+        let src = buf.as_ptr() as usize;
+        let dst = self.virt_addr.bits() + offset;
+        let size = buf.len();
+        unsafe {
+            // SAFETY: `src` and `dst` are both valid.
+            copy_bytes(src, dst, size);
+        }
     }
-}
 
-impl AsMut<[u8; PAGE_SIZE]> for PageRef {
-    /// Returns a mutable reference to the underlying array representing the memory page.
-    fn as_mut(&mut self) -> &mut [u8; PAGE_SIZE] {
-        let ptr = self.virt_addr.as_mut_ptr::<[u8; PAGE_SIZE]>();
-        unsafe { ptr.as_mut().unwrap() }
+    pub fn read(&self, offset: usize, buf: &mut [u8]) {
+        assert!(offset.checked_add(buf.len()).unwrap() <= PAGE_SIZE);
+
+        let src = self.virt_addr.bits() + offset;
+        let dst = buf.as_mut_ptr() as usize;
+        let size = buf.len();
+        unsafe {
+            // SAFETY: `src` and `dst` are both valid.
+            copy_bytes(src, dst, size);
+        }
+    }
+
+    pub fn fill(&self, offset: usize, value: u8) {
+        let dst = self.virt_addr.bits() + offset;
+        let size = PAGE_SIZE.checked_sub(offset).unwrap();
+
+        unsafe {
+            // SAFETY: `dst` is valid.
+            write_bytes(dst, size, value);
+        }
     }
 }
 
@@ -1074,19 +1095,6 @@ pub fn allocate_file_page() -> Result<VirtAddr, SvsmError> {
     Ok(vaddr)
 }
 
-/// Allocate a reference-counted file page.
-///
-/// # Returns
-///
-/// Result containing a page reference to the virtual address of the
-/// allocated file page or an `SvsmError` if allocation fails.
-pub fn allocate_file_page_ref() -> Result<PageRef, SvsmError> {
-    let v = allocate_file_page()?;
-    let p = virt_to_phys(v);
-
-    Ok(PageRef::new(v, p))
-}
-
 fn get_file_page(vaddr: VirtAddr) -> Result<(), SvsmError> {
     Ok(ROOT_MEM.lock().get_file_page(vaddr)?)
 }

kernel/src/mm/mod.rs

@@ -25,6 +25,6 @@ pub use ptguards::*;
 
 pub use pagetable::PageTablePart;
 
-pub use alloc::{allocate_file_page, allocate_file_page_ref, PageRef};
+pub use alloc::{allocate_file_page, PageRef};
 
 pub use mappings::{mmap_kernel, mmap_user, munmap_kernel, munmap_user, VMMappingGuard};

kernel/src/mm/vm/mapping/rawalloc.rs

@@ -8,7 +8,7 @@ use core::iter;
 
 use crate::address::PhysAddr;
 use crate::error::SvsmError;
-use crate::mm::alloc::{allocate_file_page_ref, PageRef};
+use crate::mm::alloc::PageRef;
 use crate::types::{PAGE_SHIFT, PAGE_SIZE};
 use crate::utils::align_up;
 
@@ -56,7 +56,7 @@ impl RawAllocMapping {
         let index = offset >> PAGE_SHIFT;
         if index < self.count {
             let entry = self.pages.get_mut(index).ok_or(SvsmError::Mem)?;
-            entry.get_or_insert(allocate_file_page_ref()?);
+            entry.get_or_insert(PageRef::new()?);
         }
         Ok(())
     }