Add section about UART driver without safe-mmio back in (#2792)

Having taught the course again since the change to use safe-mmio, it
seems worth keeping this as an intermediate step rather than jumping
straight from pointer arithmetic to safe-mmio's abstractions.

src/SUMMARY.md

@@ -357,7 +357,9 @@
     - [Bitflags](bare-metal/aps/better-uart/bitflags.md)
     - [Multiple Registers](bare-metal/aps/better-uart/registers.md)
     - [Driver](bare-metal/aps/better-uart/driver.md)
-    - [Using It](bare-metal/aps/better-uart/using.md)
+  - [safe-mmio](bare-metal/aps/safemmio/registers.md)
+    - [Driver](bare-metal/aps/safemmio/driver.md)
+    - [Using It](bare-metal/aps/safemmio/using.md)
   - [Logging](bare-metal/aps/logging.md)
     - [Using It](bare-metal/aps/logging/using.md)
   - [Exceptions](bare-metal/aps/exceptions.md)

src/bare-metal/aps/better-uart/bitflags.md

@@ -4,14 +4,12 @@ The [`bitflags`](https://crates.io/crates/bitflags) crate is useful for working
 with bitflags.
 
 ```rust,editable,compile_fail
-{{#include ../examples/src/pl011.rs:Flags}}
+{{#include ../examples/src/pl011_struct.rs:Flags}}
 ```
 
 <details>
 
-- The `bitflags!` macro creates a newtype something like `Flags(u16)`, along
+- The `bitflags!` macro creates a newtype something like `struct Flags(u16)`,
-  with a bunch of method implementations to get and set flags.
+  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.
 
 </details>

src/bare-metal/aps/better-uart/driver.md

@@ -3,22 +3,15 @@
 Now let's use the new `Registers` struct in our driver.
 
 ```rust,editable,compile_fail
-{{#include ../examples/src/pl011.rs:Uart}}
+{{#include ../examples/src/pl011_struct.rs:Uart}}
 ```
 
 <details>
 
-- `UniqueMmioPointer` is a wrapper around a raw pointer to an MMIO device or
+- Note the use of `&raw const` / `&raw mut` to get pointers to individual fields
-  register. The caller of `UniqueMmioPointer::new` promises that it is valid and
+  without creating an intermediate reference, which would be unsound.
-  unique for the given lifetime, so it can provide safe methods to read and
+- The example isn't included in the slides because it is very similar to the
-  write fields.
+  `safe-mmio` example which comes next. You can run it in QEMU with `make qemu`
-- Note that `Uart::new` is now safe; `UniqueMmioPointer::new` is unsafe instead.
+  under `src/bare-metal/aps/examples` if you need to.
-- 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.
 
 </details>

src/bare-metal/aps/better-uart/registers.md

@@ -1,13 +1,11 @@
 # 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.
 
 <!-- mdbook-xgettext: skip -->
 
 ```rust,editable,compile_fail
-{{#include ../examples/src/pl011.rs:Registers}}
+{{#include ../examples/src/pl011_struct.rs:Registers}}
 ```
 
 <details>
@@ -17,12 +15,5 @@ safely.
   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.
 
 </details>

src/bare-metal/aps/examples/Cargo.toml

@@ -29,10 +29,14 @@ path = "src/main_logger.rs"
 name = "minimal"
 path = "src/main_minimal.rs"
 
+[[bin]]
+name = "psci"
+path = "src/main_psci.rs"
+
 [[bin]]
 name = "rt"
 path = "src/main_rt.rs"
 
 [[bin]]
-name = "psci"
+name = "safemmio"
-path = "src/main_psci.rs"
+path = "src/main_safemmio.rs"

src/bare-metal/aps/examples/Makefile

@@ -29,6 +29,8 @@ psci.bin: build
 	cargo objcopy --bin psci -- -O binary $@
 rt.bin: build
 	cargo objcopy --bin rt -- -O binary $@
+safemmio.bin: build
+	cargo objcopy --bin safemmio -- -O binary $@
 
 qemu: improved.bin
 	qemu-system-aarch64 -machine virt -cpu max -serial mon:stdio -display none -kernel $< -s
@@ -40,6 +42,8 @@ qemu_psci: psci.bin
 	qemu-system-aarch64 -machine virt -cpu max -serial mon:stdio -display none -kernel $< -s
 qemu_rt: rt.bin
 	qemu-system-aarch64 -machine virt -cpu max -serial mon:stdio -display none -kernel $< -s
+qemu_safemmio: safemmio.bin
+	qemu-system-aarch64 -machine virt -cpu max -serial mon:stdio -display none -kernel $< -s
 
 clean:
 	cargo clean

src/bare-metal/aps/examples/src/main_improved.rs

@@ -18,27 +18,24 @@
 
 mod asm;
 mod exceptions;
-mod pl011;
+mod pl011_struct;
 
-use crate::pl011::Uart;
+use crate::pl011_struct::Uart;
 use core::fmt::Write;
 use core::panic::PanicInfo;
-use core::ptr::NonNull;
 use log::error;
-use safe_mmio::UniqueMmioPointer;
 use smccc::Hvc;
 use smccc::psci::system_off;
 
 /// Base address of the primary PL011 UART.
-const PL011_BASE_ADDRESS: NonNull<pl011::Registers> =
+const PL011_BASE_ADDRESS: *mut pl011_struct::Registers = 0x900_0000 as _;
-    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(UniqueMmioPointer::new(PL011_BASE_ADDRESS)) };
+    let mut uart = unsafe { Uart::new(PL011_BASE_ADDRESS) };
 
     writeln!(uart, "main({x0:#x}, {x1:#x}, {x2:#x}, {x3:#x})").unwrap();
 

src/bare-metal/aps/examples/src/main_rt.rs

@@ -68,7 +68,7 @@ fn main(x0: u64, x1: u64, x2: u64, x3: u64) -> ! {
 }
 
 #[panic_handler]
-fn panic(info: &PanicInfo) -> ! {
+fn panic(_info: &PanicInfo) -> ! {
     system_off::<Hvc>().unwrap();
     loop {}
 }

src/bare-metal/aps/examples/src/main_safemmio.rs

@@ -0,0 +1,66 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// ANCHOR: main
+#![no_main]
+#![no_std]
+
+mod asm;
+mod exceptions;
+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::Hvc;
+use smccc::psci::system_off;
+
+/// Base address of the primary PL011 UART.
+const PL011_BASE_ADDRESS: NonNull<pl011::Registers> =
+    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 = Uart::new(unsafe { UniqueMmioPointer::new(PL011_BASE_ADDRESS) });
+
+    writeln!(uart, "main({x0:#x}, {x1:#x}, {x2:#x}, {x3:#x})").unwrap();
+
+    loop {
+        if let Some(byte) = uart.read_byte() {
+            uart.write_byte(byte);
+            match byte {
+                b'\r' => uart.write_byte(b'\n'),
+                b'q' => break,
+                _ => continue,
+            }
+        }
+    }
+
+    writeln!(uart, "\n\nBye!").unwrap();
+    system_off::<Hvc>().unwrap();
+}
+// ANCHOR_END: main
+
+#[panic_handler]
+fn panic(info: &PanicInfo) -> ! {
+    error!("{info}");
+    system_off::<Hvc>().unwrap();
+    loop {}
+}

src/bare-metal/aps/examples/src/pl011_struct.rs

@@ -0,0 +1,167 @@
+// Copyright 2023 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+#![allow(dead_code)]
+
+use core::fmt::{self, Write};
+
+// ANCHOR: Flags
+use bitflags::bitflags;
+
+bitflags! {
+    /// Flags from the UART flag register.
+    #[repr(transparent)]
+    #[derive(Copy, Clone, Debug, Eq, PartialEq)]
+    struct Flags: u16 {
+        /// Clear to send.
+        const CTS = 1 << 0;
+        /// Data set ready.
+        const DSR = 1 << 1;
+        /// Data carrier detect.
+        const DCD = 1 << 2;
+        /// UART busy transmitting data.
+        const BUSY = 1 << 3;
+        /// Receive FIFO is empty.
+        const RXFE = 1 << 4;
+        /// Transmit FIFO is full.
+        const TXFF = 1 << 5;
+        /// Receive FIFO is full.
+        const RXFF = 1 << 6;
+        /// Transmit FIFO is empty.
+        const TXFE = 1 << 7;
+        /// Ring indicator.
+        const RI = 1 << 8;
+    }
+}
+// ANCHOR_END: Flags
+
+bitflags! {
+    /// Flags from the UART Receive Status Register / Error Clear Register.
+    #[repr(transparent)]
+    #[derive(Copy, Clone, Debug, Eq, PartialEq)]
+    struct ReceiveStatus: u16 {
+        /// Framing error.
+        const FE = 1 << 0;
+        /// Parity error.
+        const PE = 1 << 1;
+        /// Break error.
+        const BE = 1 << 2;
+        /// Overrun error.
+        const OE = 1 << 3;
+    }
+}
+
+// ANCHOR: Registers
+#[repr(C, align(4))]
+pub struct Registers {
+    dr: u16,
+    _reserved0: [u8; 2],
+    rsr: ReceiveStatus,
+    _reserved1: [u8; 19],
+    fr: Flags,
+    _reserved2: [u8; 6],
+    ilpr: u8,
+    _reserved3: [u8; 3],
+    ibrd: u16,
+    _reserved4: [u8; 2],
+    fbrd: u8,
+    _reserved5: [u8; 3],
+    lcr_h: u8,
+    _reserved6: [u8; 3],
+    cr: u16,
+    _reserved7: [u8; 3],
+    ifls: u8,
+    _reserved8: [u8; 3],
+    imsc: u16,
+    _reserved9: [u8; 2],
+    ris: u16,
+    _reserved10: [u8; 2],
+    mis: u16,
+    _reserved11: [u8; 2],
+    icr: u16,
+    _reserved12: [u8; 2],
+    dmacr: u8,
+    _reserved13: [u8; 3],
+}
+// ANCHOR_END: Registers
+
+// ANCHOR: Uart
+/// Driver for a PL011 UART.
+#[derive(Debug)]
+pub struct Uart {
+    registers: *mut Registers,
+}
+
+impl Uart {
+    /// Constructs a new instance of the UART driver for a PL011 device with the
+    /// given set of registers.
+    ///
+    /// # Safety
+    ///
+    /// The given pointer 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(registers: *mut Registers) -> Self {
+        Self { registers }
+    }
+
+    /// Writes a single byte to the UART.
+    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());
+        }
+
+        // Wait until the UART is no longer busy.
+        while self.read_flag_register().contains(Flags::BUSY) {}
+    }
+
+    /// Reads and returns a pending byte, or `None` if nothing has been
+    /// received.
+    pub fn read_byte(&mut self) -> Option<u8> {
+        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() };
+            // 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() }
+    }
+}
+// ANCHOR_END: Uart
+
+impl Write for Uart {
+    fn write_str(&mut self, s: &str) -> fmt::Result {
+        for c in s.as_bytes() {
+            self.write_byte(*c);
+        }
+        Ok(())
+    }
+}
+
+// Safe because it just contains a pointer to device memory, which can be
+// accessed from any context.
+unsafe impl Send for Uart {}

