Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/hyperlight_host/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ windows = { version = "0.62", features = [
"Win32_System_JobObjects",
"Win32_System_SystemServices",
] }
windows-sys = { version = "0.61", features = ["Win32"] }
windows-sys = { version = "0.61", features = ["Win32", "Win32_System", "Win32_System_Threading"] }
windows-result = "0.4"
rust-embed = { version = "8.7.2", features = ["debug-embed", "include-exclude", "interpolate-folder-path"] }
sha256 = "1.6.0"
Expand Down
88 changes: 48 additions & 40 deletions src/hyperlight_host/src/hypervisor/hyperv_linux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,8 @@ impl HypervLinuxDriver {

let interrupt_handle = Arc::new(LinuxInterruptHandle {
running: AtomicU64::new(0),
cancel_requested: AtomicBool::new(false),
cancel_requested: AtomicU64::new(0),
call_active: AtomicBool::new(false),
#[cfg(gdb)]
debug_interrupt: AtomicBool::new(false),
#[cfg(all(
Expand Down Expand Up @@ -669,16 +670,13 @@ impl Hypervisor for HypervLinuxDriver {
self.interrupt_handle
.tid
.store(unsafe { libc::pthread_self() as u64 }, Ordering::Relaxed);
// Note: if a `InterruptHandle::kill()` called while this thread is **here**
// Then this is fine since `cancel_requested` is set to true, so we will skip the `VcpuFd::run()` call
self.interrupt_handle
.set_running_and_increment_generation()
.map_err(|e| {
new_error!(
"Error setting running state and incrementing generation: {}",
e
)
})?;
// Note: if `InterruptHandle::kill()` is called while this thread is **here**
// (after set_running_bit but before checking cancel_requested):
// - kill() will stamp cancel_requested with the current generation
// - We will check cancel_requested below and skip the VcpuFd::run() call
// - This is the desired behavior - the kill takes effect immediately
let generation = self.interrupt_handle.set_running_bit();

#[cfg(not(gdb))]
let debug_interrupt = false;
#[cfg(gdb)]
Expand All @@ -687,14 +685,16 @@ impl Hypervisor for HypervLinuxDriver {
.debug_interrupt
.load(Ordering::Relaxed);

// Don't run the vcpu if `cancel_requested` is true
// Don't run the vcpu if `cancel_requested` is set for our generation
//
// Note: if a `InterruptHandle::kill()` called while this thread is **here**
// Then this is fine since `cancel_requested` is set to true, so we will skip the `VcpuFd::run()` call
// Note: if `InterruptHandle::kill()` is called while this thread is **here**
// (after checking cancel_requested but before vcpu.run()):
// - kill() will stamp cancel_requested with the current generation
// - We will proceed with vcpu.run(), but signals will be sent to interrupt it
// - The vcpu will be interrupted and return EINTR (handled below)
let exit_reason = if self
.interrupt_handle
.cancel_requested
.load(Ordering::Relaxed)
.is_cancel_requested_for_generation(generation)
|| debug_interrupt
{
Err(mshv_ioctls::MshvError::from(libc::EINTR))
Expand All @@ -707,11 +707,12 @@ impl Hypervisor for HypervLinuxDriver {
Some(hyperlight_guest_tracing::invariant_tsc::read_tsc());
self.trace_info.guest_start_epoch = Some(std::time::Instant::now());
}
// Note: if a `InterruptHandle::kill()` called while this thread is **here**
// Then the vcpu will run, but we will keep sending signals to this thread
// to interrupt it until `running` is set to false. The `vcpu_fd::run()` call will
// return either normally with an exit reason, or from being "kicked" by out signal handler, with an EINTR error,
// both of which are fine.
// Note: if `InterruptHandle::kill()` is called while this thread is **here**
// (during vcpu.run() execution):
// - kill() stamps cancel_requested with the current generation
// - kill() sends signals (SIGRTMIN+offset) to this thread repeatedly
// - The signal handler is a no-op, but it causes vcpu.run() to return EINTR
// - We check cancel_requested below and return Cancelled if generation matches
#[cfg(mshv2)]
{
let hv_message: hv_message = Default::default();
Expand All @@ -720,27 +721,32 @@ impl Hypervisor for HypervLinuxDriver {
#[cfg(mshv3)]
self.vcpu_fd.run()
};
// Note: if a `InterruptHandle::kill()` called while this thread is **here**
// Then signals will be sent to this thread until `running` is set to false.
// This is fine since the signal handler is a no-op.
// Note: if `InterruptHandle::kill()` is called while this thread is **here**
// (after vcpu.run() returns but before clear_running_bit):
// - kill() continues sending signals to this thread (running bit is still set)
// - The signals are harmless (no-op handler), we just need to check cancel_requested
// - We load cancel_requested below to determine if this run was cancelled
let cancel_requested = self
.interrupt_handle
.cancel_requested
.load(Ordering::Relaxed);
.is_cancel_requested_for_generation(generation);
#[cfg(gdb)]
let debug_interrupt = self
.interrupt_handle
.debug_interrupt
.load(Ordering::Relaxed);
// Note: if a `InterruptHandle::kill()` called while this thread is **here**
// Then `cancel_requested` will be set to true again, which will cancel the **next vcpu run**.
// Additionally signals will be sent to this thread until `running` is set to false.
// This is fine since the signal handler is a no-op.
// Note: if `InterruptHandle::kill()` is called while this thread is **here**
// (after loading cancel_requested but before clear_running_bit):
// - kill() stamps cancel_requested with the CURRENT generation (not the one we just loaded)
// - kill() continues sending signals until running bit is cleared
// - The newly stamped cancel_requested will affect the NEXT vcpu.run() call
// - Signals sent now are harmless (no-op handler)
self.interrupt_handle.clear_running_bit();
// At this point, `running` is false so no more signals will be sent to this thread,
// but we may still receive async signals that were sent before this point.
// To prevent those signals from interrupting subsequent calls to `run()`,
// we make sure to check `cancel_requested` before cancelling (see `libc::EINTR` match-arm below).
// At this point, running bit is clear so kill() will stop sending signals.
// However, we may still receive delayed signals that were sent before clear_running_bit.
// These stale signals are harmless because:
// - The signal handler is a no-op
// - We check generation matching in cancel_requested before treating EINTR as cancellation
// - If generation doesn't match, we return Retry instead of Cancelled
let result = match exit_reason {
Ok(m) => match m.header.message_type {
HALT_MESSAGE => {
Expand Down Expand Up @@ -820,14 +826,16 @@ impl Hypervisor for HypervLinuxDriver {
}
},
Err(e) => match e.errno() {
// we send a signal to the thread to cancel execution this results in EINTR being returned by KVM so we return Cancelled
// We send a signal (SIGRTMIN+offset) to interrupt the vcpu, which causes EINTR
libc::EINTR => {
// If cancellation was not requested for this specific vm, the vcpu was interrupted because of debug interrupt or
// a stale signal that meant to be delivered to a previous/other vcpu on this same thread, so let's ignore it
// Check if cancellation was requested for THIS specific generation.
// If not, the EINTR came from:
// - A debug interrupt (if GDB is enabled)
// - A stale signal from a previous guest call (generation mismatch)
// - A signal meant for a different sandbox on the same thread
// In these cases, we return Retry to continue execution.
if cancel_requested {
self.interrupt_handle
.cancel_requested
.store(false, Ordering::Relaxed);
self.interrupt_handle.clear_cancel_requested();
HyperlightExit::Cancelled()
} else {
#[cfg(gdb)]
Expand Down
57 changes: 55 additions & 2 deletions src/hyperlight_host/src/hypervisor/hyperv_windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ impl HypervWindowsDriver {
cancel_requested: AtomicBool::new(false),
#[cfg(gdb)]
debug_interrupt: AtomicBool::new(false),
call_active: AtomicBool::new(false),
partition_handle,
dropped: AtomicBool::new(false),
});
Expand Down Expand Up @@ -581,6 +582,10 @@ impl Hypervisor for HypervWindowsDriver {
}
self.processor.run()?
};
let cancel_was_requested_manually = self
.interrupt_handle
.cancel_requested
.load(Ordering::Relaxed);
self.interrupt_handle
.cancel_requested
.store(false, Ordering::Relaxed);
Expand Down Expand Up @@ -663,11 +668,31 @@ impl Hypervisor for HypervWindowsDriver {
// and resume execution
HyperlightExit::Debug(VcpuStopReason::Interrupt)
} else {
HyperlightExit::Cancelled()
if !cancel_was_requested_manually {
// This was an internal cancellation
// The virtualization stack can use this function to return the control
// of a virtual processor back to the virtualization stack in case it
// needs to change the state of a VM or to inject an event into the processor
debug!("Internal cancellation detected, returning Retry error");
HyperlightExit::Retry()
} else {
HyperlightExit::Cancelled()
}
}

#[cfg(not(gdb))]
HyperlightExit::Cancelled()
{
if !cancel_was_requested_manually {
// This was an internal cancellation
// The virtualization stack can use this function to return the control
// of a virtual processor back to the virtualization stack in case it
// needs to change the state of a VM or to inject an event into the processor
debug!("Internal cancellation detected, returning Retry error");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HyperlightExit::Retry()
} else {
HyperlightExit::Cancelled()
}
}
}
#[cfg(gdb)]
WHV_RUN_VP_EXIT_REASON(4098i32) => {
Expand Down Expand Up @@ -957,12 +982,32 @@ pub struct WindowsInterruptHandle {
// This is used to signal the GDB thread to stop the vCPU
#[cfg(gdb)]
debug_interrupt: AtomicBool,
/// Flag indicating whether a guest function call is currently in progress.
///
/// **true**: A guest function call is active (between call start and completion)
/// **false**: No guest function call is active
///
/// # Purpose
///
/// This flag prevents kill() from having any effect when called outside of a
/// guest function call. This solves the "kill-in-advance" problem where kill()
/// could be called before a guest function starts and would incorrectly cancel it.
call_active: AtomicBool,
partition_handle: WHV_PARTITION_HANDLE,
dropped: AtomicBool,
}

impl InterruptHandle for WindowsInterruptHandle {
fn kill(&self) -> bool {
// Check if a call is actually active first
if !self.call_active.load(Ordering::Acquire) {
return false;
}

// don't send the signal if the the vm isn't running
// In the case this is called before the vm is running the cancel_requested would be set
// and stay set while the vm is running.

self.cancel_requested.store(true, Ordering::Relaxed);
self.running.load(Ordering::Relaxed)
&& unsafe { WHvCancelRunVirtualProcessor(self.partition_handle, 0, 0).is_ok() }
Expand All @@ -977,4 +1022,12 @@ impl InterruptHandle for WindowsInterruptHandle {
fn dropped(&self) -> bool {
self.dropped.load(Ordering::Relaxed)
}

fn set_call_active(&self) {
self.call_active.store(true, Ordering::Release);
}

fn clear_call_active(&self) {
self.call_active.store(false, Ordering::Release);
}
}
Loading
Loading