From 5fa060546b9dcf8b9d9ad82b04f5feedce0a7523 Mon Sep 17 00:00:00 2001 From: Andrew Walbran Date: Fri, 16 May 2025 13:40:36 +0000 Subject: [PATCH] Use safe-mmio in RTC exercise solution. --- src/exercises/bare-metal/rtc/Cargo.lock | 2 + src/exercises/bare-metal/rtc/Cargo.toml | 2 + src/exercises/bare-metal/rtc/src/main.rs | 5 +- src/exercises/bare-metal/rtc/src/pl031.rs | 71 +++++++------------ .../bare-metal/solutions-afternoon.md | 2 +- 5 files changed, 34 insertions(+), 48 deletions(-) diff --git a/src/exercises/bare-metal/rtc/Cargo.lock b/src/exercises/bare-metal/rtc/Cargo.lock index 409be740..f95f1574 100644 --- a/src/exercises/bare-metal/rtc/Cargo.lock +++ b/src/exercises/bare-metal/rtc/Cargo.lock @@ -157,8 +157,10 @@ dependencies = [ "bitflags", "chrono", "log", + "safe-mmio", "smccc", "spin", + "zerocopy", ] [[package]] diff --git a/src/exercises/bare-metal/rtc/Cargo.toml b/src/exercises/bare-metal/rtc/Cargo.toml index aac094a2..48a2f4e5 100644 --- a/src/exercises/bare-metal/rtc/Cargo.toml +++ b/src/exercises/bare-metal/rtc/Cargo.toml @@ -14,5 +14,7 @@ arm-pl011-uart = "0.3.1" bitflags = "2.9.0" chrono = { version = "0.4.41", default-features = false } log = "0.4.27" +safe-mmio = "0.2.5" smccc = "0.2.1" spin = "0.10.0" +zerocopy = "0.8.25" diff --git a/src/exercises/bare-metal/rtc/src/main.rs b/src/exercises/bare-metal/rtc/src/main.rs index 55b813da..cabed1f9 100644 --- a/src/exercises/bare-metal/rtc/src/main.rs +++ b/src/exercises/bare-metal/rtc/src/main.rs @@ -72,7 +72,8 @@ initial_pagetable!({ // ANCHOR_END: imports /// Base address of the PL031 RTC. -const PL031_BASE_ADDRESS: *mut u32 = 0x901_0000 as _; +const PL031_BASE_ADDRESS: NonNull = + NonNull::new(0x901_0000 as _).unwrap(); /// The IRQ used by the PL031 RTC. const PL031_IRQ: IntId = IntId::spi(2); @@ -96,7 +97,7 @@ fn main(x0: u64, x1: u64, x2: u64, x3: u64) -> ! { // SAFETY: `PL031_BASE_ADDRESS` is the base address of a PL031 device, and // nothing else accesses that address range. - let mut rtc = unsafe { Rtc::new(PL031_BASE_ADDRESS) }; + let mut rtc = unsafe { Rtc::new(UniqueMmioPointer::new(PL031_BASE_ADDRESS)) }; let timestamp = rtc.read(); let time = Utc.timestamp_opt(timestamp.into(), 0).unwrap(); info!("RTC: {time}"); diff --git a/src/exercises/bare-metal/rtc/src/pl031.rs b/src/exercises/bare-metal/rtc/src/pl031.rs index 842e7440..bd76dfc3 100644 --- a/src/exercises/bare-metal/rtc/src/pl031.rs +++ b/src/exercises/bare-metal/rtc/src/pl031.rs @@ -12,72 +12,63 @@ // See the License for the specific language governing permissions and // limitations under the License. +use safe_mmio::fields::{ReadPure, ReadPureWrite, WriteOnly}; +use safe_mmio::{field, field_shared, UniqueMmioPointer}; + // ANCHOR: solution #[repr(C, align(4))] -struct Registers { +pub struct Registers { /// Data register - dr: u32, + dr: ReadPure, /// Match register - mr: u32, + mr: ReadPureWrite, /// Load register - lr: u32, + lr: ReadPureWrite, /// Control register - cr: u8, + cr: ReadPureWrite, _reserved0: [u8; 3], /// Interrupt Mask Set or Clear register - imsc: u8, + imsc: ReadPureWrite, _reserved1: [u8; 3], /// Raw Interrupt Status - ris: u8, + ris: ReadPure, _reserved2: [u8; 3], /// Masked Interrupt Status - mis: u8, + mis: ReadPure, _reserved3: [u8; 3], /// Interrupt Clear Register - icr: u8, + icr: WriteOnly, _reserved4: [u8; 3], } /// Driver for a PL031 real-time clock. #[derive(Debug)] -pub struct Rtc { - registers: *mut Registers, +pub struct Rtc<'a> { + registers: UniqueMmioPointer<'a, Registers>, } -impl Rtc { - /// Constructs a new instance of the RTC driver for a PL031 device at the - /// given base address. - /// - /// # Safety - /// - /// The given base address must point to the MMIO control registers of a - /// PL031 device, which must be mapped into the address space of the process - /// as device memory and not have any other aliases. - pub unsafe fn new(base_address: *mut u32) -> Self { - Self { registers: base_address as *mut Registers } +impl<'a> Rtc<'a> { + /// Constructs a new instance of the RTC driver for a PL031 device with the + /// given set of registers. + pub unsafe fn new(registers: UniqueMmioPointer<'a, Registers>) -> Self { + Self { registers } } /// Reads the current RTC value. pub fn read(&self) -> u32 { - // SAFETY: We know that self.registers points to the control registers - // of a PL031 device which is appropriately mapped. - unsafe { (&raw const (*self.registers).dr).read_volatile() } + field_shared!(self.registers, dr).read() } /// Writes a match value. When the RTC value matches this then an interrupt /// will be generated (if it is enabled). pub fn set_match(&mut self, value: u32) { - // SAFETY: We know that self.registers points to the control registers - // of a PL031 device which is appropriately mapped. - unsafe { (&raw mut (*self.registers).mr).write_volatile(value) } + field!(self.registers, mr).write(value); } /// Returns whether the match register matches the RTC value, whether or not /// the interrupt is enabled. pub fn matched(&self) -> bool { - // SAFETY: We know that self.registers points to the control registers - // of a PL031 device which is appropriately mapped. - let ris = unsafe { (&raw const (*self.registers).ris).read_volatile() }; + let ris = field_shared!(self.registers, ris).read(); (ris & 0x01) != 0 } @@ -86,10 +77,8 @@ impl Rtc { /// This should be true if and only if `matched` returns true and the /// interrupt is masked. pub fn interrupt_pending(&self) -> bool { - // SAFETY: We know that self.registers points to the control registers - // of a PL031 device which is appropriately mapped. - let ris = unsafe { (&raw const (*self.registers).mis).read_volatile() }; - (ris & 0x01) != 0 + let mis = field_shared!(self.registers, mis).read(); + (mis & 0x01) != 0 } /// Sets or clears the interrupt mask. @@ -98,19 +87,11 @@ impl Rtc { /// interrupt is disabled. pub fn enable_interrupt(&mut self, mask: bool) { let imsc = if mask { 0x01 } else { 0x00 }; - // SAFETY: We know that self.registers points to the control registers - // of a PL031 device which is appropriately mapped. - unsafe { (&raw mut (*self.registers).imsc).write_volatile(imsc) } + field!(self.registers, imsc).write(imsc); } /// Clears a pending interrupt, if any. pub fn clear_interrupt(&mut self) { - // SAFETY: We know that self.registers points to the control registers - // of a PL031 device which is appropriately mapped. - unsafe { (&raw mut (*self.registers).icr).write_volatile(0x01) } + field!(self.registers, icr).write(0x01); } } - -// SAFETY: `Rtc` just contains a pointer to device memory, which can be -// accessed from any context. -unsafe impl Send for Rtc {} diff --git a/src/exercises/bare-metal/solutions-afternoon.md b/src/exercises/bare-metal/solutions-afternoon.md index 38f85d1c..fbb079b6 100644 --- a/src/exercises/bare-metal/solutions-afternoon.md +++ b/src/exercises/bare-metal/solutions-afternoon.md @@ -12,6 +12,6 @@ _main.rs_: _pl031.rs_: -```rust +```rust,compile_fail {{#include rtc/src/pl031.rs:solution}} ```