From 8514a34e6c3b9237a8f990b9efcd7dea0f356c2e Mon Sep 17 00:00:00 2001 From: Lucy Menon <168595099+syntactically@users.noreply.github.com> Date: Mon, 20 Jul 2026 19:31:22 +0100 Subject: [PATCH 01/17] nix: Support developing on macOS Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- flake.lock | 6 +++--- flake.nix | 34 ++++++++++++++++++---------------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/flake.lock b/flake.lock index 487dd3b5c..faf4c21f8 100644 --- a/flake.lock +++ b/flake.lock @@ -2,11 +2,11 @@ "nodes": { "nixpkgs": { "locked": { - "lastModified": 1770115704, - "narHash": "sha256-KHFT9UWOF2yRPlAnSXQJh6uVcgNcWlFqqiAZ7OVlHNc=", + "lastModified": 1782723713, + "narHash": "sha256-oPXCU/SSUokcGaJREHibG1CBX3+s/W7orDWQOZDsEeQ=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "e6eae2ee2110f3d31110d5c222cd395303343b08", + "rev": "b5aa0fbd538984f6e3d201be0005b4463d8b09f8", "type": "github" }, "original": { diff --git a/flake.nix b/flake.nix index ae01e57b9..f8a0f5ce8 100644 --- a/flake.nix +++ b/flake.nix @@ -11,8 +11,8 @@ orig = super.rustChannelOf args; patchRustPkg = pkg: (pkg.overrideAttrs (oA: { buildCommand = (builtins.replaceStrings - [ "rustc,rustdoc" ] - [ "rustc,rustdoc,clippy-driver,cargo-clippy,miri,cargo-miri" ] + [ "rustc,rustdoc" "librustc_driver-*.so" ] + [ "rustc,rustdoc,clippy-driver,cargo-clippy,miri,cargo-miri" "librustc_driver-*.{so,dylib}" ] oA.buildCommand) + (let wrapperPath = self.path + "/pkgs/build-support/bintools-wrapper/ld-wrapper.sh"; baseOut = self.clangStdenv.cc.bintools.out; @@ -51,7 +51,7 @@ toolchainVersionAttrs = args; }; })) // { - targetPlatforms = [ "aarch64-linux" "x86_64-linux" ]; + targetPlatforms = [ "aarch64-linux" "x86_64-linux" "aarch64-darwin" ]; badTargetPlatforms = [ ]; }; overrideRustPkg = pkg: self.lib.makeOverridable (origArgs: @@ -82,7 +82,7 @@ "x86_64-unknown-linux-gnu" "x86_64-pc-windows-msvc" "x86_64-unknown-none" "wasm32-wasip1" "wasm32-wasip2" "wasm32-unknown-unknown" - "aarch64-unknown-none" + "aarch64-unknown-none" "aarch64-apple-darwin" ]; extensions = [ "rust-src" ] ++ (if args.channel == "nightly" then [ "miri-preview" ] else []); }); @@ -166,13 +166,14 @@ esac if [ -f ''${root}/flake.nix ]; then - mkdir -p $root/$.cargo - cat >$root/.cargo/config.toml <>$root/.cargo/config.toml < Date: Mon, 6 Jul 2026 15:18:36 +0100 Subject: [PATCH 02/17] Reduce duplication of interrupt handle state machines Previously, the Linux and WHP interrupt handle implementations used exactly the same logic to update three bits of atomic state, and further interrupt handle implementations would likely need the same. This commit extracts the state machine logic into a single shared implementation used by both interrupt handles. It also refactors `WindowsInterruptHandle` to pull the logic around locking into another separate type, which will be shared with other interrupt handle implementations in the future Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- .../src/hypervisor/hyperlight_vm/mod.rs | 16 +- .../src/hypervisor/hyperlight_vm/x86_64.rs | 18 +- src/hyperlight_host/src/hypervisor/mod.rs | 385 ++++++++---------- 3 files changed, 175 insertions(+), 244 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs index 93368a335..65552a93f 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs @@ -591,7 +591,7 @@ impl HyperlightVm { } pub(crate) fn clear_cancel(&self) { - self.interrupt_handle.clear_cancel(); + self.interrupt_handle.state().clear_cancel(); } pub(super) fn run( @@ -610,12 +610,12 @@ impl HyperlightVm { // without sending any signals/WHV api calls #[cfg(any(kvm, mshv3))] self.interrupt_handle.set_tid(); - self.interrupt_handle.set_running(); + self.interrupt_handle.state().set_running(); // NOTE: `set_running()`` must be called before checking `is_cancelled()` // otherwise we risk missing a call to `kill()` because the vcpu would not be marked as running yet so signals won't be sent - let exit_reason = if self.interrupt_handle.is_cancelled() - || self.interrupt_handle.is_debug_interrupted() + let exit_reason = if self.interrupt_handle.state().is_cancelled() + || self.interrupt_handle.state().is_debug_interrupted() { Ok(VmExit::Cancelled()) } else { @@ -655,14 +655,14 @@ impl HyperlightVm { // If kill() is called and ran to completion BEFORE this line executes: // - CANCEL_BIT will be set. Cancellation is deferred to the next iteration. // - Signals will be sent until `clear_running()` is called, which is ok - self.interrupt_handle.clear_running(); + self.interrupt_handle.state().clear_running(); // ===== KILL() TIMING POINT 5: Before capturing cancel_requested ===== // If kill() is called and ran to completion BEFORE this line executes: // - CANCEL_BIT will be set. Cancellation is deferred to the next iteration. // - Signals will not be sent - let cancel_requested = self.interrupt_handle.is_cancelled(); - let debug_interrupted = self.interrupt_handle.is_debug_interrupted(); + let cancel_requested = self.interrupt_handle.state().is_cancelled(); + let debug_interrupted = self.interrupt_handle.state().is_debug_interrupted(); // ===== KILL() TIMING POINT 6: Before checking exit_reason ===== // If kill() is called and ran to completion BEFORE this line executes: @@ -757,7 +757,7 @@ impl HyperlightVm { // If the vcpu was interrupted by a debugger, we need to handle it #[cfg(gdb)] { - self.interrupt_handle.clear_debug_interrupt(); + self.interrupt_handle.state().clear_debug_interrupt(); if let Err(e) = self.handle_debug(mem_mgr, VcpuStopReason::Interrupt) { break Err(e.into()); } diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs index 1227d60ec..6cd3052ea 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs @@ -16,11 +16,6 @@ limitations under the License. #[cfg(gdb)] use std::collections::HashMap; -#[cfg(any(kvm, mshv3))] -use std::sync::atomic::AtomicBool; -use std::sync::atomic::AtomicU8; -#[cfg(any(kvm, mshv3))] -use std::sync::atomic::AtomicU64; use std::sync::{Arc, Mutex}; use tracing::{Span, instrument}; @@ -30,6 +25,8 @@ use super::*; use crate::hypervisor::InterruptHandleImpl; #[cfg(any(kvm, mshv3))] use crate::hypervisor::LinuxInterruptHandle; +#[cfg(target_os = "windows")] +use crate::hypervisor::WindowsInterruptHandle; #[cfg(crashdump)] use crate::hypervisor::crashdump; #[cfg(gdb)] @@ -50,8 +47,6 @@ use crate::hypervisor::virtual_machine::whp::WhpVm; use crate::hypervisor::virtual_machine::{ HypervisorType, RegisterError, VmError, XCR0_RESET, get_available_hypervisor, }; -#[cfg(target_os = "windows")] -use crate::hypervisor::{PartitionState, WindowsInterruptHandle}; #[cfg(crashdump)] use crate::mem::memory_region::MemoryRegion; use crate::mem::mgr::SandboxMemoryManager; @@ -130,13 +125,8 @@ impl HyperlightVm { }); #[cfg(target_os = "windows")] - let interrupt_handle: Arc = Arc::new(WindowsInterruptHandle { - state: AtomicU8::new(0), - partition_state: std::sync::RwLock::new(PartitionState { - handle: vm.partition_handle(), - dropped: false, - }), - }); + let interrupt_handle: Arc = + Arc::new(WindowsInterruptHandle::new(vm.partition_handle())); let reset_indices = vm .msr_reset_indices(config.get_guest_msrs()) diff --git a/src/hyperlight_host/src/hypervisor/mod.rs b/src/hyperlight_host/src/hypervisor/mod.rs index 51c658423..bcda61b08 100644 --- a/src/hyperlight_host/src/hypervisor/mod.rs +++ b/src/hyperlight_host/src/hypervisor/mod.rs @@ -40,39 +40,108 @@ pub(crate) mod hyperlight_vm; use std::fmt::Debug; #[cfg(any(kvm, mshv3))] -use std::sync::atomic::{AtomicBool, AtomicU8, AtomicU64, Ordering}; -#[cfg(target_os = "windows")] +use std::sync::atomic::{AtomicBool, AtomicU64}; use std::sync::atomic::{AtomicU8, Ordering}; #[cfg(any(kvm, mshv3))] use std::time::Duration; -/// A trait for platform-specific interrupt handle implementation details -pub(crate) trait InterruptHandleImpl: InterruptHandle { - /// Set the thread ID for the vcpu thread - #[cfg(any(kvm, mshv3))] - fn set_tid(&self); +#[derive(Debug)] +pub(crate) struct InterruptHandleStateMachine(AtomicU8); +impl InterruptHandleStateMachine { + const RUNNING_BIT: u8 = 1 << 1; + const CANCEL_BIT: u8 = 1 << 0; + #[cfg(gdb)] + const DEBUG_INTERRUPT_BIT: u8 = 1 << 2; + + fn new() -> Self { + Self(AtomicU8::new(0)) + } /// Set the running state - fn set_running(&self); + pub(crate) fn set_running(&self) { + // Release ordering to ensure that the tid store (which uses Release) + // is visible to any thread that observes running=true via Acquire ordering. + // This prevents the interrupt thread from reading a stale tid value. + self.0.fetch_or(Self::RUNNING_BIT, Ordering::Release); + } /// Clear the running state - fn clear_running(&self); - - /// Mark the handle as dropped - fn set_dropped(&self); + pub(crate) fn clear_running(&self) { + // Release ordering to ensure all vcpu operations are visible before clearing running + self.0.fetch_and(!Self::RUNNING_BIT, Ordering::Release); + } /// Check if cancellation was requested - fn is_cancelled(&self) -> bool; + pub(crate) fn is_cancelled(&self) -> bool { + self.get_running_cancel_debug().1 + } + + /// Set the cancellation request flag + fn set_cancel(&self) { + // Release ordering ensures that any writes before kill() are visible to the vcpu thread + // when it checks is_cancelled() with Acquire ordering + self.0.fetch_or(Self::CANCEL_BIT, Ordering::Release); + } /// Clear the cancellation request flag - fn clear_cancel(&self); + fn clear_cancel(&self) { + // Release ordering to ensure that any operations from the previous run() + // are visible to other threads. While this is typically called by the vcpu thread + // at the start of run(), the VM itself can move between threads across guest calls. + self.0.fetch_and(!Self::CANCEL_BIT, Ordering::Release); + } /// Check if debug interrupt was requested (always returns false when gdb feature is disabled) - fn is_debug_interrupted(&self) -> bool; + pub(crate) fn is_debug_interrupted(&self) -> bool { + #[cfg(gdb)] + { + self.get_running_cancel_debug().2 + } + #[cfg(not(gdb))] + { + false + } + } - // Clear the debug interrupt request flag + /// Clear the debug interrupt request flag #[cfg(gdb)] - fn clear_debug_interrupt(&self); + fn set_debug_interrupt(&self) { + self.0 + .fetch_or(Self::DEBUG_INTERRUPT_BIT, Ordering::Release); + } + + /// Clear the debug interrupt request flag + #[cfg(gdb)] + fn clear_debug_interrupt(&self) { + self.0 + .fetch_and(!Self::DEBUG_INTERRUPT_BIT, Ordering::Release); + } + + /// Get the running, cancel and debug flags atomically. + fn get_running_cancel_debug(&self) -> (bool, bool, bool) { + let state = self.0.load(Ordering::Acquire); + let running = state & Self::RUNNING_BIT != 0; + let cancel = state & Self::CANCEL_BIT != 0; + #[cfg(gdb)] + let debug = state & Self::DEBUG_INTERRUPT_BIT != 0; + #[cfg(not(gdb))] + let debug = false; + (running, cancel, debug) + } +} + +/// A trait for platform-specific interrupt handle implementation details +pub(crate) trait InterruptHandleImpl: InterruptHandle { + /// Set the thread ID for the vcpu thread + #[cfg(any(kvm, mshv3))] + fn set_tid(&self); + + /// Mark the handle as dropped + fn set_dropped(&self); + + /// Local access the shared state, which does not perform any + /// operations other than updating the state machine + fn state(&self) -> &InterruptHandleStateMachine; } /// A trait for handling interrupts to a sandbox's vcpu @@ -105,16 +174,7 @@ pub trait InterruptHandle: Send + Sync + Debug { #[cfg(any(kvm, mshv3))] #[derive(Debug)] pub(super) struct LinuxInterruptHandle { - /// Atomic value packing vcpu execution state. - /// - /// Bit layout: - /// - Bit 2: DEBUG_INTERRUPT_BIT - set when debugger interrupt is requested - /// - Bit 1: RUNNING_BIT - set when vcpu is actively running - /// - Bit 0: CANCEL_BIT - set when cancellation has been requested - /// - /// CANCEL_BIT persists across vcpu exits/re-entries within a single `VirtualCPU::run()` call - /// (e.g., during host function calls), but is cleared at the start of each new `VirtualCPU::run()` call. - state: AtomicU8, + state: InterruptHandleStateMachine, /// Thread ID where the vcpu is running. /// @@ -134,33 +194,18 @@ pub(super) struct LinuxInterruptHandle { #[cfg(any(kvm, mshv3))] impl LinuxInterruptHandle { - const RUNNING_BIT: u8 = 1 << 1; - const CANCEL_BIT: u8 = 1 << 0; - #[cfg(gdb)] - const DEBUG_INTERRUPT_BIT: u8 = 1 << 2; - /// Get the running, cancel and debug flags atomically. /// /// # Memory Ordering /// Uses `Acquire` ordering to synchronize with the `Release` in `set_running()` and `kill()`. /// This ensures that when we observe running=true, we also see the correct `tid` value. - fn get_running_cancel_debug(&self) -> (bool, bool, bool) { - let state = self.state.load(Ordering::Acquire); - let running = state & Self::RUNNING_BIT != 0; - let cancel = state & Self::CANCEL_BIT != 0; - #[cfg(gdb)] - let debug = state & Self::DEBUG_INTERRUPT_BIT != 0; - #[cfg(not(gdb))] - let debug = false; - (running, cancel, debug) - } fn send_signal(&self) -> bool { let signal_number = libc::SIGRTMIN() + self.sig_rt_min_offset as libc::c_int; let mut sent_signal = false; loop { - let (running, cancel, debug) = self.get_running_cancel_debug(); + let (running, cancel, debug) = self.state.get_running_cancel_debug(); // Check if we should continue sending signals // Exit if not running OR if neither cancel nor debug_interrupt is set @@ -194,70 +239,27 @@ impl InterruptHandleImpl for LinuxInterruptHandle { .store(unsafe { libc::pthread_self() as u64 }, Ordering::Release); } - fn set_running(&self) { - // Release ordering to ensure that the tid store (which uses Release) - // is visible to any thread that observes running=true via Acquire ordering. - // This prevents the interrupt thread from reading a stale tid value. - self.state.fetch_or(Self::RUNNING_BIT, Ordering::Release); - } - - fn is_cancelled(&self) -> bool { - // Acquire ordering to synchronize with the Release in kill() - // This ensures we see the cancel flag set by the interrupt thread - self.state.load(Ordering::Acquire) & Self::CANCEL_BIT != 0 - } - - fn clear_cancel(&self) { - // Release ordering to ensure that any operations from the previous run() - // are visible to other threads. While this is typically called by the vcpu thread - // at the start of run(), the VM itself can move between threads across guest calls. - self.state.fetch_and(!Self::CANCEL_BIT, Ordering::Release); - } - - fn clear_running(&self) { - // Release ordering to ensure all vcpu operations are visible before clearing running - self.state.fetch_and(!Self::RUNNING_BIT, Ordering::Release); - } - - fn is_debug_interrupted(&self) -> bool { - #[cfg(gdb)] - { - self.state.load(Ordering::Acquire) & Self::DEBUG_INTERRUPT_BIT != 0 - } - #[cfg(not(gdb))] - { - false - } - } - - #[cfg(gdb)] - fn clear_debug_interrupt(&self) { - self.state - .fetch_and(!Self::DEBUG_INTERRUPT_BIT, Ordering::Release); - } - fn set_dropped(&self) { // Release ordering to ensure all VM cleanup operations are visible // to any thread that checks dropped() via Acquire self.dropped.store(true, Ordering::Release); } + + fn state(&self) -> &InterruptHandleStateMachine { + &self.state + } } #[cfg(any(kvm, mshv3))] impl InterruptHandle for LinuxInterruptHandle { fn kill(&self) -> bool { - // Release ordering ensures that any writes before kill() are visible to the vcpu thread - // when it checks is_cancelled() with Acquire ordering - self.state.fetch_or(Self::CANCEL_BIT, Ordering::Release); - - // Send signals to interrupt the vcpu if it's currently running + self.state.set_cancel(); self.send_signal() } #[cfg(gdb)] fn kill_from_debugger(&self) -> bool { - self.state - .fetch_or(Self::DEBUG_INTERRUPT_BIT, Ordering::Release); + self.state.set_debug_interrupt(); self.send_signal() } fn dropped(&self) -> bool { @@ -269,101 +271,37 @@ impl InterruptHandle for LinuxInterruptHandle { #[cfg(target_os = "windows")] #[derive(Debug)] -pub(super) struct WindowsInterruptHandle { - /// Atomic value packing vcpu execution state. - /// - /// Bit layout: - /// - Bit 2: DEBUG_INTERRUPT_BIT - set when debugger interrupt is requested - /// - Bit 1: RUNNING_BIT - set when vcpu is actively running - /// - Bit 0: CANCEL_BIT - set when cancellation has been requested - /// - /// `WHvCancelRunVirtualProcessor()` will return Ok even if the vcpu is not running, - /// which is why we need the RUNNING_BIT. - /// - /// CANCEL_BIT persists across vcpu exits/re-entries within a single `VirtualCPU::run()` call - /// (e.g., during host function calls), but is cleared at the start of each new `VirtualCPU::run()` call. - state: AtomicU8, - +/// An interrupt handle that captures the pattern that requests to +/// cancel need to be mutually exclusive with partition destruction +#[allow(private_bounds)] +pub(super) struct SynchronousInterruptHandle { + state: InterruptHandleStateMachine, /// RwLock protecting the partition handle and dropped state. /// - /// This lock prevents a race condition between `kill()` calling `WHvCancelRunVirtualProcessor` - /// and `WhpVm::drop()` calling `WHvDeletePartition`. These two Windows Hypervisor Platform APIs - /// must not execute concurrently - if `WHvDeletePartition` frees the partition while - /// `WHvCancelRunVirtualProcessor` is still accessing it, the result is a use-after-free - /// causing STATUS_ACCESS_VIOLATION or STATUS_HEAP_CORRUPTION. + /// Fox example, on Windows, this lock prevents a race condition + /// between `kill()` calling `WHvCancelRunVirtualProcessor` and + /// `WhpVm::drop()` calling `WHvDeletePartition`. These two + /// Windows Hypervisor Platform APIs must not execute concurrently + /// - if `WHvDeletePartition` frees the partition while + /// `WHvCancelRunVirtualProcessor` is still accessing it, the + /// result is a use-after-free causing STATUS_ACCESS_VIOLATION or + /// STATUS_HEAP_CORRUPTION. /// /// The synchronization works as follows: /// - `kill()` takes a read lock before calling `WHvCancelRunVirtualProcessor` /// - `set_dropped()` takes a write lock, which blocks until all in-flight `kill()` calls complete, /// then sets `dropped = true`. This is called from `HyperlightVm::drop()` before `WhpVm::drop()` /// runs, ensuring no `kill()` is accessing the partition when `WHvDeletePartition` is called. - partition_state: std::sync::RwLock, + dropped_state: std::sync::RwLock<(bool, T)>, } - -/// State protected by the RwLock in `WindowsInterruptHandle`. -/// -/// Contains a copy of the partition handle from `WhpVm` (not an owning reference). -/// The RwLock and `dropped` flag ensure this handle is never used after `WhpVm` -/// deletes the partition. -#[cfg(target_os = "windows")] -#[derive(Debug)] -pub(super) struct PartitionState { - /// Copy of partition handle from `WhpVm`. Only valid while `dropped` is false. - pub(super) handle: windows::Win32::System::Hypervisor::WHV_PARTITION_HANDLE, - /// Set true before partition deletion; prevents further use of `handle`. - pub(super) dropped: bool, +trait SynchronousInterruptState: Debug + Send + Sync { + /// The inside-the-lock part of the common part of both kill() + /// and kill_from_debugger() + fn actually_cancel(&self) -> bool; } #[cfg(target_os = "windows")] -impl WindowsInterruptHandle { - const RUNNING_BIT: u8 = 1 << 1; - const CANCEL_BIT: u8 = 1 << 0; - #[cfg(gdb)] - const DEBUG_INTERRUPT_BIT: u8 = 1 << 2; -} - -#[cfg(target_os = "windows")] -impl InterruptHandleImpl for WindowsInterruptHandle { - fn set_running(&self) { - // Release ordering to ensure prior memory operations are visible when another thread observes running=true - self.state.fetch_or(Self::RUNNING_BIT, Ordering::Release); - } - - fn is_cancelled(&self) -> bool { - // Acquire ordering to synchronize with the Release in kill() - // This ensures we see the CANCEL_BIT set by the interrupt thread - self.state.load(Ordering::Acquire) & Self::CANCEL_BIT != 0 - } - - fn clear_cancel(&self) { - // Release ordering to ensure that any operations from the previous run() - // are visible to other threads. While this is typically called by the vcpu thread - // at the start of run(), the VM itself can move between threads across guest calls. - self.state.fetch_and(!Self::CANCEL_BIT, Ordering::Release); - } - - fn clear_running(&self) { - // Release ordering to ensure all vcpu operations are visible before clearing running - self.state.fetch_and(!Self::RUNNING_BIT, Ordering::Release); - } - - fn is_debug_interrupted(&self) -> bool { - #[cfg(gdb)] - { - self.state.load(Ordering::Acquire) & Self::DEBUG_INTERRUPT_BIT != 0 - } - #[cfg(not(gdb))] - { - false - } - } - - #[cfg(gdb)] - fn clear_debug_interrupt(&self) { - self.state - .fetch_and(!Self::DEBUG_INTERRUPT_BIT, Ordering::Release); - } - +impl InterruptHandleImpl for SynchronousInterruptHandle { fn set_dropped(&self) { // Take write lock to: // 1. Wait for any in-flight kill() calls (holding read locks) to complete @@ -371,37 +309,31 @@ impl InterruptHandleImpl for WindowsInterruptHandle { // 3. Set dropped=true so no future kill() calls will use the handle // After this returns, no WHvCancelRunVirtualProcessor calls are in progress // or will ever be made, so WHvDeletePartition can safely be called. - match self.partition_state.write() { + match self.dropped_state.write() { Ok(mut guard) => { - guard.dropped = true; + guard.0 = true; } Err(e) => { tracing::error!("Failed to acquire partition_state write lock: {}", e); } } } -} -#[cfg(target_os = "windows")] -impl InterruptHandle for WindowsInterruptHandle { - fn kill(&self) -> bool { - use windows::Win32::System::Hypervisor::WHvCancelRunVirtualProcessor; - - // Release ordering ensures that any writes before kill() are visible to the vcpu thread - // when it checks is_cancelled() with Acquire ordering - self.state.fetch_or(Self::CANCEL_BIT, Ordering::Release); - - // Acquire ordering to synchronize with the Release in set_running() - // This ensures we see the running state set by the vcpu thread - let state = self.state.load(Ordering::Acquire); - if state & Self::RUNNING_BIT == 0 { + fn state(&self) -> &InterruptHandleStateMachine { + &self.state + } +} +#[allow(private_bounds)] +impl SynchronousInterruptHandle { + fn lock_and_actually_cancel(&self) -> bool { + if !self.state.get_running_cancel_debug().0 { return false; } // Take read lock to prevent race with WHvDeletePartition in set_dropped(). // Multiple kill() calls can proceed concurrently (read locks don't block each other), // but set_dropped() will wait for all kill() calls to complete before proceeding. - let guard = match self.partition_state.read() { + let guard = match self.dropped_state.read() { Ok(guard) => guard, Err(e) => { tracing::error!("Failed to acquire partition_state read lock: {}", e); @@ -409,45 +341,29 @@ impl InterruptHandle for WindowsInterruptHandle { } }; - if guard.dropped { + if guard.0 { return false; } - unsafe { WHvCancelRunVirtualProcessor(guard.handle, 0, 0).is_ok() } + guard.1.actually_cancel() + } +} + +#[cfg(any(target_os = "windows", hvf))] +impl InterruptHandle for SynchronousInterruptHandle { + fn kill(&self) -> bool { + self.state.set_cancel(); + self.lock_and_actually_cancel() } #[cfg(gdb)] fn kill_from_debugger(&self) -> bool { - use windows::Win32::System::Hypervisor::WHvCancelRunVirtualProcessor; - - self.state - .fetch_or(Self::DEBUG_INTERRUPT_BIT, Ordering::Release); - - // Acquire ordering to synchronize with the Release in set_running() - let state = self.state.load(Ordering::Acquire); - if state & Self::RUNNING_BIT == 0 { - return false; - } - - // Take read lock to prevent race with WHvDeletePartition in set_dropped() - let guard = match self.partition_state.read() { - Ok(guard) => guard, - Err(e) => { - tracing::error!("Failed to acquire partition_state read lock: {}", e); - return false; - } - }; - - if guard.dropped { - return false; - } - - unsafe { WHvCancelRunVirtualProcessor(guard.handle, 0, 0).is_ok() } + self.state.set_debug_interrupt(); + self.lock_and_actually_cancel() } - fn dropped(&self) -> bool { // Take read lock to check dropped state consistently - match self.partition_state.read() { - Ok(guard) => guard.dropped, + match self.dropped_state.read() { + Ok(guard) => guard.0, Err(e) => { tracing::error!("Failed to acquire partition_state read lock: {}", e); true // Assume dropped if we can't acquire lock @@ -456,6 +372,31 @@ impl InterruptHandle for WindowsInterruptHandle { } } +#[cfg(target_os = "windows")] +use windows::Win32::System::Hypervisor::WHV_PARTITION_HANDLE; +#[cfg(target_os = "windows")] +pub(super) type WindowsInterruptHandle = SynchronousInterruptHandle; +#[cfg(target_os = "windows")] +impl WindowsInterruptHandle { + fn new(hdl: WHV_PARTITION_HANDLE) -> Self { + SynchronousInterruptHandle { + state: InterruptHandleStateMachine::new(), + dropped_state: std::sync::RwLock::new((false, hdl)), + } + } +} +#[cfg(target_os = "windows")] +impl SynchronousInterruptState for WHV_PARTITION_HANDLE { + fn actually_cancel(&self) -> bool { + use windows::Win32::System::Hypervisor::WHvCancelRunVirtualProcessor; + unsafe { WHvCancelRunVirtualProcessor(*self, 0, 0).is_ok() } + } +} + + } + } +} + #[cfg(all(test, any(target_os = "windows", kvm)))] pub(crate) mod tests { use std::sync::{Arc, Mutex}; From 436292f7fd6d4a6eb749ccc90bdfee3259661f2a Mon Sep 17 00:00:00 2001 From: Lucy Menon <168595099+syntactically@users.noreply.github.com> Date: Mon, 20 Jul 2026 15:29:09 +0100 Subject: [PATCH 03/17] Abstract out interrupt handle retry logic Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- .../src/hypervisor/hyperlight_vm/aarch64.rs | 11 +- .../src/hypervisor/hyperlight_vm/x86_64.rs | 22 +-- src/hyperlight_host/src/hypervisor/mod.rs | 187 ++++++++++-------- 3 files changed, 111 insertions(+), 109 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs index 6ac13815a..1c4ac4c6c 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs @@ -17,7 +17,6 @@ limitations under the License. // TODO(aarch64): implement arch-specific HyperlightVm methods use std::sync::Arc; -use std::sync::atomic::{AtomicBool, AtomicU8, AtomicU64}; use super::{ AccessPageTableError, CreateHyperlightVmError, DispatchGuestCallError, HyperlightVm, @@ -71,13 +70,9 @@ impl HyperlightVm { }; vm.set_sregs(&CommonSpecialRegisters::defaults(root_pt_addr)) .map_err(VmError::Register)?; - let interrupt_handle: Arc = Arc::new(LinuxInterruptHandle { - state: AtomicU8::new(0), - tid: AtomicU64::new(unsafe { libc::pthread_self() as u64 }), - retry_delay: config.get_interrupt_retry_delay(), - sig_rt_min_offset: config.get_interrupt_vcpu_sigrtmin_offset(), - dropped: AtomicBool::new(false), - }); + #[cfg(any(kvm, mshv3))] + let interrupt_handle: Arc = + Arc::new(LinuxInterruptHandle::new(config)); let snapshot_slot = 0u32; let scratch_slot = 1u32; diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs index 6cd3052ea..588ab7a80 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs @@ -103,26 +103,8 @@ impl HyperlightVm { .map_err(VmError::Register)?; #[cfg(any(kvm, mshv3))] - let interrupt_handle: Arc = Arc::new(LinuxInterruptHandle { - state: AtomicU8::new(0), - #[cfg(all( - target_arch = "x86_64", - target_vendor = "unknown", - target_os = "linux", - target_env = "musl" - ))] - tid: AtomicU64::new(unsafe { libc::pthread_self() as u64 }), - #[cfg(not(all( - target_arch = "x86_64", - target_vendor = "unknown", - target_os = "linux", - target_env = "musl" - )))] - tid: AtomicU64::new(unsafe { libc::pthread_self() }), - retry_delay: config.get_interrupt_retry_delay(), - sig_rt_min_offset: config.get_interrupt_vcpu_sigrtmin_offset(), - dropped: AtomicBool::new(false), - }); + let interrupt_handle: Arc = + Arc::new(LinuxInterruptHandle::new(config)); #[cfg(target_os = "windows")] let interrupt_handle: Arc = diff --git a/src/hyperlight_host/src/hypervisor/mod.rs b/src/hyperlight_host/src/hypervisor/mod.rs index bcda61b08..397921a0d 100644 --- a/src/hyperlight_host/src/hypervisor/mod.rs +++ b/src/hyperlight_host/src/hypervisor/mod.rs @@ -138,14 +138,20 @@ pub(crate) trait InterruptHandleImpl: InterruptHandle { /// Mark the handle as dropped fn set_dropped(&self); +} +pub(crate) trait InterruptHandleInternal { /// Local access the shared state, which does not perform any /// operations other than updating the state machine fn state(&self) -> &InterruptHandleStateMachine; + /// Trigger the actual kill-like operation without + /// modifying the state + fn common_kill(&self) -> bool; } /// A trait for handling interrupts to a sandbox's vcpu -pub trait InterruptHandle: Send + Sync + Debug { +#[allow(private_bounds)] +pub trait InterruptHandle: Send + Sync + Debug + InterruptHandleInternal { /// Interrupt the corresponding sandbox from running. /// /// - If this is called while the the sandbox currently executing a guest function call, it will interrupt the sandbox and return `true`. @@ -153,7 +159,10 @@ pub trait InterruptHandle: Send + Sync + Debug { /// /// # Note /// This function will block for the duration of the time it takes for the vcpu thread to be interrupted. - fn kill(&self) -> bool; + fn kill(&self) -> bool { + self.state().set_cancel(); + self.common_kill() + } /// Used by a debugger to interrupt the corresponding sandbox from running. /// @@ -165,15 +174,63 @@ pub trait InterruptHandle: Send + Sync + Debug { /// # Note /// This function will block for the duration of the time it takes for the vcpu thread to be interrupted. #[cfg(gdb)] - fn kill_from_debugger(&self) -> bool; + fn kill_from_debugger(&self) -> bool { + self.state().set_debug_interrupt(); + self.common_kill() + } /// Returns true if the corresponding sandbox has been dropped fn dropped(&self) -> bool; } +#[cfg(any(kvm, mshv3, hvf))] +#[derive(Debug)] +pub(super) struct RetryingInterruptHandle { + retry_delay: Duration, + inner: T, +} + +impl InterruptHandleImpl for RetryingInterruptHandle { + #[cfg(any(kvm, mshv3))] + fn set_tid(&self) { + self.inner.set_tid(); + } + + + fn set_dropped(&self) { + self.inner.set_dropped(); + } +} +impl InterruptHandle for RetryingInterruptHandle { + fn dropped(&self) -> bool { + self.inner.dropped() + } +} +impl InterruptHandleInternal for RetryingInterruptHandle { + fn state(&self) -> &InterruptHandleStateMachine { + self.inner.state() + } + fn common_kill(&self) -> bool { + let mut succeeded = false; + loop { + let (running, cancel, debug) = self.state().get_running_cancel_debug(); + // Check if we should continue sending signals + // Exit if not running OR if neither cancel nor debug_interrupt is set + let should_continue = running && (cancel || debug); + if !should_continue { + break; + } + tracing::info!("Trying to kill vcpu thread..."); + succeeded |= self.inner.common_kill(); + std::thread::sleep(self.retry_delay); + } + succeeded + } +} + #[cfg(any(kvm, mshv3))] #[derive(Debug)] -pub(super) struct LinuxInterruptHandle { +pub(super) struct LinuxInterruptHandleState { state: InterruptHandleStateMachine, /// Thread ID where the vcpu is running. @@ -185,52 +242,29 @@ pub(super) struct LinuxInterruptHandle { /// Whether the corresponding VM has been dropped. dropped: AtomicBool, - /// Delay between retry attempts when sending signals to interrupt the vcpu. - retry_delay: Duration, - /// Offset from SIGRTMIN for the signal used to interrupt the vcpu thread. sig_rt_min_offset: u8, } +#[cfg(any(kvm, mshv3))] +pub(super) type LinuxInterruptHandle = RetryingInterruptHandle; #[cfg(any(kvm, mshv3))] impl LinuxInterruptHandle { - /// Get the running, cancel and debug flags atomically. - /// - /// # Memory Ordering - /// Uses `Acquire` ordering to synchronize with the `Release` in `set_running()` and `kill()`. - /// This ensures that when we observe running=true, we also see the correct `tid` value. - - fn send_signal(&self) -> bool { - let signal_number = libc::SIGRTMIN() + self.sig_rt_min_offset as libc::c_int; - let mut sent_signal = false; - - loop { - let (running, cancel, debug) = self.state.get_running_cancel_debug(); - - // Check if we should continue sending signals - // Exit if not running OR if neither cancel nor debug_interrupt is set - let should_continue = running && (cancel || debug); - - if !should_continue { - break; - } - - tracing::info!("Sending signal to kill vcpu thread..."); - sent_signal = true; - // Acquire ordering to synchronize with the Release store in set_tid() - // This ensures we see the correct tid value for the currently running vcpu - unsafe { - libc::pthread_kill(self.tid.load(Ordering::Acquire) as _, signal_number); - } - std::thread::sleep(self.retry_delay); + fn new(config: &crate::sandbox::SandboxConfiguration) -> Self { + RetryingInterruptHandle { + retry_delay: config.get_interrupt_retry_delay(), + inner: LinuxInterruptHandleState { + state: InterruptHandleStateMachine::new(), + tid: AtomicU64::new(unsafe { libc::pthread_self() as u64 }), + sig_rt_min_offset: config.get_interrupt_vcpu_sigrtmin_offset(), + dropped: AtomicBool::new(false), + }, } - - sent_signal } } #[cfg(any(kvm, mshv3))] -impl InterruptHandleImpl for LinuxInterruptHandle { +impl InterruptHandleImpl for LinuxInterruptHandleState { fn set_tid(&self) { // Release ordering to synchronize with the Acquire load of `running` in send_signal() // This ensures that when send_signal() observes RUNNING_BIT=true (via Acquire), @@ -244,24 +278,10 @@ impl InterruptHandleImpl for LinuxInterruptHandle { // to any thread that checks dropped() via Acquire self.dropped.store(true, Ordering::Release); } - - fn state(&self) -> &InterruptHandleStateMachine { - &self.state - } } #[cfg(any(kvm, mshv3))] -impl InterruptHandle for LinuxInterruptHandle { - fn kill(&self) -> bool { - self.state.set_cancel(); - self.send_signal() - } - - #[cfg(gdb)] - fn kill_from_debugger(&self) -> bool { - self.state.set_debug_interrupt(); - self.send_signal() - } +impl InterruptHandle for LinuxInterruptHandleState { fn dropped(&self) -> bool { // Acquire ordering to synchronize with the Release in set_dropped() // This ensures we see all VM cleanup operations that happened before drop @@ -269,6 +289,20 @@ impl InterruptHandle for LinuxInterruptHandle { } } +#[cfg(any(kvm, mshv3))] +impl InterruptHandleInternal for LinuxInterruptHandleState { + fn state(&self) -> &InterruptHandleStateMachine { + &self.state + } + fn common_kill(&self) -> bool { + let signal_number = libc::SIGRTMIN() + self.sig_rt_min_offset as libc::c_int; + unsafe { + libc::pthread_kill(self.tid.load(Ordering::Acquire) as _, signal_number); + } + true + } +} + #[cfg(target_os = "windows")] #[derive(Debug)] /// An interrupt handle that captures the pattern that requests to @@ -294,6 +328,7 @@ pub(super) struct SynchronousInterruptHandle { /// runs, ensuring no `kill()` is accessing the partition when `WHvDeletePartition` is called. dropped_state: std::sync::RwLock<(bool, T)>, } +#[cfg(any(target_os = "windows", hvf))] trait SynchronousInterruptState: Debug + Send + Sync { /// The inside-the-lock part of the common part of both kill() /// and kill_from_debugger() @@ -318,14 +353,27 @@ impl InterruptHandleImpl for SynchronousInterruptH } } } +} +#[cfg(any(target_os = "windows", hvf))] +impl InterruptHandle for SynchronousInterruptHandle { + fn dropped(&self) -> bool { + // Take read lock to check dropped state consistently + match self.dropped_state.read() { + Ok(guard) => guard.0, + Err(e) => { + tracing::error!("Failed to acquire partition_state read lock: {}", e); + true // Assume dropped if we can't acquire lock + } + } + } +} +#[cfg(any(target_os = "windows", hvf))] +impl InterruptHandleInternal for SynchronousInterruptHandle { fn state(&self) -> &InterruptHandleStateMachine { &self.state } -} -#[allow(private_bounds)] -impl SynchronousInterruptHandle { - fn lock_and_actually_cancel(&self) -> bool { + fn common_kill(&self) -> bool { if !self.state.get_running_cancel_debug().0 { return false; } @@ -349,29 +397,6 @@ impl SynchronousInterruptHandle { } } -#[cfg(any(target_os = "windows", hvf))] -impl InterruptHandle for SynchronousInterruptHandle { - fn kill(&self) -> bool { - self.state.set_cancel(); - self.lock_and_actually_cancel() - } - #[cfg(gdb)] - fn kill_from_debugger(&self) -> bool { - self.state.set_debug_interrupt(); - self.lock_and_actually_cancel() - } - fn dropped(&self) -> bool { - // Take read lock to check dropped state consistently - match self.dropped_state.read() { - Ok(guard) => guard.0, - Err(e) => { - tracing::error!("Failed to acquire partition_state read lock: {}", e); - true // Assume dropped if we can't acquire lock - } - } - } -} - #[cfg(target_os = "windows")] use windows::Win32::System::Hypervisor::WHV_PARTITION_HANDLE; #[cfg(target_os = "windows")] From 23ab9a983df046c5e956a5f374ba8e36910b3924 Mon Sep 17 00:00:00 2001 From: Lucy Menon <168595099+syntactically@users.noreply.github.com> Date: Mon, 6 Jul 2026 15:31:03 +0100 Subject: [PATCH 04/17] Be more careful about cfg(target_os = "linux") vs cfg(unix) This commit moves a bunch of code (primarily mmap-based implementations of things) from cfg(target_os = "linux") to cfg(unix), allowing it to compile on MacOS (where it will be useful). It also moves the kvm and mshv package dependencies from cfg(unix) to cfg(target_os = "linux"), so that Cargo does not try (and fail) to build them on MacOS. Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- src/hyperlight_host/Cargo.toml | 2 +- src/hyperlight_host/src/mem/shared_mem.rs | 10 +++++----- src/hyperlight_host/src/sandbox/file_mapping.rs | 6 +++--- .../src/sandbox/snapshot/file/config.rs | 2 +- src/hyperlight_host/src/sandbox/uninitialized.rs | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/hyperlight_host/Cargo.toml b/src/hyperlight_host/Cargo.toml index 60be1555d..4eb44c799 100644 --- a/src/hyperlight_host/Cargo.toml +++ b/src/hyperlight_host/Cargo.toml @@ -78,7 +78,7 @@ rust-embed = { version = "8.11.0", features = ["debug-embed", "include-exclude", windows-version = "0.1" lazy_static = "1.4.0" -[target.'cfg(unix)'.dependencies] +[target.'cfg(target_os = "linux")'.dependencies] kvm-bindings = { version = "0.14", features = ["fam-wrappers"], optional = true } kvm-ioctls = { version = "0.25", optional = true } mshv-bindings = { version = "0.6", optional = true } diff --git a/src/hyperlight_host/src/mem/shared_mem.rs b/src/hyperlight_host/src/mem/shared_mem.rs index 4b706cc41..c1b29ede5 100644 --- a/src/hyperlight_host/src/mem/shared_mem.rs +++ b/src/hyperlight_host/src/mem/shared_mem.rs @@ -18,7 +18,7 @@ use std::any::type_name; use std::ffi::c_void; use std::io::Error; use std::mem::{align_of, size_of}; -#[cfg(target_os = "linux")] +#[cfg(unix)] use std::ptr::null_mut; use std::sync::{Arc, RwLock}; @@ -178,14 +178,14 @@ impl HostMapping { } /// RAII guard for an `mmap` reservation. Calls `munmap` on drop. -#[cfg(target_os = "linux")] +#[cfg(unix)] #[derive(Debug)] struct Mmap { base: *mut c_void, len: usize, } -#[cfg(target_os = "linux")] +#[cfg(unix)] impl Drop for Mmap { fn drop(&mut self) { // SAFETY: `self.base` and `self.len` are exactly what was @@ -537,7 +537,7 @@ impl ExclusiveSharedMemory { /// size in bytes. The region will be surrounded by guard pages. /// /// Return `Err` if shared memory could not be allocated. - #[cfg(target_os = "linux")] + #[cfg(unix)] #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub fn new(min_size_bytes: usize) -> Result { use libc::{ @@ -1563,7 +1563,7 @@ impl ReadonlySharedMemory { /// Linux: reserve `[guard][blob][guard]` as one anonymous /// `PROT_NONE` mapping, then `MAP_FIXED` the file over the /// middle slot. - #[cfg(target_os = "linux")] + #[cfg(unix)] fn map_file(file: &std::fs::File, len: usize) -> Result> { use std::os::unix::io::AsRawFd; diff --git a/src/hyperlight_host/src/sandbox/file_mapping.rs b/src/hyperlight_host/src/sandbox/file_mapping.rs index 4f3ed2a4d..58f1dce6f 100644 --- a/src/hyperlight_host/src/sandbox/file_mapping.rs +++ b/src/hyperlight_host/src/sandbox/file_mapping.rs @@ -71,7 +71,7 @@ pub(crate) enum HostFileResources { view_base: *mut c_void, }, /// Linux: `mmap` base pointer. - #[cfg(target_os = "linux")] + #[cfg(unix)] Linux { mmap_base: *mut c_void, mmap_size: usize, @@ -103,7 +103,7 @@ impl Drop for PreparedFileMapping { tracing::error!("PreparedFileMapping::drop: CloseHandle failed: {:?}", e); } }, - #[cfg(target_os = "linux")] + #[cfg(unix)] HostFileResources::Linux { mmap_base, mmap_size, @@ -168,7 +168,7 @@ impl PreparedFileMapping { region_type: MemoryRegionType::MappedFile, }) } - #[cfg(target_os = "linux")] + #[cfg(unix)] HostFileResources::Linux { mmap_base, mmap_size, diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs index 01c2f501b..d813a6bfb 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs @@ -125,7 +125,7 @@ impl CpuVendor { bytes[8..12].copy_from_slice(&r.ecx.to_le_bytes()); Self(String::from_utf8_lossy(&bytes).into_owned()) } - #[cfg(all(target_arch = "aarch64", target_os = "linux"))] + #[cfg(all(target_arch = "aarch64"))] { let midr: u64; // SAFETY: Linux emulates MIDR_EL1 reads from EL0. diff --git a/src/hyperlight_host/src/sandbox/uninitialized.rs b/src/hyperlight_host/src/sandbox/uninitialized.rs index 59f89cbac..d32982100 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized.rs @@ -416,7 +416,7 @@ mod tests { use crate::sandbox::uninitialized::{GuestBinary, GuestEnvironment}; use crate::{MultiUseSandbox, Result, UninitializedSandbox, new_error}; - #[cfg(unix)] + #[cfg(target_os = "linux")] #[test] fn guest_binary_loads_from_non_utf8_path() { use std::ffi::OsString; From cb28c4bb7850e8d286d21ab482a7e7ed9a1f2b04 Mon Sep 17 00:00:00 2001 From: Lucy Menon <168595099+syntactically@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:15:07 +0100 Subject: [PATCH 05/17] hvf: add cfg items and detection stub Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- src/hyperlight_host/Cargo.toml | 3 ++- src/hyperlight_host/build.rs | 1 + .../src/hypervisor/hyperlight_vm/aarch64.rs | 12 ++++++++---- .../src/hypervisor/virtual_machine/hvf/mod.rs | 19 +++++++++++++++++++ .../src/hypervisor/virtual_machine/mod.rs | 13 +++++++++++++ .../src/sandbox/snapshot/file/config.rs | 5 +++++ .../tests/snapshot_goldens/platform.rs | 13 ++++++++++--- 7 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 src/hyperlight_host/src/hypervisor/virtual_machine/hvf/mod.rs diff --git a/src/hyperlight_host/Cargo.toml b/src/hyperlight_host/Cargo.toml index 4eb44c799..5a825f972 100644 --- a/src/hyperlight_host/Cargo.toml +++ b/src/hyperlight_host/Cargo.toml @@ -125,7 +125,7 @@ cfg_aliases = "0.2.1" built = { version = "0.8.1", optional = true, features = ["chrono", "git2"] } [features] -default = ["kvm", "mshv3", "build-metadata"] +default = ["kvm", "mshv3", "hvf", "build-metadata"] function_call_metrics = [] executable_heap = [] # This feature enables printing of debug information to stdout in debug builds @@ -136,6 +136,7 @@ trace_guest = ["dep:opentelemetry", "dep:tracing-opentelemetry", "dep:hyperlight mem_profile = [ "trace_guest", "dep:framehop", "dep:fallible-iterator", "hyperlight-common/mem_profile" ] kvm = ["dep:kvm-bindings", "dep:kvm-ioctls"] mshv3 = ["dep:mshv-bindings", "dep:mshv-ioctls"] +hvf = [] hw-interrupts = [] # This enables easy debug in the guest gdb = ["dep:gdbstub", "dep:gdbstub_arch"] diff --git a/src/hyperlight_host/build.rs b/src/hyperlight_host/build.rs index 57a06e4bb..ac153419e 100644 --- a/src/hyperlight_host/build.rs +++ b/src/hyperlight_host/build.rs @@ -108,6 +108,7 @@ fn main() -> Result<()> { gdb: { all(feature = "gdb", debug_assertions, target_arch = "x86_64") }, kvm: { all(feature = "kvm", target_os = "linux") }, mshv3: { all(feature = "mshv3", target_os = "linux") }, + hvf: { all(feature = "hvf", target_os = "macos") }, crashdump: { all(feature = "crashdump", target_arch = "x86_64") }, // print_debug feature is aliased with debug_assertions to make it only available in debug-builds. print_debug: { all(feature = "print_debug", debug_assertions) }, diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs index 1c4ac4c6c..e204cb05f 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs @@ -22,18 +22,19 @@ use super::{ AccessPageTableError, CreateHyperlightVmError, DispatchGuestCallError, HyperlightVm, InitializeError, }; +use crate::hypervisor::InterruptHandleImpl; +#[cfg(any(kvm, mshv3))] +use crate::hypervisor::LinuxInterruptHandle; #[cfg(gdb)] use crate::hypervisor::gdb::{DebugCommChannel, DebugMsg, DebugResponse}; use crate::hypervisor::hyperlight_vm::get_guest_log_filter; use crate::hypervisor::regs::{CommonFpu, CommonRegisters, CommonSpecialRegisters}; #[cfg(kvm)] use crate::hypervisor::virtual_machine::kvm::KvmVm; -#[cfg(kvm)] -use crate::hypervisor::virtual_machine::{HypervisorType, VmError}; use crate::hypervisor::virtual_machine::{ - RegisterError, ResetVcpuError, VirtualMachine, get_available_hypervisor, + HypervisorType, RegisterError, ResetVcpuError, VirtualMachine, VmError, + get_available_hypervisor, }; -use crate::hypervisor::{InterruptHandleImpl, LinuxInterruptHandle}; use crate::mem::mgr::{SandboxMemoryManager, SnapshotSharedMemory}; use crate::mem::shared_mem::{GuestSharedMemory, HostSharedMemory}; use crate::sandbox::SandboxConfiguration; @@ -46,6 +47,7 @@ use crate::sandbox::uninitialized::SandboxRuntimeConfig; impl HyperlightVm { #[allow(clippy::too_many_arguments)] + #[cfg_attr(target_os = "macos", allow(unused))] pub(crate) fn new( snapshot_mem: SnapshotSharedMemory, scratch_mem: GuestSharedMemory, @@ -66,6 +68,8 @@ impl HyperlightVm { // TODO: mshv support #[cfg(mshv3)] Some(HypervisorType::Mshv) => return Err(CreateHyperlightVmError::NoHypervisorFound), + #[cfg(hvf)] + Some(HypervisorType::Hvf) => return Err(CreateHyperlightVmError::NoHypervisorFound), None => return Err(CreateHyperlightVmError::NoHypervisorFound), }; vm.set_sregs(&CommonSpecialRegisters::defaults(root_pt_addr)) diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/hvf/mod.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/hvf/mod.rs new file mode 100644 index 000000000..f88f4981b --- /dev/null +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/hvf/mod.rs @@ -0,0 +1,19 @@ +/* +Copyright 2025 The Hyperlight Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +pub(super) fn is_hypervisor_present() -> bool { + false +} diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs index d0c613e04..a673c02f9 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs @@ -34,6 +34,9 @@ use crate::mem::memory_region::MemoryRegion; #[cfg(feature = "trace_guest")] use crate::sandbox::trace::TraceContext as SandboxTraceContext; +/// Hypervisor.framework functionality (MacOS) +#[cfg(hvf)] +pub(crate) mod hvf; /// KVM (Kernel-based Virtual Machine) functionality (linux) #[cfg(kvm)] pub(crate) mod kvm; @@ -83,6 +86,12 @@ pub fn get_available_hypervisor() -> &'static Option { } else { None } + } else if #[cfg(hvf)] { + if hvf::is_hypervisor_present() { + Some(HypervisorType::Hvf) + } else { + None + } } else { None } @@ -108,6 +117,9 @@ pub(crate) enum HypervisorType { #[cfg(target_os = "windows")] Whp, + + #[cfg(hvf)] + Hvf, } /// Minimum XSAVE buffer size: 512 bytes legacy region + 64 bytes header. @@ -132,6 +144,7 @@ compile_error!( ); /// The various reasons a VM's vCPU can exit +#[cfg_attr(target_os = "macos", allow(unused))] pub(crate) enum VmExit { /// The vCPU has exited due to a debug event (usually breakpoint) #[cfg(gdb)] diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs index d813a6bfb..ca60c1963 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs @@ -66,6 +66,7 @@ pub(super) enum Hypervisor { Kvm, Mshv, Whp, + Hvf, } impl Hypervisor { @@ -81,6 +82,8 @@ impl Hypervisor { Some(HypervisorType::Mshv) => Some(Self::Mshv), #[cfg(target_os = "windows")] Some(HypervisorType::Whp) => Some(Self::Whp), + #[cfg(hvf)] + Some(HypervisorType::Hvf) => Some(Self::Hvf), None => None, } } @@ -90,6 +93,7 @@ impl Hypervisor { Self::Kvm => "KVM", Self::Mshv => "MSHV", Self::Whp => "WHP", + Self::Hvf => "HVF", } } @@ -101,6 +105,7 @@ impl Hypervisor { Self::Kvm => "kvm", Self::Mshv => "mshv", Self::Whp => "whp", + Self::Hvf => "hvf", } } } diff --git a/src/hyperlight_host/tests/snapshot_goldens/platform.rs b/src/hyperlight_host/tests/snapshot_goldens/platform.rs index 54be335c9..bb274538f 100644 --- a/src/hyperlight_host/tests/snapshot_goldens/platform.rs +++ b/src/hyperlight_host/tests/snapshot_goldens/platform.rs @@ -26,12 +26,14 @@ use crate::goldens_version::GOLDENS_VERSION; #[derive(Copy, Clone, Debug, PartialEq, Eq)] enum Hypervisor { - #[cfg_attr(target_os = "windows", allow(dead_code))] + #[cfg_attr(not(kvm), allow(dead_code))] Kvm, - #[cfg_attr(target_os = "windows", allow(dead_code))] + #[cfg_attr(not(mshv3), allow(dead_code))] Mshv, #[cfg_attr(not(target_os = "windows"), allow(dead_code))] Whp, + #[cfg_attr(not(hvf), allow(dead_code))] + Hvf, } impl Hypervisor { @@ -40,6 +42,7 @@ impl Hypervisor { Self::Kvm => "kvm", Self::Mshv => "mshv", Self::Whp => "whp", + Self::Hvf => "hvf", } } @@ -61,7 +64,11 @@ impl Hypervisor { { Some(Self::Whp) } - #[cfg(not(any(target_os = "linux", target_os = "windows")))] + #[cfg(hvf)] + { + Some(Self::Hvf) + } + #[cfg(not(any(target_os = "linux", target_os = "windows", hvf)))] { None } From 9344629db4b21dc07dc5958e1fda110fb8b5098a Mon Sep 17 00:00:00 2001 From: Lucy Menon <168595099+syntactically@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:22:36 +0100 Subject: [PATCH 06/17] hvf: add Rust bindings Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- Cargo.lock | 2 + src/hyperlight_host/Cargo.toml | 2 + src/hyperlight_host/build.rs | 66 +++++++++++++++++-- .../hypervisor/virtual_machine/hvf/bindings.h | 22 +++++++ .../hypervisor/virtual_machine/hvf/fp_abi.c | 36 ++++++++++ .../src/hypervisor/virtual_machine/hvf/mod.rs | 10 +++ 6 files changed, 134 insertions(+), 4 deletions(-) create mode 100644 src/hyperlight_host/src/hypervisor/virtual_machine/hvf/bindings.h create mode 100644 src/hyperlight_host/src/hypervisor/virtual_machine/hvf/fp_abi.c diff --git a/Cargo.lock b/Cargo.lock index 9def63022..015ce934b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1664,10 +1664,12 @@ name = "hyperlight-host" version = "0.16.0" dependencies = [ "anyhow", + "bindgen", "bitflags 2.13.1", "blake3", "built", "bytemuck", + "cc", "cfg-if", "cfg_aliases", "chrono", diff --git a/src/hyperlight_host/Cargo.toml b/src/hyperlight_host/Cargo.toml index 5a825f972..4073b4226 100644 --- a/src/hyperlight_host/Cargo.toml +++ b/src/hyperlight_host/Cargo.toml @@ -123,6 +123,8 @@ proc-maps = "0.5.0" anyhow = { version = "1.0.102" } cfg_aliases = "0.2.1" built = { version = "0.8.1", optional = true, features = ["chrono", "git2"] } +bindgen = { version = "0.72", features = ["prettyplease"] } +cc = "1.2" [features] default = ["kvm", "mshv3", "hvf", "build-metadata"] diff --git a/src/hyperlight_host/build.rs b/src/hyperlight_host/build.rs index ac153419e..0c2674929 100644 --- a/src/hyperlight_host/build.rs +++ b/src/hyperlight_host/build.rs @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -use anyhow::Result; +use anyhow::{Context, Result}; #[cfg(feature = "build-metadata")] use built::write_built_file; @@ -22,6 +22,9 @@ fn main() -> Result<()> { // re-run the build if this script is changed (or deleted!), // even if the rust code is completely unchanged. println!("cargo:rerun-if-changed=build.rs"); + let out_dir = std::env::var("OUT_DIR")?; + let out_path = std::path::PathBuf::from(&out_dir); + let manifest_dir = std::env::var("CARGO_MANIFEST_DIR")?; // Windows requires the hyperlight_surrogate.exe binary to be next to the executable running // hyperlight. We are using rust-embed to include the binary in the hyperlight-host library @@ -41,9 +44,7 @@ fn main() -> Result<()> { // We need to copy/rename the source for hyperlight surrogate into a // temp directory because we cannot include a file name `Cargo.toml` // inside this package. - let out_dir = std::env::var("OUT_DIR")?; std::fs::create_dir_all(format!("{out_dir}/hyperlight_surrogate/src"))?; - let manifest_dir = std::env::var("CARGO_MANIFEST_DIR")?; std::fs::copy( format!("{manifest_dir}/src/hyperlight_surrogate/src/main.rs"), format!("{out_dir}/hyperlight_surrogate/src/main.rs"), @@ -62,7 +63,7 @@ fn main() -> Result<()> { // be the same as the CARGO_TARGET_DIR for the hyperlight-host otherwise // the build script will hang. Using a sub directory works tho! // xref - https://github.com/rust-lang/cargo/issues/6412 - let target_dir = std::path::PathBuf::from(&out_dir).join("../../hls"); + let target_dir = out_path.join("../../hls"); let profile = std::env::var("PROFILE")?; let build_profile = if profile.to_lowercase() == "debug" { @@ -101,6 +102,63 @@ fn main() -> Result<()> { ); } + if std::env::var("CARGO_CFG_TARGET_OS")? == "macos" + && std::env::var("CARGO_FEATURE_HVF").is_ok() + { + println!("cargo:rerun-if-env-changed=BINDGEN_EXTRA_CLANG_ARGS"); + println!( + "cargo:rerun-if-changed=src/hyperlight_host/src/hypervisor/virtual_machine/hvf/bindings.h" + ); + println!( + "cargo:rerun-if-changed=src/hyperlight_host/src/hypervisor/virtual_machine/hvf/fp_abi.c" + ); + println!("cargo:rustc-link-lib=framework=Hypervisor"); + let sdk_path = std::process::Command::new("xcrun") + .args(["-sdk", "macosx", "-show-sdk-path"]) + .output()? + .stdout; + let sdk_path = String::from_utf8_lossy(&sdk_path); + eprintln!("sdk path: {}", sdk_path.trim()); + bindgen::Builder::default() + .clang_args(&[ + "-isysroot", + sdk_path.trim(), + "-I", + &format!("{manifest_dir}/src/hypervisor/virtual_machine/hvf"), + ]) + // The hvf simd register functions use C simd vector + // parameters/returns for the register value, which Rust + // does not presently stably support for `extern "C"` + // code. So, we don't generate native bindings for these + // functions, and instead generate C stubs (see `fp_abi.c`, + // which is built below) + .blocklist_type("hv_simd_fp_uchar16_t") + .blocklist_function("hv_vcpu_get_simd_fp_reg") + .blocklist_function("hv_vcpu_set_simd_fp_reg") + .default_alias_style(bindgen::AliasVariation::NewType) + .type_alias("hv_memory_flags_t") + .newtype_enum("hv_reg_t") + .newtype_enum("hv_simd_fp_reg_t") + .newtype_enum("hv_sys_reg_t") + .newtype_enum("hv_exit_reason_t") + .no_debug("hv_return_t") + .formatter(bindgen::Formatter::Prettyplease) + .header(format!( + "{manifest_dir}/src/hypervisor/virtual_machine/hvf/bindings.h" + )) + .generate() + .context("Unable to generate hvf bindings")? + .write_to_file(out_path.join("hvf_bindings.rs")) + .context("Couldn't write hvf bindings")?; + + cc::Build::new() + .opt_level(3) + .file(format!( + "{manifest_dir}/src/hypervisor/virtual_machine/hvf/fp_abi.c" + )) + .compile("hyperlight_host_hvf_abi_wrapper"); + } + // Makes #[cfg(kvm)] == #[cfg(all(feature = "kvm", target_os = "linux"))] // Essentially the kvm and mshv3 features are ignored on windows as long as you use #[cfg(kvm)] and not #[cfg(feature = "kvm")]. // You should never use #[cfg(feature = "kvm")] or #[cfg(feature = "mshv3")] in the codebase. diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/hvf/bindings.h b/src/hyperlight_host/src/hypervisor/virtual_machine/hvf/bindings.h new file mode 100644 index 000000000..7d0dd17d8 --- /dev/null +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/hvf/bindings.h @@ -0,0 +1,22 @@ +/* +Copyright 2025 The Hyperlight Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +#include + +hv_return_t +hv_vcpu_get_simd_fp_reg_rsabi(hv_vcpu_t vcpu, hv_simd_fp_reg_t reg, char *val); +hv_return_t +hv_vcpu_set_simd_fp_reg_rsabi(hv_vcpu_t vcpu, hv_simd_fp_reg_t reg, const char *val); diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/hvf/fp_abi.c b/src/hyperlight_host/src/hypervisor/virtual_machine/hvf/fp_abi.c new file mode 100644 index 000000000..a6fa3edac --- /dev/null +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/hvf/fp_abi.c @@ -0,0 +1,36 @@ +/* +Copyright 2025 The Hyperlight Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +#include "bindings.h" + +hv_return_t +hv_vcpu_get_simd_fp_reg_rsabi(hv_vcpu_t vcpu, hv_simd_fp_reg_t reg, char *val) { + hv_simd_fp_uchar16_t simd = {0}; + hv_return_t ret = hv_vcpu_get_simd_fp_reg(vcpu, reg, &simd); + for (int i = 0; i < 16; ++i) { + val[i] = simd[i]; + } + return ret; +} + +hv_return_t +hv_vcpu_set_simd_fp_reg_rsabi(hv_vcpu_t vcpu, hv_simd_fp_reg_t reg, const char *val) { + hv_simd_fp_uchar16_t simd; + for (int i = 0; i < 16; ++i) { + simd[i] = val[i]; + } + return hv_vcpu_set_simd_fp_reg(vcpu, reg, simd); +} diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/hvf/mod.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/hvf/mod.rs index f88f4981b..e0c405abf 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/hvf/mod.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/hvf/mod.rs @@ -14,6 +14,16 @@ See the License for the specific language governing permissions and limitations under the License. */ +#[allow( + dead_code, + non_snake_case, + non_upper_case_globals, + non_camel_case_types +)] // bindgen +pub(crate) mod bindings { + include!(concat!(env!("OUT_DIR"), "/hvf_bindings.rs")); +} + pub(super) fn is_hypervisor_present() -> bool { false } From 2168568f9b71f9cc2fe8c57a995987e32831de9a Mon Sep 17 00:00:00 2001 From: Lucy Menon <168595099+syntactically@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:24:09 +0100 Subject: [PATCH 07/17] hvf: add interrupt handle using `hv_vcpus_exit` todo: squash: hvf interrupt handle implementation Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- .../src/hypervisor/hyperlight_vm/aarch64.rs | 5 ++ src/hyperlight_host/src/hypervisor/mod.rs | 70 ++++++++++++++++--- src/hyperlight_host/src/sandbox/config.rs | 4 +- 3 files changed, 69 insertions(+), 10 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs index e204cb05f..82f3f9420 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs @@ -22,6 +22,8 @@ use super::{ AccessPageTableError, CreateHyperlightVmError, DispatchGuestCallError, HyperlightVm, InitializeError, }; +#[cfg(hvf)] +use crate::hypervisor::HvfInterruptHandle; use crate::hypervisor::InterruptHandleImpl; #[cfg(any(kvm, mshv3))] use crate::hypervisor::LinuxInterruptHandle; @@ -62,6 +64,9 @@ impl HyperlightVm { ) -> std::result::Result { // TODO: support gdb on aarch64 type VmType = Box; + #[cfg(hvf)] + let interrupt_handle: Arc = + Arc::new(HvfInterruptHandle::new(config.get_interrupt_retry_delay())); let vm: VmType = match get_available_hypervisor() { #[cfg(kvm)] Some(HypervisorType::Kvm) => Box::new(KvmVm::new().map_err(VmError::CreateVm)?), diff --git a/src/hyperlight_host/src/hypervisor/mod.rs b/src/hyperlight_host/src/hypervisor/mod.rs index 397921a0d..bca7e6ba6 100644 --- a/src/hyperlight_host/src/hypervisor/mod.rs +++ b/src/hyperlight_host/src/hypervisor/mod.rs @@ -42,7 +42,7 @@ use std::fmt::Debug; #[cfg(any(kvm, mshv3))] use std::sync::atomic::{AtomicBool, AtomicU64}; use std::sync::atomic::{AtomicU8, Ordering}; -#[cfg(any(kvm, mshv3))] +#[cfg(any(kvm, mshv3, hvf))] use std::time::Duration; #[derive(Debug)] @@ -136,6 +136,10 @@ pub(crate) trait InterruptHandleImpl: InterruptHandle { #[cfg(any(kvm, mshv3))] fn set_tid(&self); + /// Set the currently-executing vcpu id + #[cfg(hvf)] + fn set_vcpu(&self, vcpu: Option); + /// Mark the handle as dropped fn set_dropped(&self); } @@ -190,22 +194,29 @@ pub(super) struct RetryingInterruptHandle { inner: T, } +#[cfg(any(kvm, mshv3, hvf))] impl InterruptHandleImpl for RetryingInterruptHandle { #[cfg(any(kvm, mshv3))] fn set_tid(&self) { self.inner.set_tid(); } + #[cfg(hvf)] + fn set_vcpu(&self, vcpu: Option) { + self.inner.set_vcpu(vcpu); + } fn set_dropped(&self) { self.inner.set_dropped(); } } +#[cfg(any(kvm, mshv3, hvf))] impl InterruptHandle for RetryingInterruptHandle { fn dropped(&self) -> bool { self.inner.dropped() } } +#[cfg(any(kvm, mshv3, hvf))] impl InterruptHandleInternal for RetryingInterruptHandle { fn state(&self) -> &InterruptHandleStateMachine { self.inner.state() @@ -303,7 +314,7 @@ impl InterruptHandleInternal for LinuxInterruptHandleState { } } -#[cfg(target_os = "windows")] +#[cfg(any(target_os = "windows", hvf))] #[derive(Debug)] /// An interrupt handle that captures the pattern that requests to /// cancel need to be mutually exclusive with partition destruction @@ -315,11 +326,11 @@ pub(super) struct SynchronousInterruptHandle { /// Fox example, on Windows, this lock prevents a race condition /// between `kill()` calling `WHvCancelRunVirtualProcessor` and /// `WhpVm::drop()` calling `WHvDeletePartition`. These two - /// Windows Hypervisor Platform APIs must not execute concurrently - /// - if `WHvDeletePartition` frees the partition while - /// `WHvCancelRunVirtualProcessor` is still accessing it, the - /// result is a use-after-free causing STATUS_ACCESS_VIOLATION or - /// STATUS_HEAP_CORRUPTION. + /// Windows Hypervisor Platform APIs must not execute + /// concurrently---if `WHvDeletePartition` frees the partition + /// while `WHvCancelRunVirtualProcessor` is still accessing it, + /// the result is a use-after-free causing STATUS_ACCESS_VIOLATION + /// or STATUS_HEAP_CORRUPTION. /// /// The synchronization works as follows: /// - `kill()` takes a read lock before calling `WHvCancelRunVirtualProcessor` @@ -333,10 +344,21 @@ trait SynchronousInterruptState: Debug + Send + Sync { /// The inside-the-lock part of the common part of both kill() /// and kill_from_debugger() fn actually_cancel(&self) -> bool; + + #[cfg(hvf)] + fn set_vcpu(&mut self, vcpu: Option); } -#[cfg(target_os = "windows")] +#[cfg(any(target_os = "windows", hvf))] impl InterruptHandleImpl for SynchronousInterruptHandle { + #[cfg(hvf)] + fn set_vcpu(&self, vcpu: Option) { + let Ok(mut guard) = self.dropped_state.write() else { + return; + }; + guard.1.set_vcpu(vcpu); + } + fn set_dropped(&self) { // Take write lock to: // 1. Wait for any in-flight kill() calls (holding read locks) to complete @@ -418,6 +440,38 @@ impl SynchronousInterruptState for WHV_PARTITION_HANDLE { } } +#[cfg(hvf)] +use crate::hypervisor::virtual_machine::hvf::bindings::hv_vcpu_t; +#[cfg(hvf)] +pub(super) type HvfInterruptHandle = + RetryingInterruptHandle>>; +#[cfg(hvf)] +impl SynchronousInterruptState for Option { + fn actually_cancel(&self) -> bool { + use crate::hypervisor::virtual_machine::hvf::bindings::{HV_SUCCESS, hv_vcpus_exit}; + let Some(vcpu) = self else { + return false; + }; + unsafe { + // bindgen automatically uses *mut, but actually this will + // not be written to. + hv_vcpus_exit(&raw const *vcpu as *mut hv_vcpu_t, 1).0.0.0 == HV_SUCCESS + } + } + + fn set_vcpu(&mut self, vcpu: Option) { + *self = vcpu; + } +} +#[cfg(hvf)] +impl HvfInterruptHandle { + pub(super) fn new(retry_delay: Duration) -> Self { + RetryingInterruptHandle { + retry_delay, + inner: SynchronousInterruptHandle { + state: InterruptHandleStateMachine::new(), + dropped_state: std::sync::RwLock::new((false, None)), + }, } } } diff --git a/src/hyperlight_host/src/sandbox/config.rs b/src/hyperlight_host/src/sandbox/config.rs index 442da8415..70b07c522 100644 --- a/src/hyperlight_host/src/sandbox/config.rs +++ b/src/hyperlight_host/src/sandbox/config.rs @@ -169,13 +169,13 @@ impl SandboxConfiguration { } /// Sets the interrupt retry delay - #[cfg(target_os = "linux")] + #[cfg(any(kvm, mshv3, hvf))] pub fn set_interrupt_retry_delay(&mut self, delay: Duration) { self.interrupt_retry_delay = delay; } /// Get the delay between retries for interrupts - #[cfg(target_os = "linux")] + #[cfg(any(kvm, mshv3, hvf))] pub fn get_interrupt_retry_delay(&self) -> Duration { self.interrupt_retry_delay } From ed7b5e1d14d12c19f226f6475f78e62078fe9248 Mon Sep 17 00:00:00 2001 From: Lucy Menon <168595099+syntactically@users.noreply.github.com> Date: Tue, 7 Jul 2026 12:57:22 +0100 Subject: [PATCH 08/17] Disentangle host and guest page sizes On AArch64, there is no particular reason to expect the page size of the host to match the page size of the guest, but the host previously used some constants (`hyperlight_common::mem::{PAGE_SIZE, PAGE_SIZE_USIZE, PAGE_SHIFT}`) both when computing guest memory layouts and when constructing host->guest mappings. This commit removes all the uses of those ambiguous constants, replacing them with `page_size::get()` when the host page size is needed and `hyperlight_common::vmem::PAGE_SIZE` when the guest page size is required. It also makes a few tweaks to remove assumptions that the page sizes are the same, allowing (at the very least) operation on a host with 16k pages while allowing guests to continue using 4k pages. Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- src/hyperlight_common/src/mem.rs | 4 - .../src/hypervisor/hyperlight_vm/x86_64.rs | 6 +- .../src/hypervisor/surrogate_process.rs | 7 +- src/hyperlight_host/src/mem/layout.rs | 53 ++++--- src/hyperlight_host/src/mem/memory_region.rs | 8 +- src/hyperlight_host/src/mem/mgr.rs | 3 +- src/hyperlight_host/src/mem/shared_mem.rs | 150 +++++++++--------- .../src/mem/shared_mem_tests.rs | 4 +- .../src/sandbox/initialized_multi_use.rs | 12 +- .../src/sandbox/snapshot/file/config.rs | 11 +- .../src/sandbox/snapshot/mod.rs | 31 ++-- .../src/sandbox/snapshot/tripwires.rs | 5 +- 12 files changed, 152 insertions(+), 142 deletions(-) diff --git a/src/hyperlight_common/src/mem.rs b/src/hyperlight_common/src/mem.rs index 46798b7ee..358d44cff 100644 --- a/src/hyperlight_common/src/mem.rs +++ b/src/hyperlight_common/src/mem.rs @@ -14,10 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -pub const PAGE_SHIFT: u64 = 12; -pub const PAGE_SIZE: u64 = 1 << 12; -pub const PAGE_SIZE_USIZE: usize = 1 << 12; - /// A memory region in the guest address space #[derive(Debug, Clone, Copy, PartialEq, bytemuck::Pod, bytemuck::Zeroable)] #[repr(C)] diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs index 588ab7a80..530612b97 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs @@ -600,8 +600,6 @@ impl HyperlightVm { #[cfg(gdb)] pub(super) mod debug { - use hyperlight_common::mem::PAGE_SIZE; - use super::HyperlightVm; use crate::hypervisor::gdb::arch::{SW_BP, SW_BP_SIZE}; use crate::hypervisor::gdb::{ @@ -782,7 +780,7 @@ pub(super) mod debug { let read_len = std::cmp::min( data.len(), - (PAGE_SIZE - (gpa & (PAGE_SIZE - 1))).try_into().unwrap(), + page_size::get() - (gpa as usize & (page_size::get() - 1)), ); mem_access.read(&mut data[..read_len], gpa)?; @@ -810,7 +808,7 @@ pub(super) mod debug { let write_len = std::cmp::min( data.len(), - (PAGE_SIZE - (gpa & (PAGE_SIZE - 1))).try_into().unwrap(), + page_size::get() - (gpa as usize & (page_size::get() - 1)), ); // Use the memory access to write to guest memory diff --git a/src/hyperlight_host/src/hypervisor/surrogate_process.rs b/src/hyperlight_host/src/hypervisor/surrogate_process.rs index 481aa44e8..0189c6530 100644 --- a/src/hyperlight_host/src/hypervisor/surrogate_process.rs +++ b/src/hyperlight_host/src/hypervisor/surrogate_process.rs @@ -18,7 +18,6 @@ use core::ffi::c_void; use std::collections::HashMap; use std::collections::hash_map::Entry; -use hyperlight_common::mem::PAGE_SIZE_USIZE; use tracing::{Span, instrument}; use windows::Win32::Foundation::HANDLE; use windows::Win32::System::Memory::{ @@ -150,7 +149,7 @@ impl SurrogateProcess { VirtualProtectEx( self.process_handle.into(), first_guard_page_start, - PAGE_SIZE_USIZE, + page_size::get(), PAGE_NOACCESS, &mut unused_out_old_prot_flags, ) @@ -161,12 +160,12 @@ impl SurrogateProcess { // the last page of the raw_size is the guard page let last_guard_page_start = - unsafe { first_guard_page_start.add(host_size - PAGE_SIZE_USIZE) }; + unsafe { first_guard_page_start.add(host_size - page_size::get()) }; if let Err(e) = unsafe { VirtualProtectEx( self.process_handle.into(), last_guard_page_start, - PAGE_SIZE_USIZE, + page_size::get(), PAGE_NOACCESS, &mut unused_out_old_prot_flags, ) diff --git a/src/hyperlight_host/src/mem/layout.rs b/src/hyperlight_host/src/mem/layout.rs index 6422b9b11..9662be731 100644 --- a/src/hyperlight_host/src/mem/layout.rs +++ b/src/hyperlight_host/src/mem/layout.rs @@ -63,7 +63,8 @@ limitations under the License. use std::fmt::Debug; use std::mem::size_of; -use hyperlight_common::mem::{HyperlightPEB, PAGE_SIZE_USIZE}; +use hyperlight_common::mem::HyperlightPEB; +use hyperlight_common::vmem::PAGE_SIZE; use tracing::{Span, instrument}; use super::memory_region::MemoryRegionType::{Code, Heap, InitData, Peb}; @@ -544,6 +545,17 @@ impl SandboxMemoryLayout { let expected_final_offset = TryInto::::try_into(self.get_memory_size()?)?; + // This function is primarily used to construct + // GuestMemoryRegions used to populate initial guest page + // tables. Therefore, the regions above are aligned based on + // the guest page size. However, the total final size of the + // region mapped into the guest needs to be aligned based on + // the host page size. Therefore, align both of these values + // to both page sizes before comparing them. + let final_offset = final_offset.next_multiple_of(page_size::get()); + let expected_final_offset = + expected_final_offset.next_multiple_of(hyperlight_common::vmem::PAGE_SIZE); + if final_offset != expected_final_offset { return Err(new_error!( "Final offset does not match expected Final offset expected: {}, actual: {}", @@ -654,7 +666,7 @@ impl SandboxMemoryLayout { impl SandboxMemoryLayout { /// Offset of the PEB struct within the snapshot region. pub(crate) fn peb_offset(&self) -> usize { - self.code_size.next_multiple_of(PAGE_SIZE_USIZE) + self.code_size.next_multiple_of(PAGE_SIZE) } /// Guest physical address of the PEB. @@ -664,12 +676,12 @@ impl SandboxMemoryLayout { /// Offset of the guest heap buffer within the snapshot region. pub(crate) fn guest_heap_buffer_offset(&self) -> usize { - (self.peb_offset() + size_of::()).next_multiple_of(PAGE_SIZE_USIZE) + (self.peb_offset() + size_of::()).next_multiple_of(PAGE_SIZE) } /// Offset of the init data section within the snapshot region. pub(crate) fn init_data_offset(&self) -> usize { - (self.guest_heap_buffer_offset() + self.heap_size).next_multiple_of(PAGE_SIZE_USIZE) + (self.guest_heap_buffer_offset() + self.heap_size).next_multiple_of(PAGE_SIZE) } /// The code offset is always 0. @@ -705,8 +717,7 @@ impl SandboxMemoryLayout { /// Offset from the beginning of the scratch region to the location /// where page tables are eagerly copied on restore. pub(crate) fn get_pt_base_scratch_offset(&self) -> usize { - (self.input_data_size + self.output_data_size) - .next_multiple_of(hyperlight_common::vmem::PAGE_SIZE) + (self.input_data_size + self.output_data_size).next_multiple_of(PAGE_SIZE) } /// Base GPA to which the page tables are eagerly copied on restore. @@ -731,12 +742,12 @@ impl SandboxMemoryLayout { pub(crate) fn get_memory_size(&self) -> Result { let total_memory = self.get_unaligned_memory_size(); - // Size should be a multiple of page size. - let remainder = total_memory % PAGE_SIZE_USIZE; - let multiples = total_memory / PAGE_SIZE_USIZE; + // Size should be a multiple of host page size. + let remainder = total_memory % page_size::get(); + let multiples = total_memory / page_size::get(); let size = match remainder { 0 => total_memory, - _ => (multiples + 1) * PAGE_SIZE_USIZE, + _ => (multiples + 1) * page_size::get(), }; if size > Self::MAX_MEMORY_SIZE { @@ -749,8 +760,6 @@ impl SandboxMemoryLayout { #[cfg(test)] mod tests { - use hyperlight_common::mem::PAGE_SIZE_USIZE; - use super::*; // helper func for testing @@ -761,9 +770,9 @@ mod tests { // PEB let peb_and_array = size_of::(); - expected_size += peb_and_array.next_multiple_of(PAGE_SIZE_USIZE); + expected_size += peb_and_array.next_multiple_of(PAGE_SIZE); - expected_size += layout.heap_size.next_multiple_of(PAGE_SIZE_USIZE); + expected_size += layout.heap_size.next_multiple_of(PAGE_SIZE); expected_size } @@ -805,8 +814,8 @@ mod tests { let cfg = SandboxConfiguration::default(); let a = SandboxMemoryLayout::new(cfg, 4096, 0, None).unwrap(); let mut b = a; - b.snapshot_size = a.snapshot_size + PAGE_SIZE_USIZE; - b.set_pt_size(PAGE_SIZE_USIZE).unwrap(); + b.snapshot_size = a.snapshot_size + PAGE_SIZE; + b.set_pt_size(PAGE_SIZE).unwrap(); assert!(a.is_compatible_with(&b)); assert!(b.is_compatible_with(&a)); } @@ -818,12 +827,12 @@ mod tests { // Each mutation must independently break compatibility. let mutators: &[fn(&mut SandboxMemoryLayout)] = &[ - |l| l.input_data_size += PAGE_SIZE_USIZE, - |l| l.output_data_size += PAGE_SIZE_USIZE, - |l| l.heap_size += PAGE_SIZE_USIZE, - |l| l.code_size += PAGE_SIZE_USIZE, - |l| l.init_data_size += PAGE_SIZE_USIZE, - |l| l.scratch_size += PAGE_SIZE_USIZE, + |l| l.input_data_size += PAGE_SIZE, + |l| l.output_data_size += PAGE_SIZE, + |l| l.heap_size += PAGE_SIZE, + |l| l.code_size += PAGE_SIZE, + |l| l.init_data_size += PAGE_SIZE, + |l| l.scratch_size += PAGE_SIZE, |l| { l.init_data_permissions = Some(MemoryRegionFlags::READ); }, diff --git a/src/hyperlight_host/src/mem/memory_region.rs b/src/hyperlight_host/src/mem/memory_region.rs index 5d839647a..690bfe806 100644 --- a/src/hyperlight_host/src/mem/memory_region.rs +++ b/src/hyperlight_host/src/mem/memory_region.rs @@ -17,9 +17,7 @@ limitations under the License. use std::ops::Range; use bitflags::bitflags; -#[cfg(mshv3)] -use hyperlight_common::mem::PAGE_SHIFT; -use hyperlight_common::mem::PAGE_SIZE_USIZE; +use hyperlight_common::vmem::PAGE_SIZE; #[cfg(kvm)] use kvm_bindings::{KVM_MEM_READONLY, kvm_userspace_memory_region}; #[cfg(mshv3)] @@ -388,7 +386,7 @@ impl MemoryRegionVecBuilder { flags: MemoryRegionFlags, region_type: MemoryRegionType, ) -> usize { - let aligned_size = (size + PAGE_SIZE_USIZE - 1) & !(PAGE_SIZE_USIZE - 1); + let aligned_size = (size + PAGE_SIZE - 1) & !(PAGE_SIZE - 1); self.push(aligned_size, flags, region_type) } @@ -403,7 +401,7 @@ impl MemoryRegionVecBuilder { 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 >> PAGE_SHIFT; + let guest_pfn = (region.guest_region.start / page_size::get()) as u64; let userspace_addr = region.host_region.start as u64; let flags: u8 = region.flags.iter().fold(0, |acc, flag| { diff --git a/src/hyperlight_host/src/mem/mgr.rs b/src/hyperlight_host/src/mem/mgr.rs index 8e4558770..9a2933f78 100644 --- a/src/hyperlight_host/src/mem/mgr.rs +++ b/src/hyperlight_host/src/mem/mgr.rs @@ -574,7 +574,8 @@ impl SandboxMemoryManager { // immediately after the snapshot in the guest PA space. let snapshot_pt_end = self.shared_mem.mem_size(); let snapshot_pt_size = self.layout.get_pt_size(); - let snapshot_pt_start = snapshot_pt_end - snapshot_pt_size; + let snapshot_pt_start = + snapshot_pt_end - snapshot_pt_size.next_multiple_of(page_size::get()); self.scratch_mem.with_exclusivity(|scratch| { #[cfg(not(unshared_snapshot_mem))] let bytes = &self.shared_mem.as_slice()[snapshot_pt_start..snapshot_pt_end]; diff --git a/src/hyperlight_host/src/mem/shared_mem.rs b/src/hyperlight_host/src/mem/shared_mem.rs index c1b29ede5..0632dc35e 100644 --- a/src/hyperlight_host/src/mem/shared_mem.rs +++ b/src/hyperlight_host/src/mem/shared_mem.rs @@ -23,7 +23,6 @@ use std::ptr::null_mut; use std::sync::{Arc, RwLock}; use bytemuck::Pod; -use hyperlight_common::mem::PAGE_SIZE_USIZE; use tracing::{Span, instrument}; #[cfg(target_os = "windows")] use windows::Win32::Foundation::{CloseHandle, HANDLE, INVALID_HANDLE_VALUE}; @@ -294,7 +293,7 @@ impl Placeholder { fn split_front(self, front_size: usize) -> Result<(Placeholder, Placeholder)> { debug_assert!(front_size > 0 && front_size < self.size); - debug_assert!(front_size.is_multiple_of(PAGE_SIZE_USIZE)); + debug_assert!(front_size.is_multiple_of(page_size::get())); // SAFETY: `self` owns the placeholder reservation at // `[self.addr, self.addr + self.size)`. `MEM_RELEASE | // MEM_PRESERVE_PLACEHOLDER` is the Win32 idiom for splitting @@ -401,7 +400,7 @@ pub trait SharedMemory { /// need to be marked as `unsafe` because doing anything with this /// pointer itself requires `unsafe`. fn base_addr(&self) -> usize { - self.region().ptr() as usize + PAGE_SIZE_USIZE + self.region().ptr() as usize + page_size::get() } /// Return the base address of the host mapping of this region as @@ -409,14 +408,14 @@ pub trait SharedMemory { /// not need to be marked as `unsafe` because doing anything with /// this pointer itself requires `unsafe`. fn base_ptr(&self) -> *mut u8 { - self.region().ptr().wrapping_add(PAGE_SIZE_USIZE) + self.region().ptr().wrapping_add(page_size::get()) } /// Return the length of usable memory contained in `self`. /// The returned size does not include the size of the surrounding /// guard pages. fn mem_size(&self) -> usize { - self.region().size() - 2 * PAGE_SIZE_USIZE + self.region().size() - 2 * page_size::get() } /// Return the raw base address of the host mapping, including the @@ -448,7 +447,7 @@ pub trait SharedMemory { from_handle: self.region().file_mapping_handle().into(), handle_base: self.region().ptr() as usize, handle_size: self.region().size(), - offset: PAGE_SIZE_USIZE, + offset: page_size::get(), } } } @@ -552,13 +551,13 @@ impl ExclusiveSharedMemory { } let total_size = min_size_bytes - .checked_add(2 * PAGE_SIZE_USIZE) // guard page around the memory + .checked_add(2 * page_size::get()) // guard page around the memory .ok_or_else(|| new_error!("Memory required for sandbox exceeded usize::MAX"))?; - if total_size % PAGE_SIZE_USIZE != 0 { + if total_size % page_size::get() != 0 { return Err(new_error!( "shared memory must be a multiple of {}", - PAGE_SIZE_USIZE + page_size::get() )); } @@ -600,7 +599,7 @@ impl ExclusiveSharedMemory { // protect the guard pages #[cfg(not(miri))] { - let res = unsafe { mprotect(mmap.base, PAGE_SIZE_USIZE, PROT_NONE) }; + let res = unsafe { mprotect(mmap.base, page_size::get(), PROT_NONE) }; if res != 0 { return Err(HyperlightError::MprotectFailed( Error::last_os_error().raw_os_error(), @@ -608,8 +607,8 @@ impl ExclusiveSharedMemory { } let res = unsafe { mprotect( - (mmap.base as *const u8).add(total_size - PAGE_SIZE_USIZE) as *mut c_void, - PAGE_SIZE_USIZE, + (mmap.base as *const u8).add(total_size - page_size::get()) as *mut c_void, + page_size::get(), PROT_NONE, ) }; @@ -646,13 +645,13 @@ impl ExclusiveSharedMemory { } let total_size = min_size_bytes - .checked_add(2 * PAGE_SIZE_USIZE) + .checked_add(2 * page_size::get()) .ok_or_else(|| new_error!("Memory required for sandbox exceeded {}", usize::MAX))?; - if total_size % PAGE_SIZE_USIZE != 0 { + if total_size % page_size::get() != 0 { return Err(new_error!( "shared memory must be a multiple of {}", - PAGE_SIZE_USIZE + page_size::get() )); } @@ -719,7 +718,7 @@ impl ExclusiveSharedMemory { if let Err(e) = unsafe { VirtualProtect( first_guard_page_start, - PAGE_SIZE_USIZE, + page_size::get(), PAGE_NOACCESS, &mut unused_out_old_prot_flags, ) @@ -727,11 +726,11 @@ 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 { view.addr.add(total_size - page_size::get()) }; if let Err(e) = unsafe { VirtualProtect( last_guard_page_start, - PAGE_SIZE_USIZE, + page_size::get(), PAGE_NOACCESS, &mut unused_out_old_prot_flags, ) @@ -1489,7 +1488,7 @@ unsafe impl Sync for ReadonlySharedMemory {} impl ReadonlySharedMemory { pub(crate) fn from_bytes(contents: &[u8], guest_mapped_size: usize) -> Result { - if guest_mapped_size == 0 || !guest_mapped_size.is_multiple_of(PAGE_SIZE_USIZE) { + if guest_mapped_size == 0 || !guest_mapped_size.is_multiple_of(page_size::get()) { return Err(new_error!( "guest_mapped_size {} must be a non-zero multiple of PAGE_SIZE", guest_mapped_size @@ -1502,7 +1501,8 @@ impl ReadonlySharedMemory { contents.len() )); } - let mut anon = ExclusiveSharedMemory::new(contents.len())?; + let mut anon = + ExclusiveSharedMemory::new(contents.len().next_multiple_of(page_size::get()))?; anon.copy_from_slice(contents, 0)?; Ok(ReadonlySharedMemory { region: anon.region, @@ -1535,7 +1535,7 @@ impl ReadonlySharedMemory { )); } - if !len.is_multiple_of(PAGE_SIZE_USIZE) { + if !len.is_multiple_of(page_size::get()) { return Err(new_error!( "file length {} must be a multiple of PAGE_SIZE", len @@ -1544,7 +1544,7 @@ impl ReadonlySharedMemory { if guest_mapped_size == 0 || guest_mapped_size > len - || !guest_mapped_size.is_multiple_of(PAGE_SIZE_USIZE) + || !guest_mapped_size.is_multiple_of(page_size::get()) { return Err(new_error!( "guest_mapped_size {} must be a non-zero multiple of PAGE_SIZE no greater than file length {}", @@ -1574,7 +1574,7 @@ impl ReadonlySharedMemory { mmap, off_t, size_t, }; - let total_size = len.checked_add(2 * PAGE_SIZE_USIZE).ok_or_else(|| { + let total_size = len.checked_add(2 * page_size::get()).ok_or_else(|| { new_error!("Memory required for file-backed mapping exceeded usize::MAX") })?; @@ -1619,7 +1619,7 @@ impl ReadonlySharedMemory { let file_prot = PROT_READ; // SAFETY: `total_size = len + 2 * PAGE_SIZE_USIZE`, so // `base + PAGE_SIZE_USIZE` is in-bounds of the reservation. - let usable_ptr = unsafe { (base as *mut u8).add(PAGE_SIZE_USIZE) }; + let usable_ptr = unsafe { (base as *mut u8).add(page_size::get()) }; // SAFETY: `usable_ptr..usable_ptr + len` lies entirely within // the reservation owned by `reservation`. `MAP_FIXED` // replaces that sub-range in place; on failure the @@ -1655,7 +1655,7 @@ impl ReadonlySharedMemory { fn map_file(file: &std::fs::File, len: usize) -> Result> { use std::os::windows::io::AsRawHandle; - let total_size = len.checked_add(2 * PAGE_SIZE_USIZE).ok_or_else(|| { + let total_size = len.checked_add(2 * page_size::get()).ok_or_else(|| { new_error!("Memory required for file-backed mapping exceeded usize::MAX") })?; @@ -1668,7 +1668,7 @@ impl ReadonlySharedMemory { // 2. Split the placeholder into three adjacent slots. The // leading and trailing slots stay unmapped and act as // guard pages. The middle slot will receive the file view. - let (leading, middle, trailing) = whole.split_into_three(PAGE_SIZE_USIZE, len)?; + let (leading, middle, trailing) = whole.split_into_three(page_size::get(), len)?; // 3. Create a read-only file mapping section over the file. // SAFETY: `file_handle` is a valid file HANDLE borrowed from @@ -1757,7 +1757,7 @@ impl SharedMemory for ReadonlySharedMemory { from_handle: self.region().file_mapping_handle().into(), handle_base: self.region().ptr() as usize, handle_size: self.region().size(), - offset: PAGE_SIZE_USIZE, + offset: page_size::get(), }, WindowsMapping::FileBacked { .. } => super::memory_region::HostRegionBase { from_handle: self.region().file_mapping_handle().into(), @@ -1791,7 +1791,6 @@ impl PartialEq for ReadonlySharedMemory { #[cfg(test)] mod tests { - use hyperlight_common::mem::PAGE_SIZE_USIZE; #[cfg(not(miri))] use proptest::prelude::*; @@ -1804,7 +1803,7 @@ mod tests { #[test] fn fill() { - let mem_size: usize = 4096; + let mem_size: usize = page_size::get(); let eshm = ExclusiveSharedMemory::new(mem_size).unwrap(); let (mut hshm, _) = eshm.build(); @@ -1822,7 +1821,7 @@ mod tests { assert!(vec[2048..3072].iter().all(|&x| x == 3)); assert!(vec[3072..4096].iter().all(|&x| x == 4)); - hshm.fill(5, 0, 4096).unwrap(); + hshm.fill(5, 0, mem_size).unwrap(); let vec2 = hshm .with_exclusivity(|e| e.copy_all_to_vec().unwrap()) @@ -1837,7 +1836,7 @@ mod tests { /// would overflow `usize`. #[test] fn bounds_check_overflow() { - let mem_size: usize = 4096; + let mem_size: usize = page_size::get(); let mut eshm = ExclusiveSharedMemory::new(mem_size).unwrap(); // ExclusiveSharedMemory methods @@ -1863,7 +1862,7 @@ mod tests { #[test] fn copy_into_from() -> Result<()> { - let mem_size: usize = 4096; + let mem_size: usize = page_size::get(); let vec_len = 10; let eshm = ExclusiveSharedMemory::new(mem_size)?; let (hshm, _) = eshm.build(); @@ -1954,7 +1953,7 @@ mod tests { #[test] fn clone() { - let eshm = ExclusiveSharedMemory::new(PAGE_SIZE_USIZE).unwrap(); + let eshm = ExclusiveSharedMemory::new(page_size::get()).unwrap(); let (hshm1, _) = eshm.build(); let hshm2 = hshm1.clone(); @@ -1991,7 +1990,7 @@ mod tests { #[test] fn copy_all_to_vec() { let mut data = vec![b'a', b'b', b'c']; - data.resize(4096, 0); + data.resize(page_size::get(), 0); let mut eshm = ExclusiveSharedMemory::new(data.len()).unwrap(); eshm.copy_from_slice(data.as_slice(), 0).unwrap(); let ret_vec = eshm.copy_all_to_vec().unwrap(); @@ -2014,11 +2013,11 @@ mod tests { // where another test allocates memory at the same address between our // drop and the mapping check. Ensure UNIQUE_SIZE is not used by any // other test in the codebase to avoid this. - const UNIQUE_SIZE: usize = PAGE_SIZE_USIZE * 17; + let unique_size: usize = page_size::get() * 17; let pid = std::process::id(); - let eshm = ExclusiveSharedMemory::new(UNIQUE_SIZE).unwrap(); + let eshm = ExclusiveSharedMemory::new(unique_size).unwrap(); let (hshm1, gshm) = eshm.build(); let hshm2 = hshm1.clone(); @@ -2068,7 +2067,7 @@ mod tests { #[test] fn copy_with_various_alignments() { // Use a buffer large enough to test all alignment cases - let mem_size: usize = 4096; + let mem_size: usize = page_size::get(); let eshm = ExclusiveSharedMemory::new(mem_size).unwrap(); let (hshm, _) = eshm.build(); @@ -2095,7 +2094,7 @@ mod tests { /// Test copy operations with lengths smaller than chunk size (< 16 bytes) #[test] fn copy_small_lengths() { - let mem_size: usize = 4096; + let mem_size: usize = page_size::get(); let eshm = ExclusiveSharedMemory::new(mem_size).unwrap(); let (hshm, _) = eshm.build(); @@ -2114,7 +2113,7 @@ mod tests { /// Test copy operations with lengths that don't align to chunk boundaries #[test] fn copy_non_aligned_lengths() { - let mem_size: usize = 4096; + let mem_size: usize = page_size::get(); let eshm = ExclusiveSharedMemory::new(mem_size).unwrap(); let (hshm, _) = eshm.build(); @@ -2136,7 +2135,7 @@ mod tests { /// Test copy with exactly one chunk (16 bytes) #[test] fn copy_exact_chunk_size() { - let mem_size: usize = 4096; + let mem_size: usize = page_size::get(); let eshm = ExclusiveSharedMemory::new(mem_size).unwrap(); let (hshm, _) = eshm.build(); @@ -2153,7 +2152,7 @@ mod tests { /// Test fill with various alignment offsets #[test] fn fill_with_various_alignments() { - let mem_size: usize = 4096; + let mem_size: usize = page_size::get(); let eshm = ExclusiveSharedMemory::new(mem_size).unwrap(); let (mut hshm, _) = eshm.build(); @@ -2182,7 +2181,7 @@ mod tests { /// Test fill with lengths smaller than chunk size #[test] fn fill_small_lengths() { - let mem_size: usize = 4096; + let mem_size: usize = page_size::get(); let eshm = ExclusiveSharedMemory::new(mem_size).unwrap(); let (mut hshm, _) = eshm.build(); @@ -2206,7 +2205,7 @@ mod tests { /// Test fill with non-aligned lengths #[test] fn fill_non_aligned_lengths() { - let mem_size: usize = 4096; + let mem_size: usize = page_size::get(); let eshm = ExclusiveSharedMemory::new(mem_size).unwrap(); let (mut hshm, _) = eshm.build(); @@ -2232,7 +2231,7 @@ mod tests { /// Test edge cases: length 0 and length 1 #[test] fn copy_edge_cases() { - let mem_size: usize = 4096; + let mem_size: usize = page_size::get(); let eshm = ExclusiveSharedMemory::new(mem_size).unwrap(); let (hshm, _) = eshm.build(); @@ -2254,7 +2253,7 @@ mod tests { /// Test combined: unaligned start + non-aligned length #[test] fn copy_unaligned_start_and_length() { - let mem_size: usize = 4096; + let mem_size: usize = page_size::get(); let eshm = ExclusiveSharedMemory::new(mem_size).unwrap(); let (hshm, _) = eshm.build(); @@ -2296,7 +2295,7 @@ mod tests { #[test] fn normal_push_pop_roundtrip() { - let mem_size = 4096; + let mem_size = page_size::get(); let mut hshm = make_buffer(mem_size); // Size-prefixed flatbuffer-like payload: [size: u32 LE][payload] @@ -2312,7 +2311,7 @@ mod tests { #[test] fn malicious_flatbuffer_size_prefix() { - let mem_size = 4096; + let mem_size = page_size::get(); let mut hshm = make_buffer(mem_size); let payload = b"small"; @@ -2335,7 +2334,7 @@ mod tests { #[test] fn malicious_element_offset_too_small() { - let mem_size = 4096; + let mem_size = page_size::get(); let mut hshm = make_buffer(mem_size); let payload = b"test"; @@ -2360,7 +2359,7 @@ mod tests { #[test] fn malicious_element_offset_past_stack_pointer() { - let mem_size = 4096; + let mem_size = page_size::get(); let mut hshm = make_buffer(mem_size); let payload = b"test"; @@ -2385,7 +2384,7 @@ mod tests { #[test] fn malicious_flatbuffer_size_off_by_one() { - let mem_size = 4096; + let mem_size = page_size::get(); let mut hshm = make_buffer(mem_size); let payload = b"abcd"; @@ -2410,7 +2409,7 @@ mod tests { /// `stack_pointer_rel - last_element_offset_rel - 8`. #[test] fn back_pointer_near_stack_pointer_underflow() { - let mem_size = 4096; + let mem_size = page_size::get(); let mut hshm = make_buffer(mem_size); let payload = b"test"; @@ -2436,7 +2435,7 @@ mod tests { /// Size prefix of 0xFFFF_FFFD causes u32 overflow: 0xFFFF_FFFD + 4 wraps. #[test] fn size_prefix_u32_overflow() { - let mem_size = 4096; + let mem_size = page_size::get(); let mut hshm = make_buffer(mem_size); let payload = b"test"; @@ -2481,7 +2480,7 @@ mod tests { fn read() { setup_signal_handler(); - let eshm = ExclusiveSharedMemory::new(4096).unwrap(); + let eshm = ExclusiveSharedMemory::new(page_size::get()).unwrap(); let (hshm, _) = eshm.build(); let guard_page_ptr = hshm.raw_ptr(); unsafe { std::ptr::read_volatile(guard_page_ptr) }; @@ -2492,7 +2491,7 @@ mod tests { fn write() { setup_signal_handler(); - let eshm = ExclusiveSharedMemory::new(4096).unwrap(); + let eshm = ExclusiveSharedMemory::new(page_size::get()).unwrap(); let (hshm, _) = eshm.build(); let guard_page_ptr = hshm.raw_ptr(); unsafe { std::ptr::write_volatile(guard_page_ptr, 0u8) }; @@ -2503,7 +2502,7 @@ mod tests { fn exec() { setup_signal_handler(); - let eshm = ExclusiveSharedMemory::new(4096).unwrap(); + let eshm = ExclusiveSharedMemory::new(page_size::get()).unwrap(); let (hshm, _) = eshm.build(); let guard_page_ptr = hshm.raw_ptr(); let func: fn() = unsafe { std::mem::transmute(guard_page_ptr) }; @@ -2550,7 +2549,6 @@ mod tests { mod from_file_tests { use std::io::Write; - use hyperlight_common::mem::PAGE_SIZE_USIZE; use tempfile::NamedTempFile; use crate::mem::shared_mem::{ReadonlySharedMemory, SharedMemory}; @@ -2570,10 +2568,10 @@ mod tests { #[test] fn from_file_success_single_page() { - let tmp = make_temp_file(PAGE_SIZE_USIZE); - let mut rsm = ReadonlySharedMemory::from_file(tmp.as_file(), PAGE_SIZE_USIZE) + let tmp = make_temp_file(page_size::get()); + let mut rsm = ReadonlySharedMemory::from_file(tmp.as_file(), page_size::get()) .expect("from_file should succeed"); - assert_eq!(rsm.mem_size(), PAGE_SIZE_USIZE); + assert_eq!(rsm.mem_size(), page_size::get()); rsm.with_contents(|slice| { for (i, b) in slice.iter().enumerate() { assert_eq!(*b, (i & 0xff) as u8); @@ -2584,31 +2582,31 @@ mod tests { #[test] fn from_file_success_smaller_guest_mapped_size() { - let tmp = make_temp_file(2 * PAGE_SIZE_USIZE); - let rsm = ReadonlySharedMemory::from_file(tmp.as_file(), PAGE_SIZE_USIZE) + let tmp = make_temp_file(2 * page_size::get()); + let rsm = ReadonlySharedMemory::from_file(tmp.as_file(), page_size::get()) .expect("from_file should succeed"); - assert_eq!(rsm.mem_size(), 2 * PAGE_SIZE_USIZE); + assert_eq!(rsm.mem_size(), 2 * page_size::get()); } #[test] fn from_file_rejects_empty_file() { let tmp = make_temp_file(0); - let err = ReadonlySharedMemory::from_file(tmp.as_file(), PAGE_SIZE_USIZE) + let err = ReadonlySharedMemory::from_file(tmp.as_file(), page_size::get()) .expect_err("empty file should be rejected"); assert!(format!("{}", err).contains("size 0")); } #[test] fn from_file_rejects_unaligned_file_length() { - let tmp = make_temp_file(PAGE_SIZE_USIZE + 1); - let err = ReadonlySharedMemory::from_file(tmp.as_file(), PAGE_SIZE_USIZE) + let tmp = make_temp_file(page_size::get() + 1); + let err = ReadonlySharedMemory::from_file(tmp.as_file(), page_size::get()) .expect_err("unaligned file length should be rejected"); assert!(format!("{}", err).contains("multiple of PAGE_SIZE")); } #[test] fn from_file_rejects_zero_guest_mapped_size() { - let tmp = make_temp_file(PAGE_SIZE_USIZE); + let tmp = make_temp_file(page_size::get()); let err = ReadonlySharedMemory::from_file(tmp.as_file(), 0) .expect_err("zero guest_mapped_size should be rejected"); assert!(format!("{}", err).contains("guest_mapped_size")); @@ -2616,16 +2614,16 @@ mod tests { #[test] fn from_file_rejects_unaligned_guest_mapped_size() { - let tmp = make_temp_file(2 * PAGE_SIZE_USIZE); - let err = ReadonlySharedMemory::from_file(tmp.as_file(), PAGE_SIZE_USIZE + 1) + let tmp = make_temp_file(2 * page_size::get()); + let err = ReadonlySharedMemory::from_file(tmp.as_file(), page_size::get() + 1) .expect_err("unaligned guest_mapped_size should be rejected"); assert!(format!("{}", err).contains("guest_mapped_size")); } #[test] fn from_file_rejects_guest_mapped_size_exceeding_file() { - let tmp = make_temp_file(PAGE_SIZE_USIZE); - let err = ReadonlySharedMemory::from_file(tmp.as_file(), 2 * PAGE_SIZE_USIZE) + let tmp = make_temp_file(page_size::get()); + let err = ReadonlySharedMemory::from_file(tmp.as_file(), 2 * page_size::get()) .expect_err("guest_mapped_size > file length should be rejected"); assert!(format!("{}", err).contains("guest_mapped_size")); } @@ -2636,8 +2634,6 @@ mod tests { /// test in the parent module re-executes each one in a subprocess /// and asserts the trap occurred. mod guard_page_crash_tests { - use hyperlight_common::mem::PAGE_SIZE_USIZE; - use super::make_temp_file; use crate::mem::shared_mem::{ReadonlySharedMemory, SharedMemory}; @@ -2645,10 +2641,10 @@ mod tests { #[test] #[ignore] pub(super) fn leading_guard_page_traps() { - let tmp = make_temp_file(PAGE_SIZE_USIZE); - let rsm = ReadonlySharedMemory::from_file(tmp.as_file(), PAGE_SIZE_USIZE) + let tmp = make_temp_file(page_size::get()); + let rsm = ReadonlySharedMemory::from_file(tmp.as_file(), page_size::get()) .expect("from_file should succeed"); - let guard_ptr = unsafe { rsm.base_ptr().sub(PAGE_SIZE_USIZE) }; + let guard_ptr = unsafe { rsm.base_ptr().sub(page_size::get()) }; println!("reached_guard"); let _ = unsafe { std::ptr::read_volatile(guard_ptr) }; println!("survived_guard"); @@ -2658,8 +2654,8 @@ mod tests { #[test] #[ignore] pub(super) fn trailing_guard_page_traps() { - let tmp = make_temp_file(PAGE_SIZE_USIZE); - let rsm = ReadonlySharedMemory::from_file(tmp.as_file(), PAGE_SIZE_USIZE) + let tmp = make_temp_file(page_size::get()); + let rsm = ReadonlySharedMemory::from_file(tmp.as_file(), page_size::get()) .expect("from_file should succeed"); let guard_ptr = unsafe { rsm.base_ptr().add(rsm.mem_size()) }; println!("reached_guard"); diff --git a/src/hyperlight_host/src/mem/shared_mem_tests.rs b/src/hyperlight_host/src/mem/shared_mem_tests.rs index 09e3c4f8e..1da949333 100644 --- a/src/hyperlight_host/src/mem/shared_mem_tests.rs +++ b/src/hyperlight_host/src/mem/shared_mem_tests.rs @@ -20,8 +20,6 @@ use std::convert::TryFrom; use std::fmt::Debug; use std::mem::size_of; -use hyperlight_common::mem::PAGE_SIZE_USIZE; - use crate::{Result, log_then_return, new_error}; /// A function that knows how to read data of type `T` from a @@ -53,7 +51,7 @@ where T: PartialEq + Debug + Clone + TryFrom, U: Debug + Clone, { - let mem_size = PAGE_SIZE_USIZE; + let mem_size = page_size::get(); let test_read = |mem_size, offset| { let sm = shared_memory_new(mem_size)?; (reader)(&sm, offset) diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index f455dffa5..8a8f2f3a3 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -1538,9 +1538,8 @@ mod tests { } fn page_aligned_memory(src: &[u8]) -> GuestSharedMemory { - use hyperlight_common::mem::PAGE_SIZE_USIZE; - - let len = src.len().div_ceil(PAGE_SIZE_USIZE) * PAGE_SIZE_USIZE; + let page_size = page_size::get(); + let len = src.len().div_ceil(page_size) * page_size; let mut mem = ExclusiveSharedMemory::new(len).unwrap(); mem.copy_from_slice(src, 0).unwrap(); @@ -2647,15 +2646,16 @@ mod tests { }; // Use multi-page regions so partial overlap is geometrically possible - let mem1 = page_aligned_memory(&[0xAA; 8192]); // 2 pages - let mem2 = page_aligned_memory(&[0xBB; 8192]); // 2 pages + let ps = page_size::get(); + let mem1 = page_aligned_memory(&vec![0xAA; ps * 2]); // 2 pages + let mem2 = page_aligned_memory(&vec![0xBB; ps * 2]); // 2 pages let guest_base: usize = 0x200000000; let region1 = region_for_memory(&mem1, guest_base, MemoryRegionFlags::READ); unsafe { sbox.map_region(®ion1).unwrap() }; // region2 starts one page before region1, overlapping by one page - let overlap_base = guest_base - 0x1000; + let overlap_base = guest_base - ps; let region2 = region_for_memory(&mem2, overlap_base, MemoryRegionFlags::READ); let err = unsafe { sbox.map_region(®ion2) }.unwrap_err(); assert!( diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs index ca60c1963..10ad00006 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs @@ -451,11 +451,18 @@ impl OciSnapshotConfig { pt )); } - if (self.layout.snapshot_size as u64).saturating_add(pt as u64) != self.memory_size { + // The total memory size might be bigger because it has to + // take into account the host page size, as well as the guest + // page size. + let total_size = (self.layout.snapshot_size as u64) + .saturating_add(pt as u64) + .next_multiple_of(page_size::get() as u64); + if total_size != self.memory_size { return Err(crate::new_error!( - "snapshot snapshot_size ({}) + pt_size ({}) does not equal memory_size ({})", + "snapshot snapshot_size ({}) + pt_size ({}), rounded to {}, does not equal memory_size ({})", self.layout.snapshot_size, pt, + total_size, self.memory_size )); } diff --git a/src/hyperlight_host/src/sandbox/snapshot/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/mod.rs index e1578e2e7..792496906 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/mod.rs @@ -559,6 +559,11 @@ impl Snapshot { } pt_buf.set_root(pt_buf.initial_root()); + snapshot_memory.resize( + snapshot_memory.len().next_multiple_of(page_size::get()), + 0u8, + ); + // Phase 4: finalize PT bytes. let pt_data = pt_buf.into_bytes(); layout.set_pt_size(pt_data.len())?; @@ -573,7 +578,7 @@ impl Snapshot { // `map_file_cow` regions installed immediately after the // snapshot in guest PA space. let guest_visible_size = memory.len() - layout.get_pt_size(); - debug_assert!(guest_visible_size.is_multiple_of(PAGE_SIZE)); + debug_assert!(guest_visible_size.is_multiple_of(page_size::get())); layout.set_snapshot_size(guest_visible_size); Ok(Self { @@ -744,14 +749,16 @@ mod tests { CommonSpecialRegisters::default() } - const SIMPLE_PT_BASE: usize = PAGE_SIZE + SandboxMemoryLayout::BASE_ADDRESS; + fn simple_pt_base() -> usize { + page_size::get() + SandboxMemoryLayout::BASE_ADDRESS + } fn make_simple_pt_mem(contents: &[u8]) -> SnapshotSharedMemory { - let pt_buf = GuestPageTableBuffer::new(SIMPLE_PT_BASE); + let pt_buf = GuestPageTableBuffer::new(simple_pt_base()); let mapping = Mapping { phys_base: SandboxMemoryLayout::BASE_ADDRESS as u64, virt_base: SandboxMemoryLayout::BASE_ADDRESS as u64, - len: PAGE_SIZE as u64, + len: page_size::get() as u64, kind: MappingKind::Basic(BasicMapping { readable: true, writable: true, @@ -762,10 +769,10 @@ mod tests { super::map_specials(&pt_buf, PAGE_SIZE); let pt_bytes = pt_buf.into_bytes(); - let mut snapshot_mem = vec![0u8; PAGE_SIZE + pt_bytes.len()]; - snapshot_mem[0..PAGE_SIZE].copy_from_slice(contents); - snapshot_mem[PAGE_SIZE..].copy_from_slice(&pt_bytes); - ReadonlySharedMemory::from_bytes(&snapshot_mem, PAGE_SIZE) + let mut snapshot_mem = vec![0u8; page_size::get() + pt_bytes.len()]; + snapshot_mem[0..page_size::get()].copy_from_slice(contents); + snapshot_mem[page_size::get()..].copy_from_slice(&pt_bytes); + ReadonlySharedMemory::from_bytes(&snapshot_mem, page_size::get()) .unwrap() .to_mgr_snapshot_mem() .unwrap() @@ -776,12 +783,12 @@ mod tests { let scratch_mem = ExclusiveSharedMemory::new(cfg.get_scratch_size()).unwrap(); let mgr = SandboxMemoryManager::new( SandboxMemoryLayout::new(cfg, 4096, 0x3000, None).unwrap(), - make_simple_pt_mem(&[0u8; PAGE_SIZE]), + make_simple_pt_mem(&vec![0u8; page_size::get()]), scratch_mem, super::NextAction::None, ); let (mgr, _) = mgr.build().unwrap(); - (mgr, SIMPLE_PT_BASE as u64) + (mgr, simple_pt_base() as u64) } #[test] @@ -789,7 +796,7 @@ mod tests { let (mut mgr, pt_base) = make_simple_pt_mgr(); // Create first snapshot with pattern A - let pattern_a = vec![0xAA; PAGE_SIZE]; + let pattern_a = vec![0xAA; page_size::get()]; let snapshot_a = super::Snapshot::new( &mut make_simple_pt_mem(&pattern_a).build().0, &mut mgr.scratch_mem, @@ -809,7 +816,7 @@ mod tests { .unwrap(); // Create second snapshot with pattern B - let pattern_b = vec![0xBB; PAGE_SIZE]; + let pattern_b = vec![0xBB; page_size::get()]; let snapshot_b = super::Snapshot::new( &mut make_simple_pt_mem(&pattern_b).build().0, &mut mgr.scratch_mem, diff --git a/src/hyperlight_host/src/sandbox/snapshot/tripwires.rs b/src/hyperlight_host/src/sandbox/snapshot/tripwires.rs index ee04373a8..1744cde35 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/tripwires.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/tripwires.rs @@ -54,14 +54,15 @@ const _: () = { }; const _: () = { - use hyperlight_common::mem::{HyperlightPEB, PAGE_SIZE_USIZE}; + use hyperlight_common::mem::HyperlightPEB; + use hyperlight_common::vmem::PAGE_SIZE; // The loading host derives `guest_heap_buffer_offset`, and every // offset after it, from the PEB's page-rounded size. Existing // snapshots place the PEB in a single page, so the only thing that // must hold is that the PEB keeps fitting in one page. Field layout // is not pinned: only the captured guest reads the fields, and it // travels inside the snapshot, so it stays self-consistent. - abi_assert!(std::mem::size_of::() <= PAGE_SIZE_USIZE); + abi_assert!(std::mem::size_of::() <= PAGE_SIZE); }; const _: () = { From b4602422a73b201062a35656c70b5b82d88d62eb Mon Sep 17 00:00:00 2001 From: Lucy Menon <168595099+syntactically@users.noreply.github.com> Date: Wed, 15 Jul 2026 15:12:25 +0100 Subject: [PATCH 09/17] Change guest layout to support 16k host pages Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- CHANGELOG.md | 5 +++++ src/hyperlight_common/src/arch/aarch64/layout.rs | 2 +- src/hyperlight_host/src/mem/layout.rs | 15 +++++++++------ .../src/sandbox/snapshot/file/media_types.rs | 2 +- .../src/sandbox/snapshot/tripwires.rs | 4 ++-- src/hyperlight_host/tests/sandbox_host_tests.rs | 2 +- .../tests/snapshot_goldens/goldens_version.rs | 2 +- 7 files changed, 20 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 516928815..d4ac7a70e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). MSRs, on MSHV and WHP this is not enforced. by @ludfjig in https://github.com/hyperlight-dev/hyperlight/pull/991 * **Breaking:** Filesystem paths are now represented using `PathBuf`. `GuestBinary::FilePath` now stores a `PathBuf` instead of a `String`, and `MultiUseSandbox::generate_crashdump_to_dir` accepts `Into` instead of `Into`. Callers passing a `String` to `GuestBinary::FilePath` must convert it using `.into()`. +Certain fixed guest addresses were changed on AArch64 to more easily +accommodate 16k pages without wasting memory. Snapshots taken from +sandboxes using the old addresses will not be loadable by new +hyperlight versions. + ### Removed ### Fixed diff --git a/src/hyperlight_common/src/arch/aarch64/layout.rs b/src/hyperlight_common/src/arch/aarch64/layout.rs index cb32cfe8e..a8b6710c4 100644 --- a/src/hyperlight_common/src/arch/aarch64/layout.rs +++ b/src/hyperlight_common/src/arch/aarch64/layout.rs @@ -19,7 +19,7 @@ limitations under the License. pub const SCRATCH_TOP_GVA: usize = 0x0000_ffff_ffff_dfff; pub const SNAPSHOT_PT_GVA_MIN: usize = 0x0000_8000_0000_0000; pub const SNAPSHOT_PT_GVA_MAX: usize = 0x0000_80ff_ffff_ffff; -pub const SCRATCH_TOP_GPA: usize = 0x0000_000f_ffff_efff; +pub const SCRATCH_TOP_GPA: usize = 0x0000_000f_ffff_bfff; pub const IO_PAGE_GVA: u64 = 0x0000_ffff_ffff_e000; pub const IO_PAGE_GPA: u64 = 0x0000_000f_ffff_f000; diff --git a/src/hyperlight_host/src/mem/layout.rs b/src/hyperlight_host/src/mem/layout.rs index 9662be731..3bc615cb4 100644 --- a/src/hyperlight_host/src/mem/layout.rs +++ b/src/hyperlight_host/src/mem/layout.rs @@ -360,7 +360,7 @@ impl SandboxMemoryLayout { pub(crate) const MAX_MEMORY_SIZE: usize = (16 * 1024 * 1024 * 1024) - Self::BASE_ADDRESS; // 16 GiB - BASE_ADDRESS /// The base address of the sandbox's memory. - pub(crate) const BASE_ADDRESS: usize = 0x1000; + pub(crate) const BASE_ADDRESS: usize = 0x4000; // the offset into a sandbox's input/output buffer where the stack starts pub(crate) const STACK_POINTER_SIZE_BYTES: u64 = 8; @@ -774,7 +774,7 @@ mod tests { expected_size += layout.heap_size.next_multiple_of(PAGE_SIZE); - expected_size + expected_size.next_multiple_of(page_size::get()) } #[test] @@ -891,7 +891,7 @@ mod tests { ); pin_eq!( hyperlight_common::layout::SCRATCH_TOP_GPA, - 0x0000_000f_ffff_efff + 0x0000_000f_ffff_bfff ); } @@ -915,7 +915,7 @@ mod tests { pin_eq!(layout.guest_code_offset(), 0); pin_eq!(layout.peb_offset(), 0x1000); - pin_eq!(layout.peb_address(), 0x2000); + pin_eq!(layout.peb_address(), 0x5000); pin_eq!(layout.guest_heap_buffer_offset(), 0x2000); pin_eq!(layout.init_data_offset(), 0x4000); pin_eq!(layout.get_memory_size().unwrap(), 0x4000); @@ -964,10 +964,13 @@ mod tests { pin_eq!(layout.guest_code_offset(), 0); pin_eq!(layout.peb_offset(), 0x3000); - pin_eq!(layout.peb_address(), 0x4000); + pin_eq!(layout.peb_address(), 0x7000); pin_eq!(layout.guest_heap_buffer_offset(), 0x4000); pin_eq!(layout.init_data_offset(), 0x9000); - pin_eq!(layout.get_memory_size().unwrap(), 0x9000); + pin_eq!( + layout.get_memory_size().unwrap(), + 0x9000_usize.next_multiple_of(page_size::get()) + ); pin_eq!(layout.get_scratch_size(), 0x20000); pin_eq!(layout.get_pt_size(), 0); diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/media_types.rs b/src/hyperlight_host/src/sandbox/snapshot/file/media_types.rs index 661bd4a04..347e1e8b6 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/media_types.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/media_types.rs @@ -27,7 +27,7 @@ pub(in crate::sandbox::snapshot) const MT_SNAPSHOT_CURRENT: &str = MT_SNAPSHOT_V /// ABI version for the snapshot memory blob. Bumped when the /// host-guest contract for the snapshot bytes changes. See /// docs/snapshot-versioning.md. -pub(in crate::sandbox::snapshot) const SNAPSHOT_ABI_VERSION: u32 = 1; +pub(in crate::sandbox::snapshot) const SNAPSHOT_ABI_VERSION: u32 = 2; /// OCI standard annotation key for a manifest's tag inside an image /// index. Set on the manifest descriptor in `index.json`, not on the diff --git a/src/hyperlight_host/src/sandbox/snapshot/tripwires.rs b/src/hyperlight_host/src/sandbox/snapshot/tripwires.rs index 1744cde35..f8cd3c9bb 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/tripwires.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/tripwires.rs @@ -28,7 +28,7 @@ use super::file::{ MT_CONFIG_CURRENT, MT_SNAPSHOT_CURRENT, OCI_LAYOUT_VERSION, SNAPSHOT_ABI_VERSION, }; -const EXPECTED_ABI_VERSION: u32 = 1; +const EXPECTED_ABI_VERSION: u32 = 2; const EXPECTED_MT_CONFIG: &str = "application/vnd.hyperlight.snapshot.config.v1+json"; const EXPECTED_MT_SNAPSHOT: &str = "application/vnd.hyperlight.snapshot.memory.v1"; const EXPECTED_OCI_LAYOUT_VERSION: &str = "1.0.0"; @@ -87,7 +87,7 @@ const _: () = { const _: () = { use crate::mem::layout::SandboxMemoryLayout; - abi_assert!(SandboxMemoryLayout::BASE_ADDRESS == 0x1000); + abi_assert!(SandboxMemoryLayout::BASE_ADDRESS == 0x4000); }; const fn str_eq(a: &str, b: &str) -> bool { diff --git a/src/hyperlight_host/tests/sandbox_host_tests.rs b/src/hyperlight_host/tests/sandbox_host_tests.rs index b1a1a9918..723b51f05 100644 --- a/src/hyperlight_host/tests/sandbox_host_tests.rs +++ b/src/hyperlight_host/tests/sandbox_host_tests.rs @@ -123,7 +123,7 @@ fn invalid_guest_function_name() { #[test] fn set_static() { let mut cfg: SandboxConfiguration = Default::default(); - cfg.set_scratch_size(0x100A000); + cfg.set_scratch_size(0x100C000); with_all_sandboxes_cfg(Some(cfg), |mut sandbox| { let fn_name = "SetStatic"; let res = sandbox.call::(fn_name, ()); diff --git a/src/hyperlight_host/tests/snapshot_goldens/goldens_version.rs b/src/hyperlight_host/tests/snapshot_goldens/goldens_version.rs index 32572f417..c0b6c0c24 100644 --- a/src/hyperlight_host/tests/snapshot_goldens/goldens_version.rs +++ b/src/hyperlight_host/tests/snapshot_goldens/goldens_version.rs @@ -21,7 +21,7 @@ limitations under the License. //! publish. See `docs/snapshot-versioning.md`. /// Goldens version, a `vMAJOR.MINOR` string. -pub(crate) const GOLDENS_VERSION: &str = "v1.0"; +pub(crate) const GOLDENS_VERSION: &str = "v2.0"; /// Old majors kept loadable through a compatibility path, verified /// alongside `GOLDENS_VERSION`. A backwards-compatible break (Option 2) From d4f30aeb44d4d7460e9cb5a4183a782662a4eea1 Mon Sep 17 00:00:00 2001 From: Lucy Menon <168595099+syntactically@users.noreply.github.com> Date: Mon, 10 Aug 2026 21:10:44 +0100 Subject: [PATCH 10/17] Make VirtualMachine::set_{regs,sregs,fpu} take &mut These conceptually mutate the state of the guest, so it makes sense that they should take &mut. This is useful for HVF, which stores a copy of the regs when they are set, and in this manner can avoid an extra Rust lock. Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- .../src/hypervisor/hyperlight_vm/aarch64.rs | 2 +- .../src/hypervisor/hyperlight_vm/x86_64.rs | 2 +- .../src/hypervisor/virtual_machine/kvm/aarch64.rs | 6 +++--- .../src/hypervisor/virtual_machine/kvm/x86_64.rs | 9 ++++++--- .../src/hypervisor/virtual_machine/mod.rs | 9 ++++++--- .../src/hypervisor/virtual_machine/mshv/x86_64.rs | 9 ++++++--- .../src/hypervisor/virtual_machine/whp.rs | 9 ++++++--- 7 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs index 82f3f9420..bd93b48f4 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs @@ -67,7 +67,7 @@ impl HyperlightVm { #[cfg(hvf)] let interrupt_handle: Arc = Arc::new(HvfInterruptHandle::new(config.get_interrupt_retry_delay())); - let vm: VmType = match get_available_hypervisor() { + let mut vm: VmType = match get_available_hypervisor() { #[cfg(kvm)] Some(HypervisorType::Kvm) => Box::new(KvmVm::new().map_err(VmError::CreateVm)?), // TODO: mshv support diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs index 530612b97..82d21454c 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs @@ -81,7 +81,7 @@ impl HyperlightVm { #[cfg(crashdump)] rt_cfg: SandboxRuntimeConfig, #[cfg(feature = "mem_profile")] trace_info: MemTraceInfo, ) -> std::result::Result { - let vm: BoxedVm = match get_available_hypervisor() { + let mut vm: BoxedVm = match get_available_hypervisor() { #[cfg(kvm)] Some(HypervisorType::Kvm) => { let kvm_vm = KvmVm::new().map_err(VmError::CreateVm)?; diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/aarch64.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/aarch64.rs index 07c6db953..162e4f678 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/aarch64.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/aarch64.rs @@ -286,7 +286,7 @@ impl VirtualMachine for KvmVm { }) } - fn set_regs(&self, regs: &CommonRegisters) -> std::result::Result<(), RegisterError> { + fn set_regs(&mut self, regs: &CommonRegisters) -> std::result::Result<(), RegisterError> { use crate::hypervisor::regs::kvm_reg::{PC, PSTATE, SP, X}; for (i, xi) in X.iter().enumerate() { xi.set(RegisterError::SetSregs, &self.vcpu_fd, regs.x[i])?; @@ -311,7 +311,7 @@ impl VirtualMachine for KvmVm { }) } - fn set_fpu(&self, fpu: &CommonFpu) -> Result<(), RegisterError> { + fn set_fpu(&mut self, fpu: &CommonFpu) -> Result<(), RegisterError> { use crate::hypervisor::regs::kvm_reg::{FPCR, FPSR, V}; for (i, vi) in V.iter().enumerate() { vi.set(RegisterError::SetFpu, &self.vcpu_fd, fpu.v[i])?; @@ -336,7 +336,7 @@ impl VirtualMachine for KvmVm { }) } - fn set_sregs(&self, sregs: &CommonSpecialRegisters) -> Result<(), RegisterError> { + fn set_sregs(&mut self, sregs: &CommonSpecialRegisters) -> Result<(), RegisterError> { use crate::hypervisor::regs::kvm_reg::{ CPACR_EL1, MAIR_EL1, SCTLR_EL1, SP_EL1, TCR_EL1, TTBR0_EL1, VBAR_EL1, }; diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs index 4343a0ee1..b6a499fa1 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs @@ -455,7 +455,7 @@ impl VirtualMachine for KvmVm { Ok((&kvm_regs).into()) } - fn set_regs(&self, regs: &CommonRegisters) -> std::result::Result<(), RegisterError> { + fn set_regs(&mut self, regs: &CommonRegisters) -> std::result::Result<(), RegisterError> { let kvm_regs: kvm_regs = regs.into(); self.vcpu_fd .set_regs(&kvm_regs) @@ -473,7 +473,7 @@ impl VirtualMachine for KvmVm { Ok((&kvm_fpu).into()) } - fn set_fpu(&self, fpu: &CommonFpu) -> std::result::Result<(), RegisterError> { + fn set_fpu(&mut self, fpu: &CommonFpu) -> std::result::Result<(), RegisterError> { let kvm_fpu: kvm_fpu = fpu.into(); // Note: On KVM this ignores MXCSR. // See https://github.com/torvalds/linux/blob/d358e5254674b70f34c847715ca509e46eb81e6f/arch/x86/kvm/x86.c#L12554-L12599 @@ -491,7 +491,10 @@ impl VirtualMachine for KvmVm { Ok((&kvm_sregs).into()) } - fn set_sregs(&self, sregs: &CommonSpecialRegisters) -> std::result::Result<(), RegisterError> { + fn set_sregs( + &mut self, + sregs: &CommonSpecialRegisters, + ) -> std::result::Result<(), RegisterError> { let kvm_sregs: kvm_sregs = sregs.into(); self.vcpu_fd .set_sregs(&kvm_sregs) diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs index a673c02f9..89864608f 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs @@ -432,17 +432,20 @@ pub(crate) trait VirtualMachine: Debug + Send { #[allow(dead_code)] fn regs(&self) -> std::result::Result; /// Set regs - fn set_regs(&self, regs: &CommonRegisters) -> std::result::Result<(), RegisterError>; + fn set_regs(&mut self, regs: &CommonRegisters) -> std::result::Result<(), RegisterError>; /// Get fpu regs #[allow(dead_code)] fn fpu(&self) -> std::result::Result; /// Set fpu regs - fn set_fpu(&self, fpu: &CommonFpu) -> std::result::Result<(), RegisterError>; + fn set_fpu(&mut self, fpu: &CommonFpu) -> std::result::Result<(), RegisterError>; /// Get special regs #[allow(dead_code)] fn sregs(&self) -> std::result::Result; /// Set special regs - fn set_sregs(&self, sregs: &CommonSpecialRegisters) -> std::result::Result<(), RegisterError>; + fn set_sregs( + &mut self, + sregs: &CommonSpecialRegisters, + ) -> std::result::Result<(), RegisterError>; /// Get the debug registers of the vCPU #[allow(dead_code)] fn debug_regs(&self) -> std::result::Result; diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/mshv/x86_64.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/mshv/x86_64.rs index 5c6232089..24407c164 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/mshv/x86_64.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/mshv/x86_64.rs @@ -457,7 +457,7 @@ impl VirtualMachine for MshvVm { Ok((&mshv_regs).into()) } - fn set_regs(&self, regs: &CommonRegisters) -> std::result::Result<(), RegisterError> { + fn set_regs(&mut self, regs: &CommonRegisters) -> std::result::Result<(), RegisterError> { let mshv_regs: StandardRegisters = regs.into(); self.vcpu_fd .set_regs(&mshv_regs) @@ -473,7 +473,7 @@ impl VirtualMachine for MshvVm { Ok((&mshv_fpu).into()) } - fn set_fpu(&self, fpu: &CommonFpu) -> std::result::Result<(), RegisterError> { + fn set_fpu(&mut self, fpu: &CommonFpu) -> std::result::Result<(), RegisterError> { let mshv_fpu: FloatingPointUnit = fpu.into(); self.vcpu_fd .set_fpu(&mshv_fpu) @@ -489,7 +489,10 @@ impl VirtualMachine for MshvVm { Ok((&mshv_sregs).into()) } - fn set_sregs(&self, sregs: &CommonSpecialRegisters) -> std::result::Result<(), RegisterError> { + fn set_sregs( + &mut self, + sregs: &CommonSpecialRegisters, + ) -> std::result::Result<(), RegisterError> { let mshv_sregs: SpecialRegisters = sregs.into(); self.vcpu_fd .set_sregs(&mshv_sregs) diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs index 22cc967be..f9557d4d5 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs @@ -709,7 +709,7 @@ impl VirtualMachine for WhpVm { }) } - fn set_regs(&self, regs: &CommonRegisters) -> std::result::Result<(), RegisterError> { + fn set_regs(&mut self, regs: &CommonRegisters) -> std::result::Result<(), RegisterError> { let whp_regs: [(WHV_REGISTER_NAME, Align16); WHP_REGS_NAMES_LEN] = regs.into(); self.set_registers(&whp_regs) @@ -738,7 +738,7 @@ impl VirtualMachine for WhpVm { }) } - fn set_fpu(&self, fpu: &CommonFpu) -> std::result::Result<(), RegisterError> { + fn set_fpu(&mut self, fpu: &CommonFpu) -> std::result::Result<(), RegisterError> { let whp_fpu: [(WHV_REGISTER_NAME, Align16); WHP_FPU_NAMES_LEN] = fpu.into(); self.set_registers(&whp_fpu) @@ -767,7 +767,10 @@ impl VirtualMachine for WhpVm { }) } - fn set_sregs(&self, sregs: &CommonSpecialRegisters) -> std::result::Result<(), RegisterError> { + fn set_sregs( + &mut self, + sregs: &CommonSpecialRegisters, + ) -> std::result::Result<(), RegisterError> { let whp_regs: [(WHV_REGISTER_NAME, Align16); WHP_SREGS_NAMES_LEN] = sregs.into(); From b6fb97c4254763522104c0e8a928692ccd447f4d Mon Sep 17 00:00:00 2001 From: Lucy Menon <168595099+syntactically@users.noreply.github.com> Date: Mon, 20 Jul 2026 15:24:18 +0100 Subject: [PATCH 11/17] [tests] hvf: support identifiers and adjust for hvf This adds new variants of the existing create_200_sandboxes test that stress-test the behaviour of the hvf code in situations with many more threads than vcpus. It also adjusts one test which relies on the preservation of system register state whose preservation Hyperlight does not guarantee; while that state is preserved on other backends (up to a snapshot restore) it can be lost on hvf if the vcpu is destroyed and recreated. Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- .../src/sandbox/initialized_multi_use.rs | 47 +++++++++++++++---- .../src/sandbox/snapshot/file_tests.rs | 3 +- src/hyperlight_host/tests/integration_test.rs | 4 +- .../tests/sandbox_host_tests.rs | 3 +- 4 files changed, 43 insertions(+), 14 deletions(-) diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index 8a8f2f3a3..2f7dc2ba5 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -1418,11 +1418,7 @@ mod tests { } } - #[test] - fn create_200_sandboxes() { - const NUM_THREADS: usize = 10; - const SANDBOXES_PER_THREAD: usize = 20; - + fn create_many_on_threads_test() { // barrier to make sure all threads start their work simultaneously let start_barrier = Arc::new(Barrier::new(NUM_THREADS + 1)); let mut thread_handles = vec![]; @@ -1455,6 +1451,21 @@ mod tests { } } + #[test] + fn create_200_sandboxes() { + create_many_on_threads_test::<20, 10>(); + } + + #[test] + fn create_200_threads() { + create_many_on_threads_test::<200, 1>(); + } + + #[test] + fn create_2000_sandboxes() { + create_many_on_threads_test::<200, 10>(); + } + #[test] fn test_mmap() { let mut sbox = @@ -1899,11 +1910,27 @@ mod tests { // bits that are reserved in aarch64 DBGBVR0_EL1 const DIRTY_VALUE: u64 = 0xFFFF_FEDC_7654_3210; sandbox.call::<()>("SetDr0", DIRTY_VALUE).unwrap(); - let dr0_dirty: u64 = sandbox.call("GetDr0", ()).unwrap(); - assert_eq!( - dr0_dirty, DIRTY_VALUE, - "DR0 should be dirty after SetDr0 call" - ); + + // Validate that DR0 was in fact dirtied + #[cfg(not(hvf))] + { + // This check does not work on hvf, because it relies on + // state being persisted across sandbox calls in a system + // register that is not usually supported by Hyperlight + // (DBGBVR0), whereas hvf may (if there is a lot of + // contention on the system) destroy and re-create its + // vcpu, preserving only the "supported" hyperlight state + // msrs. + // + // We could disable this test entirely on hvf, but a test + // that occasionally checks for what it is meant to is + // probably better than one that never does. + let dr0_dirty: u64 = sandbox.call("GetDr0", ()).unwrap(); + assert_eq!( + dr0_dirty, DIRTY_VALUE, + "DR0 should be dirty after SetDr0 call" + ); + } // Restore to the snapshot - this should reset vCPU state including debug registers sandbox.restore(snapshot).unwrap(); diff --git a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs index 4e0604c86..f22809c1f 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file_tests.rs @@ -657,6 +657,7 @@ fn cfg_current_hypervisor() -> &'static str { "kvm" => "kvm", "mshv" => "mshv", "whp" => "whp", + "hvf" => "hvf", other => panic!("unknown hypervisor tag {other}"), } } @@ -1234,7 +1235,7 @@ fn save_appends_into_existing_layout_with_new_tag() { ); let hv = anns["dev.hyperlight.snapshot.hypervisor"].as_str().unwrap(); assert!( - ["kvm", "mshv", "whp"].contains(&hv), + ["kvm", "mshv", "whp", "hvf"].contains(&hv), "unexpected hypervisor annotation: {}", hv ); diff --git a/src/hyperlight_host/tests/integration_test.rs b/src/hyperlight_host/tests/integration_test.rs index 24db9134a..60b21706e 100644 --- a/src/hyperlight_host/tests/integration_test.rs +++ b/src/hyperlight_host/tests/integration_test.rs @@ -1472,11 +1472,11 @@ fn interrupt_infinite_moving_loop_stress_test() { use std::thread; // We have a high thread count to stress test and to have interesting interleavings - const NUM_THREADS: usize = 200; + let num_threads: usize = 200; let mut handles = vec![]; - for _ in 0..NUM_THREADS { + for _ in 0..num_threads { handles.push(thread::spawn(move || { let entered_guest = Arc::new(AtomicBool::new(false)); let entered_guest_clone = entered_guest.clone(); diff --git a/src/hyperlight_host/tests/sandbox_host_tests.rs b/src/hyperlight_host/tests/sandbox_host_tests.rs index 723b51f05..5e6209650 100644 --- a/src/hyperlight_host/tests/sandbox_host_tests.rs +++ b/src/hyperlight_host/tests/sandbox_host_tests.rs @@ -327,7 +327,8 @@ fn callback_test() { #[test] fn callback_test_parallel() { - let handles: Vec<_> = (0..100) + let n_threads = 100; + let handles: Vec<_> = (0..n_threads) .map(|_| { std::thread::spawn(|| { callback_test_helper(); From 2939956199fcc27ba8e26e717dba1119a995faa1 Mon Sep 17 00:00:00 2001 From: Lucy Menon <168595099+syntactically@users.noreply.github.com> Date: Mon, 20 Jul 2026 17:52:52 +0100 Subject: [PATCH 12/17] Restrict usage of MIDR_EL1 to Linux Other operating systems do not support its use in EL0 Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- src/hyperlight_host/src/sandbox/snapshot/file/config.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs index 10ad00006..609479fcf 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/file/config.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/file/config.rs @@ -130,7 +130,7 @@ impl CpuVendor { bytes[8..12].copy_from_slice(&r.ecx.to_le_bytes()); Self(String::from_utf8_lossy(&bytes).into_owned()) } - #[cfg(all(target_arch = "aarch64"))] + #[cfg(all(target_arch = "aarch64", target_os = "linux"))] { let midr: u64; // SAFETY: Linux emulates MIDR_EL1 reads from EL0. @@ -139,6 +139,10 @@ impl CpuVendor { // `0x` prefix padded to width 4, e.g. Apple `0x61`, Arm `0x41`. Self(format!("{implementer:#04x}")) } + #[cfg(all(target_arch = "aarch64", target_os = "macos"))] + { + Self("0x61".to_string()) + } } pub(super) fn as_str(&self) -> &str { @@ -751,6 +755,8 @@ mod tests { #[cfg(all(target_arch = "aarch64", target_os = "linux"))] // MIDR_EL1 implementer byte for Apple silicon. assert_eq!(v, "0x61", "unexpected aarch64 CPU implementer"); + #[cfg(all(target_arch = "aarch64", target_os = "macos"))] + assert_eq!(v, "0x61", "unexpected aarch64 CPU implementer"); } /// The architecture the current host is not running. From 4ac8c030bed8a8b1ed2a31101a87c2dd625269eb Mon Sep 17 00:00:00 2001 From: Lucy Menon <168595099+syntactically@users.noreply.github.com> Date: Mon, 20 Jul 2026 17:57:09 +0100 Subject: [PATCH 13/17] Initial single-address-space support for Hypervisor.framework Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- Cargo.lock | 1 + src/hyperlight_host/Cargo.toml | 1 + .../src/hypervisor/hyperlight_vm/aarch64.rs | 6 +- .../src/hypervisor/virtual_machine/hvf/mod.rs | 1223 ++++++++++++++++- .../src/hypervisor/virtual_machine/mod.rs | 32 + src/hyperlight_host/src/mem/shared_mem.rs | 11 +- typos.toml | 2 + 7 files changed, 1270 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 015ce934b..6f6a323d2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1707,6 +1707,7 @@ dependencies = [ "opentelemetry-semantic-conventions", "opentelemetry_sdk", "page_size", + "parking_lot", "proc-maps", "proptest", "rand 0.10.2", diff --git a/src/hyperlight_host/Cargo.toml b/src/hyperlight_host/Cargo.toml index 4073b4226..3735febc0 100644 --- a/src/hyperlight_host/Cargo.toml +++ b/src/hyperlight_host/Cargo.toml @@ -56,6 +56,7 @@ oci-spec = { version = "0.10", default-features = false, features = ["image"] } sha2 = "0.11" hex = "0.4" tempfile = "3.27.0" +parking_lot = "0.12.5" [target.'cfg(windows)'.dependencies] windows = { version = "0.62", features = [ diff --git a/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs b/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs index bd93b48f4..daa018425 100644 --- a/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs +++ b/src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs @@ -31,6 +31,8 @@ use crate::hypervisor::LinuxInterruptHandle; use crate::hypervisor::gdb::{DebugCommChannel, DebugMsg, DebugResponse}; use crate::hypervisor::hyperlight_vm::get_guest_log_filter; use crate::hypervisor::regs::{CommonFpu, CommonRegisters, CommonSpecialRegisters}; +#[cfg(hvf)] +use crate::hypervisor::virtual_machine::hvf::HvfVm; #[cfg(kvm)] use crate::hypervisor::virtual_machine::kvm::KvmVm; use crate::hypervisor::virtual_machine::{ @@ -74,7 +76,9 @@ impl HyperlightVm { #[cfg(mshv3)] Some(HypervisorType::Mshv) => return Err(CreateHyperlightVmError::NoHypervisorFound), #[cfg(hvf)] - Some(HypervisorType::Hvf) => return Err(CreateHyperlightVmError::NoHypervisorFound), + Some(HypervisorType::Hvf) => { + Box::new(HvfVm::new(interrupt_handle.clone()).map_err(VmError::CreateVm)?) + } None => return Err(CreateHyperlightVmError::NoHypervisorFound), }; vm.set_sregs(&CommonSpecialRegisters::defaults(root_pt_addr)) diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/hvf/mod.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/hvf/mod.rs index e0c405abf..b5552b897 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/hvf/mod.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/hvf/mod.rs @@ -12,7 +12,84 @@ distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. -*/ + */ + +//! # Bridging Hyperlight's assumptions with Hypervisor.framework +//! +//! Hypervisor.framework has some constraints that run counter to the +//! flexibility provided by the usual Hyperlight API. In particular, +//! hvf assumes that there is only <= 1 VM per process, and <= 1 VCPU +//! per thread. +//! +//! ## Supporting more than one Sandbox per process +//! +//! This is not yet implemented, but we plan to support >1 sandbox per +//! process by using nested virtualisation on platforms where it is +//! available, making each sandbox a nested guest VM. This does, +//! unfortunately, of course have some performance implications. +//! +//! ## Supporting [`core::marker::Send`] on Sandboxes +//! +//! The Hyperlight public API constraints sandboxes (and, by +//! extension, hypervisor API implementations) to implement +//! [`core::marker::Send`]. Other hypervisors have one vCPU but allow +//! the vCPU handle to be migrated across threads, although this comes +//! with severe performance impact in some cases (e.g. KVM). +//! +//! Unfortunately, this does not work on hvf, since cross-thread +//! access is prohibited (most `hv_vcpu_` functions note that "This +//! function must be called by the owning thread") rather than merely +//! unperformant. +//! +//! There are, largely, two approaches that we could use to work +//! around this. We could either: +//! +//! 1. Create a dedicated vcpu thread, and implement sandbox +//! operations as RPCs to that thread +//! 2. Create one vcpu per thread-from-which-a-Sandbox-is-used, and +//! reset that vcpu's state to match the correct sandbox whenever a +//! sandbox operation is called. +//! +//! A long time ago, Hyperlight briefly unconditionally used approach +//! (1) on all hypervisors, but it had unacceptable performance impact +//! on most of them. +//! +//! Although an implementation of (1) scoped just to hvf could make +//! sense, this module presently implements (2), on the rationale that +//! it ought to be /possible/ for a host application to get decent +//! performance out of (2) (if the host application exercises some +//! discipline and uses a 1:1 mapping between sandboxes and threads; +//! although there is some unavoidable overhead due to needing to sync +//! registers on every VM exit, unfortunately), but an implementation +//! based on (1) would have to create the thread up-front (not knowing +//! if the sandbox would in fact be Sent to another thread) and so +//! would impose unavoidable overhead on all consumers. +//! +//! Applications which wish to use multiple sandboxes per thread, or +//! multiple threads per sandbox, and to have decent performance on +//! Hypervisor.framework, should consider (and benchmark!) the +//! alternative architecture of keeping the sandbox itself on a single +//! thread and pass data/make RPCs to/from that thread. + +use core::cell::RefCell; +use core::ffi; +use core::sync::atomic::{AtomicU64, Ordering}; +use std::sync::{Arc, LazyLock}; +use std::time::Duration; + +use hyperlight_common::outb::VmAction; +use parking_lot::{Condvar, Mutex, RwLock, RwLockWriteGuard}; + +use super::{ + CreateVmError, HvfSyncError, HypervisorError, MapMemoryError, MemorySpaceInstallError, + RegisterError, ResetVcpuError, RunVcpuError, UnmapMemoryError, VirtualMachine, VmExit, +}; +use crate::hypervisor::InterruptHandleImpl; +use crate::hypervisor::regs::{ + CommonDebugRegs, CommonFpu, CommonRegisters, CommonSpecialRegisters, +}; +use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags, MemoryRegionType}; +use crate::mem::shared_mem::{ReadonlySharedMemory, SharedMemoryError}; #[allow( dead_code, @@ -22,8 +99,1150 @@ limitations under the License. )] // bindgen pub(crate) mod bindings { include!(concat!(env!("OUT_DIR"), "/hvf_bindings.rs")); + impl hv_return_t { + pub(super) fn is_success(&self) -> Result<(), super::HypervisorError> { + if self.0.0.0 == HV_SUCCESS { + Ok(()) + } else { + Err(super::HypervisorError::HvfError(*self)) + } + } + pub(super) fn unless_success( + &self, + f: impl Fn(super::HypervisorError) -> T, + ) -> Result<(), T> { + self.is_success().map_err(f) + } + } + impl core::fmt::Display for hv_return_t { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> { + write!(f, "0x{:x}", self.0.0.0) + } + } + // the default base-10 signed printout is totally useless + impl core::fmt::Debug for hv_return_t { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> { + write!(f, "0x{:x}", self.0.0.0) + } + } } pub(super) fn is_hypervisor_present() -> bool { - false + let mut val: ffi::c_int = 0; + let mut len: usize = core::mem::size_of::(); + let ret = unsafe { + libc::sysctlbyname( + c"kern.hv_support".as_ptr(), + &raw mut val as *mut ffi::c_void, + &raw mut len, + std::ptr::null_mut(), + 0, + ) + }; + ret == 0 && val == 1 +} + +// Used to figure out when registers have been updated since the last +// time that the sandbox ran on this vcpu & need to be sync'd back. +#[derive(Clone, Copy, Default, Debug)] +struct EpochStamped { + value: T, + epoch: u64, +} + +#[derive(Clone, Copy, PartialEq, Debug)] +struct SandboxId(u64); + +static MAX_CPUS: LazyLock = LazyLock::new(|| { + let mut count: u32 = 0; + if unsafe { bindings::hv_vm_get_max_vcpu_count(&mut count) } + .is_success() + .is_ok() + { + count as u64 + } else { + // If this simple query was not successful, we may not be + // likely to have much more success with anything + // else. However, since signalling an error from here is + // somewhat awkward, we instead return 1, allowing code to try + // to create 1 but not assume anything about how many are + // supported. + 1u64 + } +}); +static CPUS_CREATED: Mutex = Mutex::new(0); +static CPUS_NOTIFIER: Condvar = Condvar::new(); + +struct HvfCpu { + id: bindings::hv_vcpu_t, + exit: *mut bindings::hv_vcpu_exit_t, + current_loaded: Option>, + + // reset_vcpu() needs to destroy the current vcpu before it can + // create the new one, but when it creates the new one and assigns + // it to the thread-local, the HvfCpu for the old one will be + // dropped. This lets the Drop implementation know that that has + // happened and avoid destroying the cpu a second time. + destroyed_in_reset_vcpu: bool, +} + +#[derive(Debug)] +enum TimeoutableError { + Error(E), + Timeout, +} +impl From for TimeoutableError { + fn from(e: E) -> Self { + TimeoutableError::Error(e) + } +} +trait IntoTimeoutableError: Sized { + fn itme(self) -> Result>; +} +impl> IntoTimeoutableError for Result { + fn itme(self) -> Result> { + self.map_err(|e| TimeoutableError::Error(>::from(e))) + } +} + +type VcpuCreateError = TimeoutableError; + +impl HvfCpu { + /// Do just the core FFI operations to create a new vcpu, without + /// checking or modifying [`CPUS_CREATED`]. Used in [`reset_vcpu`] + /// below, where the current thread's vcpu has already been + /// accounted for. + fn new_unconditional() -> Result { + use core::mem::MaybeUninit; + let mut vcpu: MaybeUninit = MaybeUninit::zeroed(); + let mut exit: *mut bindings::hv_vcpu_exit_t = core::ptr::null_mut(); + unsafe { + let config = bindings::hv_vcpu_config_create(); + let ret = bindings::hv_vcpu_create(vcpu.as_mut_ptr(), &raw mut exit, config); + bindings::os_release(config.0 as *mut core::ffi::c_void); + ret + } + .is_success()?; + Ok(Self { + id: unsafe { vcpu.assume_init() }, + exit, + current_loaded: None, + destroyed_in_reset_vcpu: false, + }) + } + /// Must be called only once per thread. + /// + /// Returns the cpu and a boolean indicating whether it's wise to + /// keep this cpu around for a nontrivial amount of time. + fn new() -> Result<(Self, bool), VcpuCreateError> { + let mut nr = CPUS_CREATED.lock(); + let max = *MAX_CPUS; + if CPUS_NOTIFIER + .wait_while_for(&mut nr, |nr| *nr >= max, Duration::from_millis(10)) + .timed_out() + { + return Err(TimeoutableError::Timeout); + } + let prev_nr = *nr; + + // This doesn't really need to be serialised, and most of it + // could be moved outside of the lock if that becomes a + // performance problem. It's inside for now to avoid having to + // acquire the lock a second time to decrement the count if it + // fails. + let cpu = Self::new_unconditional()?; + + *nr += 1; + drop(nr); + + // This could be a lot smarter, but at least try to make sure + // that there is always at least 1 vcpu available. + let should_keep = prev_nr < max - 1; + Ok((cpu, should_keep)) + } + + fn reset_vcpu(&mut self) -> Result<(), HypervisorError> { + // TODO: figure out if there is a more efficient way to clear + // all the state + if !self.destroyed_in_reset_vcpu { + unsafe { bindings::hv_vcpu_destroy(self.id) }.is_success()?; + self.destroyed_in_reset_vcpu = true; + } + *self = HvfCpu::new_unconditional()?; + Ok(()) + } + + fn hv_vcpu_get_reg(&self, reg: bindings::hv_reg_t) -> Result { + let mut value: u64 = 0; + unsafe { bindings::hv_vcpu_get_reg(self.id, reg, &raw mut value) }.is_success()?; + Ok(value) + } + + fn hv_vcpu_set_reg( + &mut self, + reg: bindings::hv_reg_t, + value: u64, + ) -> Result<(), HypervisorError> { + unsafe { bindings::hv_vcpu_set_reg(self.id, reg, value) }.is_success() + } + + fn hv_vcpu_get_simd_fp_reg( + &self, + reg: bindings::hv_simd_fp_reg_t, + ) -> Result { + let mut value: [u8; 16] = [0; 16]; + unsafe { + bindings::hv_vcpu_get_simd_fp_reg_rsabi(self.id, reg, value.as_mut_ptr() as *mut i8) + } + .is_success()?; + Ok(u128::from_ne_bytes(value)) + } + + fn hv_vcpu_set_simd_fp_reg( + &mut self, + reg: bindings::hv_simd_fp_reg_t, + value: u128, + ) -> Result<(), HypervisorError> { + let bytes: [u8; 16] = value.to_ne_bytes(); + unsafe { + bindings::hv_vcpu_set_simd_fp_reg_rsabi(self.id, reg, bytes.as_ptr() as *const i8) + } + .is_success() + } + + fn hv_vcpu_get_sys_reg(&self, reg: bindings::hv_sys_reg_t) -> Result { + let mut value: u64 = 0; + unsafe { bindings::hv_vcpu_get_sys_reg(self.id, reg, &raw mut value) }.is_success()?; + Ok(value) + } + + fn hv_vcpu_set_sys_reg( + &mut self, + reg: bindings::hv_sys_reg_t, + value: u64, + ) -> Result<(), HypervisorError> { + unsafe { bindings::hv_vcpu_set_sys_reg(self.id, reg, value) }.is_success() + } + + fn set_vcpu_regs(&mut self, regs: &CommonRegisters) -> Result<(), HypervisorError> { + use bindings::{hv_reg_t, hv_sys_reg_t}; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X0, regs.x[0])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X1, regs.x[1])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X2, regs.x[2])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X3, regs.x[3])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X4, regs.x[4])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X5, regs.x[5])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X6, regs.x[6])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X7, regs.x[7])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X8, regs.x[8])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X9, regs.x[9])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X10, regs.x[10])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X11, regs.x[11])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X12, regs.x[12])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X13, regs.x[13])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X14, regs.x[14])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X15, regs.x[15])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X16, regs.x[16])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X17, regs.x[17])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X18, regs.x[18])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X19, regs.x[19])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X20, regs.x[20])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X21, regs.x[21])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X22, regs.x[22])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X23, regs.x[23])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X24, regs.x[24])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X25, regs.x[25])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X26, regs.x[26])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X27, regs.x[27])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X28, regs.x[28])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X29, regs.x[29])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_X30, regs.x[30])?; + // set SP_EL0 or SP_EL1 depending on SPSel + if regs.pstate & 0x1 == 0x1 { + self.hv_vcpu_set_sys_reg(hv_sys_reg_t::HV_SYS_REG_SP_EL1, regs.sp)?; + } else { + self.hv_vcpu_set_sys_reg(hv_sys_reg_t::HV_SYS_REG_SP_EL0, regs.sp)?; + } + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_PC, regs.pc)?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_CPSR, regs.pstate)?; + Ok(()) + } + + fn get_vcpu_regs(&self) -> Result { + use bindings::{hv_reg_t, hv_sys_reg_t}; + let pstate = self.hv_vcpu_get_reg(hv_reg_t::HV_REG_CPSR)?; + Ok(CommonRegisters { + x: [ + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X0)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X1)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X2)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X3)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X4)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X5)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X6)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X7)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X8)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X9)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X10)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X11)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X12)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X13)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X14)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X15)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X16)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X17)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X18)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X19)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X20)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X21)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X22)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X23)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X24)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X25)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X26)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X27)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X28)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X29)?, + self.hv_vcpu_get_reg(hv_reg_t::HV_REG_X30)?, + ], + sp: if pstate & 0x1 == 0x1 { + self.hv_vcpu_get_sys_reg(hv_sys_reg_t::HV_SYS_REG_SP_EL1)? + } else { + self.hv_vcpu_get_sys_reg(hv_sys_reg_t::HV_SYS_REG_SP_EL0)? + }, + pc: self.hv_vcpu_get_reg(hv_reg_t::HV_REG_PC)?, + pstate, + }) + } + + fn set_vcpu_fpregs(&mut self, regs: &CommonFpu) -> Result<(), HypervisorError> { + use bindings::{hv_reg_t, hv_simd_fp_reg_t}; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q0, regs.v[0])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q1, regs.v[1])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q2, regs.v[2])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q3, regs.v[3])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q4, regs.v[4])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q5, regs.v[5])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q6, regs.v[6])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q7, regs.v[7])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q8, regs.v[8])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q9, regs.v[9])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q10, regs.v[10])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q11, regs.v[11])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q12, regs.v[12])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q13, regs.v[13])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q14, regs.v[14])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q15, regs.v[15])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q16, regs.v[16])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q17, regs.v[17])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q18, regs.v[18])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q19, regs.v[19])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q20, regs.v[20])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q21, regs.v[21])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q22, regs.v[22])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q23, regs.v[23])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q24, regs.v[24])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q25, regs.v[25])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q26, regs.v[26])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q27, regs.v[27])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q28, regs.v[28])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q29, regs.v[29])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q30, regs.v[30])?; + self.hv_vcpu_set_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q31, regs.v[31])?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_FPSR, regs.fpsr as u64)?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_FPCR, regs.fpcr as u64)?; + Ok(()) + } + + fn get_vcpu_fpregs(&mut self) -> Result { + use bindings::{hv_reg_t, hv_simd_fp_reg_t}; + Ok(CommonFpu { + v: [ + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q0)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q1)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q2)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q3)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q4)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q5)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q6)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q7)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q8)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q9)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q10)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q11)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q12)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q13)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q14)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q15)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q16)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q17)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q18)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q19)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q20)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q21)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q22)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q23)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q24)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q25)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q26)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q27)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q28)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q29)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q30)?, + self.hv_vcpu_get_simd_fp_reg(hv_simd_fp_reg_t::HV_SIMD_FP_REG_Q31)?, + ], + fpsr: self.hv_vcpu_get_reg(hv_reg_t::HV_REG_FPSR)? as u32, + fpcr: self.hv_vcpu_get_reg(hv_reg_t::HV_REG_FPCR)? as u32, + }) + } + + fn set_vcpu_sregs(&mut self, regs: &CommonSpecialRegisters) -> Result<(), HypervisorError> { + use bindings::hv_sys_reg_t; + self.hv_vcpu_set_sys_reg(hv_sys_reg_t::HV_SYS_REG_TTBR0_EL1, regs.ttbr0_el1)?; + self.hv_vcpu_set_sys_reg(hv_sys_reg_t::HV_SYS_REG_TCR_EL1, regs.tcr_el1)?; + self.hv_vcpu_set_sys_reg(hv_sys_reg_t::HV_SYS_REG_MAIR_EL1, regs.mair_el1)?; + self.hv_vcpu_set_sys_reg(hv_sys_reg_t::HV_SYS_REG_SCTLR_EL1, regs.sctlr_el1)?; + self.hv_vcpu_set_sys_reg(hv_sys_reg_t::HV_SYS_REG_CPACR_EL1, regs.cpacr_el1)?; + self.hv_vcpu_set_sys_reg(hv_sys_reg_t::HV_SYS_REG_VBAR_EL1, regs.vbar_el1)?; + self.hv_vcpu_set_sys_reg(hv_sys_reg_t::HV_SYS_REG_SP_EL1, regs.sp_el1)?; + Ok(()) + } + + fn get_vcpu_sregs(&self) -> Result { + use bindings::hv_sys_reg_t; + Ok(CommonSpecialRegisters { + ttbr0_el1: self.hv_vcpu_get_sys_reg(hv_sys_reg_t::HV_SYS_REG_TTBR0_EL1)?, + tcr_el1: self.hv_vcpu_get_sys_reg(hv_sys_reg_t::HV_SYS_REG_TCR_EL1)?, + mair_el1: self.hv_vcpu_get_sys_reg(hv_sys_reg_t::HV_SYS_REG_MAIR_EL1)?, + sctlr_el1: self.hv_vcpu_get_sys_reg(hv_sys_reg_t::HV_SYS_REG_SCTLR_EL1)?, + cpacr_el1: self.hv_vcpu_get_sys_reg(hv_sys_reg_t::HV_SYS_REG_CPACR_EL1)?, + vbar_el1: self.hv_vcpu_get_sys_reg(hv_sys_reg_t::HV_SYS_REG_VBAR_EL1)?, + sp_el1: self.hv_vcpu_get_sys_reg(hv_sys_reg_t::HV_SYS_REG_SP_EL1)?, + }) + } + + fn sync_state_from( + &mut self, + sandbox_id: EpochStamped, + regs: &EpochStamped, + sregs: &EpochStamped, + fpregs: &EpochStamped, + ) -> Result { + let (regs_dirty, fpregs_dirty, sregs_dirty) = match self.current_loaded { + None => (true, true, true), + Some(s) => { + if sandbox_id.value != s.value || sandbox_id.epoch > s.epoch { + self.reset_vcpu().map_err(HvfSyncError::ResetVcpu)?; + (true, true, true) + } else { + ( + regs.epoch > s.epoch, + fpregs.epoch > s.epoch, + sregs.epoch > s.epoch, + ) + } + } + }; + if regs_dirty { + self.set_vcpu_regs(®s.value) + .map_err(RegisterError::SetRegs)?; + } + if fpregs_dirty { + self.set_vcpu_fpregs(&fpregs.value) + .map_err(RegisterError::SetFpu)?; + } + if sregs_dirty { + self.set_vcpu_sregs(&sregs.value) + .map_err(RegisterError::SetSregs)?; + } + let sync_epoch = core::cmp::max(sandbox_id.epoch, core::cmp::max(regs.epoch, sregs.epoch)); + self.current_loaded = Some(EpochStamped { + value: sandbox_id.value, + epoch: sync_epoch, + }); + Ok(sync_epoch) + } + + fn sync_state_from_vm(&mut self, vm: &mut HvfVm) -> Result<(), HvfSyncError> { + let epoch = self.sync_state_from(vm.id, &vm.regs, &vm.sregs, &vm.fpu)?; + // update epoch eagerly, since at this point the vcpu + // epoch has been updated to the max, so if we returned early (e.g. due + // to an error in the next call(s)) it would be possible + // to miss updates + vm.id.epoch = epoch; + vm.regs.epoch = epoch; + vm.fpu.epoch = epoch; + vm.sregs.epoch = epoch; + Ok(()) + } + + fn sync_state_to_vm(&mut self, vm: &mut HvfVm) -> Result<(), HvfSyncError> { + let Some(EpochStamped { + value: sandbox_id, + epoch, + }) = self.current_loaded + else { + // sync_state_to_vm is always used just after + // sync_state_from_vm, so this should be impossible + debug_assert!(false); + return Err(HvfSyncError::SyncInvariant( + "Missing loaded sandbox".to_string(), + )); + }; + if sandbox_id != vm.id.value { + // sync_state_to_vm is always used just after + // sync_state_from_vm, so this should be impossible + debug_assert!(false); + return Err(HvfSyncError::SyncInvariant( + "Wrong loaded sandbox".to_string(), + )); + } + vm.id.epoch = epoch; + vm.regs = EpochStamped { + value: self.get_vcpu_regs().map_err(RegisterError::GetRegs)?, + epoch, + }; + vm.fpu = EpochStamped { + value: self.get_vcpu_fpregs().map_err(RegisterError::GetFpu)?, + epoch, + }; + vm.sregs = EpochStamped { + value: self.get_vcpu_sregs().map_err(RegisterError::GetSregs)?, + epoch, + }; + Ok(()) + } + + fn advance_pc(&mut self) -> Result<(), HypervisorError> { + use bindings::hv_reg_t; + let old_pc = self.hv_vcpu_get_reg(hv_reg_t::HV_REG_PC)?; + self.hv_vcpu_set_reg(hv_reg_t::HV_REG_PC, old_pc + 4)?; + Ok(()) + } + + fn run(&mut self) -> Result { + let ret = unsafe { bindings::hv_vcpu_run(self.id) }; + if let Some(EpochStamped { ref mut epoch, .. }) = self.current_loaded { + *epoch += 1; + } + ret.is_success()?; + let exit = unsafe { self.exit.read() }; + + use bindings::hv_exit_reason_t; + let hl_exit = match exit.reason { + hv_exit_reason_t::HV_EXIT_REASON_CANCELED => VmExit::Cancelled(), + hv_exit_reason_t::HV_EXIT_REASON_EXCEPTION => { + let esr = exit.exception.syndrome.0; + let ipa = exit.exception.physical_address.0; + + use hyperlight_common::arch::exn::{ + DataFault, DataFaultInstructionSyndrome, DataFaultKind, Exception, + decode_syndrome, + }; + match decode_syndrome(esr) { + Exception::DataFault(DataFault { + is_write, + is_s1ptw: false, + kind: DataFaultKind::TranslationFault(_) | DataFaultKind::PermissionFault(_), + insn, + .. + }) => { + // For MMIO exits, always resume after the + // faulting instruction to match kvm behaviour + self.advance_pc()?; + + if is_write { + let io_page_gpa = + const { hyperlight_common::layout::io_page().unwrap().0 }; + if ipa >= io_page_gpa + && let off = (ipa - io_page_gpa) as usize + && off < hyperlight_common::vmem::PAGE_SIZE + // Hyperlight only uses load/stores + // that should have ISV=1, and never + // uses xzr/sp + && let Some(DataFaultInstructionSyndrome { + srt + }) = insn + { + let port = off / core::mem::size_of::(); + if port == VmAction::Halt as usize { + VmExit::Halt() + } else { + let data = if srt < 31 { + self.get_vcpu_regs()?.x[srt as usize] + } else { + 0 + }; + VmExit::IoOut(port as u16, data.to_ne_bytes().to_vec()) + } + } else { + VmExit::MmioWrite(ipa) + } + } else { + VmExit::MmioRead(ipa) + } + } + _ => VmExit::Unknown(format!( + "Unknown HVF vcpu exit ESR_EL2: {:16x} IPA {:16x}", + esr, ipa, + )), + } + } + reason => VmExit::Unknown(format!("Unknown HVF vcpu exit reason: {}", reason.0)), + }; + Ok(hl_exit) + } +} + +impl Drop for HvfCpu { + fn drop(&mut self) { + unsafe { + if !self.destroyed_in_reset_vcpu { + bindings::hv_vcpu_destroy(self.id); + *CPUS_CREATED.lock() -= 1; + let _ = CPUS_NOTIFIER.notify_one(); + }; + } + } +} + +std::thread_local! { + static HVF_VCPU: RefCell> = const { RefCell::new(None) }; +} + +#[derive(Debug)] +pub(crate) struct HvfVm { + id: EpochStamped, + regs: EpochStamped, + fpu: EpochStamped, + sregs: EpochStamped, + memory_space: MemorySpace, + interrupt_handle: Arc, +} + +/// This lock only ever goes from false to true, so there are probably +/// more efficient state machines. OnceLock::<()>::get_or_try_init +/// would probably provide the correct semantics, but is unstable :( +static HV_VM_CREATED: Mutex = Mutex::new(false); + +impl HvfVm { + pub(crate) fn new( + interrupt_handle: Arc, + ) -> Result { + static NEXT_AVAILABLE_ID: AtomicU64 = AtomicU64::new(0); + // If the vm for this process has not yet been created, create it + let mut created = HV_VM_CREATED.lock(); + if !*created { + unsafe { + let cfg = bindings::hv_vm_config_create(); + let ret = bindings::hv_vm_create(cfg); + bindings::os_release(cfg.0 as *mut core::ffi::c_void); + ret + } + .unless_success(CreateVmError::CreateVmFd)?; + *created = true; + } + drop(created); + Ok(Self { + id: EpochStamped { + value: SandboxId(NEXT_AVAILABLE_ID.fetch_add(1, Ordering::Relaxed)), + epoch: 0, + }, + regs: Default::default(), + fpu: Default::default(), + sregs: Default::default(), + memory_space: MemorySpace::new(), + interrupt_handle, + }) + } +} + +impl From for bindings::hv_memory_flags_t { + fn from(mrf: MemoryRegionFlags) -> bindings::hv_memory_flags_t { + let mut flags: bindings::hv_memory_flags_t = 0; + if mrf.contains(MemoryRegionFlags::READ) { + flags |= bindings::HV_MEMORY_READ as bindings::hv_memory_flags_t; + } + if mrf.contains(MemoryRegionFlags::WRITE) { + flags |= bindings::HV_MEMORY_WRITE as bindings::hv_memory_flags_t; + } + if mrf.contains(MemoryRegionFlags::EXECUTE) { + flags |= bindings::HV_MEMORY_EXEC as bindings::hv_memory_flags_t; + } + flags + } +} + +struct TlbiRegion { + insn_memory: ReadonlySharedMemory, +} +impl TlbiRegion { + fn new() -> Result { + let mut bytes = vec![0; page_size::get()]; + bytes[0..20].copy_from_slice(&[ + 0x9f, 0x3b, 0x03, 0xd5, // dsb ish + 0x1f, 0x87, 0x08, 0xd5, // tlbi vmalle1 + 0x9f, 0x3b, 0x03, 0xd5, // dsb ish + 0xdf, 0x3f, 0x03, 0xd5, // isb sy + 0x02, 0x00, 0x00, 0xd4, // hvc #0 + ]); + Ok(Self { + insn_memory: ReadonlySharedMemory::from_bytes(&bytes, page_size::get())?, + }) + } +} +impl From<&TlbiRegion> for MemoryRegion { + fn from(t: &TlbiRegion) -> MemoryRegion { + t.insn_memory + .mapping_at(page_size::get() as u64, MemoryRegionType::Snapshot) + } +} +struct LoadedMemorySpace { + space_id: Option, + mappings: Vec>, + /// This needs to keep track of the last vcpu used, as well as the + /// space installed. Suppose that we have a flow like: + /// - vCPU 0 on CPU 0: Load memory space 0 + /// - vCPU 1 on CPU 0: Load memory space 1 + /// - vCPU 1 on CPU 1: Load memory space 0 + /// - vCPU 0 on CPU 0: Load memory space 0 + /// + /// then on the final operation we need a tlbi on vCPU 0, even + /// though the last-loaded-space matches, because the CPU TLB may + /// still contain incorrect values that it resolved from the wrong + /// memory space during an earlier and/or prefetching table walk. + /// + /// This means that we can only use the currently loaded space + /// without a tlb if we are running on a vcpu whose last + /// tlbi/creation operation was after the last change to the + /// loaded memory space. + /// + /// TODO[before push]: we should probably do the optimisation of + /// updating this to be a Vec and setting it when + /// creating/destroying vcpus. + valid_vcpu: Option, + /// For the memory region with a TLBI in it used in tlbi_vmalle1 + /// below, we really want the semantics of a global + /// [`OnceLock`]. Unfortunately, [`OnceLock::get_or_try_init`] is + /// not stable, so we can't use it, so we would have to use an + /// [`RwLock`] or [`Mutex`] instead. However, the region is only + /// used from [`CURRENT_LOADED_MEMORY_SPACE`], which is also the + /// only [`LoadedMemorySpace`] object to exist, and which is + /// locked. So, we just carry it along inside the loaded memory + /// space singleton, even though it doesn't exactly conceptually + /// belong to it. + tlbi_region: Option, +} +impl LoadedMemorySpace { + const fn new() -> Self { + Self { + space_id: None, + mappings: Vec::new(), + valid_vcpu: None, + tlbi_region: None, + } + } + fn do_map(region: &MemoryRegion) -> Result<(), HypervisorError> { + unsafe { + bindings::hv_vm_map( + region.host_region.start as *mut core::ffi::c_void, + bindings::hv_ipa_t(region.guest_region.start as u64), + region.guest_region.end - region.guest_region.start, + region.flags.into(), + ) + } + .is_success() + } + fn do_unmap(region: &MemoryRegion) -> Result<(), HypervisorError> { + unsafe { + bindings::hv_vm_unmap( + bindings::hv_ipa_t(region.guest_region.start as u64), + region.guest_region.end - region.guest_region.start, + ) + } + .is_success() + } + fn update_mapping( + &mut self, + slot: usize, + region: Option, + ) -> Result<(), HypervisorError> { + if self.mappings.len() <= slot { + self.mappings.resize(slot + 1, None); + } + if let Some(ref old_region) = self.mappings[slot] { + Self::do_unmap(old_region)?; + } + if let Some(ref new_region) = region { + Self::do_map(new_region)?; + } + self.mappings[slot] = region; + Ok(()) + } + fn get_tlbi_region(&mut self) -> Result<&'_ TlbiRegion, SharedMemoryError> { + Ok(if let Some(ref rgn) = self.tlbi_region { + rgn + } else { + self.tlbi_region.get_or_insert(TlbiRegion::new()?) + }) + } + fn tlbi_vmalle1( + &mut self, + cpu: &mut HvfCpu, + ) -> Result<(), TimeoutableError> { + // Save some state that we will scribble over in a moment + use bindings::{hv_reg_t, hv_sys_reg_t}; + let orig_pstate = cpu.hv_vcpu_get_reg(hv_reg_t::HV_REG_CPSR).itme()?; + let orig_pc = cpu.hv_vcpu_get_reg(hv_reg_t::HV_REG_PC).itme()?; + let orig_sctlr_el1 = cpu + .hv_vcpu_get_sys_reg(hv_sys_reg_t::HV_SYS_REG_SCTLR_EL1) + .itme()?; + + let rgn: MemoryRegion = self.get_tlbi_region().itme()?.into(); + Self::do_map(&rgn).itme()?; + // Save the fact that we've done this mapping, in case + // we get interrupted + let tlbi_pc = rgn.guest_region.start as u64; + self.mappings = vec![Some(rgn)]; + + let ret: Result< + Result<_, TimeoutableError>, + TimeoutableError, + > = { + // Separate errors/cancellations in the tlbi process from + // errors in the state save/restore process + let ret = (|| { + // Make sure that interrupts are disabled and we're in EL1(t, + // but that part doesn't matter) + cpu.hv_vcpu_set_reg(hv_reg_t::HV_REG_CPSR, 0b11 << 6 | 0b100) + .itme()?; + cpu.hv_vcpu_set_reg(hv_reg_t::HV_REG_PC, tlbi_pc).itme()?; + // Disable SCTLR_EL1.{M,C,I} + cpu.hv_vcpu_set_sys_reg( + hv_sys_reg_t::HV_SYS_REG_SCTLR_EL1, + crate::hypervisor::regs::SCTLR_EL1_RES1, + ) + .itme()?; + + // We don't use `HvfVcpu::run` because we don't need to update + // the vcpu epoch and we don't need its processing of + // Hyperlight vmexits---we use our own here. + unsafe { bindings::hv_vcpu_run(cpu.id) } + .is_success() + .itme()?; + let exit = unsafe { cpu.exit.read() }; + + use bindings::hv_exit_reason_t; + match exit.reason { + hv_exit_reason_t::HV_EXIT_REASON_EXCEPTION + if exit.exception.syndrome.0 == 0x5a000000 => {} // hvc #0 is the success case + hv_exit_reason_t::HV_EXIT_REASON_CANCELED => { + return Err(TimeoutableError::Timeout); + } + _ => { + return Err(TimeoutableError::Error( + MemorySpaceInstallError::UnexpectedExit(exit), + )); + } + }; + + // The access/unwrap can't panic, since we just set it above. + Self::do_unmap(self.mappings[0].as_ref().unwrap()).itme()?; + self.mappings = Vec::new(); + Ok(()) + })(); + + // Even if the actual tlbi failed or was cancelled, + // unconditionally try to restore the state that we saved + cpu.hv_vcpu_set_reg(hv_reg_t::HV_REG_CPSR, orig_pstate) + .itme()?; + cpu.hv_vcpu_set_reg(hv_reg_t::HV_REG_PC, orig_pc).itme()?; + cpu.hv_vcpu_set_sys_reg(hv_sys_reg_t::HV_SYS_REG_SCTLR_EL1, orig_sctlr_el1) + .itme()?; + Ok(ret) + }; + + if ret.is_err() { + // If there was a failure in state restoration, the vcpu + // isn't valid for this vm anymore + cpu.current_loaded = None; + } + ret? + } + fn valid_for_vcpu(&self, cpu: bindings::hv_vcpu_t) -> bool { + match self.valid_vcpu { + None => false, + Some(valid_cpu) => valid_cpu.0 == cpu.0, + } + } + #[allow(clippy::collapsible_if, clippy::manual_flatten)] + fn sync_space( + &mut self, + space: &MemorySpace, + cpu: &mut HvfCpu, + ) -> Result<(), TimeoutableError> { + if self.space_id == Some(space.id) && self.valid_for_vcpu(cpu.id) { + return Ok(()); + } + self.space_id = None; + self.valid_vcpu = None; + for mapping in &mut self.mappings { + if let Some(region) = mapping { + Self::do_unmap(region).itme()?; + *mapping = None; + } + } + self.mappings = Vec::new(); + self.tlbi_vmalle1(cpu)?; + for mapping in &space.mappings { + if let Some(region) = mapping { + Self::do_map(region).itme()? + } + self.mappings.push(mapping.clone()); + } + self.valid_vcpu = Some(cpu.id); + self.space_id = Some(space.id); + Ok(()) + } +} +// This is an RwLock because we use a writable lock when actually +// using the space to run a VM, and a readable lock when checking if +// anyone else is using it. +static CURRENT_LOADED_MEMORY_SPACE: RwLock = + RwLock::new(LoadedMemorySpace::new()); +#[derive(Debug)] +struct MemorySpace { + id: u64, + mappings: Vec>, +} +struct MemorySpaceInstalledGuard<'a> { + _loaded_mutex_guard: RwLockWriteGuard<'a, LoadedMemorySpace>, +} +impl MemorySpace { + fn new() -> Self { + static NEXT_AVAILABLE_ID: AtomicU64 = AtomicU64::new(0); + Self { + id: NEXT_AVAILABLE_ID.fetch_add(1, Ordering::Relaxed), + mappings: Vec::new(), + } + } + + /// This function is used both as a performance optimisation when + /// this space is not being swapped out) and in order to allow + /// [`unmap_memory`] to guarantee that the region is no longer in + /// use in the kernel when it returns. + fn opportunistically_sync_slot(&mut self, slot: usize) -> Result<(), HypervisorError> { + // If the lock is currently being held exclusively, then it + // must be held by a different memory space (since we have an + // &mut self reference, and the write-side of the lock is only + // taken here and by a codepath that also uses an &mut self + // reference and does not cover any calls to us), so we only + // need to opportunistically try to take it. + // + // Unlike pthread_mutex_trylock, + // ParkingLot::RwLock::try_read() does not guarantee that a + // failure to lock is due to a write lock being already + // held. This event is rare in practice, and we only hold the + // read side of the lock for an extremely short amount of + // time, so in practice we spin until we either get the read + // lock or see an exclusive write lock appear. + // + // (Even Mutex, which one really might expect to have that + // property, does not: it uses a `compare_exchange_weak` test + // to attempt to take the lock, which is allowed to spuriously + // fail (due to e.g. concurrent accesses) even if the value in + // memory is indeed the same. This means that it is possible + // for `parking_lot::Mutex::try_lock()` to fail, but for the + // lock to never have been acquired.) + if let Some(mut current) = loop { + match CURRENT_LOADED_MEMORY_SPACE.try_upgradable_read() { + Some(guard) => break Some(guard), + None if CURRENT_LOADED_MEMORY_SPACE.is_locked_exclusive() => break None, + _ => continue, + } + } && current.space_id == Some(self.id) + { + // This can't block for long: the only place that read + // locks are taken is here, and every other reader will + // bail out quickly when it sees that its id does not + // match. Given parking_lot's fairness guarantees, it + // should be impossible for this to block for long enough + // to need to have a timeout. + current.with_upgraded(|current| { + current.update_mapping(slot, self.mappings[slot].clone()) + })?; + } + Ok(()) + } + + fn map_memory(&mut self, region: (u32, &MemoryRegion)) -> Result<(), HypervisorError> { + let slot = region.0 as usize; + if self.mappings.len() <= slot { + self.mappings.resize(slot + 1, None); + } + self.mappings[slot] = Some(region.1.clone()); + self.opportunistically_sync_slot(slot) + } + + fn unmap_memory(&mut self, region: (u32, &MemoryRegion)) -> Result<(), HypervisorError> { + let slot = region.0 as usize; + if self.mappings.len() <= slot { + self.mappings.resize(slot + 1, None); + } + self.mappings[slot] = None; + self.opportunistically_sync_slot(slot) + } + + fn install_in_cpu( + &mut self, + cpu: &mut HvfCpu, + ) -> Result, TimeoutableError> { + let mut guard = CURRENT_LOADED_MEMORY_SPACE + .try_write_for(Duration::from_millis(10)) + .ok_or(TimeoutableError::Timeout)?; + guard.sync_space(self, cpu)?; + Ok(MemorySpaceInstalledGuard { + _loaded_mutex_guard: guard, + }) + } +} + +impl VirtualMachine for HvfVm { + unsafe fn map_memory( + &mut self, + region: (u32, &MemoryRegion), + ) -> std::result::Result<(), MapMemoryError> { + self.memory_space + .map_memory(region) + .map_err(MapMemoryError::Hypervisor) + } + + fn unmap_memory( + &mut self, + region: (u32, &MemoryRegion), + ) -> std::result::Result<(), UnmapMemoryError> { + self.memory_space + .unmap_memory(region) + .map_err(UnmapMemoryError::Hypervisor) + } + + fn run_vcpu( + &mut self, + #[cfg(feature = "trace_guest")] tc: &mut SandboxTraceContext, + ) -> std::result::Result { + macro_rules! retry_until_timeout { + ($e:expr) => { + loop { + let e = $e; + match e { + Ok(x) => break Ok(x), + Err(TimeoutableError::Error(e)) => break Err(e), + Err(TimeoutableError::Timeout) => { + drop(e); + let (_, cancel, debug) = + self.interrupt_handle.state().get_running_cancel_debug(); + if cancel || debug { + return Ok(VmExit::Cancelled()); + } else { + continue; + } + } + } + } + }; + } + HVF_VCPU.with_borrow_mut(|thread_vcpu| { + let (vcpu, should_keep) = match thread_vcpu { + None => { + let (vcpu, should_keep) = retry_until_timeout!(HvfCpu::new()) + .map_err(HvfSyncError::CreateVcpu) + .map_err(RunVcpuError::HvfSync)?; + (thread_vcpu.insert(vcpu), should_keep) + } + Some(v) => (v, true), + }; + + let ret = (|| { + vcpu.sync_state_from_vm(self) + .map_err(RunVcpuError::HvfSync)?; + // tlbi can invoke the cpu, so make sure the interrupt + // handle is set up before it + self.interrupt_handle.set_vcpu(Some(vcpu.id)); + let space_installed_guard = + retry_until_timeout!(self.memory_space.install_in_cpu(vcpu)) + .map_err(HvfSyncError::MemorySpace) + .map_err(RunVcpuError::HvfSync)?; + + let exit = vcpu.run().map_err(RunVcpuError::Unknown); + drop(space_installed_guard); + // Do a little dance to make sure that we call + // sync_state_to_vm even if vcpu.run() has reported an + // error, since vcpu.run() updated the epoch of the + // cpu even if it did encounter an error + let sync_err = vcpu.sync_state_to_vm(self).map_err(RunVcpuError::HvfSync); + let exit = exit?; + sync_err?; + + Ok(exit) + })(); + + self.interrupt_handle.set_vcpu(None); + if !should_keep { + *thread_vcpu = None; + } + ret + }) + } + + fn regs(&self) -> std::result::Result { + Ok(self.regs.value) + } + + fn set_regs(&mut self, regs: &CommonRegisters) -> std::result::Result<(), RegisterError> { + self.regs.value = *regs; + self.regs.epoch += 1; + Ok(()) + } + + fn fpu(&self) -> std::result::Result { + Ok(self.fpu.value) + } + + fn set_fpu(&mut self, fpu: &CommonFpu) -> std::result::Result<(), RegisterError> { + self.fpu.value = *fpu; + self.fpu.epoch += 1; + Ok(()) + } + + fn sregs(&self) -> std::result::Result { + Ok(self.sregs.value) + } + + fn set_sregs( + &mut self, + sregs: &CommonSpecialRegisters, + ) -> std::result::Result<(), RegisterError> { + self.sregs.value = *sregs; + self.sregs.epoch += 1; + Ok(()) + } + + fn debug_regs(&self) -> std::result::Result { + todo!() + } + + fn set_debug_regs(&self, _drs: &CommonDebugRegs) -> std::result::Result<(), RegisterError> { + todo!() + } + + #[cfg(target_arch = "aarch64")] + fn can_reset_vcpu(&self) -> bool { + true + } + + #[cfg(target_arch = "aarch64")] + fn reset_vcpu(&mut self) -> std::result::Result<(), ResetVcpuError> { + self.id.epoch += 1; + Ok(()) + } } diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs index 89864608f..611a9e956 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs @@ -255,6 +255,9 @@ pub enum RunVcpuError { #[cfg(target_arch = "aarch64")] #[error("Flush MMIO pending state failed: {0}")] FlushMmioPending(String), + #[cfg(hvf)] + #[error("HVF sync error: {0}")] + HvfSync(HvfSyncError), #[error("Unknown error: {0}")] Unknown(HypervisorError), } @@ -396,6 +399,35 @@ pub enum HypervisorError { #[cfg(target_os = "windows")] #[error("Windows error: {0}")] WindowsError(#[from] windows_result::Error), + #[cfg(hvf)] + #[error("HVF error: {0}")] + HvfError(hvf::bindings::hv_return_t), +} + +/// HVF-specific error synchronising vcpu state +#[cfg(hvf)] +#[derive(Debug, thiserror::Error)] +pub enum MemorySpaceInstallError { + #[error("Failed to update VM/VCPU state: {0}")] + Hypervisor(#[from] HypervisorError), + #[error("Unexpected VCPU exit: {0:?}")] + UnexpectedExit(hvf::bindings::hv_vcpu_exit_t), + #[error("Failed to allocate ReadonlySharedMemory: {0}")] + SharedMemoryCreation(#[from] crate::mem::shared_mem::SharedMemoryError), +} +#[cfg(hvf)] +#[derive(Debug, thiserror::Error)] +pub enum HvfSyncError { + #[error("Error creating VCPU: {0}")] + CreateVcpu(HypervisorError), + #[error("Error resetting VCPU: {0}")] + ResetVcpu(HypervisorError), + #[error("Error reading/writing registers: {0}")] + Register(#[from] RegisterError), + #[error("Error updating memory space: {0}")] + MemorySpace(#[from] MemorySpaceInstallError), + #[error("Invariant violation: vcpu in unexpected sync state: {0}")] + SyncInvariant(String), } /// Trait for single-vCPU VMs. Provides a common interface for basic VM operations. diff --git a/src/hyperlight_host/src/mem/shared_mem.rs b/src/hyperlight_host/src/mem/shared_mem.rs index 0632dc35e..a77d7e374 100644 --- a/src/hyperlight_host/src/mem/shared_mem.rs +++ b/src/hyperlight_host/src/mem/shared_mem.rs @@ -2757,13 +2757,18 @@ mod tests { } /// Returns true if `status` indicates the process died from a - /// memory access fault (SIGSEGV on unix, STATUS_ACCESS_VIOLATION - /// (or 0xDEAD) on Windows). + /// memory access fault (SIGBUS on macos, SIGSEGV on linux, + /// STATUS_ACCESS_VIOLATION (or 0xDEAD) on Windows). fn killed_by_access_violation(status: &std::process::ExitStatus) -> bool { #[cfg(unix)] { use std::os::unix::process::ExitStatusExt; - status.signal() == Some(libc::SIGSEGV) + let expected_signal = if cfg!(target_os = "macos") { + libc::SIGBUS + } else { + libc::SIGSEGV + }; + status.signal() == Some(expected_signal) } #[cfg(windows)] { diff --git a/typos.toml b/typos.toml index e4cd5f295..1e102e5a4 100644 --- a/typos.toml +++ b/typos.toml @@ -16,3 +16,5 @@ ist="ist" finitel="finitel" # writables as number of writable buffers writables="writables" +# itme is an acronym used in HVF support for "Into TiMoutable Error" +itme="itme" From 056ef1701f31765fb928f25e1bc19855ba1d5004 Mon Sep 17 00:00:00 2001 From: Lucy Menon <168595099+syntactically@users.noreply.github.com> Date: Mon, 20 Jul 2026 19:35:11 +0100 Subject: [PATCH 14/17] cargo: Add codesigning runner script for MacOS On MacOS, communicating with the hypervisor requires that a binary be signed with an "entitlement" `com.apple.security.hypervisor`. This commit adds a script that locally signs and then runs a binary, and configures Cargo to use that script to run tests/exmaples/etc. Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- .cargo/config.toml | 2 ++ dev/macos-entitlements.plist | 8 ++++++++ dev/macos-sign-and-run.sh | 6 ++++++ 3 files changed, 16 insertions(+) create mode 100644 .cargo/config.toml create mode 100644 dev/macos-entitlements.plist create mode 100755 dev/macos-sign-and-run.sh diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 000000000..b1c8f87b3 --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,2 @@ +[target.'cfg(target_os = "macos")'] +runner = "dev/macos-sign-and-run.sh" diff --git a/dev/macos-entitlements.plist b/dev/macos-entitlements.plist new file mode 100644 index 000000000..c2ef1a38b --- /dev/null +++ b/dev/macos-entitlements.plist @@ -0,0 +1,8 @@ + + + + + com.apple.security.hypervisor + + + diff --git a/dev/macos-sign-and-run.sh b/dev/macos-sign-and-run.sh new file mode 100755 index 000000000..cc9bcef93 --- /dev/null +++ b/dev/macos-sign-and-run.sh @@ -0,0 +1,6 @@ +#!/usr/bin/env bash +set -Eeuo pipefail + + +codesign -f -s - --entitlements "$(dirname "$0")/macos-entitlements.plist" "$1" +exec "$@" From 34b58764c5b1ad6f774abcc7ef50a5539f879f21 Mon Sep 17 00:00:00 2001 From: Lucy Menon <168595099+syntactically@users.noreply.github.com> Date: Sat, 1 Aug 2026 15:55:38 +0100 Subject: [PATCH 15/17] aarch64: Move machinery for decoding ESR_ELx to hyperlight-common It will be used in hyperlight-host in the Hypervisor.framework implementation. Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- src/hyperlight_common/src/arch/aarch64/exn.rs | 100 ++++++++++++++++++ src/hyperlight_common/src/lib.rs | 7 ++ src/hyperlight_common/src/vmem.rs | 2 +- .../src/arch/aarch64/exception/handle.rs | 81 +++----------- 4 files changed, 123 insertions(+), 67 deletions(-) create mode 100644 src/hyperlight_common/src/arch/aarch64/exn.rs diff --git a/src/hyperlight_common/src/arch/aarch64/exn.rs b/src/hyperlight_common/src/arch/aarch64/exn.rs new file mode 100644 index 000000000..f07d3acb0 --- /dev/null +++ b/src/hyperlight_common/src/arch/aarch64/exn.rs @@ -0,0 +1,100 @@ +/* +Copyright 2026 The Hyperlight Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. + */ + +use crate::vmem::bits; + +const ESR_EC_DATA_ABORT_LOWER_EL: u64 = 0b100100; +const ESR_EC_DATA_ABORT_SAME_EL: u64 = 0b100101; + +// some of the data in these is not used presently, but is logically +// part of the code being decoded & should be accounted for +#[allow(dead_code)] +#[derive(Debug, Copy, Clone)] +pub enum DataFaultKind { + TranslationFault(i64), + PermissionFault(i64), + Other(u64), +} +fn decode_data_fault_status_code(dfsc: u64) -> DataFaultKind { + if bits::<5, 2>(dfsc) == 0b0011 { + DataFaultKind::PermissionFault(bits::<1, 0>(dfsc) as i64) + } else if bits::<5, 2>(dfsc) == 0b0001 { + DataFaultKind::TranslationFault(bits::<1, 0>(dfsc) as i64) + } else if bits::<5, 2>(dfsc) == 0b1010 { + if bits::<1, 0>(dfsc) >= 2 { + DataFaultKind::TranslationFault(bits::<1, 0>(dfsc) as i64 - 4) + } else { + DataFaultKind::Other(dfsc) + } + } else { + DataFaultKind::Other(dfsc) + } +} + +#[derive(Debug, Copy, Clone)] +pub struct DataFaultInstructionSyndrome { + pub srt: u8, + // ... +} +fn decode_data_fault_instruction_syndrome(iss: u64) -> Option { + let isv = bits::<24, 24>(iss); + if isv != 0b1 { + return None; + } + Some(DataFaultInstructionSyndrome { + srt: bits::<20, 16>(iss) as u8, + }) +} + +#[derive(Debug, Copy, Clone)] +pub struct DataFault { + pub from_lower_el: bool, + pub is_s1ptw: bool, + pub is_write: bool, + pub kind: DataFaultKind, + pub insn: Option, +} + +fn decode_data_fault(from_lower_el: bool, iss: u64) -> DataFault { + DataFault { + from_lower_el, + is_s1ptw: bits::<7, 7>(iss) == 0b1, + is_write: bits::<6, 6>(iss) == 0b1, + kind: decode_data_fault_status_code(bits::<5, 0>(iss)), + insn: decode_data_fault_instruction_syndrome(iss), + } +} + +// some of the data in these is not used presently, but is logically +// part of the code being decoded & should be accounted for +#[allow(dead_code)] +#[derive(Debug, Copy, Clone)] +pub enum Exception { + /// lower el?, faulting address, status code + DataFault(DataFault), + Other(u64), +} +/// Decode the value of ESR_ELx into a nice enum. Also takes FAR_ELx, +/// which will be embedded in the structure if relevant. +pub fn decode_syndrome(esr: u64) -> Exception { + let ec = bits::<31, 26>(esr); + match ec { + ESR_EC_DATA_ABORT_LOWER_EL | ESR_EC_DATA_ABORT_SAME_EL => Exception::DataFault( + decode_data_fault(ec == ESR_EC_DATA_ABORT_LOWER_EL, bits::<24, 0>(esr)), + ), + _ => Exception::Other(esr), + } +} diff --git a/src/hyperlight_common/src/lib.rs b/src/hyperlight_common/src/lib.rs index f342cf842..aac487ff5 100644 --- a/src/hyperlight_common/src/lib.rs +++ b/src/hyperlight_common/src/lib.rs @@ -57,3 +57,10 @@ pub mod version_note; /// cbindgen:ignore pub mod virtq; + +/// cbindgen:ignore +pub mod arch { + #[cfg(target_arch = "aarch64")] + #[path = "aarch64/exn.rs"] + pub mod exn; +} diff --git a/src/hyperlight_common/src/vmem.rs b/src/hyperlight_common/src/vmem.rs index 272a85680..fc184a7b0 100644 --- a/src/hyperlight_common/src/vmem.rs +++ b/src/hyperlight_common/src/vmem.rs @@ -37,7 +37,7 @@ pub use arch::ATTR_INDEX_NORMAL; /// Utility function to extract an (inclusive on both ends) bit range /// from a quadword. #[inline(always)] -pub(in crate::vmem) fn bits(x: u64) -> u64 { +pub(crate) fn bits(x: u64) -> u64 { (x & ((1 << (HIGH_BIT + 1)) - 1)) >> LOW_BIT } diff --git a/src/hyperlight_guest_bin/src/arch/aarch64/exception/handle.rs b/src/hyperlight_guest_bin/src/arch/aarch64/exception/handle.rs index 28054ca13..bc600d064 100644 --- a/src/hyperlight_guest_bin/src/arch/aarch64/exception/handle.rs +++ b/src/hyperlight_guest_bin/src/arch/aarch64/exception/handle.rs @@ -15,6 +15,7 @@ limitations under the License. */ use core::fmt::Write; +use hyperlight_common::arch::exn::{DataFault, DataFaultKind, Exception, decode_syndrome}; use hyperlight_common::vmem::{ BasicMapping, CowMapping, MappingKind, PAGE_SIZE, PhysAddr, VirtAddr, }; @@ -26,67 +27,6 @@ use super::super::mrs; use super::types::*; use crate::HyperlightAbortWriter; -/// Utility function to extract an (inclusive on both ends) bit range -/// from a quadword. -#[inline(always)] -fn bits(x: u64) -> u64 { - (x & ((1 << (HIGH_BIT + 1)) - 1)) >> LOW_BIT -} - -const ESR_EC_DATA_ABORT_LOWER_EL: u64 = 0b100100; -const ESR_EC_DATA_ABORT_SAME_EL: u64 = 0b100101; - -// some of the data in these is not used presently, but is logically -// part of the code being decoded & should be accounted for -#[allow(dead_code)] -#[derive(Debug, Copy, Clone)] -enum DataFault { - TranslationFault(i64), - PermissionFault(i64), - Other(u64), -} -fn decode_data_fault(dfsc: u64) -> DataFault { - if bits::<5, 2>(dfsc) == 0b0011 { - DataFault::PermissionFault(bits::<1, 0>(dfsc) as i64) - } else if bits::<5, 2>(dfsc) == 0b0001 { - DataFault::TranslationFault(bits::<1, 0>(dfsc) as i64) - } else if bits::<5, 2>(dfsc) == 0b1010 { - if bits::<1, 0>(dfsc) >= 2 { - DataFault::TranslationFault(bits::<1, 0>(dfsc) as i64 - 4) - } else { - DataFault::Other(dfsc) - } - } else { - DataFault::Other(dfsc) - } -} - -// some of the data in these is not used presently, but is logically -// part of the code being decoded & should be accounted for -#[allow(dead_code)] -#[derive(Debug, Copy, Clone)] -enum Exception { - /// lower el?, faulting address, status code - DataFault(bool, u64, DataFault), - Other(u64), -} -fn decode_syndrome(esr: u64) -> Exception { - let ec = bits::<31, 26>(esr); - match ec { - ESR_EC_DATA_ABORT_LOWER_EL => Exception::DataFault( - true, - unsafe { mrs!(FAR_EL1) }, - decode_data_fault(bits::<5, 0>(esr)), - ), - ESR_EC_DATA_ABORT_SAME_EL => Exception::DataFault( - false, - unsafe { mrs!(FAR_EL1) }, - decode_data_fault(bits::<5, 0>(esr)), - ), - _ => Exception::Other(esr), - } -} - fn handle_stack_fault(far: u64) { // TODO: perhaps we should have a sanity check that the // stack grows only one page at a time, which should be @@ -159,9 +99,13 @@ pub extern "Rust" fn _debug_print(x: &str) { hyperlight_guest::exit::debug_print(x); } -fn handle_internal_fault(exn: Exception) -> bool { +fn handle_internal_fault(exn: Exception, far: u64) -> bool { match exn { - Exception::DataFault(false, far, DataFault::TranslationFault(_)) => { + Exception::DataFault(DataFault { + from_lower_el: false, + kind: DataFaultKind::TranslationFault(_), + .. + }) => { if (MAIN_STACK_LIMIT_GVA..MAIN_STACK_TOP_GVA).contains(&far) { handle_stack_fault(far); true @@ -169,7 +113,12 @@ fn handle_internal_fault(exn: Exception) -> bool { false } } - Exception::DataFault(false, far, DataFault::PermissionFault(_)) => { + Exception::DataFault(DataFault { + from_lower_el: false, + is_write: true, + kind: DataFaultKind::PermissionFault(_), + .. + }) => { let mut orig_mappings = crate::paging::virt_to_phys(far); if let Some(mapping) = orig_mappings.next() && let None = orig_mappings.next() @@ -191,17 +140,17 @@ pub(super) extern "C" fn handle_exception( _regs: *mut ExceptionContext, ) { let esr = unsafe { mrs!(ESR_EL1) }; + let far = unsafe { mrs!(FAR_EL1) }; if typ == ExceptionType::Synchronous && from == ExceptionFrom::CurrentSP0 { let exn = decode_syndrome(esr); - if handle_internal_fault(exn) { + if handle_internal_fault(exn, far) { return; } } // Die with some diagnostic information let elr = unsafe { mrs!(ELR_EL1) }; - let far = unsafe { mrs!(FAR_EL1) }; let insn_bytes = unsafe { (elr as *const [u8; 8]).read_volatile() }; // amd64 provides the exception vector as the first byte of the // abort sequence after the guest error identifier code, but the From daa4a5ec57b973f785fe45994cc100e39abfb5d3 Mon Sep 17 00:00:00 2001 From: Lucy Menon <168595099+syntactically@users.noreply.github.com> Date: Sat, 1 Aug 2026 15:02:09 +0100 Subject: [PATCH 16/17] ci: enable running on MacOS/HVF Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- .github/workflows/DailyArm64.yml | 6 +++-- .github/workflows/RegenSnapshotGoldens.yml | 29 +++++++++++----------- .github/workflows/ValidatePullRequest.yml | 4 ++- .github/workflows/dep_build_test.yml | 4 ++- Justfile | 1 + 5 files changed, 26 insertions(+), 18 deletions(-) diff --git a/.github/workflows/DailyArm64.yml b/.github/workflows/DailyArm64.yml index 238f3f087..14e4d80f4 100644 --- a/.github/workflows/DailyArm64.yml +++ b/.github/workflows/DailyArm64.yml @@ -50,10 +50,11 @@ jobs: fail-fast: false matrix: config: [debug, release] + hypervisor: [hvf, kvm] uses: ./.github/workflows/dep_build_test.yml secrets: inherit with: - hypervisor: kvm + hypervisor: ${{ matrix.hypervisor }} cpu_vendor: apple arch: arm64 config: ${{ matrix.config }} @@ -65,10 +66,11 @@ jobs: fail-fast: false matrix: config: [debug, release] + hypervisor: [hvf, kvm] uses: ./.github/workflows/dep_run_examples.yml secrets: inherit with: - hypervisor: kvm + hypervisor: ${{ matrix.hypervisor }} cpu_vendor: apple arch: arm64 config: ${{ matrix.config }} diff --git a/.github/workflows/RegenSnapshotGoldens.yml b/.github/workflows/RegenSnapshotGoldens.yml index fd2cd2230..ed4f7b65c 100644 --- a/.github/workflows/RegenSnapshotGoldens.yml +++ b/.github/workflows/RegenSnapshotGoldens.yml @@ -138,7 +138,7 @@ jobs: strategy: fail-fast: false matrix: - hypervisor: [kvm, mshv3, hyperv-ws2025] + hypervisor: [kvm, mshv3, hyperv-ws2025, hvf] cpu: [amd, intel, apple] arch: [X64, arm64] config: [debug, release] @@ -148,25 +148,26 @@ jobs: hypervisor: mshv3 - cpu: apple hypervisor: hyperv-ws2025 + - arch: X64 + hypervisor: hvf - cpu: apple arch: X64 - cpu: amd arch: arm64 - cpu: intel arch: arm64 - runs-on: ${{ fromJson( - format('["self-hosted", "{0}", "{1}"{2}]', - matrix.hypervisor == 'hyperv-ws2025' && 'Windows' || 'Linux', - matrix.arch, - matrix.arch == 'X64' - && format(', "1ES.Pool=hld-{0}-{1}", "JobId=regen-goldens-{2}-{3}-{4}-{5}"', - matrix.hypervisor == 'hyperv-ws2025' && 'win2025' || matrix.hypervisor == 'mshv3' && 'azlinux3-mshv' || matrix.hypervisor, - matrix.cpu, - matrix.config, - github.run_id, - github.run_number, - github.run_attempt) - || ', "kvm", "ubuntu-24.04"')) }} + runs-on: ${{ fromJson(matrix.arch == 'X64' + && format('["self-hosted", "{0}", "X64", "1ES.Pool=hld-{1}-{2}", "JobId=regen-goldens-{3}-{4}-{5}-{6}"]', + matrix.hypervisor == 'hyperv-ws2025' && 'Windows' || 'Linux', + matrix.hypervisor == 'hyperv-ws2025' && 'win2025' || matrix.hypervisor == 'mshv3' && 'azlinux3-mshv' || matrix.hypervisor, + matrix.cpu, + matrix.config, + github.run_id, + github.run_number, + github.run_attempt) + || format('["self-hosted", "{0}", "arm64", "{1}"]', + matrix.hypervisor == 'hvf' && 'macos' || 'Linux', + matrix.hypervisor)) }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/.github/workflows/ValidatePullRequest.yml b/.github/workflows/ValidatePullRequest.yml index 78f94970f..3ecc02815 100644 --- a/.github/workflows/ValidatePullRequest.yml +++ b/.github/workflows/ValidatePullRequest.yml @@ -149,7 +149,7 @@ jobs: strategy: fail-fast: true matrix: - hypervisor: ['hyperv-ws2025', mshv3, kvm] + hypervisor: ['hyperv-ws2025', mshv3, kvm, hvf] cpu_vendor: [amd, intel, apple] arch: [X64, arm64] config: [debug, release] @@ -158,6 +158,8 @@ jobs: hypervisor: hyperv-ws2025 - cpu_vendor: apple hypervisor: mshv3 + - arch: X64 + hypervisor: hvf - cpu_vendor: amd arch: arm64 - cpu_vendor: intel diff --git a/.github/workflows/dep_build_test.yml b/.github/workflows/dep_build_test.yml index 7f51350d3..fb98b57ca 100644 --- a/.github/workflows/dep_build_test.yml +++ b/.github/workflows/dep_build_test.yml @@ -67,7 +67,9 @@ jobs: github.run_id, github.run_number, github.run_attempt) - || '["self-hosted", "Linux", "arm64", "kvm", "ubuntu-24.04"]') }} + || format('["self-hosted", "{0}", "arm64", "{1}"]', + inputs.hypervisor == 'hvf' && 'macos' || 'Linux', + inputs.hypervisor)) }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/Justfile b/Justfile index 77289c68f..1583900f0 100644 --- a/Justfile +++ b/Justfile @@ -622,6 +622,7 @@ snapshot-goldens-pull target=default-target which="all" image=default-snapshot-g if [[ -e /dev/mshv ]]; then hv=mshv elif [[ -e /dev/kvm ]]; then hv=kvm elif [[ "${OS:-}" == "Windows_NT" ]]; then hv=whp + elif [[ "$(sysctl -n kern.hv_support)" == "1" ]]; then hv=hvf else echo "snapshot-goldens-pull: no hypervisor found" >&2; exit 1 fi # Mirror of `CpuVendor::golden_tag` in file/config.rs. x86_64 reads From 644b66a3fd4458af6c5b8541e021bd1d1a423111 Mon Sep 17 00:00:00 2001 From: Lucy Menon <168595099+syntactically@users.noreply.github.com> Date: Mon, 10 Aug 2026 15:04:56 +0100 Subject: [PATCH 17/17] Convert mem::shared_mem module to use thiserror structured errors It will be used from the hvf code, which needs to return a structured error itself. Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- src/hyperlight_host/src/error.rs | 36 +- src/hyperlight_host/src/hypervisor/gdb/mod.rs | 4 +- .../src/hypervisor/virtual_machine/mod.rs | 10 +- src/hyperlight_host/src/mem/layout.rs | 2 +- src/hyperlight_host/src/mem/mgr.rs | 46 +- src/hyperlight_host/src/mem/shared_mem.rs | 422 +++++++++++------- .../src/mem/shared_mem_tests.rs | 13 +- src/hyperlight_host/tests/integration_test.rs | 6 +- 8 files changed, 307 insertions(+), 232 deletions(-) diff --git a/src/hyperlight_host/src/error.rs b/src/hyperlight_host/src/error.rs index c6738374d..3ad920c8f 100644 --- a/src/hyperlight_host/src/error.rs +++ b/src/hyperlight_host/src/error.rs @@ -42,9 +42,6 @@ pub enum HyperlightError { /// Anyhow error #[error("Anyhow Error was returned: {0}")] AnyhowError(#[from] anyhow::Error), - /// Memory access out of bounds - #[error("Offset: {0} out of bounds, Max is: {1}")] - BoundsCheckFailed(u64, usize), /// Checked Add Overflow #[error("Couldn't add offset to base address. Offset: {0}, Base Address: {1}")] @@ -154,18 +151,6 @@ pub enum HyperlightError { #[error("Memory Access Violation at address {0:#x} of type {1}, but memory is marked as {2}")] MemoryAccessViolation(u64, MemoryRegionFlags, MemoryRegionFlags), - /// Memory Allocation Failed. - #[error("Memory Allocation Failed with OS Error {0:?}.")] - MemoryAllocationFailed(Option), - - /// Memory Protection Failed - #[error("Memory Protection Failed with OS Error {0:?}.")] - MemoryProtectionFailed(Option), - - /// Memory region size mismatch - #[error("Memory region size mismatch: host size {0:?}, guest size {1:?} region {2:?}")] - MemoryRegionSizeMismatch(usize, usize, String), - /// The memory request exceeds the maximum size allowed #[error("Memory requested {0} exceeds maximum size allowed {1}")] MemoryRequestTooBig(usize, usize), @@ -179,14 +164,6 @@ pub enum HyperlightError { #[error("Metric Not Found {0:?}.")] MetricNotFound(&'static str), - /// mmap Failed. - #[error("mmap failed with os error {0:?}")] - MmapFailed(Option), - - /// mprotect Failed. - #[error("mprotect failed with os error {0:?}")] - MprotectFailed(Option), - /// No Hypervisor was found for Sandbox. #[error("No Hypervisor was found for Sandbox")] NoHypervisorFound(), @@ -240,6 +217,10 @@ pub enum HyperlightError { #[error("Failed To Convert Return Value {0:?} to {1:?}")] ReturnValueConversionFailure(ReturnValue, &'static str), + /// Error creating or operating on memory shared with the guest + #[error("Failed to execute shared memory operation: {0}")] + SharedMemory(#[from] crate::mem::shared_mem::SharedMemoryError), + /// Tried to restore a snapshot into a sandbox whose memory /// layout is not compatible with the snapshot's. #[error("Snapshot memory layout is not compatible with this sandbox")] @@ -347,7 +328,6 @@ impl HyperlightError { | HyperlightError::PoisonedSandbox | HyperlightError::ExecutionAccessViolation(_) | HyperlightError::MemoryAccessViolation(_, _, _) - | HyperlightError::MemoryRegionSizeMismatch(_, _, _) // HyperlightVmError::Restore is already handled manually in restore(), but we mark it // as poisoning here too for defense in depth. | HyperlightError::HyperlightVmError(HyperlightVmError::Restore(_)) => true, @@ -365,7 +345,6 @@ impl HyperlightError { // All other errors do not poison the sandbox. HyperlightError::AnyhowError(_) - | HyperlightError::BoundsCheckFailed(_, _) | HyperlightError::CheckedAddOverflow(_, _) | HyperlightError::CStringConversionError(_) | HyperlightError::Error(_) @@ -386,13 +365,9 @@ impl HyperlightError { | HyperlightError::InvalidFlatBuffer(_) | HyperlightError::JsonConversionFailure(_) | HyperlightError::LockAttemptFailed(_) - | HyperlightError::MemoryAllocationFailed(_) - | HyperlightError::MemoryProtectionFailed(_) | HyperlightError::MemoryRequestTooBig(_, _) | HyperlightError::MemoryRequestTooSmall(_, _) | HyperlightError::MetricNotFound(_) - | HyperlightError::MmapFailed(_) - | HyperlightError::MprotectFailed(_) | HyperlightError::NoHypervisorFound() | HyperlightError::NoMemorySnapshot | HyperlightError::ParameterValueConversionFailure(_, _) @@ -409,7 +384,8 @@ impl HyperlightError { | HyperlightError::UnexpectedParameterValueType(_, _) | HyperlightError::UnexpectedReturnValueType(_, _) | HyperlightError::UTF8StringConversionFailure(_) - | HyperlightError::VectorCapacityIncorrect(_, _, _) => false, + | HyperlightError::VectorCapacityIncorrect(_, _, _) + | HyperlightError::SharedMemory(_) => false, #[cfg(target_os = "windows")] HyperlightError::CrossBeamReceiveError(_) => false, diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index 131dd350f..d036ab985 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -159,12 +159,12 @@ impl<'a> DebugMemoryView<'a> { .mem_mgr .shared_mem .copy_from_slice(data, resolved.offset) - .map_err(|e| DebugMemoryAccessError::CopyFailed(Box::new(e))), + .map_err(|e| DebugMemoryAccessError::CopyFailed(Box::new(e.into()))), BaseGpaRegion::Scratch(()) => self .mem_mgr .scratch_mem .copy_from_slice(data, resolved.offset) - .map_err(|e| DebugMemoryAccessError::CopyFailed(Box::new(e))), + .map_err(|e| DebugMemoryAccessError::CopyFailed(Box::new(e.into()))), _ => Err(DebugMemoryAccessError::WriteToReadOnly), } } diff --git a/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs b/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs index 611a9e956..86a6dc7a8 100644 --- a/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs +++ b/src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs @@ -178,7 +178,7 @@ pub(crate) enum VmExit { } /// VM error -#[derive(Debug, Clone, thiserror::Error)] +#[derive(Debug, thiserror::Error)] pub enum VmError { #[error("Failed to create vm: {0}")] CreateVm(#[from] CreateVmError), @@ -241,7 +241,7 @@ pub enum CreateVmError { } /// RunVCPU error -#[derive(Debug, Clone, thiserror::Error)] +#[derive(Debug, thiserror::Error)] pub enum RunVcpuError { #[error("Failed to decode message type: {0}")] DecodeIOMessage(u32), @@ -343,7 +343,7 @@ pub enum RegisterError { ConversionFailed(String), } -#[derive(Debug, Clone, thiserror::Error)] +#[derive(Debug, thiserror::Error)] pub enum ResetVcpuError { #[error("Single-operation vcpu reset not supported on this hypervisor")] NotSupported, @@ -356,7 +356,7 @@ pub enum ResetVcpuError { } /// Map memory error -#[derive(Debug, Clone, thiserror::Error)] +#[derive(Debug, thiserror::Error)] pub enum MapMemoryError { #[cfg(target_os = "windows")] #[error("Address conversion failed: {0}")] @@ -381,7 +381,7 @@ pub enum MapMemoryError { } /// Unmap memory error -#[derive(Debug, Clone, thiserror::Error)] +#[derive(Debug, thiserror::Error)] pub enum UnmapMemoryError { #[error("Hypervisor error: {0}")] Hypervisor(HypervisorError), diff --git a/src/hyperlight_host/src/mem/layout.rs b/src/hyperlight_host/src/mem/layout.rs index 3bc615cb4..88372486b 100644 --- a/src/hyperlight_host/src/mem/layout.rs +++ b/src/hyperlight_host/src/mem/layout.rs @@ -158,7 +158,7 @@ pub(crate) trait ReadableSharedMemory { #[cfg(readable_shared_mem)] impl ReadableSharedMemory for &HostSharedMemory { fn copy_to_slice(&self, slice: &mut [u8], offset: usize) -> Result<()> { - HostSharedMemory::copy_to_slice(self, slice, offset) + Ok(HostSharedMemory::copy_to_slice(self, slice, offset)?) } } /// Coherence workaround for the blanket impl below. diff --git a/src/hyperlight_host/src/mem/mgr.rs b/src/hyperlight_host/src/mem/mgr.rs index 9a2933f78..2b3338e78 100644 --- a/src/hyperlight_host/src/mem/mgr.rs +++ b/src/hyperlight_host/src/mem/mgr.rs @@ -390,10 +390,12 @@ impl SandboxMemoryManager { /// Reads a host function call from memory #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] pub(crate) fn get_host_function_call(&mut self) -> Result { - self.scratch_mem.try_pop_buffer_into::( - self.layout.get_output_data_buffer_scratch_host_offset(), - self.layout.output_data_size(), - ) + self.scratch_mem + .try_pop_buffer_into::( + self.layout.get_output_data_buffer_scratch_host_offset(), + self.layout.output_data_size(), + ) + .map_err(From::from) } /// Writes a host function call result to memory @@ -405,11 +407,13 @@ impl SandboxMemoryManager { let mut builder = FlatBufferBuilder::new(); let data = res.encode(&mut builder); - self.scratch_mem.push_buffer( - self.layout.get_input_data_buffer_scratch_host_offset(), - self.layout.input_data_size(), - data, - ) + self.scratch_mem + .push_buffer( + self.layout.get_input_data_buffer_scratch_host_offset(), + self.layout.input_data_size(), + data, + ) + .map_err(From::from) } /// Writes a guest function call to memory @@ -434,19 +438,23 @@ impl SandboxMemoryManager { /// A function call result can be either an error or a successful return value. #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] pub(crate) fn get_guest_function_call_result(&mut self) -> Result { - self.scratch_mem.try_pop_buffer_into::( - self.layout.get_output_data_buffer_scratch_host_offset(), - self.layout.output_data_size(), - ) + self.scratch_mem + .try_pop_buffer_into::( + self.layout.get_output_data_buffer_scratch_host_offset(), + self.layout.output_data_size(), + ) + .map_err(From::from) } /// Read guest log data from the `SharedMemory` contained within `self` #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] pub(crate) fn read_guest_log_data(&mut self) -> Result { - self.scratch_mem.try_pop_buffer_into::( - self.layout.get_output_data_buffer_scratch_host_offset(), - self.layout.output_data_size(), - ) + self.scratch_mem + .try_pop_buffer_into::( + self.layout.get_output_data_buffer_scratch_host_offset(), + self.layout.output_data_size(), + ) + .map_err(From::from) } pub(crate) fn clear_io_buffers(&mut self) { @@ -527,7 +535,9 @@ impl SandboxMemoryManager { fn update_scratch_bookkeeping_item(&mut self, offset: u64, value: u64) -> Result<()> { let scratch_size = self.scratch_mem.mem_size(); let base_offset = scratch_size - offset as usize; - self.scratch_mem.write::(base_offset, value) + self.scratch_mem + .write::(base_offset, value) + .map_err(From::from) } fn update_scratch_bookkeeping(&mut self) -> Result<()> { diff --git a/src/hyperlight_host/src/mem/shared_mem.rs b/src/hyperlight_host/src/mem/shared_mem.rs index a77d7e374..c0cd28219 100644 --- a/src/hyperlight_host/src/mem/shared_mem.rs +++ b/src/hyperlight_host/src/mem/shared_mem.rs @@ -23,6 +23,7 @@ use std::ptr::null_mut; use std::sync::{Arc, RwLock}; use bytemuck::Pod; +use thiserror::Error; use tracing::{Span, instrument}; #[cfg(target_os = "windows")] use windows::Win32::Foundation::{CloseHandle, HANDLE, INVALID_HANDLE_VALUE}; @@ -42,20 +43,147 @@ use windows::core::PCSTR; use super::memory_region::{ HostGuestMemoryRegion, MemoryRegion, MemoryRegionFlags, MemoryRegionKind, MemoryRegionType, }; -#[cfg(target_os = "windows")] -use crate::HyperlightError::WindowsAPIError; -use crate::{HyperlightError, Result, log_then_return, new_error}; +use crate::log_then_return; + +type Result = core::result::Result; + +/// Whether a [`StackError`] was encountered whilst pushing or popping +/// from the guest stack +#[derive(Debug)] +pub enum StackOp { + /// The error was encountered while pushing to the guest stack + Push, + /// The error was encountered while popping from the guest stack + Pop, +} +/// An error related to the stack discipline of guest I/O +#[derive(Error, Debug)] +pub enum StackError { + /// The stack pointer for a stack entry was out-of-bounds for the + /// stack + #[error( + "Unable to {0:?} data from buffer: Stack pointer is out of bounds. Stack pointer: {1}, Buffer size: {2}" + )] + SpOob(StackOp, usize, usize), + + /// The back pointer for a stack entry was corrupt + #[error("Corrupt buffer back-pointer: element offset {0} is outside valid range [8, {1}].")] + CorruptBackPointer(usize, usize), + + /// A stack entry size prefix was too large for necessary + /// operations on it to remain in the range of a u32 + #[error("Corrupt buffer size prefix: value {0} overflows when adding 4-byte header.")] + OverflowingPrefix(u32), + + /// It was not possible to convert a stack entry size prefix into + /// a usize. This should be impossible on all currently supported + /// architectures, since usize is 64 bits on all of them. + #[error("Prefix too large: {0}")] + PrefixTooLarge(std::num::TryFromIntError), + + /// A stack entry size prefix is larger than its + /// logically-enclosing element + #[error( + "Corrupt buffer size prefix: flatbuffer claims {0} bytes but the element slot is only {1} bytes." + )] + CorruptPrefix(usize, usize), + + /// An error was encountered during a routine error conversion + /// that should have been infallible + #[error("pop_buffer_into: failed to convert buffer to {0}")] + ConvertError(String), + + /// There was not enough free space available on the stack for an + /// element to be pushed + #[error("Not enough space in buffer to push data. Required: {0}, Available: {1}")] + BufferFullError(usize, usize), +} +/// This is just an alias for std::backtrace::Backtrace that we +/// introduce to stop thiserror from using its backtrace +/// functionality, which depends on nightly APIs. +type ThisErrorHackBacktrace = std::backtrace::Backtrace; + +/// An error encountered while setting up or manipulating a shared memory region +#[derive(Error, Debug)] +pub enum SharedMemoryError { + /// Some operation on the shared memory attempted to read or write + /// out of bounds + #[error("Cannot access a value with size {0} at offset {1} in memory of size {2}")] + Bounds(usize, usize, usize), + + /// When creating a memory with contents from a file, metadata for + /// that file could not be read + #[error("Could not access metadata for file: {0}")] + FileMetadata(std::io::Error), + + /// When creating a memory with contents from a file, that file + /// was logically larger than the range of a usize. + #[error("File size exceeded usize: {0}")] + FileTooLarge(std::num::TryFromIntError), + + /// The locking discipline used to enforce temporary exclusive + /// access by the host to a shared memory failed in some way + #[error("Could not acquire memory lock: {0} at {1}")] + LockError(String, ThisErrorHackBacktrace), + + /// A request to allocate a shared memory could not be fulfilled + /// by the host operating system + #[error("Memory Allocation Failed with OS Error {0:?}.")] + MemoryAllocationFailed(Option), + + /// A ruqest to allocate a shared shared memory had an invalid + /// size, due either to bounds or alignment. + #[error( + "Memory request does not satisfy constraints: 0x{1:x} < 0x{0:x} <= 0x{2:x} && 0x{0:x} % 0x{3:x} = 0" + )] + MemoryRequest(usize, usize, usize, usize), + + /// An request to mmap a file or create an anonymous memory could + /// not be fulfilled by the host operating system + #[error("mmap failed with os error {0:?}")] + MmapFailed(Option), + + /// A request to change the host-side permissions on the guard + /// pages of a shared memory region could not be fulfilled by the + /// host operating system + #[error("mprotect failed with os error {0:?}")] + MprotectFailed(Option), + + /// A Windows virtual memory API call failed + #[cfg(target_os = "windows")] + #[error("Windows API Error Result {0:?}")] + WindowsAPIError(#[from] windows_result::Error), + + /// Calling code attempted to take exclusive (write) access to a + /// [`ReadonlySharedMemory`]. + #[error("Cannot take exclusive access to a ReadonlySharedMemory")] + ReadonlySharedMemoryExclusiveRequest, + + /// The stack discipline of guest I/O was violated in some way + #[error("{0}")] + Stack(#[from] StackError), + + /// An error was encountered when trying to convert a slice of raw + /// bytes into some logical data + #[error("Error reading slice {0}")] + TryFromSlice(#[from] std::array::TryFromSliceError), + + /// An error was encountered during a routine error conversion + /// that should have been infallible + #[error("Error reading int: {0}")] + TryFromInt(#[from] std::num::TryFromIntError), +} +impl From> for SharedMemoryError { + fn from(e: std::sync::TryLockError) -> SharedMemoryError { + SharedMemoryError::LockError(format!("{:?}", e), std::backtrace::Backtrace::capture()) + } +} /// Makes sure that the given `offset` and `size` are within the bounds of the memory with size `mem_size`. macro_rules! bounds_check { ($offset:expr, $size:expr, $mem_size:expr) => { if $offset.checked_add($size).is_none_or(|end| end > $mem_size) { - return Err(new_error!( - "Cannot read value from offset {} with size {} in memory of size {}", - $offset, - $size, - $mem_size - )); + return Err(SharedMemoryError::Bounds($offset, $size, $mem_size)); } }; } @@ -284,7 +412,7 @@ impl Placeholder { ) }; if addr.is_null() { - log_then_return!(HyperlightError::MemoryAllocationFailed( + log_then_return!(SharedMemoryError::MemoryAllocationFailed( Error::last_os_error().raw_os_error() )); } @@ -307,7 +435,7 @@ impl Placeholder { ) } { // `self` drops here, releasing the unsplit reservation. - log_then_return!(WindowsAPIError(e.clone())); + log_then_return!(SharedMemoryError::WindowsAPIError(e.clone())); } let addr = self.addr; let total = self.size; @@ -359,7 +487,7 @@ impl Placeholder { }; if mapped.Value.is_null() { // `self` drops here, releasing the placeholder. - log_then_return!(HyperlightError::MemoryAllocationFailed( + log_then_return!(SharedMemoryError::MemoryAllocationFailed( Error::last_os_error().raw_os_error() )); } @@ -532,6 +660,29 @@ pub struct ExclusiveSharedMemory { unsafe impl Send for ExclusiveSharedMemory {} impl ExclusiveSharedMemory { + /// Helper function used to abstract common checks from Windows + /// and Linux implementations of [`ExclusiveSharedMemory::new()`] + fn total_size(min_size_bytes: usize) -> Result { + if min_size_bytes > 0 && + // guard page around the memory + let Some(total_size) = min_size_bytes.checked_add(2 * page_size::get()) && + total_size % page_size::get() == 0 && + // usize and isize are guaranteed to be the same size, and + // isize::MAX should be positive, so this cast should be + // safe. + total_size <= isize::MAX as usize + { + Ok(total_size) + } else { + Err(SharedMemoryError::MemoryRequest( + min_size_bytes, + 2, + isize::MAX as usize - 2 * page_size::get(), + page_size::get(), + )) + } + } + /// Create a new region of shared memory with the given minimum /// size in bytes. The region will be surrounded by guard pages. /// @@ -543,32 +694,11 @@ impl ExclusiveSharedMemory { MAP_ANONYMOUS, MAP_FAILED, MAP_PRIVATE, PROT_READ, PROT_WRITE, c_int, mmap, off_t, size_t, }; - #[cfg(not(miri))] - use libc::{MAP_NORESERVE, PROT_NONE, mprotect}; - - if min_size_bytes == 0 { - return Err(new_error!("Cannot create shared memory with size 0")); - } - - let total_size = min_size_bytes - .checked_add(2 * page_size::get()) // guard page around the memory - .ok_or_else(|| new_error!("Memory required for sandbox exceeded usize::MAX"))?; - if total_size % page_size::get() != 0 { - return Err(new_error!( - "shared memory must be a multiple of {}", - page_size::get() - )); - } + let total_size = Self::total_size(min_size_bytes)?; - // usize and isize are guaranteed to be the same size, and - // isize::MAX should be positive, so this cast should be safe. - if total_size > isize::MAX as usize { - return Err(HyperlightError::MemoryRequestTooBig( - total_size, - isize::MAX as usize, - )); - } + #[cfg(not(miri))] + use libc::{MAP_NORESERVE, PROT_NONE, mprotect}; // allocate the memory #[cfg(not(miri))] @@ -587,7 +717,7 @@ impl ExclusiveSharedMemory { ) }; if addr == MAP_FAILED { - log_then_return!(HyperlightError::MmapFailed( + log_then_return!(SharedMemoryError::MmapFailed( Error::last_os_error().raw_os_error() )); } @@ -601,7 +731,7 @@ impl ExclusiveSharedMemory { { let res = unsafe { mprotect(mmap.base, page_size::get(), PROT_NONE) }; if res != 0 { - return Err(HyperlightError::MprotectFailed( + return Err(SharedMemoryError::MprotectFailed( Error::last_os_error().raw_os_error(), )); } @@ -613,7 +743,7 @@ impl ExclusiveSharedMemory { ) }; if res != 0 { - return Err(HyperlightError::MprotectFailed( + return Err(SharedMemoryError::MprotectFailed( Error::last_os_error().raw_os_error(), )); } @@ -640,29 +770,7 @@ impl ExclusiveSharedMemory { #[cfg(target_os = "windows")] #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub fn new(min_size_bytes: usize) -> Result { - if min_size_bytes == 0 { - return Err(new_error!("Cannot create shared memory with size 0")); - } - - let total_size = min_size_bytes - .checked_add(2 * page_size::get()) - .ok_or_else(|| new_error!("Memory required for sandbox exceeded {}", usize::MAX))?; - - if total_size % page_size::get() != 0 { - return Err(new_error!( - "shared memory must be a multiple of {}", - page_size::get() - )); - } - - // usize and isize are guaranteed to be the same size, and - // isize::MAX should be positive, so this cast should be safe. - if total_size > isize::MAX as usize { - return Err(HyperlightError::MemoryRequestTooBig( - total_size, - isize::MAX as usize, - )); - } + let total_size = Self::total_size(min_size_bytes)?; let mut dwmaximumsizehigh = 0; let mut dwmaximumsizelow = 0; @@ -689,7 +797,7 @@ impl ExclusiveSharedMemory { }; if handle.is_invalid() { - log_then_return!(HyperlightError::MemoryAllocationFailed( + log_then_return!(SharedMemoryError::MemoryAllocationFailed( Error::last_os_error().raw_os_error() )); } @@ -699,7 +807,7 @@ impl ExclusiveSharedMemory { let addr = unsafe { MapViewOfFile(file_mapping.0, file_map, 0, 0, 0) }; if addr.Value.is_null() { - log_then_return!(HyperlightError::MemoryAllocationFailed( + log_then_return!(SharedMemoryError::MemoryAllocationFailed( Error::last_os_error().raw_os_error() )); } @@ -723,7 +831,7 @@ impl ExclusiveSharedMemory { &mut unused_out_old_prot_flags, ) } { - log_then_return!(WindowsAPIError(e.clone())); + log_then_return!(SharedMemoryError::WindowsAPIError(e.clone())); } let last_guard_page_start = unsafe { view.addr.add(total_size - page_size::get()) }; @@ -735,7 +843,7 @@ impl ExclusiveSharedMemory { &mut unused_out_old_prot_flags, ) } { - log_then_return!(WindowsAPIError(e.clone())); + log_then_return!(SharedMemoryError::WindowsAPIError(e.clone())); } Ok(Self { @@ -955,10 +1063,7 @@ impl SharedMemory for GuestSharedMemory { &mut self, f: F, ) -> Result { - let guard = self - .lock - .try_write() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?; + let guard = self.lock.try_write()?; let mut excl = ExclusiveSharedMemory { region: self.region.clone(), }; @@ -1152,10 +1257,7 @@ impl HostSharedMemory { pub fn copy_to_slice(&self, slice: &mut [u8], offset: usize) -> Result<()> { bounds_check!(offset, slice.len(), self.mem_size()); let base = self.base_ptr().wrapping_add(offset); - let guard = self - .lock - .try_read() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?; + let guard = self.lock.try_read()?; const CHUNK: usize = size_of::(); let len = slice.len(); @@ -1203,10 +1305,7 @@ impl HostSharedMemory { pub fn copy_from_slice(&self, slice: &[u8], offset: usize) -> Result<()> { bounds_check!(offset, slice.len(), self.mem_size()); let base = self.base_ptr().wrapping_add(offset); - let guard = self - .lock - .try_read() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?; + let guard = self.lock.try_read()?; const CHUNK: usize = size_of::(); let len = slice.len(); @@ -1254,10 +1353,7 @@ impl HostSharedMemory { pub fn fill(&mut self, value: u8, offset: usize, len: usize) -> Result<()> { bounds_check!(offset, len, self.mem_size()); let base = self.base_ptr().wrapping_add(offset); - let guard = self - .lock - .try_read() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?; + let guard = self.lock.try_read()?; const CHUNK: usize = size_of::(); let value_u128 = u128::from_ne_bytes([value; CHUNK]); @@ -1307,25 +1403,20 @@ impl HostSharedMemory { data: &[u8], ) -> Result<()> { let stack_pointer_rel = self.read::(buffer_start_offset)? as usize; - let buffer_size_u64: u64 = buffer_size.try_into()?; if stack_pointer_rel > buffer_size || stack_pointer_rel < 8 { - return Err(new_error!( - "Unable to push data to buffer: Stack pointer is out of bounds. Stack pointer: {}, Buffer size: {}", + Err(StackError::SpOob( + StackOp::Push, stack_pointer_rel, - buffer_size_u64 - )); + buffer_size, + ))?; } let size_required = data.len() + 8; let size_available = buffer_size - stack_pointer_rel; if size_required > size_available { - return Err(new_error!( - "Not enough space in buffer to push data. Required: {}, Available: {}", - size_required, - size_available - )); + Err(StackError::BufferFullError(size_required, size_available))?; } // get absolute @@ -1361,11 +1452,11 @@ impl HostSharedMemory { let stack_pointer_rel = self.read::(buffer_start_offset)? as usize; if stack_pointer_rel > buffer_size || stack_pointer_rel < 16 { - return Err(new_error!( - "Unable to pop data from buffer: Stack pointer is out of bounds. Stack pointer: {}, Buffer size: {}", + Err(StackError::SpOob( + StackOp::Pop, stack_pointer_rel, - buffer_size - )); + buffer_size, + ))?; } // make it absolute @@ -1381,11 +1472,10 @@ impl HostSharedMemory { if last_element_offset_rel > stack_pointer_rel.saturating_sub(16) || last_element_offset_rel < 8 { - return Err(new_error!( - "Corrupt buffer back-pointer: element offset {} is outside valid range [8, {}].", + Err(StackError::CorruptBackPointer( last_element_offset_rel, stack_pointer_rel.saturating_sub(16), - )); + ))?; } // make it absolute @@ -1399,32 +1489,21 @@ impl HostSharedMemory { let raw_prefix = self.read::(last_element_offset_abs)?; // flatbuffer byte arrays are prefixed by 4 bytes indicating // the remaining size; add 4 for the prefix itself. - let total = raw_prefix.checked_add(4).ok_or_else(|| { - new_error!( - "Corrupt buffer size prefix: value {} overflows when adding 4-byte header.", - raw_prefix - ) - })?; - usize::try_from(total) - }?; + let total = raw_prefix + .checked_add(4) + .ok_or(StackError::OverflowingPrefix(raw_prefix))?; + usize::try_from(total).map_err(StackError::PrefixTooLarge)? + }; if fb_buffer_size > max_element_size { - return Err(new_error!( - "Corrupt buffer size prefix: flatbuffer claims {} bytes but the element slot is only {} bytes.", - fb_buffer_size, - max_element_size - )); + Err(StackError::CorruptPrefix(fb_buffer_size, max_element_size))?; } let mut result_buffer = vec![0; fb_buffer_size]; self.copy_to_slice(&mut result_buffer, last_element_offset_abs)?; - let to_return = T::try_from(result_buffer.as_slice()).map_err(|_e| { - new_error!( - "pop_buffer_into: failed to convert buffer to {}", - type_name::() - ) - })?; + let to_return = T::try_from(result_buffer.as_slice()) + .map_err(|_| StackError::ConvertError(type_name::().to_string()))?; // update the stack pointer to point to the element we just popped off since that is now free self.write::(buffer_start_offset, last_element_offset_rel as u64)?; @@ -1445,10 +1524,7 @@ impl SharedMemory for HostSharedMemory { &mut self, f: F, ) -> Result { - let guard = self - .lock - .try_write() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?; + let guard = self.lock.try_write()?; let mut excl = ExclusiveSharedMemory { region: self.region.clone(), }; @@ -1488,17 +1564,15 @@ unsafe impl Sync for ReadonlySharedMemory {} impl ReadonlySharedMemory { pub(crate) fn from_bytes(contents: &[u8], guest_mapped_size: usize) -> Result { - if guest_mapped_size == 0 || !guest_mapped_size.is_multiple_of(page_size::get()) { - return Err(new_error!( - "guest_mapped_size {} must be a non-zero multiple of PAGE_SIZE", - guest_mapped_size - )); - } - if guest_mapped_size > contents.len() { - return Err(new_error!( - "guest_mapped_size {} exceeds blob length {}", + if guest_mapped_size == 0 + || guest_mapped_size > contents.len() + || !guest_mapped_size.is_multiple_of(page_size::get()) + { + return Err(SharedMemoryError::MemoryRequest( guest_mapped_size, - contents.len() + 0, + contents.len(), + page_size::get(), )); } let mut anon = @@ -1524,21 +1598,17 @@ impl ReadonlySharedMemory { pub(crate) fn from_file(file: &std::fs::File, guest_mapped_size: usize) -> Result { let len: usize = file .metadata() - .map_err(|e| new_error!("Failed to read file metadata: {}", e))? + .map_err(SharedMemoryError::FileMetadata)? .len() .try_into() - .map_err(|_| new_error!("File length exceeds usize::MAX"))?; - - if len == 0 { - return Err(new_error!( - "Cannot create file-backed shared memory with size 0" - )); - } + .map_err(SharedMemoryError::FileTooLarge)?; - if !len.is_multiple_of(page_size::get()) { - return Err(new_error!( - "file length {} must be a multiple of PAGE_SIZE", - len + if len == 0 || !len.is_multiple_of(page_size::get()) { + return Err(SharedMemoryError::MemoryRequest( + len, + 0, + usize::MAX, + page_size::get(), )); } @@ -1546,10 +1616,11 @@ impl ReadonlySharedMemory { || guest_mapped_size > len || !guest_mapped_size.is_multiple_of(page_size::get()) { - return Err(new_error!( - "guest_mapped_size {} must be a non-zero multiple of PAGE_SIZE no greater than file length {}", + return Err(SharedMemoryError::MemoryRequest( guest_mapped_size, - len + 0, + len, + page_size::get(), )); } @@ -1574,9 +1645,14 @@ impl ReadonlySharedMemory { mmap, off_t, size_t, }; - let total_size = len.checked_add(2 * page_size::get()).ok_or_else(|| { - new_error!("Memory required for file-backed mapping exceeded usize::MAX") - })?; + let total_size = + len.checked_add(2 * page_size::get()) + .ok_or(SharedMemoryError::MemoryRequest( + len, + 0, + usize::MAX - 2 * page_size::get(), + 1, + ))?; let fd = file.as_raw_fd(); @@ -1596,7 +1672,7 @@ impl ReadonlySharedMemory { ) }; if base == MAP_FAILED { - return Err(HyperlightError::MmapFailed( + return Err(SharedMemoryError::MmapFailed( std::io::Error::last_os_error().raw_os_error(), )); } @@ -1636,7 +1712,7 @@ impl ReadonlySharedMemory { ) }; if mapped == MAP_FAILED { - return Err(HyperlightError::MmapFailed( + return Err(SharedMemoryError::MmapFailed( std::io::Error::last_os_error().raw_os_error(), )); } @@ -1655,9 +1731,14 @@ impl ReadonlySharedMemory { fn map_file(file: &std::fs::File, len: usize) -> Result> { use std::os::windows::io::AsRawHandle; - let total_size = len.checked_add(2 * page_size::get()).ok_or_else(|| { - new_error!("Memory required for file-backed mapping exceeded usize::MAX") - })?; + let total_size = + len.checked_add(2 * page_size::get()) + .ok_or(SharedMemoryError::MemoryRequest( + len, + 0, + usize::MAX - 2 * page_size::get(), + 1, + ))?; let file_handle = HANDLE(file.as_raw_handle()); @@ -1677,7 +1758,7 @@ impl ReadonlySharedMemory { let raw_handle = unsafe { CreateFileMappingA(file_handle, None, PAGE_READONLY, 0, 0, PCSTR::null()) }?; if raw_handle.is_invalid() { - log_then_return!(HyperlightError::MemoryAllocationFailed( + log_then_return!(SharedMemoryError::MemoryAllocationFailed( Error::last_os_error().raw_os_error() )); } @@ -1773,9 +1854,7 @@ impl SharedMemory for ReadonlySharedMemory { &mut self, _: F, ) -> Result { - Err(new_error!( - "Cannot take exclusive access to a ReadonlySharedMemory" - )) + Err(SharedMemoryError::ReadonlySharedMemoryExclusiveRequest) } // However, just access to the contents as a slice is doable fn with_contents T>(&mut self, f: F) -> Result { @@ -1796,8 +1875,7 @@ mod tests { #[cfg(not(miri))] use super::HostSharedMemory; - use super::{ExclusiveSharedMemory, SharedMemory}; - use crate::Result; + use super::{ExclusiveSharedMemory, Result, SharedMemory}; #[cfg(not(miri))] use crate::mem::shared_mem_tests::read_write_test_suite; @@ -2593,7 +2671,7 @@ mod tests { let tmp = make_temp_file(0); let err = ReadonlySharedMemory::from_file(tmp.as_file(), page_size::get()) .expect_err("empty file should be rejected"); - assert!(format!("{}", err).contains("size 0")); + assert!(format!("{}", err).contains("0x0 < 0x0")); } #[test] @@ -2601,7 +2679,11 @@ mod tests { let tmp = make_temp_file(page_size::get() + 1); let err = ReadonlySharedMemory::from_file(tmp.as_file(), page_size::get()) .expect_err("unaligned file length should be rejected"); - assert!(format!("{}", err).contains("multiple of PAGE_SIZE")); + assert!(format!("{}", err).contains(&format!( + "0x{:x} % 0x{:x} = 0", + page_size::get() + 1, + page_size::get() + ))); } #[test] @@ -2609,7 +2691,7 @@ mod tests { let tmp = make_temp_file(page_size::get()); let err = ReadonlySharedMemory::from_file(tmp.as_file(), 0) .expect_err("zero guest_mapped_size should be rejected"); - assert!(format!("{}", err).contains("guest_mapped_size")); + assert!(format!("{}", err).contains("0x0 < 0x0")); } #[test] @@ -2617,7 +2699,11 @@ mod tests { let tmp = make_temp_file(2 * page_size::get()); let err = ReadonlySharedMemory::from_file(tmp.as_file(), page_size::get() + 1) .expect_err("unaligned guest_mapped_size should be rejected"); - assert!(format!("{}", err).contains("guest_mapped_size")); + assert!(format!("{}", err).contains(&format!( + "0x{:x} % 0x{:x} = ", + page_size::get() + 1, + page_size::get() + ))); } #[test] @@ -2625,7 +2711,11 @@ mod tests { let tmp = make_temp_file(page_size::get()); let err = ReadonlySharedMemory::from_file(tmp.as_file(), 2 * page_size::get()) .expect_err("guest_mapped_size > file length should be rejected"); - assert!(format!("{}", err).contains("guest_mapped_size")); + assert!(format!("{}", err).contains(&format!( + "0x{:x} <= 0x{:x}", + 2 * page_size::get(), + page_size::get() + ))); } /// Tests in this submodule are `#[ignore]`'d because each one diff --git a/src/hyperlight_host/src/mem/shared_mem_tests.rs b/src/hyperlight_host/src/mem/shared_mem_tests.rs index 1da949333..e82cbf48f 100644 --- a/src/hyperlight_host/src/mem/shared_mem_tests.rs +++ b/src/hyperlight_host/src/mem/shared_mem_tests.rs @@ -20,14 +20,15 @@ use std::convert::TryFrom; use std::fmt::Debug; use std::mem::size_of; -use crate::{Result, log_then_return, new_error}; +use super::shared_mem::SharedMemoryError; +use crate::{HyperlightError, log_then_return, new_error}; /// A function that knows how to read data of type `T` from a /// `SharedMemory` at a specified offset -type ReaderFn = dyn Fn(&S, usize) -> Result; +type ReaderFn = dyn Fn(&S, usize) -> Result; /// A function that knows how to write data of type `T` from a /// `SharedMemory` at a specified offset. -type WriterFn = dyn Fn(&mut S, usize, T) -> Result<()>; +type WriterFn = dyn Fn(&mut S, usize, T) -> Result<(), SharedMemoryError>; /// Run the standard suite of tests for a specified type `U` to write to /// a `SharedMemory` and a specified type `T` to read back out of @@ -41,12 +42,12 @@ type WriterFn = dyn Fn(&mut S, usize, T) -> Result<()>; /// Regardless of which types you choose, they must be `Clone`able, /// `Debug`able, and you must be able to check if `T`, the one returned /// by the `reader`, is equal to `U`, the one accepted by the writer. -pub(super) fn read_write_test_suite Result>( +pub(super) fn read_write_test_suite Result>( initial_val: U, shared_memory_new: ShmNew, reader: Box>, writer: Box>, -) -> Result<()> +) -> Result<(), HyperlightError> where T: PartialEq + Debug + Clone + TryFrom, U: Debug + Clone, @@ -102,7 +103,7 @@ where /// Swaps a result's status. If it was passed as an `Ok`, it will be returned /// as an `Err` with a hard-coded error message. If it was passed as an `Err`, /// it will be returned as an `Ok(_)`. -fn swap_res(r: Result) -> Result<()> { +fn swap_res(r: Result) -> Result<(), HyperlightError> { match r { Ok(_) => { log_then_return!("result was expected to be an error, but wasn't"); diff --git a/src/hyperlight_host/tests/integration_test.rs b/src/hyperlight_host/tests/integration_test.rs index 60b21706e..7fd5ec2e1 100644 --- a/src/hyperlight_host/tests/integration_test.rs +++ b/src/hyperlight_host/tests/integration_test.rs @@ -589,7 +589,7 @@ fn corrupt_output_size_prefix_rejected() { ); let err_msg = format!("{:?}", res.unwrap_err()); assert!( - err_msg.contains("Corrupt buffer size prefix: flatbuffer claims 4294967295 bytes but the element slot is only 8 bytes"), + err_msg.contains("SharedMemory(Stack(CorruptPrefix(4294967295, 8)))"), "Unexpected error message: {err_msg}" ); }); @@ -606,9 +606,7 @@ fn corrupt_output_back_pointer_rejected() { ); let err_msg = format!("{:?}", res.unwrap_err()); assert!( - err_msg.contains( - "Corrupt buffer back-pointer: element offset 57005 is outside valid range [8, 8]" - ), + err_msg.contains("SharedMemory(Stack(CorruptBackPointer(57005, 8)))"), "Unexpected error message: {err_msg}" ); });