The current safety comment on `io_read`/`io_write` does not cover the topic about alignment. Add it so it can be relied on by implementor of `IoCapable`.
Expand the check performed by `Io` by taking `self.addr()` into consideration when checking if `offset` is aligned. For the compile-time `io_addr_assert` check, check using the known minimum alignment of `Io::Target` and the accessed type. While at it, fix the alignment check to use `align_of` instead of `size_of`. The values match for all primitives (including u64, given that we do not provide u64 accessor on 32-bit platforms), but are not necessarily true for custom types. Signed-off-by: Gary Guo <[email protected]> --- rust/kernel/io.rs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs index d1c5f0121994..d821ee48ed31 100644 --- a/rust/kernel/io.rs +++ b/rust/kernel/io.rs @@ -195,13 +195,14 @@ pub fn maxsize(&self) -> usize { #[repr(transparent)] pub struct Mmio<const SIZE: usize = 0>(MmioRaw<SIZE>); -/// Checks whether an access of type `U` at the given `offset` +/// Checks whether an access of type `U` at the given `base` and the given `offset` /// is valid within this region. +/// +/// The `base` is used for alignment checking only. This can be set to 0 to skip the check. #[inline] -const fn offset_valid<U>(offset: usize, size: usize) -> bool { - let type_size = core::mem::size_of::<U>(); - if let Some(end) = offset.checked_add(type_size) { - end <= size && offset % type_size == 0 +const fn offset_valid<U>(base: usize, offset: usize, size: usize) -> bool { + if let Some(end) = offset.checked_add(size_of::<U>()) { + end <= size && (base.wrapping_add(offset) % align_of::<U>() == 0) } else { false } @@ -220,14 +221,16 @@ pub trait IoCapable<T> { /// /// # Safety /// - /// The range `[address..address + size_of::<T>()]` must be within the bounds of `Self`. + /// - The range `[address..address + size_of::<T>()]` must be within the bounds of `Self`. + /// - `address` must be aligned. unsafe fn io_read(&self, address: usize) -> T; /// Performs an I/O write of `value` at `address`. /// /// # Safety /// - /// The range `[address..address + size_of::<T>()]` must be within the bounds of `Self`. + /// - The range `[address..address + size_of::<T>()]` must be within the bounds of `Self`. + /// - `address` must be aligned. unsafe fn io_write(&self, value: T, address: usize); } @@ -309,7 +312,11 @@ pub trait Io { // Always inline to optimize out error path of `build_assert`. #[inline(always)] fn io_addr_assert<U>(&self, offset: usize) -> usize { - build_assert!(offset_valid::<U>(offset, Self::Target::MIN_SIZE)); + // We cannot check alignment with `offset_valid` using `self.addr()`. So set 0 for it and + // ensure alignment by checking that the alignment of `U` is smaller or equal to the + // alignment of `Self::Target`. + const_assert!(Alignment::of::<U>().as_usize() <= Self::Target::MIN_ALIGN.as_usize()); + build_assert!(offset_valid::<U>(0, offset, Self::Target::MIN_SIZE)); self.addr() + offset } @@ -318,7 +325,7 @@ fn io_addr_assert<U>(&self, offset: usize) -> usize { /// performing runtime bound checks. #[inline] fn io_addr<U>(&self, offset: usize) -> Result<usize> { - if !offset_valid::<U>(offset, self.maxsize()) { + if !offset_valid::<U>(self.addr(), offset, self.maxsize()) { return Err(EINVAL); } -- 2.54.0
