From 9e263ccb1be765735771ba9df53cc945d8e7c7f6 Mon Sep 17 00:00:00 2001 From: Tyler Holmes Date: Sat, 1 Jan 2022 16:01:11 -0800 Subject: [PATCH 01/10] Differentiate comparator 0 as the only one capable of cycle compare Statically enforces that only comparator 0 on `armv7m` can be configured for cycle comparison by introducing a marker trait differentiating comparator 0 and the rest of them, and only implementing the ability for this configuration on comparator 0. Closes #376 --- src/peripheral/dwt.rs | 163 +++++++++++++++++++++++++++-------------- src/peripheral/test.rs | 21 ++++-- 2 files changed, 122 insertions(+), 62 deletions(-) diff --git a/src/peripheral/dwt.rs b/src/peripheral/dwt.rs index 72575d37..e01829fd 100644 --- a/src/peripheral/dwt.rs +++ b/src/peripheral/dwt.rs @@ -37,10 +37,13 @@ pub struct RegisterBlock { pub pcsr: RO, /// Comparators #[cfg(armv6m)] - pub c: [Comparator; 2], + pub comp: [Comparator; 2], + #[cfg(not(armv6m))] + /// Cycle count compare enabled Comparator + pub comp0: Comparator, #[cfg(not(armv6m))] /// Comparators - pub c: [Comparator; 16], + pub comp: [Comparator; 15], #[cfg(not(armv6m))] reserved: [u32; 932], /// Lock Access @@ -66,9 +69,31 @@ bitfield! { u8, numcomp, _: 31, 28; } +mod private { + /// A public trait inaccessible by external users to ensure no one else can + /// impl a sealed trait outside of the crate of origin. For more info on this + /// design pattern, see https://rust-lang.github.io/api-guidelines/future-proofing.html + pub trait Sealed {} +} + +/// A zero-sized marker trait indicating the capabilities of a given comparator. +pub trait ComparatorSupportedFunctions: private::Sealed {} + +/// Marker indicating that this comparator has cycle comparison abilities. This +/// is the case only for the first comparator for armv7m. +pub enum HasCycleCompare {} +impl ComparatorSupportedFunctions for HasCycleCompare {} +impl private::Sealed for HasCycleCompare {} + +/// Marker indicating this comparator does not have cycle comparison abilities. This +/// is the case for all armv6m comparators and comparators 1-15 for armv7m. +pub enum NoCycleCompare {} +impl ComparatorSupportedFunctions for NoCycleCompare {} +impl private::Sealed for NoCycleCompare {} + /// Comparator #[repr(C)] -pub struct Comparator { +pub struct Comparator { /// Comparator pub comp: RW, /// Comparator Mask @@ -76,6 +101,7 @@ pub struct Comparator { /// Comparator Function pub function: RW, reserved: u32, + _supported_functions: core::marker::PhantomData, } bitfield! { @@ -414,63 +440,88 @@ pub enum ComparatorFunction { pub enum DwtError { /// Invalid combination of [AccessType] and [EmitOption]. InvalidFunction, + /// An unsupported function was requested, such as [`CycleCount`](ComparatorFunction::CycleCount) on + /// `armv6m`, or on a comparator other than 0 on `armv7m`. + UnsupportedFunction, } -impl Comparator { - /// Configure the function of the comparator +impl Comparator { + /// Private function for configuring address compare on any [`Comparator`] since they all support this. + /// Utilized publicly through [`Comparator::configure`] + fn configure_address_compare( + &self, + settings: ComparatorAddressSettings, + ) -> Result<(), DwtError> { + // FUNCTION, EMITRANGE + // See Table C1-14 + let (function, emit_range) = match (&settings.access_type, &settings.emit) { + (AccessType::ReadOnly, EmitOption::Data) => (0b1100, false), + (AccessType::ReadOnly, EmitOption::Address) => (0b1100, true), + (AccessType::ReadOnly, EmitOption::AddressData) => (0b1110, true), + (AccessType::ReadOnly, EmitOption::PCData) => (0b1110, false), + (AccessType::ReadOnly, EmitOption::WatchpointDebugEvent) => (0b0101, false), + (AccessType::ReadOnly, EmitOption::CompareMatchEvent) => (0b1001, false), + + (AccessType::WriteOnly, EmitOption::Data) => (0b1101, false), + (AccessType::WriteOnly, EmitOption::Address) => (0b1101, true), + (AccessType::WriteOnly, EmitOption::AddressData) => (0b1111, true), + (AccessType::WriteOnly, EmitOption::PCData) => (0b1111, false), + (AccessType::WriteOnly, EmitOption::WatchpointDebugEvent) => (0b0110, false), + (AccessType::WriteOnly, EmitOption::CompareMatchEvent) => (0b1010, false), + + (AccessType::ReadWrite, EmitOption::Data) => (0b0010, false), + (AccessType::ReadWrite, EmitOption::Address) => (0b0001, true), + (AccessType::ReadWrite, EmitOption::AddressData) => (0b0010, true), + (AccessType::ReadWrite, EmitOption::PCData) => (0b0011, false), + (AccessType::ReadWrite, EmitOption::WatchpointDebugEvent) => (0b0111, false), + (AccessType::ReadWrite, EmitOption::CompareMatchEvent) => (0b1011, false), + + (AccessType::ReadWrite, EmitOption::PC) => (0b0001, false), + (_, EmitOption::PC) => return Err(DwtError::InvalidFunction), + }; + + unsafe { + self.function.modify(|mut r| { + r.set_function(function); + r.set_emitrange(emit_range); + // don't compare data value + r.set_datavmatch(false); + // don't compare cycle counter value + // NOTE: only needed for comparator 0, but is SBZP. + r.set_cycmatch(false); + // SBZ as needed, see Page 784/C1-724 + r.set_datavsize(0); + r.set_datavaddr0(0); + r.set_datavaddr1(0); + + r + }); + + self.comp.write(settings.address); + self.mask.write(settings.mask); + } + + Ok(()) + } +} + +impl Comparator { + /// Configure the function of the [`Comparator`]. Does not support cycle count comparison. #[allow(clippy::missing_inline_in_public_items)] pub fn configure(&self, settings: ComparatorFunction) -> Result<(), DwtError> { match settings { - ComparatorFunction::Address(settings) => { - // FUNCTION, EMITRANGE - // See Table C1-14 - let (function, emit_range) = match (&settings.access_type, &settings.emit) { - (AccessType::ReadOnly, EmitOption::Data) => (0b1100, false), - (AccessType::ReadOnly, EmitOption::Address) => (0b1100, true), - (AccessType::ReadOnly, EmitOption::AddressData) => (0b1110, true), - (AccessType::ReadOnly, EmitOption::PCData) => (0b1110, false), - (AccessType::ReadOnly, EmitOption::WatchpointDebugEvent) => (0b0101, false), - (AccessType::ReadOnly, EmitOption::CompareMatchEvent) => (0b1001, false), - - (AccessType::WriteOnly, EmitOption::Data) => (0b1101, false), - (AccessType::WriteOnly, EmitOption::Address) => (0b1101, true), - (AccessType::WriteOnly, EmitOption::AddressData) => (0b1111, true), - (AccessType::WriteOnly, EmitOption::PCData) => (0b1111, false), - (AccessType::WriteOnly, EmitOption::WatchpointDebugEvent) => (0b0110, false), - (AccessType::WriteOnly, EmitOption::CompareMatchEvent) => (0b1010, false), - - (AccessType::ReadWrite, EmitOption::Data) => (0b0010, false), - (AccessType::ReadWrite, EmitOption::Address) => (0b0001, true), - (AccessType::ReadWrite, EmitOption::AddressData) => (0b0010, true), - (AccessType::ReadWrite, EmitOption::PCData) => (0b0011, false), - (AccessType::ReadWrite, EmitOption::WatchpointDebugEvent) => (0b0111, false), - (AccessType::ReadWrite, EmitOption::CompareMatchEvent) => (0b1011, false), - - (AccessType::ReadWrite, EmitOption::PC) => (0b0001, false), - (_, EmitOption::PC) => return Err(DwtError::InvalidFunction), - }; - - unsafe { - self.function.modify(|mut r| { - r.set_function(function); - r.set_emitrange(emit_range); - // don't compare data value - r.set_datavmatch(false); - // don't compare cycle counter value - // NOTE: only needed for comparator 0, but is SBZP. - r.set_cycmatch(false); - // SBZ as needed, see Page 784/C1-724 - r.set_datavsize(0); - r.set_datavaddr0(0); - r.set_datavaddr1(0); - - r - }); + ComparatorFunction::Address(settings) => self.configure_address_compare(settings), + ComparatorFunction::CycleCount(_settings) => Err(DwtError::UnsupportedFunction), + } + } +} - self.comp.write(settings.address); - self.mask.write(settings.mask); - } - } +impl Comparator { + /// Configure the function of the [`Comparator`]. Has support for cycle count comparison. + #[allow(clippy::missing_inline_in_public_items)] + pub fn configure(&self, settings: ComparatorFunction) -> Result<(), DwtError> { + match settings { + ComparatorFunction::Address(settings) => self.configure_address_compare(settings), ComparatorFunction::CycleCount(settings) => { let function = match &settings.emit { EmitOption::PCData => 0b0001, @@ -499,9 +550,9 @@ impl Comparator { self.comp.write(settings.compare); self.mask.write(0); // SBZ, see Page 784/C1-724 } + + Ok(()) } } - - Ok(()) } } diff --git a/src/peripheral/test.rs b/src/peripheral/test.rs index cab064aa..4c19be24 100644 --- a/src/peripheral/test.rs +++ b/src/peripheral/test.rs @@ -42,12 +42,21 @@ fn dwt() { #[cfg(not(armv6m))] assert_eq!(address(&dwt.foldcnt), 0xE000_1018); assert_eq!(address(&dwt.pcsr), 0xE000_101C); - assert_eq!(address(&dwt.c[0].comp), 0xE000_1020); - assert_eq!(address(&dwt.c[0].mask), 0xE000_1024); - assert_eq!(address(&dwt.c[0].function), 0xE000_1028); - assert_eq!(address(&dwt.c[1].comp), 0xE000_1030); - assert_eq!(address(&dwt.c[1].mask), 0xE000_1034); - assert_eq!(address(&dwt.c[1].function), 0xE000_1038); + if cfg!(not(armv6m)) { + assert_eq!(address(&dwt.comp0.comp), 0xE000_1020); + assert_eq!(address(&dwt.comp0.mask), 0xE000_1024); + assert_eq!(address(&dwt.comp0.function), 0xE000_1028); + + assert_eq!(address(&dwt.comp[0].comp), 0xE000_1030); + assert_eq!(address(&dwt.comp[0].mask), 0xE000_1034); + assert_eq!(address(&dwt.comp[0].function), 0xE000_1038); + } + if cfg!(armv6m) { + assert_eq!(address(&dwt.comp[0].comp), 0xE000_1020); + assert_eq!(address(&dwt.comp[0].mask), 0xE000_1024); + assert_eq!(address(&dwt.comp[0].function), 0xE000_1028); + } + #[cfg(not(armv6m))] assert_eq!(address(&dwt.lar), 0xE000_1FB0); #[cfg(not(armv6m))] From 22efd365ffd4121a22364883a4659936a420c917 Mon Sep 17 00:00:00 2001 From: Tyler Holmes Date: Sat, 1 Jan 2022 16:32:23 -0800 Subject: [PATCH 02/10] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebcd2c49..58afc5e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). Also fixes `VectActive::from` to take a `u16` and subtract `16` for `VectActive::Interrupt`s to match `SBC::vect_active()` (#373). - DWT: add `configure` API for address, cycle count comparison (#342, #367). + - Differentiated the first `DWT` `Comparator` as the only one able to do cycle + count comparisons, and only on `armv7m` (#377). + - renamed the field from `c` to `comp0` and `comps[15]` for `armv7m` and + `comps[2]` for `armv6m` (#377). - ITM: add `configure` API (#342). - TPIU: add API for *Formatter and Flush Control* (FFCR) and *Selected Pin Control* (SPPR) registers (#342). - TPIU: add `swo_supports` for checking what SWO configurations the target supports. (#381) From f24999f34d1f603eea95d0e2af07711a082d0f2a Mon Sep 17 00:00:00 2001 From: Tyler Holmes Date: Thu, 6 Jan 2022 13:49:57 -0800 Subject: [PATCH 03/10] move the `has_*` DWT methods to associated functions --- src/peripheral/dwt.rs | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/peripheral/dwt.rs b/src/peripheral/dwt.rs index e01829fd..0cfecd3c 100644 --- a/src/peripheral/dwt.rs +++ b/src/peripheral/dwt.rs @@ -127,29 +127,33 @@ impl DWT { /// /// A value of zero indicates no comparator support. #[inline] - pub fn num_comp(&self) -> u8 { - self.ctrl.read().numcomp() + pub fn num_comp() -> u8 { + // NOTE(unsafe) atomic read with no side effects + unsafe { (*DWT::PTR).ctrl.read().numcomp() } } /// Returns `true` if the the implementation supports sampling and exception tracing #[cfg(not(armv6m))] #[inline] - pub fn has_exception_trace(&self) -> bool { - !self.ctrl.read().notrcpkt() + pub fn has_exception_trace() -> bool { + // NOTE(unsafe) atomic read with no side effects + unsafe { !(*DWT::PTR).ctrl.read().notrcpkt() } } /// Returns `true` if the implementation includes external match signals #[cfg(not(armv6m))] #[inline] - pub fn has_external_match(&self) -> bool { - !self.ctrl.read().noexttrig() + pub fn has_external_match() -> bool { + // NOTE(unsafe) atomic read with no side effects + unsafe { !(*DWT::PTR).ctrl.read().noexttrig() } } /// Returns `true` if the implementation supports a cycle counter #[inline] - pub fn has_cycle_counter(&self) -> bool { + pub fn has_cycle_counter() -> bool { #[cfg(not(armv6m))] - return !self.ctrl.read().nocyccnt(); + // NOTE(unsafe) atomic read with no side effects + return !unsafe { (*DWT::PTR).ctrl.read().nocyccnt() }; #[cfg(armv6m)] return false; @@ -158,8 +162,9 @@ impl DWT { /// Returns `true` if the implementation the profiling counters #[cfg(not(armv6m))] #[inline] - pub fn has_profiling_counter(&self) -> bool { - !self.ctrl.read().noprfcnt() + pub fn has_profiling_counter() -> bool { + // NOTE(unsafe) atomic read with no side effects + !unsafe { (*DWT::PTR).ctrl.read().noprfcnt() } } /// Enables the cycle counter From 916e494632e7268c97a79bf45e6c3d31f29d6775 Mon Sep 17 00:00:00 2001 From: Tyler Holmes Date: Thu, 6 Jan 2022 13:50:33 -0800 Subject: [PATCH 04/10] register field rename as suggested --- src/peripheral/dwt.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/peripheral/dwt.rs b/src/peripheral/dwt.rs index 0cfecd3c..dbc92108 100644 --- a/src/peripheral/dwt.rs +++ b/src/peripheral/dwt.rs @@ -37,13 +37,13 @@ pub struct RegisterBlock { pub pcsr: RO, /// Comparators #[cfg(armv6m)] - pub comp: [Comparator; 2], + pub comps: [Comparator; 2], #[cfg(not(armv6m))] /// Cycle count compare enabled Comparator pub comp0: Comparator, #[cfg(not(armv6m))] /// Comparators - pub comp: [Comparator; 15], + pub comps: [Comparator; 15], #[cfg(not(armv6m))] reserved: [u32; 932], /// Lock Access From ffa638c71c451719e4a25a295199af0bce7ba4e9 Mon Sep 17 00:00:00 2001 From: Tyler Holmes Date: Thu, 6 Jan 2022 13:51:05 -0800 Subject: [PATCH 05/10] check DWT::has_cycle_counter when configuring a comparator for that function & some cleanup --- src/peripheral/dwt.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/peripheral/dwt.rs b/src/peripheral/dwt.rs index dbc92108..9fd36041 100644 --- a/src/peripheral/dwt.rs +++ b/src/peripheral/dwt.rs @@ -452,7 +452,7 @@ pub enum DwtError { impl Comparator { /// Private function for configuring address compare on any [`Comparator`] since they all support this. - /// Utilized publicly through [`Comparator::configure`] + /// Utilized publicly through [`Comparator::configure`]. fn configure_address_compare( &self, settings: ComparatorAddressSettings, @@ -522,12 +522,19 @@ impl Comparator { } impl Comparator { - /// Configure the function of the [`Comparator`]. Has support for cycle count comparison. + /// Configure the function of the [`Comparator`]. Has support for cycle count comparison + /// and checks [`DWT::has_cycle_counter`] for hardware support if + /// [`CycleCount`](ComparatorFunction::CycleCount) is requested. #[allow(clippy::missing_inline_in_public_items)] pub fn configure(&self, settings: ComparatorFunction) -> Result<(), DwtError> { match settings { ComparatorFunction::Address(settings) => self.configure_address_compare(settings), ComparatorFunction::CycleCount(settings) => { + // Check if the HW advertises that it has the cycle counter or not + if !DWT::has_cycle_counter() { + return Err(DwtError::UnsupportedFunction); + } + let function = match &settings.emit { EmitOption::PCData => 0b0001, EmitOption::WatchpointDebugEvent => 0b0100, From 82b19756017edce1fcdadfab5bb5e4280b86ed88 Mon Sep 17 00:00:00 2001 From: Tyler Holmes Date: Thu, 6 Jan 2022 13:57:42 -0800 Subject: [PATCH 06/10] call out the has_ changes in changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58afc5e2..ce7699d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). count comparisons, and only on `armv7m` (#377). - renamed the field from `c` to `comp0` and `comps[15]` for `armv7m` and `comps[2]` for `armv6m` (#377). + - Made the `has_*` implementation checks associated functions (#377). - ITM: add `configure` API (#342). - TPIU: add API for *Formatter and Flush Control* (FFCR) and *Selected Pin Control* (SPPR) registers (#342). - TPIU: add `swo_supports` for checking what SWO configurations the target supports. (#381) From 79111ff3679532992fbf5e41c58bd66c63b8583a Mon Sep 17 00:00:00 2001 From: Tyler Holmes Date: Thu, 6 Jan 2022 14:00:41 -0800 Subject: [PATCH 07/10] fix tests --- src/peripheral/test.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/peripheral/test.rs b/src/peripheral/test.rs index 4c19be24..f2ab4122 100644 --- a/src/peripheral/test.rs +++ b/src/peripheral/test.rs @@ -47,14 +47,14 @@ fn dwt() { assert_eq!(address(&dwt.comp0.mask), 0xE000_1024); assert_eq!(address(&dwt.comp0.function), 0xE000_1028); - assert_eq!(address(&dwt.comp[0].comp), 0xE000_1030); - assert_eq!(address(&dwt.comp[0].mask), 0xE000_1034); - assert_eq!(address(&dwt.comp[0].function), 0xE000_1038); + assert_eq!(address(&dwt.comps[0].comp), 0xE000_1030); + assert_eq!(address(&dwt.comps[0].mask), 0xE000_1034); + assert_eq!(address(&dwt.comps[0].function), 0xE000_1038); } if cfg!(armv6m) { - assert_eq!(address(&dwt.comp[0].comp), 0xE000_1020); - assert_eq!(address(&dwt.comp[0].mask), 0xE000_1024); - assert_eq!(address(&dwt.comp[0].function), 0xE000_1028); + assert_eq!(address(&dwt.comps[0].comp), 0xE000_1020); + assert_eq!(address(&dwt.comps[0].mask), 0xE000_1024); + assert_eq!(address(&dwt.comps[0].function), 0xE000_1028); } #[cfg(not(armv6m))] From 457864d1624eae3e002c8fa78665ea74b0e89e24 Mon Sep 17 00:00:00 2001 From: Tyler Holmes Date: Fri, 7 Jan 2022 11:39:05 -0800 Subject: [PATCH 08/10] review comments --- src/peripheral/dwt.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/peripheral/dwt.rs b/src/peripheral/dwt.rs index 9fd36041..c83d420e 100644 --- a/src/peripheral/dwt.rs +++ b/src/peripheral/dwt.rs @@ -153,7 +153,7 @@ impl DWT { pub fn has_cycle_counter() -> bool { #[cfg(not(armv6m))] // NOTE(unsafe) atomic read with no side effects - return !unsafe { (*DWT::PTR).ctrl.read().nocyccnt() }; + return unsafe { !(*DWT::PTR).ctrl.read().nocyccnt() }; #[cfg(armv6m)] return false; @@ -164,7 +164,7 @@ impl DWT { #[inline] pub fn has_profiling_counter() -> bool { // NOTE(unsafe) atomic read with no side effects - !unsafe { (*DWT::PTR).ctrl.read().noprfcnt() } + unsafe { !(*DWT::PTR).ctrl.read().noprfcnt() } } /// Enables the cycle counter From 49a8438c49a298efa51146cc95b0be8e41d55562 Mon Sep 17 00:00:00 2001 From: Tyler Holmes Date: Sat, 8 Oct 2022 13:30:55 -0700 Subject: [PATCH 09/10] has_cycle_counter is now an associated function --- testsuite/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testsuite/src/main.rs b/testsuite/src/main.rs index 46ab629b..027c162f 100644 --- a/testsuite/src/main.rs +++ b/testsuite/src/main.rs @@ -33,7 +33,7 @@ mod tests { { use cortex_m::peripheral::DWT; - assert!(p.DWT.has_cycle_counter()); + assert!(DWT::has_cycle_counter()); p.DCB.enable_trace(); p.DWT.disable_cycle_counter(); @@ -48,7 +48,7 @@ mod tests { #[cfg(armv6m)] { - assert!(!p.DWT.has_cycle_counter()); + assert!(!DWT::has_cycle_counter()); } } } From ef728e899c3896e8ac45e7d54436211e86b52dd9 Mon Sep 17 00:00:00 2001 From: Tyler Holmes Date: Sat, 8 Oct 2022 13:46:38 -0700 Subject: [PATCH 10/10] fix tests --- testsuite/src/main.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/testsuite/src/main.rs b/testsuite/src/main.rs index 027c162f..12a5770f 100644 --- a/testsuite/src/main.rs +++ b/testsuite/src/main.rs @@ -28,11 +28,12 @@ mod tests { #[test] #[cfg(not(feature = "semihosting"))] // QEMU does not model the cycle counter + #[cfg_attr(armv6m, allow(unused_variables))] fn cycle_count(p: &mut cortex_m::Peripherals) { + use cortex_m::peripheral::DWT; + #[cfg(not(armv6m))] { - use cortex_m::peripheral::DWT; - assert!(DWT::has_cycle_counter()); p.DCB.enable_trace();