Richer error handling for entire HyperlightVm

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

Author: ludfjig

src/hyperlight_host/src/error.rs

@@ -30,6 +30,7 @@ use hyperlight_common::flatbuffer_wrappers::function_types::{ParameterValue, Ret
 use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode;
 use thiserror::Error;
 
+use crate::hypervisor::hyperlight_vm::HyperlightVmError;
 #[cfg(target_os = "windows")]
 use crate::hypervisor::wrappers::HandleWrapper;
 use crate::mem::memory_region::MemoryRegionFlags;
@@ -111,6 +112,14 @@ pub enum HyperlightError {
     #[error("HostFunction {0} was not found")]
     HostFunctionNotFound(String),
 
+    /// Hyperlight VM error.
+    ///
+    /// **Note:** This error variant is considered internal and its structure is not stable.
+    /// It may change between versions without notice. Users should not rely on this.
+    #[doc(hidden)]
+    #[error("Internal Hyperlight VM error: {0}")]
+    HyperlightVmError(#[from] HyperlightVmError),
+
     /// Reading Writing or Seeking data failed.
     #[error("Reading Writing or Seeking data failed {0:?}")]
     IOError(#[from] std::io::Error),
@@ -127,11 +136,6 @@ pub enum HyperlightError {
     #[error("Conversion of str data to json failed")]
     JsonConversionFailure(#[from] serde_json::Error),
 
-    /// KVM Error Occurred
-    #[error("KVM Error {0:?}")]
-    #[cfg(kvm)]
-    KVMError(#[from] kvm_ioctls::Error),
-
     /// An attempt to get a lock from a Mutex failed.
     #[error("Unable to lock resource")]
     LockAttemptFailed(String),
@@ -168,11 +172,6 @@ pub enum HyperlightError {
     #[error("mprotect failed with os error {0:?}")]
     MprotectFailed(Option<i32>),
 
-    /// mshv Error Occurred
-    #[error("mshv Error {0:?}")]
-    #[cfg(mshv3)]
-    MSHVError(#[from] mshv_ioctls::MshvError),
-
     /// No Hypervisor was found for Sandbox.
     #[error("No Hypervisor was found for Sandbox")]
     NoHypervisorFound(),
@@ -242,11 +241,6 @@ pub enum HyperlightError {
     #[error("SystemTimeError {0:?}")]
     SystemTimeError(#[from] SystemTimeError),
 
-    /// Error occurred when translating guest address
-    #[error("An error occurred when translating guest address: {0:?}")]
-    #[cfg(gdb)]
-    TranslateGuestAddress(u64),
-
     /// Error occurred converting a slice to an array
     #[error("TryFromSliceError {0:?}")]
     TryFromSliceError(#[from] TryFromSliceError),
@@ -334,6 +328,11 @@ impl HyperlightError {
             | HyperlightError::SnapshotSizeMismatch(_, _)
             | HyperlightError::MemoryRegionSizeMismatch(_, _, _) => true,
 
+            // HyperlightVmError::DispatchGuestCall may poison the sandbox
+            HyperlightError::HyperlightVmError(HyperlightVmError::DispatchGuestCall(e)) => {
+                e.is_poison_error()
+            }
+
             // All other errors do not poison the sandbox.
             HyperlightError::AnyhowError(_)
             | HyperlightError::BoundsCheckFailed(_, _)
@@ -348,6 +347,10 @@ impl HyperlightError {
             | HyperlightError::GuestInterfaceUnsupportedType(_)
             | HyperlightError::GuestOffsetIsInvalid(_)
             | HyperlightError::HostFunctionNotFound(_)
+            | HyperlightError::HyperlightVmError(HyperlightVmError::Create(_))
+            | HyperlightError::HyperlightVmError(HyperlightVmError::Initialize(_))
+            | HyperlightError::HyperlightVmError(HyperlightVmError::MapRegion(_))
+            | HyperlightError::HyperlightVmError(HyperlightVmError::UnmapRegion(_))
             | HyperlightError::IOError(_)
             | HyperlightError::IntConversionFailure(_)
             | HyperlightError::InvalidFlatBuffer(_)
@@ -384,12 +387,6 @@ impl HyperlightError {
             HyperlightError::WindowsAPIError(_) => false,
             #[cfg(target_os = "linux")]
             HyperlightError::VmmSysError(_) => false,
-            #[cfg(kvm)]
-            HyperlightError::KVMError(_) => false,
-            #[cfg(mshv3)]
-            HyperlightError::MSHVError(_) => false,
-            #[cfg(gdb)]
-            HyperlightError::TranslateGuestAddress(_) => false,
         }
     }
 }

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

@@ -16,9 +16,18 @@ limitations under the License.
 
 //! This file contains architecture specific code for the x86_64
 
-use super::{DebuggableVm, VcpuStopReason};
-use crate::Result;
+use super::{DebugError, DebuggableVm, VcpuStopReason};
 use crate::hypervisor::regs::CommonRegisters;
+use crate::hypervisor::virtual_machine::RegisterError;
+
+/// Errors that can occur when determining the vCPU stop reason
+#[derive(Debug, thiserror::Error)]
+pub enum VcpuStopReasonError {
+    #[error("Failed to get registers: {0}")]
+    GetRegs(#[from] RegisterError),
+    #[error("Failed to remove hardware breakpoint: {0}")]
+    RemoveHwBreakpoint(#[from] DebugError),
+}
 
 // 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
@@ -56,7 +65,7 @@ pub(crate) fn vcpu_stop_reason(
     dr6: u64,
     entrypoint: u64,
     exception: u32,
-) -> Result<VcpuStopReason> {
+) -> std::result::Result<VcpuStopReason, VcpuStopReasonError> {
     let CommonRegisters { rip, .. } = vm.regs()?;
     if DB_EX_ID == exception {
         // If the BS flag in DR6 register is set, it means a single step

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

@@ -33,16 +33,16 @@ use x86_64_target::HyperlightSandboxTarget;
 
 use super::InterruptHandle;
 use super::regs::CommonRegisters;
+use crate::HyperlightError;
 use crate::hypervisor::regs::CommonFpu;
-use crate::hypervisor::virtual_machine::VirtualMachine;
+use crate::hypervisor::virtual_machine::{HypervisorError, RegisterError, VirtualMachine};
 use crate::mem::layout::SandboxMemoryLayout;
 use crate::mem::memory_region::MemoryRegion;
 use crate::mem::mgr::SandboxMemoryManager;
 use crate::mem::shared_mem::HostSharedMemory;
-use crate::{HyperlightError, new_error};
 
 #[derive(Debug, Error)]
-pub(crate) enum GdbTargetError {
+pub enum GdbTargetError {
     #[error("Error encountered while binding to address and port")]
     CannotBind,
     #[error("Error encountered while listening for connections")]
@@ -86,6 +86,17 @@ pub(crate) struct DebugMemoryAccess {
     pub(crate) guest_mmap_regions: Vec<MemoryRegion>,
 }
 
+/// Errors that can occur during debug memory access operations
+#[derive(Debug, thiserror::Error)]
+pub enum DebugMemoryAccessError {
+    #[error("Failed to copy memory: {0}")]
+    CopyFailed(Box<HyperlightError>),
+    #[error("Failed to acquire lock at {0}:{1} - {2}")]
+    LockFailed(&'static str, u32, String),
+    #[error("Failed to translate guest address {0:#x}")]
+    TranslateGuestAddress(u64),
+}
+
 impl DebugMemoryAccess {
     /// Reads memory from the guest's address space with a maximum length of a PAGE_SIZE
     ///
@@ -94,8 +105,12 @@ impl DebugMemoryAccess {
     /// * `gpa` - Guest physical address to read from.
     ///   This address is shall be translated before calling this function
     /// # Returns
-    /// * `Result<(), HyperlightError>` - Ok if successful, Err otherwise
-    pub(crate) fn read(&self, data: &mut [u8], gpa: u64) -> crate::Result<()> {
+    /// * `Result<(), DebugMemoryAccessError>` - Ok if successful, Err otherwise
+    pub(crate) fn read(
+        &self,
+        data: &mut [u8],
+        gpa: u64,
+    ) -> std::result::Result<(), DebugMemoryAccessError> {
         let read_len = data.len();
 
         let mem_offset = (gpa as usize)
@@ -107,7 +122,7 @@ impl DebugMemoryAccess {
                     gpa,
                     SandboxMemoryLayout::BASE_ADDRESS
                 );
-                HyperlightError::TranslateGuestAddress(gpa)
+                DebugMemoryAccessError::TranslateGuestAddress(gpa)
             })?;
 
         // First check the mapped memory regions to see if the address is within any of them
@@ -123,7 +138,7 @@ impl DebugMemoryAccess {
                         mem_offset,
                         reg.guest_region.start,
                     );
-                    HyperlightError::TranslateGuestAddress(mem_offset as u64)
+                    DebugMemoryAccessError::TranslateGuestAddress(mem_offset as u64)
                 })?;
 
                 let bytes: &[u8] = unsafe {
@@ -144,9 +159,10 @@ impl DebugMemoryAccess {
 
             self.dbg_mem_access_fn
                 .try_lock()
-                .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?
+                .map_err(|e| DebugMemoryAccessError::LockFailed(file!(), line!(), e.to_string()))?
                 .get_shared_mem_mut()
-                .copy_to_slice(&mut data[..read_len], mem_offset)?;
+                .copy_to_slice(&mut data[..read_len], mem_offset)
+                .map_err(|e| DebugMemoryAccessError::CopyFailed(Box::new(e)))?;
         }
 
         Ok(())
@@ -159,8 +175,12 @@ impl DebugMemoryAccess {
     /// * `gpa` - Guest physical address to write to.
     ///   This address is shall be translated before calling this function
     /// # Returns
-    /// * `Result<(), HyperlightError>` - Ok if successful, Err otherwise
-    pub(crate) fn write(&self, data: &[u8], gpa: u64) -> crate::Result<()> {
+    /// * `Result<(), DebugMemoryAccessError>` - Ok if successful, Err otherwise
+    pub(crate) fn write(
+        &self,
+        data: &[u8],
+        gpa: u64,
+    ) -> std::result::Result<(), DebugMemoryAccessError> {
         let write_len = data.len();
 
         let mem_offset = (gpa as usize)
@@ -172,7 +192,7 @@ impl DebugMemoryAccess {
                     gpa,
                     SandboxMemoryLayout::BASE_ADDRESS
                 );
-                HyperlightError::TranslateGuestAddress(gpa)
+                DebugMemoryAccessError::TranslateGuestAddress(gpa)
             })?;
 
         // First check the mapped memory regions to see if the address is within any of them
@@ -188,7 +208,7 @@ impl DebugMemoryAccess {
                         mem_offset,
                         reg.guest_region.start,
                     );
-                    HyperlightError::TranslateGuestAddress(mem_offset as u64)
+                    DebugMemoryAccessError::TranslateGuestAddress(mem_offset as u64)
                 })?;
 
                 let bytes: &mut [u8] = unsafe {
@@ -213,9 +233,10 @@ impl DebugMemoryAccess {
 
             self.dbg_mem_access_fn
                 .try_lock()
-                .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?
+                .map_err(|e| DebugMemoryAccessError::LockFailed(file!(), line!(), e.to_string()))?
                 .get_shared_mem_mut()
-                .copy_from_slice(&data[..write_len], mem_offset)?;
+                .copy_from_slice(&data[..write_len], mem_offset)
+                .map_err(|e| DebugMemoryAccessError::CopyFailed(Box::new(e)))?;
         }
 
         Ok(())
@@ -275,24 +296,42 @@ pub(crate) enum DebugResponse {
     WriteRegisters,
 }
 
+/// Errors that can occur during debug operations
+#[derive(Debug, Clone, thiserror::Error)]
+pub enum DebugError {
+    #[error("Hardware breakpoint not found at address {0:#x}")]
+    HwBreakpointNotFound(u64),
+    #[error("Failed to enable/disable intercept: {enable}, {inner}")]
+    Intercept {
+        enable: bool,
+        inner: HypervisorError,
+    },
+    #[error("Register operation failed: {0}")]
+    Register(#[from] RegisterError),
+    #[error("Maximum hardware breakpoints ({0}) exceeded")]
+    TooManyHwBreakpoints(usize),
+    #[error("Translation of guest virtual address failed: {0}")]
+    TranslateGva(u64),
+}
+
 /// Trait for VMs that support debugging capabilities.
 /// This extends the base VirtualMachine trait with GDB-specific functionality.
 pub(crate) trait DebuggableVm: VirtualMachine {
     /// Translates a guest virtual address to a guest physical address
-    fn translate_gva(&self, gva: u64) -> crate::Result<u64>;
+    fn translate_gva(&self, gva: u64) -> std::result::Result<u64, DebugError>;
 
     /// Enable/disable debugging
-    fn set_debug(&mut self, enable: bool) -> crate::Result<()>;
+    fn set_debug(&mut self, enable: bool) -> std::result::Result<(), DebugError>;
 
     /// Enable/disable single stepping
-    fn set_single_step(&mut self, enable: bool) -> crate::Result<()>;
+    fn set_single_step(&mut self, enable: bool) -> std::result::Result<(), DebugError>;
 
     /// Add a hardware breakpoint at the given address.
     /// Must be idempotent.
-    fn add_hw_breakpoint(&mut self, addr: u64) -> crate::Result<()>;
+    fn add_hw_breakpoint(&mut self, addr: u64) -> std::result::Result<(), DebugError>;
 
     /// Remove a hardware breakpoint at the given address
-    fn remove_hw_breakpoint(&mut self, addr: u64) -> crate::Result<()>;
+    fn remove_hw_breakpoint(&mut self, addr: u64) -> std::result::Result<(), DebugError>;
 }
 
 /// Debug communication channel that is used for sending a request type and
@@ -513,7 +552,9 @@ mod tests {
             }
 
             let mut read_data = [0u8; 1];
-            mem_access.read(&mut read_data, (BASE_VIRT + offset) as u64)?;
+            mem_access
+                .read(&mut read_data, (BASE_VIRT + offset) as u64)
+                .unwrap();
 
             assert_eq!(read_data[0], 0xAA);
 
@@ -536,7 +577,9 @@ mod tests {
             }
 
             let mut read_data = [0u8; 16];
-            mem_access.read(&mut read_data, (BASE_VIRT + offset) as u64)?;
+            mem_access
+                .read(&mut read_data, (BASE_VIRT + offset) as u64)
+                .unwrap();
 
             assert_eq!(
                 read_data,
@@ -556,7 +599,9 @@ mod tests {
             }
 
             let write_data = [0xCCu8; 1];
-            mem_access.write(&write_data, (BASE_VIRT + offset) as u64)?;
+            mem_access
+                .write(&write_data, (BASE_VIRT + offset) as u64)
+                .unwrap();
 
             let slice = unsafe { get_mmap_slice(&mut mem_access) };
             assert_eq!(slice[offset], write_data[0]);
@@ -577,7 +622,9 @@ mod tests {
             }
 
             let write_data = [0xAAu8; 16];
-            mem_access.write(&write_data, (BASE_VIRT + offset) as u64)?;
+            mem_access
+                .write(&write_data, (BASE_VIRT + offset) as u64)
+                .unwrap();
 
             let slice = unsafe { get_mmap_slice(&mut mem_access) };
             assert_eq!(slice[offset..offset + 16], write_data);