All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "Gary Guo" <gary@garyguo.net>
Cc: "Alice Ryhl" <aliceryhl@google.com>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"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>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
	"Abdiel Janulgue" <abdiel.janulgue@gmail.com>,
	"Robin Murphy" <robin.murphy@arm.com>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Danilo Krummrich" <dakr@kernel.org>,
	driver-core@lists.linux.dev, rust-for-linux@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	nova-gpu@lists.linux.dev, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v4 01/20] rust: io: add dynamically-sized `Region` type
Date: Mon, 15 Jun 2026 13:03:36 +0900	[thread overview]
Message-ID: <DJ9BPBO51J9U.FQYCSB4SSC8Z@nvidia.com> (raw)
In-Reply-To: <20260611-io_projection-v4-1-1f7224b02dcb@garyguo.net>

On Fri Jun 12, 2026 at 1:28 AM JST, Gary Guo wrote:
> Currently many I/O related structs carry a `SIZE` parameter to denote the
> minimum size of the I/O region, while they also carry a field indicating
> the actual size. Proliferation of the pattern creates a lot of duplicated
> code, and makes it hard to create typed views of I/O.
>
> Introduce a `Region` type that carries the `SIZE` parameter. It is a
> wrapper of `[u8]`, which makes it dynamically sized with a metadata of
> `usize`. This way, pointers to `Region` naturally carry size information.
> This type is required to be 4-byte aligned.
>
> Expose the minimum size information via `MIN_SIZE` constant of the
> `KnownSize` trait. Similarly, expose the minimum alignment information via
> `KnownSize::MIN_ALIGN`.
>
> With these changes, it is possible to add an associated type to `Io` trait
> to represent the type of I/O region. For untyped regions, this is the newly
> added `Region` type. Remove `IoKnownSize` as it is no longer necessary. Use
> the same mechanism to indicate minimum size of PCI config spaces.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>
> ---
>  rust/kernel/devres.rs |   6 +--
>  rust/kernel/io.rs     | 130 +++++++++++++++++++++++++++++++++-----------------
>  rust/kernel/lib.rs    |   3 ++
>  rust/kernel/pci.rs    |   1 -
>  rust/kernel/pci/io.rs |  40 +++++++---------
>  rust/kernel/ptr.rs    |  12 +++++
>  6 files changed, 118 insertions(+), 74 deletions(-)
>
> diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
> index 11ce500e9b76..ed30ccc6e68e 100644
> --- a/rust/kernel/devres.rs
> +++ b/rust/kernel/devres.rs
> @@ -68,7 +68,6 @@ struct Inner<T> {
>  ///     devres::Devres,
>  ///     io::{
>  ///         Io,
> -///         IoKnownSize,
>  ///         Mmio,
>  ///         MmioRaw,
>  ///         PhysAddr, //
> @@ -297,10 +296,7 @@ pub fn device(&self) -> &Device {
>      /// use kernel::{
>      ///     device::Core,
>      ///     devres::Devres,
> -    ///     io::{
> -    ///         Io,
> -    ///         IoKnownSize, //
> -    ///     },
> +    ///     io::Io,
>      ///     pci, //
>      /// };
>      ///
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index fcc7678fd9e3..bef571dad6eb 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -6,7 +6,11 @@
>  
>  use crate::{
>      bindings,
> -    prelude::*, //
> +    prelude::*,
> +    ptr::{
> +        Alignment,
> +        KnownSize, //
> +    }, //
>  };
>  
>  pub mod mem;
> @@ -31,6 +35,58 @@
>  /// `CONFIG_PHYS_ADDR_T_64BIT`, and it can be a u64 even on 32-bit architectures.
>  pub type ResourceSize = bindings::resource_size_t;
>  
> +/// Untyped I/O region.
> +///
> +/// This type can be used when an I/O region without known type information has a compile-time known
> +/// minimum size (and a runtime known actual size).
> +///
> +/// This must be 4-byte aligned.
> +///
> +/// # Invariants
> +///
> +/// Size of the region is at least as large as the `SIZE` generic parameter.

I noticed that patch 13 adds the "size must be multiple of 4" invariant.
The doccomment for `ptr_from_raw_parts_mut` says that "size should be
4-bytes aligned" though, which sounds like the same to me. So should
that second invariant be introduced in this patch instead of patch 13?

> +#[repr(C, align(4))]
> +pub struct Region<const SIZE: usize = 0> {
> +    inner: [u8],
> +}
> +
> +impl<const SIZE: usize> Region<SIZE> {
> +    /// Create a raw mutable pointer from given base address and size.
> +    ///
> +    /// `size` should be at least as large as the minimum size `SIZE`, and `base` and `size` should
> +    /// be 4-byte aligned to uphold the type invariant.

s/should/must? I guess we are running into all sort of issues if we
create regions which runtime size is smaller than the compile-time one,
and this is an invariant of `Region` itself.

Maybe this method should even be made `unsafe` for this reason? The
caller will need to write a `SAFETY` comment before dereferencing the
pointer, but IIUC this comment is bound to cover the pointer invariants,
not necessarily those of `Region`. Making the method `unsafe` would
force the user to cover them here.

> +    ///
> +    /// Just like other methods on raw pointers, it is not unsafe to create a raw pointer
> +    /// that does not uphold the type invariants. However such pointers are not valid.
> +    #[inline]
> +    pub fn ptr_from_raw_parts_mut(base: *mut u8, size: usize) -> *mut Self {
> +        core::ptr::slice_from_raw_parts_mut(base, size) as *mut Region<SIZE>
> +    }
> +
> +    /// Create a raw mutable pointer from given base address and size.
> +    ///
> +    /// The alignment of `base` is checked, and `size` is checked against the minimum size specified
> +    /// via const generics.
> +    #[inline]
> +    pub fn ptr_try_from_raw_parts_mut(base: *mut u8, size: usize) -> Result<*mut Self> {
> +        if size < SIZE || base.align_offset(4) != 0 || !size.is_multiple_of(4) {
> +            return Err(EINVAL);
> +        }
> +
> +        Ok(Self::ptr_from_raw_parts_mut(base, size))
> +    }
> +}
> +
> +impl<const SIZE: usize> KnownSize for Region<SIZE> {
> +    const MIN_SIZE: usize = SIZE;
> +    const MIN_ALIGN: Alignment = Alignment::new::<4>();
> +
> +    #[inline(always)]
> +    fn size(p: *const Self) -> usize {
> +        (p as *const [u8]).len()
> +    }
> +}
> +
>  /// Raw representation of an MMIO region.
>  ///
>  /// By itself, the existence of an instance of this structure does not provide any guarantees that
> @@ -85,7 +141,6 @@ pub fn maxsize(&self) -> usize {
>  ///     ffi::c_void,
>  ///     io::{
>  ///         Io,
> -///         IoKnownSize,
>  ///         Mmio,
>  ///         MmioRaw,
>  ///         PhysAddr,
> @@ -241,12 +296,25 @@ fn offset(self) -> usize {
>  /// For MMIO regions, all widths (u8, u16, u32, and u64 on 64-bit systems) are typically
>  /// supported. For PCI configuration space, u8, u16, and u32 are supported but u64 is not.
>  pub trait Io {
> +    /// Type of this I/O region. For untyped regions, [`Region`] can be used.
> +    type Target: ?Sized + KnownSize;
> +
>      /// Returns the base address of this mapping.
>      fn addr(&self) -> usize;
>  
>      /// Returns the maximum size of this mapping.
>      fn maxsize(&self) -> usize;
>  
> +    /// Returns the absolute I/O address for a given `offset`,
> +    /// performing compile-time bound checks.

nit: this doccomment could be a one liner.

  parent reply	other threads:[~2026-06-15  4:03 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-11 16:28 [PATCH v4 00/20] rust: I/O type generalization and projection Gary Guo
2026-06-11 16:28 ` [PATCH v4 01/20] rust: io: add dynamically-sized `Region` type Gary Guo
2026-06-13 10:05   ` Miguel Ojeda
2026-06-15  4:03   ` Alexandre Courbot [this message]
2026-06-15 10:05     ` Gary Guo
2026-06-15 11:47     ` Miguel Ojeda
2026-06-11 16:28 ` [PATCH v4 02/20] rust: io: add missing safety requirement in `IoCapable` methods Gary Guo
2026-06-15  4:28   ` Alexandre Courbot
2026-06-15 10:13     ` Gary Guo
2026-06-15 14:10       ` Alexandre Courbot
2026-06-11 16:28 ` [PATCH v4 03/20] rust: io: restrict untyped IO access and `register!` to `Region` Gary Guo
2026-06-15  5:17   ` Alexandre Courbot
2026-06-15 10:22     ` Gary Guo
2026-06-11 16:28 ` [PATCH v4 04/20] rust: io: implement `Io` on reference types instead Gary Guo
2026-06-11 17:07   ` sashiko-bot
2026-06-15  5:29   ` Alexandre Courbot
2026-06-11 16:28 ` [PATCH v4 05/20] rust: io: generalize `MmioRaw` to pointer to arbitrary type Gary Guo
2026-06-11 17:15   ` sashiko-bot
2026-06-15  8:04   ` Alexandre Courbot
2026-06-11 16:28 ` [PATCH v4 06/20] rust: io: rename `Mmio` to `MmioOwned` Gary Guo
2026-06-11 17:21   ` sashiko-bot
2026-06-15  8:09   ` Alexandre Courbot
2026-06-11 16:28 ` [PATCH v4 07/20] rust: io: implement `Mmio` as view type Gary Guo
2026-06-11 17:31   ` sashiko-bot
2026-06-15 14:52   ` Alexandre Courbot
2026-06-15 15:13     ` Gary Guo
2026-06-16  0:18       ` Alexandre Courbot
2026-06-11 16:28 ` [PATCH v4 08/20] rust: pci: io: make `ConfigSpace` a view Gary Guo
2026-06-11 17:37   ` sashiko-bot
2026-06-16  6:34   ` Alexandre Courbot
2026-06-11 16:28 ` [PATCH v4 09/20] rust: io: use view types instead of addresses for `Io` Gary Guo
2026-06-11 17:46   ` sashiko-bot
2026-06-11 16:28 ` [PATCH v4 10/20] rust: io: remove `MmioOwned` Gary Guo
2026-06-11 17:54   ` sashiko-bot
2026-06-11 16:28 ` [PATCH v4 11/20] rust: io: move `Io` methods to extension trait Gary Guo
2026-06-11 18:00   ` sashiko-bot
2026-06-11 16:28 ` [PATCH v4 12/20] rust: prelude: add `zerocopy{,_derive}::IntoBytes` Gary Guo
2026-06-11 18:01   ` sashiko-bot
2026-06-11 16:28 ` [PATCH v4 13/20] rust: io: add projection macro and methods Gary Guo
2026-06-11 18:14   ` sashiko-bot
2026-06-11 18:34     ` Gary Guo
2026-06-11 16:28 ` [PATCH v4 14/20] rust: io: add I/O backend for system memory with volatile access Gary Guo
2026-06-11 18:23   ` sashiko-bot
2026-06-11 16:28 ` [PATCH v4 15/20] rust: io: implement a view type for `Coherent` Gary Guo
2026-06-11 18:30   ` sashiko-bot
2026-06-11 16:28 ` [PATCH v4 16/20] rust: io: add `read_val` and `write_val` functions on `Io` Gary Guo
2026-06-11 18:37   ` sashiko-bot
2026-06-11 16:28 ` [PATCH v4 17/20] gpu: nova-core: use I/O projection for cleaner encapsulation Gary Guo
2026-06-11 18:47   ` sashiko-bot
2026-06-11 16:28 ` [PATCH v4 18/20] rust: dma: drop `dma_read!` and `dma_write!` API Gary Guo
2026-06-11 19:01   ` sashiko-bot
2026-06-11 16:28 ` [PATCH v4 19/20] rust: io: add copying methods Gary Guo
2026-06-11 19:11   ` sashiko-bot
2026-06-11 19:36   ` Gary Guo
2026-06-11 16:28 ` [PATCH v4 20/20] rust: io: implement `IoSysMap` Gary Guo
2026-06-11 19:13   ` sashiko-bot

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=DJ9BPBO51J9U.FQYCSB4SSC8Z@nvidia.com \
    --to=acourbot@nvidia.com \
    --cc=a.hindborg@kernel.org \
    --cc=abdiel.janulgue@gmail.com \
    --cc=airlied@gmail.com \
    --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=dri-devel@lists.freedesktop.org \
    --cc=driver-core@lists.linux.dev \
    --cc=gary@garyguo.net \
    --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=nova-gpu@lists.linux.dev \
    --cc=ojeda@kernel.org \
    --cc=rafael@kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=simona@ffwll.ch \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.