Skip to content

Commit 71d6582

Browse files
authored
Merge pull request #50 from phip1611/safety
Enhance MmioAddress safety and fix AArch64 instruction emission
2 parents 62153d6 + 996c549 commit 71d6582

File tree

7 files changed

+104
-25
lines changed

7 files changed

+104
-25
lines changed

.github/workflows/rust.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ jobs:
1616
fail-fast: false
1717
matrix:
1818
runs-on:
19-
- macos-latest
20-
- ubuntu-latest
21-
- windows-latest
19+
- macos-latest # aarch64
20+
- ubuntu-latest # x86_64
21+
- windows-latest # x86_64
2222
rust:
2323
- stable
2424
- nightly

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
- New public methods:
88
- `Uart16550::ready_to_receive()`
99
- `Uart16550::ready_to_send()`
10+
- Documentation improvements
11+
- Internal safety fixes (there was no UB, just making the internal code more
12+
robust)
1013

1114
## 0.5.0 - 2026-03-20
1215

README.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,23 @@ support for MMIO-mapped devices.
1414
Serial ports are especially useful for debugging or operating system
1515
learning projects. See [`Uart16550`] to get started.
1616

17+
[`Uart16550`]: https://docs.rs/uart_16550/latest/uart_16550/struct.Uart16550.html
18+
1719
## Features
1820

1921
- ✅ Full configure, transmit, receive, and interrupt support for UART
2022
16550–compatible devices
2123
- ✅ High-level, ergonomic abstractions and types paired with support for
2224
plain integers
23-
-Very easy to integrate, highly configurable when needed
25+
-Straightforward to integrate, highly configurable when needed
2426
- ✅ Validated on **real hardware** as well as across different virtual
2527
machines
2628
- ✅ Fully type-safe and derived directly from the official
2729
[specification][uart]
2830
- ✅ Supports both **x86 port-mapped I/O** and **memory-mapped I/O** (MMIO)
2931
-`no_std`-compatible and allocation-free by design
32+
- ✅ Compatible with all architectures supported by Rust (x86/x86_64, ARM,
33+
RISC-V, ...)
3034

3135
## Focus, Scope & Limitations
3236

src/backend.rs

Lines changed: 76 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,20 @@ use crate::spec::NUM_REGISTERS;
1212
use core::arch::asm;
1313
use core::fmt::Debug;
1414
use core::num::NonZeroU8;
15-
use core::ptr::{self, read_volatile, write_volatile};
15+
use core::ptr::NonNull;
1616

1717
mod private {
1818
pub trait Sealed {}
1919
}
2020

2121
/// Abstraction over register addresses in [`Backend`].
22+
///
23+
/// # Safety
24+
///
25+
/// All implementations and instances of this trait are created within this
26+
/// crate and do follow all safety invariants. API users don't get access to the
27+
/// underlying register addresses, nor can they construct one themselves, as this
28+
/// type et al. are sealed.
2229
pub trait RegisterAddress: Copy + Clone + Debug + Sized + private::Sealed {
2330
/// Adds a byte offset onto the base register address.
2431
fn add_offset(self, offset: u8) -> Self;
@@ -49,7 +56,7 @@ impl private::Sealed for PortIoAddress {}
4956
///
5057
/// See [`RegisterAddress`].
5158
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Hash)]
52-
pub struct MmioAddress(pub(crate) *mut u8);
59+
pub struct MmioAddress(pub(crate) NonNull<u8>);
5360

5461
// SAFETY: `Uart16550` is not `Sync`, so concurrent access from multiple
5562
// threads is not possible through this type's API alone. Implementing `Send`
@@ -239,6 +246,67 @@ impl Backend for PioBackend {
239246
}
240247
}
241248

