diff --git a/src/hyperlight_host/src/hypervisor/handlers.rs b/src/hyperlight_host/src/hypervisor/handlers.rs index 9a091b2b1..b63ede713 100644 --- a/src/hyperlight_host/src/hypervisor/handlers.rs +++ b/src/hyperlight_host/src/hypervisor/handlers.rs @@ -18,46 +18,34 @@ use std::sync::{Arc, Mutex}; use tracing::{Span, instrument}; +use crate::mem::shared_mem::HostSharedMemory; +use crate::sandbox::host_funcs::FunctionRegistry; +use crate::sandbox::mem_mgr::MemMgrWrapper; 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 trait OutBHandlerCaller: Sync + Send { - /// Function that gets called when an outb operation has occurred. - fn call(&mut self, 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 type OutBHandlerWrapper = Arc>; - -pub(crate) type OutBHandlerFunction = Box Result<()> + Send>; - -/// A `OutBHandler` implementation using a `OutBHandlerFunction` +/// A `OutBHandler` implementation that holds the memory manager and function registry directly /// /// Note: This handler must live no longer than the `Sandbox` to which it belongs -pub(crate) struct OutBHandler(Arc>); +pub(crate) struct OutBHandler { + mem_mgr: MemMgrWrapper, + host_funcs: 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 OutBHandler { + pub fn new( + mem_mgr: MemMgrWrapper, + host_funcs: Arc>, + ) -> Self { + Self { + mem_mgr, + host_funcs, + } } -} -impl OutBHandlerCaller for OutBHandler { #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] - fn call(&mut self, port: u16, payload: u32) -> Result<()> { - let mut func = self - .0 - .try_lock() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?; - func(port, payload) + pub fn handle_outb(&mut self, port: u16, payload: u32) -> Result<()> { + // Call the handle_outb function directly + crate::sandbox::outb::handle_outb(&mut self.mem_mgr, self.host_funcs.clone(), port, payload) } } diff --git a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs index d9160b6f2..d70a04c5e 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)] @@ -60,7 +60,7 @@ use super::gdb::{ }; #[cfg(gdb)] use super::handlers::DbgMemAccessHandlerWrapper; -use super::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper}; +use super::handlers::{MemAccessHandlerWrapper, OutBHandler}; #[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, @@ -544,7 +544,7 @@ impl Hypervisor for HypervLinuxDriver { peb_addr: RawPtr, seed: u64, page_size: u32, - outb_hdl: OutBHandlerWrapper, + outb_hdl: Arc>, mem_access_hdl: MemAccessHandlerWrapper, max_guest_log_level: Option, #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, @@ -617,7 +617,7 @@ impl Hypervisor for HypervLinuxDriver { fn dispatch_call_from_host( &mut self, dispatch_func_addr: RawPtr, - outb_handle_fn: OutBHandlerWrapper, + outb_handle_fn: Arc>, mem_access_fn: MemAccessHandlerWrapper, #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()> { @@ -658,7 +658,7 @@ impl Hypervisor for HypervLinuxDriver { data: Vec, rip: u64, instruction_length: u64, - outb_handle_fn: OutBHandlerWrapper, + outb_handle_fn: Arc>, ) -> Result<()> { let mut padded = [0u8; 4]; let copy_len = data.len().min(4); @@ -668,7 +668,7 @@ impl Hypervisor for HypervLinuxDriver { outb_handle_fn .try_lock() .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? - .call(port, val)?; + .handle_outb(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 a057c41cc..5a9cbcb5c 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,11 +41,10 @@ use { super::handlers::DbgMemAccessHandlerWrapper, crate::HyperlightError, crate::hypervisor::handlers::DbgMemAccessHandlerCaller, - std::sync::Mutex, }; use super::fpu::{FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; -use super::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper}; +use super::handlers::{MemAccessHandlerWrapper, OutBHandler}; use super::surrogate_process::SurrogateProcess; use super::surrogate_process_manager::*; use super::windows_hypervisor_platform::{VMPartition, VMProcessor}; @@ -579,7 +578,7 @@ impl Hypervisor for HypervWindowsDriver { peb_address: RawPtr, seed: u64, page_size: u32, - outb_hdl: OutBHandlerWrapper, + outb_hdl: Arc>, mem_access_hdl: MemAccessHandlerWrapper, max_guest_log_level: Option, #[cfg(gdb)] dbg_mem_access_hdl: DbgMemAccessHandlerWrapper, @@ -634,7 +633,7 @@ impl Hypervisor for HypervWindowsDriver { fn dispatch_call_from_host( &mut self, dispatch_func_addr: RawPtr, - outb_hdl: OutBHandlerWrapper, + outb_hdl: Arc>, mem_access_hdl: MemAccessHandlerWrapper, #[cfg(gdb)] dbg_mem_access_hdl: DbgMemAccessHandlerWrapper, ) -> Result<()> { @@ -673,7 +672,7 @@ impl Hypervisor for HypervWindowsDriver { data: Vec, rip: u64, instruction_length: u64, - outb_handle_fn: OutBHandlerWrapper, + outb_handle_fn: Arc>, ) -> Result<()> { let mut padded = [0u8; 4]; let copy_len = data.len().min(4); @@ -683,7 +682,7 @@ impl Hypervisor for HypervWindowsDriver { outb_handle_fn .try_lock() .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? - .call(port, val)?; + .handle_outb(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 0802ecb6b..ec990dc09 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; @@ -34,7 +32,7 @@ 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}; +use super::handlers::{MemAccessHandlerWrapper, OutBHandler}; #[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, @@ -448,7 +446,7 @@ impl Hypervisor for KVMDriver { peb_addr: RawPtr, seed: u64, page_size: u32, - outb_hdl: OutBHandlerWrapper, + outb_hdl: Arc>, mem_access_hdl: MemAccessHandlerWrapper, max_guest_log_level: Option, #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, @@ -528,7 +526,7 @@ impl Hypervisor for KVMDriver { fn dispatch_call_from_host( &mut self, dispatch_func_addr: RawPtr, - outb_handle_fn: OutBHandlerWrapper, + outb_handle_fn: Arc>, mem_access_fn: MemAccessHandlerWrapper, #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()> { @@ -568,7 +566,7 @@ impl Hypervisor for KVMDriver { data: Vec, _rip: u64, _instruction_length: u64, - outb_handle_fn: OutBHandlerWrapper, + outb_handle_fn: Arc>, ) -> Result<()> { // KVM does not need RIP or instruction length, as it automatically sets the RIP @@ -586,7 +584,7 @@ impl Hypervisor for KVMDriver { outb_handle_fn .try_lock() .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? - .call(port, value)?; + .handle_outb(port, value)?; } Ok(()) diff --git a/src/hyperlight_host/src/hypervisor/mod.rs b/src/hyperlight_host/src/hypervisor/mod.rs index ecf6acbc5..fe0b43cc0 100644 --- a/src/hyperlight_host/src/hypervisor/mod.rs +++ b/src/hyperlight_host/src/hypervisor/mod.rs @@ -70,9 +70,7 @@ use gdb::VcpuStopReason; #[cfg(gdb)] use self::handlers::{DbgMemAccessHandlerCaller, DbgMemAccessHandlerWrapper}; -use self::handlers::{ - MemAccessHandlerCaller, MemAccessHandlerWrapper, OutBHandlerCaller, OutBHandlerWrapper, -}; +use self::handlers::{MemAccessHandlerCaller, MemAccessHandlerWrapper, OutBHandler}; use crate::mem::ptr::RawPtr; cfg_if::cfg_if! { @@ -126,7 +124,7 @@ pub(crate) trait Hypervisor: Debug + Sync + Send { peb_addr: RawPtr, seed: u64, page_size: u32, - outb_handle_fn: OutBHandlerWrapper, + outb_handle_fn: Arc>, mem_access_fn: MemAccessHandlerWrapper, guest_max_log_level: Option, #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, @@ -151,7 +149,7 @@ pub(crate) trait Hypervisor: Debug + Sync + Send { fn dispatch_call_from_host( &mut self, dispatch_func_addr: RawPtr, - outb_handle_fn: OutBHandlerWrapper, + outb_handle_fn: Arc>, mem_access_fn: MemAccessHandlerWrapper, #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, ) -> Result<()>; @@ -163,7 +161,7 @@ pub(crate) trait Hypervisor: Debug + Sync + Send { data: Vec, rip: u64, instruction_length: u64, - outb_handle_fn: OutBHandlerWrapper, + outb_handle_fn: Arc>, ) -> Result<()>; /// Run the vCPU @@ -261,7 +259,7 @@ impl VirtualCPU { #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")] pub fn run( hv: &mut dyn Hypervisor, - outb_handle_fn: Arc>, + outb_handle_fn: Arc>, mem_access_fn: Arc>, #[cfg(gdb)] dbg_mem_access_fn: Arc>, ) -> Result<()> { @@ -538,11 +536,18 @@ pub(crate) mod tests { return Ok(()); } - let outb_handler: Arc> = { - let func: Box Result<()> + Send> = - Box::new(|_, _| -> Result<()> { Ok(()) }); - Arc::new(Mutex::new(OutBHandler::from(func))) - }; + // For testing, we'll create a minimal sandbox to get the required components + let filename = dummy_guest_as_string().map_err(|e| new_error!("{}", e))?; + let config: SandboxConfiguration = Default::default(); + let sandbox = + UninitializedSandbox::new(GuestBinary::FilePath(filename.clone()), Some(config))?; + let (hshm, mut gshm) = sandbox.mgr.build(); + + // Create outb_handler using the same approach as the actual code + let outb_handler = Arc::new(Mutex::new(OutBHandler::new( + hshm.clone(), + sandbox.host_funcs.clone(), + ))); let mem_access_handler = { let func: Box Result<()> + Send> = Box::new(|| -> Result<()> { Ok(()) }); Arc::new(Mutex::new(MemAccessHandler::from(func))) @@ -550,14 +555,8 @@ pub(crate) mod tests { #[cfg(gdb)] let dbg_mem_access_handler = Arc::new(Mutex::new(DbgMemAccessHandler {})); - let filename = dummy_guest_as_string().map_err(|e| new_error!("{}", e))?; - - let config: SandboxConfiguration = Default::default(); #[cfg(any(crashdump, gdb))] let rt_cfg: SandboxRuntimeConfig = Default::default(); - let sandbox = - UninitializedSandbox::new(GuestBinary::FilePath(filename.clone()), Some(config))?; - let (_hshm, mut gshm) = sandbox.mgr.build(); let mut vm = set_up_hypervisor_partition( &mut gshm, &config, diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index 6208d8f85..c61eae67a 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -34,7 +34,7 @@ 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::handlers::{MemAccessHandlerCaller, OutBHandler}; use crate::hypervisor::{Hypervisor, InterruptHandle}; #[cfg(unix)] use crate::mem::memory_region::MemoryRegionType; @@ -57,7 +57,7 @@ pub struct MultiUseSandbox { pub(super) _host_funcs: Arc>, pub(crate) mem_mgr: MemMgrWrapper, vm: Box, - out_hdl: Arc>, + out_hdl: Arc>, mem_hdl: Arc>, dispatch_ptr: RawPtr, #[cfg(gdb)] @@ -75,7 +75,7 @@ impl MultiUseSandbox { host_funcs: Arc>, mgr: MemMgrWrapper, vm: Box, - out_hdl: Arc>, + out_hdl: Arc>, mem_hdl: Arc>, dispatch_ptr: RawPtr, #[cfg(gdb)] dbg_mem_access_fn: DbgMemAccessHandlerWrapper, diff --git a/src/hyperlight_host/src/sandbox/outb.rs b/src/hyperlight_host/src/sandbox/outb.rs index dcdd96589..03348138f 100644 --- a/src/hyperlight_host/src/sandbox/outb.rs +++ b/src/hyperlight_host/src/sandbox/outb.rs @@ -26,7 +26,7 @@ use tracing_log::format_trace; use super::host_funcs::FunctionRegistry; use super::mem_mgr::MemMgrWrapper; -use crate::hypervisor::handlers::{OutBHandler, OutBHandlerFunction, OutBHandlerWrapper}; +use crate::hypervisor::handlers::OutBHandler; use crate::mem::mgr::SandboxMemoryManager; use crate::mem::shared_mem::HostSharedMemory; use crate::{HyperlightError, Result, new_error}; @@ -146,7 +146,7 @@ fn outb_abort(mem_mgr: &mut MemMgrWrapper, data: u32) -> Resul /// 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>, port: u16, @@ -184,23 +184,15 @@ fn handle_outb_impl( } /// Given a `MemMgrWrapper` and ` HostFuncsWrapper` -- both passed by _value_ -/// -- return an `OutBHandlerWrapper` wrapping the core OUTB handler logic. +/// -- return an `Arc>` 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, + mem_mgr_wrapper: MemMgrWrapper, host_funcs_wrapper: Arc>, -) -> OutBHandlerWrapper { - let outb_func: OutBHandlerFunction = Box::new(move |port, payload| { - handle_outb_impl( - &mut mem_mgr_wrapper, - host_funcs_wrapper.clone(), - port, - payload, - ) - }); - let outb_hdl = OutBHandler::from(outb_func); +) -> Arc> { + let outb_hdl = OutBHandler::new(mem_mgr_wrapper, host_funcs_wrapper); Arc::new(Mutex::new(outb_hdl)) } diff --git a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs index 696eb92a1..040473875 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs @@ -27,7 +27,7 @@ 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::hypervisor::handlers::{MemAccessHandlerCaller, OutBHandler}; use crate::mem::layout::SandboxMemoryLayout; use crate::mem::mgr::SandboxMemoryManager; use crate::mem::ptr::{GuestPtr, RawPtr}; @@ -66,7 +66,7 @@ where Arc>, MemMgrWrapper, Box, - Arc>, + Arc>, Arc>, RawPtr, ) -> Result,