Merge #249

249: More v0.6.3 backports r=jonas-schievink a=adamgreig

I think this includes the remainder of the non-breaking changes since v0.6.2, with a couple of exceptions:

* #240 seemed low-impact but had loads of separate commits to cherry-pick
* #220 is I think non-breaking but was quite a substantial change, perhaps I could still include it

~~I did include #226 which adds a new field to `Peripherals`, but as I understand it that should be a non-breaking change since it's non-exhaustive.~~

I've updated the CHANGELOG with all the changes from this PR and the previous #248.

Co-authored-by: Cliff L. Biffle <[email protected]>
Co-authored-by: Peter Taylor <[email protected]>
Co-authored-by: Hugues de Valon <[email protected]>
Co-authored-by: Jonas Schievink <[email protected]>
Co-authored-by: Adam Greig <[email protected]>

Author: 6 people

CHANGELOG.md

@@ -7,6 +7,23 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+## [v0.6.3] - 2020-07-20
+
+### Added
+
+- Initial Cortex-M Security Extension support for armv8m
+- `UDF` intrinsic
+- Methods to enable/disable exceptions in SCB
+
+### Fixed
+
+- Fix bug in `asm::delay` not updating status clobber flags
+- Swapped to `llvm_asm!` to support inline assembly on new nightlies
+- Our precompiled assembly routines have additional debug information
+- ITM `is_fifo_ready` improved to support armv8
+- Cache enabling moved to pre-built assembly routines to prevent possible
+  undefined behaviour
+
 ## [v0.6.2] - 2020-01-12
 
 ### Added
