diff --git a/.github/workflows/dep_build_test.yml b/.github/workflows/dep_build_test.yml index 91ce867aa..d512ac568 100644 --- a/.github/workflows/dep_build_test.yml +++ b/.github/workflows/dep_build_test.yml @@ -118,6 +118,14 @@ jobs: # with only one driver enabled (kvm/mshv3 features are unix-only, no-op on Windows) just test ${{ inputs.config }} ${{ inputs.hypervisor == 'mshv3' && 'mshv3' || 'kvm' }} + - name: Run no-surrogate VM tests (WHP only) + if: runner.os == 'Windows' + env: + HYPERLIGHT_MAX_SURROGATES: "0" + HYPERLIGHT_INITIAL_SURROGATES: "0" + run: | + cargo test -p hyperlight-host --profile=${{ inputs.config == 'debug' && 'dev' || inputs.config }} --lib no_surrogate_tests -- --test-threads=1 + - name: Run Rust tests with hw-interrupts run: | # with hw-interrupts feature enabled (+ explicit driver on Linux) diff --git a/src/hyperlight_host/src/hypervisor/surrogate_process_manager.rs b/src/hyperlight_host/src/hypervisor/surrogate_process_manager.rs index de15fde0b..e4bd03092 100644 --- a/src/hyperlight_host/src/hypervisor/surrogate_process_manager.rs +++ b/src/hyperlight_host/src/hypervisor/surrogate_process_manager.rs @@ -19,6 +19,7 @@ use std::fs::File; use std::io::Write; use std::mem::size_of; use std::path::{Path, PathBuf}; +use std::sync::OnceLock; use std::sync::atomic::{AtomicUsize, Ordering}; use crossbeam_channel::{Receiver, Sender, TryRecvError, unbounded}; @@ -86,17 +87,20 @@ fn surrogate_binary_name() -> Result { /// (or `None` when the variable is unset or unparsable). /// /// Resolution order: -/// 1. `max` is clamped to `1..=HARD_MAX_SURROGATE_PROCESSES`, defaulting +/// 1. `max` is clamped to `0..=HARD_MAX_SURROGATE_PROCESSES`, defaulting /// to `HARD_MAX_SURROGATE_PROCESSES` when `None`. -/// 2. `initial` is clamped to `1..=max`, defaulting to `max` when `None`. +/// 2. `initial` is clamped to `0..=max`, defaulting to `max` when `None`. /// This guarantees `initial <= max` without an extra conditional. +/// +/// When `max == 0`, surrogates are disabled entirely and the system +/// falls back to `WHvMapGpaRange` (single-VM-per-process mode). fn compute_surrogate_counts(raw_initial: Option, raw_max: Option) -> (usize, usize) { let max = raw_max - .map(|n| n.clamp(1, HARD_MAX_SURROGATE_PROCESSES)) + .map(|n| n.clamp(0, HARD_MAX_SURROGATE_PROCESSES)) .unwrap_or(HARD_MAX_SURROGATE_PROCESSES); - // Clamp initial to 1..=max so it can never exceed the authoritative limit. - let initial = raw_initial.map(|n| n.clamp(1, max)).unwrap_or(max); + // Clamp initial to 0..=max so it can never exceed the authoritative limit. + let initial = raw_initial.map(|n| n.clamp(0, max)).unwrap_or(max); (initial, max) } @@ -104,8 +108,8 @@ fn compute_surrogate_counts(raw_initial: Option, raw_max: Option) /// Returns the (initial, max) surrogate process counts from environment /// variables, applying validation and clamping. /// -/// - `HYPERLIGHT_INITIAL_SURROGATES`: clamped to `1..=max`, default `max`. -/// - `HYPERLIGHT_MAX_SURROGATES`: clamped to `1..=512`, default 512. +/// - `HYPERLIGHT_INITIAL_SURROGATES`: clamped to `0..=max`, default `max`. +/// - `HYPERLIGHT_MAX_SURROGATES`: clamped to `0..=512`, default 512. fn surrogate_process_counts() -> (usize, usize) { let raw_initial = std::env::var(INITIAL_SURROGATES_ENV_VAR) .ok() @@ -353,6 +357,21 @@ pub(crate) fn get_surrogate_process_manager() -> Result<&'static SurrogateProces } } +/// Returns `true` when `HYPERLIGHT_MAX_SURROGATES=0`, meaning surrogate +/// processes are disabled and the system should use `WHvMapGpaRange` +/// (single-VM-per-process mode) instead of `WHvMapGpaRange2`. +/// +/// The result is cached on first call — the env var is read only once. +pub(crate) fn surrogates_disabled() -> bool { + static DISABLED: OnceLock = OnceLock::new(); + *DISABLED.get_or_init(|| { + std::env::var(MAX_SURROGATES_ENV_VAR) + .ok() + .and_then(|v| v.parse::().ok()) + .is_some_and(|n| n == 0) + }) +} + // Creates a job object that will terminate all the surrogate processes when the struct instance is dropped. #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] fn create_job_object() -> Result { @@ -885,9 +904,9 @@ mod tests { "initial should be clamped down to max when it exceeds it" ); - // --- initial below minimum → clamped to 1 --- + // --- initial at zero → allowed (surrogates disabled when max is also 0) --- let (initial, max) = compute_surrogate_counts(Some(0), None); - assert_eq!(initial, 1, "initial should be clamped to minimum of 1"); + assert_eq!(initial, 0, "initial of 0 should be allowed"); assert_eq!( max, HARD_MAX_SURROGATE_PROCESSES, "max should default when unset" @@ -909,10 +928,10 @@ mod tests { "initial should be clamped down to max when it defaults above it" ); - // --- max below minimum → clamped to 1, initial follows --- + // --- max at zero → allowed (surrogates disabled), initial follows --- let (initial, max) = compute_surrogate_counts(None, Some(0)); - assert_eq!(max, 1, "max should be clamped to minimum of 1"); - assert_eq!(initial, 1, "initial should be clamped down to max"); + assert_eq!(max, 0, "max of 0 should be allowed"); + assert_eq!(initial, 0, "initial should be clamped down to max"); // --- max above hard limit → clamped to 512 --- let (initial, max) = compute_surrogate_counts(None, Some(9999)); @@ -947,12 +966,12 @@ mod tests { // gracefully adapts: it only asserts the invariant initial <= max <= 512. let (initial, max) = surrogate_process_counts(); assert!( - (1..=HARD_MAX_SURROGATE_PROCESSES).contains(&initial), - "initial {initial} should be in 1..={HARD_MAX_SURROGATE_PROCESSES}" + (0..=HARD_MAX_SURROGATE_PROCESSES).contains(&initial), + "initial {initial} should be in 0..={HARD_MAX_SURROGATE_PROCESSES}" ); assert!( - (1..=HARD_MAX_SURROGATE_PROCESSES).contains(&max), - "max {max} should be in 1..={HARD_MAX_SURROGATE_PROCESSES}" + (0..=HARD_MAX_SURROGATE_PROCESSES).contains(&max), + "max {max} should be in 0..={HARD_MAX_SURROGATE_PROCESSES}" ); assert!(initial <= max, "initial ({initial}) must be <= max ({max})"); } diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs index 868766eee..3daeeb983 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs @@ -15,6 +15,7 @@ limitations under the License. */ use std::os::raw::c_void; +use std::sync::atomic::{AtomicBool, Ordering}; use hyperlight_common::outb::VmAction; #[cfg(feature = "trace_guest")] @@ -37,7 +38,9 @@ use crate::hypervisor::regs::{ WHP_SREGS_NAMES_LEN, }; use crate::hypervisor::surrogate_process::SurrogateProcess; -use crate::hypervisor::surrogate_process_manager::get_surrogate_process_manager; +use crate::hypervisor::surrogate_process_manager::{ + get_surrogate_process_manager, surrogates_disabled, +}; #[cfg(feature = "hw-interrupts")] use crate::hypervisor::virtual_machine::x86_64::hw_interrupts::TimerThread; use crate::hypervisor::virtual_machine::{ @@ -87,12 +90,19 @@ fn release_file_mapping(view_base: *mut c_void, mapping_handle: HandleWrapper) { } } +/// When surrogates are disabled (`HYPERLIGHT_MAX_SURROGATES=0`), only one WHP +/// partition can exist per process. This flag enforces that constraint and is +/// cleared when the sole `WhpVm` is dropped. +static NO_SURROGATE_VM_ACTIVE: AtomicBool = AtomicBool::new(false); + /// A Windows Hypervisor Platform implementation of a single-vcpu VM #[derive(Debug)] pub(crate) struct WhpVm { partition: WHV_PARTITION_HANDLE, - // Surrogate process for memory mapping - surrogate_process: SurrogateProcess, + // Surrogate process for memory mapping. `None` when surrogates are + // disabled (`HYPERLIGHT_MAX_SURROGATES=0`), in which case + // `WHvMapGpaRange` is used instead of `WHvMapGpaRange2`. + surrogate_process: Option, /// Tracks host-side file mappings (view_base, mapping_handle) for /// cleanup on unmap or drop. Only populated for MappedFile regions. file_mappings: Vec<(HandleWrapper, *mut c_void)>, @@ -101,11 +111,13 @@ pub(crate) struct WhpVm { timer: Option, } -// Safety: `WhpVm` is !Send because it holds `SurrogateProcess` which contains a raw pointer -// `allocated_address` (*mut c_void). This pointer represents a memory mapped view address -// in the surrogate process. It is never dereferenced, only used for address arithmetic and -// resource management (unmapping). This is a system resource that is not bound to the creating -// thread and can be safely transferred between threads. +// Safety: `WhpVm` is !Send because it holds `Option` which +// contains a raw pointer `allocated_address` (*mut c_void). This pointer +// represents a memory mapped view address in the surrogate process. It is +// never dereferenced, only used for address arithmetic and resource management +// (unmapping). This is a system resource that is not bound to the creating +// thread and can be safely transferred between threads. When the `Option` is +// `None` (surrogates disabled), no such pointer exists. // `file_mappings` contains raw pointers that are also kernel resource handles, // safe to use from any thread. unsafe impl Send for WhpVm {} @@ -143,11 +155,23 @@ impl WhpVm { p }; - let mgr = get_surrogate_process_manager() - .map_err(|e| CreateVmError::SurrogateProcess(e.to_string()))?; - let surrogate_process = mgr - .get_surrogate_process() - .map_err(|e| CreateVmError::SurrogateProcess(e.to_string()))?; + let surrogate_process = if surrogates_disabled() { + if NO_SURROGATE_VM_ACTIVE.swap(true, Ordering::SeqCst) { + return Err(CreateVmError::SurrogateProcess( + "HYPERLIGHT_MAX_SURROGATES=0 limits the process to a single VM; \ + a VM is already active" + .into(), + )); + } + None + } else { + let mgr = get_surrogate_process_manager() + .map_err(|e| CreateVmError::SurrogateProcess(e.to_string()))?; + Some( + mgr.get_surrogate_process() + .map_err(|e| CreateVmError::SurrogateProcess(e.to_string()))?, + ) + }; Ok(WhpVm { partition, @@ -183,18 +207,6 @@ impl VirtualMachine for WhpVm { &mut self, (_slot, region): (u32, &MemoryRegion), ) -> Result<(), MapMemoryError> { - // Calculate the surrogate process address for this region - let surrogate_base = self - .surrogate_process - .map( - region.host_region.start.from_handle, - region.host_region.start.handle_base, - region.host_region.start.handle_size, - ®ion.region_type.surrogate_mapping(), - ) - .map_err(|e| MapMemoryError::SurrogateProcess(e.to_string()))?; - let surrogate_addr = surrogate_base.wrapping_add(region.host_region.start.offset); - let flags = region .flags .iter() @@ -212,32 +224,71 @@ impl VirtualMachine for WhpVm { .iter() .fold(WHvMapGpaRangeFlagNone, |acc, flag| acc | *flag); - let whvmapgparange2_func = unsafe { - match try_load_whv_map_gpa_range2() { - Ok(func) => func, - Err(e) => { - return Err(MapMemoryError::LoadApi { - api_name: "WHvMapGpaRange2", - source: e, - }); + match &mut self.surrogate_process { + None => { + let host_addr = (region.host_region.start.handle_base + + region.host_region.start.offset) + as *const c_void; + let res = unsafe { + WHvMapGpaRange( + self.partition, + host_addr, + region.guest_region.start as u64, + region.guest_region.len() as u64, + flags, + ) + }; + if let Err(e) = res { + return Err(MapMemoryError::Hypervisor(HypervisorError::WindowsError(e))); + } + } + Some(surrogate) => { + // Calculate the surrogate process address for this region + let surrogate_base = surrogate + .map( + region.host_region.start.from_handle, + region.host_region.start.handle_base, + region.host_region.start.handle_size, + ®ion.region_type.surrogate_mapping(), + ) + .map_err(|e| MapMemoryError::SurrogateProcess(e.to_string()))?; + let surrogate_addr = surrogate_base.wrapping_add(region.host_region.start.offset); + + // This function dynamically loads the WHvMapGpaRange2 function from the winhvplatform.dll + // WHvMapGpaRange2 only available on Windows 11 or Windows Server 2022 and later + // we do things this way to allow a user trying to load hyperlight on an older version of windows to + // get an error message saying that hyperlight requires a newer version of windows, rather than just failing + // with an error about a missing entrypoint + // This function should always succeed since before we get here we have already checked that the hypervisor is present and + // that we are on a supported version of windows. + let whvmapgparange2_func = unsafe { + match try_load_whv_map_gpa_range2() { + Ok(func) => func, + Err(e) => { + return Err(MapMemoryError::LoadApi { + api_name: "WHvMapGpaRange2", + source: e, + }); + } + } + }; + + let res = unsafe { + whvmapgparange2_func( + self.partition, + surrogate.process_handle.into(), + surrogate_addr, + region.guest_region.start as u64, + region.guest_region.len() as u64, + flags, + ) + }; + if res.is_err() { + return Err(MapMemoryError::Hypervisor(HypervisorError::WindowsError( + windows_result::Error::from_hresult(res), + ))); } } - }; - - let res = unsafe { - whvmapgparange2_func( - self.partition, - self.surrogate_process.process_handle.into(), - surrogate_addr, - region.guest_region.start as u64, - region.guest_region.len() as u64, - flags, - ) - }; - if res.is_err() { - return Err(MapMemoryError::Hypervisor(HypervisorError::WindowsError( - windows_result::Error::from_hresult(res), - ))); } // Track host-side file mappings for cleanup on unmap or drop. @@ -263,8 +314,9 @@ impl VirtualMachine for WhpVm { ) .map_err(|e| UnmapMemoryError::Hypervisor(HypervisorError::WindowsError(e)))?; } - self.surrogate_process - .unmap(region.host_region.start.handle_base); + if let Some(surrogate) = &mut self.surrogate_process { + surrogate.unmap(region.host_region.start.handle_base); + } // Clean up host-side file mapping resources for MappedFile regions. if region.region_type == MemoryRegionType::MappedFile { @@ -1123,6 +1175,10 @@ impl Drop for WhpVm { if let Err(e) = unsafe { WHvDeletePartition(self.partition) } { tracing::error!("Failed to delete partition: {}", e); } + + if self.surrogate_process.is_none() { + NO_SURROGATE_VM_ACTIVE.store(false, Ordering::SeqCst); + } } } @@ -1164,6 +1220,51 @@ unsafe fn try_load_whv_map_gpa_range2() -> windows_result::Result view.addr as *mut u8, WindowsMapping::FileBacked { leading, .. } => leading.addr as *mut u8, + WindowsMapping::DirectAlloc(alloc) => alloc.addr as *mut u8, } } @@ -164,6 +194,7 @@ impl HostMapping { trailing, .. } => leading.size + view.len + trailing.size, + WindowsMapping::DirectAlloc(alloc) => alloc.size, } } @@ -173,6 +204,7 @@ impl HostMapping { match &self.mapping { WindowsMapping::Anonymous { file_mapping, .. } | WindowsMapping::FileBacked { file_mapping, .. } => file_mapping.0, + WindowsMapping::DirectAlloc(_) => INVALID_HANDLE_VALUE, } } } @@ -665,57 +697,95 @@ impl ExclusiveSharedMemory { )); } - let mut dwmaximumsizehigh = 0; - let mut dwmaximumsizelow = 0; + let mapping = if surrogates_disabled() { + let addr = + unsafe { VirtualAlloc(None, total_size, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE) }; + if addr.is_null() { + log_then_return!(HyperlightError::MemoryAllocationFailed( + Error::last_os_error().raw_os_error() + )); + } + let alloc = DirectAllocation { + addr, + size: total_size, + }; - if std::mem::size_of::() == 8 { - dwmaximumsizehigh = (total_size >> 32) as u32; - dwmaximumsizelow = (total_size & 0xFFFFFFFF) as u32; - } + Self::setup_guard_pages(alloc.addr, total_size)?; - // Allocate the memory use CreateFileMapping instead of VirtualAlloc - // This allows us to map the memory into the surrogate process using MapViewOfFile2 + WindowsMapping::DirectAlloc(alloc) + } else { + let mut dwmaximumsizehigh = 0; + let mut dwmaximumsizelow = 0; - let flags = PAGE_READWRITE; + if std::mem::size_of::() == 8 { + dwmaximumsizehigh = (total_size >> 32) as u32; + dwmaximumsizelow = (total_size & 0xFFFFFFFF) as u32; + } - let handle = unsafe { - CreateFileMappingA( - INVALID_HANDLE_VALUE, - None, - flags, - dwmaximumsizehigh, - dwmaximumsizelow, - PCSTR::null(), - )? - }; + // Allocate the memory use CreateFileMapping instead of VirtualAlloc + // This allows us to map the memory into the surrogate process using MapViewOfFile2 - if handle.is_invalid() { - log_then_return!(HyperlightError::MemoryAllocationFailed( - Error::last_os_error().raw_os_error() - )); - } - let file_mapping = FileMapping(handle); + let flags = PAGE_READWRITE; - let file_map = FILE_MAP_ALL_ACCESS; - let addr = unsafe { MapViewOfFile(file_mapping.0, file_map, 0, 0, 0) }; + let handle = unsafe { + CreateFileMappingA( + INVALID_HANDLE_VALUE, + None, + flags, + dwmaximumsizehigh, + dwmaximumsizelow, + PCSTR::null(), + )? + }; - if addr.Value.is_null() { - log_then_return!(HyperlightError::MemoryAllocationFailed( - Error::last_os_error().raw_os_error() - )); - } - let view = MappedView { - addr: addr.Value, - len: total_size, + if handle.is_invalid() { + log_then_return!(HyperlightError::MemoryAllocationFailed( + Error::last_os_error().raw_os_error() + )); + } + let file_mapping = FileMapping(handle); + + let file_map = FILE_MAP_ALL_ACCESS; + let addr = unsafe { MapViewOfFile(file_mapping.0, file_map, 0, 0, 0) }; + + if addr.Value.is_null() { + log_then_return!(HyperlightError::MemoryAllocationFailed( + Error::last_os_error().raw_os_error() + )); + } + let view = MappedView { + addr: addr.Value, + len: total_size, + }; + + Self::setup_guard_pages(view.addr, total_size)?; + + WindowsMapping::Anonymous { view, file_mapping } }; - // Set the first and last pages to be guard pages + Ok(Self { + // HostMapping is only non-Send/Sync because raw pointers + // are not ("as a lint", as the Rust docs say). We don't + // want to mark HostMapping Send/Sync immediately, because + // that could socially imply that it's "safe" to use + // unsafe accesses from multiple threads at once. Instead, we + // directly impl Send and Sync on this type. Since this + // type does have Send and Sync manually impl'd, the Arc + // is not pointless as the lint suggests. + #[allow(clippy::arc_with_non_send_sync)] + region: Arc::new(HostMapping { mapping }), + }) + } + /// Set up guard pages at the first and last pages of the given + /// address range. + #[cfg(target_os = "windows")] + fn setup_guard_pages(base: *mut c_void, total_size: usize) -> Result<()> { let mut unused_out_old_prot_flags = PAGE_PROTECTION_FLAGS(0); // If the following calls to VirtualProtect are changed make sure to update the calls to VirtualProtectEx in surrogate_process_manager.rs - let first_guard_page_start = view.addr; + let first_guard_page_start = base; if let Err(e) = unsafe { VirtualProtect( first_guard_page_start, @@ -727,7 +797,8 @@ impl ExclusiveSharedMemory { log_then_return!(WindowsAPIError(e.clone())); } - let last_guard_page_start = unsafe { view.addr.add(total_size - PAGE_SIZE_USIZE) }; + let last_guard_page_start = + unsafe { (base as *mut u8).add(total_size - PAGE_SIZE_USIZE) as *mut c_void }; if let Err(e) = unsafe { VirtualProtect( last_guard_page_start, @@ -739,20 +810,7 @@ impl ExclusiveSharedMemory { log_then_return!(WindowsAPIError(e.clone())); } - Ok(Self { - // HostMapping is only non-Send/Sync because raw pointers - // are not ("as a lint", as the Rust docs say). We don't - // want to mark HostMapping Send/Sync immediately, because - // that could socially imply that it's "safe" to use - // unsafe accesses from multiple threads at once. Instead, we - // directly impl Send and Sync on this type. Since this - // type does have Send and Sync manually impl'd, the Arc - // is not pointless as the lint suggests. - #[allow(clippy::arc_with_non_send_sync)] - region: Arc::new(HostMapping { - mapping: WindowsMapping::Anonymous { view, file_mapping }, - }), - }) + Ok(()) } /// Internal helper method to get the backing memory as a mutable slice. @@ -1753,12 +1811,14 @@ impl SharedMemory for ReadonlySharedMemory { #[cfg(windows)] fn host_region_base(&self) -> ::HostBaseType { match &self.region().mapping { - WindowsMapping::Anonymous { .. } => super::memory_region::HostRegionBase { - from_handle: self.region().file_mapping_handle().into(), - handle_base: self.region().ptr() as usize, - handle_size: self.region().size(), - offset: PAGE_SIZE_USIZE, - }, + WindowsMapping::Anonymous { .. } | WindowsMapping::DirectAlloc(_) => { + super::memory_region::HostRegionBase { + from_handle: self.region().file_mapping_handle().into(), + handle_base: self.region().ptr() as usize, + handle_size: self.region().size(), + offset: PAGE_SIZE_USIZE, + } + } WindowsMapping::FileBacked { .. } => super::memory_region::HostRegionBase { from_handle: self.region().file_mapping_handle().into(), handle_base: self.base_ptr() as usize,