gdb: interrupt vcpu using the new InterruptHandle

- Previously, interrupting a running vCPU under debug meant directly sending a SIGINT signal,
since we have a way to handle interrupt sending, there is no need to do that.

Signed-off-by: Doru Blânzeanu <[email protected]>

Author: dblnz

src/hyperlight_host/src/hypervisor/gdb/event_loop.rs

@@ -19,10 +19,12 @@ use gdbstub::conn::ConnectionExt;
 use gdbstub::stub::{
     BaseStopReason, DisconnectReason, GdbStub, SingleThreadStopReason, run_blocking,
 };
-use libc::{SIGRTMIN, pthread_kill};
 
 use super::x86_64_target::HyperlightSandboxTarget;
 use super::{DebugResponse, GdbTargetError, VcpuStopReason};
+mod signals {
+    pub use libc::{SIGINT, SIGSEGV};
+}
 
 struct GdbBlockingEventLoop;
 
@@ -56,11 +58,11 @@ impl run_blocking::BlockingEventLoop for GdbBlockingEventLoop {
                         // to the target thread
                         VcpuStopReason::Interrupt => BaseStopReason::SignalWithThread {
                             tid: (),
-                            signal: Signal(SIGRTMIN() as u8),
+                            signal: Signal(signals::SIGINT as u8),
                         },
                         VcpuStopReason::Crash => BaseStopReason::SignalWithThread {
                             tid: (),
-                            signal: Signal(11),
+                            signal: Signal(signals::SIGSEGV as u8),
                         },
                         VcpuStopReason::Unknown => {
                             log::warn!("Unknown stop reason received");
@@ -105,11 +107,9 @@ impl run_blocking::BlockingEventLoop for GdbBlockingEventLoop {
         log::info!("Received interrupt from GDB client - sending signal to target thread");
 
         // Send a signal to the target thread to interrupt it
-        let ret = unsafe { pthread_kill(target.get_thread_id(), SIGRTMIN()) };
-
-        log::info!("pthread_kill returned {}", ret);
+        let res = target.interrupt_vcpu();
 
-        if ret < 0 && ret != libc::ESRCH {
+        if !res {
             log::error!("Failed to send signal to target thread");
             return Err(GdbTargetError::SendSignalError);
         }

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

@@ -41,6 +41,7 @@ pub(crate) use mshv_debug::MshvDebug;
 use thiserror::Error;
 use x86_64_target::HyperlightSandboxTarget;
 
+use super::InterruptHandle;
 use crate::hypervisor::handlers::DbgMemAccessHandlerCaller;
 use crate::mem::layout::SandboxMemoryLayout;
 use crate::{HyperlightError, new_error};
@@ -147,6 +148,7 @@ pub(crate) enum DebugResponse {
     ErrorOccurred,
     GetCodeSectionOffset(u64),
     NotAllowed,
+    InterruptHandle(Arc<dyn InterruptHandle>),
     ReadAddr(Vec<u8>),
     ReadRegisters(X86_64Regs),
     RemoveHwBreakpoint(bool),
@@ -158,7 +160,7 @@ pub(crate) enum DebugResponse {
 }
 
 /// This trait is used to define common debugging functionality for Hypervisors
-pub(crate) trait GuestDebug {
+pub(super) trait GuestDebug {
     /// Type that wraps the vCPU functionality
     type Vcpu;
 
@@ -380,7 +382,6 @@ impl<T, U> DebugCommChannel<T, U> {
 /// Creates a thread that handles gdb protocol
 pub(crate) fn create_gdb_thread(
     port: u16,
-    thread_id: u64,
 ) -> Result<DebugCommChannel<DebugResponse, DebugMsg>, GdbTargetError> {
     let (gdb_conn, hyp_conn) = DebugCommChannel::unbounded();
     let socket = format!("localhost:{}", port);
@@ -398,12 +399,23 @@ pub(crate) fn create_gdb_thread(
             let conn: Box<dyn ConnectionExt<Error = io::Error>> = Box::new(conn);
             let debugger = GdbStub::new(conn);
 
-            let mut target = HyperlightSandboxTarget::new(hyp_conn, thread_id);
+            let mut target = HyperlightSandboxTarget::new(hyp_conn);
 
             // Waits for vCPU to stop at entrypoint breakpoint
-            let res = target.recv()?;
-            if let DebugResponse::VcpuStopped(_) = res {
+            let msg = target.recv()?;
+            if let DebugResponse::InterruptHandle(handle) = msg {
+                log::info!("Received interrupt handle: {:?}", handle);
+                target.set_interrupt_handle(handle);
+            } else {
+                return Err(GdbTargetError::UnexpectedMessage);
+            }
+
+            // Waits for vCPU to stop at entrypoint breakpoint
+            let msg = target.recv()?;
+            if let DebugResponse::VcpuStopped(_) = msg {
                 event_loop_thread(debugger, &mut target);
+            } else {
+                return Err(GdbTargetError::UnexpectedMessage);
             }
 
             Ok(())

src/hyperlight_host/src/hypervisor/gdb/x86_64_target.rs

@@ -14,6 +14,8 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+use std::sync::Arc;
+
 use crossbeam_channel::TryRecvError;
 use gdbstub::arch::Arch;
 use gdbstub::common::Signal;
@@ -30,20 +32,21 @@ use gdbstub::target::{Target, TargetError, TargetResult};
 use gdbstub_arch::x86::X86_64_SSE as GdbTargetArch;
 
 use super::{DebugCommChannel, DebugMsg, DebugResponse, GdbTargetError, X86_64Regs};
+use crate::hypervisor::InterruptHandle;
 
 /// Gdbstub target used by the gdbstub crate to provide GDB protocol implementation
 pub(crate) struct HyperlightSandboxTarget {
     /// Hypervisor communication channels
     hyp_conn: DebugCommChannel<DebugMsg, DebugResponse>,
-    /// Thread ID
-    thread_id: u64,
+    /// Interrupt handle for the vCPU thread
+    interrupt_handle: Option<Arc<dyn InterruptHandle>>,
 }
 
 impl HyperlightSandboxTarget {
-    pub(crate) fn new(hyp_conn: DebugCommChannel<DebugMsg, DebugResponse>, thread_id: u64) -> Self {
+    pub(crate) fn new(hyp_conn: DebugCommChannel<DebugMsg, DebugResponse>) -> Self {
         HyperlightSandboxTarget {
             hyp_conn,
-            thread_id,
+            interrupt_handle: None,
         }
     }
 
@@ -60,9 +63,9 @@ impl HyperlightSandboxTarget {
         self.hyp_conn.send(ev)
     }
 
-    /// Returns the thread ID
-    pub(crate) fn get_thread_id(&self) -> u64 {
-        self.thread_id
+    /// Set the interrupt handle for the vCPU thread
+    pub(crate) fn set_interrupt_handle(&mut self, handle: Arc<dyn InterruptHandle>) {
+        self.interrupt_handle = Some(handle);
     }
 
     /// Waits for a response over the communication channel
@@ -113,6 +116,17 @@ impl HyperlightSandboxTarget {
             }
         }
     }
+
+    /// Interrupts the vCPU execution
+    pub(crate) fn interrupt_vcpu(&mut self) -> bool {
+        if let Some(handle) = &self.interrupt_handle {
+            handle.kill_from_debugger()
+        } else {
+            log::warn!("No interrupt handle set, cannot interrupt vCPU");
+
+            false
+        }
+    }
 }
 
 impl Target for HyperlightSandboxTarget {
@@ -464,7 +478,7 @@ mod tests {
     fn test_gdb_target() {
         let (gdb_conn, hyp_conn) = DebugCommChannel::unbounded();
 
-        let mut target = HyperlightSandboxTarget::new(hyp_conn, 0);
+        let mut target = HyperlightSandboxTarget::new(hyp_conn);
 
         // Check response to read registers - send the response first to not be blocked
         // by the recv call in the target

src/hyperlight_host/src/hypervisor/hyperv_linux.rs

@@ -397,42 +397,53 @@ impl HypervLinuxDriver {
 
         Self::setup_initial_sregs(&mut vcpu_fd, pml4_ptr.absolute()?)?;
 
-        Ok(Self {
+        let interrupt_handle = Arc::new(LinuxInterruptHandle {
+            running: AtomicU64::new(0),
+            cancel_requested: AtomicBool::new(false),
+            #[cfg(gdb)]
+            debug_interrupt: AtomicBool::new(false),
+            #[cfg(all(
+                target_arch = "x86_64",
+                target_vendor = "unknown",
+                target_os = "linux",
+                target_env = "musl"
+            ))]
+            tid: AtomicU64::new(unsafe { libc::pthread_self() as u64 }),
+            #[cfg(not(all(
+                target_arch = "x86_64",
+                target_vendor = "unknown",
+                target_os = "linux",
+                target_env = "musl"
+            )))]
+            tid: AtomicU64::new(unsafe { libc::pthread_self() }),
+            retry_delay: config.get_interrupt_retry_delay(),
+            sig_rt_min_offset: config.get_interrupt_vcpu_sigrtmin_offset(),
+            dropped: AtomicBool::new(false),
+        });
+
+        #[allow(unused_mut)]
+        let mut hv = Self {
             _mshv: mshv,
             vm_fd,
             vcpu_fd,
             mem_regions,
             entrypoint: entrypoint_ptr.absolute()?,
             orig_rsp: rsp_ptr,
-            interrupt_handle: Arc::new(LinuxInterruptHandle {
-                running: AtomicU64::new(0),
-                cancel_requested: AtomicBool::new(false),
-                #[cfg(all(
-                    target_arch = "x86_64",
-                    target_vendor = "unknown",
-                    target_os = "linux",
-                    target_env = "musl"
-                ))]
-                tid: AtomicU64::new(unsafe { libc::pthread_self() as u64 }),
-                #[cfg(not(all(
-                    target_arch = "x86_64",
-                    target_vendor = "unknown",
-                    target_os = "linux",
-                    target_env = "musl"
-                )))]
-                tid: AtomicU64::new(unsafe { libc::pthread_self() }),
-                retry_delay: config.get_interrupt_retry_delay(),
-                sig_rt_min_offset: config.get_interrupt_vcpu_sigrtmin_offset(),
-                dropped: AtomicBool::new(false),
-            }),
-
+            interrupt_handle: interrupt_handle.clone(),
             #[cfg(gdb)]
             debug,
             #[cfg(gdb)]
             gdb_conn,
             #[cfg(crashdump)]
             rt_cfg,
-        })
+        };
+
+        // Send the interrupt handle to the GDB thread if debugging is enabled
+        // This is used to allow the GDB thread to stop the vCPU
+        #[cfg(gdb)]
+        hv.send_dbg_msg(DebugResponse::InterruptHandle(interrupt_handle))?;
+
+        Ok(hv)
     }
 
     #[instrument(err(Debug), skip_all, parent = Span::current(), level = "Trace")]
