Separate debugging related methods from Vm trait

Signed-off-by: Ludvig Liljenberg <[email protected]>

Author: ludfjig

src/hyperlight_host/src/hypervisor/gdb/arch.rs

@@ -16,10 +16,9 @@ limitations under the License.
 
 //! This file contains architecture specific code for the x86_64
 
-use super::VcpuStopReason;
+use super::{DebuggableVm, VcpuStopReason};
 use crate::Result;
 use crate::hypervisor::regs::CommonRegisters;
-use crate::hypervisor::vm::Vm;
 
 // Described in Table 6-1. Exceptions and Interrupts at Page 6-13 Vol. 1
 // of Intel 64 and IA-32 Architectures Software Developer's Manual
@@ -53,7 +52,7 @@ pub(crate) const DR6_HW_BP_FLAGS_MASK: u64 = 0x0F << DR6_HW_BP_FLAGS_POS;
 /// Determine the reason the vCPU stopped
 /// This is done by checking the DR6 register and the exception id
 pub(crate) fn vcpu_stop_reason(
-    vm: &mut dyn Vm,
+    vm: &mut dyn DebuggableVm,
     entrypoint: u64,
     dr6: u64,
     exception: u32,

src/hyperlight_host/src/hypervisor/gdb/mod.rs

@@ -33,6 +33,7 @@ use x86_64_target::HyperlightSandboxTarget;
 
 use super::InterruptHandle;
 use super::regs::CommonRegisters;
+use super::vm::Vm;
 use crate::hypervisor::regs::CommonFpu;
 use crate::mem::layout::SandboxMemoryLayout;
 use crate::mem::memory_region::MemoryRegion;
@@ -221,6 +222,26 @@ impl DebugMemoryAccess {
     }
 }
 
+/// Trait for VMs that support debugging capabilities.
+/// This extends the base Vm trait with GDB-specific functionality.
+pub(crate) trait DebuggableVm: Vm {
+    /// Translates a guest virtual address to a guest physical address
+    fn translate_gva(&self, gva: u64) -> crate::Result<u64>;
+
+    /// Enable/disable debugging
+    fn set_debug(&mut self, enable: bool) -> crate::Result<()>;
+
+    /// Enable/disable single stepping
+    fn set_single_step(&mut self, enable: bool) -> crate::Result<()>;
+
+    /// Add a hardware breakpoint at the given address.
+    /// Must be idempotent.
+    fn add_hw_breakpoint(&mut self, addr: u64) -> crate::Result<()>;
+
+    /// Remove a hardware breakpoint at the given address
+    fn remove_hw_breakpoint(&mut self, addr: u64) -> crate::Result<()>;
+}
+
 /// Defines the possible reasons for which a vCPU can be stopped when debugging
 #[derive(Debug)]
 pub enum VcpuStopReason {

src/hyperlight_host/src/hypervisor/hyperlight_vm.rs

@@ -34,8 +34,10 @@ use super::LinuxInterruptHandle;
 #[cfg(target_os = "windows")]
 use super::WindowsInterruptHandle;
 #[cfg(gdb)]
-use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse, VcpuStopReason, arch};
+use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse, DebuggableVm, VcpuStopReason, arch};
 use super::regs::{CommonFpu, CommonRegisters};
+#[cfg(not(gdb))]
+use super::vm::Vm;
 use super::{InterruptHandle, InterruptHandleImpl, get_max_log_level};
 use crate::HyperlightError::{ExecutionCanceledByHost, NoHypervisorFound};
 #[cfg(crashdump)]
@@ -47,7 +49,7 @@ use crate::hypervisor::hyperv_windows::WhpVm;
 #[cfg(kvm)]
 use crate::hypervisor::kvm::KvmVm;
 use crate::hypervisor::regs::CommonSpecialRegisters;
-use crate::hypervisor::vm::{Vm, VmExit};
+use crate::hypervisor::vm::VmExit;
 #[cfg(target_os = "windows")]
 use crate::hypervisor::wrappers::HandleWrapper;
 use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags, MemoryRegionType};
