From 4b4f825f4fbf3ffca1e7eedc363f4f005de6fed3 Mon Sep 17 00:00:00 2001 From: Adam Greig Date: Wed, 15 Jan 2020 23:33:13 +0000 Subject: [PATCH 1/4] Update and improve cache operations. Closes #47, #188. Breaking change due to changing safety of d-cache invalidation functions. --- src/peripheral/cbp.rs | 46 ++++----- src/peripheral/scb.rs | 226 +++++++++++++++++++++++++++++++++--------- 2 files changed, 200 insertions(+), 72 deletions(-) diff --git a/src/peripheral/cbp.rs b/src/peripheral/cbp.rs index 8d82e2a7..6a1defd9 100644 --- a/src/peripheral/cbp.rs +++ b/src/peripheral/cbp.rs @@ -39,34 +39,28 @@ const CBP_SW_SET_MASK: u32 = 0x1FF << CBP_SW_SET_POS; impl CBP { /// I-cache invalidate all to PoU - #[inline] + #[inline(always)] pub fn iciallu(&mut self) { - unsafe { - self.iciallu.write(0); - } + unsafe { self.iciallu.write(0) }; } /// I-cache invalidate by MVA to PoU - #[inline] + #[inline(always)] pub fn icimvau(&mut self, mva: u32) { - unsafe { - self.icimvau.write(mva); - } + unsafe { self.icimvau.write(mva) }; } /// D-cache invalidate by MVA to PoC - #[inline] - pub fn dcimvac(&mut self, mva: u32) { - unsafe { - self.dcimvac.write(mva); - } + #[inline(always)] + pub unsafe fn dcimvac(&mut self, mva: u32) { + self.dcimvac.write(mva); } /// D-cache invalidate by set-way /// /// `set` is masked to be between 0 and 3, and `way` between 0 and 511. - #[inline] - pub fn dcisw(&mut self, set: u16, way: u16) { + #[inline(always)] + pub unsafe fn dcisw(&mut self, set: u16, way: u16) { // The ARMv7-M Architecture Reference Manual, as of Revision E.b, says these set/way // operations have a register data format which depends on the implementation's // associativity and number of sets. Specifically the 'way' and 'set' fields have @@ -76,16 +70,14 @@ impl CBP { // Generic User Guide section 4.8.3. Since no other ARMv7-M implementations except the // Cortex-M7 have a DCACHE or ICACHE at all, it seems safe to do the same thing as the // CMSIS-Core implementation and use fixed values. - unsafe { - self.dcisw.write( - ((u32::from(way) & (CBP_SW_WAY_MASK >> CBP_SW_WAY_POS)) << CBP_SW_WAY_POS) - | ((u32::from(set) & (CBP_SW_SET_MASK >> CBP_SW_SET_POS)) << CBP_SW_SET_POS), - ); - } + self.dcisw.write( + ((u32::from(way) & (CBP_SW_WAY_MASK >> CBP_SW_WAY_POS)) << CBP_SW_WAY_POS) + | ((u32::from(set) & (CBP_SW_SET_MASK >> CBP_SW_SET_POS)) << CBP_SW_SET_POS), + ); } /// D-cache clean by MVA to PoU - #[inline] + #[inline(always)] pub fn dccmvau(&mut self, mva: u32) { unsafe { self.dccmvau.write(mva); @@ -93,7 +85,7 @@ impl CBP { } /// D-cache clean by MVA to PoC - #[inline] + #[inline(always)] pub fn dccmvac(&mut self, mva: u32) { unsafe { self.dccmvac.write(mva); @@ -103,7 +95,7 @@ impl CBP { /// D-cache clean by set-way /// /// `set` is masked to be between 0 and 3, and `way` between 0 and 511. - #[inline] + #[inline(always)] pub fn dccsw(&mut self, set: u16, way: u16) { // See comment for dcisw() about the format here unsafe { @@ -115,7 +107,7 @@ impl CBP { } /// D-cache clean and invalidate by MVA to PoC - #[inline] + #[inline(always)] pub fn dccimvac(&mut self, mva: u32) { unsafe { self.dccimvac.write(mva); @@ -125,7 +117,7 @@ impl CBP { /// D-cache clean and invalidate by set-way /// /// `set` is masked to be between 0 and 3, and `way` between 0 and 511. - #[inline] + #[inline(always)] pub fn dccisw(&mut self, set: u16, way: u16) { // See comment for dcisw() about the format here unsafe { @@ -137,7 +129,7 @@ impl CBP { } /// Branch predictor invalidate all - #[inline] + #[inline(always)] pub fn bpiall(&mut self) { unsafe { self.bpiall.write(0); diff --git a/src/peripheral/scb.rs b/src/peripheral/scb.rs index 9d58b038..a04d45f2 100644 --- a/src/peripheral/scb.rs +++ b/src/peripheral/scb.rs @@ -314,65 +314,71 @@ use self::scb_consts::*; #[cfg(not(armv6m))] impl SCB { - /// Enables I-Cache if currently disabled + /// Enables I-cache if currently disabled + /// + /// This operation first invalidates the entire I-cache. #[inline] pub fn enable_icache(&mut self) { - // Don't do anything if ICache is already enabled + // Don't do anything if I-cache is already enabled if Self::icache_enabled() { return; } - // NOTE(unsafe) All CBP registers are write-only and stateless + // NOTE(unsafe): No races as all CBP registers are write-only and stateless let mut cbp = unsafe { CBP::new() }; - // Invalidate I-Cache + // Invalidate I-cache cbp.iciallu(); - // Enable I-Cache + // Enable I-cache + // NOTE(unsafe): We have synchronised access by &mut self unsafe { self.ccr.modify(|r| r | SCB_CCR_IC_MASK) }; crate::asm::dsb(); crate::asm::isb(); } - /// Disables I-Cache if currently enabled + /// Disables I-cache if currently enabled + /// + /// This operation invalidates the entire I-cache after disabling. #[inline] pub fn disable_icache(&mut self) { - // Don't do anything if ICache is already disabled + // Don't do anything if I-cache is already disabled if !Self::icache_enabled() { return; } - // NOTE(unsafe) All CBP registers are write-only and stateless + // NOTE(unsafe): No races as all CBP registers are write-only and stateless let mut cbp = unsafe { CBP::new() }; - // Disable I-Cache + // Disable I-cache + // NOTE(unsafe): We have synchronised access by &mut self unsafe { self.ccr.modify(|r| r & !SCB_CCR_IC_MASK) }; - // Invalidate I-Cache + // Invalidate I-cache cbp.iciallu(); crate::asm::dsb(); crate::asm::isb(); } - /// Returns whether the I-Cache is currently enabled - #[inline] + /// Returns whether the I-cache is currently enabled + #[inline(always)] pub fn icache_enabled() -> bool { crate::asm::dsb(); crate::asm::isb(); - // NOTE(unsafe) atomic read with no side effects + // NOTE(unsafe): atomic read with no side effects unsafe { (*Self::ptr()).ccr.read() & SCB_CCR_IC_MASK == SCB_CCR_IC_MASK } } - /// Invalidates I-Cache + /// Invalidates entire I-cache #[inline] pub fn invalidate_icache(&mut self) { - // NOTE(unsafe) All CBP registers are write-only and stateless + // NOTE(unsafe): No races as all CBP registers are write-only and stateless let mut cbp = unsafe { CBP::new() }; - // Invalidate I-Cache + // Invalidate I-cache cbp.iciallu(); crate::asm::dsb(); @@ -380,17 +386,21 @@ impl SCB { } /// Enables D-cache if currently disabled + /// + /// This operation first invalidates the entire D-cache, ensuring it does + /// not contain stale values before being enabled. #[inline] pub fn enable_dcache(&mut self, cpuid: &mut CPUID) { - // Don't do anything if DCache is already enabled + // Don't do anything if D-cache is already enabled if Self::dcache_enabled() { return; } - // Invalidate anything currently in the DCache - self.invalidate_dcache(cpuid); + // Invalidate anything currently in the D-cache + unsafe { self.invalidate_dcache(cpuid) }; - // Now turn on the DCache + // Now turn on the D-cache + // NOTE(unsafe): We have synchronised access by &mut self unsafe { self.ccr.modify(|r| r | SCB_CCR_DC_MASK) }; crate::asm::dsb(); @@ -398,21 +408,25 @@ impl SCB { } /// Disables D-cache if currently enabled + /// + /// This operation subsequently cleans and invalidates the entire D-cache, + /// ensuring all contents are safely written back to main memory after disabling. #[inline] pub fn disable_dcache(&mut self, cpuid: &mut CPUID) { - // Don't do anything if DCache is already disabled + // Don't do anything if D-cache is already disabled if !Self::dcache_enabled() { return; } - // Turn off the DCache + // Turn off the D-cache + // NOTE(unsafe): We have synchronised access by &mut self unsafe { self.ccr.modify(|r| r & !SCB_CCR_DC_MASK) }; // Clean and invalidate whatever was left in it self.clean_invalidate_dcache(cpuid); } - /// Returns whether the D-Cache is currently enabled + /// Returns whether the D-cache is currently enabled #[inline] pub fn dcache_enabled() -> bool { crate::asm::dsb(); @@ -422,20 +436,21 @@ impl SCB { unsafe { (*Self::ptr()).ccr.read() & SCB_CCR_DC_MASK == SCB_CCR_DC_MASK } } - /// Invalidates D-cache + /// Invalidates entire D-cache + /// + /// Note that calling this while the dcache is enabled will probably wipe out the + /// stack, depending on optimisations, therefore breaking returning to the call point. /// - /// Note that calling this while the dcache is enabled will probably wipe out your - /// stack, depending on optimisations, breaking returning to the call point. /// It's used immediately before enabling the dcache, but not exported publicly. #[inline] - fn invalidate_dcache(&mut self, cpuid: &mut CPUID) { - // NOTE(unsafe) All CBP registers are write-only and stateless - let mut cbp = unsafe { CBP::new() }; + unsafe fn invalidate_dcache(&mut self, cpuid: &mut CPUID) { + // NOTE(unsafe): No races as all CBP registers are write-only and stateless + let mut cbp = CBP::new(); // Read number of sets and ways let (sets, ways) = cpuid.cache_num_sets_ways(0, CsselrCacheType::DataOrUnified); - // Invalidate entire D-Cache + // Invalidate entire D-cache for set in 0..sets { for way in 0..ways { cbp.dcisw(set, way); @@ -446,10 +461,13 @@ impl SCB { crate::asm::isb(); } - /// Cleans D-cache + /// Cleans entire D-cache + /// + /// This function causes everything in the D-cache to be written back to main memory, + /// overwriting whatever is already there. #[inline] pub fn clean_dcache(&mut self, cpuid: &mut CPUID) { - // NOTE(unsafe) All CBP registers are write-only and stateless + // NOTE(unsafe): No races as all CBP registers are write-only and stateless let mut cbp = unsafe { CBP::new() }; // Read number of sets and ways @@ -465,10 +483,14 @@ impl SCB { crate::asm::isb(); } - /// Cleans and invalidates D-cache + /// Cleans and invalidates entire D-cache + /// + /// This function causes everything in the D-cache to be written back to main memory, + /// and then marks the entire D-cache as invalid, causing future reads to first fetch + /// from main memory. #[inline] pub fn clean_invalidate_dcache(&mut self, cpuid: &mut CPUID) { - // NOTE(unsafe) All CBP registers are write-only and stateless + // NOTE(unsafe): No races as all CBP registers are write-only and stateless let mut cbp = unsafe { CBP::new() }; // Read number of sets and ways @@ -486,20 +508,35 @@ impl SCB { /// Invalidates D-cache by address /// - /// `addr`: the address to invalidate - /// `size`: size of the memory block, in number of bytes + /// * `addr`: the address to invalidate + /// * `size`: size of the memory block, in number of bytes /// /// Invalidates cache starting from the lowest 32-byte aligned address represented by `addr`, /// in blocks of 32 bytes until at least `size` bytes have been invalidated. + /// + /// Invalidation causes the next read access to memory to be fetched from main memory instead + /// of the cache. + /// + /// # Safety + /// + /// After invalidating, the next read of invalidated data will be from main memory. This may + /// cause recent writes to be lost, potentially including writes that initialised objects. + /// Therefore, this method may cause uninitialised memory or invalid values to be read, + /// resulting in undefined behaviour. You must ensure that main memory contains valid and + /// initialised values before invalidating. + /// + /// If `addr` is not 32-byte aligned or if `size` is not a multiple of 32, other memory + /// will also be invalidated unexpectedly. It is highly recommended that `addr` is 32-byte + /// aligned and `size` is a multiple of 32 bytes. #[inline] - pub fn invalidate_dcache_by_address(&mut self, addr: usize, size: usize) { + pub unsafe fn invalidate_dcache_by_address(&mut self, addr: usize, size: usize) { // No-op zero sized operations if size == 0 { return; } - // NOTE(unsafe) All CBP registers are write-only and stateless - let mut cbp = unsafe { CBP::new() }; + // NOTE(unsafe): No races as all CBP registers are write-only and stateless + let mut cbp = CBP::new(); crate::asm::dsb(); @@ -518,13 +555,72 @@ impl SCB { crate::asm::isb(); } + /// Invalidates an object from the D-cache + /// + /// * `obj`: Object to invalidate + /// + /// Invalidates cache starting from the lowest 32-byte aligned address containing `obj`, + /// in blocks of 32 bytes until all of `obj` has been invalidated. + /// + /// Invalidation causes the next read access to memory to be fetched from main memory instead + /// of the cache. + /// + /// # Safety + /// + /// After invalidating, `obj` will be read from main memory on next access. This may cause + /// recent writes to `obj` to be lost, potentially including the write that initialised it. + /// Therefore, this method may cause uninitialised memory or invalid values to be read, + /// resulting in undefined behaviour. You must ensure that main memory contains a valid and + /// initialised value for T before invalidating `obj`. + /// + /// If `obj` is not both 32-byte aligned and a multiple of 32 bytes long, other memory + /// will also be invalidated unexpectedly. It is highly recommended that `obj` is 32-byte + /// aligned and a multiple of 32 bytes long. + #[inline] + pub unsafe fn invalidate_dcache_by_ref(&mut self, obj: &T) { + self.invalidate_dcache_by_address(obj as *const T as usize, core::mem::size_of::()); + } + + /// Invalidates a slice from the D-cache + /// + /// * `slice`: Slice to invalidate + /// + /// Invalidates cache starting from the lowest 32-byte aligned address containing members of + /// `slice`, in blocks of 32 bytes until all of `slice` has been invalidated. + /// + /// Invalidation causes the next read access to memory to be fetched from main memory instead + /// of the cache. + /// + /// # Safety + /// + /// After invalidating, `slice` will be read from main memory on next access. This may cause + /// recent writes to `slice` to be lost, potentially including the write that initialised it. + /// Therefore, this method may cause uninitialised memory or invalid values to be read, + /// resulting in undefined behaviour. You must ensure that main memory contains valid and + /// initialised values for T before invalidating `slice`. + /// + /// If `slice` is not both 32-byte aligned and a multiple of 32 bytes long, other memory + /// will also be invalidated unexpectedly. It is highly recommended that `slice` is 32-byte + /// aligned and a multiple of 32 bytes long. + #[inline] + pub unsafe fn invalidate_dcache_by_slice(&mut self, slice: &[T]) { + self.invalidate_dcache_by_address(slice.as_ptr() as usize, + slice.len() * core::mem::size_of::()); + } + /// Cleans D-cache by address /// - /// `addr`: the address to clean - /// `size`: size of the memory block, in number of bytes + /// * `addr`: the address to clean + /// * `size`: size of the memory block, in number of bytes /// /// Cleans cache starting from the lowest 32-byte aligned address represented by `addr`, /// in blocks of 32 bytes until at least `size` bytes have been cleaned. + /// + /// It is highly recommended that `addr` is 32-byte aligned and a `size` is a multiple of + /// 32 bytes long, otherwise surrounding data will also be cleaned. + /// + /// Cleaning the cache causes whatever data is present in the cache to be immediately written + /// to main memory, overwriting whatever was in main memory. #[inline] pub fn clean_dcache_by_address(&mut self, addr: usize, size: usize) { // No-op zero sized operations @@ -532,7 +628,7 @@ impl SCB { return; } - // NOTE(unsafe) All CBP registers are write-only and stateless + // NOTE(unsafe): No races as all CBP registers are write-only and stateless let mut cbp = unsafe { CBP::new() }; crate::asm::dsb(); @@ -552,14 +648,54 @@ impl SCB { crate::asm::isb(); } + /// Cleans an object in D-cache + /// + /// * `obj`: Object to clean + /// + /// Cleans cache starting from the lowest 32-byte aligned address containing `obj`, + /// in blocks of 32 bytes until all of `obj` has been cleaned. + /// + /// It is highly recommended that `obj` is both 32-byte aligned and a multiple of + /// 32 bytes long, otherwise surrounding data will also be cleaned. + /// + /// Cleaning the cache causes whatever data is present in the cache to be immediately written + /// to main memory, overwriting whatever was in main memory. + pub fn clean_dcache_by_ref(&mut self, obj: &T) { + self.clean_dcache_by_address(obj as *const T as usize, core::mem::size_of::()); + } + + /// Cleans a slice in D-cache + /// + /// * `slice`: Slice to clean + /// + /// Cleans cache starting from the lowest 32-byte aligned address containing members of + /// `slice`, in blocks of 32 bytes until all of `slice` has been cleaned. + /// + /// It is highly recommended that `slice` is both 32-byte aligned and a multiple of + /// 32 bytes long, otherwise surrounding data will also be cleaned. + /// + /// Cleaning the cache causes whatever data is present in the cache to be immediately written + /// to main memory, overwriting whatever was in main memory. + pub fn clean_dcache_by_slice(&mut self, slice: &[T]) { + self.clean_dcache_by_address(slice.as_ptr() as usize, + slice.len() * core::mem::size_of::()); + } + /// Cleans and invalidates D-cache by address /// - /// `addr`: the address to clean and invalidate - /// `size`: size of the memory block, in number of bytes + /// * `addr`: the address to clean and invalidate + /// * `size`: size of the memory block, in number of bytes /// /// Cleans and invalidates cache starting from the lowest 32-byte aligned address represented /// by `addr`, in blocks of 32 bytes until at least `size` bytes have been cleaned and /// invalidated. + /// + /// It is highly recommended that `addr` is 32-byte aligned and a `size` is a multiple of + /// 32 bytes long, otherwise surrounding data will also be cleaned. + /// + /// Cleaning and invalidating causes data in the D-cache to be written back to main memory, + /// and then marks that data in the D-cache as invalid, causing future reads to first fetch + /// from main memory. #[inline] pub fn clean_invalidate_dcache_by_address(&mut self, addr: usize, size: usize) { // No-op zero sized operations @@ -567,7 +703,7 @@ impl SCB { return; } - // NOTE(unsafe) All CBP registers are write-only and stateless + // NOTE(unsafe): No races as all CBP registers are write-only and stateless let mut cbp = unsafe { CBP::new() }; crate::asm::dsb(); From 9fac3d9f4c598c5a0929dcae0e62a486e247d021 Mon Sep 17 00:00:00 2001 From: Adam Greig Date: Tue, 4 Feb 2020 00:36:55 +0000 Subject: [PATCH 2/4] Update documentation for cache functions; use dynamic cache line size --- src/peripheral/cpuid.rs | 23 ++++++ src/peripheral/scb.rs | 150 +++++++++++++++++++++++++++------------- 2 files changed, 124 insertions(+), 49 deletions(-) diff --git a/src/peripheral/cpuid.rs b/src/peripheral/cpuid.rs index 787be5c6..3cb00799 100644 --- a/src/peripheral/cpuid.rs +++ b/src/peripheral/cpuid.rs @@ -114,4 +114,27 @@ impl CPUID { (1 + ((ccsidr & CCSIDR_ASSOCIATIVITY_MASK) >> CCSIDR_ASSOCIATIVITY_POS)) as u16, ) } + + /// Returns log2 of the number of words in the smallest cache line of all the data cache and + /// unified caches that are controlled by the processor. + /// + /// This is the `DminLine` field of the CTR register. + #[inline(always)] + pub fn cache_dminline() -> u32 { + const CTR_DMINLINE_POS: u32 = 16; + const CTR_DMINLINE_MASK: u32 = 0xF << CTR_DMINLINE_POS; + let ctr = unsafe { (*Self::ptr()).ctr.read() }; + (ctr & CTR_DMINLINE_MASK) >> CTR_DMINLINE_POS + } + + /// Returns log2 of the number of words in the smallest cache line of all the instruction + /// caches that are controlled by the processor. + /// + /// This is the `IminLine` field of the CTR register. + pub fn cache_iminline() -> u32 { + const CTR_IMINLINE_POS: u32 = 0; + const CTR_IMINLINE_MASK: u32 = 0xF << CTR_IMINLINE_POS; + let ctr = unsafe { (*Self::ptr()).ctr.read() }; + (ctr & CTR_IMINLINE_MASK) >> CTR_IMINLINE_POS + } } diff --git a/src/peripheral/scb.rs b/src/peripheral/scb.rs index a04d45f2..65173235 100644 --- a/src/peripheral/scb.rs +++ b/src/peripheral/scb.rs @@ -508,15 +508,25 @@ impl SCB { /// Invalidates D-cache by address /// - /// * `addr`: the address to invalidate - /// * `size`: size of the memory block, in number of bytes + /// * `addr`: the address to invalidate, which must be cache-line aligned + /// * `size`: number of bytes to invalidate, which must be a multiple of the cache line size /// - /// Invalidates cache starting from the lowest 32-byte aligned address represented by `addr`, - /// in blocks of 32 bytes until at least `size` bytes have been invalidated. + /// Invalidates D-cache cache lines, starting from the first line containing `addr`, + /// finishing once at least `size` bytes have been invalidated. /// /// Invalidation causes the next read access to memory to be fetched from main memory instead /// of the cache. /// + /// # Cache Line Sizes + /// + /// Cache line sizes vary by core. For all Cortex-M7 cores, the cache line size is fixed + /// to 32 bytes, which means `addr` must be 32-byte aligned and `size` must be a multiple + /// of 32. At the time of writing, no other Cortex-M cores have data caches. + /// + /// If `addr` is not cache-line aligned, or `size` is not a multiple of the cache line size, + /// other data before or after the desired memory would also be invalidated, which can very + /// easily cause memory corruption and undefined behaviour. + /// /// # Safety /// /// After invalidating, the next read of invalidated data will be from main memory. This may @@ -525,9 +535,11 @@ impl SCB { /// resulting in undefined behaviour. You must ensure that main memory contains valid and /// initialised values before invalidating. /// - /// If `addr` is not 32-byte aligned or if `size` is not a multiple of 32, other memory - /// will also be invalidated unexpectedly. It is highly recommended that `addr` is 32-byte - /// aligned and `size` is a multiple of 32 bytes. + /// `addr` **must** be aligned to the size of the cache lines, and `size` **must** be a + /// multiple of the cache line size, otherwise this function will invalidate other memory, + /// easily leading to memory corruption and undefined behaviour. This precondition is checked + /// in debug builds using a `debug_assert!()`, but not checked in release builds to avoid + /// a runtime-dependent `panic!()` call. #[inline] pub unsafe fn invalidate_dcache_by_address(&mut self, addr: usize, size: usize) { // No-op zero sized operations @@ -538,17 +550,25 @@ impl SCB { // NOTE(unsafe): No races as all CBP registers are write-only and stateless let mut cbp = CBP::new(); + // dminline is log2(num words), so 2**dminline * 4 gives size in bytes + let dminline = CPUID::cache_dminline(); + let line_size = (1 << dminline) * 4; + + debug_assert!((addr & (line_size - 1)) == 0); + debug_assert!((size & (line_size - 1)) == 0); + crate::asm::dsb(); - // Cache lines are fixed to 32 bit on Cortex-M7 and not present in earlier Cortex-M - const LINESIZE: usize = 32; - let num_lines = ((size - 1) / LINESIZE) + 1; + // Find number of cache lines to invalidate + let num_lines = ((size - 1) / line_size) + 1; - let mut addr = addr & 0xFFFF_FFE0; + // Compute address of first cache line + let mask = 0xFFFF_FFFF - (line_size - 1); + let mut addr = addr & mask; for _ in 0..num_lines { cbp.dcimvac(addr as u32); - addr += LINESIZE; + addr += line_size; } crate::asm::dsb(); @@ -559,12 +579,22 @@ impl SCB { /// /// * `obj`: Object to invalidate /// - /// Invalidates cache starting from the lowest 32-byte aligned address containing `obj`, - /// in blocks of 32 bytes until all of `obj` has been invalidated. + /// Invalidates D-cache starting from the first cache line containing `obj`, + /// continuing to invalidate cache lines until all of `obj` has been invalidated. /// /// Invalidation causes the next read access to memory to be fetched from main memory instead /// of the cache. /// + /// # Cache Line Sizes + /// + /// Cache line sizes vary by core. For all Cortex-M7 cores, the cache line size is fixed + /// to 32 bytes, which means `obj` must be 32-byte aligned, and its size must be a multiple + /// of 32 bytes. At the time of writing, no other Cortex-M cores have data caches. + /// + /// If `obj` is not cache-line aligned, or its size is not a multiple of the cache line size, + /// other data before or after the desired memory would also be invalidated, which can very + /// easily cause memory corruption and undefined behaviour. + /// /// # Safety /// /// After invalidating, `obj` will be read from main memory on next access. This may cause @@ -573,11 +603,13 @@ impl SCB { /// resulting in undefined behaviour. You must ensure that main memory contains a valid and /// initialised value for T before invalidating `obj`. /// - /// If `obj` is not both 32-byte aligned and a multiple of 32 bytes long, other memory - /// will also be invalidated unexpectedly. It is highly recommended that `obj` is 32-byte - /// aligned and a multiple of 32 bytes long. + /// `obj` **must** be aligned to the size of the cache lines, and its size **must** be a + /// multiple of the cache line size, otherwise this function will invalidate other memory, + /// easily leading to memory corruption and undefined behaviour. This precondition is checked + /// in debug builds using a `debug_assert!()`, but not checked in release builds to avoid + /// a runtime-dependent `panic!()` call. #[inline] - pub unsafe fn invalidate_dcache_by_ref(&mut self, obj: &T) { + pub unsafe fn invalidate_dcache_by_ref(&mut self, obj: &mut T) { self.invalidate_dcache_by_address(obj as *const T as usize, core::mem::size_of::()); } @@ -585,12 +617,22 @@ impl SCB { /// /// * `slice`: Slice to invalidate /// - /// Invalidates cache starting from the lowest 32-byte aligned address containing members of - /// `slice`, in blocks of 32 bytes until all of `slice` has been invalidated. + /// Invalidates D-cache starting from the first cache line containing members of `slice`, + /// continuing to invalidate cache lines until all of `slice` has been invalidated. /// /// Invalidation causes the next read access to memory to be fetched from main memory instead /// of the cache. /// + /// # Cache Line Sizes + /// + /// Cache line sizes vary by core. For all Cortex-M7 cores, the cache line size is fixed + /// to 32 bytes, which means `slice` must be 32-byte aligned, and its size must be a multiple + /// of 32 bytes. At the time of writing, no other Cortex-M cores have data caches. + /// + /// If `slice` is not cache-line aligned, or its size is not a multiple of the cache line size, + /// other data before or after the desired memory would also be invalidated, which can very + /// easily cause memory corruption and undefined behaviour. + /// /// # Safety /// /// After invalidating, `slice` will be read from main memory on next access. This may cause @@ -599,11 +641,13 @@ impl SCB { /// resulting in undefined behaviour. You must ensure that main memory contains valid and /// initialised values for T before invalidating `slice`. /// - /// If `slice` is not both 32-byte aligned and a multiple of 32 bytes long, other memory - /// will also be invalidated unexpectedly. It is highly recommended that `slice` is 32-byte - /// aligned and a multiple of 32 bytes long. + /// `slice` **must** be aligned to the size of the cache lines, and its size **must** be a + /// multiple of the cache line size, otherwise this function will invalidate other memory, + /// easily leading to memory corruption and undefined behaviour. This precondition is checked + /// in debug builds using a `debug_assert!()`, but not checked in release builds to avoid + /// a runtime-dependent `panic!()` call. #[inline] - pub unsafe fn invalidate_dcache_by_slice(&mut self, slice: &[T]) { + pub unsafe fn invalidate_dcache_by_slice(&mut self, slice: &mut [T]) { self.invalidate_dcache_by_address(slice.as_ptr() as usize, slice.len() * core::mem::size_of::()); } @@ -611,16 +655,24 @@ impl SCB { /// Cleans D-cache by address /// /// * `addr`: the address to clean - /// * `size`: size of the memory block, in number of bytes + /// * `size`: number of bytes to clean /// - /// Cleans cache starting from the lowest 32-byte aligned address represented by `addr`, - /// in blocks of 32 bytes until at least `size` bytes have been cleaned. - /// - /// It is highly recommended that `addr` is 32-byte aligned and a `size` is a multiple of - /// 32 bytes long, otherwise surrounding data will also be cleaned. + /// Cleans D-cache cache lines, starting from the first line containing `addr`, + /// finishing once at least `size` bytes have been invalidated. /// /// Cleaning the cache causes whatever data is present in the cache to be immediately written /// to main memory, overwriting whatever was in main memory. + /// + /// # Cache Line Sizes + /// + /// Cache line sizes vary by core. For all Cortex-M7 cores, the cache line size is fixed + /// to 32 bytes, which means `addr` should generally be 32-byte aligned and `size` should be a + /// multiple of 32. At the time of writing, no other Cortex-M cores have data caches. + /// + /// If `addr` is not cache-line aligned, or `size` is not a multiple of the cache line size, + /// other data before or after the desired memory will also be cleaned. From the point of view + /// of the core executing this function, memory remains consistent, so this is not unsound, + /// but is worth knowing about. #[inline] pub fn clean_dcache_by_address(&mut self, addr: usize, size: usize) { // No-op zero sized operations @@ -633,15 +685,16 @@ impl SCB { crate::asm::dsb(); - // Cache lines are fixed to 32 bit on Cortex-M7 and not present in earlier Cortex-M - const LINESIZE: usize = 32; - let num_lines = ((size - 1) / LINESIZE) + 1; + let dminline = CPUID::cache_dminline(); + let line_size = (1 << dminline) * 4; + let num_lines = ((size - 1) / line_size) + 1; - let mut addr = addr & 0xFFFF_FFE0; + let mask = 0xFFFF_FFFF - (line_size - 1); + let mut addr = addr & mask; for _ in 0..num_lines { cbp.dccmvac(addr as u32); - addr += LINESIZE; + addr += line_size; } crate::asm::dsb(); @@ -652,11 +705,11 @@ impl SCB { /// /// * `obj`: Object to clean /// - /// Cleans cache starting from the lowest 32-byte aligned address containing `obj`, - /// in blocks of 32 bytes until all of `obj` has been cleaned. + /// Cleans D-cache starting from the first cache line containing `obj`, + /// continuing to clean cache lines until all of `obj` has been cleaned. /// - /// It is highly recommended that `obj` is both 32-byte aligned and a multiple of - /// 32 bytes long, otherwise surrounding data will also be cleaned. + /// It is recommended that `obj` is both aligned to the cache line size and a multiple of + /// the cache line size long, otherwise surrounding data will also be cleaned. /// /// Cleaning the cache causes whatever data is present in the cache to be immediately written /// to main memory, overwriting whatever was in main memory. @@ -668,11 +721,11 @@ impl SCB { /// /// * `slice`: Slice to clean /// - /// Cleans cache starting from the lowest 32-byte aligned address containing members of - /// `slice`, in blocks of 32 bytes until all of `slice` has been cleaned. + /// Cleans D-cache starting from the first cache line containing members of `slice`, + /// continuing to clean cache lines until all of `slice` has been cleaned. /// - /// It is highly recommended that `slice` is both 32-byte aligned and a multiple of - /// 32 bytes long, otherwise surrounding data will also be cleaned. + /// It is recommended that `slice` is both aligned to the cache line size and a multiple of + /// the cache line size long, otherwise surrounding data will also be cleaned. /// /// Cleaning the cache causes whatever data is present in the cache to be immediately written /// to main memory, overwriting whatever was in main memory. @@ -684,14 +737,13 @@ impl SCB { /// Cleans and invalidates D-cache by address /// /// * `addr`: the address to clean and invalidate - /// * `size`: size of the memory block, in number of bytes + /// * `size`: number of bytes to clean and invalidate /// - /// Cleans and invalidates cache starting from the lowest 32-byte aligned address represented - /// by `addr`, in blocks of 32 bytes until at least `size` bytes have been cleaned and - /// invalidated. + /// Cleans and invalidates D-cache starting from the first cache line containing `addr`, + /// finishing once at least `size` bytes have been cleaned and invalidated. /// - /// It is highly recommended that `addr` is 32-byte aligned and a `size` is a multiple of - /// 32 bytes long, otherwise surrounding data will also be cleaned. + /// It is recommended that `addr` is aligned to the cache line size and `size` is a multiple of + /// the cache line size, otherwise surrounding data will also be cleaned. /// /// Cleaning and invalidating causes data in the D-cache to be written back to main memory, /// and then marks that data in the D-cache as invalid, causing future reads to first fetch From 2608c234067b1da2e9aaf76ff13bbd6ac532edd3 Mon Sep 17 00:00:00 2001 From: Adam Greig Date: Tue, 4 Feb 2020 23:36:46 +0000 Subject: [PATCH 3/4] Apply suggestions from code review Co-Authored-By: Jonas Schievink --- src/peripheral/scb.rs | 56 +++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/src/peripheral/scb.rs b/src/peripheral/scb.rs index 65173235..4ab1e8f8 100644 --- a/src/peripheral/scb.rs +++ b/src/peripheral/scb.rs @@ -314,7 +314,7 @@ use self::scb_consts::*; #[cfg(not(armv6m))] impl SCB { - /// Enables I-cache if currently disabled + /// Enables I-cache if currently disabled. /// /// This operation first invalidates the entire I-cache. #[inline] @@ -338,7 +338,7 @@ impl SCB { crate::asm::isb(); } - /// Disables I-cache if currently enabled + /// Disables I-cache if currently enabled. /// /// This operation invalidates the entire I-cache after disabling. #[inline] @@ -362,7 +362,7 @@ impl SCB { crate::asm::isb(); } - /// Returns whether the I-cache is currently enabled + /// Returns whether the I-cache is currently enabled. #[inline(always)] pub fn icache_enabled() -> bool { crate::asm::dsb(); @@ -372,7 +372,7 @@ impl SCB { unsafe { (*Self::ptr()).ccr.read() & SCB_CCR_IC_MASK == SCB_CCR_IC_MASK } } - /// Invalidates entire I-cache + /// Invalidates the entire I-cache. #[inline] pub fn invalidate_icache(&mut self) { // NOTE(unsafe): No races as all CBP registers are write-only and stateless @@ -385,7 +385,7 @@ impl SCB { crate::asm::isb(); } - /// Enables D-cache if currently disabled + /// Enables D-cache if currently disabled. /// /// This operation first invalidates the entire D-cache, ensuring it does /// not contain stale values before being enabled. @@ -407,7 +407,7 @@ impl SCB { crate::asm::isb(); } - /// Disables D-cache if currently enabled + /// Disables D-cache if currently enabled. /// /// This operation subsequently cleans and invalidates the entire D-cache, /// ensuring all contents are safely written back to main memory after disabling. @@ -426,7 +426,7 @@ impl SCB { self.clean_invalidate_dcache(cpuid); } - /// Returns whether the D-cache is currently enabled + /// Returns whether the D-cache is currently enabled. #[inline] pub fn dcache_enabled() -> bool { crate::asm::dsb(); @@ -436,7 +436,7 @@ impl SCB { unsafe { (*Self::ptr()).ccr.read() & SCB_CCR_DC_MASK == SCB_CCR_DC_MASK } } - /// Invalidates entire D-cache + /// Invalidates the entire D-cache. /// /// Note that calling this while the dcache is enabled will probably wipe out the /// stack, depending on optimisations, therefore breaking returning to the call point. @@ -461,7 +461,7 @@ impl SCB { crate::asm::isb(); } - /// Cleans entire D-cache + /// Cleans the entire D-cache. /// /// This function causes everything in the D-cache to be written back to main memory, /// overwriting whatever is already there. @@ -483,7 +483,7 @@ impl SCB { crate::asm::isb(); } - /// Cleans and invalidates entire D-cache + /// Cleans and invalidates the entire D-cache. /// /// This function causes everything in the D-cache to be written back to main memory, /// and then marks the entire D-cache as invalid, causing future reads to first fetch @@ -506,10 +506,10 @@ impl SCB { crate::asm::isb(); } - /// Invalidates D-cache by address + /// Invalidates D-cache by address. /// - /// * `addr`: the address to invalidate, which must be cache-line aligned - /// * `size`: number of bytes to invalidate, which must be a multiple of the cache line size + /// * `addr`: The address to invalidate, which must be cache-line aligned. + /// * `size`: Number of bytes to invalidate, which must be a multiple of the cache line size. /// /// Invalidates D-cache cache lines, starting from the first line containing `addr`, /// finishing once at least `size` bytes have been invalidated. @@ -530,7 +530,7 @@ impl SCB { /// # Safety /// /// After invalidating, the next read of invalidated data will be from main memory. This may - /// cause recent writes to be lost, potentially including writes that initialised objects. + /// cause recent writes to be lost, potentially including writes that initialized objects. /// Therefore, this method may cause uninitialised memory or invalid values to be read, /// resulting in undefined behaviour. You must ensure that main memory contains valid and /// initialised values before invalidating. @@ -575,9 +575,9 @@ impl SCB { crate::asm::isb(); } - /// Invalidates an object from the D-cache + /// Invalidates an object from the D-cache. /// - /// * `obj`: Object to invalidate + /// * `obj`: The object to invalidate. /// /// Invalidates D-cache starting from the first cache line containing `obj`, /// continuing to invalidate cache lines until all of `obj` has been invalidated. @@ -613,9 +613,9 @@ impl SCB { self.invalidate_dcache_by_address(obj as *const T as usize, core::mem::size_of::()); } - /// Invalidates a slice from the D-cache + /// Invalidates a slice from the D-cache. /// - /// * `slice`: Slice to invalidate + /// * `slice`: The slice to invalidate. /// /// Invalidates D-cache starting from the first cache line containing members of `slice`, /// continuing to invalidate cache lines until all of `slice` has been invalidated. @@ -652,10 +652,10 @@ impl SCB { slice.len() * core::mem::size_of::()); } - /// Cleans D-cache by address + /// Cleans D-cache by address. /// - /// * `addr`: the address to clean - /// * `size`: number of bytes to clean + /// * `addr`: The address to start cleaning at. + /// * `size`: The number of bytes to clean. /// /// Cleans D-cache cache lines, starting from the first line containing `addr`, /// finishing once at least `size` bytes have been invalidated. @@ -701,9 +701,9 @@ impl SCB { crate::asm::isb(); } - /// Cleans an object in D-cache + /// Cleans an object from the D-cache. /// - /// * `obj`: Object to clean + /// * `obj`: The object to clean. /// /// Cleans D-cache starting from the first cache line containing `obj`, /// continuing to clean cache lines until all of `obj` has been cleaned. @@ -717,9 +717,9 @@ impl SCB { self.clean_dcache_by_address(obj as *const T as usize, core::mem::size_of::()); } - /// Cleans a slice in D-cache + /// Cleans a slice from D-cache. /// - /// * `slice`: Slice to clean + /// * `slice`: The slice to clean. /// /// Cleans D-cache starting from the first cache line containing members of `slice`, /// continuing to clean cache lines until all of `slice` has been cleaned. @@ -734,10 +734,10 @@ impl SCB { slice.len() * core::mem::size_of::()); } - /// Cleans and invalidates D-cache by address + /// Cleans and invalidates D-cache by address. /// - /// * `addr`: the address to clean and invalidate - /// * `size`: number of bytes to clean and invalidate + /// * `addr`: The address to clean and invalidate. + /// * `size`: The number of bytes to clean and invalidate. /// /// Cleans and invalidates D-cache starting from the first cache line containing `addr`, /// finishing once at least `size` bytes have been cleaned and invalidated. From 9ad4e10d8a96b283b31b3309a65678513d50e270 Mon Sep 17 00:00:00 2001 From: Adam Greig Date: Tue, 4 Feb 2020 23:38:47 +0000 Subject: [PATCH 4/4] Add missing inline(always) and change more initialised->initialized --- src/peripheral/cpuid.rs | 1 + src/peripheral/scb.rs | 16 ++++++++-------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/peripheral/cpuid.rs b/src/peripheral/cpuid.rs index 3cb00799..32d0bafa 100644 --- a/src/peripheral/cpuid.rs +++ b/src/peripheral/cpuid.rs @@ -131,6 +131,7 @@ impl CPUID { /// caches that are controlled by the processor. /// /// This is the `IminLine` field of the CTR register. + #[inline(always)] pub fn cache_iminline() -> u32 { const CTR_IMINLINE_POS: u32 = 0; const CTR_IMINLINE_MASK: u32 = 0xF << CTR_IMINLINE_POS; diff --git a/src/peripheral/scb.rs b/src/peripheral/scb.rs index 4ab1e8f8..eb7bb720 100644 --- a/src/peripheral/scb.rs +++ b/src/peripheral/scb.rs @@ -531,9 +531,9 @@ impl SCB { /// /// After invalidating, the next read of invalidated data will be from main memory. This may /// cause recent writes to be lost, potentially including writes that initialized objects. - /// Therefore, this method may cause uninitialised memory or invalid values to be read, + /// Therefore, this method may cause uninitialized memory or invalid values to be read, /// resulting in undefined behaviour. You must ensure that main memory contains valid and - /// initialised values before invalidating. + /// initialized values before invalidating. /// /// `addr` **must** be aligned to the size of the cache lines, and `size` **must** be a /// multiple of the cache line size, otherwise this function will invalidate other memory, @@ -598,10 +598,10 @@ impl SCB { /// # Safety /// /// After invalidating, `obj` will be read from main memory on next access. This may cause - /// recent writes to `obj` to be lost, potentially including the write that initialised it. - /// Therefore, this method may cause uninitialised memory or invalid values to be read, + /// recent writes to `obj` to be lost, potentially including the write that initialized it. + /// Therefore, this method may cause uninitialized memory or invalid values to be read, /// resulting in undefined behaviour. You must ensure that main memory contains a valid and - /// initialised value for T before invalidating `obj`. + /// initialized value for T before invalidating `obj`. /// /// `obj` **must** be aligned to the size of the cache lines, and its size **must** be a /// multiple of the cache line size, otherwise this function will invalidate other memory, @@ -636,10 +636,10 @@ impl SCB { /// # Safety /// /// After invalidating, `slice` will be read from main memory on next access. This may cause - /// recent writes to `slice` to be lost, potentially including the write that initialised it. - /// Therefore, this method may cause uninitialised memory or invalid values to be read, + /// recent writes to `slice` to be lost, potentially including the write that initialized it. + /// Therefore, this method may cause uninitialized memory or invalid values to be read, /// resulting in undefined behaviour. You must ensure that main memory contains valid and - /// initialised values for T before invalidating `slice`. + /// initialized values for T before invalidating `slice`. /// /// `slice` **must** be aligned to the size of the cache lines, and its size **must** be a /// multiple of the cache line size, otherwise this function will invalidate other memory,