From ab1735950b2108eaa8d51eb63efadcd2e25c35c4 Mon Sep 17 00:00:00 2001
From: Chris Ballance <chris.ballance@physics.ox.ac.uk>
Date: Tue, 12 Nov 2019 22:37:25 +0000
Subject: [PATCH] fix memory safety issue in ethernet interface (closes #33)

The CPU is allowed to access normal memory writes out-of-order. Here
the write to the OWN flag in the DMA descriptor (normal memory) was
placed after the DMA tail pointer advance (in device memory, so not
reorderable). This meant the ethernet DMA engine stalled as it saw
a descriptor it did not own, and only restarted and sent the packet
when the next packet was released.

This fix will work as long as the CPU data cache is disabled. If we
want to enable the cache, the simplest method would be to mark SRAM3
as uncacheable via the MPU.
---
 src/eth.rs | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/src/eth.rs b/src/eth.rs
index 603dc40..1da9c19 100644
--- a/src/eth.rs
+++ b/src/eth.rs
@@ -245,7 +245,13 @@ impl RxRing {
 
         let addr = &self.desc_buf[self.cur_desc] as *const _ as u32;
         assert_eq!(addr & 0x3, 0);
+
         let dma = unsafe { pac::Peripherals::steal().ETHERNET_DMA };
+
+        // Ensure changes to the descriptor (in particular, the OWN flag) are
+        // committed before DMA engine sees tail pointer store.
+        cortex_m::asm::dsb();
+
         dma.dmacrx_dtpr.write(|w| unsafe { w.bits(addr) });
 
         self.cur_desc = self.next_desc();
@@ -314,7 +320,13 @@ impl TxRing {
 
         let addr = &self.desc_buf[self.cur_desc] as *const _ as u32;
         assert_eq!(addr & 0x3, 0);
+
         let dma = unsafe { pac::Peripherals::steal().ETHERNET_DMA };
+
+        // Ensure packet contents as well as changes to the descriptor have been
+        // committed before DMA engine sees the tail pointer store.
+        cortex_m::asm::dsb();
+
         dma.dmactx_dtpr.write(|w| unsafe { w.bits(addr) });
     }
 }
@@ -490,6 +502,10 @@ impl Device {
         });
         eth_mtl.mtltx_qomr.modify(|_, w| w.ftq().set_bit());
 
+        // Ensure ring buffer descriptors have been set up in memory before
+        // enabling DMA engine.
+        cortex_m::asm::dsb();
+
         // Manage DMA transmission and reception
         eth_dma.dmactx_cr.modify(|_, w| w.st().set_bit());
         eth_dma.dmacrx_cr.modify(|_, w| w.sr().set_bit());