From: "Gary Guo" <gary@garyguo.net>
To: "Alexandre Courbot" <acourbot@nvidia.com>,
"Danilo Krummrich" <dakr@kernel.org>,
"Alice Ryhl" <aliceryhl@google.com>,
"Daniel Almeida" <daniel.almeida@collabora.com>,
"Miguel Ojeda" <ojeda@kernel.org>, "Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <lossin@kernel.org>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Trevor Gross" <tmgross@umich.edu>,
"Boqun Feng" <boqun@kernel.org>
Cc: "Yury Norov" <yury.norov@gmail.com>,
"John Hubbard" <jhubbard@nvidia.com>,
"Alistair Popple" <apopple@nvidia.com>,
"Joel Fernandes" <joelagnelf@nvidia.com>,
"Timur Tabi" <ttabi@nvidia.com>, "Edwin Peer" <epeer@nvidia.com>,
"Eliot Courtney" <ecourtney@nvidia.com>,
"Dirk Behme" <dirk.behme@de.bosch.com>,
"Steven Price" <steven.price@arm.com>,
<rust-for-linux@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v8 06/10] rust: io: use generic read/write accessors for primitive accesses
Date: Mon, 09 Mar 2026 15:29:14 +0000 [thread overview]
Message-ID: <DGYCWW4AUI1Y.29UXVO4XOFCOA@garyguo.net> (raw)
In-Reply-To: <20260310-register-v8-6-424f80dd43bc@nvidia.com>
On Mon Mar 9, 2026 at 3:14 PM GMT, Alexandre Courbot wrote:
> By providing the required `IoLoc` implementations on `usize`, we can
> leverage the generic accessors and reduce the number of unsafe blocks in
> the module.
>
> This also allows us to directly call the generic `read/write/update`
> methods with primitive types, so add examples illustrating this.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> rust/kernel/io.rs | 199 +++++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 131 insertions(+), 68 deletions(-)
>
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index 1db6572f4a42..ed6fab001a39 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -197,6 +197,25 @@ pub trait IoLoc<T> {
> fn offset(&self) -> usize;
> }
>
> +/// Implements [`IoLoc<$ty>`] for [`usize`], allowing to use `usize` as a parameter of
> +/// [`Io::read`] and [`Io::write`].
> +macro_rules! impl_usize_ioloc {
> + ($($ty:ty),*) => {
> + $(
> + impl IoLoc<$ty> for usize {
> + type IoType = $ty;
> +
#[inline(always)]
the fact that this is a pointer is somewhat uneasy to me. I wonder if Clippy
with its inlining tweak would cause optimisation failure here.
Could this be just `fn offset(self)`?
The rest LGTM.
Best,
Gary
> + fn offset(&self) -> usize {
> + *self
> + }
> + }
> + )*
> + }
> +}
> +
> +// Provide the ability to read any primitive type from a [`usize`].
> +impl_usize_ioloc!(u8, u16, u32, u64);
> +
> /// Types implementing this trait (e.g. MMIO BARs or PCI config regions)
> /// can perform I/O operations on regions of memory.
> ///
> @@ -241,10 +260,7 @@ fn try_read8(&self, offset: usize) -> Result<u8>
> where
> Self: IoCapable<u8>,
> {
> - let address = self.io_addr::<u8>(offset)?;
> -
> - // SAFETY: `address` has been validated by `io_addr`.
> - Ok(unsafe { self.io_read(address) })
> + self.try_read(offset)
> }
>
> /// Fallible 16-bit read with runtime bounds check.
> @@ -253,10 +269,7 @@ fn try_read16(&self, offset: usize) -> Result<u16>
> where
> Self: IoCapable<u16>,
> {
> - let address = self.io_addr::<u16>(offset)?;
> -
> - // SAFETY: `address` has been validated by `io_addr`.
> - Ok(unsafe { self.io_read(address) })
> + self.try_read(offset)
> }
>
> /// Fallible 32-bit read with runtime bounds check.
> @@ -265,10 +278,7 @@ fn try_read32(&self, offset: usize) -> Result<u32>
> where
> Self: IoCapable<u32>,
> {
> - let address = self.io_addr::<u32>(offset)?;
> -
> - // SAFETY: `address` has been validated by `io_addr`.
> - Ok(unsafe { self.io_read(address) })
> + self.try_read(offset)
> }
>
> /// Fallible 64-bit read with runtime bounds check.
> @@ -277,10 +287,7 @@ fn try_read64(&self, offset: usize) -> Result<u64>
> where
> Self: IoCapable<u64>,
> {
> - let address = self.io_addr::<u64>(offset)?;
> -
> - // SAFETY: `address` has been validated by `io_addr`.
> - Ok(unsafe { self.io_read(address) })
> + self.try_read(offset)
> }
>
> /// Fallible 8-bit write with runtime bounds check.
> @@ -289,11 +296,7 @@ fn try_write8(&self, value: u8, offset: usize) -> Result
> where
> Self: IoCapable<u8>,
> {
> - let address = self.io_addr::<u8>(offset)?;
> -
> - // SAFETY: `address` has been validated by `io_addr`.
> - unsafe { self.io_write(value, address) };
> - Ok(())
> + self.try_write(offset, value)
> }
>
> /// Fallible 16-bit write with runtime bounds check.
> @@ -302,11 +305,7 @@ fn try_write16(&self, value: u16, offset: usize) -> Result
> where
> Self: IoCapable<u16>,
> {
> - let address = self.io_addr::<u16>(offset)?;
> -
> - // SAFETY: `address` has been validated by `io_addr`.
> - unsafe { self.io_write(value, address) };
> - Ok(())
> + self.try_write(offset, value)
> }
>
> /// Fallible 32-bit write with runtime bounds check.
> @@ -315,11 +314,7 @@ fn try_write32(&self, value: u32, offset: usize) -> Result
> where
> Self: IoCapable<u32>,
> {
> - let address = self.io_addr::<u32>(offset)?;
> -
> - // SAFETY: `address` has been validated by `io_addr`.
> - unsafe { self.io_write(value, address) };
> - Ok(())
> + self.try_write(offset, value)
> }
>
> /// Fallible 64-bit write with runtime bounds check.
> @@ -328,11 +323,7 @@ fn try_write64(&self, value: u64, offset: usize) -> Result
> where
> Self: IoCapable<u64>,
> {
> - let address = self.io_addr::<u64>(offset)?;
> -
> - // SAFETY: `address` has been validated by `io_addr`.
> - unsafe { self.io_write(value, address) };
> - Ok(())
> + self.try_write(offset, value)
> }
>
> /// Infallible 8-bit read with compile-time bounds check.
> @@ -341,10 +332,7 @@ fn read8(&self, offset: usize) -> u8
> where
> Self: IoKnownSize + IoCapable<u8>,
> {
> - let address = self.io_addr_assert::<u8>(offset);
> -
> - // SAFETY: `address` has been validated by `io_addr_assert`.
> - unsafe { self.io_read(address) }
> + self.read(offset)
> }
>
> /// Infallible 16-bit read with compile-time bounds check.
> @@ -353,10 +341,7 @@ fn read16(&self, offset: usize) -> u16
> where
> Self: IoKnownSize + IoCapable<u16>,
> {
> - let address = self.io_addr_assert::<u16>(offset);
> -
> - // SAFETY: `address` has been validated by `io_addr_assert`.
> - unsafe { self.io_read(address) }
> + self.read(offset)
> }
>
> /// Infallible 32-bit read with compile-time bounds check.
> @@ -365,10 +350,7 @@ fn read32(&self, offset: usize) -> u32
> where
> Self: IoKnownSize + IoCapable<u32>,
> {
> - let address = self.io_addr_assert::<u32>(offset);
> -
> - // SAFETY: `address` has been validated by `io_addr_assert`.
> - unsafe { self.io_read(address) }
> + self.read(offset)
> }
>
> /// Infallible 64-bit read with compile-time bounds check.
> @@ -377,10 +359,7 @@ fn read64(&self, offset: usize) -> u64
> where
> Self: IoKnownSize + IoCapable<u64>,
> {
> - let address = self.io_addr_assert::<u64>(offset);
> -
> - // SAFETY: `address` has been validated by `io_addr_assert`.
> - unsafe { self.io_read(address) }
> + self.read(offset)
> }
>
> /// Infallible 8-bit write with compile-time bounds check.
> @@ -389,10 +368,7 @@ fn write8(&self, value: u8, offset: usize)
> where
> Self: IoKnownSize + IoCapable<u8>,
> {
> - let address = self.io_addr_assert::<u8>(offset);
> -
> - // SAFETY: `address` has been validated by `io_addr_assert`.
> - unsafe { self.io_write(value, address) }
> + self.write(offset, value)
> }
>
> /// Infallible 16-bit write with compile-time bounds check.
> @@ -401,10 +377,7 @@ fn write16(&self, value: u16, offset: usize)
> where
> Self: IoKnownSize + IoCapable<u16>,
> {
> - let address = self.io_addr_assert::<u16>(offset);
> -
> - // SAFETY: `address` has been validated by `io_addr_assert`.
> - unsafe { self.io_write(value, address) }
> + self.write(offset, value)
> }
>
> /// Infallible 32-bit write with compile-time bounds check.
> @@ -413,10 +386,7 @@ fn write32(&self, value: u32, offset: usize)
> where
> Self: IoKnownSize + IoCapable<u32>,
> {
> - let address = self.io_addr_assert::<u32>(offset);
> -
> - // SAFETY: `address` has been validated by `io_addr_assert`.
> - unsafe { self.io_write(value, address) }
> + self.write(offset, value)
> }
>
> /// Infallible 64-bit write with compile-time bounds check.
> @@ -425,13 +395,28 @@ fn write64(&self, value: u64, offset: usize)
> where
> Self: IoKnownSize + IoCapable<u64>,
> {
> - let address = self.io_addr_assert::<u64>(offset);
> -
> - // SAFETY: `address` has been validated by `io_addr_assert`.
> - unsafe { self.io_write(value, address) }
> + self.write(offset, value)
> }
>
> /// Generic fallible read with runtime bounds check.
> + ///
> + /// # Examples
> + ///
> + /// Read a primitive type from an I/O address:
> + ///
> + /// ```no_run
> + /// use kernel::io::{Io, Mmio};
> + ///
> + /// fn do_reads(io: &Mmio) -> Result {
> + /// // 32-bit read from address `0x10`.
> + /// let v: u32 = io.try_read(0x10)?;
> + ///
> + /// // 8-bit read from address `0xfff`.
> + /// let v: u8 = io.try_read(0xfff)?;
> + ///
> + /// Ok(())
> + /// }
> + /// ```
> #[inline(always)]
> fn try_read<T, L>(&self, location: L) -> Result<T>
> where
> @@ -445,6 +430,24 @@ fn try_read<T, L>(&self, location: L) -> Result<T>
> }
>
> /// Generic fallible write with runtime bounds check.
> + ///
> + /// # Examples
> + ///
> + /// Write a primitive type to an I/O address:
> + ///
> + /// ```no_run
> + /// use kernel::io::{Io, Mmio};
> + ///
> + /// fn do_writes(io: &Mmio) -> Result {
> + /// // 32-bit write of value `1` at address `0x10`.
> + /// io.try_write(0x10, 1u32)?;
> + ///
> + /// // 8-bit write of value `0xff` at address `0xfff`.
> + /// io.try_write(0xfff, 0xffu8)?;
> + ///
> + /// Ok(())
> + /// }
> + /// ```
> #[inline(always)]
> fn try_write<T, L>(&self, location: L, value: T) -> Result
> where
> @@ -464,6 +467,20 @@ fn try_write<T, L>(&self, location: L, value: T) -> Result
> ///
> /// Caution: this does not perform any synchronization. Race conditions can occur in case of
> /// concurrent access.
> + ///
> + /// # Examples
> + ///
> + /// Read the u32 value at address `0x10`, increment it, and store the updated value back:
> + ///
> + /// ```no_run
> + /// use kernel::io::{Io, Mmio};
> + ///
> + /// fn do_update(io: &Mmio<0x1000>) -> Result {
> + /// io.try_update(0x10, |v: u32| {
> + /// v + 1
> + /// })
> + /// }
> + /// ```
> #[inline(always)]
> fn try_update<T, L, F>(&self, location: L, f: F) -> Result
> where
> @@ -484,6 +501,22 @@ fn try_update<T, L, F>(&self, location: L, f: F) -> Result
> }
>
> /// Generic infallible read with compile-time bounds check.
> + ///
> + /// # Examples
> + ///
> + /// Read a primitive type from an I/O address:
> + ///
> + /// ```no_run
> + /// use kernel::io::{Io, Mmio};
> + ///
> + /// fn do_reads(io: &Mmio<0x1000>) {
> + /// // 32-bit read from address `0x10`.
> + /// let v: u32 = io.read(0x10);
> + ///
> + /// // 8-bit read from the top of the I/O space.
> + /// let v: u8 = io.read(0xfff);
> + /// }
> + /// ```
> #[inline(always)]
> fn read<T, L>(&self, location: L) -> T
> where
> @@ -497,6 +530,22 @@ fn read<T, L>(&self, location: L) -> T
> }
>
> /// Generic infallible write with compile-time bounds check.
> + ///
> + /// # Examples
> + ///
> + /// Write a primitive type to an I/O address:
> + ///
> + /// ```no_run
> + /// use kernel::io::{Io, Mmio};
> + ///
> + /// fn do_writes(io: &Mmio<0x1000>) {
> + /// // 32-bit write of value `1` at address `0x10`.
> + /// io.write(0x10, 1u32);
> + ///
> + /// // 8-bit write of value `0xff` at the top of the I/O space.
> + /// io.write(0xfff, 0xffu8);
> + /// }
> + /// ```
> #[inline(always)]
> fn write<T, L>(&self, location: L, value: T)
> where
> @@ -514,6 +563,20 @@ fn write<T, L>(&self, location: L, value: T)
> ///
> /// Caution: this does not perform any synchronization. Race conditions can occur in case of
> /// concurrent access.
> + ///
> + /// # Examples
> + ///
> + /// Read the u32 value at address `0x10`, increment it, and store the updated value back:
> + ///
> + /// ```no_run
> + /// use kernel::io::{Io, Mmio};
> + ///
> + /// fn do_update(io: &Mmio<0x1000>) {
> + /// io.update(0x10, |v: u32| {
> + /// v + 1
> + /// })
> + /// }
> + /// ```
> #[inline(always)]
> fn update<T, L, F>(&self, location: L, f: F)
> where
next prev parent reply other threads:[~2026-03-09 15:29 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-09 15:13 [PATCH v8 00/10] rust: add `register!` macro Alexandre Courbot
2026-03-09 15:13 ` [PATCH v8 01/10] rust: enable the `generic_arg_infer` feature Alexandre Courbot
2026-03-11 0:13 ` Yury Norov
2026-03-11 6:16 ` Miguel Ojeda
2026-03-09 15:13 ` [PATCH v8 02/10] rust: num: add `shr` and `shl` methods to `Bounded` Alexandre Courbot
2026-03-11 0:26 ` Yury Norov
2026-03-09 15:14 ` [PATCH v8 03/10] rust: num: add `into_bool` method " Alexandre Courbot
2026-03-11 0:29 ` Yury Norov
2026-03-09 15:14 ` [PATCH v8 04/10] rust: num: make Bounded::get const Alexandre Courbot
2026-03-09 15:14 ` [PATCH v8 05/10] rust: io: add IoLoc type and generic I/O accessors Alexandre Courbot
2026-03-10 15:15 ` Danilo Krummrich
2026-03-09 15:14 ` [PATCH v8 06/10] rust: io: use generic read/write accessors for primitive accesses Alexandre Courbot
2026-03-09 15:29 ` Gary Guo [this message]
2026-03-10 1:57 ` Alexandre Courbot
2026-03-10 1:59 ` Alexandre Courbot
2026-03-10 2:04 ` Gary Guo
2026-03-10 15:17 ` Danilo Krummrich
2026-03-09 15:14 ` [PATCH v8 07/10] rust: io: introduce `IntoIoVal` trait and single-argument `write_val` Alexandre Courbot
2026-03-09 15:30 ` Gary Guo
2026-03-10 2:05 ` Alexandre Courbot
2026-03-10 16:34 ` Danilo Krummrich
2026-03-10 22:33 ` Gary Guo
2026-03-11 13:25 ` Danilo Krummrich
2026-03-11 13:42 ` Alexandre Courbot
2026-03-11 13:28 ` Alexandre Courbot
2026-03-11 14:56 ` Danilo Krummrich
2026-03-11 15:42 ` Gary Guo
2026-03-11 16:22 ` Danilo Krummrich
2026-03-11 17:16 ` Gary Guo
2026-03-12 13:53 ` Alexandre Courbot
2026-03-12 15:54 ` Danilo Krummrich
2026-03-14 1:19 ` Alexandre Courbot
2026-03-09 15:14 ` [PATCH v8 08/10] rust: io: add `register!` macro Alexandre Courbot
2026-03-10 16:39 ` Danilo Krummrich
2026-03-09 15:14 ` [PATCH v8 09/10] sample: rust: pci: use " Alexandre Courbot
2026-03-09 15:14 ` [PATCH FOR REFERENCE v8 10/10] gpu: nova-core: use the kernel " Alexandre Courbot
2026-03-09 15:43 ` Joel Fernandes
2026-03-09 17:34 ` John Hubbard
2026-03-09 17:51 ` Joel Fernandes
2026-03-09 18:01 ` John Hubbard
2026-03-09 18:28 ` Gary Guo
2026-03-09 18:34 ` John Hubbard
2026-03-09 18:42 ` Danilo Krummrich
2026-03-09 18:47 ` John Hubbard
2026-03-09 18:42 ` Gary Guo
2026-03-09 18:50 ` John Hubbard
2026-03-09 18:04 ` Danilo Krummrich
2026-03-09 18:06 ` John Hubbard
2026-03-10 2:18 ` Alexandre Courbot
2026-03-09 18:05 ` [PATCH v8 00/10] rust: add " John Hubbard
2026-03-09 21:08 ` Daniel Almeida
2026-03-10 17:20 ` Danilo Krummrich
2026-03-11 13:01 ` Alexandre Courbot
2026-03-11 13:07 ` Danilo Krummrich
2026-03-11 13:35 ` Alexandre Courbot
2026-03-11 13:38 ` Danilo Krummrich
2026-03-12 2:23 ` Alexandre Courbot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DGYCWW4AUI1Y.29UXVO4XOFCOA@garyguo.net \
--to=gary@garyguo.net \
--cc=a.hindborg@kernel.org \
--cc=acourbot@nvidia.com \
--cc=aliceryhl@google.com \
--cc=apopple@nvidia.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun@kernel.org \
--cc=dakr@kernel.org \
--cc=daniel.almeida@collabora.com \
--cc=dirk.behme@de.bosch.com \
--cc=ecourtney@nvidia.com \
--cc=epeer@nvidia.com \
--cc=jhubbard@nvidia.com \
--cc=joelagnelf@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=steven.price@arm.com \
--cc=tmgross@umich.edu \
--cc=ttabi@nvidia.com \
--cc=yury.norov@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox