Linux PCI subsystem development
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: "Alexandre Courbot" <acourbot@nvidia.com>, "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 07/20] rust: io: implement `Mmio` as view type
Date: Tue, 16 Jun 2026 12:12:56 +0100	[thread overview]
Message-ID: <DJAFGLBC8WQY.L3P1T92AFN45@garyguo.net> (raw)
In-Reply-To: <DJA1J6H6Z6CI.3LLUM4T35FXXK@nvidia.com>

On Tue Jun 16, 2026 at 1:18 AM BST, Alexandre Courbot wrote:
> On Tue Jun 16, 2026 at 12:13 AM JST, Gary Guo wrote:
>>> Is there a reason for not just declaring `RelaxedMmio` as
>>>
>>>     #[repr(transparent)]
>>>     pub struct RelaxedMmio<'a, T: ?Sized>(Mmio<'a, T>);
>>>
>>> similarly to what the original code did with `MmioOwned`?
>>>
>>> I tried locally and could build. This avoids declaring `Mmio` and
>>> `RelaxedMmio` as basically identical types, and lets us remove the
>>> explicit `Send` and `Sync` implementations. IIUC you can also reduce or
>>> even remove the invariant section as it is enforced by `Mmio`.
>>
>> This is what I did originally, but this would cause duplication for
>> RelaxedMmioBackend, as now you have to do
>>
>>      unsafe { bindings::$read_fn(view.0.ptr.cast_const().cast()) }
>>
>> and the `.0` causes the macro not being shared with MmioBackend.
>
> Since `MmioBackend` and `RelaxedMmioBackend` both implement `IoBackend`,
> I think using `IoBackend::as_ptr` in the macro should let you avoid the
> duplication?

Good point. Are you okay with the following diff?

Best,
Gary

diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index c6f3ab530b4f..a8da625560a0 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -1136,14 +1136,16 @@ macro_rules! impl_mmio_io_capable {
         impl IoCapable<$ty> for $backend {
             #[inline]
             fn io_read(view: <$backend as IoBackend>::View<'_, $ty>) -> $ty {
-                // SAFETY: By the type invariant, `view.ptr` is a valid address for MMIO operations.
-                unsafe { bindings::$read_fn(view.ptr.cast_const().cast()) }
+                // SAFETY: `$backend::as_ptr(view)` is a valid address for MMIO operations for both
+                // `MmioBackend` and `RelaxedMmioBackend`.
+                unsafe { bindings::$read_fn($backend::as_ptr(view).cast_const().cast()) }
             }
 
             #[inline]
             fn io_write(view: <$backend as IoBackend>::View<'_, $ty>, value: $ty) {
-                // SAFETY: By the type invariant, `view.ptr` is a valid address for MMIO operations.
-                unsafe { bindings::$write_fn(value, view.ptr.cast()) }
+                // SAFETY: `$backend::as_ptr(view)` is a valid address for MMIO operations for both
+                // `MmioBackend` and `RelaxedMmioBackend`.
+                unsafe { bindings::$write_fn(value, $backend::as_ptr(view).cast()) }
             }
         }
     };
@@ -1185,14 +1187,7 @@ unsafe fn copy_to_io(view: Self::View<'_, [u8]>, buffer: *const u8) {
 /// the regular ones.
 ///
 /// See [`Mmio::relaxed`] for a usage example.
-///
-/// # Invariant
-///
-/// `ptr` points to a valid and aligned memory-mapped I/O region for the duration lifetime `'a`.
-pub struct RelaxedMmio<'a, T: ?Sized> {
-    ptr: *mut T,
-    phantom: PhantomData<&'a ()>,
-}
+pub struct RelaxedMmio<'a, T: ?Sized>(Mmio<'a, T>);
 
 impl<T: ?Sized> Copy for RelaxedMmio<'_, T> {}
 impl<T: ?Sized> Clone for RelaxedMmio<'_, T> {
@@ -1202,12 +1197,6 @@ fn clone(&self) -> Self {
     }
 }
 
-// SAFETY: `RelaxedMmio<'_, T>` is conceptually `&T` but in I/O memory.
-unsafe impl<T: ?Sized + Sync> Send for RelaxedMmio<'_, T> {}
-
-// SAFETY: `RelaxedMmio<'_, T>` is conceptually `&T` but in I/O memory.
-unsafe impl<T: ?Sized + Sync> Sync for RelaxedMmio<'_, T> {}
-
 /// I/O Backend for memory-mapped I/O, with relaxed access semantics.
 pub struct RelaxedMmioBackend;
 
@@ -1216,20 +1205,16 @@ impl IoBackend for RelaxedMmioBackend {
 
     #[inline]
     fn as_ptr<'a, T: ?Sized + KnownSize>(view: Self::View<'a, T>) -> *mut T {
-        view.ptr
+        MmioBackend::as_ptr(view.0)
     }
 
     #[inline]
     unsafe fn project_view<'a, T: ?Sized + KnownSize, U: ?Sized + KnownSize>(
-        _view: Self::View<'a, T>,
+        view: Self::View<'a, T>,
         ptr: *mut U,
     ) -> Self::View<'a, U> {
-        // INVARIANT: Per safety requirement, `ptr` is projection from `view`, so it is also a valid
-        // memory-mapped I/O region.
-        RelaxedMmio {
-            ptr,
-            phantom: PhantomData,
-        }
+        // SAFETY: Per safety requirement.
+        RelaxedMmio(unsafe { MmioBackend::project_view(view.0, ptr) })
     }
 }
 
@@ -1267,11 +1252,7 @@ impl<'a, T: ?Sized> Mmio<'a, T> {
     /// ```
     #[inline]
     pub fn relaxed(self) -> RelaxedMmio<'a, T> {
-        // INVARIANT: `RelaxedMmio` has the same invariant as `Mmio`.
-        RelaxedMmio {
-            ptr: self.ptr,
-            phantom: PhantomData,
-        }
+        RelaxedMmio(self)
     }
 }

  reply	other threads:[~2026-06-16 11:13 UTC|newest]

Thread overview: 62+ 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
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-16 11:12         ` Gary Guo [this message]
2026-06-16 14:22           ` 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-16 10:58     ` Gary Guo
2026-06-16 14:28       ` 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-16 14:05   ` Alexandre Courbot
2026-06-16 14:50     ` Gary Guo
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=DJAFGLBC8WQY.L3P1T92AFN45@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@kernel.org \
    --cc=abdiel.janulgue@gmail.com \
    --cc=acourbot@nvidia.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=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox