From 21be879a78ef8eab739f70aa30b56dee4bfd2799 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Wed, 19 Mar 2025 13:41:49 +0200 Subject: [PATCH 01/17] dbg: change visibility of debugging related functions/structs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/gdb/event_loop.rs | 4 +- src/hyperlight_host/src/hypervisor/gdb/mod.rs | 58 +++++++++---------- .../src/hypervisor/gdb/x86_64_target.rs | 14 ++--- src/hyperlight_host/src/hypervisor/kvm.rs | 18 +++--- 4 files changed, 47 insertions(+), 47 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs b/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs index ee009caa1..5f885b0ac 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs @@ -23,7 +23,7 @@ use libc::{pthread_kill, SIGRTMIN}; use super::x86_64_target::HyperlightSandboxTarget; use super::{DebugResponse, GdbTargetError, VcpuStopReason}; -pub struct GdbBlockingEventLoop; +struct GdbBlockingEventLoop; impl run_blocking::BlockingEventLoop for GdbBlockingEventLoop { type Connection = Box>; @@ -115,7 +115,7 @@ impl run_blocking::BlockingEventLoop for GdbBlockingEventLoop { } } -pub fn event_loop_thread( +pub(crate) fn event_loop_thread( debugger: GdbStub>>, target: &mut HyperlightSandboxTarget, ) { diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index b941040a3..f5e532493 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -15,7 +15,7 @@ limitations under the License. */ mod event_loop; -pub mod x86_64_target; +mod x86_64_target; use std::io::{self, ErrorKind}; use std::net::TcpListener; @@ -30,7 +30,7 @@ use thiserror::Error; use x86_64_target::HyperlightSandboxTarget; #[derive(Debug, Error)] -pub enum GdbTargetError { +pub(crate) enum GdbTargetError { #[error("Error encountered while binding to address and port")] CannotBind, #[error("Error encountered while listening for connections")] @@ -68,25 +68,25 @@ impl From for TargetError { /// Struct that contains the x86_64 core registers #[derive(Debug, Default)] -pub struct X86_64Regs { - pub rax: u64, - pub rbx: u64, - pub rcx: u64, - pub rdx: u64, - pub rsi: u64, - pub rdi: u64, - pub rbp: u64, - pub rsp: u64, - pub r8: u64, - pub r9: u64, - pub r10: u64, - pub r11: u64, - pub r12: u64, - pub r13: u64, - pub r14: u64, - pub r15: u64, - pub rip: u64, - pub rflags: u64, +pub(crate) struct X86_64Regs { + pub(crate) rax: u64, + pub(crate) rbx: u64, + pub(crate) rcx: u64, + pub(crate) rdx: u64, + pub(crate) rsi: u64, + pub(crate) rdi: u64, + pub(crate) rbp: u64, + pub(crate) rsp: u64, + pub(crate) r8: u64, + pub(crate) r9: u64, + pub(crate) r10: u64, + pub(crate) r11: u64, + pub(crate) r12: u64, + pub(crate) r13: u64, + pub(crate) r14: u64, + pub(crate) r15: u64, + pub(crate) rip: u64, + pub(crate) rflags: u64, } /// Defines the possible reasons for which a vCPU can be stopped when debugging @@ -101,7 +101,7 @@ pub enum VcpuStopReason { /// Enumerates the possible actions that a debugger can ask from a Hypervisor #[derive(Debug)] -pub enum DebugMsg { +pub(crate) enum DebugMsg { AddHwBreakpoint(u64), AddSwBreakpoint(u64), Continue, @@ -118,7 +118,7 @@ pub enum DebugMsg { /// Enumerates the possible responses that a hypervisor can provide to a debugger #[derive(Debug)] -pub enum DebugResponse { +pub(crate) enum DebugResponse { AddHwBreakpoint(bool), AddSwBreakpoint(bool), Continue, @@ -137,7 +137,7 @@ pub enum DebugResponse { /// Debug communication channel that is used for sending a request type and /// receive a different response type -pub struct DebugCommChannel { +pub(crate) struct DebugCommChannel { /// Transmit channel tx: Sender, /// Receive channel @@ -145,7 +145,7 @@ pub struct DebugCommChannel { } impl DebugCommChannel { - pub fn unbounded() -> (DebugCommChannel, DebugCommChannel) { + pub(crate) fn unbounded() -> (DebugCommChannel, DebugCommChannel) { let (hyp_tx, gdb_rx): (Sender, Receiver) = crossbeam_channel::unbounded(); let (gdb_tx, hyp_rx): (Sender, Receiver) = crossbeam_channel::unbounded(); @@ -163,23 +163,23 @@ impl DebugCommChannel { } /// Sends message over the transmit channel and expects a response - pub fn send(&self, msg: T) -> Result<(), GdbTargetError> { + pub(crate) fn send(&self, msg: T) -> Result<(), GdbTargetError> { self.tx.send(msg).map_err(|_| GdbTargetError::CannotSendMsg) } /// Waits for a message over the receive channel - pub fn recv(&self) -> Result { + pub(crate) fn recv(&self) -> Result { self.rx.recv().map_err(|_| GdbTargetError::CannotReceiveMsg) } /// Checks whether there's a message waiting on the receive channel - pub fn try_recv(&self) -> Result { + pub(crate) fn try_recv(&self) -> Result { self.rx.try_recv() } } /// Creates a thread that handles gdb protocol -pub fn create_gdb_thread( +pub(crate) fn create_gdb_thread( port: u16, thread_id: u64, ) -> Result, GdbTargetError> { diff --git a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs index 51bca3d19..fbdea448a 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs @@ -32,7 +32,7 @@ use gdbstub_arch::x86::X86_64_SSE as GdbTargetArch; use super::{DebugCommChannel, DebugMsg, DebugResponse, GdbTargetError, X86_64Regs}; /// Gdbstub target used by the gdbstub crate to provide GDB protocol implementation -pub struct HyperlightSandboxTarget { +pub(crate) struct HyperlightSandboxTarget { /// Hypervisor communication channels hyp_conn: DebugCommChannel, /// Thread ID @@ -40,7 +40,7 @@ pub struct HyperlightSandboxTarget { } impl HyperlightSandboxTarget { - pub fn new(hyp_conn: DebugCommChannel, thread_id: u64) -> Self { + pub(crate) fn new(hyp_conn: DebugCommChannel, thread_id: u64) -> Self { HyperlightSandboxTarget { hyp_conn, thread_id, @@ -61,23 +61,23 @@ impl HyperlightSandboxTarget { } /// Returns the thread ID - pub fn get_thread_id(&self) -> u64 { + pub(crate) fn get_thread_id(&self) -> u64 { self.thread_id } /// Waits for a response over the communication channel - pub fn recv(&self) -> Result { + pub(crate) fn recv(&self) -> Result { self.hyp_conn.recv() } /// Non-Blocking check for a response over the communication channel - pub fn try_recv(&self) -> Result { + pub(crate) fn try_recv(&self) -> Result { self.hyp_conn.try_recv() } /// Sends an event to the Hypervisor that tells it to resume vCPU execution /// Note: The method waits for a confirmation message - pub fn resume_vcpu(&mut self) -> Result<(), GdbTargetError> { + pub(crate) fn resume_vcpu(&mut self) -> Result<(), GdbTargetError> { log::info!("Resume vCPU execution"); match self.send_command(DebugMsg::Continue)? { @@ -92,7 +92,7 @@ impl HyperlightSandboxTarget { /// Sends an event to the Hypervisor that tells it to disable debugging /// and continue executing /// Note: The method waits for a confirmation message - pub fn disable_debug(&mut self) -> Result<(), GdbTargetError> { + pub(crate) fn disable_debug(&mut self) -> Result<(), GdbTargetError> { log::info!("Disable debugging and resume execution"); match self.send_command(DebugMsg::DisableDebug)? { diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 5d01ec8ac..3f33980c7 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -82,17 +82,17 @@ mod debug { use crate::{new_error, HyperlightError, Result}; /// Software Breakpoint size in memory - pub const SW_BP_SIZE: usize = 1; + pub(crate) const SW_BP_SIZE: usize = 1; /// Software Breakpoint opcode const SW_BP_OP: u8 = 0xCC; /// Software Breakpoint written to memory - pub const SW_BP: [u8; SW_BP_SIZE] = [SW_BP_OP]; + pub(crate) const SW_BP: [u8; SW_BP_SIZE] = [SW_BP_OP]; /// KVM Debug struct /// This struct is used to abstract the internal details of the kvm /// guest debugging settings #[derive(Default)] - pub struct KvmDebug { + pub(crate) struct KvmDebug { /// vCPU stepping state single_step: bool, @@ -108,7 +108,7 @@ mod debug { impl KvmDebug { const MAX_NO_OF_HW_BP: usize = 4; - pub fn new() -> Self { + pub(crate) fn new() -> Self { let dbg = kvm_guest_debug { control: KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP, ..Default::default() @@ -436,7 +436,7 @@ mod debug { /// Gdb expects the target to be stopped when connected. /// This method provides a way to set a breakpoint at the entry point /// it does not keep this breakpoint set after the vCPU already stopped at the address - pub fn set_entrypoint_bp(&self) -> Result<()> { + pub(crate) fn set_entrypoint_bp(&self) -> Result<()> { if self.debug.is_some() { log::debug!("Setting entrypoint bp {:X}", self.entrypoint); let mut entrypoint_debug = KvmDebug::new(); @@ -449,7 +449,7 @@ mod debug { } /// Get the reason the vCPU has stopped - pub fn get_stop_reason(&self) -> Result { + pub(crate) fn get_stop_reason(&self) -> Result { let debug = self .debug .as_ref() @@ -476,7 +476,7 @@ mod debug { Ok(VcpuStopReason::Unknown) } - pub fn process_dbg_request( + pub(crate) fn process_dbg_request( &mut self, req: DebugMsg, dbg_mem_access_fn: Arc>, @@ -539,7 +539,7 @@ mod debug { } } - pub fn recv_dbg_msg(&mut self) -> Result { + pub(crate) fn recv_dbg_msg(&mut self) -> Result { let gdb_conn = self .gdb_conn .as_mut() @@ -553,7 +553,7 @@ mod debug { }) } - pub fn send_dbg_msg(&mut self, cmd: DebugResponse) -> Result<()> { + pub(crate) fn send_dbg_msg(&mut self, cmd: DebugResponse) -> Result<()> { log::debug!("Sending {:?}", cmd); let gdb_conn = self From 77b69fca76590664cf5cdac9622afd3400fd15c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Mon, 3 Mar 2025 12:35:09 +0200 Subject: [PATCH 02/17] dbg: move kvm vcpu debug type to another file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - this is done in preparation of accomodating support for other hypervisors which means some of the functionality whould be common. - moving the debug code to a separate file can enable us to group common behavior in traits Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/gdb/kvm_debug.rs | 126 ++++++++++++++++++ src/hyperlight_host/src/hypervisor/gdb/mod.rs | 13 ++ src/hyperlight_host/src/hypervisor/kvm.rs | 124 +---------------- 3 files changed, 146 insertions(+), 117 deletions(-) create mode 100644 src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs diff --git a/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs b/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs new file mode 100644 index 000000000..ec8163a77 --- /dev/null +++ b/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs @@ -0,0 +1,126 @@ +/* +Copyright 2024 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 std::collections::HashMap; + +use kvm_bindings::{ + kvm_guest_debug, KVM_GUESTDBG_ENABLE, KVM_GUESTDBG_SINGLESTEP, KVM_GUESTDBG_USE_HW_BP, + KVM_GUESTDBG_USE_SW_BP, +}; +use kvm_ioctls::VcpuFd; + +use super::*; +use crate::{new_error, Result}; + +/// KVM Debug struct +/// This struct is used to abstract the internal details of the kvm +/// guest debugging settings +#[derive(Default)] +pub(crate) struct KvmDebug { + /// vCPU stepping state + pub(crate) single_step: bool, + + /// Array of addresses for HW breakpoints + pub(crate) hw_breakpoints: Vec, + /// Saves the bytes modified to enable SW breakpoints + pub(crate) sw_breakpoints: HashMap, + + /// Sent to KVM for enabling guest debug + dbg_cfg: kvm_guest_debug, +} + +impl KvmDebug { + const MAX_NO_OF_HW_BP: usize = 4; + + pub(crate) fn new() -> Self { + let dbg = kvm_guest_debug { + control: KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP, + ..Default::default() + }; + + Self { + single_step: false, + hw_breakpoints: vec![], + sw_breakpoints: HashMap::new(), + dbg_cfg: dbg, + } + } + + /// This method sets the kvm debugreg fields to enable breakpoints at + /// specific addresses + /// + /// The first 4 debug registers are used to set the addresses + /// The 4th and 5th debug registers are obsolete and not used + /// The 7th debug register is used to enable the breakpoints + /// For more information see: DEBUG REGISTERS chapter in the architecture + /// manual + pub(crate) fn set_debug_config(&mut self, vcpu_fd: &VcpuFd, step: bool) -> Result<()> { + let addrs = &self.hw_breakpoints; + + self.dbg_cfg.arch.debugreg = [0; 8]; + for (k, addr) in addrs.iter().enumerate() { + self.dbg_cfg.arch.debugreg[k] = *addr; + self.dbg_cfg.arch.debugreg[7] |= 1 << (k * 2); + } + + if !addrs.is_empty() { + self.dbg_cfg.control |= KVM_GUESTDBG_USE_HW_BP; + } else { + self.dbg_cfg.control &= !KVM_GUESTDBG_USE_HW_BP; + } + + if step { + self.dbg_cfg.control |= KVM_GUESTDBG_SINGLESTEP; + } else { + self.dbg_cfg.control &= !KVM_GUESTDBG_SINGLESTEP; + } + + log::debug!("Setting bp: {:?} cfg: {:?}", addrs, self.dbg_cfg); + vcpu_fd + .set_guest_debug(&self.dbg_cfg) + .map_err(|e| new_error!("Could not set guest debug: {:?}", e))?; + + self.single_step = step; + + Ok(()) + } + + /// Method that adds a breakpoint + pub(crate) fn add_breakpoint(&mut self, vcpu_fd: &VcpuFd, addr: u64) -> Result { + if self.hw_breakpoints.len() >= Self::MAX_NO_OF_HW_BP { + Ok(false) + } else if self.hw_breakpoints.contains(&addr) { + Ok(true) + } else { + self.hw_breakpoints.push(addr); + self.set_debug_config(vcpu_fd, self.single_step)?; + + Ok(true) + } + } + + /// Method that removes a breakpoint + pub(crate) fn remove_breakpoint(&mut self, vcpu_fd: &VcpuFd, addr: u64) -> Result { + if self.hw_breakpoints.contains(&addr) { + self.hw_breakpoints.retain(|&a| a != addr); + self.set_debug_config(vcpu_fd, self.single_step)?; + + Ok(true) + } else { + Ok(false) + } + } +} diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index f5e532493..de932bd85 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -15,6 +15,8 @@ limitations under the License. */ mod event_loop; +#[cfg(kvm)] +mod kvm_debug; mod x86_64_target; use std::io::{self, ErrorKind}; @@ -26,9 +28,20 @@ use event_loop::event_loop_thread; use gdbstub::conn::ConnectionExt; use gdbstub::stub::GdbStub; use gdbstub::target::TargetError; +#[cfg(kvm)] +pub(crate) use kvm_debug::KvmDebug; use thiserror::Error; use x86_64_target::HyperlightSandboxTarget; +/// Software Breakpoint size in memory +pub(crate) const SW_BP_SIZE: usize = 1; +/// Software Breakpoint opcode - INT3 +/// Check page 7-28 Vol. 3A of Intel 64 and IA-32 +/// Architectures Software Developer's Manual +const SW_BP_OP: u8 = 0xCC; +/// Software Breakpoint written to memory +pub(crate) const SW_BP: [u8; SW_BP_SIZE] = [SW_BP_OP]; + #[derive(Debug, Error)] pub(crate) enum GdbTargetError { #[error("Error encountered while binding to address and port")] diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 3f33980c7..0ecf8d6b1 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -26,7 +26,7 @@ use tracing::{instrument, Span}; use super::fpu::{FP_CONTROL_WORD_DEFAULT, FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; #[cfg(gdb)] -use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse, VcpuStopReason}; +use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse, KvmDebug, VcpuStopReason}; #[cfg(gdb)] use super::handlers::DbgMemAccessHandlerWrapper; use super::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper}; @@ -65,129 +65,19 @@ pub(crate) fn is_hypervisor_present() -> bool { #[cfg(gdb)] mod debug { - use std::collections::HashMap; use std::sync::{Arc, Mutex}; use hyperlight_common::mem::PAGE_SIZE; - use kvm_bindings::{ - kvm_guest_debug, kvm_regs, KVM_GUESTDBG_ENABLE, KVM_GUESTDBG_SINGLESTEP, - KVM_GUESTDBG_USE_HW_BP, KVM_GUESTDBG_USE_SW_BP, - }; - use kvm_ioctls::VcpuFd; + use kvm_bindings::kvm_regs; use super::KVMDriver; - use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason, X86_64Regs}; + use crate::hypervisor::gdb::{ + DebugMsg, DebugResponse, KvmDebug, VcpuStopReason, X86_64Regs, SW_BP, SW_BP_SIZE, + }; use crate::hypervisor::handlers::DbgMemAccessHandlerCaller; use crate::mem::layout::SandboxMemoryLayout; use crate::{new_error, HyperlightError, Result}; - /// Software Breakpoint size in memory - pub(crate) const SW_BP_SIZE: usize = 1; - /// Software Breakpoint opcode - const SW_BP_OP: u8 = 0xCC; - /// Software Breakpoint written to memory - pub(crate) const SW_BP: [u8; SW_BP_SIZE] = [SW_BP_OP]; - - /// KVM Debug struct - /// This struct is used to abstract the internal details of the kvm - /// guest debugging settings - #[derive(Default)] - pub(crate) struct KvmDebug { - /// vCPU stepping state - single_step: bool, - - /// Array of addresses for HW breakpoints - hw_breakpoints: Vec, - /// Saves the bytes modified to enable SW breakpoints - sw_breakpoints: HashMap, - - /// Sent to KVM for enabling guest debug - pub dbg_cfg: kvm_guest_debug, - } - - impl KvmDebug { - const MAX_NO_OF_HW_BP: usize = 4; - - pub(crate) fn new() -> Self { - let dbg = kvm_guest_debug { - control: KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP, - ..Default::default() - }; - - Self { - single_step: false, - hw_breakpoints: vec![], - sw_breakpoints: HashMap::new(), - dbg_cfg: dbg, - } - } - - /// This method sets the kvm debugreg fields to enable breakpoints at - /// specific addresses - /// - /// The first 4 debug registers are used to set the addresses - /// The 4th and 5th debug registers are obsolete and not used - /// The 7th debug register is used to enable the breakpoints - /// For more information see: DEBUG REGISTERS chapter in the architecture - /// manual - fn set_debug_config(&mut self, vcpu_fd: &VcpuFd, step: bool) -> Result<()> { - let addrs = &self.hw_breakpoints; - - self.dbg_cfg.arch.debugreg = [0; 8]; - for (k, addr) in addrs.iter().enumerate() { - self.dbg_cfg.arch.debugreg[k] = *addr; - self.dbg_cfg.arch.debugreg[7] |= 1 << (k * 2); - } - - if !addrs.is_empty() { - self.dbg_cfg.control |= KVM_GUESTDBG_USE_HW_BP; - } else { - self.dbg_cfg.control &= !KVM_GUESTDBG_USE_HW_BP; - } - - if step { - self.dbg_cfg.control |= KVM_GUESTDBG_SINGLESTEP; - } else { - self.dbg_cfg.control &= !KVM_GUESTDBG_SINGLESTEP; - } - - log::debug!("Setting bp: {:?} cfg: {:?}", addrs, self.dbg_cfg); - vcpu_fd - .set_guest_debug(&self.dbg_cfg) - .map_err(|e| new_error!("Could not set guest debug: {:?}", e))?; - - self.single_step = step; - - Ok(()) - } - - /// Method that adds a breakpoint - fn add_breakpoint(&mut self, vcpu_fd: &VcpuFd, addr: u64) -> Result { - if self.hw_breakpoints.len() >= Self::MAX_NO_OF_HW_BP { - Ok(false) - } else if self.hw_breakpoints.contains(&addr) { - Ok(true) - } else { - self.hw_breakpoints.push(addr); - self.set_debug_config(vcpu_fd, self.single_step)?; - - Ok(true) - } - } - - /// Method that removes a breakpoint - fn remove_breakpoint(&mut self, vcpu_fd: &VcpuFd, addr: u64) -> Result { - if self.hw_breakpoints.contains(&addr) { - self.hw_breakpoints.retain(|&a| a != addr); - self.set_debug_config(vcpu_fd, self.single_step)?; - - Ok(true) - } else { - Ok(false) - } - } - } - impl KVMDriver { /// Resets the debug information to disable debugging fn disable_debug(&mut self) -> Result<()> { @@ -581,7 +471,7 @@ pub(super) struct KVMDriver { mem_regions: Vec, #[cfg(gdb)] - debug: Option, + debug: Option, #[cfg(gdb)] gdb_conn: Option>, } @@ -625,7 +515,7 @@ impl KVMDriver { #[cfg(gdb)] let (debug, gdb_conn) = if let Some(gdb_conn) = gdb_conn { - (Some(debug::KvmDebug::new()), Some(gdb_conn)) + (Some(KvmDebug::new()), Some(gdb_conn)) } else { (None, None) }; From d60a5cabc8040705d300f228392b589fa9135d31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Mon, 3 Mar 2025 13:28:26 +0200 Subject: [PATCH 03/17] dbg: move hardware breakpoints functionality to separate file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - also add a trait that contains all the needed methods for interacting with a vCPU Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/gdb/kvm_debug.rs | 47 +++++- src/hyperlight_host/src/hypervisor/gdb/mod.rs | 15 ++ src/hyperlight_host/src/hypervisor/kvm.rs | 157 ++++++++---------- 3 files changed, 123 insertions(+), 96 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs b/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs index ec8163a77..333d402d7 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs @@ -22,8 +22,8 @@ use kvm_bindings::{ }; use kvm_ioctls::VcpuFd; -use super::*; -use crate::{new_error, Result}; +use super::{GuestDebug, MAX_NO_OF_HW_BP, SW_BP_SIZE}; +use crate::{new_error, HyperlightError, Result}; /// KVM Debug struct /// This struct is used to abstract the internal details of the kvm @@ -43,8 +43,6 @@ pub(crate) struct KvmDebug { } impl KvmDebug { - const MAX_NO_OF_HW_BP: usize = 4; - pub(crate) fn new() -> Self { let dbg = kvm_guest_debug { control: KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP, @@ -67,7 +65,7 @@ impl KvmDebug { /// The 7th debug register is used to enable the breakpoints /// For more information see: DEBUG REGISTERS chapter in the architecture /// manual - pub(crate) fn set_debug_config(&mut self, vcpu_fd: &VcpuFd, step: bool) -> Result<()> { + fn set_debug_config(&mut self, vcpu_fd: &VcpuFd, step: bool) -> Result<()> { let addrs = &self.hw_breakpoints; self.dbg_cfg.arch.debugreg = [0; 8]; @@ -99,8 +97,8 @@ impl KvmDebug { } /// Method that adds a breakpoint - pub(crate) fn add_breakpoint(&mut self, vcpu_fd: &VcpuFd, addr: u64) -> Result { - if self.hw_breakpoints.len() >= Self::MAX_NO_OF_HW_BP { + fn add_breakpoint(&mut self, vcpu_fd: &VcpuFd, addr: u64) -> Result { + if self.hw_breakpoints.len() >= MAX_NO_OF_HW_BP { Ok(false) } else if self.hw_breakpoints.contains(&addr) { Ok(true) @@ -113,7 +111,7 @@ impl KvmDebug { } /// Method that removes a breakpoint - pub(crate) fn remove_breakpoint(&mut self, vcpu_fd: &VcpuFd, addr: u64) -> Result { + fn remove_breakpoint(&mut self, vcpu_fd: &VcpuFd, addr: u64) -> Result { if self.hw_breakpoints.contains(&addr) { self.hw_breakpoints.retain(|&a| a != addr); self.set_debug_config(vcpu_fd, self.single_step)?; @@ -123,4 +121,37 @@ impl KvmDebug { Ok(false) } } + + /// Translates the guest address to physical address + fn translate_gva(&self, vcpu_fd: &VcpuFd, gva: u64) -> Result { + let tr = vcpu_fd + .translate_gva(gva) + .map_err(|_| HyperlightError::TranslateGuestAddress(gva))?; + + if tr.valid == 0 { + Err(HyperlightError::TranslateGuestAddress(gva)) + } else { + Ok(tr.physical_address) + } + } +} + +impl GuestDebug for KvmDebug { + type Vcpu = VcpuFd; + + fn add_hw_breakpoint(&mut self, vcpu_fd: &Self::Vcpu, addr: u64) -> Result { + let addr = self.translate_gva(vcpu_fd, addr)?; + + self.add_breakpoint(vcpu_fd, addr) + } + + fn remove_hw_breakpoint(&mut self, vcpu_fd: &Self::Vcpu, addr: u64) -> Result { + let addr = self.translate_gva(vcpu_fd, addr)?; + + self.remove_breakpoint(vcpu_fd, addr) + } + + fn set_single_step(&mut self, vcpu_fd: &Self::Vcpu, enable: bool) -> Result<()> { + self.set_debug_config(vcpu_fd, enable) + } } diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index de932bd85..587f5d5e8 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -41,6 +41,8 @@ pub(crate) const SW_BP_SIZE: usize = 1; const SW_BP_OP: u8 = 0xCC; /// Software Breakpoint written to memory pub(crate) const SW_BP: [u8; SW_BP_SIZE] = [SW_BP_OP]; +/// Maximum number of supported hardware breakpoints +pub(crate) const MAX_NO_OF_HW_BP: usize = 4; #[derive(Debug, Error)] pub(crate) enum GdbTargetError { @@ -148,6 +150,19 @@ pub(crate) enum DebugResponse { WriteRegisters, } +/// This trait is used to define common debugging functionality for Hypervisors +pub(crate) trait GuestDebug { + /// Type that wraps the vCPU functionality + type Vcpu; + + /// Adds hardware breakpoint + fn add_hw_breakpoint(&mut self, vcpu_fd: &Self::Vcpu, addr: u64) -> crate::Result; + /// Removes hardware breakpoint + fn remove_hw_breakpoint(&mut self, vcpu_fd: &Self::Vcpu, addr: u64) -> crate::Result; + /// Enables or disables stepping and sets the vCPU debug configuration + fn set_single_step(&mut self, vcpu_fd: &Self::Vcpu, enable: bool) -> crate::Result<()>; +} + /// Debug communication channel that is used for sending a request type and /// receive a different response type pub(crate) struct DebugCommChannel { diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 0ecf8d6b1..54dc14835 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -72,7 +72,8 @@ mod debug { use super::KVMDriver; use crate::hypervisor::gdb::{ - DebugMsg, DebugResponse, KvmDebug, VcpuStopReason, X86_64Regs, SW_BP, SW_BP_SIZE, + DebugMsg, DebugResponse, GuestDebug, KvmDebug, VcpuStopReason, X86_64Regs, SW_BP, + SW_BP_SIZE, }; use crate::hypervisor::handlers::DbgMemAccessHandlerCaller; use crate::mem::layout::SandboxMemoryLayout; @@ -81,9 +82,13 @@ mod debug { impl KVMDriver { /// Resets the debug information to disable debugging fn disable_debug(&mut self) -> Result<()> { - self.debug = Some(KvmDebug::default()); + let mut debug = KvmDebug::default(); - self.set_single_step(false) + debug.set_single_step(&self.vcpu_fd, false)?; + + self.debug = Some(debug); + + Ok(()) } /// Returns the instruction pointer from the stopped vCPU @@ -96,16 +101,6 @@ mod debug { Ok(regs.rip) } - /// Sets or clears stepping for vCPU - fn set_single_step(&mut self, enable: bool) -> Result<()> { - let debug = self - .debug - .as_mut() - .ok_or_else(|| new_error!("Debug is not enabled"))?; - - debug.set_debug_config(&self.vcpu_fd, enable) - } - /// Translates the guest address to physical address fn translate_gva(&self, gva: u64) -> Result { let tr = self @@ -239,26 +234,6 @@ mod debug { .map_err(|e| new_error!("Could not write guest registers: {:?}", e)) } - fn add_hw_breakpoint(&mut self, addr: u64) -> Result { - let addr = self.translate_gva(addr)?; - - if let Some(debug) = self.debug.as_mut() { - debug.add_breakpoint(&self.vcpu_fd, addr) - } else { - Ok(false) - } - } - - fn remove_hw_breakpoint(&mut self, addr: u64) -> Result { - let addr = self.translate_gva(addr)?; - - if let Some(debug) = self.debug.as_mut() { - debug.remove_breakpoint(&self.vcpu_fd, addr) - } else { - Ok(false) - } - } - fn add_sw_breakpoint( &mut self, addr: u64, @@ -330,7 +305,7 @@ mod debug { if self.debug.is_some() { log::debug!("Setting entrypoint bp {:X}", self.entrypoint); let mut entrypoint_debug = KvmDebug::new(); - entrypoint_debug.add_breakpoint(&self.vcpu_fd, self.entrypoint)?; + entrypoint_debug.add_hw_breakpoint(&self.vcpu_fd, self.entrypoint)?; Ok(()) } else { @@ -371,61 +346,67 @@ mod debug { req: DebugMsg, dbg_mem_access_fn: Arc>, ) -> Result { - match req { - DebugMsg::AddHwBreakpoint(addr) => self - .add_hw_breakpoint(addr) - .map(DebugResponse::AddHwBreakpoint), - DebugMsg::AddSwBreakpoint(addr) => self - .add_sw_breakpoint(addr, dbg_mem_access_fn) - .map(DebugResponse::AddSwBreakpoint), - DebugMsg::Continue => { - self.set_single_step(false)?; - Ok(DebugResponse::Continue) - } - DebugMsg::DisableDebug => { - self.disable_debug()?; - - Ok(DebugResponse::DisableDebug) - } - DebugMsg::GetCodeSectionOffset => { - let offset = dbg_mem_access_fn - .try_lock() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? - .get_code_offset()?; - - Ok(DebugResponse::GetCodeSectionOffset(offset as u64)) - } - DebugMsg::ReadAddr(addr, len) => { - let mut data = vec![0u8; len]; - - self.read_addrs(addr, &mut data, dbg_mem_access_fn)?; - - Ok(DebugResponse::ReadAddr(data)) - } - DebugMsg::ReadRegisters => { - let mut regs = X86_64Regs::default(); - - self.read_regs(&mut regs) - .map(|_| DebugResponse::ReadRegisters(regs)) - } - DebugMsg::RemoveHwBreakpoint(addr) => self - .remove_hw_breakpoint(addr) - .map(DebugResponse::RemoveHwBreakpoint), - DebugMsg::RemoveSwBreakpoint(addr) => self - .remove_sw_breakpoint(addr, dbg_mem_access_fn) - .map(DebugResponse::RemoveSwBreakpoint), - DebugMsg::Step => { - self.set_single_step(true)?; - Ok(DebugResponse::Step) - } - DebugMsg::WriteAddr(addr, data) => { - self.write_addrs(addr, &data, dbg_mem_access_fn)?; - - Ok(DebugResponse::WriteAddr) + if let Some(debug) = self.debug.as_mut() { + match req { + DebugMsg::AddHwBreakpoint(addr) => debug + .add_hw_breakpoint(&self.vcpu_fd, addr) + .map(DebugResponse::AddHwBreakpoint), + DebugMsg::AddSwBreakpoint(addr) => self + .add_sw_breakpoint(addr, dbg_mem_access_fn) + .map(DebugResponse::AddSwBreakpoint), + DebugMsg::Continue => { + debug.set_single_step(&self.vcpu_fd, false)?; + Ok(DebugResponse::Continue) + } + DebugMsg::DisableDebug => { + self.disable_debug()?; + + Ok(DebugResponse::DisableDebug) + } + DebugMsg::GetCodeSectionOffset => { + let offset = dbg_mem_access_fn + .try_lock() + .map_err(|e| { + new_error!("Error locking at {}:{}: {}", file!(), line!(), e) + })? + .get_code_offset()?; + + Ok(DebugResponse::GetCodeSectionOffset(offset as u64)) + } + DebugMsg::ReadAddr(addr, len) => { + let mut data = vec![0u8; len]; + + self.read_addrs(addr, &mut data, dbg_mem_access_fn)?; + + Ok(DebugResponse::ReadAddr(data)) + } + DebugMsg::ReadRegisters => { + let mut regs = X86_64Regs::default(); + + self.read_regs(&mut regs) + .map(|_| DebugResponse::ReadRegisters(regs)) + } + DebugMsg::RemoveHwBreakpoint(addr) => debug + .remove_hw_breakpoint(&self.vcpu_fd, addr) + .map(DebugResponse::RemoveHwBreakpoint), + DebugMsg::RemoveSwBreakpoint(addr) => self + .remove_sw_breakpoint(addr, dbg_mem_access_fn) + .map(DebugResponse::RemoveSwBreakpoint), + DebugMsg::Step => { + debug.set_single_step(&self.vcpu_fd, true)?; + Ok(DebugResponse::Step) + } + DebugMsg::WriteAddr(addr, data) => { + self.write_addrs(addr, &data, dbg_mem_access_fn)?; + + Ok(DebugResponse::WriteAddr) + } + DebugMsg::WriteRegisters(regs) => self + .write_regs(®s) + .map(|_| DebugResponse::WriteRegisters), } - DebugMsg::WriteRegisters(regs) => self - .write_regs(®s) - .map(|_| DebugResponse::WriteRegisters), + } else { + Err(new_error!("Debugging is not enabled")) } } From 5f6357d6679f5224f97698c81a36476e4625e3b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Mon, 3 Mar 2025 15:03:03 +0200 Subject: [PATCH 04/17] dbg: move read/write registers and get_stop_reason to vCPU functionality trait MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/gdb/kvm_debug.rs | 105 +++++++++++++++++- src/hyperlight_host/src/hypervisor/gdb/mod.rs | 4 + src/hyperlight_host/src/hypervisor/kvm.rs | 97 +--------------- 3 files changed, 109 insertions(+), 97 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs b/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs index 333d402d7..f696d676c 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs @@ -17,12 +17,12 @@ limitations under the License. use std::collections::HashMap; use kvm_bindings::{ - kvm_guest_debug, KVM_GUESTDBG_ENABLE, KVM_GUESTDBG_SINGLESTEP, KVM_GUESTDBG_USE_HW_BP, - KVM_GUESTDBG_USE_SW_BP, + kvm_guest_debug, kvm_regs, KVM_GUESTDBG_ENABLE, KVM_GUESTDBG_SINGLESTEP, + KVM_GUESTDBG_USE_HW_BP, KVM_GUESTDBG_USE_SW_BP, }; use kvm_ioctls::VcpuFd; -use super::{GuestDebug, MAX_NO_OF_HW_BP, SW_BP_SIZE}; +use super::{GuestDebug, VcpuStopReason, X86_64Regs, MAX_NO_OF_HW_BP, SW_BP_SIZE}; use crate::{new_error, HyperlightError, Result}; /// KVM Debug struct @@ -31,10 +31,10 @@ use crate::{new_error, HyperlightError, Result}; #[derive(Default)] pub(crate) struct KvmDebug { /// vCPU stepping state - pub(crate) single_step: bool, + single_step: bool, /// Array of addresses for HW breakpoints - pub(crate) hw_breakpoints: Vec, + hw_breakpoints: Vec, /// Saves the bytes modified to enable SW breakpoints pub(crate) sw_breakpoints: HashMap, @@ -57,6 +57,15 @@ impl KvmDebug { } } + /// Returns the instruction pointer from the stopped vCPU + fn get_instruction_pointer(&self, vcpu_fd: &VcpuFd) -> Result { + let regs = vcpu_fd + .get_regs() + .map_err(|e| new_error!("Could not retrieve registers from vCPU: {:?}", e))?; + + Ok(regs.rip) + } + /// This method sets the kvm debugreg fields to enable breakpoints at /// specific addresses /// @@ -122,6 +131,34 @@ impl KvmDebug { } } + /// Get the reason the vCPU has stopped + pub(crate) fn get_stop_reason( + &self, + vcpu_fd: &VcpuFd, + entrypoint: u64, + ) -> Result { + if self.single_step { + return Ok(VcpuStopReason::DoneStep); + } + + let ip = self.get_instruction_pointer(vcpu_fd)?; + let gpa = self.translate_gva(vcpu_fd, ip)?; + + if self.sw_breakpoints.contains_key(&gpa) { + return Ok(VcpuStopReason::SwBp); + } + + if self.hw_breakpoints.contains(&gpa) { + return Ok(VcpuStopReason::HwBp); + } + + if gpa == entrypoint { + return Ok(VcpuStopReason::HwBp); + } + + Ok(VcpuStopReason::Unknown) + } + /// Translates the guest address to physical address fn translate_gva(&self, vcpu_fd: &VcpuFd, gva: u64) -> Result { let tr = vcpu_fd @@ -151,7 +188,65 @@ impl GuestDebug for KvmDebug { self.remove_breakpoint(vcpu_fd, addr) } + fn read_regs(&self, vcpu_fd: &Self::Vcpu, regs: &mut X86_64Regs) -> Result<()> { + log::debug!("Read registers"); + let vcpu_regs = vcpu_fd + .get_regs() + .map_err(|e| new_error!("Could not read guest registers: {:?}", e))?; + + regs.rax = vcpu_regs.rax; + regs.rbx = vcpu_regs.rbx; + regs.rcx = vcpu_regs.rcx; + regs.rdx = vcpu_regs.rdx; + regs.rsi = vcpu_regs.rsi; + regs.rdi = vcpu_regs.rdi; + regs.rbp = vcpu_regs.rbp; + regs.rsp = vcpu_regs.rsp; + regs.r8 = vcpu_regs.r8; + regs.r9 = vcpu_regs.r9; + regs.r10 = vcpu_regs.r10; + regs.r11 = vcpu_regs.r11; + regs.r12 = vcpu_regs.r12; + regs.r13 = vcpu_regs.r13; + regs.r14 = vcpu_regs.r14; + regs.r15 = vcpu_regs.r15; + + regs.rip = vcpu_regs.rip; + regs.rflags = vcpu_regs.rflags; + + Ok(()) + } + fn set_single_step(&mut self, vcpu_fd: &Self::Vcpu, enable: bool) -> Result<()> { self.set_debug_config(vcpu_fd, enable) } + + fn write_regs(&self, vcpu_fd: &Self::Vcpu, regs: &X86_64Regs) -> Result<()> { + log::debug!("Write registers"); + let new_regs = kvm_regs { + rax: regs.rax, + rbx: regs.rbx, + rcx: regs.rcx, + rdx: regs.rdx, + rsi: regs.rsi, + rdi: regs.rdi, + rbp: regs.rbp, + rsp: regs.rsp, + r8: regs.r8, + r9: regs.r9, + r10: regs.r10, + r11: regs.r11, + r12: regs.r12, + r13: regs.r13, + r14: regs.r14, + r15: regs.r15, + + rip: regs.rip, + rflags: regs.rflags, + }; + + vcpu_fd + .set_regs(&new_regs) + .map_err(|e| new_error!("Could not write guest registers: {:?}", e)) + } } diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index 587f5d5e8..46e621348 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -157,10 +157,14 @@ pub(crate) trait GuestDebug { /// Adds hardware breakpoint fn add_hw_breakpoint(&mut self, vcpu_fd: &Self::Vcpu, addr: u64) -> crate::Result; + /// Read registers + fn read_regs(&self, vcpu_fd: &Self::Vcpu, regs: &mut X86_64Regs) -> crate::Result<()>; /// Removes hardware breakpoint fn remove_hw_breakpoint(&mut self, vcpu_fd: &Self::Vcpu, addr: u64) -> crate::Result; /// Enables or disables stepping and sets the vCPU debug configuration fn set_single_step(&mut self, vcpu_fd: &Self::Vcpu, enable: bool) -> crate::Result<()>; + /// Write registers + fn write_regs(&self, vcpu_fd: &Self::Vcpu, regs: &X86_64Regs) -> crate::Result<()>; } /// Debug communication channel that is used for sending a request type and diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 54dc14835..5d025a0b7 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -68,7 +68,6 @@ mod debug { use std::sync::{Arc, Mutex}; use hyperlight_common::mem::PAGE_SIZE; - use kvm_bindings::kvm_regs; use super::KVMDriver; use crate::hypervisor::gdb::{ @@ -91,16 +90,6 @@ mod debug { Ok(()) } - /// Returns the instruction pointer from the stopped vCPU - fn get_instruction_pointer(&self) -> Result { - let regs = self - .vcpu_fd - .get_regs() - .map_err(|e| new_error!("Could not retrieve registers from vCPU: {:?}", e))?; - - Ok(regs.rip) - } - /// Translates the guest address to physical address fn translate_gva(&self, gva: u64) -> Result { let tr = self @@ -175,65 +164,6 @@ mod debug { Ok(()) } - fn read_regs(&self, regs: &mut X86_64Regs) -> Result<()> { - log::debug!("Read registers"); - let vcpu_regs = self - .vcpu_fd - .get_regs() - .map_err(|e| new_error!("Could not read guest registers: {:?}", e))?; - - regs.rax = vcpu_regs.rax; - regs.rbx = vcpu_regs.rbx; - regs.rcx = vcpu_regs.rcx; - regs.rdx = vcpu_regs.rdx; - regs.rsi = vcpu_regs.rsi; - regs.rdi = vcpu_regs.rdi; - regs.rbp = vcpu_regs.rbp; - regs.rsp = vcpu_regs.rsp; - regs.r8 = vcpu_regs.r8; - regs.r9 = vcpu_regs.r9; - regs.r10 = vcpu_regs.r10; - regs.r11 = vcpu_regs.r11; - regs.r12 = vcpu_regs.r12; - regs.r13 = vcpu_regs.r13; - regs.r14 = vcpu_regs.r14; - regs.r15 = vcpu_regs.r15; - - regs.rip = vcpu_regs.rip; - regs.rflags = vcpu_regs.rflags; - - Ok(()) - } - - fn write_regs(&self, regs: &X86_64Regs) -> Result<()> { - log::debug!("Write registers"); - let new_regs = kvm_regs { - rax: regs.rax, - rbx: regs.rbx, - rcx: regs.rcx, - rdx: regs.rdx, - rsi: regs.rsi, - rdi: regs.rdi, - rbp: regs.rbp, - rsp: regs.rsp, - r8: regs.r8, - r9: regs.r9, - r10: regs.r10, - r11: regs.r11, - r12: regs.r12, - r13: regs.r13, - r14: regs.r14, - r15: regs.r15, - - rip: regs.rip, - rflags: regs.rflags, - }; - - self.vcpu_fd - .set_regs(&new_regs) - .map_err(|e| new_error!("Could not write guest registers: {:?}", e)) - } - fn add_sw_breakpoint( &mut self, addr: u64, @@ -320,25 +250,7 @@ mod debug { .as_ref() .ok_or_else(|| new_error!("Debug is not enabled"))?; - if debug.single_step { - return Ok(VcpuStopReason::DoneStep); - } - - let ip = self.get_instruction_pointer()?; - let gpa = self.translate_gva(ip)?; - if debug.sw_breakpoints.contains_key(&gpa) { - return Ok(VcpuStopReason::SwBp); - } - - if debug.hw_breakpoints.contains(&gpa) { - return Ok(VcpuStopReason::HwBp); - } - - if ip == self.entrypoint { - return Ok(VcpuStopReason::HwBp); - } - - Ok(VcpuStopReason::Unknown) + debug.get_stop_reason(&self.vcpu_fd, self.entrypoint) } pub(crate) fn process_dbg_request( @@ -383,7 +295,8 @@ mod debug { DebugMsg::ReadRegisters => { let mut regs = X86_64Regs::default(); - self.read_regs(&mut regs) + debug + .read_regs(&self.vcpu_fd, &mut regs) .map(|_| DebugResponse::ReadRegisters(regs)) } DebugMsg::RemoveHwBreakpoint(addr) => debug @@ -401,8 +314,8 @@ mod debug { Ok(DebugResponse::WriteAddr) } - DebugMsg::WriteRegisters(regs) => self - .write_regs(®s) + DebugMsg::WriteRegisters(regs) => debug + .write_regs(&self.vcpu_fd, ®s) .map(|_| DebugResponse::WriteRegisters), } } else { From 10d2c71e45f270a1da40f78c02fa8694e64cfe2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Mon, 3 Mar 2025 17:00:47 +0200 Subject: [PATCH 05/17] dbg: modify hw breakpoints and move common functionality in trait MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/gdb/kvm_debug.rs | 71 +++++++------------ src/hyperlight_host/src/hypervisor/gdb/mod.rs | 39 ++++++++-- src/hyperlight_host/src/hypervisor/kvm.rs | 12 ++-- 3 files changed, 65 insertions(+), 57 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs b/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs index f696d676c..1d3909ce5 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs @@ -105,32 +105,6 @@ impl KvmDebug { Ok(()) } - /// Method that adds a breakpoint - fn add_breakpoint(&mut self, vcpu_fd: &VcpuFd, addr: u64) -> Result { - if self.hw_breakpoints.len() >= MAX_NO_OF_HW_BP { - Ok(false) - } else if self.hw_breakpoints.contains(&addr) { - Ok(true) - } else { - self.hw_breakpoints.push(addr); - self.set_debug_config(vcpu_fd, self.single_step)?; - - Ok(true) - } - } - - /// Method that removes a breakpoint - fn remove_breakpoint(&mut self, vcpu_fd: &VcpuFd, addr: u64) -> Result { - if self.hw_breakpoints.contains(&addr) { - self.hw_breakpoints.retain(|&a| a != addr); - self.set_debug_config(vcpu_fd, self.single_step)?; - - Ok(true) - } else { - Ok(false) - } - } - /// Get the reason the vCPU has stopped pub(crate) fn get_stop_reason( &self, @@ -158,34 +132,25 @@ impl KvmDebug { Ok(VcpuStopReason::Unknown) } - - /// Translates the guest address to physical address - fn translate_gva(&self, vcpu_fd: &VcpuFd, gva: u64) -> Result { - let tr = vcpu_fd - .translate_gva(gva) - .map_err(|_| HyperlightError::TranslateGuestAddress(gva))?; - - if tr.valid == 0 { - Err(HyperlightError::TranslateGuestAddress(gva)) - } else { - Ok(tr.physical_address) - } - } } impl GuestDebug for KvmDebug { type Vcpu = VcpuFd; - fn add_hw_breakpoint(&mut self, vcpu_fd: &Self::Vcpu, addr: u64) -> Result { - let addr = self.translate_gva(vcpu_fd, addr)?; - - self.add_breakpoint(vcpu_fd, addr) + fn is_hw_breakpoint(&self, addr: &u64) -> bool { + self.hw_breakpoints.contains(addr) } + fn save_hw_breakpoint(&mut self, addr: &u64) -> bool { + if self.hw_breakpoints.len() >= MAX_NO_OF_HW_BP { + false + } else { + self.hw_breakpoints.push(*addr); - fn remove_hw_breakpoint(&mut self, vcpu_fd: &Self::Vcpu, addr: u64) -> Result { - let addr = self.translate_gva(vcpu_fd, addr)?; - - self.remove_breakpoint(vcpu_fd, addr) + true + } + } + fn delete_hw_breakpoint(&mut self, addr: &u64) { + self.hw_breakpoints.retain(|&a| a != *addr); } fn read_regs(&self, vcpu_fd: &Self::Vcpu, regs: &mut X86_64Regs) -> Result<()> { @@ -221,6 +186,18 @@ impl GuestDebug for KvmDebug { self.set_debug_config(vcpu_fd, enable) } + fn translate_gva(&self, vcpu_fd: &Self::Vcpu, gva: u64) -> Result { + let tr = vcpu_fd + .translate_gva(gva) + .map_err(|_| HyperlightError::TranslateGuestAddress(gva))?; + + if tr.valid == 0 { + Err(HyperlightError::TranslateGuestAddress(gva)) + } else { + Ok(tr.physical_address) + } + } + fn write_regs(&self, vcpu_fd: &Self::Vcpu, regs: &X86_64Regs) -> Result<()> { log::debug!("Write registers"); let new_regs = kvm_regs { diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index 46e621348..426ded61b 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -33,6 +33,8 @@ pub(crate) use kvm_debug::KvmDebug; use thiserror::Error; use x86_64_target::HyperlightSandboxTarget; +use crate::new_error; + /// Software Breakpoint size in memory pub(crate) const SW_BP_SIZE: usize = 1; /// Software Breakpoint opcode - INT3 @@ -155,16 +157,45 @@ pub(crate) trait GuestDebug { /// Type that wraps the vCPU functionality type Vcpu; - /// Adds hardware breakpoint - fn add_hw_breakpoint(&mut self, vcpu_fd: &Self::Vcpu, addr: u64) -> crate::Result; + /// Returns true whether the provided address is a hardware breakpoint + fn is_hw_breakpoint(&self, addr: &u64) -> bool; + /// Stores the address of the hw breakpoint + fn save_hw_breakpoint(&mut self, addr: &u64) -> bool; + /// Deletes the address of the hw breakpoint from storage + fn delete_hw_breakpoint(&mut self, addr: &u64); + /// Read registers fn read_regs(&self, vcpu_fd: &Self::Vcpu, regs: &mut X86_64Regs) -> crate::Result<()>; - /// Removes hardware breakpoint - fn remove_hw_breakpoint(&mut self, vcpu_fd: &Self::Vcpu, addr: u64) -> crate::Result; /// Enables or disables stepping and sets the vCPU debug configuration fn set_single_step(&mut self, vcpu_fd: &Self::Vcpu, enable: bool) -> crate::Result<()>; + /// Translates the guest address to physical address + fn translate_gva(&self, vcpu_fd: &Self::Vcpu, gva: u64) -> crate::Result; /// Write registers fn write_regs(&self, vcpu_fd: &Self::Vcpu, regs: &X86_64Regs) -> crate::Result<()>; + + /// Adds hardware breakpoint + fn add_hw_breakpoint(&mut self, vcpu_fd: &Self::Vcpu, addr: u64) -> crate::Result<()> { + let addr = self.translate_gva(vcpu_fd, addr)?; + + if self.is_hw_breakpoint(&addr) { + return Ok(()); + } + + self.save_hw_breakpoint(&addr) + .then(|| self.set_single_step(vcpu_fd, false)) + .ok_or_else(|| new_error!("Failed to save hw breakpoint"))? + } + /// Removes hardware breakpoint + fn remove_hw_breakpoint(&mut self, vcpu_fd: &Self::Vcpu, addr: u64) -> crate::Result<()> { + let addr = self.translate_gva(vcpu_fd, addr)?; + + self.is_hw_breakpoint(&addr) + .then(|| { + self.delete_hw_breakpoint(&addr); + self.set_single_step(vcpu_fd, false) + }) + .ok_or_else(|| new_error!("The address: {:?} is not a hw breakpoint", addr))? + } } /// Debug communication channel that is used for sending a request type and diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 5d025a0b7..1d38780ed 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -260,9 +260,9 @@ mod debug { ) -> Result { if let Some(debug) = self.debug.as_mut() { match req { - DebugMsg::AddHwBreakpoint(addr) => debug - .add_hw_breakpoint(&self.vcpu_fd, addr) - .map(DebugResponse::AddHwBreakpoint), + DebugMsg::AddHwBreakpoint(addr) => Ok(DebugResponse::AddHwBreakpoint( + debug.add_hw_breakpoint(&self.vcpu_fd, addr).is_ok(), + )), DebugMsg::AddSwBreakpoint(addr) => self .add_sw_breakpoint(addr, dbg_mem_access_fn) .map(DebugResponse::AddSwBreakpoint), @@ -299,9 +299,9 @@ mod debug { .read_regs(&self.vcpu_fd, &mut regs) .map(|_| DebugResponse::ReadRegisters(regs)) } - DebugMsg::RemoveHwBreakpoint(addr) => debug - .remove_hw_breakpoint(&self.vcpu_fd, addr) - .map(DebugResponse::RemoveHwBreakpoint), + DebugMsg::RemoveHwBreakpoint(addr) => Ok(DebugResponse::RemoveHwBreakpoint( + debug.remove_hw_breakpoint(&self.vcpu_fd, addr).is_ok(), + )), DebugMsg::RemoveSwBreakpoint(addr) => self .remove_sw_breakpoint(addr, dbg_mem_access_fn) .map(DebugResponse::RemoveSwBreakpoint), From 480a3d9867a65139ae5990424243dbdcae247753 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Mon, 3 Mar 2025 17:20:31 +0200 Subject: [PATCH 06/17] dbg: move read/write address and add/remove sw breakpoint to separate file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - add trait to define common behavior to be used by other hypervisors later Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/gdb/kvm_debug.rs | 11 +- src/hyperlight_host/src/hypervisor/gdb/mod.rs | 127 +++++++++++++- src/hyperlight_host/src/hypervisor/kvm.rs | 166 ++---------------- 3 files changed, 148 insertions(+), 156 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs b/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs index 1d3909ce5..4b162470f 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs @@ -36,7 +36,7 @@ pub(crate) struct KvmDebug { /// Array of addresses for HW breakpoints hw_breakpoints: Vec, /// Saves the bytes modified to enable SW breakpoints - pub(crate) sw_breakpoints: HashMap, + sw_breakpoints: HashMap, /// Sent to KVM for enabling guest debug dbg_cfg: kvm_guest_debug, @@ -140,6 +140,9 @@ impl GuestDebug for KvmDebug { fn is_hw_breakpoint(&self, addr: &u64) -> bool { self.hw_breakpoints.contains(addr) } + fn is_sw_breakpoint(&self, addr: &u64) -> bool { + self.sw_breakpoints.contains_key(addr) + } fn save_hw_breakpoint(&mut self, addr: &u64) -> bool { if self.hw_breakpoints.len() >= MAX_NO_OF_HW_BP { false @@ -149,9 +152,15 @@ impl GuestDebug for KvmDebug { true } } + fn save_sw_breakpoint_data(&mut self, addr: u64, data: [u8; 1]) { + _ = self.sw_breakpoints.insert(addr, data); + } fn delete_hw_breakpoint(&mut self, addr: &u64) { self.hw_breakpoints.retain(|&a| a != *addr); } + fn delete_sw_breakpoint_data(&mut self, addr: &u64) -> Option<[u8; 1]> { + self.sw_breakpoints.remove(addr) + } fn read_regs(&self, vcpu_fd: &Self::Vcpu, regs: &mut X86_64Regs) -> Result<()> { log::debug!("Read registers"); diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index 426ded61b..d1febc065 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -21,6 +21,7 @@ mod x86_64_target; use std::io::{self, ErrorKind}; use std::net::TcpListener; +use std::sync::{Arc, Mutex}; use std::thread; use crossbeam_channel::{Receiver, Sender, TryRecvError}; @@ -28,23 +29,26 @@ use event_loop::event_loop_thread; use gdbstub::conn::ConnectionExt; use gdbstub::stub::GdbStub; use gdbstub::target::TargetError; +use hyperlight_common::mem::PAGE_SIZE; #[cfg(kvm)] pub(crate) use kvm_debug::KvmDebug; use thiserror::Error; use x86_64_target::HyperlightSandboxTarget; +use crate::hypervisor::handlers::DbgMemAccessHandlerCaller; +use crate::mem::layout::SandboxMemoryLayout; use crate::new_error; /// Software Breakpoint size in memory -pub(crate) const SW_BP_SIZE: usize = 1; +const SW_BP_SIZE: usize = 1; /// Software Breakpoint opcode - INT3 /// Check page 7-28 Vol. 3A of Intel 64 and IA-32 /// Architectures Software Developer's Manual const SW_BP_OP: u8 = 0xCC; /// Software Breakpoint written to memory -pub(crate) const SW_BP: [u8; SW_BP_SIZE] = [SW_BP_OP]; +const SW_BP: [u8; SW_BP_SIZE] = [SW_BP_OP]; /// Maximum number of supported hardware breakpoints -pub(crate) const MAX_NO_OF_HW_BP: usize = 4; +const MAX_NO_OF_HW_BP: usize = 4; #[derive(Debug, Error)] pub(crate) enum GdbTargetError { @@ -159,10 +163,16 @@ pub(crate) trait GuestDebug { /// Returns true whether the provided address is a hardware breakpoint fn is_hw_breakpoint(&self, addr: &u64) -> bool; + /// Returns true whether the provided address is a software breakpoint + fn is_sw_breakpoint(&self, addr: &u64) -> bool; /// Stores the address of the hw breakpoint fn save_hw_breakpoint(&mut self, addr: &u64) -> bool; + /// Stores the data that the sw breakpoint op code replaces + fn save_sw_breakpoint_data(&mut self, addr: u64, data: [u8; 1]); /// Deletes the address of the hw breakpoint from storage fn delete_hw_breakpoint(&mut self, addr: &u64); + /// Retrieves the saved data that the sw breakpoint op code replaces + fn delete_sw_breakpoint_data(&mut self, addr: &u64) -> Option<[u8; 1]>; /// Read registers fn read_regs(&self, vcpu_fd: &Self::Vcpu, regs: &mut X86_64Regs) -> crate::Result<()>; @@ -185,6 +195,63 @@ pub(crate) trait GuestDebug { .then(|| self.set_single_step(vcpu_fd, false)) .ok_or_else(|| new_error!("Failed to save hw breakpoint"))? } + /// Overwrites the guest memory with the SW Breakpoint op code that instructs + /// the vCPU to stop when is executed and stores the overwritten data to be + /// able to restore it + fn add_sw_breakpoint( + &mut self, + vcpu_fd: &Self::Vcpu, + addr: u64, + dbg_mem_access_fn: Arc>, + ) -> crate::Result<()> { + let addr = self.translate_gva(vcpu_fd, addr)?; + + if self.is_sw_breakpoint(&addr) { + return Ok(()); + } + + // Write breakpoint OP code to write to guest memory + let mut save_data = [0; SW_BP_SIZE]; + self.read_addrs(vcpu_fd, addr, &mut save_data[..], dbg_mem_access_fn.clone())?; + self.write_addrs(vcpu_fd, addr, &SW_BP, dbg_mem_access_fn)?; + + // Save guest memory to restore when breakpoint is removed + self.save_sw_breakpoint_data(addr, save_data); + + Ok(()) + } + /// Copies the data from the guest memory address to the provided slice + /// The address is checked to be a valid guest address + fn read_addrs( + &mut self, + vcpu_fd: &Self::Vcpu, + mut gva: u64, + mut data: &mut [u8], + dbg_mem_access_fn: Arc>, + ) -> crate::Result<()> { + let data_len = data.len(); + log::debug!("Read addr: {:X} len: {:X}", gva, data_len); + + while !data.is_empty() { + let gpa = self.translate_gva(vcpu_fd, gva)?; + + let read_len = std::cmp::min( + data.len(), + (PAGE_SIZE - (gpa & (PAGE_SIZE - 1))).try_into().unwrap(), + ); + let offset = gpa as usize - SandboxMemoryLayout::BASE_ADDRESS; + + dbg_mem_access_fn + .try_lock() + .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? + .read(offset, &mut data[..read_len])?; + + data = &mut data[read_len..]; + gva += read_len as u64; + } + + Ok(()) + } /// Removes hardware breakpoint fn remove_hw_breakpoint(&mut self, vcpu_fd: &Self::Vcpu, addr: u64) -> crate::Result<()> { let addr = self.translate_gva(vcpu_fd, addr)?; @@ -196,6 +263,60 @@ pub(crate) trait GuestDebug { }) .ok_or_else(|| new_error!("The address: {:?} is not a hw breakpoint", addr))? } + /// Restores the overwritten data to the guest memory + fn remove_sw_breakpoint( + &mut self, + vcpu_fd: &Self::Vcpu, + addr: u64, + dbg_mem_access_fn: Arc>, + ) -> crate::Result<()> { + let addr = self.translate_gva(vcpu_fd, addr)?; + + if self.is_sw_breakpoint(&addr) { + let save_data = self + .delete_sw_breakpoint_data(&addr) + .ok_or_else(|| new_error!("Expected to contain the sw breakpoint address"))?; + + // Restore saved data to the guest's memory + self.write_addrs(vcpu_fd, addr, &save_data, dbg_mem_access_fn)?; + + Ok(()) + } else { + Err(new_error!("The address: {:?} is not a sw breakpoint", addr)) + } + } + /// Copies the data from the provided slice to the guest memory address + /// The address is checked to be a valid guest address + fn write_addrs( + &mut self, + vcpu_fd: &Self::Vcpu, + mut gva: u64, + mut data: &[u8], + dbg_mem_access_fn: Arc>, + ) -> crate::Result<()> { + let data_len = data.len(); + log::debug!("Write addr: {:X} len: {:X}", gva, data_len); + + while !data.is_empty() { + let gpa = self.translate_gva(vcpu_fd, gva)?; + + let write_len = std::cmp::min( + data.len(), + (PAGE_SIZE - (gpa & (PAGE_SIZE - 1))).try_into().unwrap(), + ); + let offset = gpa as usize - SandboxMemoryLayout::BASE_ADDRESS; + + dbg_mem_access_fn + .try_lock() + .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? + .write(offset, data)?; + + data = &data[write_len..]; + gva += write_len as u64; + } + + Ok(()) + } } /// Debug communication channel that is used for sending a request type and diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 1d38780ed..18f34b77e 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -67,16 +67,12 @@ pub(crate) fn is_hypervisor_present() -> bool { mod debug { use std::sync::{Arc, Mutex}; - use hyperlight_common::mem::PAGE_SIZE; - use super::KVMDriver; use crate::hypervisor::gdb::{ - DebugMsg, DebugResponse, GuestDebug, KvmDebug, VcpuStopReason, X86_64Regs, SW_BP, - SW_BP_SIZE, + DebugMsg, DebugResponse, GuestDebug, KvmDebug, VcpuStopReason, X86_64Regs, }; use crate::hypervisor::handlers::DbgMemAccessHandlerCaller; - use crate::mem::layout::SandboxMemoryLayout; - use crate::{new_error, HyperlightError, Result}; + use crate::{new_error, Result}; impl KVMDriver { /// Resets the debug information to disable debugging @@ -90,144 +86,6 @@ mod debug { Ok(()) } - /// Translates the guest address to physical address - fn translate_gva(&self, gva: u64) -> Result { - let tr = self - .vcpu_fd - .translate_gva(gva) - .map_err(|_| HyperlightError::TranslateGuestAddress(gva))?; - - if tr.valid == 0 { - Err(HyperlightError::TranslateGuestAddress(gva)) - } else { - Ok(tr.physical_address) - } - } - - fn read_addrs( - &mut self, - mut gva: u64, - mut data: &mut [u8], - dbg_mem_access_fn: Arc>, - ) -> Result<()> { - let data_len = data.len(); - log::debug!("Read addr: {:X} len: {:X}", gva, data_len); - - while !data.is_empty() { - let gpa = self.translate_gva(gva)?; - - let read_len = std::cmp::min( - data.len(), - (PAGE_SIZE - (gpa & (PAGE_SIZE - 1))).try_into().unwrap(), - ); - let offset = gpa as usize - SandboxMemoryLayout::BASE_ADDRESS; - - dbg_mem_access_fn - .try_lock() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? - .read(offset, &mut data[..read_len])?; - - data = &mut data[read_len..]; - gva += read_len as u64; - } - - Ok(()) - } - - fn write_addrs( - &mut self, - mut gva: u64, - mut data: &[u8], - dbg_mem_access_fn: Arc>, - ) -> Result<()> { - let data_len = data.len(); - log::debug!("Write addr: {:X} len: {:X}", gva, data_len); - - while !data.is_empty() { - let gpa = self.translate_gva(gva)?; - - let write_len = std::cmp::min( - data.len(), - (PAGE_SIZE - (gpa & (PAGE_SIZE - 1))).try_into().unwrap(), - ); - let offset = gpa as usize - SandboxMemoryLayout::BASE_ADDRESS; - - dbg_mem_access_fn - .try_lock() - .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))? - .write(offset, data)?; - - data = &data[write_len..]; - gva += write_len as u64; - } - - Ok(()) - } - - fn add_sw_breakpoint( - &mut self, - addr: u64, - dbg_mem_access_fn: Arc>, - ) -> Result { - let addr = { - let debug = self - .debug - .as_ref() - .ok_or_else(|| new_error!("Debug is not enabled"))?; - let addr = self.translate_gva(addr)?; - if debug.sw_breakpoints.contains_key(&addr) { - return Ok(true); - } - - addr - }; - - let mut save_data = [0; SW_BP_SIZE]; - self.read_addrs(addr, &mut save_data[..], dbg_mem_access_fn.clone())?; - self.write_addrs(addr, &SW_BP, dbg_mem_access_fn)?; - - { - let debug = self - .debug - .as_mut() - .ok_or_else(|| new_error!("Debug is not enabled"))?; - debug.sw_breakpoints.insert(addr, save_data); - } - - Ok(true) - } - - fn remove_sw_breakpoint( - &mut self, - addr: u64, - dbg_mem_access_fn: Arc>, - ) -> Result { - let (ret, data) = { - let addr = self.translate_gva(addr)?; - let debug = self - .debug - .as_mut() - .ok_or_else(|| new_error!("Debug is not enabled"))?; - - if debug.sw_breakpoints.contains_key(&addr) { - let save_data = debug - .sw_breakpoints - .remove(&addr) - .ok_or_else(|| new_error!("Expected the hashmap to contain the address"))?; - - (true, Some(save_data)) - } else { - (false, None) - } - }; - - if ret { - self.write_addrs(addr, &data.unwrap(), dbg_mem_access_fn)?; - } - - Ok(ret) - } - /// Gdb expects the target to be stopped when connected. /// This method provides a way to set a breakpoint at the entry point /// it does not keep this breakpoint set after the vCPU already stopped at the address @@ -263,9 +121,11 @@ mod debug { DebugMsg::AddHwBreakpoint(addr) => Ok(DebugResponse::AddHwBreakpoint( debug.add_hw_breakpoint(&self.vcpu_fd, addr).is_ok(), )), - DebugMsg::AddSwBreakpoint(addr) => self - .add_sw_breakpoint(addr, dbg_mem_access_fn) - .map(DebugResponse::AddSwBreakpoint), + DebugMsg::AddSwBreakpoint(addr) => Ok(DebugResponse::AddSwBreakpoint( + debug + .add_sw_breakpoint(&self.vcpu_fd, addr, dbg_mem_access_fn) + .is_ok(), + )), DebugMsg::Continue => { debug.set_single_step(&self.vcpu_fd, false)?; Ok(DebugResponse::Continue) @@ -288,7 +148,7 @@ mod debug { DebugMsg::ReadAddr(addr, len) => { let mut data = vec![0u8; len]; - self.read_addrs(addr, &mut data, dbg_mem_access_fn)?; + debug.read_addrs(&self.vcpu_fd, addr, &mut data, dbg_mem_access_fn)?; Ok(DebugResponse::ReadAddr(data)) } @@ -302,15 +162,17 @@ mod debug { DebugMsg::RemoveHwBreakpoint(addr) => Ok(DebugResponse::RemoveHwBreakpoint( debug.remove_hw_breakpoint(&self.vcpu_fd, addr).is_ok(), )), - DebugMsg::RemoveSwBreakpoint(addr) => self - .remove_sw_breakpoint(addr, dbg_mem_access_fn) - .map(DebugResponse::RemoveSwBreakpoint), + DebugMsg::RemoveSwBreakpoint(addr) => Ok(DebugResponse::RemoveSwBreakpoint( + debug + .remove_sw_breakpoint(&self.vcpu_fd, addr, dbg_mem_access_fn) + .is_ok(), + )), DebugMsg::Step => { debug.set_single_step(&self.vcpu_fd, true)?; Ok(DebugResponse::Step) } DebugMsg::WriteAddr(addr, data) => { - self.write_addrs(addr, &data, dbg_mem_access_fn)?; + debug.write_addrs(&self.vcpu_fd, addr, &data, dbg_mem_access_fn)?; Ok(DebugResponse::WriteAddr) } From 338db622426f777e5e77350265a034b8b41fbffd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Mon, 3 Mar 2025 17:52:04 +0200 Subject: [PATCH 07/17] dbg: remove specific method that adds entry point breakpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - it can be done using the generic way to add hw breakpoints - also remove the entry point breakpoint after the vCPU stops to avoid hanging there Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/gdb/kvm_debug.rs | 13 ++++---- src/hyperlight_host/src/hypervisor/kvm.rs | 30 +++++-------------- 2 files changed, 16 insertions(+), 27 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs b/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs index 4b162470f..e861be273 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs @@ -107,7 +107,7 @@ impl KvmDebug { /// Get the reason the vCPU has stopped pub(crate) fn get_stop_reason( - &self, + &mut self, vcpu_fd: &VcpuFd, entrypoint: u64, ) -> Result { @@ -123,10 +123,13 @@ impl KvmDebug { } if self.hw_breakpoints.contains(&gpa) { - return Ok(VcpuStopReason::HwBp); - } - - if gpa == entrypoint { + // In case the hw breakpoint is the entry point, remove it to + // avoid hanging here as gdb does not remove breakpoints it + // has not set. + // Gdb expects the target to be stopped when connected. + if gpa == entrypoint { + self.remove_hw_breakpoint(vcpu_fd, entrypoint)?; + } return Ok(VcpuStopReason::HwBp); } diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 18f34b77e..247e692cb 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -26,7 +26,7 @@ use tracing::{instrument, Span}; use super::fpu::{FP_CONTROL_WORD_DEFAULT, FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; #[cfg(gdb)] -use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse, KvmDebug, VcpuStopReason}; +use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse, GuestDebug, KvmDebug, VcpuStopReason}; #[cfg(gdb)] use super::handlers::DbgMemAccessHandlerWrapper; use super::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper}; @@ -86,26 +86,11 @@ mod debug { Ok(()) } - /// Gdb expects the target to be stopped when connected. - /// This method provides a way to set a breakpoint at the entry point - /// it does not keep this breakpoint set after the vCPU already stopped at the address - pub(crate) fn set_entrypoint_bp(&self) -> Result<()> { - if self.debug.is_some() { - log::debug!("Setting entrypoint bp {:X}", self.entrypoint); - let mut entrypoint_debug = KvmDebug::new(); - entrypoint_debug.add_hw_breakpoint(&self.vcpu_fd, self.entrypoint)?; - - Ok(()) - } else { - Ok(()) - } - } - /// Get the reason the vCPU has stopped - pub(crate) fn get_stop_reason(&self) -> Result { + pub(crate) fn get_stop_reason(&mut self) -> Result { let debug = self .debug - .as_ref() + .as_mut() .ok_or_else(|| new_error!("Debug is not enabled"))?; debug.get_stop_reason(&self.vcpu_fd, self.entrypoint) @@ -271,7 +256,11 @@ impl KVMDriver { #[cfg(gdb)] let (debug, gdb_conn) = if let Some(gdb_conn) = gdb_conn { - (Some(KvmDebug::new()), Some(gdb_conn)) + let mut debug = KvmDebug::new(); + // Add breakpoint to the entry point address + debug.add_hw_breakpoint(&vcpu_fd, entrypoint)?; + + (Some(debug), Some(gdb_conn)) } else { (None, None) }; @@ -292,9 +281,6 @@ impl KVMDriver { gdb_conn, }; - #[cfg(gdb)] - ret.set_entrypoint_bp()?; - Ok(ret) } From a5e88c41e8f403bdb6697b9f7b7b40fc0ef2141e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Wed, 5 Mar 2025 13:02:07 +0200 Subject: [PATCH 08/17] dbg: improve vCPU exit reason checking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - verify the debug registers that report what kind of exception was triggered - in case there is an unknown reason, report it as a SwBp so that the gdb client can inspect what happened Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/gdb/event_loop.rs | 14 +++--- .../src/hypervisor/gdb/kvm_debug.rs | 45 +++++++++++++++---- src/hyperlight_host/src/hypervisor/gdb/mod.rs | 11 +++++ .../src/hypervisor/gdb/x86_64_target.rs | 10 ++--- src/hyperlight_host/src/hypervisor/kvm.rs | 12 +++-- 5 files changed, 68 insertions(+), 24 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs b/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs index 5f885b0ac..72dd97b29 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/event_loop.rs @@ -16,8 +16,9 @@ limitations under the License. use gdbstub::common::Signal; use gdbstub::conn::ConnectionExt; -use gdbstub::stub::run_blocking::{self, WaitForStopReasonError}; -use gdbstub::stub::{BaseStopReason, DisconnectReason, GdbStub, SingleThreadStopReason}; +use gdbstub::stub::{ + run_blocking, BaseStopReason, DisconnectReason, GdbStub, SingleThreadStopReason, +}; use libc::{pthread_kill, SIGRTMIN}; use super::x86_64_target::HyperlightSandboxTarget; @@ -57,13 +58,10 @@ impl run_blocking::BlockingEventLoop for GdbBlockingEventLoop { signal: Signal(SIGRTMIN() as u8), }, VcpuStopReason::Unknown => { - log::warn!("Unknown stop reason - resuming execution"); + log::warn!("Unknown stop reason received"); - target - .resume_vcpu() - .map_err(WaitForStopReasonError::Target)?; - - continue; + // Marking as a SwBreak so the gdb inspect where/why it stopped + BaseStopReason::SwBreak(()) } }; diff --git a/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs b/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs index e861be273..6e8f9d36f 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/kvm_debug.rs @@ -17,14 +17,20 @@ limitations under the License. use std::collections::HashMap; use kvm_bindings::{ - kvm_guest_debug, kvm_regs, KVM_GUESTDBG_ENABLE, KVM_GUESTDBG_SINGLESTEP, + kvm_debug_exit_arch, kvm_guest_debug, kvm_regs, KVM_GUESTDBG_ENABLE, KVM_GUESTDBG_SINGLESTEP, KVM_GUESTDBG_USE_HW_BP, KVM_GUESTDBG_USE_SW_BP, }; use kvm_ioctls::VcpuFd; -use super::{GuestDebug, VcpuStopReason, X86_64Regs, MAX_NO_OF_HW_BP, SW_BP_SIZE}; +use super::{ + GuestDebug, VcpuStopReason, X86_64Regs, DR6_BS_FLAG_MASK, DR6_HW_BP_FLAGS_MASK, + MAX_NO_OF_HW_BP, SW_BP_SIZE, +}; use crate::{new_error, HyperlightError, Result}; +/// Exception id for SW breakpoint +const SW_BP_ID: u32 = 3; + /// KVM Debug struct /// This struct is used to abstract the internal details of the kvm /// guest debugging settings @@ -109,20 +115,25 @@ impl KvmDebug { pub(crate) fn get_stop_reason( &mut self, vcpu_fd: &VcpuFd, + debug_exit: kvm_debug_exit_arch, entrypoint: u64, ) -> Result { - if self.single_step { + // If the BS flag in DR6 register is set, it means a single step + // instruction triggered the exit + // Check page 19-4 Vol. 3B of Intel 64 and IA-32 + // Architectures Software Developer's Manual + if debug_exit.dr6 & DR6_BS_FLAG_MASK != 0 && self.single_step { return Ok(VcpuStopReason::DoneStep); } let ip = self.get_instruction_pointer(vcpu_fd)?; let gpa = self.translate_gva(vcpu_fd, ip)?; - if self.sw_breakpoints.contains_key(&gpa) { - return Ok(VcpuStopReason::SwBp); - } - - if self.hw_breakpoints.contains(&gpa) { + // If any of the B0-B3 flags in DR6 register is set, it means a + // hardware breakpoint triggered the exit + // Check page 19-4 Vol. 3B of Intel 64 and IA-32 + // Architectures Software Developer's Manual + if DR6_HW_BP_FLAGS_MASK & debug_exit.dr6 != 0 && self.hw_breakpoints.contains(&gpa) { // In case the hw breakpoint is the entry point, remove it to // avoid hanging here as gdb does not remove breakpoints it // has not set. @@ -133,6 +144,24 @@ impl KvmDebug { return Ok(VcpuStopReason::HwBp); } + // If the exception ID matches #BP (3) - it means a software breakpoint + // caused the exit + if SW_BP_ID == debug_exit.exception && self.sw_breakpoints.contains_key(&gpa) { + return Ok(VcpuStopReason::SwBp); + } + + // Log an error and provide internal debugging info for fixing + log::error!( + r"The vCPU exited because of an unknown reason: + kvm_debug_exit_arch: {:?} + single_step: {:?} + hw_breakpoints: {:?} + sw_breakpoints: {:?}", + debug_exit, + self.single_step, + self.hw_breakpoints, + self.sw_breakpoints, + ); Ok(VcpuStopReason::Unknown) } } diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index d1febc065..96d1eaeaa 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -50,6 +50,17 @@ const SW_BP: [u8; SW_BP_SIZE] = [SW_BP_OP]; /// Maximum number of supported hardware breakpoints const MAX_NO_OF_HW_BP: usize = 4; +/// Check page 19-4 Vol. 3B of Intel 64 and IA-32 +/// Architectures Software Developer's Manual +/// Bit position of BS flag in DR6 debug register +const DR6_BS_FLAG_POS: usize = 14; +/// Bit mask of BS flag in DR6 debug register +const DR6_BS_FLAG_MASK: u64 = 1 << DR6_BS_FLAG_POS; +/// Bit position of HW breakpoints status in DR6 debug register +const DR6_HW_BP_FLAGS_POS: usize = 0; +/// Bit mask of HW breakpoints status in DR6 debug register +const DR6_HW_BP_FLAGS_MASK: u64 = 0x0F << DR6_HW_BP_FLAGS_POS; + #[derive(Debug, Error)] pub(crate) enum GdbTargetError { #[error("Error encountered while binding to address and port")] diff --git a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs index fbdea448a..a25098bf4 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs @@ -70,11 +70,6 @@ impl HyperlightSandboxTarget { self.hyp_conn.recv() } - /// Non-Blocking check for a response over the communication channel - pub(crate) fn try_recv(&self) -> Result { - self.hyp_conn.try_recv() - } - /// Sends an event to the Hypervisor that tells it to resume vCPU execution /// Note: The method waits for a confirmation message pub(crate) fn resume_vcpu(&mut self) -> Result<(), GdbTargetError> { @@ -89,6 +84,11 @@ impl HyperlightSandboxTarget { } } + /// Non-Blocking check for a response over the communication channel + pub(crate) fn try_recv(&self) -> Result { + self.hyp_conn.try_recv() + } + /// Sends an event to the Hypervisor that tells it to disable debugging /// and continue executing /// Note: The method waits for a confirmation message diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 247e692cb..1433349bf 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -67,6 +67,8 @@ pub(crate) fn is_hypervisor_present() -> bool { mod debug { use std::sync::{Arc, Mutex}; + use kvm_bindings::kvm_debug_exit_arch; + use super::KVMDriver; use crate::hypervisor::gdb::{ DebugMsg, DebugResponse, GuestDebug, KvmDebug, VcpuStopReason, X86_64Regs, @@ -87,13 +89,16 @@ mod debug { } /// Get the reason the vCPU has stopped - pub(crate) fn get_stop_reason(&mut self) -> Result { + pub(crate) fn get_stop_reason( + &mut self, + debug_exit: kvm_debug_exit_arch, + ) -> Result { let debug = self .debug .as_mut() .ok_or_else(|| new_error!("Debug is not enabled"))?; - debug.get_stop_reason(&self.vcpu_fd, self.entrypoint) + debug.get_stop_reason(&self.vcpu_fd, debug_exit, self.entrypoint) } pub(crate) fn process_dbg_request( @@ -469,7 +474,8 @@ impl Hypervisor for KVMDriver { } } #[cfg(gdb)] - Ok(VcpuExit::Debug(_)) => match self.get_stop_reason() { + // KVM provides architecture specific information about the vCPU state when exiting + Ok(VcpuExit::Debug(debug_exit)) => match self.get_stop_reason(debug_exit) { Ok(reason) => HyperlightExit::Debug(reason), Err(e) => { log_then_return!("Error getting stop reason: {:?}", e); From 06c9b94273886f64b19496b40af748525011c6b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Wed, 5 Mar 2025 15:21:10 +0200 Subject: [PATCH 09/17] dbg: prepare for mshv guest debugging implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- src/hyperlight_host/build.rs | 2 +- .../src/hypervisor/hyperv_linux.rs | 20 ++++++++++++++++++- .../src/hypervisor/hypervisor_handler.rs | 2 ++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/hyperlight_host/build.rs b/src/hyperlight_host/build.rs index a8370725d..81649b8c0 100644 --- a/src/hyperlight_host/build.rs +++ b/src/hyperlight_host/build.rs @@ -89,7 +89,7 @@ fn main() -> Result<()> { // Essentially the kvm and mshv 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 = "mshv")] in the codebase. cfg_aliases::cfg_aliases! { - gdb: { all(feature = "gdb", debug_assertions, feature = "kvm", target_os = "linux") }, + gdb: { all(feature = "gdb", debug_assertions, any(feature = "kvm", feature = "mshv2", feature = "mshv3"), target_os = "linux") }, kvm: { all(feature = "kvm", target_os = "linux") }, mshv: { all(any(feature = "mshv2", feature = "mshv3"), target_os = "linux") }, // inprocess feature is aliased with debug_assertions to make it only available in debug-builds. diff --git a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs index fbbeff222..8847b0835 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs @@ -45,6 +45,8 @@ use tracing::{instrument, Span}; use super::fpu::{FP_CONTROL_WORD_DEFAULT, FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; #[cfg(gdb)] +use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse}; +#[cfg(gdb)] use super::handlers::DbgMemAccessHandlerWrapper; use super::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper}; use super::{ @@ -84,6 +86,10 @@ pub(super) struct HypervLinuxDriver { entrypoint: u64, mem_regions: Vec, orig_rsp: GuestPtr, + + #[allow(dead_code)] + #[cfg(gdb)] + gdb_conn: Option>, } impl HypervLinuxDriver { @@ -101,6 +107,7 @@ impl HypervLinuxDriver { entrypoint_ptr: GuestPtr, rsp_ptr: GuestPtr, pml4_ptr: GuestPtr, + #[cfg(gdb)] gdb_conn: Option>, ) -> Result { let mshv = Mshv::new()?; let pr = Default::default(); @@ -138,6 +145,9 @@ impl HypervLinuxDriver { mem_regions, entrypoint: entrypoint_ptr.absolute()?, orig_rsp: rsp_ptr, + + #[cfg(gdb)] + gdb_conn, }) } @@ -457,6 +467,14 @@ mod tests { MemoryRegionFlags::READ | MemoryRegionFlags::WRITE | MemoryRegionFlags::EXECUTE, crate::mem::memory_region::MemoryRegionType::Code, ); - super::HypervLinuxDriver::new(regions.build(), entrypoint_ptr, rsp_ptr, pml4_ptr).unwrap(); + super::HypervLinuxDriver::new( + regions.build(), + entrypoint_ptr, + rsp_ptr, + pml4_ptr, + #[cfg(gdb)] + None, + ) + .unwrap(); } } diff --git a/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs b/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs index f602b840e..4e49a66c0 100644 --- a/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs +++ b/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs @@ -943,6 +943,8 @@ fn set_up_hypervisor_partition( entrypoint_ptr, rsp_ptr, pml4_ptr, + #[cfg(gdb)] + gdb_conn, )?; Ok(Box::new(hv)) } From b2dc614c25371d917c74020917c32858c39fb044 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Wed, 5 Mar 2025 15:45:41 +0200 Subject: [PATCH 10/17] dbg: add basic skeleton for mshv guest debug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- src/hyperlight_host/src/hypervisor/gdb/mod.rs | 4 + .../src/hypervisor/gdb/mshv_debug.rs | 24 ++++ .../src/hypervisor/hyperv_linux.rs | 111 +++++++++++++++++- 3 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 src/hyperlight_host/src/hypervisor/gdb/mshv_debug.rs diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index 96d1eaeaa..31550a6bb 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -17,6 +17,8 @@ limitations under the License. mod event_loop; #[cfg(kvm)] mod kvm_debug; +#[cfg(mshv)] +mod mshv_debug; mod x86_64_target; use std::io::{self, ErrorKind}; @@ -32,6 +34,8 @@ use gdbstub::target::TargetError; use hyperlight_common::mem::PAGE_SIZE; #[cfg(kvm)] pub(crate) use kvm_debug::KvmDebug; +#[cfg(mshv)] +pub(crate) use mshv_debug::MshvDebug; use thiserror::Error; use x86_64_target::HyperlightSandboxTarget; diff --git a/src/hyperlight_host/src/hypervisor/gdb/mshv_debug.rs b/src/hyperlight_host/src/hypervisor/gdb/mshv_debug.rs new file mode 100644 index 000000000..1ee12484d --- /dev/null +++ b/src/hyperlight_host/src/hypervisor/gdb/mshv_debug.rs @@ -0,0 +1,24 @@ +/* +Copyright 2024 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. +*/ + +#[derive(Debug, Default)] +pub(crate) struct MshvDebug {} + +impl MshvDebug { + pub(crate) fn new() -> Self { + Self {} + } +} diff --git a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs index 8847b0835..1e42f544b 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs @@ -45,7 +45,7 @@ use tracing::{instrument, Span}; use super::fpu::{FP_CONTROL_WORD_DEFAULT, FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; #[cfg(gdb)] -use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse}; +use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse, MshvDebug}; #[cfg(gdb)] use super::handlers::DbgMemAccessHandlerWrapper; use super::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper}; @@ -57,8 +57,58 @@ use crate::hypervisor::hypervisor_handler::HypervisorHandler; use crate::hypervisor::HyperlightExit; use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags}; use crate::mem::ptr::{GuestPtr, RawPtr}; +#[cfg(gdb)] +use crate::HyperlightError; use crate::{log_then_return, new_error, Result}; +#[cfg(gdb)] +mod debug { + use std::sync::{Arc, Mutex}; + + use super::HypervLinuxDriver; + use crate::hypervisor::gdb::{DebugMsg, DebugResponse}; + use crate::hypervisor::handlers::DbgMemAccessHandlerCaller; + use crate::{new_error, Result}; + + impl HypervLinuxDriver { + pub fn process_dbg_request( + &mut self, + _req: DebugMsg, + _dbg_mem_access_fn: Arc>, + ) -> Result { + unimplemented!() + } + + pub fn recv_dbg_msg(&mut self) -> Result { + let gdb_conn = self + .gdb_conn + .as_mut() + .ok_or_else(|| new_error!("Debug is not enabled"))?; + + gdb_conn.recv().map_err(|e| { + new_error!( + "Got an error while waiting to receive a + message: {:?}", + e + ) + }) + } + + pub fn send_dbg_msg(&mut self, cmd: DebugResponse) -> Result<()> { + log::debug!("Sending {:?}", cmd); + + let gdb_conn = self + .gdb_conn + .as_mut() + .ok_or_else(|| new_error!("Debug is not enabled"))?; + + gdb_conn + .send(cmd) + .map_err(|e| new_error!("Got an error while sending a response message {:?}", e)) + } + } +} + /// Determine whether the HyperV for Linux hypervisor API is present /// and functional. #[instrument(skip_all, parent = Span::current(), level = "Trace")] @@ -87,6 +137,9 @@ pub(super) struct HypervLinuxDriver { mem_regions: Vec, orig_rsp: GuestPtr, + #[allow(dead_code)] + #[cfg(gdb)] + debug: Option, #[allow(dead_code)] #[cfg(gdb)] gdb_conn: Option>, @@ -131,6 +184,15 @@ impl HypervLinuxDriver { let mut vcpu_fd = vm_fd.create_vcpu(0)?; + #[cfg(gdb)] + let (debug, gdb_conn) = if let Some(gdb_conn) = gdb_conn { + let debug = MshvDebug::new(); + + (Some(debug), Some(gdb_conn)) + } else { + (None, None) + }; + mem_regions.iter().try_for_each(|region| { let mshv_region = region.to_owned().into(); vm_fd.map_user_memory(mshv_region) @@ -146,6 +208,8 @@ impl HypervLinuxDriver { entrypoint: entrypoint_ptr.absolute()?, orig_rsp: rsp_ptr, + #[cfg(gdb)] + debug, #[cfg(gdb)] gdb_conn, }) @@ -401,6 +465,51 @@ impl Hypervisor for HypervLinuxDriver { fn get_memory_regions(&self) -> &[MemoryRegion] { &self.mem_regions } + + #[cfg(gdb)] + fn handle_debug( + &mut self, + dbg_mem_access_fn: std::sync::Arc< + std::sync::Mutex, + >, + stop_reason: super::gdb::VcpuStopReason, + ) -> Result<()> { + self.send_dbg_msg(DebugResponse::VcpuStopped(stop_reason)) + .map_err(|e| new_error!("Couldn't signal vCPU stopped event to GDB thread: {:?}", e))?; + + loop { + log::debug!("Debug wait for event to resume vCPU"); + + // Wait for a message from gdb + let req = self.recv_dbg_msg()?; + + let result = self.process_dbg_request(req, dbg_mem_access_fn.clone()); + + let response = match result { + Ok(response) => response, + // Treat non fatal errors separately so the guest doesn't fail + Err(HyperlightError::TranslateGuestAddress(_)) => DebugResponse::ErrorOccurred, + Err(e) => { + return Err(e); + } + }; + + // If the command was either step or continue, we need to run the vcpu + let cont = matches!( + response, + DebugResponse::Step | DebugResponse::Continue | DebugResponse::DisableDebug + ); + + self.send_dbg_msg(response) + .map_err(|e| new_error!("Couldn't send response to gdb: {:?}", e))?; + + if cont { + break; + } + } + + Ok(()) + } } impl Drop for HypervLinuxDriver { From 387333cbc232da47041a4e57a795d59f8422ee3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Wed, 5 Mar 2025 16:13:36 +0200 Subject: [PATCH 11/17] dbg: add mshv intercepts for #DB and #BP exceptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/hyperv_linux.rs | 51 ++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs index 1e42f544b..8f6d99a58 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs @@ -29,6 +29,12 @@ use std::fmt::{Debug, Formatter}; use log::error; #[cfg(mshv2)] use mshv_bindings::hv_message; +#[cfg(gdb)] +use mshv_bindings::{ + hv_intercept_parameters, hv_intercept_type_HV_INTERCEPT_TYPE_EXCEPTION, + hv_message_type_HVMSG_X64_EXCEPTION_INTERCEPT, mshv_install_intercept, + HV_INTERCEPT_ACCESS_MASK_EXECUTE, +}; use mshv_bindings::{ hv_message_type, hv_message_type_HVMSG_GPA_INTERCEPT, hv_message_type_HVMSG_UNMAPPED_GPA, hv_message_type_HVMSG_X64_HALT, hv_message_type_HVMSG_X64_IO_PORT_INTERCEPT, hv_register_assoc, @@ -66,11 +72,16 @@ mod debug { use std::sync::{Arc, Mutex}; use super::HypervLinuxDriver; - use crate::hypervisor::gdb::{DebugMsg, DebugResponse}; + use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason}; use crate::hypervisor::handlers::DbgMemAccessHandlerCaller; use crate::{new_error, Result}; impl HypervLinuxDriver { + /// Get the reason the vCPU has stopped + pub fn get_stop_reason(&mut self) -> Result { + unimplemented!() + } + pub fn process_dbg_request( &mut self, _req: DebugMsg, @@ -188,6 +199,33 @@ impl HypervLinuxDriver { let (debug, gdb_conn) = if let Some(gdb_conn) = gdb_conn { let debug = MshvDebug::new(); + // The bellow intercepts make the vCPU exit with the Exception Intercept exit code + // Check Table 6-1. Exceptions and Interrupts at Page 6-13 Vol. 1 + // of Intel 64 and IA-32 Architectures Software Developer's Manual + // Install intercept for #DB (1) exception + vm_fd + .install_intercept(mshv_install_intercept { + access_type_mask: HV_INTERCEPT_ACCESS_MASK_EXECUTE, + intercept_type: hv_intercept_type_HV_INTERCEPT_TYPE_EXCEPTION, + // Exception handler #DB (1) + intercept_parameter: hv_intercept_parameters { + exception_vector: 0x1, + }, + }) + .map_err(|e| new_error!("Cannot install debug exception intercept: {}", e))?; + + // Install intercept for #BP (3) exception + vm_fd + .install_intercept(mshv_install_intercept { + access_type_mask: HV_INTERCEPT_ACCESS_MASK_EXECUTE, + intercept_type: hv_intercept_type_HV_INTERCEPT_TYPE_EXCEPTION, + // Exception handler #BP (3) + intercept_parameter: hv_intercept_parameters { + exception_vector: 0x3, + }, + }) + .map_err(|e| new_error!("Cannot install breakpoint exception intercept: {}", e))?; + (Some(debug), Some(gdb_conn)) } else { (None, None) @@ -381,6 +419,8 @@ impl Hypervisor for HypervLinuxDriver { hv_message_type_HVMSG_X64_IO_PORT_INTERCEPT; const UNMAPPED_GPA_MESSAGE: hv_message_type = hv_message_type_HVMSG_UNMAPPED_GPA; const INVALID_GPA_ACCESS_MESSAGE: hv_message_type = hv_message_type_HVMSG_GPA_INTERCEPT; + #[cfg(gdb)] + const EXCEPTION_INTERCEPT: hv_message_type = hv_message_type_HVMSG_X64_EXCEPTION_INTERCEPT; #[cfg(mshv2)] let run_result = { @@ -438,6 +478,15 @@ impl Hypervisor for HypervLinuxDriver { None => HyperlightExit::Mmio(gpa), } } + // The only case an intercept exit is expected is when debugging is enabled + // and the intercepts are installed + #[cfg(gdb)] + EXCEPTION_INTERCEPT => match self.get_stop_reason() { + Ok(reason) => HyperlightExit::Debug(reason), + Err(e) => { + log_then_return!("Error getting stop reason: {:?}", e); + } + }, other => { crate::debug!("mshv Other Exit: Exit: {:#?} \n {:#?}", other, &self); log_then_return!("unknown Hyper-V run message type {:?}", other); From 7f542e19cef007a7996ad52ea59e69efdc90278d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Wed, 5 Mar 2025 16:31:16 +0200 Subject: [PATCH 12/17] dbg: mshv guest debugging support for vCPU operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/gdb/mshv_debug.rs | 248 +++++++++++++++++- .../src/hypervisor/hyperv_linux.rs | 88 ++++++- 2 files changed, 320 insertions(+), 16 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/gdb/mshv_debug.rs b/src/hyperlight_host/src/hypervisor/gdb/mshv_debug.rs index 1ee12484d..03d3ba5c2 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mshv_debug.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mshv_debug.rs @@ -14,11 +14,255 @@ See the License for the specific language governing permissions and limitations under the License. */ +#[cfg(mshv2)] +extern crate mshv_bindings2 as mshv_bindings; +#[cfg(mshv2)] +extern crate mshv_ioctls2 as mshv_ioctls; + +#[cfg(mshv3)] +extern crate mshv_bindings3 as mshv_bindings; +#[cfg(mshv3)] +extern crate mshv_ioctls3 as mshv_ioctls; + +use mshv_bindings::{ + DebugRegisters, StandardRegisters, HV_TRANSLATE_GVA_VALIDATE_READ, + HV_TRANSLATE_GVA_VALIDATE_WRITE, +}; +use mshv_ioctls::VcpuFd; + +use super::{ + GuestDebug, VcpuStopReason, X86_64Regs, DR6_BS_FLAG_MASK, DR6_HW_BP_FLAGS_MASK, MAX_NO_OF_HW_BP, +}; +use crate::{new_error, HyperlightError, Result}; + #[derive(Debug, Default)] -pub(crate) struct MshvDebug {} +pub(crate) struct MshvDebug { + /// vCPU stepping state + single_step: bool, + + /// Array of addresses for HW breakpoints + hw_breakpoints: Vec, + /// Debug registers + dbg_cfg: DebugRegisters, +} impl MshvDebug { pub(crate) fn new() -> Self { - Self {} + Self { + single_step: false, + hw_breakpoints: vec![], + dbg_cfg: DebugRegisters::default(), + } + } + + /// Returns the instruction pointer from the stopped vCPU + fn get_instruction_pointer(&self, vcpu_fd: &VcpuFd) -> Result { + let regs = vcpu_fd + .get_regs() + .map_err(|e| new_error!("Could not retrieve registers from vCPU: {:?}", e))?; + + Ok(regs.rip) + } + + /// This method sets the vCPU debug register fields to enable breakpoints at + /// specific addresses + /// + /// The first 4 debug registers are used to set the addresses + /// The 4th and 5th debug registers are obsolete and not used + /// The 7th debug register is used to enable the breakpoints + /// For more information see: DEBUG REGISTERS chapter in the architecture + /// manual + fn set_debug_config(&mut self, vcpu_fd: &VcpuFd, step: bool) -> Result<()> { + let addrs = &self.hw_breakpoints; + + let mut dbg_cfg = DebugRegisters::default(); + for (k, addr) in addrs.iter().enumerate() { + match k { + 0 => { + dbg_cfg.dr0 = *addr; + } + 1 => { + dbg_cfg.dr1 = *addr; + } + 2 => { + dbg_cfg.dr2 = *addr; + } + 3 => { + dbg_cfg.dr3 = *addr; + } + _ => { + Err(new_error!("Tried to set more than 4 HW breakpoints"))?; + } + } + dbg_cfg.dr7 |= 1 << (k * 2); + } + + self.dbg_cfg = dbg_cfg; + vcpu_fd + .set_debug_regs(&self.dbg_cfg) + .map_err(|e| new_error!("Could not set guest debug: {:?}", e))?; + + self.single_step = step; + + let mut regs = vcpu_fd + .get_regs() + .map_err(|e| new_error!("Could not get registers: {:?}", e))?; + + // Set TF Flag to enable Traps + if self.single_step { + regs.rflags |= 1 << 8; + } else { + regs.rflags &= !(1 << 8); + } + + vcpu_fd + .set_regs(®s) + .map_err(|e| new_error!("Could not set registers: {:?}", e))?; + + Ok(()) + } + + /// Returns the vCPU stop reason + pub(crate) fn get_stop_reason( + &mut self, + vcpu_fd: &VcpuFd, + entrypoint: u64, + ) -> Result { + // MSHV does not provide info on the vCPU exits but the debug + // information can be retrieved from the DEBUG REGISTERS + let regs = vcpu_fd + .get_debug_regs() + .map_err(|e| new_error!("Cannot retrieve debug registers from vCPU: {}", e))?; + + // DR6 register contains debug state related information + let debug_status = regs.dr6; + + // If the BS flag in DR6 register is set, it means a single step + // instruction triggered the exit + // Check page 19-4 Vol. 3B of Intel 64 and IA-32 + // Architectures Software Developer's Manual + if debug_status & DR6_BS_FLAG_MASK != 0 && self.single_step { + return Ok(VcpuStopReason::DoneStep); + } + + let ip = self.get_instruction_pointer(vcpu_fd)?; + let gpa = self.translate_gva(vcpu_fd, ip)?; + + // If any of the B0-B3 flags in DR6 register is set, it means a + // hardware breakpoint triggered the exit + // Check page 19-4 Vol. 3B of Intel 64 and IA-32 + // Architectures Software Developer's Manual + if debug_status & DR6_HW_BP_FLAGS_MASK != 0 && self.hw_breakpoints.contains(&gpa) { + // In case the hw breakpoint is the entry point, remove it to + // avoid hanging here as gdb does not remove breakpoints it + // has not set. + // Gdb expects the target to be stopped when connected. + if gpa == entrypoint { + self.remove_hw_breakpoint(vcpu_fd, entrypoint)?; + } + return Ok(VcpuStopReason::HwBp); + } + + Ok(VcpuStopReason::Unknown) + } +} + +impl GuestDebug for MshvDebug { + type Vcpu = VcpuFd; + + fn is_hw_breakpoint(&self, addr: &u64) -> bool { + self.hw_breakpoints.contains(addr) + } + fn is_sw_breakpoint(&self, _addr: &u64) -> bool { + unimplemented!() + } + fn save_hw_breakpoint(&mut self, addr: &u64) -> bool { + if self.hw_breakpoints.len() >= MAX_NO_OF_HW_BP { + false + } else { + self.hw_breakpoints.push(*addr); + + true + } + } + fn save_sw_breakpoint_data(&mut self, _addr: u64, _data: [u8; 1]) { + unimplemented!() + } + fn delete_hw_breakpoint(&mut self, addr: &u64) { + self.hw_breakpoints.retain(|&a| a != *addr); + } + fn delete_sw_breakpoint_data(&mut self, _addr: &u64) -> Option<[u8; 1]> { + unimplemented!() + } + + fn read_regs(&self, vcpu_fd: &Self::Vcpu, regs: &mut X86_64Regs) -> Result<()> { + log::debug!("Read registers"); + let vcpu_regs = vcpu_fd + .get_regs() + .map_err(|e| new_error!("Could not read guest registers: {:?}", e))?; + + regs.rax = vcpu_regs.rax; + regs.rbx = vcpu_regs.rbx; + regs.rcx = vcpu_regs.rcx; + regs.rdx = vcpu_regs.rdx; + regs.rsi = vcpu_regs.rsi; + regs.rdi = vcpu_regs.rdi; + regs.rbp = vcpu_regs.rbp; + regs.rsp = vcpu_regs.rsp; + regs.r8 = vcpu_regs.r8; + regs.r9 = vcpu_regs.r9; + regs.r10 = vcpu_regs.r10; + regs.r11 = vcpu_regs.r11; + regs.r12 = vcpu_regs.r12; + regs.r13 = vcpu_regs.r13; + regs.r14 = vcpu_regs.r14; + regs.r15 = vcpu_regs.r15; + + regs.rip = vcpu_regs.rip; + regs.rflags = vcpu_regs.rflags; + + Ok(()) + } + + fn set_single_step(&mut self, vcpu_fd: &Self::Vcpu, enable: bool) -> Result<()> { + self.set_debug_config(vcpu_fd, enable) + } + + fn translate_gva(&self, vcpu_fd: &Self::Vcpu, gva: u64) -> Result { + let flags = (HV_TRANSLATE_GVA_VALIDATE_READ | HV_TRANSLATE_GVA_VALIDATE_WRITE) as u64; + let (addr, _) = vcpu_fd + .translate_gva(gva, flags) + .map_err(|_| HyperlightError::TranslateGuestAddress(gva))?; + + Ok(addr) + } + + fn write_regs(&self, vcpu_fd: &Self::Vcpu, regs: &X86_64Regs) -> Result<()> { + log::debug!("Write registers"); + let new_regs = StandardRegisters { + rax: regs.rax, + rbx: regs.rbx, + rcx: regs.rcx, + rdx: regs.rdx, + rsi: regs.rsi, + rdi: regs.rdi, + rbp: regs.rbp, + rsp: regs.rsp, + r8: regs.r8, + r9: regs.r9, + r10: regs.r10, + r11: regs.r11, + r12: regs.r12, + r13: regs.r13, + r14: regs.r14, + r15: regs.r15, + + rip: regs.rip, + rflags: regs.rflags, + }; + + vcpu_fd + .set_regs(&new_regs) + .map_err(|e| new_error!("Could not write guest registers: {:?}", e)) } } diff --git a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs index 8f6d99a58..2b2f0da7a 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs @@ -51,7 +51,7 @@ use tracing::{instrument, Span}; use super::fpu::{FP_CONTROL_WORD_DEFAULT, FP_TAG_WORD_DEFAULT, MXCSR_DEFAULT}; #[cfg(gdb)] -use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse, MshvDebug}; +use super::gdb::{DebugCommChannel, DebugMsg, DebugResponse, GuestDebug, MshvDebug}; #[cfg(gdb)] use super::handlers::DbgMemAccessHandlerWrapper; use super::handlers::{MemAccessHandlerWrapper, OutBHandlerWrapper}; @@ -71,26 +71,87 @@ use crate::{log_then_return, new_error, Result}; mod debug { use std::sync::{Arc, Mutex}; - use super::HypervLinuxDriver; - use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason}; + use super::{HypervLinuxDriver, *}; + use crate::hypervisor::gdb::{DebugMsg, DebugResponse, VcpuStopReason, X86_64Regs}; use crate::hypervisor::handlers::DbgMemAccessHandlerCaller; use crate::{new_error, Result}; impl HypervLinuxDriver { + /// Resets the debug information to disable debugging + fn disable_debug(&mut self) -> Result<()> { + let mut debug = MshvDebug::default(); + + debug.set_single_step(&self.vcpu_fd, false)?; + + self.debug = Some(debug); + + Ok(()) + } + /// Get the reason the vCPU has stopped - pub fn get_stop_reason(&mut self) -> Result { - unimplemented!() + pub(crate) fn get_stop_reason(&mut self) -> Result { + let debug = self + .debug + .as_mut() + .ok_or_else(|| new_error!("Debug is not enabled"))?; + + debug.get_stop_reason(&self.vcpu_fd, self.entrypoint) } - pub fn process_dbg_request( + pub(crate) fn process_dbg_request( &mut self, - _req: DebugMsg, - _dbg_mem_access_fn: Arc>, + req: DebugMsg, + dbg_mem_access_fn: Arc>, ) -> Result { - unimplemented!() + if let Some(debug) = self.debug.as_mut() { + match req { + DebugMsg::AddHwBreakpoint(addr) => Ok(DebugResponse::AddHwBreakpoint( + debug.add_hw_breakpoint(&self.vcpu_fd, addr).is_ok(), + )), + DebugMsg::Continue => { + debug.set_single_step(&self.vcpu_fd, false)?; + Ok(DebugResponse::Continue) + } + DebugMsg::DisableDebug => { + self.disable_debug()?; + + Ok(DebugResponse::DisableDebug) + } + DebugMsg::GetCodeSectionOffset => { + let offset = dbg_mem_access_fn + .try_lock() + .map_err(|e| { + new_error!("Error locking at {}:{}: {}", file!(), line!(), e) + })? + .get_code_offset()?; + + Ok(DebugResponse::GetCodeSectionOffset(offset as u64)) + } + DebugMsg::ReadRegisters => { + let mut regs = X86_64Regs::default(); + + debug + .read_regs(&self.vcpu_fd, &mut regs) + .map(|_| DebugResponse::ReadRegisters(regs)) + } + DebugMsg::RemoveHwBreakpoint(addr) => Ok(DebugResponse::RemoveHwBreakpoint( + debug.remove_hw_breakpoint(&self.vcpu_fd, addr).is_ok(), + )), + DebugMsg::Step => { + debug.set_single_step(&self.vcpu_fd, true)?; + Ok(DebugResponse::Step) + } + DebugMsg::WriteRegisters(regs) => debug + .write_regs(&self.vcpu_fd, ®s) + .map(|_| DebugResponse::WriteRegisters), + _ => Err(new_error!("Not yet implemented")), + } + } else { + Err(new_error!("Debugging is not enabled")) + } } - pub fn recv_dbg_msg(&mut self) -> Result { + pub(crate) fn recv_dbg_msg(&mut self) -> Result { let gdb_conn = self .gdb_conn .as_mut() @@ -105,7 +166,7 @@ mod debug { }) } - pub fn send_dbg_msg(&mut self, cmd: DebugResponse) -> Result<()> { + pub(crate) fn send_dbg_msg(&mut self, cmd: DebugResponse) -> Result<()> { log::debug!("Sending {:?}", cmd); let gdb_conn = self @@ -148,10 +209,8 @@ pub(super) struct HypervLinuxDriver { mem_regions: Vec, orig_rsp: GuestPtr, - #[allow(dead_code)] #[cfg(gdb)] debug: Option, - #[allow(dead_code)] #[cfg(gdb)] gdb_conn: Option>, } @@ -197,7 +256,8 @@ impl HypervLinuxDriver { #[cfg(gdb)] let (debug, gdb_conn) = if let Some(gdb_conn) = gdb_conn { - let debug = MshvDebug::new(); + let mut debug = MshvDebug::new(); + debug.add_hw_breakpoint(&vcpu_fd, entrypoint_ptr.absolute()?)?; // The bellow intercepts make the vCPU exit with the Exception Intercept exit code // Check Table 6-1. Exceptions and Interrupts at Page 6-13 Vol. 1 From f3701a7d81f87602245ccb5e61e555aefc7ced47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Wed, 5 Mar 2025 16:37:49 +0200 Subject: [PATCH 13/17] dbg: mshv guest debugging support for memory inspection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/gdb/mshv_debug.rs | 29 ++++++++++++++----- .../src/hypervisor/hyperv_linux.rs | 23 ++++++++++++++- 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/gdb/mshv_debug.rs b/src/hyperlight_host/src/hypervisor/gdb/mshv_debug.rs index 03d3ba5c2..036aadbd3 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mshv_debug.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mshv_debug.rs @@ -24,6 +24,8 @@ extern crate mshv_bindings3 as mshv_bindings; #[cfg(mshv3)] extern crate mshv_ioctls3 as mshv_ioctls; +use std::collections::HashMap; + use mshv_bindings::{ DebugRegisters, StandardRegisters, HV_TRANSLATE_GVA_VALIDATE_READ, HV_TRANSLATE_GVA_VALIDATE_WRITE, @@ -31,7 +33,8 @@ use mshv_bindings::{ use mshv_ioctls::VcpuFd; use super::{ - GuestDebug, VcpuStopReason, X86_64Regs, DR6_BS_FLAG_MASK, DR6_HW_BP_FLAGS_MASK, MAX_NO_OF_HW_BP, + GuestDebug, VcpuStopReason, X86_64Regs, DR6_BS_FLAG_MASK, DR6_HW_BP_FLAGS_MASK, + MAX_NO_OF_HW_BP, SW_BP_SIZE, }; use crate::{new_error, HyperlightError, Result}; @@ -42,6 +45,9 @@ pub(crate) struct MshvDebug { /// Array of addresses for HW breakpoints hw_breakpoints: Vec, + /// Saves the bytes modified to enable SW breakpoints + sw_breakpoints: HashMap, + /// Debug registers dbg_cfg: DebugRegisters, } @@ -51,6 +57,7 @@ impl MshvDebug { Self { single_step: false, hw_breakpoints: vec![], + sw_breakpoints: HashMap::new(), dbg_cfg: DebugRegisters::default(), } } @@ -163,6 +170,14 @@ impl MshvDebug { return Ok(VcpuStopReason::HwBp); } + // mshv does not provide a way to specify which exception triggered the + // vCPU exit as the mshv intercepts both #DB and #BP + // We check against the SW breakpoints Hashmap to detect whether the + // vCPU exited due to a SW breakpoint + if self.sw_breakpoints.contains_key(&gpa) { + return Ok(VcpuStopReason::SwBp); + } + Ok(VcpuStopReason::Unknown) } } @@ -173,8 +188,8 @@ impl GuestDebug for MshvDebug { fn is_hw_breakpoint(&self, addr: &u64) -> bool { self.hw_breakpoints.contains(addr) } - fn is_sw_breakpoint(&self, _addr: &u64) -> bool { - unimplemented!() + fn is_sw_breakpoint(&self, addr: &u64) -> bool { + self.sw_breakpoints.contains_key(addr) } fn save_hw_breakpoint(&mut self, addr: &u64) -> bool { if self.hw_breakpoints.len() >= MAX_NO_OF_HW_BP { @@ -185,14 +200,14 @@ impl GuestDebug for MshvDebug { true } } - fn save_sw_breakpoint_data(&mut self, _addr: u64, _data: [u8; 1]) { - unimplemented!() + fn save_sw_breakpoint_data(&mut self, addr: u64, data: [u8; 1]) { + _ = self.sw_breakpoints.insert(addr, data); } fn delete_hw_breakpoint(&mut self, addr: &u64) { self.hw_breakpoints.retain(|&a| a != *addr); } - fn delete_sw_breakpoint_data(&mut self, _addr: &u64) -> Option<[u8; 1]> { - unimplemented!() + fn delete_sw_breakpoint_data(&mut self, addr: &u64) -> Option<[u8; 1]> { + self.sw_breakpoints.remove(addr) } fn read_regs(&self, vcpu_fd: &Self::Vcpu, regs: &mut X86_64Regs) -> Result<()> { diff --git a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs index 2b2f0da7a..c6eb41bcc 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs @@ -108,6 +108,11 @@ mod debug { DebugMsg::AddHwBreakpoint(addr) => Ok(DebugResponse::AddHwBreakpoint( debug.add_hw_breakpoint(&self.vcpu_fd, addr).is_ok(), )), + DebugMsg::AddSwBreakpoint(addr) => Ok(DebugResponse::AddSwBreakpoint( + debug + .add_sw_breakpoint(&self.vcpu_fd, addr, dbg_mem_access_fn) + .is_ok(), + )), DebugMsg::Continue => { debug.set_single_step(&self.vcpu_fd, false)?; Ok(DebugResponse::Continue) @@ -127,6 +132,13 @@ mod debug { Ok(DebugResponse::GetCodeSectionOffset(offset as u64)) } + DebugMsg::ReadAddr(addr, len) => { + let mut data = vec![0u8; len]; + + debug.read_addrs(&self.vcpu_fd, addr, &mut data, dbg_mem_access_fn)?; + + Ok(DebugResponse::ReadAddr(data)) + } DebugMsg::ReadRegisters => { let mut regs = X86_64Regs::default(); @@ -137,14 +149,23 @@ mod debug { DebugMsg::RemoveHwBreakpoint(addr) => Ok(DebugResponse::RemoveHwBreakpoint( debug.remove_hw_breakpoint(&self.vcpu_fd, addr).is_ok(), )), + DebugMsg::RemoveSwBreakpoint(addr) => Ok(DebugResponse::RemoveSwBreakpoint( + debug + .remove_sw_breakpoint(&self.vcpu_fd, addr, dbg_mem_access_fn) + .is_ok(), + )), DebugMsg::Step => { debug.set_single_step(&self.vcpu_fd, true)?; Ok(DebugResponse::Step) } + DebugMsg::WriteAddr(addr, data) => { + debug.write_addrs(&self.vcpu_fd, addr, &data, dbg_mem_access_fn)?; + + Ok(DebugResponse::WriteAddr) + } DebugMsg::WriteRegisters(regs) => debug .write_regs(&self.vcpu_fd, ®s) .map(|_| DebugResponse::WriteRegisters), - _ => Err(new_error!("Not yet implemented")), } } else { Err(new_error!("Debugging is not enabled")) From 3c7178305616f0ae98bd847e7b8e07ddc713655b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Wed, 5 Mar 2025 18:22:58 +0200 Subject: [PATCH 14/17] dbg: read/write addresses check offset calculation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - use checked_sub to avoid issues from addresses that could cause underflow Signed-off-by: Doru Blânzeanu --- src/hyperlight_host/src/hypervisor/gdb/mod.rs | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/gdb/mod.rs b/src/hyperlight_host/src/hypervisor/gdb/mod.rs index 31550a6bb..95b81ed6a 100644 --- a/src/hyperlight_host/src/hypervisor/gdb/mod.rs +++ b/src/hyperlight_host/src/hypervisor/gdb/mod.rs @@ -41,7 +41,7 @@ use x86_64_target::HyperlightSandboxTarget; use crate::hypervisor::handlers::DbgMemAccessHandlerCaller; use crate::mem::layout::SandboxMemoryLayout; -use crate::new_error; +use crate::{new_error, HyperlightError}; /// Software Breakpoint size in memory const SW_BP_SIZE: usize = 1; @@ -254,7 +254,14 @@ pub(crate) trait GuestDebug { data.len(), (PAGE_SIZE - (gpa & (PAGE_SIZE - 1))).try_into().unwrap(), ); - let offset = gpa as usize - SandboxMemoryLayout::BASE_ADDRESS; + let offset = (gpa as usize) + .checked_sub(SandboxMemoryLayout::BASE_ADDRESS) + .ok_or_else(|| { + log::warn!( + "gva=0x{:#X} causes subtract with underflow: \"gpa - BASE_ADDRESS={:#X}-{:#X}\"", + gva, gpa, SandboxMemoryLayout::BASE_ADDRESS); + HyperlightError::TranslateGuestAddress(gva) + })?; dbg_mem_access_fn .try_lock() @@ -319,7 +326,14 @@ pub(crate) trait GuestDebug { data.len(), (PAGE_SIZE - (gpa & (PAGE_SIZE - 1))).try_into().unwrap(), ); - let offset = gpa as usize - SandboxMemoryLayout::BASE_ADDRESS; + let offset = (gpa as usize) + .checked_sub(SandboxMemoryLayout::BASE_ADDRESS) + .ok_or_else(|| { + log::warn!( + "gva=0x{:#X} causes subtract with underflow: \"gpa - BASE_ADDRESS={:#X}-{:#X}\"", + gva, gpa, SandboxMemoryLayout::BASE_ADDRESS); + HyperlightError::TranslateGuestAddress(gva) + })?; dbg_mem_access_fn .try_lock() From 0b8ded6a444c9d3cb6a77eac8037aef9d42f63fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Wed, 5 Mar 2025 18:26:20 +0200 Subject: [PATCH 15/17] dbg: enable ci guest debugging tests on mshv MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - change gdb commands in test to be usable with older gdb versions - change Justfile to provide mshv3 feature - change gdb test to invoke cargo test with correct features for mshv Signed-off-by: Doru Blânzeanu --- .github/workflows/dep_rust.yml | 4 ++-- Justfile | 6 +++--- src/hyperlight_host/examples/guest-debugging/main.rs | 11 ++++++++--- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/.github/workflows/dep_rust.yml b/.github/workflows/dep_rust.yml index ef847f2f2..ba9d3f2e6 100644 --- a/.github/workflows/dep_rust.yml +++ b/.github/workflows/dep_rust.yml @@ -124,11 +124,11 @@ jobs: run: just run-rust-examples-linux ${{ matrix.config }} ${{ matrix.hypervisor == 'mshv3' && 'mshv3' || ''}} - name: Run Rust Gdb tests - linux - if: runner.os == 'Linux' && matrix.hypervisor == 'kvm' + if: runner.os == 'Linux' env: CARGO_TERM_COLOR: always RUST_LOG: debug - run: just test-rust-gdb-debugging ${{ matrix.config }} + run: just test-rust-gdb-debugging ${{ matrix.config }} ${{ matrix.hypervisor == 'mshv3' && 'mshv3' || ''}} ### Benchmarks ### - name: Install github-cli (Linux mariner) diff --git a/Justfile b/Justfile index 2dd3fbc27..8b02d63d2 100644 --- a/Justfile +++ b/Justfile @@ -117,9 +117,9 @@ test-rust-feature-compilation-fail target=default-target: {{ if os() == "linux" { "! cargo check -p hyperlight-host --no-default-features 2> /dev/null"} else { "" } }} # Test rust gdb debugging -test-rust-gdb-debugging target=default-target: (build-rust target) - {{ set-trace-env-vars }} cargo test --profile={{ if target == "debug" { "dev" } else { target } }} --example guest-debugging --features gdb - {{ set-trace-env-vars }} cargo test --profile={{ if target == "debug" { "dev" } else { target } }} --features gdb -- test_gdb +test-rust-gdb-debugging target=default-target features="": (build-rust target) + {{ set-trace-env-vars }} cargo test --profile={{ if target == "debug" { "dev" } else { target } }} --example guest-debugging {{ if features =="" {'--features gdb'} else { "--features gdb," + features } }} + {{ set-trace-env-vars }} cargo test --profile={{ if target == "debug" { "dev" } else { target } }} {{ if features =="" {'--features gdb'} else { "--features gdb," + features } }} -- test_gdb test target=default-target: (test-rust target) diff --git a/src/hyperlight_host/examples/guest-debugging/main.rs b/src/hyperlight_host/examples/guest-debugging/main.rs index da3010b9c..9ebde14b4 100644 --- a/src/hyperlight_host/examples/guest-debugging/main.rs +++ b/src/hyperlight_host/examples/guest-debugging/main.rs @@ -107,7 +107,7 @@ mod tests { set pagination off set logging file {out_file_path} - set logging enabled on + set logging on break hyperlight_main commands @@ -118,7 +118,7 @@ mod tests { continue - set logging enabled off + set logging off quit " ) @@ -134,12 +134,17 @@ mod tests { write_cmds_file(&cmd_file_path, &out_file_path) .expect("Failed to write gdb commands to file"); + #[cfg(mshv3)] + let features = "gdb,mshv3"; + #[cfg(not(mshv3))] + let features = "gdb"; + let mut guest_child = Command::new("cargo") .arg("run") .arg("--example") .arg("guest-debugging") .arg("--features") - .arg("gdb") + .arg(features) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .spawn() From f8aff0d40e8a892b8d80bb7efde4ce5c703958bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Thu, 6 Mar 2025 12:07:04 +0200 Subject: [PATCH 16/17] dbg: update documentation to specify the mshv debug support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- docs/debugging-hyperlight.md | 4 ++++ docs/how-to-debug-a-hyperlight-guest.md | 14 +++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/docs/debugging-hyperlight.md b/docs/debugging-hyperlight.md index 9ff3b8229..8abb9a645 100644 --- a/docs/debugging-hyperlight.md +++ b/docs/debugging-hyperlight.md @@ -42,3 +42,7 @@ cargo test --package hyperlight-host --test integration_test --features print_de To dump the details of the memory configuration, the virtual processors register state and the contents of the VM memory set the feature `crashdump` and run a debug build. This will result in a dump file being created in the temporary directory. The name and location of the dump file will be printed to the console and logged as an error message. There are no tools at this time to analyze the dump file, but it can be useful for debugging. + +## Debugging guests + +For more information on how to debug the Hyperlight guests check the following [link](./how-to-debug-a-hyperlight-guest.md). diff --git a/docs/how-to-debug-a-hyperlight-guest.md b/docs/how-to-debug-a-hyperlight-guest.md index 7f9c14732..abf571062 100644 --- a/docs/how-to-debug-a-hyperlight-guest.md +++ b/docs/how-to-debug-a-hyperlight-guest.md @@ -1,13 +1,13 @@ -# How to debug a Hyperlight **KVM** guest using gdb +# How to debug a Hyperlight guest using gdb on Linux -Hyperlight supports gdb debugging of a **KVM** guest running inside a Hyperlight sandbox. -When Hyperlight is compiled with the `gdb` feature enabled, a Hyperlight KVM sandbox can be configured +Hyperlight supports gdb debugging of a **KVM** or **MSHV** guest running inside a Hyperlight sandbox on Linux. +When Hyperlight is compiled with the `gdb` feature enabled, a Hyperlight sandbox can be configured to start listening for a gdb connection. ## Supported features -The Hyperlight `gdb` feature enables **KVM** guest debugging: - - an entry point breakpoint is automatically set for the guest to stop +The Hyperlight `gdb` feature enables **KVM** and **MSHV** guest debugging to: + - stop at an entry point breakpoint which is automatically set by Hyperlight - add and remove HW breakpoints (maximum 4 set breakpoints at a time) - add and remove SW breakpoints - read and write registers @@ -18,7 +18,7 @@ The Hyperlight `gdb` feature enables **KVM** guest debugging: ## Expected behavior Below is a list describing some cases of expected behavior from a gdb debug -session of a guest binary running inside a KVM Hyperlight sandbox. +session of a guest binary running inside a Hyperlight sandbox on Linux. - when the `gdb` feature is enabled and a SandboxConfiguration is provided a debug port, the created sandbox will wait for a gdb client to connect on the @@ -154,7 +154,7 @@ is sent over the communication channel to the hypervisor handler for the sandbox to resolve. Below is a sequence diagram that shows the interaction between the entities -involved in the gdb debugging of a Hyperlight guest running inside a KVM sandbox. +involved in the gdb debugging of a Hyperlight guest running inside a **KVM** or **MSHV** sandbox. ``` ┌───────────────────────────────────────────────────────────────────────────────────────────────┐ From 025a7399a2bf85d96c5f35effce7a1d92c000bf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Doru=20Bl=C3=A2nzeanu?= Date: Wed, 19 Mar 2025 17:39:18 +0200 Subject: [PATCH 17/17] dbg: add specific logging for each debug command error encountered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Doru Blânzeanu --- .../src/hypervisor/hyperv_linux.rs | 81 +++++++++++++++++-- src/hyperlight_host/src/hypervisor/kvm.rs | 81 +++++++++++++++++-- 2 files changed, 146 insertions(+), 16 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs index c6eb41bcc..8c419c18b 100644 --- a/src/hyperlight_host/src/hypervisor/hyperv_linux.rs +++ b/src/hyperlight_host/src/hypervisor/hyperv_linux.rs @@ -106,19 +106,40 @@ mod debug { if let Some(debug) = self.debug.as_mut() { match req { DebugMsg::AddHwBreakpoint(addr) => Ok(DebugResponse::AddHwBreakpoint( - debug.add_hw_breakpoint(&self.vcpu_fd, addr).is_ok(), + debug + .add_hw_breakpoint(&self.vcpu_fd, addr) + .map_err(|e| { + log::error!("Failed to add hw breakpoint: {:?}", e); + + e + }) + .is_ok(), )), DebugMsg::AddSwBreakpoint(addr) => Ok(DebugResponse::AddSwBreakpoint( debug .add_sw_breakpoint(&self.vcpu_fd, addr, dbg_mem_access_fn) + .map_err(|e| { + log::error!("Failed to add sw breakpoint: {:?}", e); + + e + }) .is_ok(), )), DebugMsg::Continue => { - debug.set_single_step(&self.vcpu_fd, false)?; + debug.set_single_step(&self.vcpu_fd, false).map_err(|e| { + log::error!("Failed to continue execution: {:?}", e); + + e + })?; + Ok(DebugResponse::Continue) } DebugMsg::DisableDebug => { - self.disable_debug()?; + self.disable_debug().map_err(|e| { + log::error!("Failed to disable debugging: {:?}", e); + + e + })?; Ok(DebugResponse::DisableDebug) } @@ -128,14 +149,25 @@ mod debug { .map_err(|e| { new_error!("Error locking at {}:{}: {}", file!(), line!(), e) })? - .get_code_offset()?; + .get_code_offset() + .map_err(|e| { + log::error!("Failed to get code offset: {:?}", e); + + e + })?; Ok(DebugResponse::GetCodeSectionOffset(offset as u64)) } DebugMsg::ReadAddr(addr, len) => { let mut data = vec![0u8; len]; - debug.read_addrs(&self.vcpu_fd, addr, &mut data, dbg_mem_access_fn)?; + debug + .read_addrs(&self.vcpu_fd, addr, &mut data, dbg_mem_access_fn) + .map_err(|e| { + log::error!("Failed to read from address: {:?}", e); + + e + })?; Ok(DebugResponse::ReadAddr(data)) } @@ -144,27 +176,60 @@ mod debug { debug .read_regs(&self.vcpu_fd, &mut regs) + .map_err(|e| { + log::error!("Failed to read registers: {:?}", e); + + e + }) .map(|_| DebugResponse::ReadRegisters(regs)) } DebugMsg::RemoveHwBreakpoint(addr) => Ok(DebugResponse::RemoveHwBreakpoint( - debug.remove_hw_breakpoint(&self.vcpu_fd, addr).is_ok(), + debug + .remove_hw_breakpoint(&self.vcpu_fd, addr) + .map_err(|e| { + log::error!("Failed to remove hw breakpoint: {:?}", e); + + e + }) + .is_ok(), )), DebugMsg::RemoveSwBreakpoint(addr) => Ok(DebugResponse::RemoveSwBreakpoint( debug .remove_sw_breakpoint(&self.vcpu_fd, addr, dbg_mem_access_fn) + .map_err(|e| { + log::error!("Failed to remove sw breakpoint: {:?}", e); + + e + }) .is_ok(), )), DebugMsg::Step => { - debug.set_single_step(&self.vcpu_fd, true)?; + debug.set_single_step(&self.vcpu_fd, true).map_err(|e| { + log::error!("Failed to enable step instruction: {:?}", e); + + e + })?; + Ok(DebugResponse::Step) } DebugMsg::WriteAddr(addr, data) => { - debug.write_addrs(&self.vcpu_fd, addr, &data, dbg_mem_access_fn)?; + debug + .write_addrs(&self.vcpu_fd, addr, &data, dbg_mem_access_fn) + .map_err(|e| { + log::error!("Failed to write to address: {:?}", e); + + e + })?; Ok(DebugResponse::WriteAddr) } DebugMsg::WriteRegisters(regs) => debug .write_regs(&self.vcpu_fd, ®s) + .map_err(|e| { + log::error!("Failed to write registers: {:?}", e); + + e + }) .map(|_| DebugResponse::WriteRegisters), } } else { diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 1433349bf..67cc8093c 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -109,19 +109,40 @@ mod debug { if let Some(debug) = self.debug.as_mut() { match req { DebugMsg::AddHwBreakpoint(addr) => Ok(DebugResponse::AddHwBreakpoint( - debug.add_hw_breakpoint(&self.vcpu_fd, addr).is_ok(), + debug + .add_hw_breakpoint(&self.vcpu_fd, addr) + .map_err(|e| { + log::error!("Failed to add hw breakpoint: {:?}", e); + + e + }) + .is_ok(), )), DebugMsg::AddSwBreakpoint(addr) => Ok(DebugResponse::AddSwBreakpoint( debug .add_sw_breakpoint(&self.vcpu_fd, addr, dbg_mem_access_fn) + .map_err(|e| { + log::error!("Failed to add sw breakpoint: {:?}", e); + + e + }) .is_ok(), )), DebugMsg::Continue => { - debug.set_single_step(&self.vcpu_fd, false)?; + debug.set_single_step(&self.vcpu_fd, false).map_err(|e| { + log::error!("Failed to continue execution: {:?}", e); + + e + })?; + Ok(DebugResponse::Continue) } DebugMsg::DisableDebug => { - self.disable_debug()?; + self.disable_debug().map_err(|e| { + log::error!("Failed to disable debugging: {:?}", e); + + e + })?; Ok(DebugResponse::DisableDebug) } @@ -131,14 +152,25 @@ mod debug { .map_err(|e| { new_error!("Error locking at {}:{}: {}", file!(), line!(), e) })? - .get_code_offset()?; + .get_code_offset() + .map_err(|e| { + log::error!("Failed to get code offset: {:?}", e); + + e + })?; Ok(DebugResponse::GetCodeSectionOffset(offset as u64)) } DebugMsg::ReadAddr(addr, len) => { let mut data = vec![0u8; len]; - debug.read_addrs(&self.vcpu_fd, addr, &mut data, dbg_mem_access_fn)?; + debug + .read_addrs(&self.vcpu_fd, addr, &mut data, dbg_mem_access_fn) + .map_err(|e| { + log::error!("Failed to read from address: {:?}", e); + + e + })?; Ok(DebugResponse::ReadAddr(data)) } @@ -147,27 +179,60 @@ mod debug { debug .read_regs(&self.vcpu_fd, &mut regs) + .map_err(|e| { + log::error!("Failed to read registers: {:?}", e); + + e + }) .map(|_| DebugResponse::ReadRegisters(regs)) } DebugMsg::RemoveHwBreakpoint(addr) => Ok(DebugResponse::RemoveHwBreakpoint( - debug.remove_hw_breakpoint(&self.vcpu_fd, addr).is_ok(), + debug + .remove_hw_breakpoint(&self.vcpu_fd, addr) + .map_err(|e| { + log::error!("Failed to remove hw breakpoint: {:?}", e); + + e + }) + .is_ok(), )), DebugMsg::RemoveSwBreakpoint(addr) => Ok(DebugResponse::RemoveSwBreakpoint( debug .remove_sw_breakpoint(&self.vcpu_fd, addr, dbg_mem_access_fn) + .map_err(|e| { + log::error!("Failed to remove sw breakpoint: {:?}", e); + + e + }) .is_ok(), )), DebugMsg::Step => { - debug.set_single_step(&self.vcpu_fd, true)?; + debug.set_single_step(&self.vcpu_fd, true).map_err(|e| { + log::error!("Failed to enable step instruction: {:?}", e); + + e + })?; + Ok(DebugResponse::Step) } DebugMsg::WriteAddr(addr, data) => { - debug.write_addrs(&self.vcpu_fd, addr, &data, dbg_mem_access_fn)?; + debug + .write_addrs(&self.vcpu_fd, addr, &data, dbg_mem_access_fn) + .map_err(|e| { + log::error!("Failed to write to address: {:?}", e); + + e + })?; Ok(DebugResponse::WriteAddr) } DebugMsg::WriteRegisters(regs) => debug .write_regs(&self.vcpu_fd, ®s) + .map_err(|e| { + log::error!("Failed to write registers: {:?}", e); + + e + }) .map(|_| DebugResponse::WriteRegisters), } } else {