vmm: Enable GDB HW Watchpoint

This commit setup the skeleton of HW watchpoint support.

Signed-off-by: Michael Zhao <michael.zhao@arm.com>

hypervisor/Cargo.toml

@@ -15,6 +15,7 @@ anyhow = "1.0.58"
 byteorder = "1.4.3"
 thiserror = "1.0.31"
 libc = "0.2.126"
+gdbstub = "0.6.2"
 log = "0.4.17"
 kvm-ioctls = { git = "https://github.com/rust-vmm/kvm-ioctls", branch = "main", optional = true }
 kvm-bindings = { git = "https://github.com/cloud-hypervisor/kvm-bindings", branch = "ch-v0.5.0-tdx", features = ["with-serde", "fam-wrappers"], optional  = true }

hypervisor/src/cpu.rs

@@ -18,8 +18,8 @@ use crate::arch::x86::{
 use crate::kvm::{TdxExitDetails, TdxExitStatus};
 use crate::CpuState;
 use crate::MpState;
+use gdbstub::target::ext::breakpoints::WatchKind;
 use thiserror::Error;
-use vm_memory::GuestAddress;
 
 #[derive(Error, Debug)]
 ///
@@ -349,7 +349,12 @@ pub trait Vcpu: Send + Sync {
     ///
     /// Sets debug registers to set hardware breakpoints and/or enable single step.
     ///
-    fn set_guest_debug(&self, _addrs: &[GuestAddress], _singlestep: bool) -> Result<()> {
+    fn set_guest_debug(
+        &self,
+        _breakpoints: &[vm_memory::GuestAddress],
+        _watchpoints: &[(vm_memory::GuestAddress, u64, WatchKind)],
+        _singlestep: bool,
+    ) -> Result<()> {
         Err(HypervisorCpuError::SetDebugRegs(anyhow!("unimplemented")))
     }
     ///

hypervisor/src/kvm/mod.rs

@@ -24,6 +24,7 @@ use crate::vm::{self, InterruptSourceConfig, VmOps};
 use crate::HypervisorType;
 #[cfg(target_arch = "aarch64")]
 use crate::{arm64_core_reg_id, offset__of};
+use gdbstub::target::ext::breakpoints::WatchKind;
 use kvm_ioctls::{NoDatamatch, VcpuFd, VmFd};
 use std::any::Any;
 use std::collections::HashMap;
@@ -1588,13 +1589,14 @@ impl cpu::Vcpu for KvmVcpu {
     ///
     fn set_guest_debug(
         &self,
-        addrs: &[vm_memory::GuestAddress],
+        breakpoints: &[vm_memory::GuestAddress],
+        _watchpoints: &[(vm_memory::GuestAddress, u64, WatchKind)],
         singlestep: bool,
     ) -> cpu::Result<()> {
-        if addrs.len() > 4 {
+        if breakpoints.len() > 4 {
             return Err(cpu::HypervisorCpuError::SetDebugRegs(anyhow!(
                 "Support 4 breakpoints at most but {} addresses are passed",
-                addrs.len()
+                breakpoints.len()
             )));
         }
 
@@ -1609,22 +1611,23 @@ impl cpu::Vcpu for KvmVcpu {
             dbg.control |= KVM_GUESTDBG_SINGLESTEP;
         }
 
+        // Breakpoints
         #[cfg(target_arch = "x86_64")]
         {
             // Set bits 9 and 10.
             // bit 9: GE (global exact breakpoint enable) flag.
             // bit 10: always 1.
             dbg.arch.debugreg[7] = 0x0600;
 
-            for (i, addr) in addrs.iter().enumerate() {
+            for (i, addr) in breakpoints.iter().enumerate() {
                 dbg.arch.debugreg[i] = addr.0;
                 // Set global breakpoint enable flag
                 dbg.arch.debugreg[7] |= 2 << (i * 2);
             }
         }
         #[cfg(target_arch = "aarch64")]
         {
-            for (i, addr) in addrs.iter().enumerate() {
+            for (i, addr) in breakpoints.iter().enumerate() {
                 // DBGBCR_EL1 (Debug Breakpoint Control Registers, D13.3.2):
                 // bit 0: 1 (Enabled)
                 // bit 1~2: 0b11 (PMC = EL1/EL0)
@@ -1636,6 +1639,8 @@ impl cpu::Vcpu for KvmVcpu {
                 dbg.arch.dbg_bvr[i] = (!0u64 >> 11) & addr.0;
             }
         }
+
+        // Watchpoints
         self.fd
             .set_guest_debug(&dbg)
             .map_err(|e| cpu::HypervisorCpuError::SetDebugRegs(e.into()))

vmm/src/cpu.rs

@@ -34,6 +34,8 @@ use arch::aarch64::regs;
 use arch::EntryPoint;
 use arch::NumaNodes;
 use devices::interrupt_controller::InterruptController;
+#[cfg(feature = "gdb")]
+use gdbstub::target::ext::breakpoints::WatchKind;
 #[cfg(all(target_arch = "aarch64", feature = "gdb"))]
 use gdbstub_arch::arm::reg::Aarch64CoreRegs as CoreRegs;
 #[cfg(all(target_arch = "x86_64", feature = "gdb"))]
@@ -2096,14 +2098,15 @@ impl Debuggable for CpuManager {
     fn set_guest_debug(
         &self,
         cpu_id: usize,
-        addrs: &[GuestAddress],
+        breakpoints: &[GuestAddress],
+        watchpoints: &[(GuestAddress, u64, WatchKind)],
         singlestep: bool,
     ) -> std::result::Result<(), DebuggableError> {
         self.vcpus[cpu_id]
             .lock()
             .unwrap()
             .vcpu
-            .set_guest_debug(addrs, singlestep)
+            .set_guest_debug(breakpoints, watchpoints, singlestep)
             .map_err(DebuggableError::SetDebug)
     }
 

vmm/src/gdb.rs

@@ -19,7 +19,10 @@ use gdbstub::{
                 },
                 BaseOps,
             },
-            breakpoints::{Breakpoints, BreakpointsOps, HwBreakpoint, HwBreakpointOps},
+            breakpoints::{
+                Breakpoints, BreakpointsOps, HwBreakpoint, HwBreakpointOps, HwWatchpoint,
+                HwWatchpointOps, WatchKind,
+            },
         },
         Target, TargetError, TargetResult,
     },
@@ -55,7 +58,8 @@ pub trait Debuggable: vm_migration::Pausable {
     fn set_guest_debug(
         &self,
         cpu_id: usize,
-        addrs: &[GuestAddress],
+        breakpoints: &[GuestAddress],
+        watchpoints: &[(GuestAddress, u64, WatchKind)],
         singlestep: bool,
     ) -> Result<(), DebuggableError>;
     fn debug_pause(&mut self) -> std::result::Result<(), DebuggableError>;
@@ -107,7 +111,8 @@ pub enum GdbRequestPayload {
     Pause,
     Resume,
     SetSingleStep(bool),
-    SetHwBreakPoint(Vec<GuestAddress>),
+    SetHwBreakPoint(Vec<GuestAddress>, Vec<(GuestAddress, u64, WatchKind)>),
+    SetHwWatchPoint(Vec<GuestAddress>, Vec<(GuestAddress, u64, WatchKind)>),
     ActiveVcpus,
 }
 
@@ -126,6 +131,7 @@ pub struct GdbStub {
     gdb_event: vmm_sys_util::eventfd::EventFd,
     vm_event: vmm_sys_util::eventfd::EventFd,
     hw_breakpoints: Vec<GuestAddress>,
+    hw_watchpoints: Vec<(GuestAddress, u64, WatchKind)>,
     single_step: bool,
 }
 
@@ -140,6 +146,7 @@ impl GdbStub {
             gdb_event,
             vm_event,
             hw_breakpoints: Default::default(),
+            hw_watchpoints: Default::default(),
             single_step: false,
         }
     }
@@ -376,6 +383,11 @@ impl Breakpoints for GdbStub {
     fn support_hw_breakpoint(&mut self) -> Option<HwBreakpointOps<Self>> {
         Some(self)
     }
+
+    #[inline(always)]
+    fn support_hw_watchpoint(&mut self) -> Option<HwWatchpointOps<Self>> {
+        Some(self)
+    }
 }
 
 impl HwBreakpoint for GdbStub {
@@ -392,7 +404,10 @@ impl HwBreakpoint for GdbStub {
 
         self.hw_breakpoints.push(GuestAddress(addr));
 
-        let payload = GdbRequestPayload::SetHwBreakPoint(self.hw_breakpoints.clone());
+        let payload = GdbRequestPayload::SetHwBreakPoint(
+            self.hw_breakpoints.clone(),
+            self.hw_watchpoints.clone(),
+        );
         match self.vm_request(payload, 0) {
             Ok(_) => Ok(true),
             Err(e) => {
@@ -411,7 +426,10 @@ impl HwBreakpoint for GdbStub {
             Some(pos) => self.hw_breakpoints.remove(pos),
         };
 
-        let payload = GdbRequestPayload::SetHwBreakPoint(self.hw_breakpoints.clone());
+        let payload = GdbRequestPayload::SetHwBreakPoint(
+            self.hw_breakpoints.clone(),
+            self.hw_watchpoints.clone(),
+        );
         match self.vm_request(payload, 0) {
             Ok(_) => Ok(true),
             Err(e) => {
@@ -422,6 +440,61 @@ impl HwBreakpoint for GdbStub {
     }
 }
 
+impl HwWatchpoint for GdbStub {
+    fn add_hw_watchpoint(
+        &mut self,
+        addr: <Self::Arch as Arch>::Usize,
+        len: <Self::Arch as Arch>::Usize,
+        kind: WatchKind,
+    ) -> TargetResult<bool, Self> {
+        // FIXME: The max number of watchpoints is to be confirmed:
+        // - Do WP and BP share the same limit?
+        // - Do WP and BP have limit of each own?
+        if self.hw_watchpoints.len() >= 4 {
+            error!("Not allowed to set more than 4 HW watchpoints");
+            return Ok(false);
+        }
+
+        self.hw_watchpoints.push((GuestAddress(addr), len, kind));
+
+        let payload = GdbRequestPayload::SetHwWatchPoint(
+            self.hw_breakpoints.clone(),
+            self.hw_watchpoints.clone(),
+        );
+        match self.vm_request(payload, 0) {
+            Ok(_) => Ok(true),
+            Err(e) => {
+                error!("Failed to request SetHwWatchPoint: {:?}", e);
+                Err(TargetError::NonFatal)
+            }
+        }
+    }
+
+    fn remove_hw_watchpoint(
+        &mut self,
+        addr: <Self::Arch as Arch>::Usize,
+        _len: <Self::Arch as Arch>::Usize,
+        _kind: WatchKind,
+    ) -> TargetResult<bool, Self> {
+        match self.hw_watchpoints.iter().position(|&w| w.0 .0 == addr) {
+            None => return Ok(false),
+            Some(pos) => self.hw_watchpoints.remove(pos),
+        };
+
+        let payload = GdbRequestPayload::SetHwWatchPoint(
+            self.hw_breakpoints.clone(),
+            self.hw_watchpoints.clone(),
+        );
+        match self.vm_request(payload, 0) {
+            Ok(_) => Ok(true),
+            Err(e) => {
+                error!("Failed to request SetHwWatchPoint: {:?}", e);
+                Err(TargetError::NonFatal)
+            }
+        }
+    }
+}
+
 enum GdbEventLoop {}
 
 impl run_blocking::BlockingEventLoop for GdbEventLoop {
@@ -519,9 +592,10 @@ pub fn gdb_thread(mut gdbstub: GdbStub, path: &std::path::Path) {
                     error!("Failed to disable single step: {:?}", e);
                 }
 
-                if let Err(e) =
-                    gdbstub.vm_request(GdbRequestPayload::SetHwBreakPoint(Vec::new()), 0)
-                {
+                if let Err(e) = gdbstub.vm_request(
+                    GdbRequestPayload::SetHwBreakPoint(Vec::new(), Vec::new()),
+                    0,
+                ) {
                     error!("Failed to remove breakpoints: {:?}", e);
                 }
 

vmm/src/vm.rs

@@ -51,6 +51,8 @@ use devices::gic::GIC_V3_ITS_SNAPSHOT_ID;
 #[cfg(target_arch = "aarch64")]
 use devices::interrupt_controller::{self, InterruptController};
 use devices::AcpiNotificationFlags;
+#[cfg(feature = "gdb")]
+use gdbstub::target::ext::breakpoints::WatchKind;
 #[cfg(all(target_arch = "aarch64", feature = "gdb"))]
 use gdbstub_arch::arm::reg::Aarch64CoreRegs as CoreRegs;
 #[cfg(all(target_arch = "x86_64", feature = "gdb"))]
@@ -2463,13 +2465,18 @@ impl Vm {
         use GdbRequestPayload::*;
         match gdb_request {
             SetSingleStep(single_step) => {
-                self.set_guest_debug(cpu_id, &[], *single_step)
+                self.set_guest_debug(cpu_id, &[], &[], *single_step)
+                    .map_err(Error::Debug)?;
+            }
+            SetHwBreakPoint(breakpoints, watchpoints) => {
+                self.set_guest_debug(cpu_id, breakpoints, watchpoints, false)
                     .map_err(Error::Debug)?;
             }
-            SetHwBreakPoint(addrs) => {
-                self.set_guest_debug(cpu_id, addrs, false)
+            SetHwWatchPoint(breakpoints, watchpoints) => {
+                self.set_guest_debug(cpu_id, breakpoints, watchpoints, false)
                     .map_err(Error::Debug)?;
             }
+
             Pause => {
                 self.debug_pause().map_err(Error::Debug)?;
             }
@@ -2875,13 +2882,16 @@ impl Debuggable for Vm {
     fn set_guest_debug(
         &self,
         cpu_id: usize,
-        addrs: &[GuestAddress],
+        breakpoints: &[GuestAddress],
+        watchpoints: &[(GuestAddress, u64, WatchKind)],
         singlestep: bool,
     ) -> std::result::Result<(), DebuggableError> {
-        self.cpu_manager
-            .lock()
-            .unwrap()
-            .set_guest_debug(cpu_id, addrs, singlestep)
+        self.cpu_manager.lock().unwrap().set_guest_debug(
+            cpu_id,
+            breakpoints,
+            watchpoints,
+            singlestep,
+        )
     }
 
     fn debug_pause(&mut self) -> std::result::Result<(), DebuggableError> {