linux-kernel - Re: [PATCH v6 RESEND 5/7] rust: io: factor out MMIO read/write macros

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DE7E7YXGLRWP.39EGH02PEV46Q@nvidia.com>
Date: Thu, 13 Nov 2025 16:36:47 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Zhi Wang" <zhiw@...dia.com>, <rust-for-linux@...r.kernel.org>,
 <linux-pci@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Cc: <dakr@...nel.org>, <aliceryhl@...gle.com>, <bhelgaas@...gle.com>,
 <kwilczynski@...nel.org>, <ojeda@...nel.org>, <alex.gaynor@...il.com>,
 <boqun.feng@...il.com>, <gary@...yguo.net>, <bjorn3_gh@...tonmail.com>,
 <lossin@...nel.org>, <a.hindborg@...nel.org>, <tmgross@...ch.edu>,
 <markus.probst@...teo.de>, <helgaas@...nel.org>, <cjia@...dia.com>,
 <smitra@...dia.com>, <ankita@...dia.com>, <aniketa@...dia.com>,
 <kwankhede@...dia.com>, <targupta@...dia.com>, <acourbot@...dia.com>,
 <joelagnelf@...dia.com>, <jhubbard@...dia.com>, <zhiwang@...nel.org>
Subject: Re: [PATCH v6 RESEND 5/7] rust: io: factor out MMIO read/write
 macros

On Tue Nov 11, 2025 at 5:41 AM JST, Zhi Wang wrote:
> Refactor the existing MMIO accessors to use common call macros
> instead of inlining the bindings calls in each `define_{read,write}!`
> expansion.
>
> This factoring separates the common offset/bounds checks from the
> low-level call pattern, making it easier to add additional I/O accessor
> families.
>
> No functional change intended.
>
> Signed-off-by: Zhi Wang <zhiw@...dia.com>
> ---
>  rust/kernel/io.rs | 110 ++++++++++++++++++++++++++++++----------------
>  1 file changed, 73 insertions(+), 37 deletions(-)
>
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index 4d98d431b523..090d1b11a896 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -124,8 +124,34 @@ pub fn maxsize(&self) -> usize {
>  #[repr(transparent)]
>  pub struct Mmio<const SIZE: usize = 0>(MmioRaw<SIZE>);
>  
> +macro_rules! call_mmio_read {
> +    (infallible, $c_fn:ident, $self:ident, $type:ty, $addr:expr) => {
> +        // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
> +        unsafe { bindings::$c_fn($addr as *const c_void) as $type }
> +    };
> +
> +    (fallible, $c_fn:ident, $self:ident, $type:ty, $addr:expr) => {{
> +        // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
> +        Ok(unsafe { bindings::$c_fn($addr as *const c_void) as $type })
> +    }};
> +}
> +
> +macro_rules! call_mmio_write {
> +    (infallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr, $value:expr) => {
> +        // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
> +        unsafe { bindings::$c_fn($value, $addr as *mut c_void) }
> +    };
> +
> +    (fallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr, $value:expr) => {{
> +        // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
> +        unsafe { bindings::$c_fn($value, $addr as *mut c_void) };
> +        Ok(())
> +    }};
> +}

I understand the intent from the commit message, but this is starting to
look like an intricate maze of macro expansion and it might not be as
easy for first-time readers - could you add an explanatory doccomment
for these?

> +
>  macro_rules! define_read {
> -    (infallible, $(#[$attr:meta])* $vis:vis $name:ident, $c_fn:ident -> $type_name:ty) => {
> +    (infallible, $(#[$attr:meta])* $vis:vis $name:ident, $call_macro:ident, $c_fn:ident ->
> +     $type_name:ty) => {
>          /// Read IO data from a given offset known at compile time.
>          ///
>          /// Bound checks are performed on compile time, hence if the offset is not known at compile
> @@ -135,12 +161,13 @@ macro_rules! define_read {
>          $vis fn $name(&self, offset: usize) -> $type_name {
>              let addr = self.io_addr_assert::<$type_name>(offset);
>  
> -            // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
> -            unsafe { bindings::$c_fn(addr as *const c_void) }
> +            // SAFETY: By the type invariant `addr` is a valid address for IO operations.
> +            $call_macro!(infallible, $c_fn, self, $type_name, addr)
>          }
>      };

To convey the fact that `$c_fn` is passed to `$call_macro`, how about
changing the syntax to something like 

  `define_read(infallible, $vis $name $call_macro($c_fn) -> $type_name`

?