Implement RegId for Mips and MSP430 (#38)

* Implement `RegId` for MIPS

* Implement `RegId` for MSP430

* Fix clippy warnings

* Requested changes for Mips/Msp430

* use non-std version of TryInto

* Small changes for Mips

src/arch/mips/mod.rs

@@ -9,7 +9,7 @@ pub mod reg;
 ///
 /// Check out the [module level docs](../index.html#whats-with-regidimpl) for
 /// more info about the `RegIdImpl` type parameter.
-pub enum Mips<RegIdImpl: RegId> {
+pub enum Mips<RegIdImpl: RegId = reg::id::MipsRegId<u32>> {
     #[doc(hidden)]
     _Marker(core::marker::PhantomData<RegIdImpl>),
 }
@@ -18,11 +18,17 @@ pub enum Mips<RegIdImpl: RegId> {
 ///
 /// Check out the [module level docs](../index.html#whats-with-regidimpl) for
 /// more info about the `RegIdImpl` type parameter.
-pub enum Mips64<RegIdImpl: RegId> {
+pub enum Mips64<RegIdImpl: RegId = reg::id::MipsRegId<u64>> {
     #[doc(hidden)]
     _Marker(core::marker::PhantomData<RegIdImpl>),
 }
 
+/// Implements `Arch` for 32-bit MIPS with the DSP feature enabled.
+pub enum MipsWithDsp {}
+
+/// Implements `Arch` for 64-bit MIPS with the DSP feature enabled.
+pub enum Mips64WithDsp {}
+
 impl<RegIdImpl: RegId> Arch for Mips<RegIdImpl> {
     type Usize = u32;
     type Registers = reg::MipsCoreRegs<u32>;
@@ -42,3 +48,27 @@ impl<RegIdImpl: RegId> Arch for Mips64<RegIdImpl> {
         Some(r#"<target version="1.0"><architecture>mips64</architecture></target>"#)
     }
 }
+
+impl Arch for MipsWithDsp {
+    type Usize = u32;
+    type Registers = reg::MipsCoreRegsWithDsp<u32>;
+    type RegId = reg::id::MipsRegId<u32>;
+
+    fn target_description_xml() -> Option<&'static str> {
+        Some(
+            r#"<target version="1.0"><architecture>mips</architecture><feature name="org.gnu.gdb.mips.dsp"></feature></target>"#,
+        )
+    }
+}
+
+impl Arch for Mips64WithDsp {
+    type Usize = u64;
+    type Registers = reg::MipsCoreRegsWithDsp<u64>;
+    type RegId = reg::id::MipsRegId<u64>;
+
+    fn target_description_xml() -> Option<&'static str> {
+        Some(
+            r#"<target version="1.0"><architecture>mips64</architecture><feature name="org.gnu.gdb.mips.dsp"></feature></target>"#,
+        )
+    }
+}

src/arch/mips/reg/id.rs

@@ -1,2 +1,129 @@
-// TODO: Add proper `RegId` implementation. See [issue #29](https://github.com/daniel5151/gdbstub/issues/29)
-// pub enum MipsRegId {}
+use crate::arch::RegId;
+
+/// MIPS register identifier.
+#[derive(Debug, Clone, Copy)]
+#[non_exhaustive]
+pub enum MipsRegId<U> {
+    /// General purpose registers (R0-R31)
+    Gpr(u8),
+    /// Status register
+    Status,
+    /// Low register
+    Lo,
+    /// High register
+    Hi,
+    /// Bad Virtual Address register
+    Badvaddr,
+    /// Exception Cause register
+    Cause,
+    /// Program Counter
+    Pc,
+    /// Floating point registers (F0-F31)
+    Fpr(u8),
+    /// Floating-point Control Status register
+    Fcsr,
+    /// Floating-point Implementation Register
+    Fir,
+    /// High 1 register
+    Hi1,
+    /// Low 1 register
+    Lo1,
+    /// High 2 register
+    Hi2,
+    /// Low 2 register
+    Lo2,
+    /// High 3 register
+    Hi3,
+    /// Low 3 register
+    Lo3,
+    /// DSP Control register
+    Dspctl,
+    /// Restart register
+    Restart,
+    #[doc(hidden)]
+    _Size(U),
+}
+
+fn from_raw_id<U>(id: usize) -> Option<(MipsRegId<U>, usize)> {
+    let reg = match id {
+        0..=31 => MipsRegId::Gpr(id as u8),
+        32 => MipsRegId::Status,
+        33 => MipsRegId::Lo,
+        34 => MipsRegId::Hi,
+        35 => MipsRegId::Badvaddr,
+        36 => MipsRegId::Cause,
+        37 => MipsRegId::Pc,
+        38..=69 => MipsRegId::Fpr((id as u8) - 38),
+        70 => MipsRegId::Fcsr,
+        71 => MipsRegId::Fir,
+        72 => MipsRegId::Hi1,
+        73 => MipsRegId::Lo1,
+        74 => MipsRegId::Hi2,
+        75 => MipsRegId::Lo2,
+        76 => MipsRegId::Hi3,
+        77 => MipsRegId::Lo3,
+        // `MipsRegId::Dspctl` is the only register that will always be 4 bytes wide
+        78 => return Some((MipsRegId::Dspctl, 4)),
+        79 => MipsRegId::Restart,
+        _ => return None,
+    };
+
+    let ptrsize = core::mem::size_of::<U>();
+    Some((reg, ptrsize))
+}
+
+impl RegId for MipsRegId<u32> {
+    fn from_raw_id(id: usize) -> Option<(Self, usize)> {
+        from_raw_id::<u32>(id)
+    }
+}
+
+impl RegId for MipsRegId<u64> {
+    fn from_raw_id(id: usize) -> Option<(Self, usize)> {
+        from_raw_id::<u64>(id)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::arch::traits::RegId;
+    use crate::arch::traits::Registers;
+
+    fn test<Rs: Registers, RId: RegId>() {
+        // Obtain the data length written by `gdb_serialize` by passing a custom
+        // closure.
+        let mut serialized_data_len = 0;
+        let counter = |b: Option<u8>| {
+            if b.is_some() {
+                serialized_data_len += 1;
+            }
+        };
+        Rs::default().gdb_serialize(counter);
+
+        // Accumulate register sizes returned by `from_raw_id`.
+        let mut i = 0;
+        let mut sum_reg_sizes = 0;
+        while let Some((_, size)) = RId::from_raw_id(i) {
+            sum_reg_sizes += size;
+            i += 1;
+        }
+
+        assert_eq!(serialized_data_len, sum_reg_sizes);
+    }
+
+    #[test]
+    fn test_mips32() {
+        test::<
+            crate::arch::mips::reg::MipsCoreRegsWithDsp<u32>,
+            crate::arch::mips::reg::id::MipsRegId<u32>,
+        >()
+    }
+
+    #[test]
+    fn test_mips64() {
+        test::<
+            crate::arch::mips::reg::MipsCoreRegsWithDsp<u64>,
+            crate::arch::mips::reg::id::MipsRegId<u64>,
+        >()
+    }
+}

src/arch/mips/reg/mips.rs

@@ -1,16 +1,18 @@
-use crate::arch::Registers;
-use crate::internal::LeBytes;
+use core::convert::TryInto;
 
 use num_traits::PrimInt;
 
+use crate::arch::Registers;
+use crate::internal::LeBytes;
+
 /// MIPS registers.
 ///
 /// The register width is set to `u32` or `u64` based on the `<U>` type.
 ///
 /// Source: https://github.com/bminor/binutils-gdb/blob/master/gdb/features/mips-cpu.xml
 #[derive(Debug, Default, Clone, Eq, PartialEq)]
 pub struct MipsCoreRegs<U> {
-    /// General purpose registers (R0-R32)
+    /// General purpose registers (R0-R31)
     pub r: [U; 32],
     /// Low register (regnum 33)
     pub lo: U,
@@ -42,14 +44,50 @@ pub struct MipsCp0Regs<U> {
 /// Source: https://github.com/bminor/binutils-gdb/blob/master/gdb/features/mips-fpu.xml
 #[derive(Debug, Default, Clone, Eq, PartialEq)]
 pub struct MipsFpuRegs<U> {
-    /// FP registers (F0-F32) starting at regnum 38
+    /// FP registers (F0-F31) starting at regnum 38
     pub r: [U; 32],
     /// Floating-point Control Status register
     pub fcsr: U,
     /// Floating-point Implementation Register
     pub fir: U,
 }
 
+/// MIPS DSP registers.
+///
+/// Source: https://github.com/bminor/binutils-gdb/blob/master/gdb/features/mips-dsp.xml
+#[derive(Debug, Default, Clone, Eq, PartialEq)]
+pub struct MipsDspRegs<U> {
+    /// High 1 register (regnum 72)
+    pub hi1: U,
+    /// Low 1 register (regnum 73)
+    pub lo1: U,
+    /// High 2 register (regnum 74)
+    pub hi2: U,
+    /// Low 2 register (regnum 75)
+    pub lo2: U,
+    /// High 3 register (regnum 76)
+    pub hi3: U,
+    /// Low 3 register (regnum 77)
+    pub lo3: U,
+    /// DSP Control register (regnum 78)
+    /// Note: This register will always be 32-bit regardless of the target
+    /// https://sourceware.org/gdb/current/onlinedocs/gdb/MIPS-Features.html#MIPS-Features
+    pub dspctl: u32,
+    /// Restart register (regnum 79)
+    pub restart: U,
+}
+
+/// MIPS core and DSP registers.
+///
+/// Source: https://github.com/bminor/binutils-gdb/blob/master/gdb/features/mips-dsp-linux.xml
+#[derive(Debug, Default, Clone, Eq, PartialEq)]
+pub struct MipsCoreRegsWithDsp<U> {
+    /// Core registers
+    pub core: MipsCoreRegs<U>,
+    /// DSP registers
+    pub dsp: MipsDspRegs<U>,
+}
+
 impl<U> Registers for MipsCoreRegs<U>
 where
     U: PrimInt + LeBytes + Default + core::fmt::Debug,
@@ -99,11 +137,12 @@ where
     fn gdb_deserialize(&mut self, bytes: &[u8]) -> Result<(), ()> {
         let ptrsize = core::mem::size_of::<U>();
 
-        // ensure bytes.chunks_exact(ptrsize) won't panic
-        if bytes.len() % ptrsize != 0 {
+        // Ensure bytes contains enough data for all 72 registers
+        if bytes.len() < ptrsize * 72 {
             return Err(());
         }
 
+        // All core registers are the same size
         let mut regs = bytes
             .chunks_exact(ptrsize)
             .map(|c| U::from_le_bytes(c).unwrap());
@@ -136,10 +175,85 @@ where
         self.fpu.fcsr = regs.next().ok_or(())?;
         self.fpu.fir = regs.next().ok_or(())?;
 
-        if regs.next().is_some() {
+        Ok(())
+    }
+}
+
+impl<U> Registers for MipsCoreRegsWithDsp<U>
+where
+    U: PrimInt + LeBytes + Default + core::fmt::Debug,
+{
+    fn gdb_serialize(&self, mut write_byte: impl FnMut(Option<u8>)) {
+        macro_rules! write_le_bytes {
+            ($value:expr) => {
+                let mut buf = [0; 16];
+                // infallible (unless digit is a >128 bit number)
+                let len = $value.to_le_bytes(&mut buf).unwrap();
+                let buf = &buf[..len];
+                for b in buf {
+                    write_byte(Some(*b));
+                }
+            };
+        }
+
+        // Serialize the core registers first
+        self.core.gdb_serialize(&mut write_byte);
+
+        // Write the DSP registers
+        write_le_bytes!(&self.dsp.hi1);
+        write_le_bytes!(&self.dsp.lo1);
+        write_le_bytes!(&self.dsp.hi2);
+        write_le_bytes!(&self.dsp.lo2);
+        write_le_bytes!(&self.dsp.hi3);
+        write_le_bytes!(&self.dsp.lo3);
+
+        for b in &self.dsp.dspctl.to_le_bytes() {
+            write_byte(Some(*b));
+        }
+
+        write_le_bytes!(&self.dsp.restart);
+    }
+
+    fn gdb_deserialize(&mut self, bytes: &[u8]) -> Result<(), ()> {
+        // Deserialize the core registers first
+        self.core.gdb_deserialize(bytes)?;
+
+        // Ensure bytes contains enough data for all 79 registers of target-width
+        // and the dspctl register which is always 4 bytes
+        let ptrsize = core::mem::size_of::<U>();
+        if bytes.len() < (ptrsize * 79) + 4 {
             return Err(());
         }
 
+        // Calculate the offsets to the DSP registers based on the ptrsize
+        let dspregs_start = ptrsize * 72;
+        let dspctl_start = ptrsize * 78;
+
+        // Read up until the dspctl register
+        let mut regs = bytes[dspregs_start..dspctl_start]
+            .chunks_exact(ptrsize)
+            .map(|c| U::from_le_bytes(c).unwrap());
+
+        self.dsp.hi1 = regs.next().ok_or(())?;
+        self.dsp.lo1 = regs.next().ok_or(())?;
+        self.dsp.hi2 = regs.next().ok_or(())?;
+        self.dsp.lo2 = regs.next().ok_or(())?;
+        self.dsp.hi3 = regs.next().ok_or(())?;
+        self.dsp.lo3 = regs.next().ok_or(())?;
+
+        // Dspctl will always be a u32
+        self.dsp.dspctl =
+            u32::from_le_bytes(bytes[dspctl_start..dspctl_start + 4].try_into().unwrap());
+
+        // Only 4 or 8 bytes should remain to be read
+        self.dsp.restart = U::from_le_bytes(
+            bytes[dspctl_start + 4..]
+                .chunks_exact(ptrsize)
+                .next()
+                .ok_or(())?,
+        )
+        .unwrap();
+
         Ok(())
     }
 }

src/arch/mips/reg/mod.rs

@@ -6,5 +6,6 @@ pub mod id;
 mod mips;
 
 pub use mips::MipsCoreRegs;
+pub use mips::MipsCoreRegsWithDsp;
 pub use mips::MipsCp0Regs;
 pub use mips::MipsFpuRegs;

src/arch/msp430/mod.rs

@@ -9,7 +9,7 @@ pub mod reg;
 ///
 /// Check out the [module level docs](../index.html#whats-with-regidimpl) for
 /// more info about the `RegIdImpl` type parameter.
-pub enum Msp430<RegIdImpl: RegId> {
+pub enum Msp430<RegIdImpl: RegId = reg::id::Msp430RegId> {
     #[doc(hidden)]
     _Marker(core::marker::PhantomData<RegIdImpl>),
 }