249+
/// Arch-specific quirks to access hardware.
250+
///
251+
/// On MMIO-access on aarch64, LLVM may emit instructions that are not properly
252+
/// virtualizable. We therefore need to be more explicit about the instruction.
253+
/// More info: <https://github.com/rust-lang/rust/issues/131894>
254+
mod arch {
255+
use crate::backend::MmioAddress;
256+
#[cfg(any(doc, not(target_arch = "aarch64")))]
257+
use core::ptr;
258+
259+
/// Wrapper around [`ptr::read_volatile`].
260+
#[cfg(target_arch = "aarch64")]
261+
#[inline(always)]
262+
pub unsafe fn mmio_read_register(address: MmioAddress) -> u8 {
263+
let ptr = address.0.as_ptr();
264+
let ret: u8;
265+
// SAFETY: Caller ensures the address is valid MMIO memory.
266+
unsafe {
267+
core::arch::asm!(
268+
"ldrb {ret:w}, [{ptr}]",
269+
ptr = in(reg) ptr,
270+
ret = out(reg) ret,
271+
options(nostack, preserves_flags)
272+
);
273+
}
274+
ret
275+
}
276+
277+
/// Wrapper around [`ptr::read_volatile`].
278+
#[cfg(not(target_arch = "aarch64"))]
279+
#[inline(always)]
280+
pub unsafe fn mmio_read_register(address: MmioAddress) -> u8 {
281+
// SAFETY: Caller ensures the address is valid MMIO memory.
282+
unsafe { ptr::read_volatile(address.0.as_ptr()) }
283+
}
284+
285+
/// Wrapper around [`ptr::write_volatile`].
286+
#[cfg(target_arch = "aarch64")]
287+
#[inline(always)]
288+
pub unsafe fn mmio_write_register(address: MmioAddress, value: u8) {
289+
let ptr = address.0.as_ptr();
290+
// SAFETY: Caller ensures the address is valid MMIO memory.
291+
unsafe {
292+
core::arch::asm!(
293+
"strb {val:w}, [{ptr}]",
294+
val = in(reg) value,
295+
ptr = in(reg) ptr,
296+
options(nostack, preserves_flags)
297+
);
298+
}
299+
}
300+
301+
/// Wrapper around [`ptr::write_volatile`].
302+
#[cfg(not(target_arch = "aarch64"))]
303+
#[inline(always)]
304+
pub unsafe fn mmio_write_register(address: MmioAddress, value: u8) {
305+
// SAFETY: Caller ensures the address is valid MMIO memory.
306+
unsafe { ptr::write_volatile(address.0.as_ptr(), value) }
307+
}
308+
}
309+
242310
/// MMIO-mapped UART 16550.
243311
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Hash)]
244312
pub struct MmioBackend {
@@ -264,23 +332,23 @@ impl Backend for MmioBackend {
264332

265333
#[inline(always)]
266334
unsafe fn _read_register(&mut self, address: MmioAddress) -> u8 {
267-
debug_assert_ne!(address.0, ptr::null_mut());
268335
debug_assert!(address >= self.base());
269336
let upper_bound_incl = (NUM_REGISTERS - 1) * usize::from(u8::from(self.stride));
270-
debug_assert!(address.0 <= self.base().0.wrapping_add(upper_bound_incl));
337+
// Address is in the device's address range
338+
debug_assert!(address.0.as_ptr() <= self.base().0.as_ptr().wrapping_add(upper_bound_incl));
271339

272340
// SAFETY: The caller ensured that the MMIO address is safe to use.
273-
unsafe { read_volatile(address.0) }
341+
unsafe { arch::mmio_read_register(address) }
274342
}
275343

276344
#[inline(always)]
277345
unsafe fn _write_register(&mut self, address: MmioAddress, value: u8) {
278-
debug_assert_ne!(address.0, ptr::null_mut());
279346
debug_assert!(address >= self.base());
280347
let upper_bound_incl = (NUM_REGISTERS - 1) * usize::from(u8::from(self.stride));
281-
debug_assert!(address.0 <= self.base().0.wrapping_add(upper_bound_incl));
348+
// Address is in the device's address range
349+
debug_assert!(address.0.as_ptr() <= self.base().0.as_ptr().wrapping_add(upper_bound_incl));
282350

283351
// SAFETY: The caller ensured that the MMIO address is safe to use.
284-
unsafe { write_volatile(address.0, value) }
352+
unsafe { arch::mmio_write_register(address, value) }
285353
}
286354
}

src/error.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@ use core::fmt::Display;
1616
/// [NUM_REGISTERS]: crate::spec::NUM_REGISTERS
1717
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
1818
pub enum InvalidAddressError<A: RegisterAddress> {
19-
/// The specified address is invalid because it is either null or doesn't allow
20-
/// for <code>[NUM_REGISTERS] - 1</code> subsequent addresses.
19+
/// The specified address is null.
20+
Null,
21+
/// The given base address is invalid, e.g., it cannot accommodate
22+
/// <code>[NUM_REGISTERS] - 1</code> consecutive addresses.
2123
///
2224
/// [NUM_REGISTERS]: crate::spec::NUM_REGISTERS
2325
InvalidBaseAddress(A),
@@ -31,6 +33,9 @@ pub enum InvalidAddressError<A: RegisterAddress> {
3133
impl<A: RegisterAddress> Display for InvalidAddressError<A> {
3234
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
3335
match self {
36+
Self::Null => {
37+
write!(f, "address is null")
38+
}
3439
Self::InvalidBaseAddress(addr) => {
3540
write!(f, "invalid register address: {addr:x?}")
3641
}

src/lib.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,15 @@
2222
//! 16550–compatible devices
2323
//! - ✅ High-level, ergonomic abstractions and types paired with support for
2424
//! plain integers
25-
//! - ✅ Very easy to integrate, highly configurable when needed
25+
//! - ✅ Straightforward to integrate, highly configurable when needed
2626
//! - ✅ Validated on **real hardware** as well as across different virtual
2727
//! machines
2828
//! - ✅ Fully type-safe and derived directly from the official
2929
//! [specification][uart]
3030
//! - ✅ Supports both **x86 port-mapped I/O** and **memory-mapped I/O** (MMIO)
3131
//! - ✅ `no_std`-compatible and allocation-free by design
32+
//! - ✅ Compatible with all architectures supported by Rust (x86/x86_64, ARM,
33+
//! RISC-V, ...)
3234
//!
3335
//! ## Focus, Scope & Limitations
3436
//!
@@ -147,6 +149,7 @@ use crate::spec::{FIFO_SIZE, NUM_REGISTERS, calc_baud_rate, calc_divisor};
147149
use core::cmp;
148150
use core::hint;
149151
use core::num::NonZeroU8;
152+
use core::ptr::NonNull;
150153

151154
pub mod backend;
152155
pub mod spec;
@@ -291,16 +294,14 @@ impl Uart16550<MmioBackend> {
291294
base_address: *mut u8,
292295
stride: u8,
293296
) -> Result<Self, InvalidAddressError<MmioAddress>> {
297+
let base_address = NonNull::new(base_address).ok_or(InvalidAddressError::Null)?;
294298
let base_address = MmioAddress(base_address);
295-
if base_address.0.is_null() {
296-
return Err(InvalidAddressError::InvalidBaseAddress(base_address));
297-
}
298299

299300
if stride == 0 || !stride.is_power_of_two() {
300301
return Err(InvalidAddressError::InvalidStride(stride));
301302
}
302303

303-
if (base_address.0 as usize)
304+
if (base_address.0.as_ptr() as usize)
304305
.checked_add((NUM_REGISTERS - 1) * stride as usize)
305306
.is_none()
306307
{

src/tty.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,11 @@ impl<A: RegisterAddress + 'static> Error for Uart16550TtyError<A> {
5656
}
5757
}
5858

59-
/// Thin opinionated abstraction over [`Uart16550`] that helps to send Rust
60-
/// strings easily to the other side, assuming the remote is a TTY (terminal).
59+
/// Lightweight, opinionated wrapper around [`Uart16550`] for sending Rust
60+
/// strings to a connected TTY (terminal).
6161
///
62-
/// It is especially suited as very easy way to see something when you develop
63-
/// and test things in a VM.
64-
///
65-
/// It implements [`fmt::Write`].
62+
/// Ideal for quickly observing debug output during VM development and testing.
63+
/// It implements [`fmt::Write`] allowing the use of `write!()`.
6664
///
6765
/// # Example
6866
/// ```rust,no_run

0 commit comments

Comments
 (0)