Skip to content

Commit

Permalink
Use SyncUnsafeCell to replace use of static mut
Browse files Browse the repository at this point in the history
  • Loading branch information
richardeoin committed Dec 30, 2024
1 parent 0735f79 commit aa24d24
Show file tree
Hide file tree
Showing 19 changed files with 489 additions and 346 deletions.
68 changes: 40 additions & 28 deletions examples/dma.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

use core::mem::MaybeUninit;

// TODO: use core::cell::SyncUnsafeCell when stabilized rust-lang/rust#95439
use utilities::sync_unsafe_cell::SyncUnsafeCell;

use cortex_m_rt::entry;
#[macro_use]
mod utilities;
Expand All @@ -24,9 +27,11 @@ use log::info;
//
// The runtime does not initialise these SRAM banks.
#[link_section = ".sram4.buffers"]
static mut SOURCE_BUFFER: MaybeUninit<[u32; 20]> = MaybeUninit::uninit();
static SOURCE_BUFFER: MaybeUninit<SyncUnsafeCell<[u32; 20]>> =
MaybeUninit::uninit();
#[link_section = ".axisram.buffers"]
static mut TARGET_BUFFER: MaybeUninit<[u32; 20]> = MaybeUninit::uninit();
static TARGET_BUFFER: MaybeUninit<SyncUnsafeCell<[u32; 20]>> =
MaybeUninit::uninit();

#[entry]
fn main() -> ! {
Expand All @@ -53,29 +58,37 @@ fn main() -> ! {
info!("stm32h7xx-hal example - Memory to Memory DMA");
info!("");

// Initialise the source buffer with truly random data, without taking any
// references to uninitialisated memory
let source_buffer: &'static mut [u32; 20] = {
let buf: &mut [MaybeUninit<u32>; 20] = unsafe {
&mut *(core::ptr::addr_of_mut!(SOURCE_BUFFER)
as *mut [MaybeUninit<u32>; 20])
};

for value in buf.iter_mut() {
unsafe {
value.as_mut_ptr().write(rng.gen().unwrap());
}
}
#[allow(static_mut_refs)] // TODO: Fix this
unsafe {
SOURCE_BUFFER.assume_init_mut()
// SOURCE_BUFFER is located in .axisram.buffers, which is not initialized by
// the runtime. We must manually initialize it to a valid value, without
// taking a reference to the uninitialized value
unsafe {
let cell = SOURCE_BUFFER.as_ptr();
for i in 0..20 {
core::ptr::addr_of_mut!((*SyncUnsafeCell::raw_get(cell))[i])
.write(rng.gen().unwrap());
}
};
}
// Now we can take a mutable reference to SOURCE_BUFFER. To avoid aliasing,
// this reference must only be taken once
let source_buffer =
unsafe { &mut *SyncUnsafeCell::raw_get(SOURCE_BUFFER.as_ptr()) };
// Save a copy on the stack so we can check it later
let source_buffer_cloned = *source_buffer;

// NOTE(unsafe): TARGET_BUFFER must also be initialised to prevent undefined
// behaviour (taking a mutable reference to uninitialised memory)
// TARGET_BUFFER is located in .axisram.buffers, which is not initialized by
// the runtime. We must manually initialize it to a valid value, without
// taking a reference to the uninitialized value
unsafe {
let cell = TARGET_BUFFER.as_ptr();
for i in 0..20 {
core::ptr::addr_of_mut!((*SyncUnsafeCell::raw_get(cell))[i])
.write(0);
}
}
// Now we can take a mutable reference to TARGET_BUFFER. To avoid aliasing,
// this reference must only be taken once
let target_buffer =
unsafe { &mut *SyncUnsafeCell::raw_get(TARGET_BUFFER.as_ptr()) };

// Setup DMA
//
Expand All @@ -92,9 +105,7 @@ fn main() -> ! {
Transfer::init(
streams.4,
MemoryToMemory::new(),
unsafe {
(*core::ptr::addr_of_mut!(TARGET_BUFFER)).assume_init_mut()
}, // Uninitialised memory
target_buffer,
Some(source_buffer),
config,
);
Expand All @@ -104,10 +115,11 @@ fn main() -> ! {
// Wait for transfer to complete
while !transfer.get_transfer_complete_flag() {}

// Now the target memory is actually initialised
#[allow(static_mut_refs)] // TODO: Fix this
let target_buffer: &'static mut [u32; 20] =
unsafe { TARGET_BUFFER.assume_init_mut() };
// Take a second(!) reference to the target buffer. Because the transfer has
// stopped, we can reason that the first reference is no longer
// active. Therefore we can take another reference without causing aliasing
let target_buffer: &[u32; 20] =
unsafe { &*SyncUnsafeCell::raw_get(TARGET_BUFFER.as_ptr()) };

// Comparison check
assert_eq!(&source_buffer_cloned, target_buffer);
Expand Down
23 changes: 17 additions & 6 deletions examples/ethernet-nucleo-h743zi2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ use core::mem::MaybeUninit;
use core::sync::atomic::{AtomicU32, Ordering};
use rt::{entry, exception};

// TODO: use core::cell::SyncUnsafeCell when stabilized rust-lang/rust#95439
use utilities::sync_unsafe_cell::SyncUnsafeCell;

extern crate cortex_m;

#[macro_use]
Expand Down Expand Up @@ -52,7 +55,7 @@ const MAC_ADDRESS: [u8; 6] = [0x02, 0x00, 0x11, 0x22, 0x33, 0x44];

/// Ethernet descriptor rings are a global singleton
#[link_section = ".sram3.eth"]
static mut DES_RING: MaybeUninit<ethernet::DesRing<4, 4>> =
static DES_RING: MaybeUninit<SyncUnsafeCell<ethernet::DesRing<4, 4>>> =
MaybeUninit::uninit();

// the program entry point
Expand Down Expand Up @@ -113,9 +116,18 @@ fn main() -> ! {
assert_eq!(ccdr.clocks.pclk4().raw(), 100_000_000); // PCLK 100MHz

let mac_addr = smoltcp::wire::EthernetAddress::from_bytes(&MAC_ADDRESS);
let (_eth_dma, eth_mac) = unsafe {
#[allow(static_mut_refs)] // TODO: Fix this
DES_RING.write(ethernet::DesRing::new());
let (_eth_dma, eth_mac) = {
// DES_RING is located in .axisram.eth, which is not initialized by
// the runtime. We must manually initialize it to a valid value,
// without taking a reference to the uninitialized value
unsafe {
SyncUnsafeCell::raw_get(DES_RING.as_ptr())
.write(ethernet::DesRing::new());
}
// Now we can take a mutable reference to DES_RING. To avoid
// aliasing, this reference must only be taken once
let des_ring =
unsafe { &mut *SyncUnsafeCell::raw_get(DES_RING.as_ptr()) };

ethernet::new(
dp.ETHERNET_MAC,
Expand All @@ -132,8 +144,7 @@ fn main() -> ! {
rmii_txd0,
rmii_txd1,
),
#[allow(static_mut_refs)] // TODO: Fix this
DES_RING.assume_init_mut(),
des_ring,
mac_addr,
ccdr.peripheral.ETH1MAC,
&ccdr.clocks,
Expand Down
52 changes: 28 additions & 24 deletions examples/ethernet-rtic-nucleo-h723zg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@
mod utilities;

use core::mem::MaybeUninit;
use core::ptr::addr_of_mut;
use core::sync::atomic::AtomicU32;

// TODO: use core::cell::SyncUnsafeCell when stabilized rust-lang/rust#95439
use utilities::sync_unsafe_cell::SyncUnsafeCell;

use smoltcp::iface::{Config, Interface, SocketSet, SocketStorage};
use smoltcp::time::Instant;
use smoltcp::wire::{HardwareAddress, IpAddress, IpCidr};
Expand All @@ -49,17 +51,19 @@ const MAC_ADDRESS: [u8; 6] = [0x02, 0x00, 0x11, 0x22, 0x33, 0x44];

/// Ethernet descriptor rings are a global singleton
#[link_section = ".axisram.eth"]
static mut DES_RING: MaybeUninit<ethernet::DesRing<4, 4>> =
static DES_RING: MaybeUninit<SyncUnsafeCell<ethernet::DesRing<4, 4>>> =
MaybeUninit::uninit();

/// Net storage with static initialisation - another global singleton
#[derive(Default)]
pub struct NetStorageStatic<'a> {
socket_storage: [SocketStorage<'a>; 8],
}

// MaybeUninit allows us write code that is correct even if STORE is not
// initialised by the runtime
static mut STORE: MaybeUninit<NetStorageStatic> = MaybeUninit::uninit();
static STORE: MaybeUninit<SyncUnsafeCell<NetStorageStatic>> =
MaybeUninit::uninit();

pub struct Net<'a> {
iface: Interface,
Expand Down Expand Up @@ -163,9 +167,18 @@ mod app {
assert_eq!(ccdr.clocks.pclk4().raw(), 100_000_000); // PCLK 100MHz

let mac_addr = smoltcp::wire::EthernetAddress::from_bytes(&MAC_ADDRESS);
let (eth_dma, eth_mac) = unsafe {
#[allow(static_mut_refs)] // TODO: Fix this
DES_RING.write(ethernet::DesRing::new());
let (eth_dma, eth_mac) = {
// DES_RING is located in .axisram.eth, which is not initialized by
// the runtime. We must manually initialize it to a valid value,
// without taking a reference to the uninitialized value
unsafe {
SyncUnsafeCell::raw_get(DES_RING.as_ptr())
.write(ethernet::DesRing::new());
}
// Now we can take a mutable reference to DES_RING. To avoid
// aliasing, this reference must only be taken once
let des_ring =
unsafe { &mut *SyncUnsafeCell::raw_get(DES_RING.as_ptr()) };

ethernet::new(
ctx.device.ETHERNET_MAC,
Expand All @@ -182,8 +195,7 @@ mod app {
rmii_txd0,
rmii_txd1,
),
#[allow(static_mut_refs)] // TODO: Fix this
DES_RING.assume_init_mut(),
des_ring,
mac_addr,
ccdr.peripheral.ETH1MAC,
&ccdr.clocks,
Expand All @@ -198,22 +210,14 @@ mod app {

unsafe { ethernet::enable_interrupt() };

// unsafe: mutable reference to static storage, we only do this once
let store = unsafe {
#[allow(static_mut_refs)] // TODO: Fix this
let store_ptr = STORE.as_mut_ptr();

// Initialise the socket_storage field. Using `write` instead of
// assignment via `=` to not call `drop` on the old, uninitialised
// value
addr_of_mut!((*store_ptr).socket_storage)
.write([SocketStorage::EMPTY; 8]);

// Now that all fields are initialised we can safely use
// assume_init_mut to return a mutable reference to STORE
#[allow(static_mut_refs)] // TODO: Fix this
STORE.assume_init_mut()
};
// Initialize STORE
unsafe {
SyncUnsafeCell::raw_get(STORE.as_ptr())
.write(NetStorageStatic::default());
}
// Now we can take a mutable reference to STORE. To avoid aliasing, this
// reference must only be taken once
let store = unsafe { &mut *SyncUnsafeCell::raw_get(STORE.as_ptr()) };

let net = Net::new(store, eth_dma, mac_addr.into());

Expand Down
53 changes: 29 additions & 24 deletions examples/ethernet-rtic-stm32h735g-dk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@
mod utilities;

use core::mem::MaybeUninit;
use core::ptr::addr_of_mut;
use core::sync::atomic::AtomicU32;

// TODO: use core::cell::SyncUnsafeCell when stabilized rust-lang/rust#95439
use utilities::sync_unsafe_cell::SyncUnsafeCell;

use smoltcp::iface::{Config, Interface, SocketSet, SocketStorage};
use smoltcp::time::Instant;
use smoltcp::wire::{HardwareAddress, IpAddress, IpCidr};
Expand All @@ -45,16 +47,19 @@ const MAC_ADDRESS: [u8; 6] = [0x02, 0x00, 0x11, 0x22, 0x33, 0x44];

/// Ethernet descriptor rings are a global singleton
#[link_section = ".axisram.eth"]
static mut DES_RING: MaybeUninit<ethernet::DesRing<4, 4>> =
static DES_RING: MaybeUninit<SyncUnsafeCell<ethernet::DesRing<4, 4>>> =
MaybeUninit::uninit();

// This data will be held by Net through a mutable reference
#[derive(Default)]
pub struct NetStorageStatic<'a> {
socket_storage: [SocketStorage<'a>; 8],
}

// MaybeUninit allows us write code that is correct even if STORE is not
// initialised by the runtime
static mut STORE: MaybeUninit<NetStorageStatic> = MaybeUninit::uninit();
static STORE: MaybeUninit<SyncUnsafeCell<NetStorageStatic>> =
MaybeUninit::uninit();

pub struct Net<'a> {
iface: Interface,
Expand Down Expand Up @@ -157,9 +162,18 @@ mod app {
assert_eq!(ccdr.clocks.pclk4().raw(), 100_000_000); // PCLK 100MHz

let mac_addr = smoltcp::wire::EthernetAddress::from_bytes(&MAC_ADDRESS);
let (eth_dma, eth_mac) = unsafe {
#[allow(static_mut_refs)] // TODO: Fix this
DES_RING.write(ethernet::DesRing::new());
let (eth_dma, eth_mac) = {
// DES_RING is located in .axisram.eth, which is not initialized by
// the runtime. We must manually initialize it to a valid value,
// without taking a reference to the uninitialized value
unsafe {
SyncUnsafeCell::raw_get(DES_RING.as_ptr())
.write(ethernet::DesRing::new());
}
// Now we can take a mutable reference to DES_RING. To avoid
// aliasing, this reference must only be taken once
let des_ring =
unsafe { &mut *SyncUnsafeCell::raw_get(DES_RING.as_ptr()) };

ethernet::new(
ctx.device.ETHERNET_MAC,
Expand All @@ -176,8 +190,7 @@ mod app {
rmii_txd0,
rmii_txd1,
),
#[allow(static_mut_refs)] // TODO: Fix this
DES_RING.assume_init_mut(),
des_ring,
mac_addr,
ccdr.peripheral.ETH1MAC,
&ccdr.clocks,
Expand All @@ -192,22 +205,14 @@ mod app {

unsafe { ethernet::enable_interrupt() };

// unsafe: mutable reference to static storage, we only do this once
let store = unsafe {
#[allow(static_mut_refs)] // TODO: Fix this
let store_ptr = STORE.as_mut_ptr();

// Initialise the socket_storage field. Using `write` instead of
// assignment via `=` to not call `drop` on the old, uninitialised
// value
addr_of_mut!((*store_ptr).socket_storage)
.write([SocketStorage::EMPTY; 8]);

// Now that all fields are initialised we can safely use
// assume_init_mut to return a mutable reference to STORE
#[allow(static_mut_refs)] // TODO: Fix this
STORE.assume_init_mut()
};
// Initialize STORE
unsafe {
SyncUnsafeCell::raw_get(STORE.as_ptr())
.write(NetStorageStatic::default());
}
// Now we can take a mutable reference to STORE. To avoid aliasing, this
// reference must only be taken once
let store = unsafe { &mut *SyncUnsafeCell::raw_get(STORE.as_ptr()) };

let net = Net::new(store, eth_dma, mac_addr.into(), Instant::ZERO);

Expand Down
Loading

0 comments on commit aa24d24

Please sign in to comment.