Moves GDB MultiThreadBase implementation out of arch-specific (#1066)

.vscode/launch.json

@@ -27,6 +27,11 @@
       "targetCreateCommands": [
         "target create ${workspaceFolder}/target/x86_64-unknown-none/debug/obkrnl"
       ],
+      "osx": {
+        "targetCreateCommands": [
+          "target create ${workspaceFolder}/target/aarch64-unknown-none-softfloat/debug/obkrnl"
+        ]
+      },
       "processCreateCommands": [
         "gdb-remote 1234"
       ]

gui/src/vmm/cpu/aarch64.rs

@@ -1,21 +1,40 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
-use super::CpuManager;
+use super::{CpuManager, GdbError};
 use crate::screen::Screen;
 use crate::vmm::hv::Hypervisor;
 use gdbstub::target::ext::base::BaseOps;
-use thiserror::Error;
+use gdbstub::target::ext::breakpoints::{
+    Breakpoints, BreakpointsOps, SwBreakpoint, SwBreakpointOps,
+};
+use gdbstub::target::TargetResult;
 
 pub type GdbRegs = gdbstub_arch::aarch64::reg::AArch64CoreRegs;
 
 impl<H: Hypervisor, S: Screen> gdbstub::target::Target for CpuManager<H, S> {
     type Arch = gdbstub_arch::aarch64::AArch64;
-    type Error = TargetError;
+    type Error = GdbError;
 
     fn base_ops(&mut self) -> BaseOps<'_, Self::Arch, Self::Error> {
-        todo!()
+        BaseOps::MultiThread(self)
+    }
+
+    fn support_breakpoints(&mut self) -> Option<BreakpointsOps<'_, Self>> {
+        Some(self)
     }
 }
 
-/// Implementation of [`gdbstub::target::Target::Error`] for AArch64.
-#[derive(Debug, Error)]
-pub enum TargetError {}
+impl<H: Hypervisor, S: Screen> Breakpoints for CpuManager<H, S> {
+    fn support_sw_breakpoint(&mut self) -> Option<SwBreakpointOps<'_, Self>> {
+        Some(self)
+    }
+}
+
+impl<H: Hypervisor, S: Screen> SwBreakpoint for CpuManager<H, S> {
+    fn add_sw_breakpoint(&mut self, addr: u64, kind: usize) -> TargetResult<bool, Self> {
+        todo!()
+    }
+
+    fn remove_sw_breakpoint(&mut self, addr: u64, kind: usize) -> TargetResult<bool, Self> {
+        todo!()
+    }
+}

gui/src/vmm/cpu/mod.rs

@@ -9,13 +9,18 @@ use super::ram::RamMap;
 use super::{VmmEvent, VmmEventHandler};
 use crate::error::RustError;
 use crate::screen::Screen;
+use gdbstub::common::Tid;
 use gdbstub::stub::MultiThreadStopReason;
+use gdbstub::target::ext::base::multithread::MultiThreadBase;
+use gdbstub::target::ext::thread_extra_info::{ThreadExtraInfo, ThreadExtraInfoOps};
+use gdbstub::target::{TargetError, TargetResult};
 use std::collections::BTreeMap;
 use std::num::NonZero;
 use std::ops::Deref;
 use std::ptr::null_mut;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{Arc, Mutex};
+use thiserror::Error;
 
 #[cfg_attr(target_arch = "aarch64", path = "aarch64.rs")]
 #[cfg_attr(target_arch = "x86_64", path = "x86_64.rs")]
