Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement RegId for Mips and MSP430 #38

Merged
merged 7 commits into from
Jan 9, 2021
Merged

Implement RegId for Mips and MSP430 #38

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9c515d4
Implement `RegId` for MIPS
starfleetcadet75 Jan 2, 2021
58f0605
Implement `RegId` for MSP430
starfleetcadet75 Jan 2, 2021
3945ae9
Implement `RegId` for PPC
starfleetcadet75 Jan 3, 2021
eb4d827
Fix clippy warnings
starfleetcadet75 Jan 4, 2021
085b779
Requested changes for Mips/Msp430
starfleetcadet75 Jan 9, 2021
bd37618
use non-std version of TryInto
starfleetcadet75 Jan 9, 2021
427b906
Small changes for Mips
starfleetcadet75 Jan 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 6 additions & 19 deletions src/arch/mips/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,42 +1,29 @@
//! Implementations for the MIPS architecture.

use crate::arch::Arch;
use crate::arch::RegId;

pub mod reg;

/// Implements `Arch` for 32-bit MIPS.
///
/// Check out the [module level docs](../index.html#whats-with-regidimpl) for
/// more info about the `RegIdImpl` type parameter.
pub enum Mips<RegIdImpl: RegId> {
#[doc(hidden)]
_Marker(core::marker::PhantomData<RegIdImpl>),
}
pub enum Mips {}

/// Implements `Arch` for 64-bit MIPS.
///
/// Check out the [module level docs](../index.html#whats-with-regidimpl) for
/// more info about the `RegIdImpl` type parameter.
starfleetcadet75 marked this conversation as resolved.
Show resolved Hide resolved
pub enum Mips64<RegIdImpl: RegId> {
#[doc(hidden)]
_Marker(core::marker::PhantomData<RegIdImpl>),
}
pub enum Mips64 {}
daniel5151 marked this conversation as resolved.
Show resolved Hide resolved

impl<RegIdImpl: RegId> Arch for Mips<RegIdImpl> {
impl Arch for Mips {
type Usize = u32;
type Registers = reg::MipsCoreRegs<u32>;
type RegId = RegIdImpl;
type RegId = reg::id::MipsRegId;

fn target_description_xml() -> Option<&'static str> {
Some(r#"<target version="1.0"><architecture>mips</architecture></target>"#)
}
}

impl<RegIdImpl: RegId> Arch for Mips64<RegIdImpl> {
impl Arch for Mips64 {
type Usize = u64;
type Registers = reg::MipsCoreRegs<u64>;
type RegId = RegIdImpl;
type RegId = reg::id::Mips64RegId;

fn target_description_xml() -> Option<&'static str> {
Some(r#"<target version="1.0"><architecture>mips64</architecture></target>"#)
Expand Down
179 changes: 177 additions & 2 deletions src/arch/mips/reg/id.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,177 @@
// TODO: Add proper `RegId` implementation. See [issue #29](https://github.com/daniel5151/gdbstub/issues/29)
// pub enum MipsRegId {}
use crate::arch::RegId;

/// 32-bit MIPS register identifier.
#[derive(Debug, Clone, Copy)]
#[non_exhaustive]
pub enum MipsRegId {
/// 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,
}

/// 64-bit MIPS register identifier.
#[derive(Debug, Clone, Copy)]
#[non_exhaustive]
pub enum Mips64RegId {
daniel5151 marked this conversation as resolved.
Show resolved Hide resolved
/// 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,
}

impl RegId for MipsRegId {
fn from_raw_id(id: usize) -> Option<(Self, usize)> {
let reg = match id {
0..=31 => Self::Gpr(id as u8),
32 => Self::Status,
33 => Self::Lo,
34 => Self::Hi,
35 => Self::Badvaddr,
36 => Self::Cause,
37 => Self::Pc,
38..=69 => Self::Fpr((id as u8) - 38),
70 => Self::Fcsr,
71 => Self::Fir,
72 => Self::Hi1,
73 => Self::Lo1,
74 => Self::Hi2,
75 => Self::Lo2,
76 => Self::Hi3,
77 => Self::Lo3,
78 => Self::Dspctl,
79 => Self::Restart,
_ => return None,
};
Some((reg, 4))
}
}

impl RegId for Mips64RegId {
fn from_raw_id(id: usize) -> Option<(Self, usize)> {
let reg = match id {
0..=31 => Self::Gpr(id as u8),
32 => Self::Status,
33 => Self::Lo,
34 => Self::Hi,
35 => Self::Badvaddr,
36 => Self::Cause,
37 => Self::Pc,
38..=69 => Self::Fpr((id as u8) - 38),
70 => Self::Fcsr,
71 => Self::Fir,
72 => Self::Hi1,
73 => Self::Lo1,
74 => Self::Hi2,
75 => Self::Lo2,
76 => Self::Hi3,
77 => Self::Lo3,
78 => Self::Dspctl,
79 => Self::Restart,
_ => return None,
};
Some((reg, 8))
}
}

#[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::MipsCoreRegs<u32>, crate::arch::mips::reg::id::MipsRegId>()
}

#[test]
fn test_mips64() {
test::<crate::arch::mips::reg::MipsCoreRegs<u64>, crate::arch::mips::reg::id::Mips64RegId>()
}
}
45 changes: 45 additions & 0 deletions src/arch/mips/reg/mips.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ pub struct MipsCoreRegs<U> {
pub cp0: MipsCp0Regs<U>,
/// FPU registers
pub fpu: MipsFpuRegs<U>,
/// DSP registers
pub dsp: MipsDspRegs<U>,
Copy link
Owner

@daniel5151 daniel5151 Jan 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, darn, adding this field will be a semver breaking change :'(

Unfortunately, this is a pretty glaring problem I've run into before, and is tracked under #12. In this case, since the DPS registers are optional, there really aught to be some way to specifying whether or not the DSP registers should be used + toggling the corresponding target.xml feature when active (see https://sourceware.org/gdb/current/onlinedocs/gdb/MIPS-Features.html#MIPS-Features)

At some point I really should sit down and hammer out a proper fix for #12, though I don't know when that'll be exactly. As such, the following workaround will have to suffice:

You'll need to create a new regs struct + associated Arch impls:

struct MipsCoreRegsWithDps<U> {
    core: MipsCoreRegs<U>, 
    dsp: MipsDspRegs<U>
}

enum MipsWithDsp {}

impl Arch for MipsWithDsp {
    type Usize = u32;
    type Registers = reg::MipsCoreRegsWithDps<u32>;
    type RegId = RegIdImpl;
    type RegId = reg::id::MipsRegId;

    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>"#)
    }
}

// similarly for Mips64WithDsp

Feel free to pick a different/better suffix than WithDps.

So yeah, not pretty, but that's just how the code is structured right now unfortunately ☹️

That said, these changes would not require a semver breaking change, as they would only add new types without adding any old ones. As such, I'd be able to publish gdbstub 0.4.3 with these definitions included, which I'm sure you'd like 😄

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(leaving this comment unresolved so that I can easily find it in the future, if I ever need to reference it again)

}

/// MIPS CP0 (coprocessor 0) registers.
Expand Down Expand Up @@ -50,6 +52,29 @@ pub struct MipsFpuRegs<U> {
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)
pub dspctl: U,
/// Restart register (regnum 79)
pub restart: U,
}