@@ -66,6 +68,9 @@ use crate::sandbox::uninitialized::SandboxRuntimeConfig;
 use crate::{HyperlightError, Result, log_then_return, new_error};
 
 pub(crate) struct HyperlightVm {
+    #[cfg(gdb)]
+    vm: Box<dyn DebuggableVm>,
+    #[cfg(not(gdb))]
     vm: Box<dyn Vm>,
     page_size: usize,
     entrypoint: u64,
@@ -105,8 +110,13 @@ impl HyperlightVm {
         #[cfg(crashdump)] rt_cfg: SandboxRuntimeConfig,
         #[cfg(feature = "mem_profile")] trace_info: MemTraceInfo,
     ) -> Result<Self> {
+        #[cfg(gdb)]
+        type VmType = Box<dyn DebuggableVm>;
+        #[cfg(not(gdb))]
+        type VmType = Box<dyn Vm>;
+
         #[allow(unused_mut)] // needs to be mutable when gdb is enabled
-        let mut vm: Box<dyn Vm> = match get_available_hypervisor() {
+        let mut vm: VmType = match get_available_hypervisor() {
             #[cfg(kvm)]
             Some(HypervisorType::Kvm) => Box::new(KvmVm::new()?),
             #[cfg(mshv3)]

src/hyperlight_host/src/hypervisor/hyperv_linux.rs

@@ -30,6 +30,8 @@ use mshv_bindings::{
 use mshv_ioctls::{Mshv, VcpuFd, VmFd};
 use tracing::{Span, instrument};
 
+#[cfg(gdb)]
+use crate::hypervisor::gdb::DebuggableVm;
 use crate::hypervisor::regs::{CommonFpu, CommonRegisters, CommonSpecialRegisters};
 use crate::hypervisor::vm::{Vm, VmExit};
 use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags};
@@ -202,10 +204,10 @@ impl Vm for MshvVm {
         };
         Ok(result)
     }
+}
 
-    // -- DEBUGGING RELATED BELOW ---
-
-    #[cfg(gdb)]
+#[cfg(gdb)]
+impl DebuggableVm for MshvVm {
     fn translate_gva(&self, gva: u64) -> Result<u64> {
         use mshv_bindings::{HV_TRANSLATE_GVA_VALIDATE_READ, HV_TRANSLATE_GVA_VALIDATE_WRITE};
 
@@ -220,7 +222,6 @@ impl Vm for MshvVm {
         Ok(addr)
     }
 
-    #[cfg(gdb)]
     fn set_debug(&mut self, enabled: bool) -> Result<()> {
         use mshv_bindings::{
             HV_INTERCEPT_ACCESS_MASK_EXECUTE, hv_intercept_parameters,
@@ -259,7 +260,6 @@ impl Vm for MshvVm {
         Ok(())
     }
 
-    #[cfg(gdb)]
     fn set_single_step(&mut self, enable: bool) -> Result<()> {
         let mut regs = self.regs()?;
         if enable {
@@ -271,7 +271,6 @@ impl Vm for MshvVm {
         Ok(())
     }
 
-    #[cfg(gdb)]
     fn add_hw_breakpoint(&mut self, addr: u64) -> Result<()> {
         use crate::hypervisor::gdb::arch::MAX_NO_OF_HW_BP;
         use crate::new_error;
@@ -310,7 +309,6 @@ impl Vm for MshvVm {
         Ok(())
     }
 
-    #[cfg(gdb)]
     fn remove_hw_breakpoint(&mut self, addr: u64) -> Result<()> {
         use crate::new_error;
 

src/hyperlight_host/src/hypervisor/hyperv_windows.rs

@@ -34,6 +34,8 @@ use super::vm::{Vm, VmExit};
 #[cfg(not(gdb))]
 use super::vm::{Vm, VmExit};
 use super::wrappers::HandleWrapper;
+#[cfg(gdb)]
+use crate::hypervisor::gdb::DebuggableVm;
 use crate::hypervisor::regs::{CommonFpu, CommonRegisters, CommonSpecialRegisters};
 use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags};
 use crate::{Result, log_then_return, new_error};
@@ -471,7 +473,14 @@ impl Vm for WhpVm {
         Ok(result)
     }
 
-    #[cfg(gdb)]
+    /// Mark that initial memory setup is complete. After this, map_memory will fail.
+    fn complete_initial_memory_setup(&mut self) {
+        self.initial_memory_setup_done = true;
+    }
+}
+
+#[cfg(gdb)]
+impl DebuggableVm for WhpVm {
     fn translate_gva(&self, gva: u64) -> Result<u64> {
         let mut gpa = 0;
         let mut result = WHV_TRANSLATE_GVA_RESULT::default();
@@ -494,7 +503,6 @@ impl Vm for WhpVm {
         Ok(gpa)
     }
 
-    #[cfg(gdb)]
     fn set_debug(&mut self, enable: bool) -> Result<()> {
         if enable {
             // Set the extended VM exits property to enable extended VM exits
@@ -527,7 +535,6 @@ impl Vm for WhpVm {
         Ok(())
     }
 
-    #[cfg(gdb)]
     fn set_single_step(&mut self, enable: bool) -> Result<()> {
         let mut regs = self.regs()?;
         if enable {
@@ -539,7 +546,6 @@ impl Vm for WhpVm {
         Ok(())
     }
 
-    #[cfg(gdb)]
     fn add_hw_breakpoint(&mut self, addr: u64) -> Result<()> {
         use crate::hypervisor::gdb::arch::MAX_NO_OF_HW_BP;
 
@@ -599,7 +605,6 @@ impl Vm for WhpVm {
         Ok(())
     }
 
-    #[cfg(gdb)]
     fn remove_hw_breakpoint(&mut self, addr: u64) -> Result<()> {
         // Get current debug registers
         const LEN: usize = 6;
@@ -651,11 +656,6 @@ impl Vm for WhpVm {
             Err(new_error!("Tried to remove non-existing hw-breakpoint"))
         }
     }
-
-    /// Mark that initial memory setup is complete. After this, map_memory will fail.
-    fn complete_initial_memory_setup(&mut self) {
-        self.initial_memory_setup_done = true;
-    }
 }
 
 impl Drop for WhpVm {

src/hyperlight_host/src/hypervisor/kvm.rs

@@ -23,6 +23,8 @@ use kvm_ioctls::Cap::UserMemory;
 use kvm_ioctls::{Kvm, VcpuExit, VcpuFd, VmFd};
 use tracing::{Span, instrument};
 
+#[cfg(gdb)]
+use crate::hypervisor::gdb::DebuggableVm;
 use crate::hypervisor::regs::{CommonFpu, CommonRegisters, CommonSpecialRegisters};
 use crate::hypervisor::vm::{Vm, VmExit};
 use crate::mem::memory_region::MemoryRegion;
@@ -161,10 +163,10 @@ impl Vm for KvmVm {
             ))),
         }
     }
+}
 
-    // --- DEBUGGING RELATED BELOW ---
-
-    #[cfg(gdb)]
+#[cfg(gdb)]
+impl DebuggableVm for KvmVm {
     fn translate_gva(&self, gva: u64) -> Result<u64> {
         use crate::HyperlightError;
 
@@ -176,7 +178,6 @@ impl Vm for KvmVm {
         }
     }
 
-    #[cfg(gdb)]
     fn set_debug(&mut self, enable: bool) -> Result<()> {
         use kvm_bindings::{KVM_GUESTDBG_ENABLE, KVM_GUESTDBG_USE_HW_BP, KVM_GUESTDBG_USE_SW_BP};
 
@@ -192,7 +193,6 @@ impl Vm for KvmVm {
         Ok(())
     }
 
-    #[cfg(gdb)]
     fn set_single_step(&mut self, enable: bool) -> Result<()> {
         use kvm_bindings::KVM_GUESTDBG_SINGLESTEP;
 
@@ -213,7 +213,6 @@ impl Vm for KvmVm {
         Ok(())
     }
 
-    #[cfg(gdb)]
     fn add_hw_breakpoint(&mut self, addr: u64) -> Result<()> {
         use crate::hypervisor::gdb::arch::MAX_NO_OF_HW_BP;
         use crate::new_error;
@@ -238,7 +237,6 @@ impl Vm for KvmVm {
         Ok(())
     }
 
-    #[cfg(gdb)]
     fn remove_hw_breakpoint(&mut self, addr: u64) -> Result<()> {
         use crate::new_error;
 

src/hyperlight_host/src/hypervisor/vm.rs

@@ -59,31 +59,6 @@ pub(crate) trait Vm: Send + Sync + Debug {
     /// Runs the vCPU until it exits
     fn run_vcpu(&mut self) -> Result<VmExit>;
 
-    // --------------------------
-    // --- DEBUGGING BELOW ------
-    // --------------------------
-
-    /// Translates a guest virtual address to a guest physical address
-    #[cfg(gdb)]
-    fn translate_gva(&self, gva: u64) -> Result<u64>;
-
-    /// Enable/disable debugging
-    #[cfg(gdb)]
-    fn set_debug(&mut self, enable: bool) -> Result<()>;
-
-    /// Enable/disable single stepping
-    #[cfg(gdb)]
-    fn set_single_step(&mut self, enable: bool) -> Result<()>;
-
-    /// Add a hardware breakpoint at the given address.
-    /// Must be idempotent.
-    #[cfg(gdb)]
-    fn add_hw_breakpoint(&mut self, addr: u64) -> Result<()>;
-
-    /// Remove a hardware breakpoint at the given address
-    #[cfg(gdb)]
-    fn remove_hw_breakpoint(&mut self, addr: u64) -> Result<()>;
-
     /// Get partition handle
     #[cfg(target_os = "windows")]
     fn partition_handle(&self) -> windows::Win32::System::Hypervisor::WHV_PARTITION_HANDLE;