@@ -35,6 +40,9 @@ pub struct CpuManager<H: Hypervisor, S: Screen> {
 }
 
 impl<H: Hypervisor, S: Screen> CpuManager<H, S> {
+    const GDB_ENOENT: u8 = 2;
+    const GDB_EFAULT: u8 = 14;
+
     pub fn new(
         hv: Arc<H>,
         screen: Arc<S::Buffer>,
@@ -416,6 +424,90 @@ impl<H: Hypervisor, S: Screen> CpuManager<H, S> {
     }
 }
 
+impl<H: Hypervisor, S: Screen> MultiThreadBase for CpuManager<H, S> {
+    fn read_registers(&mut self, regs: &mut GdbRegs, tid: Tid) -> TargetResult<(), Self> {
+        let cpu = self
+            .cpus
+            .get_mut(tid.get() - 1)
+            .ok_or(TargetError::Errno(Self::GDB_ENOENT))?;
+
+        *regs = cpu
+            .debug_mut()
+            .unwrap()
+            .get_regs()
+            .ok_or(TargetError::Errno(Self::GDB_ENOENT))?; // The CPU thread just stopped.
+
+        Ok(())
+    }
+
+    fn write_registers(&mut self, regs: &GdbRegs, tid: Tid) -> TargetResult<(), Self> {
+        todo!()
+    }
+
+    fn read_addrs(
+        &mut self,
+        start_addr: u64,
+        data: &mut [u8],
+        tid: Tid,
+    ) -> TargetResult<usize, Self> {
+        let Some(len) = NonZero::new(data.len()) else {
+            return Ok(0);
+        };
+
+        // Translate virtual address to physical address.
+        let cpu = self
+            .cpus
+            .get_mut(tid.get() - 1)
+            .ok_or(TargetError::Errno(Self::GDB_ENOENT))?;
+        let addr = cpu
+            .debug_mut()
+            .unwrap()
+            .translate_address(start_addr.try_into().unwrap())
+            .ok_or(TargetError::Errno(Self::GDB_ENOENT))?;
+
+        // Get data.
+        let src = self
+            .hv
+            .ram()
+            .lock(addr, len)
+            .ok_or(TargetError::Errno(Self::GDB_EFAULT))?;
+
+        data.copy_from_slice(unsafe { std::slice::from_raw_parts(src.as_ptr(), src.len().get()) });
+
+        Ok(len.get())
+    }
+
+    fn write_addrs(&mut self, start_addr: u64, data: &[u8], tid: Tid) -> TargetResult<(), Self> {
+        todo!()
+    }
+
+    fn is_thread_alive(&mut self, tid: Tid) -> Result<bool, Self::Error> {
+        todo!()
+    }
+
+    fn list_active_threads(
+        &mut self,
+        thread_is_active: &mut dyn FnMut(Tid),
+    ) -> Result<(), Self::Error> {
+        for id in (0..self.cpus.len()).map(|v| NonZero::new(v + 1).unwrap()) {
+            thread_is_active(id);
+        }
+
+        Ok(())
+    }
+
+    #[inline(always)]
+    fn support_thread_extra_info(&mut self) -> Option<ThreadExtraInfoOps<'_, Self>> {
+        Some(self)
+    }
+}
+
+impl<H: Hypervisor, S: Screen> ThreadExtraInfo for CpuManager<H, S> {
+    fn thread_extra_info(&self, tid: Tid, buf: &mut [u8]) -> Result<usize, Self::Error> {
+        todo!()
+    }
+}
+
 /// Encapsulates arguments for a function to run a CPU.
 struct Args<H: Hypervisor, S: Screen> {
     hv: Arc<H>,
@@ -449,3 +541,7 @@ impl<'a, C: Cpu> Device<'a, C> {
         assert!(tree.insert(addr, dev).is_none());
     }
 }
+
+/// Implementation of [`gdbstub::target::Target::Error`].
+#[derive(Debug, Error)]
+pub enum GdbError {}

gui/src/vmm/cpu/x86_64.rs

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 use super::controller::CpuController;
-use super::CpuManager;
+use super::{CpuManager, GdbError};
 use crate::screen::Screen;
 use crate::vmm::hv::Hypervisor;
 use gdbstub::common::Tid;
@@ -14,24 +14,12 @@ use gdbstub::target::{TargetError as GdbTargetError, TargetResult};
 use gdbstub_arch::x86::reg::X86_64CoreRegs;
 use gdbstub_arch::x86::X86_64_SSE;
 use std::num::NonZero;
-use thiserror::Error;
-
-const ENOENT: u8 = 2;
-const EFAULT: u8 = 14;
 
 pub type GdbRegs = gdbstub_arch::x86::reg::X86_64CoreRegs;
 
-impl<H: Hypervisor, S: Screen> CpuManager<H, S> {
-    fn get_cpu(&mut self, tid: Tid) -> TargetResult<&mut CpuController, Self> {
-        self.cpus
-            .get_mut(tid.get() - 1)
-            .ok_or(GdbTargetError::Errno(ENOENT))
-    }
-}
-
 impl<H: Hypervisor, S: Screen> gdbstub::target::Target for CpuManager<H, S> {
     type Arch = X86_64_SSE;
-    type Error = TargetError;
+    type Error = GdbError;
 
     fn base_ops(&mut self) -> BaseOps<'_, Self::Arch, Self::Error> {
         BaseOps::MultiThread(self)
@@ -42,85 +30,6 @@ impl<H: Hypervisor, S: Screen> gdbstub::target::Target for CpuManager<H, S> {
     }
 }
 
