diff --git a/src/hyperlight_host/src/hypervisor/handlers.rs b/src/hyperlight_host/src/hypervisor/handlers.rs index 186c030e2..842b44a37 100644 --- a/src/hyperlight_host/src/hypervisor/handlers.rs +++ b/src/hyperlight_host/src/hypervisor/handlers.rs @@ -16,118 +16,11 @@ limitations under the License. use std::sync::{Arc, Mutex}; -use tracing::{Span, instrument}; - -#[cfg(feature = "trace_guest")] -use super::Hypervisor; -use crate::{Result, new_error}; - -/// The trait representing custom logic to handle the case when -/// a Hypervisor's virtual CPU (vCPU) informs Hyperlight the guest -/// has initiated an outb operation. -pub(crate) trait OutBHandlerCaller: Sync + Send { - /// Function that gets called when an outb operation has occurred. - fn call( - &mut self, - #[cfg(feature = "trace_guest")] hv: &mut dyn Hypervisor, - port: u16, - payload: u32, - ) -> Result<()>; -} - -/// A convenient type representing a common way `OutBHandler` implementations -/// are passed as parameters to functions -/// -/// Note: This needs to be wrapped in a Mutex to be able to grab a mutable -/// reference to the underlying data (i.e., handle_outb in `Sandbox` takes -/// a &mut self). -pub(crate) type OutBHandlerWrapper = Arc>; - -#[cfg(feature = "trace_guest")] -pub(crate) type OutBHandlerFunction = - Box Result<()> + Send>; -#[cfg(not(feature = "trace_guest"))] -pub(crate) type OutBHandlerFunction = Box Result<()> + Send>; - -/// A `OutBHandler` implementation using a `OutBHandlerFunction` -/// -/// Note: This handler must live no longer than the `Sandbox` to which it belongs -pub(crate) struct OutBHandler(Arc>); - -impl From for OutBHandler { - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - fn from(func: OutBHandlerFunction) -> Self { - Self(Arc::new(Mutex::new(func))) - } -} - -impl OutBHandlerCaller for OutBHandler { - #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] - fn call( - &mut self, - #[cfg(feature = "trace_guest")] hv: &mut dyn Hypervisor, - port: u16, - payload: u32, - ) -> Result<()> { - let mut func = self - .0 - .try_lock() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?; - func( - #[cfg(feature = "trace_guest")] - hv, - port, - payload, - ) - } -} - -/// The trait representing custom logic to handle the case when -/// a Hypervisor's virtual CPU (vCPU) informs Hyperlight a memory access -/// outside the designated address space has occurred. -pub trait MemAccessHandlerCaller: Send { - /// Function that gets called when unexpected memory access has occurred. - fn call(&mut self) -> Result<()>; -} - -/// A convenient type representing a common way `MemAccessHandler` implementations -/// are passed as parameters to functions -/// -/// Note: This needs to be wrapped in a Mutex to be able to grab a mutable -/// reference to the underlying data (i.e., handle_mmio_exit in `Sandbox` takes -/// a &mut self). -pub type MemAccessHandlerWrapper = Arc>; - -pub(crate) type MemAccessHandlerFunction = Box Result<()> + Send>; - -/// A `MemAccessHandler` implementation using `MemAccessHandlerFunction`. -/// -/// Note: This handler must live for as long as its Sandbox or for -/// static in the case of its C API usage. -pub(crate) struct MemAccessHandler(Arc>); - -impl From for MemAccessHandler { - #[instrument(skip_all, parent = Span::current(), level= "Trace")] - fn from(func: MemAccessHandlerFunction) -> Self { - Self(Arc::new(Mutex::new(func))) - } -} - -impl MemAccessHandlerCaller for MemAccessHandler { - #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] - fn call(&mut self) -> Result<()> { - let mut func = self - .0 - .try_lock() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?; - func() - } -} +use crate::Result; /// The trait representing custom logic to handle the case when /// a Hypervisor's virtual CPU (vCPU) informs Hyperlight a debug memory access /// has been requested. -#[cfg(gdb)] pub trait DbgMemAccessHandlerCaller: Send { /// Function that gets called when a read is requested. fn read(&mut self, addr: usize, data: &mut [u8]) -> Result<()>; @@ -143,5 +36,4 @@ pub trait DbgMemAccessHandlerCaller: Send { /// /// Note: This needs to be wrapped in a Mutex to be able to grab a mutable /// reference to the underlying data -#[cfg(gdb)] pub type DbgMemAccessHandlerWrapper = Arc>; diff --git a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs index 65f8910be..df3073a09 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs @@ -25,8 +25,8 @@ extern crate mshv_bindings3 as mshv_bindings; extern crate mshv_ioctls3 as mshv_ioctls; use std::fmt::{Debug, Formatter}; -use std::sync::Arc; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; +use std::sync::{Arc, Mutex}; use log::{LevelFilter, error}; #[cfg(mshv2)] @@ -67,7 +67,6 @@ use super::gdb::{ }; #[cfg(gdb)] use super::handlers::DbgMemAccessHandlerWrapper; -use super::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper}; #[cfg(feature = "init-paging")] use super::{ CR0_AM, CR0_ET, CR0_MP, CR0_NE, CR0_PE, CR0_PG, CR0_WP, CR4_OSFXSR, CR4_OSXMMEXCPT, CR4_PAE, @@ -78,9 +77,13 @@ use super::{HyperlightExit, Hypervisor, InterruptHandle, LinuxInterruptHandle, V use crate::HyperlightError; use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags}; use crate::mem::ptr::{GuestPtr, RawPtr}; +use crate::mem::shared_mem::HostSharedMemory; use crate::sandbox::SandboxConfiguration; #[cfg(feature = "trace_guest")] use crate::sandbox::TraceInfo; +use crate::sandbox::host_funcs::FunctionRegistry; +use crate::sandbox::mem_mgr::MemMgrWrapper; +use crate::sandbox::outb::handle_outb; #[cfg(crashdump)] use crate::sandbox::uninitialized::SandboxRuntimeConfig; use crate::{Result, log_then_return, new_error}; @@ -313,7 +316,8 @@ pub(crate) struct HypervLinuxDriver { mem_regions: Vec, orig_rsp: GuestPtr, interrupt_handle: Arc, - + mem_mgr: Option>, + host_funcs: Option>>, #[cfg(gdb)] debug: Option, #[cfg(gdb)] @@ -447,6 +451,8 @@ impl HypervLinuxDriver { entrypoint: entrypoint_ptr.absolute()?, orig_rsp: rsp_ptr, interrupt_handle: interrupt_handle.clone(), + mem_mgr: None, + host_funcs: None, #[cfg(gdb)] debug, #[cfg(gdb)] @@ -574,11 +580,13 @@ impl Hypervisor for HypervLinuxDriver { peb_addr: RawPtr, seed: u64, page_size: u32, - outb_hdl: OutBHandlerWrapper, - mem_access_hdl: MemAccessHandlerWrapper, + mem_mgr: MemMgrWrapper, + host_funcs: Arc>, max_guest_log_level: Option, #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()> { + self.mem_mgr = Some(mem_mgr); + self.host_funcs = Some(host_funcs); self.page_size = page_size as usize; let max_guest_log_level: u64 = match max_guest_log_level { @@ -601,13 +609,22 @@ impl Hypervisor for HypervLinuxDriver { }; self.vcpu_fd.set_regs(®s)?; - VirtualCPU::run( + // Extract mem_mgr to avoid borrowing conflicts + let mem_mgr = self + .mem_mgr + .take() + .ok_or_else(|| new_error!("mem_mgr should be initialized"))?; + + let result = VirtualCPU::run( self.as_mut_hypervisor(), - outb_hdl, - mem_access_hdl, + &mem_mgr, #[cfg(gdb)] dbg_mem_access_fn, - )?; + ); + + // Put mem_mgr back + self.mem_mgr = Some(mem_mgr); + result?; Ok(()) } @@ -647,8 +664,7 @@ impl Hypervisor for HypervLinuxDriver { fn dispatch_call_from_host( &mut self, dispatch_func_addr: RawPtr, - outb_handle_fn: OutBHandlerWrapper, - mem_access_fn: MemAccessHandlerWrapper, + mem_mgr: &MemMgrWrapper, #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()> { // Reset general purpose registers, then set RIP and RSP @@ -672,8 +688,7 @@ impl Hypervisor for HypervLinuxDriver { // run VirtualCPU::run( self.as_mut_hypervisor(), - outb_handle_fn, - mem_access_fn, + mem_mgr, #[cfg(gdb)] dbg_mem_access_fn, )?; @@ -688,22 +703,47 @@ impl Hypervisor for HypervLinuxDriver { data: Vec, rip: u64, instruction_length: u64, - outb_handle_fn: OutBHandlerWrapper, ) -> Result<()> { let mut padded = [0u8; 4]; let copy_len = data.len().min(4); padded[..copy_len].copy_from_slice(&data[..copy_len]); let val = u32::from_le_bytes(padded); - outb_handle_fn - .try_lock() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? - .call( - #[cfg(feature = "trace_guest")] - self, - port, - val, - )?; + #[cfg(feature = "trace_guest")] + { + // We need to handle the borrow checker issue where we need both: + // - &mut MemMgrWrapper (from self.mem_mgr.as_mut()) + // - &mut dyn Hypervisor (from self) + // We'll use a temporary approach to extract the mem_mgr temporarily + let mem_mgr_option = self.mem_mgr.take(); + let mut mem_mgr = mem_mgr_option + .ok_or_else(|| new_error!("mem_mgr should be initialized before handling IO"))?; + let host_funcs = self + .host_funcs + .as_ref() + .ok_or_else(|| new_error!("host_funcs should be initialized before handling IO"))? + .clone(); + + handle_outb(&mut mem_mgr, host_funcs, self, port, val)?; + + // Put the mem_mgr back + self.mem_mgr = Some(mem_mgr); + } + + #[cfg(not(feature = "trace_guest"))] + { + let mem_mgr = self + .mem_mgr + .as_mut() + .ok_or_else(|| new_error!("mem_mgr should be initialized before handling IO"))?; + let host_funcs = self + .host_funcs + .as_ref() + .ok_or_else(|| new_error!("host_funcs should be initialized before handling IO"))? + .clone(); + + handle_outb(mem_mgr, host_funcs, port, val)?; + } // update rip self.vcpu_fd.set_reg(&[hv_register_assoc { diff --git a/src/hyperlight_host/src/hypervisor/hyperv_windows.rs b/src/hyperlight_host/src/hypervisor/hyperv_windows.rs index 01c2b0560..595372306 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_windows.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_windows.rs @@ -17,8 +17,8 @@ limitations under the License. use std::fmt; use std::fmt::{Debug, Formatter}; use std::string::String; -use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex}; use log::LevelFilter; use tracing::{Span, instrument}; @@ -41,13 +41,11 @@ use { super::handlers::DbgMemAccessHandlerWrapper, crate::HyperlightError, crate::hypervisor::handlers::DbgMemAccessHandlerCaller, - std::sync::Mutex, }; #[cfg(feature = "trace_guest")] use super::TraceRegister; use super::fpu::{FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; -use super::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper}; use super::surrogate_process::SurrogateProcess; use super::surrogate_process_manager::*; use super::windows_hypervisor_platform::{VMPartition, VMProcessor}; @@ -62,8 +60,12 @@ use crate::hypervisor::fpu::FP_CONTROL_WORD_DEFAULT; use crate::hypervisor::wrappers::WHvGeneralRegisters; use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags}; use crate::mem::ptr::{GuestPtr, RawPtr}; +use crate::mem::shared_mem::HostSharedMemory; #[cfg(feature = "trace_guest")] use crate::sandbox::TraceInfo; +use crate::sandbox::host_funcs::FunctionRegistry; +use crate::sandbox::mem_mgr::MemMgrWrapper; +use crate::sandbox::outb::handle_outb; #[cfg(crashdump)] use crate::sandbox::uninitialized::SandboxRuntimeConfig; use crate::{Result, debug, log_then_return, new_error}; @@ -75,9 +77,9 @@ mod debug { use windows::Win32::System::Hypervisor::WHV_VP_EXCEPTION_CONTEXT; use super::{HypervWindowsDriver, *}; + use crate::Result; use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason, X86_64Regs}; use crate::hypervisor::handlers::DbgMemAccessHandlerCaller; - use crate::{Result, new_error}; impl HypervWindowsDriver { /// Resets the debug information to disable debugging @@ -281,6 +283,8 @@ pub(crate) struct HypervWindowsDriver { orig_rsp: GuestPtr, mem_regions: Vec, interrupt_handle: Arc, + mem_mgr: Option>, + host_funcs: Option>>, #[cfg(gdb)] debug: Option, #[cfg(gdb)] @@ -291,13 +295,12 @@ pub(crate) struct HypervWindowsDriver { #[allow(dead_code)] trace_info: TraceInfo, } -/* This does not automatically impl Send/Sync because the host +/* This does not automatically impl Send because the host * address of the shared memory region is a raw pointer, which are - * marked as !Send and !Sync. However, the access patterns used + * marked as !Send (and !Sync). However, the access patterns used * here are safe. */ unsafe impl Send for HypervWindowsDriver {} -unsafe impl Sync for HypervWindowsDriver {} impl HypervWindowsDriver { #[allow(clippy::too_many_arguments)] @@ -357,6 +360,8 @@ impl HypervWindowsDriver { orig_rsp: GuestPtr::try_from(RawPtr::from(rsp))?, mem_regions, interrupt_handle: interrupt_handle.clone(), + mem_mgr: None, + host_funcs: None, #[cfg(gdb)] debug, #[cfg(gdb)] @@ -590,11 +595,14 @@ impl Hypervisor for HypervWindowsDriver { peb_address: RawPtr, seed: u64, page_size: u32, - outb_hdl: OutBHandlerWrapper, - mem_access_hdl: MemAccessHandlerWrapper, + mem_mgr: MemMgrWrapper, + host_funcs: Arc>, max_guest_log_level: Option, #[cfg(gdb)] dbg_mem_access_hdl: DbgMemAccessHandlerWrapper, ) -> Result<()> { + self.mem_mgr = Some(mem_mgr); + self.host_funcs = Some(host_funcs); + let max_guest_log_level: u64 = match max_guest_log_level { Some(level) => level as u64, None => self.get_max_log_level().into(), @@ -615,13 +623,22 @@ impl Hypervisor for HypervWindowsDriver { }; self.processor.set_general_purpose_registers(®s)?; - VirtualCPU::run( + // Extract mem_mgr to avoid borrowing conflicts + let mem_mgr = self + .mem_mgr + .take() + .ok_or_else(|| new_error!("mem_mgr should be initialized"))?; + + let result = VirtualCPU::run( self.as_mut_hypervisor(), - outb_hdl, - mem_access_hdl, + &mem_mgr, #[cfg(gdb)] dbg_mem_access_hdl, - )?; + ); + + // Put mem_mgr back + self.mem_mgr = Some(mem_mgr); + result?; Ok(()) } @@ -645,8 +662,7 @@ impl Hypervisor for HypervWindowsDriver { fn dispatch_call_from_host( &mut self, dispatch_func_addr: RawPtr, - outb_hdl: OutBHandlerWrapper, - mem_access_hdl: MemAccessHandlerWrapper, + mem_mgr: &MemMgrWrapper, #[cfg(gdb)] dbg_mem_access_hdl: DbgMemAccessHandlerWrapper, ) -> Result<()> { // Reset general purpose registers, then set RIP and RSP @@ -668,8 +684,7 @@ impl Hypervisor for HypervWindowsDriver { VirtualCPU::run( self.as_mut_hypervisor(), - outb_hdl, - mem_access_hdl, + mem_mgr, #[cfg(gdb)] dbg_mem_access_hdl, )?; @@ -684,22 +699,47 @@ impl Hypervisor for HypervWindowsDriver { data: Vec, rip: u64, instruction_length: u64, - outb_handle_fn: OutBHandlerWrapper, ) -> Result<()> { let mut padded = [0u8; 4]; let copy_len = data.len().min(4); padded[..copy_len].copy_from_slice(&data[..copy_len]); let val = u32::from_le_bytes(padded); - outb_handle_fn - .try_lock() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? - .call( - #[cfg(feature = "trace_guest")] - self, - port, - val, - )?; + #[cfg(feature = "trace_guest")] + { + // We need to handle the borrow checker issue where we need both: + // - &mut MemMgrWrapper (from self.mem_mgr.as_mut()) + // - &mut dyn Hypervisor (from self) + // We'll use a temporary approach to extract the mem_mgr temporarily + let mem_mgr_option = self.mem_mgr.take(); + let mut mem_mgr = mem_mgr_option + .ok_or_else(|| new_error!("mem_mgr should be initialized before handling IO"))?; + let host_funcs = self + .host_funcs + .as_ref() + .ok_or_else(|| new_error!("host_funcs should be initialized before handling IO"))? + .clone(); + + handle_outb(&mut mem_mgr, host_funcs, self, port, val)?; + + // Put the mem_mgr back + self.mem_mgr = Some(mem_mgr); + } + + #[cfg(not(feature = "trace_guest"))] + { + let mem_mgr = self + .mem_mgr + .as_mut() + .ok_or_else(|| new_error!("mem_mgr should be initialized before handling IO"))?; + let host_funcs = self + .host_funcs + .as_ref() + .ok_or_else(|| new_error!("host_funcs should be initialized before handling IO"))? + .clone(); + + handle_outb(mem_mgr, host_funcs, port, val)?; + } let mut regs = self.processor.get_regs()?; regs.rip = rip + instruction_length; diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 188c27ed7..761020c50 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -16,10 +16,8 @@ limitations under the License. use std::convert::TryFrom; use std::fmt::Debug; -use std::sync::Arc; -#[cfg(gdb)] -use std::sync::Mutex; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; +use std::sync::{Arc, Mutex}; use kvm_bindings::{kvm_fpu, kvm_regs, kvm_userspace_memory_region}; use kvm_ioctls::Cap::UserMemory; @@ -36,7 +34,6 @@ use super::fpu::{FP_CONTROL_WORD_DEFAULT, FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse, GuestDebug, KvmDebug, VcpuStopReason}; #[cfg(gdb)] use super::handlers::DbgMemAccessHandlerWrapper; -use super::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper}; #[cfg(feature = "init-paging")] use super::{ CR0_AM, CR0_ET, CR0_MP, CR0_NE, CR0_PE, CR0_PG, CR0_WP, CR4_OSFXSR, CR4_OSXMMEXCPT, CR4_PAE, @@ -47,9 +44,13 @@ use super::{HyperlightExit, Hypervisor, InterruptHandle, LinuxInterruptHandle, V use crate::HyperlightError; use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags}; use crate::mem::ptr::{GuestPtr, RawPtr}; +use crate::mem::shared_mem::HostSharedMemory; use crate::sandbox::SandboxConfiguration; #[cfg(feature = "trace_guest")] use crate::sandbox::TraceInfo; +use crate::sandbox::host_funcs::FunctionRegistry; +use crate::sandbox::mem_mgr::MemMgrWrapper; +use crate::sandbox::outb::handle_outb; #[cfg(crashdump)] use crate::sandbox::uninitialized::SandboxRuntimeConfig; use crate::{Result, log_then_return, new_error}; @@ -295,7 +296,8 @@ pub(crate) struct KVMDriver { orig_rsp: GuestPtr, mem_regions: Vec, interrupt_handle: Arc, - + mem_mgr: Option>, + host_funcs: Option>>, #[cfg(gdb)] debug: Option, #[cfg(gdb)] @@ -384,6 +386,8 @@ impl KVMDriver { orig_rsp: rsp_gp, mem_regions, interrupt_handle: interrupt_handle.clone(), + mem_mgr: None, + host_funcs: None, #[cfg(gdb)] debug, #[cfg(gdb)] @@ -460,11 +464,13 @@ impl Hypervisor for KVMDriver { peb_addr: RawPtr, seed: u64, page_size: u32, - outb_hdl: OutBHandlerWrapper, - mem_access_hdl: MemAccessHandlerWrapper, + mem_mgr: MemMgrWrapper, + host_funcs: Arc>, max_guest_log_level: Option, #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()> { + self.mem_mgr = Some(mem_mgr); + self.host_funcs = Some(host_funcs); self.page_size = page_size as usize; let max_guest_log_level: u64 = match max_guest_log_level { @@ -486,13 +492,22 @@ impl Hypervisor for KVMDriver { }; self.vcpu_fd.set_regs(®s)?; - VirtualCPU::run( + // Extract mem_mgr to avoid borrowing conflicts + let mem_mgr = self + .mem_mgr + .take() + .ok_or_else(|| new_error!("mem_mgr should be initialized"))?; + + let result = VirtualCPU::run( self.as_mut_hypervisor(), - outb_hdl, - mem_access_hdl, + &mem_mgr, #[cfg(gdb)] dbg_mem_access_fn, - )?; + ); + + // Put mem_mgr back + self.mem_mgr = Some(mem_mgr); + result?; Ok(()) } @@ -540,8 +555,7 @@ impl Hypervisor for KVMDriver { fn dispatch_call_from_host( &mut self, dispatch_func_addr: RawPtr, - outb_handle_fn: OutBHandlerWrapper, - mem_access_fn: MemAccessHandlerWrapper, + mem_mgr: &MemMgrWrapper, #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()> { // Reset general purpose registers, then set RIP and RSP @@ -564,8 +578,7 @@ impl Hypervisor for KVMDriver { // run VirtualCPU::run( self.as_mut_hypervisor(), - outb_handle_fn, - mem_access_fn, + mem_mgr, #[cfg(gdb)] dbg_mem_access_fn, )?; @@ -580,7 +593,6 @@ impl Hypervisor for KVMDriver { data: Vec, _rip: u64, _instruction_length: u64, - outb_handle_fn: OutBHandlerWrapper, ) -> Result<()> { // KVM does not need RIP or instruction length, as it automatically sets the RIP @@ -595,15 +607,41 @@ impl Hypervisor for KVMDriver { padded[..copy_len].copy_from_slice(&data[..copy_len]); let value = u32::from_le_bytes(padded); - outb_handle_fn - .try_lock() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? - .call( - #[cfg(feature = "trace_guest")] - self, - port, - value, - )?; + #[cfg(feature = "trace_guest")] + { + // We need to handle the borrow checker issue where we need both: + // - &mut MemMgrWrapper (from self.mem_mgr.as_mut()) + // - &mut dyn Hypervisor (from self) + // We'll use a temporary approach to extract the mem_mgr temporarily + let mem_mgr_option = self.mem_mgr.take(); + let mut mem_mgr = + mem_mgr_option.ok_or_else(|| new_error!("mem_mgr not initialized"))?; + let host_funcs = self + .host_funcs + .as_ref() + .ok_or_else(|| new_error!("host_funcs not initialized"))? + .clone(); + + handle_outb(&mut mem_mgr, host_funcs, self, port, value)?; + + // Put the mem_mgr back + self.mem_mgr = Some(mem_mgr); + } + + #[cfg(not(feature = "trace_guest"))] + { + let mem_mgr = self + .mem_mgr + .as_mut() + .ok_or_else(|| new_error!("mem_mgr not initialized"))?; + let host_funcs = self + .host_funcs + .as_ref() + .ok_or_else(|| new_error!("host_funcs not initialized"))? + .clone(); + + handle_outb(mem_mgr, host_funcs, port, value)?; + } } Ok(()) diff --git a/src/hyperlight_host/src/hypervisor/mod.rs b/src/hyperlight_host/src/hypervisor/mod.rs index a5332d9be..e34175ba0 100644 --- a/src/hyperlight_host/src/hypervisor/mod.rs +++ b/src/hyperlight_host/src/hypervisor/mod.rs @@ -22,12 +22,13 @@ use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags}; use crate::metrics::METRIC_GUEST_CANCELLATION; #[cfg(feature = "trace_guest")] use crate::sandbox::TraceInfo; -use crate::{HyperlightError, Result, log_then_return, new_error}; +use crate::{HyperlightError, Result, log_then_return}; /// Util for handling x87 fpu state #[cfg(any(kvm, mshv, target_os = "windows"))] pub mod fpu; /// Handlers for Hypervisor custom logic +#[cfg(gdb)] pub mod handlers; /// HyperV-on-linux functionality #[cfg(mshv)] @@ -72,10 +73,11 @@ use gdb::VcpuStopReason; #[cfg(gdb)] use self::handlers::{DbgMemAccessHandlerCaller, DbgMemAccessHandlerWrapper}; -use self::handlers::{ - MemAccessHandlerCaller, MemAccessHandlerWrapper, OutBHandlerCaller, OutBHandlerWrapper, -}; use crate::mem::ptr::RawPtr; +use crate::mem::shared_mem::HostSharedMemory; +use crate::sandbox::host_funcs::FunctionRegistry; +use crate::sandbox::mem_access::handle_mem_access; +use crate::sandbox::mem_mgr::MemMgrWrapper; cfg_if::cfg_if! { if #[cfg(feature = "init-paging")] { @@ -134,7 +136,7 @@ pub enum TraceRegister { } /// A common set of hypervisor functionality -pub(crate) trait Hypervisor: Debug + Sync + Send { +pub(crate) trait Hypervisor: Debug + Send { /// Initialise the internally stored vCPU with the given PEB address and /// random number seed, then run it until a HLT instruction. #[allow(clippy::too_many_arguments)] @@ -143,8 +145,8 @@ pub(crate) trait Hypervisor: Debug + Sync + Send { peb_addr: RawPtr, seed: u64, page_size: u32, - outb_handle_fn: OutBHandlerWrapper, - mem_access_fn: MemAccessHandlerWrapper, + mem_mgr: MemMgrWrapper, + host_funcs: Arc>, guest_max_log_level: Option, #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()>; @@ -168,8 +170,7 @@ pub(crate) trait Hypervisor: Debug + Sync + Send { fn dispatch_call_from_host( &mut self, dispatch_func_addr: RawPtr, - outb_handle_fn: OutBHandlerWrapper, - mem_access_fn: MemAccessHandlerWrapper, + mem_mgr: &MemMgrWrapper, #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()>; @@ -180,7 +181,6 @@ pub(crate) trait Hypervisor: Debug + Sync + Send { data: Vec, rip: u64, instruction_length: u64, - outb_handle_fn: OutBHandlerWrapper, ) -> Result<()>; /// Run the vCPU @@ -289,8 +289,7 @@ impl VirtualCPU { #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] pub(crate) fn run( hv: &mut dyn Hypervisor, - outb_handle_fn: Arc>, - mem_access_fn: Arc>, + mem_mgr: &MemMgrWrapper, #[cfg(gdb)] dbg_mem_access_fn: Arc>, ) -> Result<()> { loop { @@ -306,17 +305,13 @@ impl VirtualCPU { break; } Ok(HyperlightExit::IoOut(port, data, rip, instruction_length)) => { - hv.handle_io(port, data, rip, instruction_length, outb_handle_fn.clone())? + hv.handle_io(port, data, rip, instruction_length)? } Ok(HyperlightExit::Mmio(addr)) => { #[cfg(crashdump)] crashdump::generate_crashdump(hv)?; - mem_access_fn - .clone() - .try_lock() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? - .call()?; + handle_mem_access(mem_mgr)?; log_then_return!("MMIO access address {:#x}", addr); } @@ -531,10 +526,6 @@ pub(crate) mod tests { use hyperlight_testing::dummy_guest_as_string; - use super::handlers::{MemAccessHandler, OutBHandler, OutBHandlerFunction}; - #[cfg(gdb)] - use crate::hypervisor::DbgMemAccessHandlerCaller; - use crate::mem::ptr::RawPtr; use crate::sandbox::uninitialized::GuestBinary; #[cfg(any(crashdump, gdb))] use crate::sandbox::uninitialized::SandboxRuntimeConfig; @@ -542,36 +533,16 @@ pub(crate) mod tests { use crate::sandbox::{SandboxConfiguration, UninitializedSandbox}; use crate::{Result, is_hypervisor_present, new_error}; - #[cfg(gdb)] - struct DbgMemAccessHandler {} - - #[cfg(gdb)] - impl DbgMemAccessHandlerCaller for DbgMemAccessHandler { - fn read(&mut self, _offset: usize, _data: &mut [u8]) -> Result<()> { - Ok(()) - } - - fn write(&mut self, _offset: usize, _data: &[u8]) -> Result<()> { - Ok(()) - } - - fn get_code_offset(&mut self) -> Result { - Ok(0) - } - } - #[test] fn test_initialise() -> Result<()> { if !is_hypervisor_present() { return Ok(()); } - let mem_access_handler = { - let func: Box Result<()> + Send> = Box::new(|| -> Result<()> { Ok(()) }); - Arc::new(Mutex::new(MemAccessHandler::from(func))) - }; + use crate::mem::ptr::RawPtr; + use crate::sandbox::host_funcs::FunctionRegistry; #[cfg(gdb)] - let dbg_mem_access_handler = Arc::new(Mutex::new(DbgMemAccessHandler {})); + use crate::sandbox::mem_access::dbg_mem_access_handler_wrapper; let filename = dummy_guest_as_string().map_err(|e| new_error!("{}", e))?; @@ -580,7 +551,7 @@ pub(crate) mod tests { let rt_cfg: SandboxRuntimeConfig = Default::default(); let sandbox = UninitializedSandbox::new(GuestBinary::FilePath(filename.clone()), Some(config))?; - let (_hshm, mut gshm) = sandbox.mgr.build(); + let (mem_mgr, mut gshm) = sandbox.mgr.build(); let mut vm = set_up_hypervisor_partition( &mut gshm, &config, @@ -588,23 +559,29 @@ pub(crate) mod tests { &rt_cfg, sandbox.load_info, )?; - let outb_handler: Arc> = { - #[cfg(feature = "trace_guest")] - #[allow(clippy::type_complexity)] - let func: OutBHandlerFunction = Box::new(|_, _, _| -> Result<()> { Ok(()) }); - #[cfg(not(feature = "trace_guest"))] - let func: OutBHandlerFunction = Box::new(|_, _| -> Result<()> { Ok(()) }); - Arc::new(Mutex::new(OutBHandler::from(func))) - }; + + // Set up required parameters for initialise + let peb_addr = RawPtr::from(0x1000u64); // Dummy PEB address + let seed = 12345u64; // Random seed + let page_size = 4096u32; // Standard page size + let host_funcs = Arc::new(Mutex::new(FunctionRegistry::default())); + let guest_max_log_level = Some(log::LevelFilter::Error); + + #[cfg(gdb)] + let dbg_mem_access_fn = dbg_mem_access_handler_wrapper(mem_mgr.clone()); + + // Test the initialise method vm.initialise( - RawPtr::from(0x230000), - 1234567890, - 4096, - outb_handler, - mem_access_handler, - None, + peb_addr, + seed, + page_size, + mem_mgr, + host_funcs, + guest_max_log_level, #[cfg(gdb)] - dbg_mem_access_handler, - ) + dbg_mem_access_fn, + )?; + + Ok(()) } } diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index 91722457b..ada0ea174 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -34,7 +34,6 @@ use crate::func::guest_err::check_for_guest_error; use crate::func::{ParameterTuple, SupportedReturnType}; #[cfg(gdb)] use crate::hypervisor::handlers::DbgMemAccessHandlerWrapper; -use crate::hypervisor::handlers::{MemAccessHandlerCaller, OutBHandlerCaller}; use crate::hypervisor::{Hypervisor, InterruptHandle}; #[cfg(unix)] use crate::mem::memory_region::MemoryRegionType; @@ -57,8 +56,6 @@ pub struct MultiUseSandbox { pub(super) _host_funcs: Arc>, pub(crate) mem_mgr: MemMgrWrapper, vm: Box, - out_hdl: Arc>, - mem_hdl: Arc>, dispatch_ptr: RawPtr, #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, @@ -75,8 +72,6 @@ impl MultiUseSandbox { host_funcs: Arc>, mgr: MemMgrWrapper, vm: Box, - out_hdl: Arc>, - mem_hdl: Arc>, dispatch_ptr: RawPtr, #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> MultiUseSandbox { @@ -84,8 +79,6 @@ impl MultiUseSandbox { _host_funcs: host_funcs, mem_mgr: mgr, vm, - out_hdl, - mem_hdl, dispatch_ptr, #[cfg(gdb)] dbg_mem_access_fn, @@ -237,8 +230,7 @@ impl MultiUseSandbox { self.vm.dispatch_call_from_host( self.dispatch_ptr.clone(), - self.out_hdl.clone(), - self.mem_hdl.clone(), + &self.mem_mgr, #[cfg(gdb)] self.dbg_mem_access_fn.clone(), )?; diff --git a/src/hyperlight_host/src/sandbox/mem_access.rs b/src/hyperlight_host/src/sandbox/mem_access.rs index 6a6078b2c..ea0d3a3fe 100644 --- a/src/hyperlight_host/src/sandbox/mem_access.rs +++ b/src/hyperlight_host/src/sandbox/mem_access.rs @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +#[cfg(gdb)] use std::sync::{Arc, Mutex}; use tracing::{Span, instrument}; @@ -22,14 +23,11 @@ use super::mem_mgr::MemMgrWrapper; use crate::error::HyperlightError::StackOverflow; #[cfg(gdb)] use crate::hypervisor::handlers::{DbgMemAccessHandlerCaller, DbgMemAccessHandlerWrapper}; -use crate::hypervisor::handlers::{ - MemAccessHandler, MemAccessHandlerFunction, MemAccessHandlerWrapper, -}; use crate::mem::shared_mem::HostSharedMemory; use crate::{Result, log_then_return}; #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] -pub(super) fn handle_mem_access_impl(wrapper: &MemMgrWrapper) -> Result<()> { +pub(crate) fn handle_mem_access(wrapper: &MemMgrWrapper) -> Result<()> { if !wrapper.check_stack_guard()? { log_then_return!(StackOverflow()); } @@ -37,16 +35,6 @@ pub(super) fn handle_mem_access_impl(wrapper: &MemMgrWrapper) Ok(()) } -#[instrument(skip_all, parent = Span::current(), level= "Trace")] -pub(crate) fn mem_access_handler_wrapper( - wrapper: MemMgrWrapper, -) -> MemAccessHandlerWrapper { - let mem_access_func: MemAccessHandlerFunction = - Box::new(move || handle_mem_access_impl(&wrapper)); - let mem_access_hdl = MemAccessHandler::from(mem_access_func); - Arc::new(Mutex::new(mem_access_hdl)) -} - #[cfg(gdb)] struct DbgMemAccessContainer { wrapper: MemMgrWrapper, diff --git a/src/hyperlight_host/src/sandbox/outb.rs b/src/hyperlight_host/src/sandbox/outb.rs index 62b3bf0c1..20a5d8f94 100644 --- a/src/hyperlight_host/src/sandbox/outb.rs +++ b/src/hyperlight_host/src/sandbox/outb.rs @@ -34,14 +34,15 @@ use tracing_log::format_trace; use super::host_funcs::FunctionRegistry; use super::mem_mgr::MemMgrWrapper; -use crate::hypervisor::handlers::{OutBHandler, OutBHandlerFunction, OutBHandlerWrapper}; +#[cfg(feature = "trace_guest")] +use crate::hypervisor::Hypervisor; #[cfg(feature = "trace_guest")] use crate::mem::layout::SandboxMemoryLayout; use crate::mem::mgr::SandboxMemoryManager; use crate::mem::shared_mem::HostSharedMemory; -use crate::{HyperlightError, Result, new_error}; #[cfg(feature = "trace_guest")] -use crate::{hypervisor::Hypervisor, sandbox::TraceInfo}; +use crate::sandbox::TraceInfo; +use crate::{HyperlightError, Result, new_error}; #[instrument(err(Debug), skip_all, parent = Span::current(), level="Trace")] pub(super) fn outb_log(mgr: &mut SandboxMemoryManager) -> Result<()> { @@ -267,7 +268,7 @@ pub(super) fn record_guest_trace_frame( /// Handles OutB operations from the guest. #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] -fn handle_outb_impl( +pub(crate) fn handle_outb( mem_mgr: &mut MemMgrWrapper, host_funcs: Arc>, #[cfg(feature = "trace_guest")] _hv: &mut dyn Hypervisor, @@ -402,31 +403,6 @@ fn handle_outb_impl( } } } - -/// Given a `MemMgrWrapper` and ` HostFuncsWrapper` -- both passed by _value_ -/// -- return an `OutBHandlerWrapper` wrapping the core OUTB handler logic. -/// -/// TODO: pass at least the `host_funcs_wrapper` param by reference. -#[instrument(skip_all, parent = Span::current(), level= "Trace")] -pub(crate) fn outb_handler_wrapper( - mut mem_mgr_wrapper: MemMgrWrapper, - host_funcs_wrapper: Arc>, -) -> OutBHandlerWrapper { - let outb_func: OutBHandlerFunction = - Box::new(move |#[cfg(feature = "trace_guest")] hv, port, payload| { - handle_outb_impl( - &mut mem_mgr_wrapper, - host_funcs_wrapper.clone(), - #[cfg(feature = "trace_guest")] - hv, - port, - payload, - ) - }); - let outb_hdl = OutBHandler::from(outb_func); - Arc::new(Mutex::new(outb_hdl)) -} - #[cfg(test)] mod tests { use hyperlight_common::flatbuffer_wrappers::guest_log_level::LogLevel; diff --git a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs index ca6eb9e35..5db2722ef 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs @@ -27,7 +27,6 @@ use super::mem_access::dbg_mem_access_handler_wrapper; use super::uninitialized::SandboxRuntimeConfig; use crate::HyperlightError::NoHypervisorFound; use crate::hypervisor::Hypervisor; -use crate::hypervisor::handlers::{MemAccessHandlerCaller, OutBHandlerCaller}; use crate::mem::exe::LoadInfo; use crate::mem::layout::SandboxMemoryLayout; use crate::mem::mgr::SandboxMemoryManager; @@ -41,8 +40,6 @@ use crate::sandbox::TraceInfo; #[cfg(gdb)] use crate::sandbox::config::DebugInfo; use crate::sandbox::host_funcs::FunctionRegistry; -use crate::sandbox::mem_access::mem_access_handler_wrapper; -use crate::sandbox::outb::outb_handler_wrapper; use crate::sandbox::{HostSharedMemory, MemMgrWrapper}; #[cfg(target_os = "linux")] use crate::signal_handlers::setup_signal_handlers; @@ -69,8 +66,6 @@ where Arc>, MemMgrWrapper, Box, - Arc>, - Arc>, RawPtr, ) -> Result, { @@ -82,7 +77,6 @@ where &u_sbox.rt_cfg, u_sbox.load_info, )?; - let outb_hdl = outb_handler_wrapper(hshm.clone(), u_sbox.host_funcs.clone()); let seed = { let mut rng = rand::rng(); @@ -94,7 +88,6 @@ where }; let page_size = u32::try_from(page_size::get())?; - let mem_access_hdl = mem_access_handler_wrapper(hshm.clone()); #[cfg(gdb)] let dbg_mem_access_hdl = dbg_mem_access_handler_wrapper(hshm.clone()); @@ -106,8 +99,8 @@ where peb_addr, seed, page_size, - outb_hdl.clone(), - mem_access_hdl.clone(), + hshm.clone(), + u_sbox.host_funcs.clone(), u_sbox.max_guest_log_level, #[cfg(gdb)] dbg_mem_access_hdl, @@ -122,23 +115,19 @@ where u_sbox.host_funcs, hshm, vm, - outb_hdl, - mem_access_hdl, RawPtr::from(dispatch_function_addr), ) } #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] pub(super) fn evolve_impl_multi_use(u_sbox: UninitializedSandbox) -> Result { - evolve_impl(u_sbox, |hf, hshm, vm, out_hdl, mem_hdl, dispatch_ptr| { + evolve_impl(u_sbox, |hf, hshm, vm, dispatch_ptr| { #[cfg(gdb)] let dbg_mem_wrapper = dbg_mem_access_handler_wrapper(hshm.clone()); Ok(MultiUseSandbox::from_uninit( hf, hshm, vm, - out_hdl, - mem_hdl, dispatch_ptr, #[cfg(gdb)] dbg_mem_wrapper,