diff --git a/src/bare-metal/aps/better-uart/bitflags.md b/src/bare-metal/aps/better-uart/bitflags.md index 891cd2e0..ab54412b 100644 --- a/src/bare-metal/aps/better-uart/bitflags.md +++ b/src/bare-metal/aps/better-uart/bitflags.md @@ -11,5 +11,7 @@ with bitflags. - The `bitflags!` macro creates a newtype something like `Flags(u16)`, along with a bunch of method implementations to get and set flags. +- We need to derive `FromBytes` and `IntoBytes` for use with `safe-mmio`, which + we'll see on the next page. diff --git a/src/bare-metal/aps/better-uart/driver.md b/src/bare-metal/aps/better-uart/driver.md index 1661f809..488be573 100644 --- a/src/bare-metal/aps/better-uart/driver.md +++ b/src/bare-metal/aps/better-uart/driver.md @@ -8,7 +8,16 @@ Now let's use the new `Registers` struct in our driver.
-- Note the use of `&raw const` / `&raw mut` to get pointers to individual fields - without creating an intermediate reference, which would be unsound. +- `UniqueMmioPointer` is a wrapper around a raw pointer to an MMIO device or + register. The caller of `UniqueMmioPointer::new` promises that it is valid and + unique for the given lifetime, so it can provide safe methods to read and + write fields. +- These MMIO accesses are generally a wrapper around `read_volatile` and + `write_volatile`, though on aarch64 they are instead implemented in assembly + to work around a bug where the compiler can emit instructions that prevent + MMIO virtualisation. +- The `field!` and `field_shared!` macros internally use `&raw mut` and + `&raw const` to get pointers to individual fields without creating an + intermediate reference, which would be unsound.
diff --git a/src/bare-metal/aps/better-uart/registers.md b/src/bare-metal/aps/better-uart/registers.md index 48f5e5b1..cf95b665 100644 --- a/src/bare-metal/aps/better-uart/registers.md +++ b/src/bare-metal/aps/better-uart/registers.md @@ -1,6 +1,8 @@ # Multiple registers -We can use a struct to represent the memory layout of the UART's registers. +We can use a struct to represent the memory layout of the UART's registers, +using types from the `safe-mmio` crate to wrap ones which can be read or written +safely. @@ -15,5 +17,12 @@ We can use a struct to represent the memory layout of the UART's registers. rules as C. This is necessary for our struct to have a predictable layout, as default Rust representation allows the compiler to (among other things) reorder fields however it sees fit. +- There are a number of different crates providing safe abstractions around MMIO + operations; we recommend the `safe-mmio` crate. +- The difference between `ReadPure` or `ReadOnly` (and likewise between + `ReadPureWrite` and `ReadWrite`) is whether reading a register can have + side-effects which change the state of the device. E.g. reading the data + register pops a byte from the receive FIFO. `ReadPure` means that reads have + no side-effects, they are purely reading data. diff --git a/src/bare-metal/aps/examples/Cargo.lock b/src/bare-metal/aps/examples/Cargo.lock index 0ff3435f..e121e3e7 100644 --- a/src/bare-metal/aps/examples/Cargo.lock +++ b/src/bare-metal/aps/examples/Cargo.lock @@ -31,8 +31,10 @@ dependencies = [ "arm-pl011-uart", "bitflags", "log", + "safe-mmio", "smccc", "spin", + "zerocopy", ] [[package]] diff --git a/src/bare-metal/aps/examples/Cargo.toml b/src/bare-metal/aps/examples/Cargo.toml index d7cd5045..38f1de00 100644 --- a/src/bare-metal/aps/examples/Cargo.toml +++ b/src/bare-metal/aps/examples/Cargo.toml @@ -12,8 +12,10 @@ aarch64-rt = "0.1.3" arm-pl011-uart = "0.3.1" bitflags = "2.9.0" log = "0.4.27" +safe-mmio = "0.2.5" smccc = "0.2.0" spin = "0.10.0" +zerocopy = "0.8.25" [[bin]] name = "improved" diff --git a/src/bare-metal/aps/examples/src/logger.rs b/src/bare-metal/aps/examples/src/logger.rs index e27ee2b9..6e4c73ad 100644 --- a/src/bare-metal/aps/examples/src/logger.rs +++ b/src/bare-metal/aps/examples/src/logger.rs @@ -21,7 +21,7 @@ use spin::mutex::SpinMutex; static LOGGER: Logger = Logger { uart: SpinMutex::new(None) }; struct Logger { - uart: SpinMutex>, + uart: SpinMutex>>, } impl Log for Logger { @@ -43,7 +43,10 @@ impl Log for Logger { } /// Initialises UART logger. -pub fn init(uart: Uart, max_level: LevelFilter) -> Result<(), SetLoggerError> { +pub fn init( + uart: Uart<'static>, + max_level: LevelFilter, +) -> Result<(), SetLoggerError> { LOGGER.uart.lock().replace(uart); log::set_logger(&LOGGER)?; diff --git a/src/bare-metal/aps/examples/src/main_improved.rs b/src/bare-metal/aps/examples/src/main_improved.rs index 90996e3a..dfb32f4e 100644 --- a/src/bare-metal/aps/examples/src/main_improved.rs +++ b/src/bare-metal/aps/examples/src/main_improved.rs @@ -22,19 +22,22 @@ mod pl011; use crate::pl011::Uart; use core::fmt::Write; use core::panic::PanicInfo; +use core::ptr::NonNull; use log::error; +use safe_mmio::UniqueMmioPointer; use smccc::psci::system_off; use smccc::Hvc; /// Base address of the primary PL011 UART. -const PL011_BASE_ADDRESS: *mut u32 = 0x900_0000 as _; +const PL011_BASE_ADDRESS: NonNull = + NonNull::new(0x900_0000 as _).unwrap(); // SAFETY: There is no other global function of this name. #[unsafe(no_mangle)] extern "C" fn main(x0: u64, x1: u64, x2: u64, x3: u64) { // SAFETY: `PL011_BASE_ADDRESS` is the base address of a PL011 device, and // nothing else accesses that address range. - let mut uart = unsafe { Uart::new(PL011_BASE_ADDRESS) }; + let mut uart = unsafe { Uart::new(UniqueMmioPointer::new(PL011_BASE_ADDRESS)) }; writeln!(uart, "main({x0:#x}, {x1:#x}, {x2:#x}, {x3:#x})").unwrap(); diff --git a/src/bare-metal/aps/examples/src/main_logger.rs b/src/bare-metal/aps/examples/src/main_logger.rs index e6abb186..843ad734 100644 --- a/src/bare-metal/aps/examples/src/main_logger.rs +++ b/src/bare-metal/aps/examples/src/main_logger.rs @@ -22,19 +22,22 @@ mod pl011; use crate::pl011::Uart; use core::panic::PanicInfo; +use core::ptr::NonNull; use log::{error, info, LevelFilter}; +use safe_mmio::UniqueMmioPointer; use smccc::psci::system_off; use smccc::Hvc; /// Base address of the primary PL011 UART. -const PL011_BASE_ADDRESS: *mut u32 = 0x900_0000 as _; +const PL011_BASE_ADDRESS: NonNull = + NonNull::new(0x900_0000 as _).unwrap(); // SAFETY: There is no other global function of this name. #[unsafe(no_mangle)] extern "C" fn main(x0: u64, x1: u64, x2: u64, x3: u64) { // SAFETY: `PL011_BASE_ADDRESS` is the base address of a PL011 device, and // nothing else accesses that address range. - let uart = unsafe { Uart::new(PL011_BASE_ADDRESS) }; + let uart = unsafe { Uart::new(UniqueMmioPointer::new(PL011_BASE_ADDRESS)) }; logger::init(uart, LevelFilter::Trace).unwrap(); info!("main({x0:#x}, {x1:#x}, {x2:#x}, {x3:#x})"); diff --git a/src/bare-metal/aps/examples/src/pl011.rs b/src/bare-metal/aps/examples/src/pl011.rs index 2aef3621..2db50206 100644 --- a/src/bare-metal/aps/examples/src/pl011.rs +++ b/src/bare-metal/aps/examples/src/pl011.rs @@ -17,12 +17,15 @@ use core::fmt::{self, Write}; // ANCHOR: Flags use bitflags::bitflags; +use zerocopy::{FromBytes, IntoBytes}; + +/// Flags from the UART flag register. +#[repr(transparent)] +#[derive(Copy, Clone, Debug, Eq, FromBytes, IntoBytes, PartialEq)] +struct Flags(u16); bitflags! { - /// Flags from the UART flag register. - #[repr(transparent)] - #[derive(Copy, Clone, Debug, Eq, PartialEq)] - struct Flags: u16 { + impl Flags: u16 { /// Clear to send. const CTS = 1 << 0; /// Data set ready. @@ -45,11 +48,13 @@ bitflags! { } // ANCHOR_END: Flags +/// Flags from the UART Receive Status Register / Error Clear Register. +#[repr(transparent)] +#[derive(Copy, Clone, Debug, Eq, FromBytes, IntoBytes, PartialEq)] +struct ReceiveStatus(u16); + bitflags! { - /// Flags from the UART Receive Status Register / Error Clear Register. - #[repr(transparent)] - #[derive(Copy, Clone, Debug, Eq, PartialEq)] - struct ReceiveStatus: u16 { + impl ReceiveStatus: u16 { /// Framing error. const FE = 1 << 0; /// Parity error. @@ -62,70 +67,64 @@ bitflags! { } // ANCHOR: Registers +use safe_mmio::fields::{ReadPure, ReadPureWrite, ReadWrite, WriteOnly}; + #[repr(C, align(4))] -struct Registers { - dr: u16, +pub struct Registers { + dr: ReadWrite, _reserved0: [u8; 2], - rsr: ReceiveStatus, + rsr: ReadPure, _reserved1: [u8; 19], - fr: Flags, + fr: ReadPure, _reserved2: [u8; 6], - ilpr: u8, + ilpr: ReadPureWrite, _reserved3: [u8; 3], - ibrd: u16, + ibrd: ReadPureWrite, _reserved4: [u8; 2], - fbrd: u8, + fbrd: ReadPureWrite, _reserved5: [u8; 3], - lcr_h: u8, + lcr_h: ReadPureWrite, _reserved6: [u8; 3], - cr: u16, + cr: ReadPureWrite, _reserved7: [u8; 3], - ifls: u8, + ifls: ReadPureWrite, _reserved8: [u8; 3], - imsc: u16, + imsc: ReadPureWrite, _reserved9: [u8; 2], - ris: u16, + ris: ReadPure, _reserved10: [u8; 2], - mis: u16, + mis: ReadPure, _reserved11: [u8; 2], - icr: u16, + icr: WriteOnly, _reserved12: [u8; 2], - dmacr: u8, + dmacr: ReadPureWrite, _reserved13: [u8; 3], } // ANCHOR_END: Registers // ANCHOR: Uart +use safe_mmio::{field, field_shared, UniqueMmioPointer}; + /// Driver for a PL011 UART. #[derive(Debug)] -pub struct Uart { - registers: *mut Registers, +pub struct Uart<'a> { + registers: UniqueMmioPointer<'a, Registers>, } -impl Uart { - /// Constructs a new instance of the UART driver for a PL011 device at the - /// given base address. - /// - /// # Safety - /// - /// The given base address must point to the 8 MMIO control registers of a - /// PL011 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> Uart<'a> { + /// Constructs a new instance of the UART driver for a PL011 device with the + /// given set of registers. + pub fn new(registers: UniqueMmioPointer<'a, Registers>) -> Self { + Self { registers } } /// Writes a single byte to the UART. - pub fn write_byte(&self, byte: u8) { + pub fn write_byte(&mut self, byte: u8) { // Wait until there is room in the TX buffer. while self.read_flag_register().contains(Flags::TXFF) {} - // SAFETY: We know that self.registers points to the control registers - // of a PL011 device which is appropriately mapped. - unsafe { - // Write to the TX buffer. - (&raw mut (*self.registers).dr).write_volatile(byte.into()); - } + // Write to the TX buffer. + field!(self.registers, dr).write(byte.into()); // Wait until the UART is no longer busy. while self.read_flag_register().contains(Flags::BUSY) {} @@ -133,27 +132,23 @@ impl Uart { /// Reads and returns a pending byte, or `None` if nothing has been /// received. - pub fn read_byte(&self) -> Option { + pub fn read_byte(&mut self) -> Option { if self.read_flag_register().contains(Flags::RXFE) { None } else { - // SAFETY: We know that self.registers points to the control - // registers of a PL011 device which is appropriately mapped. - let data = unsafe { (&raw const (*self.registers).dr).read_volatile() }; + let data = field!(self.registers, dr).read(); // TODO: Check for error conditions in bits 8-11. Some(data as u8) } } fn read_flag_register(&self) -> Flags { - // SAFETY: We know that self.registers points to the control registers - // of a PL011 device which is appropriately mapped. - unsafe { (&raw const (*self.registers).fr).read_volatile() } + field_shared!(self.registers, fr).read() } } // ANCHOR_END: Uart -impl Write for Uart { +impl Write for Uart<'_> { fn write_str(&mut self, s: &str) -> fmt::Result { for c in s.as_bytes() { self.write_byte(*c); @@ -161,7 +156,3 @@ impl Write for Uart { Ok(()) } } - -// Safe because it just contains a pointer to device memory, which can be -// accessed from any context. -unsafe impl Send for Uart {}