Merge pull request #335 from 00xc/utils/memory_region

Use `SvsmPlatform` page state change methods and other cleanups

kernel/src/fw_meta.rs

@@ -12,7 +12,7 @@ use crate::cpu::ghcb::current_ghcb;
 use crate::error::SvsmError;
 use crate::kernel_region::new_kernel_region;
 use crate::mm::PerCPUPageMappingGuard;
-use crate::sev::ghcb::PageStateChangeOp;
+use crate::platform::PageStateChangeOp;
 use crate::sev::{pvalidate, rmp_adjust, PvalidateOp, RMPFlags};
 use crate::types::{PageSize, PAGE_SIZE};
 use crate::utils::{zero_mem_region, MemoryRegion};
@@ -370,12 +370,7 @@ fn validate_fw_mem_region(
 
     if config.page_state_change_required() {
         current_ghcb()
-            .page_state_change(
-                pstart,
-                pend,
-                PageSize::Regular,
-                PageStateChangeOp::PscPrivate,
-            )
+            .page_state_change(region, PageSize::Regular, PageStateChangeOp::Private)
             .expect("GHCB PSC call failed to validate firmware memory");
     }
 

kernel/src/greq/msg.rs

@@ -20,13 +20,12 @@ use core::{
 
 use crate::{
     address::{Address, VirtAddr},
-    cpu::ghcb::current_ghcb,
-    cpu::percpu::this_cpu_mut,
     crypto::aead::{Aes256Gcm, Aes256GcmTrait, AUTHTAG_SIZE, IV_SIZE},
-    mm::virt_to_phys,
+    mm::page_visibility::{make_page_private, make_page_shared},
     protocols::errors::SvsmReqError,
-    sev::{ghcb::PageStateChangeOp, secrets_page::VMPCK_SIZE},
+    sev::secrets_page::VMPCK_SIZE,
     types::{PageSize, PAGE_SIZE},
+    utils::MemoryRegion,
 };
 
 /// Version of the message header
@@ -239,39 +238,13 @@ impl SnpGuestRequestMsg {
     ///   (returned to the allocator)
     pub fn set_shared(&mut self) -> Result<(), SvsmReqError> {
         let vaddr = VirtAddr::from(addr_of_mut!(*self));
-        this_cpu_mut()
-            .get_pgtable()
-            .set_shared_4k(vaddr)
-            .map_err(|_| SvsmReqError::invalid_request())?;
-
-        let paddr = virt_to_phys(vaddr);
-        current_ghcb()
-            .page_state_change(
-                paddr,
-                paddr + PAGE_SIZE,
-                PageSize::Regular,
-                PageStateChangeOp::PscShared,
-            )
-            .map_err(|_| SvsmReqError::invalid_request())
+        make_page_shared(vaddr).map_err(|_| SvsmReqError::invalid_request())
     }
 
     /// Set the C-bit (memory encryption bit) for the Self page
     pub fn set_encrypted(&mut self) -> Result<(), SvsmReqError> {
         let vaddr = VirtAddr::from(addr_of_mut!(*self));
-        this_cpu_mut()
-            .get_pgtable()
-            .set_encrypted_4k(vaddr)
-            .map_err(|_| SvsmReqError::invalid_request())?;
-
-        let paddr = virt_to_phys(vaddr);
-        current_ghcb()
-            .page_state_change(
-                paddr,
-                paddr + PAGE_SIZE,
-                PageSize::Regular,
-                PageStateChangeOp::PscPrivate,
-            )
-            .map_err(|_| SvsmReqError::invalid_request())
+        make_page_private(vaddr).map_err(|_| SvsmReqError::invalid_request())
     }
 
     /// Fill the [`SnpGuestRequestMsg`] fields with zeros
@@ -412,49 +385,17 @@ fn build_iv(msg_seqno: u64) -> [u8; IV_SIZE] {
 }
 
 /// Set to encrypted all the 4k pages of a memory range
-fn set_encrypted_region_4k(start: VirtAddr, end: VirtAddr) -> Result<(), SvsmReqError> {
-    for addr in (start.bits()..end.bits())
-        .step_by(PAGE_SIZE)
-        .map(VirtAddr::from)
-    {
-        this_cpu_mut()
-            .get_pgtable()
-            .set_encrypted_4k(addr)
-            .map_err(|_| SvsmReqError::invalid_request())?;
-
-        let paddr = virt_to_phys(addr);
-        current_ghcb()
-            .page_state_change(
-                paddr,
-                paddr + PAGE_SIZE,
-                PageSize::Regular,
-                PageStateChangeOp::PscPrivate,
-            )
-            .map_err(|_| SvsmReqError::invalid_request())?;
+fn set_encrypted_region_4k(vregion: MemoryRegion<VirtAddr>) -> Result<(), SvsmReqError> {
+    for addr in vregion.iter_pages(PageSize::Regular) {
+        make_page_private(addr).map_err(|_| SvsmReqError::invalid_request())?;
     }
     Ok(())
 }
 
 /// Set to shared all the 4k pages of a memory range
-fn set_shared_region_4k(start: VirtAddr, end: VirtAddr) -> Result<(), SvsmReqError> {
-    for addr in (start.bits()..end.bits())
-        .step_by(PAGE_SIZE)
-        .map(VirtAddr::from)
-    {
-        this_cpu_mut()
-            .get_pgtable()
-            .set_shared_4k(addr)
-            .map_err(|_| SvsmReqError::invalid_request())?;
-
-        let paddr = virt_to_phys(addr);
-        current_ghcb()
-            .page_state_change(
-                paddr,
-                paddr + PAGE_SIZE,
-                PageSize::Regular,
-                PageStateChangeOp::PscShared,
-            )
-            .map_err(|_| SvsmReqError::invalid_request())?;
+fn set_shared_region_4k(vregion: MemoryRegion<VirtAddr>) -> Result<(), SvsmReqError> {
+    for addr in vregion.iter_pages(PageSize::Regular) {
+        make_page_shared(addr).map_err(|_| SvsmReqError::invalid_request())?;
     }
     Ok(())
 }
@@ -496,15 +437,15 @@ impl SnpGuestRequestExtData {
     ///   (returned to the allocator)
     pub fn set_shared(&mut self) -> Result<(), SvsmReqError> {
         let start = VirtAddr::from(addr_of_mut!(*self));
-        let end = start + size_of::<Self>();
-        set_shared_region_4k(start, end)
+        let region = MemoryRegion::new(start, size_of::<Self>());
+        set_shared_region_4k(region)
     }
 
     /// Set the C-bit (memory encryption bit) for the Self pages
     pub fn set_encrypted(&mut self) -> Result<(), SvsmReqError> {
         let start = VirtAddr::from(addr_of_mut!(*self));
-        let end = start + size_of::<Self>();
-        set_encrypted_region_4k(start, end)
+        let region = MemoryRegion::new(start, size_of::<Self>());
+        set_encrypted_region_4k(region)
     }
 
     /// Clear the first `n` bytes from data

kernel/src/mm/page_visibility.rs

@@ -6,67 +6,61 @@
 
 use crate::address::VirtAddr;
 use crate::cpu::flush_tlb_global_sync;
-use crate::cpu::ghcb::current_ghcb;
 use crate::cpu::percpu::this_cpu_mut;
+use crate::error::SvsmError;
 use crate::mm::validate::{
     valid_bitmap_clear_valid_4k, valid_bitmap_set_valid_4k, valid_bitmap_valid_addr,
 };
 use crate::mm::virt_to_phys;
-use crate::sev::ghcb::PageStateChangeOp;
-use crate::sev::utils::pvalidate;
+use crate::platform::{PageStateChangeOp, SVSM_PLATFORM};
 use crate::sev::PvalidateOp;
 use crate::types::{PageSize, PAGE_SIZE};
+use crate::utils::MemoryRegion;
+
+pub fn make_page_shared(vaddr: VirtAddr) -> Result<(), SvsmError> {
+    let platform = SVSM_PLATFORM.as_dyn_ref();
 
-pub fn make_page_shared(vaddr: VirtAddr) {
     // Revoke page validation before changing page state.
-    pvalidate(vaddr, PageSize::Regular, PvalidateOp::Invalid)
-        .expect("Pvalidate failed when making page shared");
+    platform.pvalidate_range(MemoryRegion::new(vaddr, PAGE_SIZE), PvalidateOp::Invalid)?;
     let paddr = virt_to_phys(vaddr);
     if valid_bitmap_valid_addr(paddr) {
         valid_bitmap_clear_valid_4k(paddr);
     }
 
     // Ask the hypervisor to make the page shared.
-    current_ghcb()
-        .page_state_change(
-            paddr,
-            paddr + PAGE_SIZE,
-            PageSize::Regular,
-            PageStateChangeOp::PscShared,
-        )
-        .expect("Hypervisor failed to make page shared");
+    platform.page_state_change(
+        MemoryRegion::new(paddr, PAGE_SIZE),
+        PageSize::Regular,
+        PageStateChangeOp::Shared,
+    )?;
 
     // Update the page tables to map the page as shared.
-    this_cpu_mut()
-        .get_pgtable()
-        .set_shared_4k(vaddr)
-        .expect("Failed to remap shared page in page tables");
+    this_cpu_mut().get_pgtable().set_shared_4k(vaddr)?;
     flush_tlb_global_sync();
+
+    Ok(())
 }
 
-pub fn make_page_private(vaddr: VirtAddr) {
+pub fn make_page_private(vaddr: VirtAddr) -> Result<(), SvsmError> {
     // Update the page tables to map the page as private.
-    this_cpu_mut()
-        .get_pgtable()
-        .set_encrypted_4k(vaddr)
-        .expect("Failed to remap private page in page tables");
+    this_cpu_mut().get_pgtable().set_encrypted_4k(vaddr)?;
     flush_tlb_global_sync();
 
+    let platform = SVSM_PLATFORM.as_dyn_ref();
+
     // Ask the hypervisor to make the page private.
     let paddr = virt_to_phys(vaddr);
-    current_ghcb()
-        .page_state_change(
-            paddr,
-            paddr + PAGE_SIZE,
-            PageSize::Regular,
-            PageStateChangeOp::PscPrivate,
-        )
-        .expect("Hypervisor failed to make page private");
+    platform.page_state_change(
+        MemoryRegion::new(paddr, PAGE_SIZE),
+        PageSize::Regular,
+        PageStateChangeOp::Private,
+    )?;
 
     // Revoke page validation before changing page state.
-    pvalidate(vaddr, PageSize::Regular, PvalidateOp::Valid)
-        .expect("Pvalidate failed when making page private");
+    platform.pvalidate_range(MemoryRegion::new(vaddr, PAGE_SIZE), PvalidateOp::Valid)?;
     if valid_bitmap_valid_addr(paddr) {
         valid_bitmap_set_valid_4k(paddr);
     }
+
+    Ok(())
 }

kernel/src/platform/mod.rs

@@ -10,6 +10,7 @@ use crate::error::SvsmError;
 use crate::io::IOPort;
 use crate::platform::native::NativePlatform;
 use crate::platform::snp::SnpPlatform;
+use crate::sev::PvalidateOp;
 use crate::types::PageSize;
 use crate::utils::immut_after_init::ImmutAfterInitCell;
 use crate::utils::MemoryRegion;
@@ -29,6 +30,14 @@ pub struct PageEncryptionMasks {
     pub phys_addr_sizes: u32,
 }
 
+#[derive(Debug, Clone, Copy)]
+pub enum PageStateChangeOp {
+    Private,
+    Shared,
+    Psmash,
+    Unsmash,
+}
+
 /// This defines a platform abstraction to permit the SVSM to run on different
 /// underlying architectures.
 pub trait SvsmPlatform {
@@ -57,15 +66,17 @@ pub trait SvsmPlatform {
     /// Performs a page state change between private and shared states.
     fn page_state_change(
         &self,
-        start: PhysAddr,
-        end: PhysAddr,
+        region: MemoryRegion<PhysAddr>,
         size: PageSize,
-        make_private: bool,
+        op: PageStateChangeOp,
     ) -> Result<(), SvsmError>;
 
     /// Marks a page as valid or invalid as a private page.
-    fn pvalidate_range(&self, region: MemoryRegion<VirtAddr>, valid: bool)
-        -> Result<(), SvsmError>;
+    fn pvalidate_range(
+        &self,
+        region: MemoryRegion<VirtAddr>,
+        op: PvalidateOp,
+    ) -> Result<(), SvsmError>;
 }
 
 //FIXME - remove Copy trait

kernel/src/platform/native.rs

@@ -4,9 +4,12 @@
 //
 // Author: Jon Lange <[email protected]>
 
+use crate::address::{PhysAddr, VirtAddr};
 use crate::cpu::cpuid::CpuidResult;
 use crate::cpu::percpu::PerCpu;
-use crate::platform::{IOPort, PageEncryptionMasks, PhysAddr, SvsmError, SvsmPlatform, VirtAddr};
+use crate::error::SvsmError;
+use crate::platform::{IOPort, PageEncryptionMasks, PageStateChangeOp, SvsmPlatform};
+use crate::sev::PvalidateOp;
 use crate::svsm_console::NativeIOPort;
 use crate::types::PageSize;
 use crate::utils::MemoryRegion;
@@ -59,18 +62,17 @@ impl SvsmPlatform for NativePlatform {
 
     fn page_state_change(
         &self,
-        _start: PhysAddr,
-        _end: PhysAddr,
+        _region: MemoryRegion<PhysAddr>,
         _size: PageSize,
-        _make_private: bool,
+        _op: PageStateChangeOp,
     ) -> Result<(), SvsmError> {
         Ok(())
     }
 
     fn pvalidate_range(
         &self,
         _region: MemoryRegion<VirtAddr>,
-        _valid: bool,
+        _op: PvalidateOp,
     ) -> Result<(), SvsmError> {
         Ok(())
     }

kernel/src/platform/snp.rs

@@ -4,12 +4,13 @@
 //
 // Author: Jon Lange <[email protected]>
 
+use crate::address::{PhysAddr, VirtAddr};
 use crate::cpu::cpuid::cpuid_table;
 use crate::cpu::ghcb::current_ghcb;
 use crate::cpu::percpu::PerCpu;
+use crate::error::SvsmError;
 use crate::io::IOPort;
-use crate::platform::{PageEncryptionMasks, PhysAddr, SvsmError, SvsmPlatform, VirtAddr};
-use crate::sev::ghcb::PageStateChangeOp::{PscPrivate, PscShared};
+use crate::platform::{PageEncryptionMasks, PageStateChangeOp, SvsmPlatform};
 use crate::sev::msr_protocol::verify_ghcb_version;
 use crate::sev::status::vtom_enabled;
 use crate::sev::{pvalidate_range, sev_status_init, sev_status_verify, PvalidateOp};
@@ -97,25 +98,18 @@ impl SvsmPlatform for SnpPlatform {
 
     fn page_state_change(
         &self,
-        start: PhysAddr,
-        end: PhysAddr,
+        region: MemoryRegion<PhysAddr>,
         size: PageSize,
-        make_private: bool,
+        op: PageStateChangeOp,
     ) -> Result<(), SvsmError> {
-        let psc_op = if make_private { PscPrivate } else { PscShared };
-        current_ghcb().page_state_change(start, end, size, psc_op)
+        current_ghcb().page_state_change(region, size, op)
     }
 
     fn pvalidate_range(
         &self,
         region: MemoryRegion<VirtAddr>,
-        valid: bool,
+        op: PvalidateOp,
     ) -> Result<(), SvsmError> {
-        let pvalidate_op = if valid {
-            PvalidateOp::Valid
-        } else {
-            PvalidateOp::Invalid
-        };
-        pvalidate_range(region, pvalidate_op)
+        pvalidate_range(region, op)
     }
 }