@@ -634,6 +645,14 @@ impl Hypervisor for HypervLinuxDriver {
                     e
                 )
             })?;
+        #[cfg(not(gdb))]
+        let debug_interrupt = false;
+        #[cfg(gdb)]
+        let debug_interrupt = self
+            .interrupt_handle
+            .debug_interrupt
+            .load(Ordering::Relaxed);
+
         // Don't run the vcpu if `cancel_requested` is true
         //
         // Note: if a `InterruptHandle::kill()` called while this thread is **here**
@@ -642,6 +661,7 @@ impl Hypervisor for HypervLinuxDriver {
             .interrupt_handle
             .cancel_requested
             .load(Ordering::Relaxed)
+            || debug_interrupt
         {
             Err(MshvError::Errno(vmm_sys_util::errno::Error::new(
                 libc::EINTR,
@@ -667,6 +687,11 @@ impl Hypervisor for HypervLinuxDriver {
             .interrupt_handle
             .cancel_requested
             .load(Ordering::Relaxed);
+        #[cfg(gdb)]
+        let debug_interrupt = self
+            .interrupt_handle
+            .debug_interrupt
+            .load(Ordering::Relaxed);
         // Note: if a `InterruptHandle::kill()` called while this thread is **here**
         // Then `cancel_requested` will be set to true again, which will cancel the **next vcpu run**.
         // Additionally signals will be sent to this thread until `running` is set to false.
@@ -754,27 +779,23 @@ impl Hypervisor for HypervLinuxDriver {
             Err(e) => match e.errno() {
                 // we send a signal to the thread to cancel execution this results in EINTR being returned by KVM so we return Cancelled
                 libc::EINTR => {
-                    // If cancellation was not requested for this specific vm, the vcpu was interrupted because of stale signal
-                    // that was meant to be delivered to a previous/other vcpu on this same thread, so let's ignore it
+                    // If cancellation was not requested for this specific vm, the vcpu was interrupted because of debug interrupt or
+                    // a stale signal that meant to be delivered to a previous/other vcpu on this same thread, so let's ignore it
                     if cancel_requested {
                         self.interrupt_handle
                             .cancel_requested
                             .store(false, Ordering::Relaxed);
                         HyperlightExit::Cancelled()
                     } else {
-                        // In case of the gdb feature, if no cancellation was requested,
-                        // and the debugging is enabled it means the vCPU was stopped because
-                        // of an interrupt coming from the debugger thread
                         #[cfg(gdb)]
-                        if self.debug.is_some() {
+                        if debug_interrupt {
+                            self.interrupt_handle
+                                .debug_interrupt
+                                .store(false, Ordering::Relaxed);
+
                             // If the vCPU was stopped because of an interrupt, we need to
                             // return a special exit reason so that the gdb thread can handle it
                             // and resume execution
-                            // NOTE: There is a chance that the vCPU was stopped because of a stale
-                            // signal that was meant to be delivered to a previous/other vCPU on this
-                            // same thread, however, we cannot distinguish between the two cases, so
-                            // we assume that the vCPU was stopped because of an interrupt.
-                            // This is fine, because the debugger will be notified about an interrupt
                             HyperlightExit::Debug(VcpuStopReason::Interrupt)
                         } else {
                             HyperlightExit::Retry()