Skip to content

Commit f24948e

Browse files
committed
fix(dmac): Use inline asm for compiler fences in DMAC
Signed-off-by: Justin Beaurivage <[email protected]>
1 parent 09a2188 commit f24948e

File tree

3 files changed

+37
-26
lines changed

3 files changed

+37
-26
lines changed

hal/src/dmac/async_api.rs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//! APIs for async DMAC operations.
22
33
use atsamd_hal_macros::hal_cfg;
4-
use core::sync::atomic;
54

65
use crate::{
76
async_hal::interrupts::{DMAC, Handler},
@@ -55,11 +54,7 @@ impl Handler<DMAC> for InterruptHandler {
5554
core::hint::spin_loop();
5655
}
5756

58-
// Prevent the compiler from re-ordering read/write
59-
// operations beyond this fence.
60-
// (see https://docs.rust-embedded.org/embedonomicon/dma.html#compiler-misoptimizations)
61-
atomic::fence(atomic::Ordering::Acquire); // ▼
62-
57+
super::channel::release_fence(); // ▲
6358
WAKERS[pend_channel as usize].wake();
6459
}
6560
}
@@ -112,10 +107,7 @@ impl Handler<DMAC> for InterruptHandler {
112107
core::hint::spin_loop();
113108
}
114109

115-
// Prevent the compiler from re-ordering read/write
116-
// operations beyond this fence.
117-
// (see https://docs.rust-embedded.org/embedonomicon/dma.html#compiler-misoptimizations)
118-
atomic::fence(atomic::Ordering::Acquire); // ▼
110+
super::channel::acquire_fence(); // ▼
119111

120112
WAKERS[channel].wake();
121113
}

hal/src/dmac/channel/mod.rs

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
#![allow(unused_braces)]
3535

3636
use core::marker::PhantomData;
37-
use core::sync::atomic;
3837

3938
use atsamd_hal_macros::{hal_cfg, hal_macro_helper};
4039

@@ -271,10 +270,7 @@ impl<Id: ChId, S: Status> Channel<Id, S> {
271270
/// Enable the transfer, and emit a compiler fence.
272271
#[inline]
273272
fn _enable_private(&mut self) {
274-
// Prevent the compiler from re-ordering read/write
275-
// operations beyond this fence.
276-
// (see https://docs.rust-embedded.org/embedonomicon/dma.html#compiler-misoptimizations)
277-
atomic::fence(atomic::Ordering::Release); // ▲
273+
release_fence(); // ▲
278274
self.regs.chctrla.modify(|_, w| w.enable().set_bit());
279275
}
280276

@@ -288,10 +284,7 @@ impl<Id: ChId, S: Status> Channel<Id, S> {
288284
core::hint::spin_loop();
289285
}
290286

291-
// Prevent the compiler from re-ordering read/write
292-
// operations beyond this fence.
293-
// (see https://docs.rust-embedded.org/embedonomicon/dma.html#compiler-misoptimizations)
294-
atomic::fence(atomic::Ordering::Acquire); // ▼
287+
acquire_fence(); // ▼
295288
}
296289

297290
/// Returns whether or not the transfer is complete.
@@ -882,3 +875,33 @@ pub(crate) unsafe fn write_descriptor<Src: Buffer, Dst: Buffer<Beat = Src::Beat>
882875
btctrl,
883876
};
884877
}
878+
879+
/// Prevent the compiler from re-ordering read/write
880+
/// operations beyond this function.
881+
/// (see https://docs.rust-embedded.org/embedonomicon/dma.html#compiler-misoptimizations)
882+
#[inline(always)]
883+
pub(super) fn acquire_fence() {
884+
// TODO: Seems like compiler fences aren't enough to guarantee memory accesses won't be reordered.
885+
// (see https://users.rust-lang.org/t/compiler-fence-dma/132027)
886+
// core::sync::atomic::fence(core::sync::atomic::Ordering::Acquire); // ▼
887+
888+
// Apparently, the only truly foolproof way to prevent reordering is with inline asm
889+
unsafe {
890+
core::arch::asm!("dmb");
891+
}
892+
}
893+
894+
/// Prevent the compiler from re-ordering read/write
895+
/// operations beyond this function.
896+
/// (see https://docs.rust-embedded.org/embedonomicon/dma.html#compiler-misoptimizations)
897+
#[inline(always)]
898+
pub(super) fn release_fence() {
899+
// TODO: Seems like compiler fences aren't enough to guarantee memory accesses won't be reordered.
900+
// (see https://users.rust-lang.org/t/compiler-fence-dma/132027)
901+
// core::sync::atomic::fence(atomic::Ordering::Release); // ▲
902+
903+
// Apparently, the only truly foolproof way to prevent reordering is with inline asm
904+
unsafe {
905+
core::arch::asm!("dmb");
906+
}
907+
}

hal/src/dmac/channel/reg.rs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,9 @@ use paste::paste;
1818

1919
use crate::pac::{
2020
self, Dmac, Peripherals,
21-
dmac::{Busych, Intstatus, Pendch, Swtrigctrl},
2221
dmac::{
23-
busych::BusychSpec, intstatus::IntstatusSpec, pendch::PendchSpec,
24-
swtrigctrl::SwtrigctrlSpec,
22+
Busych, Intstatus, Pendch, Swtrigctrl, busych::BusychSpec, intstatus::IntstatusSpec,
23+
pendch::PendchSpec, swtrigctrl::SwtrigctrlSpec,
2524
},
2625
};
2726

@@ -324,9 +323,6 @@ impl<Id: ChId> Drop for RegisterBlock<Id> {
324323
core::hint::spin_loop();
325324
}
326325

327-
// Prevent the compiler from re-ordering read/write
328-
// operations beyond this fence.
329-
// (see https://docs.rust-embedded.org/embedonomicon/dma.html#compiler-misoptimizations)
330-
core::sync::atomic::fence(core::sync::atomic::Ordering::Acquire); // ▼
326+
crate::dmac::channel::acquire_fence(); // ▼
331327
}
332328
}

0 commit comments

Comments
 (0)