-impl<H: Hypervisor, S: Screen> MultiThreadBase for CpuManager<H, S> {
-    fn read_registers(&mut self, regs: &mut X86_64CoreRegs, tid: Tid) -> TargetResult<(), Self> {
-        let cpu = self.get_cpu(tid)?;
-
-        *regs = cpu
-            .debug_mut()
-            .unwrap()
-            .get_regs()
-            .ok_or(GdbTargetError::Errno(ENOENT))?; // The CPU thread just stopped.
-
-        Ok(())
-    }
-
-    fn write_registers(&mut self, regs: &X86_64CoreRegs, tid: Tid) -> TargetResult<(), Self> {
-        let mut _cpu = self.get_cpu(tid)?;
-
-        todo!()
-    }
-
-    fn read_addrs(
-        &mut self,
-        start_addr: u64,
-        data: &mut [u8],
-        tid: Tid,
-    ) -> TargetResult<usize, Self> {
-        let Some(len) = NonZero::new(data.len()) else {
-            return Ok(0);
-        };
-
-        let cpu = self.get_cpu(tid)?;
-
-        let translated = cpu
-            .debug_mut()
-            .unwrap()
-            .translate_address(start_addr.try_into().unwrap())
-            .ok_or(GdbTargetError::Errno(ENOENT))?;
-
-        drop(cpu);
-
-        let ram = self.hv.ram();
-
-        let locked_addr = ram
-            .lock(translated, len)
-            .ok_or(GdbTargetError::Errno(EFAULT))?;
-
-        data.copy_from_slice(unsafe {
-            std::slice::from_raw_parts(locked_addr.as_ptr(), len.get())
-        });
-
-        Ok(len.get())
-    }
-
-    fn write_addrs(&mut self, start_addr: u64, data: &[u8], tid: Tid) -> TargetResult<(), Self> {
-        let mut _cpu = self.get_cpu(tid)?;
-
-        todo!()
-    }
-
-    fn is_thread_alive(&mut self, tid: Tid) -> Result<bool, Self::Error> {
-        todo!()
-    }
-
-    fn list_active_threads(
-        &mut self,
-        thread_is_active: &mut dyn FnMut(Tid),
-    ) -> Result<(), Self::Error> {
-        for id in (0..self.cpus.len()).map(|v| unsafe { NonZero::new_unchecked(v + 1) }) {
-            thread_is_active(id);
-        }
-
-        Ok(())
-    }
-
-    #[inline(always)]
-    fn support_thread_extra_info(&mut self) -> Option<ThreadExtraInfoOps<'_, Self>> {
-        Some(self)
-    }
-}
-
 impl<H: Hypervisor, S: Screen> Breakpoints for CpuManager<H, S> {
     fn support_sw_breakpoint(&mut self) -> Option<SwBreakpointOps<'_, Self>> {
         Some(self)
@@ -136,13 +45,3 @@ impl<H: Hypervisor, S: Screen> SwBreakpoint for CpuManager<H, S> {
         todo!()
     }
 }
-
-impl<H: Hypervisor, S: Screen> ThreadExtraInfo for CpuManager<H, S> {
-    fn thread_extra_info(&self, tid: Tid, buf: &mut [u8]) -> Result<usize, Self::Error> {
-        todo!()
-    }
-}
-
-/// Implementation of [`gdbstub::target::Target::Error`] for x86-64.
-#[derive(Debug, Error)]
-pub enum TargetError {}

gui/src/vmm/ram/mod.rs

@@ -173,6 +173,7 @@ impl Ram {
         Some(LockedAddr {
             lock: allocated,
             ptr: unsafe { self.mem.add(addr) },
+            len,
         })
     }
 
@@ -268,6 +269,7 @@ pub struct LockedAddr<'a> {
     #[allow(dead_code)]
     lock: MutexGuard<'a, BTreeSet<usize>>,
     ptr: *mut u8,
+    len: NonZero<usize>,
 }
 
 impl<'a> LockedAddr<'a> {
@@ -277,6 +279,10 @@ impl<'a> LockedAddr<'a> {
     pub fn as_ptr(&self) -> *const u8 {
         self.ptr
     }
+
+    pub fn len(&self) -> NonZero<usize> {
+        self.len
+    }
 }
 
 /// Represents an error when an operation on [`Ram`] fails.