src/bare-metal/aps/safemmio/driver.md

@@ -0,0 +1,30 @@
+# Driver
+
+Now let's use the new `Registers` struct in our driver.
+
+```rust,editable,compile_fail
+{{#include ../examples/src/pl011.rs:Uart}}
+```
+
+<details>
+
+- The driver no longer needs any unsafe code!
+- `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.
+- Note that `Uart::new` is now safe; `UniqueMmioPointer::new` is unsafe instead.
+- 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.
+- `field!` needs a mutable reference to a `UniqueMmioPointer`, and returns a
+  `UniqueMmioPointer` which allows reads with side effects and writes.
+- `field_shared!` works with a shared reference to either a `UniqueMmioPointer`
+  or a `SharedMmioPointer`. It returns a `SharedMmioPointer` which only allows
+  pure reads.
+
+</details>

src/bare-metal/aps/safemmio/registers.md

@@ -0,0 +1,37 @@
+# safe-mmio
+
+The [`safe-mmio`] crate provides types to wrap registers which can be read or
+written safely.
+
+|             | Can't read    | Read has no side-effects | Read has side-effects |
+| ----------- | ------------- | ------------------------ | --------------------- |
+| Can't write |               | [`ReadPure`]             | [`ReadOnly`]          |
+| Can write   | [`WriteOnly`] | [`ReadPureWrite`]        | [`ReadWrite`]         |
+
+<!-- mdbook-xgettext: skip -->
+
+```rust,editable,compile_fail
+{{#include ../examples/src/pl011.rs:Registers}}
+```
+
+- Reading `dr` has a side effect: it pops a byte from the receive FIFO.
+- Reading `rsr` (and other registers) has no side-effects. It is a 'pure' read.
+
+<details>
+
+- 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.
+
+</details>
+
+[`safe-mmio`]: https://crates.io/crates/safe-mmio
+[`ReadOnly`]: https://docs.rs/safe-mmio/latest/safe_mmio/fields/struct.ReadOnly.html
+[`ReadPure`]: https://docs.rs/safe-mmio/latest/safe_mmio/fields/struct.ReadPure.html
+[`ReadPureWrite`]: https://docs.rs/safe-mmio/latest/safe_mmio/fields/struct.ReadPureWrite.html
+[`ReadWrite`]: https://docs.rs/safe-mmio/latest/safe_mmio/fields/struct.ReadWrite.html
+[`WriteOnly`]: https://docs.rs/safe-mmio/latest/safe_mmio/fields/struct.WriteOnly.html

src/bare-metal/aps/safemmio/using.md

@@ -1,14 +1,15 @@
-# Using it
+# Using It
 
 Let's write a small program using our driver to write to the serial console, and
 echo incoming bytes.
 
 ```rust,editable,compile_fail
-{{#include ../examples/src/main_improved.rs:main}}
+{{#include ../examples/src/main_safemmio.rs:main}}
 ```
 
 <details>
 
-- Run the example in QEMU with `make qemu` under `src/bare-metal/aps/examples`.
+- Run the example in QEMU with `make qemu_safemmio` under
+  `src/bare-metal/aps/examples`.
 
 </details>