@@ -572,7 +589,8 @@ fn main() {
 - Functions to get the vector table
 - Wrappers over miscellaneous instructions like `bkpt`
 
-[Unreleased]: https://github.com/rust-embedded/cortex-m/compare/v0.6.2...HEAD
+[Unreleased]: https://github.com/rust-embedded/cortex-m/compare/v0.6.3...HEAD
+[v0.6.3]: https://github.com/rust-embedded/cortex-m/compare/v0.6.2...v0.6.3
 [v0.6.2]: https://github.com/rust-embedded/cortex-m/compare/v0.6.1...v0.6.2
 [v0.6.1]: https://github.com/rust-embedded/cortex-m/compare/v0.6.0...v0.6.1
 [v0.6.0]: https://github.com/rust-embedded/cortex-m/compare/v0.5.8...v0.6.0

asm-v7.s

@@ -1,3 +1,4 @@
+  .syntax unified
   .cfi_sections .debug_frame
 
   .section .text.__basepri_max
@@ -39,3 +40,39 @@ __faultmask:
   bx lr
   .cfi_endproc
   .size __faultmask, . - __faultmask
+
+  .section .text.__enable_icache
+  .global __enable_icache
+  .thumb_func
+  .cfi_startproc
+__enable_icache:
+  ldr r0, =0xE000ED14       @ CCR
+  mrs r2, PRIMASK           @ save critical nesting info
+  cpsid i                   @ mask interrupts
+  ldr r1, [r0]              @ read CCR
+  orr.w r1, r1, #(1 << 17)  @ Set bit 17, IC
+  str r1, [r0]              @ write it back
+  dsb                       @ ensure store completes
+  isb                       @ synchronize pipeline
+  msr PRIMASK, r2           @ unnest critical section
+  bx lr
+  .cfi_endproc
+  .size __enable_icache, . - __enable_icache
+
+  .section .text.__enable_dcache
+  .global __enable_dcache
+  .thumb_func
+  .cfi_startproc
+__enable_dcache:
+  ldr r0, =0xE000ED14       @ CCR
+  mrs r2, PRIMASK           @ save critical nesting info
+  cpsid i                   @ mask interrupts
+  ldr r1, [r0]              @ read CCR
+  orr.w r1, r1, #(1 << 16)  @ Set bit 16, DC
+  str r1, [r0]              @ write it back
+  dsb                       @ ensure store completes
+  isb                       @ synchronize pipeline
+  msr PRIMASK, r2           @ unnest critical section
+  bx lr
+  .cfi_endproc
+  .size __enable_dcache, . - __enable_dcache

bin/thumbv6m-none-eabi.a

@@ -6,7 +6,7 @@ fn main() {
     let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap());
     let name = env::var("CARGO_PKG_NAME").unwrap();
 
-    if target.starts_with("thumb") && env::var_os("CARGO_FEATURE_INLINE_ASM").is_none() {
+    if target.starts_with("thumb") {
         fs::copy(
             format!("bin/{}.a", target),
             out_dir.join(format!("lib{}.a", name)),

bin/thumbv7em-none-eabi.a

@@ -53,8 +53,19 @@ impl Stim {
     }
 
     /// Returns `true` if the stimulus port is ready to accept more data
+    #[cfg(not(armv8m))]
     #[inline]
     pub fn is_fifo_ready(&self) -> bool {
-        unsafe { ptr::read_volatile(self.register.get()) == 1 }
+        unsafe { ptr::read_volatile(self.register.get()) & 0b1 == 1 }
+    }
+
+    /// Returns `true` if the stimulus port is ready to accept more data
+    #[cfg(armv8m)]
+    #[inline]
+    pub fn is_fifo_ready(&self) -> bool {
+        // ARMv8-M adds a disabled bit; we indicate that we are ready to
+        // proceed with a stimulus write if the port is either ready (bit 0) or
+        // disabled (bit 1).
+        unsafe { ptr::read_volatile(self.register.get()) & 0b11 != 0 }
     }
 }

bin/thumbv7em-none-eabihf.a

@@ -143,14 +143,17 @@ pub struct Peripherals {
 // NOTE `no_mangle` is used here to prevent linking different minor versions of this crate as that
 // would let you `take` the core peripherals more than once (one per minor version)
 #[no_mangle]
-static mut CORE_PERIPHERALS: bool = false;
+static CORE_PERIPHERALS: () = ();
+
+/// Set to `true` when `take` or `steal` was called to make `Peripherals` a singleton.
+static mut TAKEN: bool = false;
 
 impl Peripherals {
     /// Returns all the core peripherals *once*
     #[inline]
     pub fn take() -> Option<Self> {
         interrupt::free(|_| {
-            if unsafe { CORE_PERIPHERALS } {
+            if unsafe { TAKEN } {
                 None
             } else {
                 Some(unsafe { Peripherals::steal() })
@@ -161,7 +164,7 @@ impl Peripherals {
     /// Unchecked version of `Peripherals::take`
     #[inline]
     pub unsafe fn steal() -> Self {
-        CORE_PERIPHERALS = true;
+        TAKEN = true;
 
         Peripherals {
             CBP: CBP {

bin/thumbv7m-none-eabi.a

@@ -1,8 +1,6 @@
 //! System Control Block
 
 use core::ptr;
-#[cfg(not(any(armv6m, armv8m_base)))]
-use crate::interrupt;
 
 use volatile_register::RW;
 
@@ -330,11 +328,16 @@ impl SCB {
         // Invalidate I-Cache
         cbp.iciallu();
 
-        // Enable I-Cache
-        unsafe { self.ccr.modify(|r| r | SCB_CCR_IC_MASK) };
+        // Enable I-cache
+        extern "C" {
+            // see asm-v7m.s
+            fn __enable_icache();
+        }
 
-        crate::asm::dsb();
-        crate::asm::isb();
+        // NOTE(unsafe): The asm routine manages exclusive access to the SCB
+        // registers and applies the proper barriers; it is technically safe on
+        // its own, and is only `unsafe` here because it's `extern "C"`.
+        unsafe { __enable_icache(); }
     }
 
     /// Disables I-Cache if currently enabled
@@ -392,11 +395,16 @@ impl SCB {
         // Invalidate anything currently in the DCache
         self.invalidate_dcache(cpuid);
 
-        // Now turn on the DCache
-        unsafe { self.ccr.modify(|r| r | SCB_CCR_DC_MASK) };
+        // Now turn on the D-cache
+        extern "C" {
+            // see asm-v7m.s
+            fn __enable_dcache();
+        }
 
-        crate::asm::dsb();
-        crate::asm::isb();
+        // NOTE(unsafe): The asm routine manages exclusive access to the SCB
+        // registers and applies the proper barriers; it is technically safe on
+        // its own, and is only `unsafe` here because it's `extern "C"`.
+        unsafe { __enable_dcache(); }
     }
 
     /// Disables D-cache if currently enabled
@@ -706,7 +714,7 @@ impl SCB {
         }
     }
 
-    /// Set the PENDSTCLR bit in the ICSR register which will clear a pending SysTick interrupt
+    /// Set the PENDSTSET bit in the ICSR register which will pend a SysTick interrupt
     #[inline]
     pub fn set_pendst() {
         unsafe {
@@ -842,6 +850,20 @@ impl SCB {
         }
     }
 
+    /// Return the bit position of the exception enable bit in the SHCSR register
+    #[inline]
+    #[cfg(not(any(armv6m, armv8m_base)))]
+    fn shcsr_enable_shift(exception: Exception) -> Option<u32> {
+        match exception {
+            Exception::MemoryManagement => Some(16),
+            Exception::BusFault => Some(17),
+            Exception::UsageFault => Some(18),
+            #[cfg(armv8m_main)]
+            Exception::SecureFault => Some(19),
+            _ => None,
+        }
+    }
+
     /// Enable the exception
     ///
     /// If the exception is enabled, when the exception is triggered, the exception handler will be executed instead of the
@@ -856,24 +878,11 @@ impl SCB {
     #[inline]
     #[cfg(not(any(armv6m, armv8m_base)))]
     pub fn enable(&mut self, exception: Exception) {
-        if self.is_enabled(exception) {
-            return;
-        }
-
-        // Make sure that the read-modify-write sequence happens during a critical section to avoid
-        // modifying pending and active interrupts.
-        interrupt::free(|_| {
-            let shift = match exception {
-                Exception::MemoryManagement => 16,
-                Exception::BusFault => 17,
-                Exception::UsageFault => 18,
-                #[cfg(armv8m_main)]
-                Exception::SecureFault => 19,
-                _ => return,
-            };
-
+        if let Some(shift) = SCB::shcsr_enable_shift(exception) {
+            // The mutable reference to SCB makes sure that only this code is currently modifying
+            // the register.
             unsafe { self.shcsr.modify(|value| value | (1 << shift)) }
-        })
+        }
     }
 
     /// Disable the exception
@@ -890,24 +899,11 @@ impl SCB {
     #[inline]
     #[cfg(not(any(armv6m, armv8m_base)))]
     pub fn disable(&mut self, exception: Exception) {
-        if !self.is_enabled(exception) {
-            return;
-        }
-
-        // Make sure that the read-modify-write sequence happens during a critical section to avoid
-        // modifying pending and active interrupts.
-        interrupt::free(|_| {
-            let shift = match exception {
-                Exception::MemoryManagement => 16,
-                Exception::BusFault => 17,
-                Exception::UsageFault => 18,
-                #[cfg(armv8m_main)]
-                Exception::SecureFault => 19,
-                _ => return,
-            };
-
+        if let Some(shift) = SCB::shcsr_enable_shift(exception) {
+            // The mutable reference to SCB makes sure that only this code is currently modifying
+            // the register.
             unsafe { self.shcsr.modify(|value| value & !(1 << shift)) }
-        })
+        }
     }
 
     /// Check if an exception is enabled
@@ -921,16 +917,11 @@ impl SCB {
     /// Calling this function with any other exception will read `false`.
     #[inline]
     #[cfg(not(any(armv6m, armv8m_base)))]
-    pub fn is_enabled(&mut self, exception: Exception) -> bool {
-        let shift = match exception {
-            Exception::MemoryManagement => 16,
-            Exception::BusFault => 17,
-            Exception::UsageFault => 18,
-            #[cfg(armv8m_main)]
-            Exception::SecureFault => 19,
-            _ => return false,
-        };
-
-        (self.shcsr.read() & (1 << shift)) > 0
+    pub fn is_enabled(&self, exception: Exception) -> bool {
+        if let Some(shift) = SCB::shcsr_enable_shift(exception) {
+            (self.shcsr.read() & (1 << shift)) > 0
+        } else {
+            false
+        }
     }
 }