Message-Id: <20250122-rust-io-offset-v1-1-914725ab55ed@kloenk.dev>
Date: Wed, 22 Jan 2025 13:38:09 +0100
From: Fiona Behrens <me@...enk.dev>
To: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, 
 Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
 Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
 Benno Lossin <benno.lossin@...ton.me>, 
 Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>, 
 Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>, 
 Daniel Almeida <daniel.almeida@...labora.com>, 
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org, 
 Fiona Behrens <me@...enk.dev>
Subject: [PATCH] rust: io: move offset_valid and io_addr(_assert) to IoRaw

Move the helper functions `offset_valid`, `io_addr` and
`io_addr_asset` from `Io` to `IoRaw`. This allows `IoRaw` to be reused
if other abstractions with different write/read functions are
needed (e.g. `writeb` vs `iowrite` vs `outb`).

Make this functions public as well so they can be used from other
modules if you aquire a `IoRaw`.

Signed-off-by: Fiona Behrens <me@...enk.dev>
---
 rust/kernel/io.rs | 98 +++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 63 insertions(+), 35 deletions(-)

diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index d4a73e52e3ee68f7b558749ed0108acde92ae5fe..a6d026f458608626113fd194ee5a8616b4ef76fe 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -15,6 +15,11 @@
 /// Instead, the bus specific MMIO implementation must convert this raw representation into an `Io`
 /// instance providing the actual memory accessors. Only by the conversion into an `Io` structure
 /// any guarantees are given.
+///
+/// # Invariant
+///
+/// `addr` plus `maxsize` has to fit in memory (smaller than [`usize::MAX`])
+/// and `maxsize` has to be smaller or equal to `SIZE`.
 pub struct IoRaw<const SIZE: usize = 0> {
     addr: usize,
     maxsize: usize,
@@ -23,7 +28,7 @@ pub struct IoRaw<const SIZE: usize = 0> {
 impl<const SIZE: usize> IoRaw<SIZE> {
     /// Returns a new `IoRaw` instance on success, an error otherwise.
     pub fn new(addr: usize, maxsize: usize) -> Result<Self> {
-        if maxsize < SIZE {
+        if maxsize < SIZE || addr.checked_add(maxsize).is_none() {
             return Err(EINVAL);
         }
 
@@ -32,15 +37,66 @@ pub fn new(addr: usize, maxsize: usize) -> Result<Self> {
 
     /// Returns the base address of the MMIO region.
     #[inline]
-    pub fn addr(&self) -> usize {
+    pub const fn addr(&self) -> usize {
         self.addr
     }
 
     /// Returns the maximum size of the MMIO region.
     #[inline]
-    pub fn maxsize(&self) -> usize {
+    pub const fn maxsize(&self) -> usize {
         self.maxsize
     }
+
+    /// Check if the offset plus the size of the type `U` fits in the bounds of `size`.
+    /// Also checks if the offset is aligned with the type size.
+    #[inline]
+    pub 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
+        } else {
+            false
+        }
+    }
+
+    /// Check if the offset (plus the type size) is out of bounds.
+    ///
+    /// Runtime checked version of [`io_addr_assert`].
+    ///
+    /// See [`offset_valid`] for the performed offset check.
+    ///
+    /// # Errors
+    ///
+    /// Returns [`EINVAL`] if the type does not fit into [`IoRaw`] at the given offset.
+    ///
+    /// [`offset_valid`]: Self::offset_valid
+    /// [`io_addr_assert`]: Self::io_addr_assert
+    #[inline]
+    pub fn io_addr<U>(&self, offset: usize) -> Result<usize> {
+        if !Self::offset_valid::<U>(offset, self.maxsize()) {
+            return Err(EINVAL);
+        }
+
+        // Probably no need to check, since the safety requirements of `Self::new` guarantee that
+        // this can't overflow.
+        self.addr().checked_add(offset).ok_or(EINVAL)
+    }
+
+    /// Check at build time if the offset (plus the type size) is out of bounds.
+    ///
+    /// Compiletime checked version of [`io_addr`].
+    ///
+    /// See [`offset_valid`] for the performed offset check.
+    ///
+    ///
+    /// [`offset_valid`]: Self::offset_valid
+    /// [`io_addr`]: Self::io_addr
+    #[inline]
+    pub const fn io_addr_assert<U>(&self, offset: usize) -> usize {
+        build_assert!(Self::offset_valid::<U>(offset, SIZE));
+
+        self.addr() + offset
+    }
 }
 
 /// IO-mapped memory, starting at the base address @addr and spanning @maxlen bytes.
@@ -116,7 +172,7 @@ macro_rules! define_read {
         $(#[$attr])*
         #[inline]
         pub fn $name(&self, offset: usize) -> $type_name {
-            let addr = self.io_addr_assert::<$type_name>(offset);
+            let addr = self.0.io_addr_assert::<$type_name>(offset);
 
             // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
             unsafe { bindings::$name(addr as _) }
@@ -128,7 +184,7 @@ pub fn $name(&self, offset: usize) -> $type_name {
         /// out of bounds.
         $(#[$attr])*
         pub fn $try_name(&self, offset: usize) -> Result<$type_name> {
-            let addr = self.io_addr::<$type_name>(offset)?;
+            let addr = self.0.io_addr::<$type_name>(offset)?;
 
             // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
             Ok(unsafe { bindings::$name(addr as _) })
@@ -145,7 +201,7 @@ macro_rules! define_write {
         $(#[$attr])*
         #[inline]
         pub fn $name(&self, value: $type_name, offset: usize) {
-            let addr = self.io_addr_assert::<$type_name>(offset);
+            let addr = self.0.io_addr_assert::<$type_name>(offset);
 
             // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
             unsafe { bindings::$name(value, addr as _, ) }
@@ -157,7 +213,7 @@ pub fn $name(&self, value: $type_name, offset: usize) {
         /// out of bounds.
         $(#[$attr])*
         pub fn $try_name(&self, value: $type_name, offset: usize) -> Result {
-            let addr = self.io_addr::<$type_name>(offset)?;
+            let addr = self.0.io_addr::<$type_name>(offset)?;
 
             // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
             unsafe { bindings::$name(value, addr as _) }
@@ -190,34 +246,6 @@ pub fn maxsize(&self) -> usize {
         self.0.maxsize()
     }
 
-    #[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
-        } else {
-            false
-        }
-    }
-
-    #[inline]
-    fn io_addr<U>(&self, offset: usize) -> Result<usize> {
-        if !Self::offset_valid::<U>(offset, self.maxsize()) {
-            return Err(EINVAL);
-        }
-
-        // Probably no need to check, since the safety requirements of `Self::new` guarantee that
-        // this can't overflow.
-        self.addr().checked_add(offset).ok_or(EINVAL)
-    }
-
-    #[inline]
-    fn io_addr_assert<U>(&self, offset: usize) -> usize {
-        build_assert!(Self::offset_valid::<U>(offset, SIZE));
-
-        self.addr() + offset
-    }
-
     define_read!(readb, try_readb, u8);
     define_read!(readw, try_readw, u16);
     define_read!(readl, try_readl, u32);

---
base-commit: 01b3cb620815fc3feb90ee117d9445a5b608a9f7
change-id: 20250122-rust-io-offset-7b39b11e84ac

Best regards,
-- 
Fiona Behrens <me@...enk.dev>

Powered by blists - more mailing lists