impl<U> Registers for MipsCoreRegs<U>
where
U: PrimInt + LeBytes + Default + core::fmt::Debug,
Expand Down Expand Up @@ -94,6 +119,16 @@ where
// Write FCSR and FIR registers
write_le_bytes!(&self.fpu.fcsr);
write_le_bytes!(&self.fpu.fir);

// Write 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);
write_le_bytes!(&self.dsp.dspctl);
write_le_bytes!(&self.dsp.restart);
}

fn gdb_deserialize(&mut self, bytes: &[u8]) -> Result<(), ()> {
Expand Down Expand Up @@ -136,6 +171,16 @@ where
self.fpu.fcsr = regs.next().ok_or(())?;
self.fpu.fir = regs.next().ok_or(())?;

// Read DSP registers
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(())?;
self.dsp.dspctl = regs.next().ok_or(())?;
self.dsp.restart = regs.next().ok_or(())?;

if regs.next().is_some() {
return Err(());
}
Expand Down
2 changes: 0 additions & 2 deletions src/arch/mips/reg/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,3 @@ pub mod id;
mod mips;

pub use mips::MipsCoreRegs;
pub use mips::MipsCp0Regs;
pub use mips::MipsFpuRegs;
starfleetcadet75 marked this conversation as resolved.
Show resolved Hide resolved
13 changes: 3 additions & 10 deletions src/arch/msp430/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,16 @@
//! Implementations for the TI-MSP430 family of MCUs.

use crate::arch::Arch;
use crate::arch::RegId;

pub mod reg;

/// Implements `Arch` for standard 16-bit TI-MSP430 MCUs.
///
/// Check out the [module level docs](../index.html#whats-with-regidimpl) for
/// more info about the `RegIdImpl` type parameter.
pub enum Msp430<RegIdImpl: RegId> {
#[doc(hidden)]
_Marker(core::marker::PhantomData<RegIdImpl>),
}
pub enum Msp430 {}

impl<RegIdImpl: RegId> Arch for Msp430<RegIdImpl> {
daniel5151 marked this conversation as resolved.
Show resolved Hide resolved
impl Arch for Msp430 {
type Usize = u32;
type Registers = reg::Msp430Regs;
type RegId = RegIdImpl;
type RegId = reg::id::Msp430RegId;

fn target_description_xml() -> Option<&'static str> {
Some(r#"<target version="1.0"><architecture>msp430</architecture></target>"#)
Expand Down
Loading