All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alice Ryhl <aliceryhl@google.com>
To: Joel Fernandes <joelagnelf@nvidia.com>
Cc: "Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"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>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	dri-devel@lists.freedesktop.org, linux-pci@vger.kernel.org,
	"Alice Ryhl" <aliceryhl@google.com>
Subject: [PATCH] rust: io: use const generics for read/write offsets
Date: Thu, 18 Sep 2025 15:02:11 +0000	[thread overview]
Message-ID: <20250918-write-offset-const-v1-1-eb51120d4117@google.com> (raw)

Using build_assert! to assert that offsets are in bounds is really
fragile and likely to result in spurious and hard-to-debug build
failures. Therefore, build_assert! should be avoided for this case.
Thus, update the code to perform the check in const evaluation instead.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 drivers/gpu/drm/tyr/regs.rs     |  4 ++--
 rust/kernel/devres.rs           |  4 ++--
 rust/kernel/io.rs               | 18 ++++++++++--------
 rust/kernel/io/mem.rs           |  6 +++---
 samples/rust/rust_driver_pci.rs | 10 +++++-----
 5 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/tyr/regs.rs b/drivers/gpu/drm/tyr/regs.rs
index f46933aaa2214ee0ac58b1ea2a6aa99506a35b70..e3c306e48e86d1d6047cab7944e0fe000901d48b 100644
--- a/drivers/gpu/drm/tyr/regs.rs
+++ b/drivers/gpu/drm/tyr/regs.rs
@@ -25,13 +25,13 @@
 impl<const OFFSET: usize> Register<OFFSET> {
     #[inline]
     pub(crate) fn read(&self, dev: &Device<Bound>, iomem: &Devres<IoMem>) -> Result<u32> {
-        let value = (*iomem).access(dev)?.read32(OFFSET);
+        let value = (*iomem).access(dev)?.read32::<OFFSET>();
         Ok(value)
     }
 
     #[inline]
     pub(crate) fn write(&self, dev: &Device<Bound>, iomem: &Devres<IoMem>, value: u32) -> Result {
-        (*iomem).access(dev)?.write32(value, OFFSET);
+        (*iomem).access(dev)?.write32::<OFFSET>(value);
         Ok(())
     }
 }
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index da18091143a67fcfbb247e7cb4f59f5a4932cac5..3e66e10c05fa078e42162c7a367161fbf735a07f 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -96,7 +96,7 @@ struct Inner<T: Send> {
 /// let devres = KBox::pin_init(Devres::new(dev, iomem), GFP_KERNEL)?;
 ///
 /// let res = devres.try_access().ok_or(ENXIO)?;
-/// res.write8(0x42, 0x0);
+/// res.write8::<0x0>(0x42);
 /// # Ok(())
 /// # }
 /// ```
@@ -232,7 +232,7 @@ pub fn device(&self) -> &Device {
     ///
     ///     // might_sleep()
     ///
-    ///     bar.write32(0x42, 0x0);
+    ///     bar.write32::<0x0>(0x42);
     ///
     ///     Ok(())
     /// }
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 03b467722b8651ebecd660ac0e2d849cf88dc915..563ff8488100d9e07a7f4bffeb085db7bd7e9d6a 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -103,7 +103,7 @@ pub fn maxsize(&self) -> usize {
 ///# fn no_run() -> Result<(), Error> {
 /// // SAFETY: Invalid usage for example purposes.
 /// let iomem = unsafe { IoMem::<{ core::mem::size_of::<u32>() }>::new(0xBAAAAAAD)? };
-/// iomem.write32(0x42, 0x0);
+/// iomem.write32::<0x0>(0x42);
 /// assert!(iomem.try_write32(0x42, 0x0).is_ok());
 /// assert!(iomem.try_write32(0x42, 0x4).is_err());
 /// # Ok(())
@@ -120,8 +120,8 @@ macro_rules! define_read {
         /// time, the build will fail.
         $(#[$attr])*
         #[inline]
-        pub fn $name(&self, offset: usize) -> $type_name {
-            let addr = self.io_addr_assert::<$type_name>(offset);
+        pub fn $name<const OFF: usize>(&self) -> $type_name {
+            let addr = self.io_addr_assert::<$type_name, OFF>();
 
             // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
             unsafe { bindings::$c_fn(addr as *const c_void) }
@@ -149,8 +149,8 @@ macro_rules! define_write {
         /// time, the build will fail.
         $(#[$attr])*
         #[inline]
-        pub fn $name(&self, value: $type_name, offset: usize) {
-            let addr = self.io_addr_assert::<$type_name>(offset);
+        pub fn $name<const OFF: usize>(&self, value: $type_name) {
+            let addr = self.io_addr_assert::<$type_name, OFF>();
 
             // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
             unsafe { bindings::$c_fn(value, addr as *mut c_void) }
@@ -217,10 +217,12 @@ fn io_addr<U>(&self, offset: usize) -> Result<usize> {
     }
 
     #[inline]
-    fn io_addr_assert<U>(&self, offset: usize) -> usize {
-        build_assert!(Self::offset_valid::<U>(offset, SIZE));
+    fn io_addr_assert<U, const OFF: usize>(&self) -> usize {
+        const {
+            build_assert!(Self::offset_valid::<U>(OFF, SIZE));
+        }
 
-        self.addr() + offset
+        self.addr() + OFF
     }
 
     define_read!(read8, try_read8, readb -> u8);
diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs
index 6f99510bfc3a63dd72c1d47dc661dcd48fa7f54e..b73557f5f57c955ac251a46c9bdd6df0687411e2 100644
--- a/rust/kernel/io/mem.rs
+++ b/rust/kernel/io/mem.rs
@@ -54,7 +54,7 @@ pub(crate) unsafe fn new(device: &'a Device<Bound>, resource: &'a Resource) -> S
     ///       pdev: &platform::Device<Core>,
     ///       info: Option<&Self::IdInfo>,
     ///    ) -> Result<Pin<KBox<Self>>> {
-    ///       let offset = 0; // Some offset.
+    ///       const OFFSET: usize = 0; // Some offset.
     ///
     ///       // If the size is known at compile time, use [`Self::iomap_sized`].
     ///       //
@@ -66,9 +66,9 @@ pub(crate) unsafe fn new(device: &'a Device<Bound>, resource: &'a Resource) -> S
     ///       let io = iomem.access(pdev.as_ref())?;
     ///
     ///       // Read and write a 32-bit value at `offset`.
-    ///       let data = io.read32_relaxed(offset);
+    ///       let data = io.read32_relaxed::<OFFSET>();
     ///
-    ///       io.write32_relaxed(data, offset);
+    ///       io.write32_relaxed::<OFFSET>(data);
     ///
     ///       # Ok(KBox::new(SampleDriver, GFP_KERNEL)?.into())
     ///     }
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 606946ff4d7fd98e206ee6420a620d1c44eb0377..6f0388853e2b36e0800df5125a5dd8b20a6d5912 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -46,17 +46,17 @@ struct SampleDriver {
 impl SampleDriver {
     fn testdev(index: &TestIndex, bar: &Bar0) -> Result<u32> {
         // Select the test.
-        bar.write8(index.0, Regs::TEST);
+        bar.write8::<{ Regs::TEST }>(index.0);
 
-        let offset = u32::from_le(bar.read32(Regs::OFFSET)) as usize;
-        let data = bar.read8(Regs::DATA);
+        let offset = u32::from_le(bar.read32::<{ Regs::OFFSET }>()) as usize;
+        let data = bar.read8::<{ Regs::DATA }>();
 
         // Write `data` to `offset` to increase `count` by one.
         //
         // Note that we need `try_write8`, since `offset` can't be checked at compile-time.
         bar.try_write8(data, offset)?;
 
-        Ok(bar.read32(Regs::COUNT))
+        Ok(bar.read32::<{ Regs::COUNT }>())
     }
 }
 
@@ -98,7 +98,7 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> Result<Pin<KBox<Self>
     fn unbind(pdev: &pci::Device<Core>, this: Pin<&Self>) {
         if let Ok(bar) = this.bar.access(pdev.as_ref()) {
             // Reset pci-testdev by writing a new test index.
-            bar.write8(this.index.0, Regs::TEST);
+            bar.write8::<{ Regs::TEST }>(this.index.0);
         }
     }
 }

---
base-commit: cf4fd52e323604ccfa8390917593e1fb965653ee
change-id: 20250918-write-offset-const-0b231c4282ea

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>


             reply	other threads:[~2025-09-18 15:02 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-18 15:02 Alice Ryhl [this message]
2025-09-18 18:13 ` [PATCH] rust: io: use const generics for read/write offsets Joel Fernandes
2025-09-18 23:26   ` Danilo Krummrich
2025-09-19  7:59     ` Joel Fernandes
2025-09-19  9:26       ` Benno Lossin
2025-09-19 20:53         ` Joel Fernandes
2025-09-22  6:25           ` Alexandre Courbot
2025-09-19 20:56     ` Gary Guo
2025-09-19 23:56       ` Danilo Krummrich

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=20250918-write-offset-const-v1-1-eb51120d4117@google.com \
    --to=aliceryhl@google.com \
    --cc=a.hindborg@kernel.org \
    --cc=airlied@gmail.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=joelagnelf@nvidia.com \
    --cc=kwilczynski@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rafael@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tmgross@umich.edu \
    --cc=tzimmermann@suse.de \
    /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.