public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "Gary Guo" <gary@kernel.org>
Cc: "Gary Guo" <gary@garyguo.net>, "Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
	rust-for-linux@vger.kernel.org, driver-core@lists.linux.dev,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org
Subject: Re: [PATCH 2/8] rust: io: generalize `Mmio` to arbitrary type
Date: Sun, 05 Apr 2026 23:55:30 +0900	[thread overview]
Message-ID: <DHLB3RO3OSF5.2R7F27U99BKLN@nvidia.com> (raw)
In-Reply-To: <20260323153807.1360705-3-gary@kernel.org>

On Tue Mar 24, 2026 at 12:37 AM JST, Gary Guo wrote:
> From: Gary Guo <gary@garyguo.net>
>
> Currently, `io::Mmio` always represent an untyped region of a compile-time
> known minimum size, which is roughly equivalent to `void __iomem*` (but
> with bound checks). However, it is useful to also be to represent I/O
> memory of a specific type, e.g. `u32 __iomem*` or `struct foo __iomem*`.
>
> Thus, make `Mmio` generic on arbitrary `T`, where `T` is a sized type, or a
> DST that implements `KnownSize`. Similar to the `MmioRaw` change, the
> existing behaviour is preserved in the form of `Mmio<Region<SIZE>>`. This
> change brings the MMIO closer to the DMA coherent allocation types that we
> have, which is already typed.

You probably noticed, but the regular `read8`, `read16` remain available
irrespective of the `T` parameter, allowing the `Mmio` to be accessed
using both the structured type and arbitrary primitives with an offset.
I cannot find a reason to label this as unsound, but it might be
confusing as it makes projection an additional capability on top of the
existing raw I/O methods. This might be worth mentioning in the
documentation, or maybe the primitive accessors should only be made
available to `Io<Region>`?

>
> To be able to implement `IoKnownSize`, add a `MIN_SIZE` constant to
> `KnownSize` trait to represent compile-time known minimum size of a
> specific type.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>
> ---
>  rust/kernel/devres.rs      |  2 +-
>  rust/kernel/io.rs          | 63 ++++++++++++++++++++++----------------
>  rust/kernel/io/mem.rs      |  4 +--
>  rust/kernel/io/poll.rs     |  6 ++--
>  rust/kernel/io/register.rs | 19 +++++++-----
>  rust/kernel/pci/io.rs      |  2 +-
>  rust/kernel/ptr.rs         |  7 +++++
>  7 files changed, 64 insertions(+), 39 deletions(-)
>
> diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
> index 65a4082122af..3e22c63efb98 100644
> --- a/rust/kernel/devres.rs
> +++ b/rust/kernel/devres.rs
> @@ -106,7 +106,7 @@ struct Inner<T> {
>  /// }
>  ///
>  /// impl<const SIZE: usize> Deref for IoMem<SIZE> {
> -///    type Target = Mmio<SIZE>;
> +///    type Target = Mmio<Region<SIZE>>;
>  ///
>  ///    fn deref(&self) -> &Self::Target {
>  ///         // SAFETY: The memory range stored in `self` has been properly mapped in `Self::new`.
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index d7f2145fa9b9..5a26b1e7e533 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -44,6 +44,8 @@ pub struct Region<const SIZE: usize = 0> {
>  }
>  
>  impl<const SIZE: usize> KnownSize for Region<SIZE> {
> +    const MIN_SIZE: usize = SIZE;
> +
>      #[inline(always)]
>      fn size(p: *const Self) -> usize {
>          (p as *const [u8]).len()
> @@ -169,7 +171,7 @@ pub fn size(&self) -> usize {
>  /// }
>  ///
>  /// impl<const SIZE: usize> Deref for IoMem<SIZE> {
> -///    type Target = Mmio<SIZE>;
> +///    type Target = Mmio<Region<SIZE>>;
>  ///
>  ///    fn deref(&self) -> &Self::Target {
>  ///         // SAFETY: The memory range stored in `self` has been properly mapped in `Self::new`.
> @@ -187,7 +189,7 @@ pub fn size(&self) -> usize {
>  /// # }
>  /// ```
>  #[repr(transparent)]
> -pub struct Mmio<const SIZE: usize = 0>(MmioRaw<Region<SIZE>>);
> +pub struct Mmio<T: ?Sized>(MmioRaw<T>);
>  
>  /// Checks whether an access of type `U` at the given `offset`
>  /// is valid within this region.
> @@ -462,9 +464,10 @@ fn write64(&self, value: u64, offset: usize)
>      /// use kernel::io::{
>      ///     Io,
>      ///     Mmio,
> +    ///     Region,
>      /// };
>      ///
> -    /// fn do_reads(io: &Mmio) -> Result {
> +    /// fn do_reads(io: &Mmio<Region<0>>) -> Result {

This can be `&Mmio<Region>`, and probably should be as it is closer to
the original code which also used a size of `0` by default (applies to
the other examples as well).

<snip>
> -impl<const SIZE: usize> Mmio<SIZE> {
> +impl<T: ?Sized + KnownSize> Mmio<T> {
>      /// Converts an `MmioRaw` into an `Mmio` instance, providing the accessors to the MMIO mapping.
>      ///
>      /// # Safety
>      ///
>      /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of size
> -    /// `maxsize`.
> -    pub unsafe fn from_raw(raw: &MmioRaw<Region<SIZE>>) -> &Self {
> +    /// `addr.size()`.

That probably shouldn't be fixed by this patch, but there are is `addr`
parameter for this method.

  parent reply	other threads:[~2026-04-05 14:55 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260323153807.1360705-1-gary@kernel.org>
2026-03-23 15:37 ` [PATCH 1/8] rust: io: generalize `MmioRaw` to pointer to arbitrary type Gary Guo
2026-03-26 12:53   ` Andreas Hindborg
2026-03-26 14:31     ` Gary Guo
2026-03-23 15:37 ` [PATCH 2/8] rust: io: generalize `Mmio` " Gary Guo
2026-03-26 13:04   ` Andreas Hindborg
2026-03-26 14:32     ` Gary Guo
2026-03-26 18:23       ` Andreas Hindborg
2026-04-02 12:57         ` Gary Guo
2026-04-04 18:57   ` Miguel Ojeda
2026-04-05 14:55   ` Alexandre Courbot [this message]
2026-04-05 23:21     ` Gary Guo
2026-04-06  4:00       ` Alexandre Courbot
2026-03-23 15:37 ` [PATCH 3/8] rust: io: use pointer types instead of address Gary Guo
2026-03-26 14:20   ` Andreas Hindborg
2026-03-26 14:35     ` Gary Guo
2026-03-27 10:11       ` Miguel Ojeda
2026-04-05 14:56   ` Alexandre Courbot
2026-04-05 15:00     ` Danilo Krummrich
2026-04-06  3:49       ` Alexandre Courbot
2026-03-23 15:37 ` [PATCH 4/8] rust: io: add view type Gary Guo
2026-03-26 14:31   ` Andreas Hindborg
2026-04-02 13:01     ` Gary Guo
2026-03-23 15:37 ` [PATCH 5/8] rust: dma: add methods to unsafely create reference from subview Gary Guo
2026-03-26 14:37   ` Andreas Hindborg
2026-03-26 14:44     ` Gary Guo
2026-03-23 15:37 ` [PATCH 6/8] rust: io: add `read_val` and `write_val` function on I/O view Gary Guo
2026-03-27  8:21   ` Andreas Hindborg
2026-03-27 12:19     ` Gary Guo
2026-03-23 15:37 ` [PATCH 7/8] gpu: nova-core: use I/O projection for cleaner encapsulation Gary Guo
2026-03-23 15:38 ` [PATCH 8/8] rust: dma: drop `dma_read!` and `dma_write!` API Gary Guo
2026-03-27  8:25   ` Andreas Hindborg

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=DHLB3RO3OSF5.2R7F27U99BKLN@nvidia.com \
    --to=acourbot@nvidia.com \
    --cc=a.hindborg@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=driver-core@lists.linux.dev \
    --cc=gary@garyguo.net \
    --cc=gary@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=kwilczynski@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rafael@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /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