Fixes issues from merge and splits trait to expose only public method (#994)

Signed-off-by: Simon Davies <[email protected]>

Author: simongdavies

src/hyperlight_host/src/hypervisor/hyperv_linux.rs

@@ -70,7 +70,7 @@ use super::{
     CR0_AM, CR0_ET, CR0_MP, CR0_NE, CR0_PE, CR0_PG, CR0_WP, CR4_OSFXSR, CR4_OSXMMEXCPT, CR4_PAE,
     EFER_LMA, EFER_LME, EFER_NX, EFER_SCE,
 };
-use super::{HyperlightExit, Hypervisor, InterruptHandle, LinuxInterruptHandle, VirtualCPU};
+use super::{HyperlightExit, Hypervisor, LinuxInterruptHandle, VirtualCPU};
 #[cfg(gdb)]
 use crate::HyperlightError;
 use crate::hypervisor::get_memory_access_violation;
@@ -764,11 +764,15 @@ impl Hypervisor for HypervLinuxDriver {
             .tid
             .store(unsafe { libc::pthread_self() as u64 }, Ordering::Release);
         // Note: if `InterruptHandle::kill()` is called while this thread is **here**
+        // Cast to internal trait for access to internal methods
+        let interrupt_handle_internal =
+            self.interrupt_handle.as_ref() as &dyn super::InterruptHandleInternal;
+
         // (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();
+        let generation = interrupt_handle_internal.set_running_bit();
 
         #[cfg(not(gdb))]
         let debug_interrupt = false;
@@ -785,8 +789,7 @@ impl Hypervisor for HypervLinuxDriver {
         // - 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
+        let exit_reason = if interrupt_handle_internal
             .is_cancel_requested_for_generation(generation)
             || debug_interrupt
         {
@@ -818,9 +821,8 @@ impl Hypervisor for HypervLinuxDriver {
         // - 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
-            .is_cancel_requested_for_generation(generation);
+        let cancel_requested =
+            interrupt_handle_internal.is_cancel_requested_for_generation(generation);
         #[cfg(gdb)]
         let debug_interrupt = self
             .interrupt_handle
@@ -832,7 +834,7 @@ impl Hypervisor for HypervLinuxDriver {
         // - 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();
+        interrupt_handle_internal.clear_running_bit();
         // 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:
@@ -927,7 +929,7 @@ impl Hypervisor for HypervLinuxDriver {
                     // - 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.clear_cancel_requested();
+                        interrupt_handle_internal.clear_cancel_requested();
                         HyperlightExit::Cancelled()
                     } else {
                         #[cfg(gdb)]
@@ -963,7 +965,7 @@ impl Hypervisor for HypervLinuxDriver {
         self as &mut dyn Hypervisor
     }
 
-    fn interrupt_handle(&self) -> Arc<dyn InterruptHandle> {
+    fn interrupt_handle(&self) -> Arc<dyn super::InterruptHandleInternal> {
         self.interrupt_handle.clone()
     }
 

src/hyperlight_host/src/hypervisor/hyperv_windows.rs

@@ -54,9 +54,8 @@ use super::{
     EFER_LMA, EFER_LME, EFER_NX, EFER_SCE,
 };
 use super::{HyperlightExit, Hypervisor, InterruptHandle, VirtualCPU};
-use crate::hypervisor::fpu::FP_CONTROL_WORD_DEFAULT;
-use crate::hypervisor::get_memory_access_violation;
-use crate::hypervisor::wrappers::WHvGeneralRegisters;
+use crate::hypervisor::regs::{CommonFpu, CommonRegisters};
+use crate::hypervisor::{InterruptHandleInternal, get_memory_access_violation};
 use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags};
 use crate::mem::ptr::{GuestPtr, RawPtr};
 use crate::mem::shared_mem::HostSharedMemory;
@@ -736,9 +735,16 @@ impl Hypervisor for HypervWindowsDriver {
     }
 
     #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")]
-    fn run(&mut self) -> Result<super::HyperlightExit> {
+    fn run(
+        &mut self,
+        #[cfg(feature = "trace_guest")] tc: &mut crate::sandbox::trace::TraceContext,
+    ) -> Result<super::HyperlightExit> {
+        // Cast to internal trait for access to internal methods
+        let interrupt_handle_internal =
+            self.interrupt_handle.as_ref() as &dyn super::InterruptHandleInternal;
+
         // Get current generation and set running bit
-        let generation = self.interrupt_handle.set_running_bit();
+        let generation = interrupt_handle_internal.set_running_bit();
 
         #[cfg(not(gdb))]
         let debug_interrupt = false;
@@ -749,8 +755,7 @@ impl Hypervisor for HypervWindowsDriver {
             .load(Ordering::Relaxed);
 
         // Check if cancellation was requested for THIS generation
-        let exit_context = if self
-            .interrupt_handle
+        let exit_context = if interrupt_handle_internal
             .is_cancel_requested_for_generation(generation)
             || debug_interrupt
         {
@@ -773,18 +778,17 @@ impl Hypervisor for HypervWindowsDriver {
         };
 
         // Clear running bit
-        self.interrupt_handle.clear_running_bit();
+        interrupt_handle_internal.clear_running_bit();
 
         let is_canceled = exit_context.ExitReason == WHV_RUN_VP_EXIT_REASON(8193i32); // WHvRunVpExitReasonCanceled
 
         // Check if this was a manual cancellation (vs internal Windows cancellation)
-        let cancel_was_requested_manually = self
-            .interrupt_handle
-            .is_cancel_requested_for_generation(generation);
+        let cancel_was_requested_manually =
+            interrupt_handle_internal.is_cancel_requested_for_generation(generation);
 
         // Only clear cancel_requested if we're actually processing a cancellation for this generation
         if is_canceled && cancel_was_requested_manually {
-            self.interrupt_handle.clear_cancel_requested();
+            interrupt_handle_internal.clear_cancel_requested();
         }
 
         #[cfg(gdb)]
@@ -915,7 +919,36 @@ impl Hypervisor for HypervWindowsDriver {
         Ok(result)
     }
 
-    fn interrupt_handle(&self) -> Arc<dyn InterruptHandle> {
+    /// Get regs
+    #[allow(dead_code)]
+    fn regs(&self) -> Result<CommonRegisters> {
+        self.processor.regs()
+    }
+    /// Set regs
+    fn set_regs(&mut self, regs: &CommonRegisters) -> Result<()> {
+        self.processor.set_regs(regs)
+    }
+    /// Get fpu regs
+    #[allow(dead_code)]
+    fn fpu(&self) -> Result<CommonFpu> {
+        self.processor.fpu()
+    }
+    /// Set fpu regs
+    fn set_fpu(&mut self, fpu: &CommonFpu) -> Result<()> {
+        self.processor.set_fpu(fpu)
+    }
+    /// Get special regs
+    #[allow(dead_code)]
+    fn sregs(&self) -> Result<CommonSpecialRegisters> {
+        self.processor.sregs()
+    }
+    /// Set special regs
+    #[allow(dead_code)]
+    fn set_sregs(&mut self, sregs: &CommonSpecialRegisters) -> Result<()> {
+        self.processor.set_sregs(sregs)
+    }
+
+    fn interrupt_handle(&self) -> Arc<dyn super::InterruptHandleInternal> {
         self.interrupt_handle.clone()
     }
 
@@ -1204,19 +1237,22 @@ impl InterruptHandle for WindowsInterruptHandle {
         // Only call WHvCancelRunVirtualProcessor if VCPU is actually running in guest mode
         running && unsafe { WHvCancelRunVirtualProcessor(self.partition_handle, 0, 0).is_ok() }
     }
+
     #[cfg(gdb)]
     fn kill_from_debugger(&self) -> bool {
         self.debug_interrupt.store(true, Ordering::Relaxed);
         let (running, _) = self.get_running_and_generation();
         running && unsafe { WHvCancelRunVirtualProcessor(self.partition_handle, 0, 0).is_ok() }
     }
 
-    fn get_call_active(&self) -> &AtomicBool {
-        &self.call_active
+    fn dropped(&self) -> bool {
+        self.dropped.load(Ordering::Relaxed)
     }
+}
 
-    fn get_dropped(&self) -> &AtomicBool {
-        &self.dropped
+impl InterruptHandleInternal for WindowsInterruptHandle {
+    fn get_call_active(&self) -> &AtomicBool {
+        &self.call_active
     }
 
     fn get_running(&self) -> &AtomicU64 {

src/hyperlight_host/src/hypervisor/kvm.rs

@@ -37,7 +37,7 @@ use super::{
     CR0_AM, CR0_ET, CR0_MP, CR0_NE, CR0_PE, CR0_PG, CR0_WP, CR4_OSFXSR, CR4_OSXMMEXCPT, CR4_PAE,
     EFER_LMA, EFER_LME, EFER_NX, EFER_SCE,
 };
-use super::{HyperlightExit, Hypervisor, InterruptHandle, LinuxInterruptHandle, VirtualCPU};
+use super::{HyperlightExit, Hypervisor, LinuxInterruptHandle, VirtualCPU};
 #[cfg(gdb)]
 use crate::HyperlightError;
 use crate::hypervisor::get_memory_access_violation;
@@ -667,11 +667,15 @@ impl Hypervisor for KVMDriver {
             .tid
             .store(unsafe { libc::pthread_self() as u64 }, Ordering::Release);
         // Note: if `InterruptHandle::kill()` is called while this thread is **here**
+        // Cast to internal trait for access to internal methods
+        let interrupt_handle_internal =
+            self.interrupt_handle.as_ref() as &dyn super::InterruptHandleInternal;
+
         // (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();
+        let generation = interrupt_handle_internal.set_running_bit();
 
         #[cfg(not(gdb))]
         let debug_interrupt = false;
@@ -687,8 +691,7 @@ impl Hypervisor for KVMDriver {
         // - 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
+        let exit_reason = if interrupt_handle_internal
             .is_cancel_requested_for_generation(generation)
             || debug_interrupt
         {
@@ -716,9 +719,8 @@ impl Hypervisor for KVMDriver {
         // - 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
-            .is_cancel_requested_for_generation(generation);
+        let cancel_requested =
+            interrupt_handle_internal.is_cancel_requested_for_generation(generation);
         #[cfg(gdb)]
         let debug_interrupt = self
             .interrupt_handle
@@ -730,7 +732,7 @@ impl Hypervisor for KVMDriver {
         // - 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();
+        interrupt_handle_internal.clear_running_bit();
         // 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:
@@ -794,7 +796,7 @@ impl Hypervisor for KVMDriver {
                     // - 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.clear_cancel_requested();
+                        interrupt_handle_internal.clear_cancel_requested();
                         HyperlightExit::Cancelled()
                     } else {
                         #[cfg(gdb)]
@@ -835,7 +837,7 @@ impl Hypervisor for KVMDriver {
         self as &mut dyn Hypervisor
     }
 
-    fn interrupt_handle(&self) -> Arc<dyn InterruptHandle> {
+    fn interrupt_handle(&self) -> Arc<dyn super::InterruptHandleInternal> {
         self.interrupt_handle.clone()
     }