[host/hypervisor/hyperv_linux] brought back mshv driver

Needed to make some modications to hyperv_linux for it to conform to the new SandboxMemorySections API. With that,
I also had to add a From impl to sandbox_builder. As an aside, I also modified the signature of get_memory_access_violation
to take a borrow of SandboxMemorySections instead of an owned value, which ended up impacting kvm too.

Signed-off-by: danbugs <[email protected]>

Author: danbugs

src/hyperlight_host/src/hypervisor/hyperv_linux.rs

@@ -26,7 +26,7 @@ extern crate mshv_ioctls3 as mshv_ioctls;
 
 use std::fmt::{Debug, Formatter};
 
-use log::LevelFilter;
+use log::{error, LevelFilter};
 #[cfg(mshv2)]
 use mshv_bindings::hv_message;
 #[cfg(gdb)]
@@ -62,7 +62,7 @@ use super::{
 use crate::hypervisor::hypervisor_handler::HypervisorHandler;
 use crate::hypervisor::HyperlightExit;
 use crate::mem::ptr::{GuestPtr, RawPtr};
-use crate::sandbox::sandbox_builder::SandboxMemorySections;
+use crate::sandbox::sandbox_builder::{MemoryRegionFlags, SandboxMemorySections};
 #[cfg(gdb)]
 use crate::HyperlightError;
 use crate::{log_then_return, new_error, Result};
@@ -289,8 +289,6 @@ pub(crate) fn is_hypervisor_present() -> bool {
 /// called the Microsoft Hypervisor (MSHV)
 pub(super) struct HypervLinuxDriver {
     _mshv: Mshv,
-    // TODO(danbugs:297): remove
-    #[allow(dead_code)]
     vm_fd: VmFd,
     vcpu_fd: VcpuFd,
     entrypoint: u64,
@@ -379,11 +377,10 @@ impl HypervLinuxDriver {
             (None, None)
         };
 
-        // TODO(danbugs:297): bring back
-        // mem_sections.iter().try_for_each(|region| {
-        //     let mshv_region = region.to_owned().into();
-        //     vm_fd.map_user_memory(mshv_region)
-        // })?;
+        mem_sections.iter().try_for_each(|(_, region)| {
+            let mshv_region = region.to_owned().into();
+            vm_fd.map_user_memory(mshv_region)
+        })?;
 
         Self::setup_initial_sregs(&mut vcpu_fd, pml4_ptr.absolute()?)?;
 
@@ -476,7 +473,7 @@ impl Hypervisor for HypervLinuxDriver {
         let regs = StandardRegisters {
             rip: self.entrypoint,
             rsp: self.orig_rsp.absolute()?,
-            rflags: 2, //bit 1 of rlags is required to be set
+            rflags: 2, //bit 1 of rflags is required to be set
 
             // function args
             rcx: hyperlight_peb_guest_memory_region_address,
@@ -496,6 +493,8 @@ impl Hypervisor for HypervLinuxDriver {
             dbg_mem_access_fn,
         )?;
 
+        // TODO(danbugs:297): here, we should update the rsp to what the guest configured.
+
         Ok(())
     }
 
@@ -512,7 +511,7 @@ impl Hypervisor for HypervLinuxDriver {
         let regs = StandardRegisters {
             rip: dispatch_func_addr.into(),
             rsp: self.orig_rsp.absolute()?,
-            rflags: 2, //bit 1 of rlags is required to be set
+            rflags: 2, //bit 1 of rflags is required to be set
             ..Default::default()
         };
         self.vcpu_fd.set_regs(&regs)?;
@@ -616,24 +615,21 @@ impl Hypervisor for HypervLinuxDriver {
                 INVALID_GPA_ACCESS_MESSAGE => {
                     let mimo_message = m.to_memory_info()?;
                     let gpa = mimo_message.guest_physical_address;
-                    // TODO(danbugs:297): bring back
-                    // let access_info = MemoryRegionFlags::try_from(mimo_message)?;
+                    let access_info = MemoryRegionFlags::try_from(mimo_message)?;
                     crate::debug!(
                         "mshv MMIO invalid GPA access -Details: Address: {} \n {:#?}",
                         gpa,
                         &self
                     );
-                    // TODO(danbugs:297): bring back
-                    // match self.get_memory_access_violation(
-                    //     gpa as usize,
-                    //     &self.mem_regions,
-                    //     access_info,
-                    // ) {
-                    //     Some(access_info_violation) => access_info_violation,
-                    //     None => HyperlightExit::Mmio(gpa),
-                    // }
-
-                    HyperlightExit::Mmio(gpa)
+
+                    match self.get_memory_access_violation(
+                        gpa as usize,
+                        &self.mem_sections,
+                        access_info,
+                    ) {
+                        Some(access_info_violation) => access_info_violation,
+                        None => HyperlightExit::Mmio(gpa),
+                    }
                 }
                 // The only case an intercept exit is expected is when debugging is enabled
                 // and the intercepts are installed
@@ -719,19 +715,18 @@ impl Hypervisor for HypervLinuxDriver {
     }
 }
 
-// TODO(danbugs:297): bring back
-// impl Drop for HypervLinuxDriver {
-//     #[instrument(skip_all, parent = Span::current(), level = "Trace")]
-//     fn drop(&mut self) {
-//         for region in self.mem_sections.iter() {
-//             let mshv_region: mshv_user_mem_region = region.to_owned().into();
-//             match self.vm_fd.unmap_user_memory(mshv_region) {
-//                 Ok(_) => (),
-//                 Err(e) => error!("Failed to unmap user memory in HyperVOnLinux ({:?})", e),
-//             }
-//         }
-//     }
-// }
+impl Drop for HypervLinuxDriver {
+    #[instrument(skip_all, parent = Span::current(), level = "Trace")]
+    fn drop(&mut self) {
+        for region in self.mem_sections.iter() {
+            let mshv_region: mshv_bindings::mshv_user_mem_region = region.1.to_owned().into();
+            match self.vm_fd.unmap_user_memory(mshv_region) {
+                Ok(_) => (),
+                Err(e) => error!("Failed to unmap user memory in HyperVOnLinux ({:?})", e),
+            }
+        }
+    }
+}
 
 // TODO(danbugs:297): bring back
 // #[cfg(test)]

src/hyperlight_host/src/hypervisor/kvm.rs

@@ -528,7 +528,7 @@ impl Hypervisor for KVMDriver {
 
                 match self.get_memory_access_violation(
                     addr as usize,
-                    self.mem_sections.clone(),
+                    &self.mem_sections,
                     MemoryRegionFlags::READ,
                 ) {
                     Some(access_violation_exit) => access_violation_exit,
@@ -540,7 +540,7 @@ impl Hypervisor for KVMDriver {
 
                 match self.get_memory_access_violation(
                     addr as usize,
-                    self.mem_sections.clone(),
+                    &self.mem_sections,
                     MemoryRegionFlags::WRITE,
                 ) {
                     Some(access_violation_exit) => access_violation_exit,

src/hyperlight_host/src/hypervisor/mod.rs

@@ -169,7 +169,7 @@ pub(crate) trait Hypervisor: Debug + Sync + Send {
     fn get_memory_access_violation(
         &self,
         gpa: usize,
-        mem_sections: SandboxMemorySections,
+        mem_sections: &SandboxMemorySections,
         access_info: MemoryRegionFlags,
     ) -> Option<HyperlightExit> {
         // find the region containing the given gpa

src/hyperlight_host/src/sandbox/sandbox_builder.rs

@@ -17,6 +17,22 @@ use crate::{
     log_build_details, log_then_return, new_error, Result, SandboxRunOptions, UninitializedSandbox,
 };
 
+#[cfg(mshv2)]
+extern crate mshv_bindings2 as mshv_bindings;
+
+#[cfg(mshv2)]
+use mshv_bindings::{
+    HV_MAP_GPA_EXECUTABLE, HV_MAP_GPA_PERMISSIONS_NONE, HV_MAP_GPA_READABLE, HV_MAP_GPA_WRITABLE,
+};
+
+#[cfg(mshv3)]
+extern crate mshv_bindings3 as mshv_bindings;
+
+#[cfg(mshv3)]
+use mshv_bindings::{
+    MSHV_SET_MEM_BIT_EXECUTABLE, MSHV_SET_MEM_BIT_UNMAP, MSHV_SET_MEM_BIT_WRITABLE,
+};
+
 const DEFAULT_GUEST_CODE_SECTION_NAME: &str = "guest code";
 const DEFAULT_TMP_STACK_SECTION_NAME: &str = "tmp stack";
 const DEFAULT_HYPERLIGHT_PEB_SECTION_NAME: &str = "HyperlightPEB";
@@ -28,7 +44,7 @@ pub(crate) const PD_OFFSET: usize = 0x2000; // this offset is from the PML4 base
 pub(crate) const PT_OFFSET: usize = 0x3000; // this offset is from the PML4 base address
 const DEFAULT_GUEST_MEMORY_SIZE: usize = 0x200_000; // 2MB
 
-/// TODO: comment
+/// Represents a memory section in the sandbox.
 #[derive(Debug, Clone)]
 pub struct SandboxMemorySection {
     /// Name of the memory section
@@ -164,16 +180,6 @@ impl SandboxMemorySections {
     }
 }
 
-#[cfg(mshv2)]
-extern crate mshv_bindings2 as mshv_bindings;
-#[cfg(mshv2)]
-extern crate mshv_ioctls2 as mshv_ioctls;
-
-#[cfg(mshv3)]
-extern crate mshv_bindings3 as mshv_bindings;
-#[cfg(mshv3)]
-extern crate mshv_ioctls3 as mshv_ioctls;
-
 use bitflags::bitflags;
 #[cfg(mshv)]
 use mshv_bindings::hv_x64_memory_intercept_message;
@@ -798,6 +804,60 @@ impl SandboxBuilder {
     }
 }
 
+#[cfg(mshv)]
+impl From<SandboxMemorySection> for mshv_bindings::mshv_user_mem_region {
+    fn from(section: SandboxMemorySection) -> Self {
+        let size = section.page_aligned_size as u64;
+        let guest_pfn = (section.page_aligned_guest_offset as u64) >> 12;
+        let userspace_addr = section.host_address.unwrap_or(0) as u64;
+
+        #[cfg(mshv2)]
+        {
+            let mut flags = 0;
+            if section.flags.contains(MemoryRegionFlags::READ) {
+                flags |= HV_MAP_GPA_READABLE;
+            }
+            if section.flags.contains(MemoryRegionFlags::WRITE) {
+                flags |= HV_MAP_GPA_WRITABLE;
+            }
+            if section.flags.contains(MemoryRegionFlags::EXECUTE) {
+                flags |= HV_MAP_GPA_EXECUTABLE;
+            }
+            if section.flags.is_empty() || section.flags.contains(MemoryRegionFlags::NONE) {
+                flags |= HV_MAP_GPA_PERMISSIONS_NONE;
+            }
+
+            mshv_bindings::mshv_user_mem_region {
+                guest_pfn,
+                size,
+                userspace_addr,
+                flags,
+            }
+        }
+        #[cfg(mshv3)]
+        {
+            let mut flags: u8 = 0;
+            if section.flags.contains(MemoryRegionFlags::WRITE) {
+                flags |= 1 << MSHV_SET_MEM_BIT_WRITABLE;
+            }
+            if section.flags.contains(MemoryRegionFlags::EXECUTE) {
+                flags |= 1 << MSHV_SET_MEM_BIT_EXECUTABLE;
+            }
+            if section.flags.is_empty() || section.flags.contains(MemoryRegionFlags::NONE) {
+                flags |= 1 << MSHV_SET_MEM_BIT_UNMAP;
+            }
+
+            mshv_bindings::mshv_user_mem_region {
+                guest_pfn,
+                size,
+                userspace_addr,
+                flags,
+                ..Default::default()
+            }
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use std::sync::{Arc, Mutex};
@@ -873,54 +933,3 @@ mod tests {
         Ok(())
     }
 }
-
-// TODO(danbugs:297): impl
-// #[cfg(mshv)]
-// impl From<MemoryRegion> for mshv_user_mem_region {
-//     fn from(region: MemoryRegion) -> Self {
-//         let size = (region.guest_region.end - region.guest_region.start) as u64;
-//         let guest_pfn = region.guest_region.start as u64 >> 12;
-//         let userspace_addr = region.host_region.start as u64;
-//
-//         #[cfg(mshv2)]
-//         {
-//             let flags = region.flags.iter().fold(0, |acc, flag| {
-//                 let flag_value = match flag {
-//                     MemoryRegionFlags::NONE => HV_MAP_GPA_PERMISSIONS_NONE,
-//                     MemoryRegionFlags::READ => HV_MAP_GPA_READABLE,
-//                     MemoryRegionFlags::WRITE => HV_MAP_GPA_WRITABLE,
-//                     MemoryRegionFlags::EXECUTE => HV_MAP_GPA_EXECUTABLE,
-//                     _ => 0, // ignore any unknown flags
-//                 };
-//                 acc | flag_value
-//             });
-//             mshv_user_mem_region {
-//                 guest_pfn,
-//                 size,
-//                 userspace_addr,
-//                 flags,
-//             }
-//         }
-//         #[cfg(mshv3)]
-//         {
-//             let flags: u8 = region.flags.iter().fold(0, |acc, flag| {
-//                 let flag_value = match flag {
-//                     MemoryRegionFlags::NONE => 1 << MSHV_SET_MEM_BIT_UNMAP,
-//                     MemoryRegionFlags::READ => 0,
-//                     MemoryRegionFlags::WRITE => 1 << MSHV_SET_MEM_BIT_WRITABLE,
-//                     MemoryRegionFlags::EXECUTE => 1 << MSHV_SET_MEM_BIT_EXECUTABLE,
-//                     _ => 0, // ignore any unknown flags
-//                 };
-//                 acc | flag_value
-//             });
-//
-//             mshv_user_mem_region {
-//                 guest_pfn,
-//                 size,
-//                 userspace_addr,
-//                 flags,
-//                 ..Default::default()
-//             }
-//         }
-//     }
-// }