Add fences

Author: thalesfragoso

Cargo.toml

@@ -22,7 +22,7 @@ volatile-register = "0.2"
 aligned = "0.3"
 stm32f7xx-hal = {version = "0.2.0", optional = true}
 stm32f4xx-hal = {version = "0.8.3", optional = true}
-cortex-m = "0.6.0"
+cortex-m = "0.6.2"
 
 smoltcp = { version = "0.6.0", default-features = false, features = ["proto-ipv4", "proto-ipv6", "socket-icmp", "socket-udp", "socket-tcp", "ethernet"], optional = true }
 log = { version = "0.4", optional = true }

src/lib.rs

@@ -17,8 +17,6 @@ pub use stm32f4xx_hal::stm32;
 use hal::rcc::Clocks;
 use stm32::{Interrupt, ETHERNET_DMA, ETHERNET_MAC, NVIC};
 
-use cortex_m::asm;
-
 pub mod phy;
 use phy::{Phy, PhyStatus};
 mod ring;
@@ -88,7 +86,7 @@ impl<'rx, 'tx> Eth<'rx, 'tx> {
     ///
     /// Make sure that the buffers reside in a memory region that is
     /// accessible by the peripheral. Core-Coupled Memory (CCM) is
-    /// usually not accessible. HCLK must be between 25MHz and 168MHz for STM32F4xx 
+    /// usually not accessible. HCLK must be between 25MHz and 168MHz for STM32F4xx
     /// or 25MHz to 216MHz for STM32F7xx.
     ///
     /// Uses an interrupt free critical section to turn on the ethernet clock for STM32F7xx.
@@ -314,8 +312,6 @@ impl<'rx, 'tx> Eth<'rx, 'tx> {
         f: F,
     ) -> Result<R, TxError> {
         let result = self.tx_ring.send(length, f);
-        //Make sure the memory write occurs before triggering DMA
-        asm::dmb();
         self.tx_ring.demand_poll(&self.eth_dma);
         result
     }

src/rx.rs

@@ -5,8 +5,11 @@ use stm32f7xx_hal::device as stm32;
 
 use stm32::ETHERNET_DMA;
 
-use core::default::Default;
-use core::ops::{Deref, DerefMut};
+use core::{
+    default::Default,
+    ops::{Deref, DerefMut},
+    sync::atomic::{self, Ordering},
+};
 
 use crate::{
     desc::Descriptor,
@@ -122,6 +125,7 @@ impl RingDescriptor for RxDescriptor {
                 self.set_end_of_ring();
             }
         };
+        // There is already a fence before starting the DMA engine
         self.set_owned();
     }
 }
@@ -136,6 +140,9 @@ impl RxRingEntry {
         } else if self.desc().is_first() && self.desc().is_last() {
             let frame_len = self.desc().get_frame_len();
 
+            // "Subsequent reads and writes cannot be moved ahead of preceding reads."
+            atomic::compiler_fence(Ordering::Acquire);
+
             // TODO: obtain ethernet frame type (RDESC_1_FT)
             let pkt = RxPacket {
                 entry: self,
@@ -170,6 +177,13 @@ impl<'a> DerefMut for RxPacket<'a> {
 
 impl<'a> Drop for RxPacket<'a> {
     fn drop(&mut self) {
+        // "Preceding reads and writes cannot be moved past subsequent writes."
+        #[cfg(feature = "stm32f7xx-hal")]
+        atomic::fence(Ordering::Release);
+
+        #[cfg(not(feature = "stm32f7xx-hal"))]
+        atomic::compiler_fence(Ordering::Release);
+
         self.entry.desc_mut().set_owned();
     }
 }
@@ -216,6 +230,13 @@ impl<'a> RxRing<'a> {
         // Register RxDescriptor
         eth_dma.dmardlar.write(|w| w.srl().bits(ring_ptr as u32));
 
+        // "Preceding reads and writes cannot be moved past subsequent writes."
+        #[cfg(feature = "stm32f7xx-hal")]
+        atomic::fence(Ordering::Release);
+
+        // We don't need a compiler fence here because all interactions with `Descriptor` are
+        // volatiles
+
         // Start receive
         eth_dma.dmaomr.modify(|_, w| w.sr().set_bit());
 
@@ -225,6 +246,11 @@ impl<'a> RxRing<'a> {
     /// Demand that the DMA engine polls the current `RxDescriptor`
     /// (when in `RunningState::Stopped`.)
     pub fn demand_poll(&self, eth_dma: &ETHERNET_DMA) {
+        // Prevent `set_owned` to me moved to after the poll
+        // "Preceding reads and writes cannot be moved past subsequent writes."
+        #[cfg(feature = "stm32f7xx-hal")]
+        atomic::fence(Ordering::Release);
+
         eth_dma.dmarpdr.write(|w| unsafe { w.rpd().bits(1) });
     }
 

src/tx.rs

@@ -5,7 +5,10 @@ use stm32f7xx_hal::device as stm32;
 
 use stm32::ETHERNET_DMA;
 
-use core::ops::{Deref, DerefMut};
+use core::{
+    ops::{Deref, DerefMut},
+    sync::atomic::{self, Ordering},
+};
 
 use crate::{
     desc::Descriptor,
@@ -152,6 +155,13 @@ impl<'a> DerefMut for TxPacket<'a> {
 impl<'a> TxPacket<'a> {
     // Pass to DMA engine
     pub fn send(self) {
+        // "Preceding reads and writes cannot be moved past subsequent writes."
+        #[cfg(feature = "stm32f7xx-hal")]
+        atomic::fence(Ordering::Release);
+
+        #[cfg(not(feature = "stm32f7xx-hal"))]
+        atomic::compiler_fence(Ordering::Release);
+
         self.entry.desc_mut().set_owned();
     }
 }
@@ -193,6 +203,13 @@ impl<'a> TxRing<'a> {
         // Register TxDescriptor
         eth_dma.dmatdlar.write(|w| w.stl().bits(ring_ptr as u32));
 
+        // "Preceding reads and writes cannot be moved past subsequent writes."
+        #[cfg(feature = "stm32f7xx-hal")]
+        atomic::fence(Ordering::Release);
+
+        // We don't need a compiler fence here because all interactions with `Descriptor` are
+        // volatiles
+
         // Start transmission
         eth_dma.dmaomr.modify(|_, w| w.st().set_bit());
     }
@@ -222,6 +239,15 @@ impl<'a> TxRing<'a> {
     /// Demand that the DMA engine polls the current `TxDescriptor`
     /// (when we just transferred ownership to the hardware).
     pub fn demand_poll(&self, eth_dma: &ETHERNET_DMA) {
+        // "Preceding reads and writes cannot be moved past subsequent writes."
+        #[cfg(feature = "stm32f7xx-hal")]
+        atomic::fence(Ordering::Release);
+
+        // We don't need a compiler fence here because all operations to the buffer were done before
+        // setting the owned bit (see fences in `TxPacket::send`). We also don't need a compiler
+        // fence between setting the owned bit and starting the poll, because setting the owned bit
+        // is a volatile operation
+
         eth_dma.dmatpdr.write(|w| w.tpd().poll());
     }