Skip to content

Commit a1262cb

Browse files
committed
fix(port_arm_m): restrict memory operation reordering around BASEPRI updates
Fixes tests failing on Cortex-M targets with `lto = true` and `+basepri`.
1 parent 7c525a8 commit a1262cb

File tree

1 file changed

+32
-1
lines changed
  • src/r3_port_arm_m/src/threading

1 file changed

+32
-1
lines changed

src/r3_port_arm_m/src/threading/imp.rs

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
use core::{cell::UnsafeCell, mem::MaybeUninit, slice};
1+
use core::{
2+
cell::UnsafeCell,
3+
mem::MaybeUninit,
4+
slice,
5+
sync::atomic::{compiler_fence, Ordering},
6+
};
27
use memoffset::offset_of;
38
use r3_core::{
49
kernel::{
@@ -391,6 +396,29 @@ impl State {
391396
if Traits::CPU_LOCK_PRIORITY_MASK > 0 {
392397
// Set `BASEPRI` to `CPU_LOCK_PRIORITY_MASK`
393398
unsafe { cortex_m::register::basepri::write(Traits::CPU_LOCK_PRIORITY_MASK) };
399+
400+
// Synchronize with the previous owner of CPU Lock.
401+
//
402+
// The semantics of `compiler_fence` we need here for this to work
403+
// might be more strict than what `compiler_fence`'s documentation
404+
// ("the compiler may be disallowed from moving reads or writes from
405+
// before or after the call to the other side of the call to
406+
// `compiler_fence`"; but it doesn't say it can't be moved past
407+
// `basepri::write`, which has `options(nomem)`[2] and therefore is
408+
// not a memory operation) and the C++ memory model ("memory
409+
// synchronization ordering of non-atomic and relaxed atomic
410+
// accesses"[3] but there's no atomic access here) say. But on the
411+
// other hand, there's a code comment[1] from the `cortex-m` package
412+
// (maintained by Rust's official embedded devices WG[4]) suggesting
413+
// that `compiler_fence` can in fact prevent the reordering as
414+
// intended. We're going to take their word for it since we are
415+
// using this package anyway.
416+
//
417+
// [1]: https://github.com/rust-embedded/cortex-m/blob/92552c73d3b56dc86007450633950d16ebe0e495/asm/inline.rs#L36
418+
// [2]: https://github.com/rust-embedded/cortex-m/blob/92552c73d3b56dc86007450633950d16ebe0e495/asm/inline.rs#L243
419+
// [3]: https://en.cppreference.com/w/cpp/atomic/atomic_signal_fence
420+
// [4]: https://github.com/rust-embedded/wg
421+
compiler_fence(Ordering::Acquire);
394422
return;
395423
}
396424

@@ -407,6 +435,9 @@ impl State {
407435
unsafe fn leave_cpu_lock_inner<Traits: PortInstance>() {
408436
#[cfg(not(any(armv6m, armv8m_base)))]
409437
if Traits::CPU_LOCK_PRIORITY_MASK > 0 {
438+
// Synchronize with the next owner of CPU Lock.
439+
compiler_fence(Ordering::Release);
440+
410441
// Set `BASEPRI` to `0` (no masking)
411442
unsafe { cortex_m::register::basepri::write(0) };
412443
return;

0 commit comments

Comments
 (0)