dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] rust: add support for Port io
@ 2025-05-09  3:15 Andrew Ballance
  2025-05-09  3:15 ` [PATCH 01/11] rust: helpers: io: use macro to generate io accessor functions Andrew Ballance
                   ` (12 more replies)
  0 siblings, 13 replies; 23+ messages in thread
From: Andrew Ballance @ 2025-05-09  3:15 UTC (permalink / raw)
  To: dakr, airlied, simona, akpm, ojeda, alex.gaynor, boqun.feng, gary,
	bjorn3_gh, benno.lossin, a.hindborg, aliceryhl, tmgross, gregkh,
	rafael, bhelgaas, kwilczynski, raag.jadav, andriy.shevchenko,
	arnd, me, andrewjballance, fujita.tomonori, daniel.almeida
  Cc: nouveau, dri-devel, linux-kernel, rust-for-linux, linux-pci

currently the rust `Io` type maps to the c read{b, w, l, q}/write{b, w, l, q}
functions and have no support for port io.this is a problem for pci::Bar
because the pointer returned by pci_iomap is expected to accessed with
the ioread/iowrite api [0].

this patch series splits the `Io` type into `Io`, `PortIo` and `MMIo`.and,
updates pci::Bar, as suggested in the zulip[1], so that it is generic over
Io and, a user can optionally give a compile time hint about the type of io. 

Link: https://docs.kernel.org/6.11/driver-api/pci/pci.html#c.pci_iomap [0]
Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/.60IoRaw.60.20and.20.60usize.60/near/514788730 [1]

Andrew Ballance (6):
  rust: io: add new Io type
  rust: io: add from_raw_cookie functions
  rust: pci: make Bar generic over Io
  samples: rust: rust_driver_pci: update to use new bar and io api
  gpu: nova-core: update to use the new bar and io api
  rust: devres: fix doctest

Fiona Behrens (5):
  rust: helpers: io: use macro to generate io accessor functions
  rust: io: Replace Io with MMIo using IoAccess trait
  rust: io: implement Debug for IoRaw and add some doctests
  rust: io: add PortIo
  io: move PIO_OFFSET to linux/io.h

 drivers/gpu/nova-core/driver.rs |   4 +-
 drivers/gpu/nova-core/regs.rs   |   1 +
 include/linux/io.h              |  13 +
 lib/iomap.c                     |  13 -
 rust/helpers/io.c               | 132 +++---
 rust/kernel/devres.rs           |   4 +-
 rust/kernel/io.rs               | 753 +++++++++++++++++++++++++-------
 rust/kernel/pci.rs              |  88 +++-
 samples/rust/rust_driver_pci.rs |   6 +-
 9 files changed, 731 insertions(+), 283 deletions(-)


base-commit: 92a09c47464d040866cf2b4cd052bc60555185fb
-- 
2.49.0


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 01/11] rust: helpers: io: use macro to generate io accessor functions
  2025-05-09  3:15 [PATCH 00/11] rust: add support for Port io Andrew Ballance
@ 2025-05-09  3:15 ` Andrew Ballance
  2025-05-09  5:32   ` Arnd Bergmann
  2025-05-13  1:59   ` kernel test robot
  2025-05-09  3:15 ` [PATCH 02/11] rust: io: Replace Io with MMIo using IoAccess trait Andrew Ballance
                   ` (11 subsequent siblings)
  12 siblings, 2 replies; 23+ messages in thread
From: Andrew Ballance @ 2025-05-09  3:15 UTC (permalink / raw)
  To: dakr, airlied, simona, akpm, ojeda, alex.gaynor, boqun.feng, gary,
	bjorn3_gh, benno.lossin, a.hindborg, aliceryhl, tmgross, gregkh,
	rafael, bhelgaas, kwilczynski, raag.jadav, andriy.shevchenko,
	arnd, me, andrewjballance, fujita.tomonori, daniel.almeida
  Cc: nouveau, dri-devel, linux-kernel, rust-for-linux, linux-pci

From: Fiona Behrens <me@kloenk.dev>

Generate the `rust_helper_read{b,w,l,q}`, `rust_helper_write{b,w,l,q}`
and the relaxed version using a C macro.

This removes a lot of redundant code and is in preparation for pio
functions which uses a similar C macro.

Signed-off-by: Fiona Behrens <me@kloenk.dev>
Signed-off-by: Andrew Ballance <andrewjballance@gmail.com>
---
 rust/helpers/io.c | 104 +++++++++++++---------------------------------
 1 file changed, 28 insertions(+), 76 deletions(-)

diff --git a/rust/helpers/io.c b/rust/helpers/io.c
index 15ea187c5466..525af02f209e 100644
--- a/rust/helpers/io.c
+++ b/rust/helpers/io.c
@@ -12,90 +12,42 @@ void rust_helper_iounmap(void __iomem *addr)
 	iounmap(addr);
 }
 
-u8 rust_helper_readb(const void __iomem *addr)
-{
-	return readb(addr);
-}
-
-u16 rust_helper_readw(const void __iomem *addr)
-{
-	return readw(addr);
-}
-
-u32 rust_helper_readl(const void __iomem *addr)
-{
-	return readl(addr);
-}
-
+#define define_rust_mmio_read_helper(name, type)    \
+	type rust_helper_##name(void __iomem *addr) \
+	{                                           \
+		return name(addr);                  \
+	}
+
+#define define_rust_mmio_write_helper(name, type)               \
+	void rust_helper_##name(type value, void __iomem *addr) \
+	{                                                       \
+		name(value, addr);                              \
+	}
+
+define_rust_mmio_read_helper(readb, u8);
+define_rust_mmio_read_helper(readw, u16);
+define_rust_mmio_read_helper(readl, u32);
 #ifdef CONFIG_64BIT
-u64 rust_helper_readq(const void __iomem *addr)
-{
-	return readq(addr);
-}
+define_rust_mmio_read_helper(readq, u64);
 #endif
 
-void rust_helper_writeb(u8 value, void __iomem *addr)
-{
-	writeb(value, addr);
-}
-
-void rust_helper_writew(u16 value, void __iomem *addr)
-{
-	writew(value, addr);
-}
-
-void rust_helper_writel(u32 value, void __iomem *addr)
-{
-	writel(value, addr);
-}
-
+define_rust_mmio_write_helper(writeb, u8);
+define_rust_mmio_write_helper(writew, u16);
+define_rust_mmio_write_helper(writel, u32);
 #ifdef CONFIG_64BIT
-void rust_helper_writeq(u64 value, void __iomem *addr)
-{
-	writeq(value, addr);
-}
+define_rust_mmio_write_helper(writeq, u64);
 #endif
 
-u8 rust_helper_readb_relaxed(const void __iomem *addr)
-{
-	return readb_relaxed(addr);
-}
-
-u16 rust_helper_readw_relaxed(const void __iomem *addr)
-{
-	return readw_relaxed(addr);
-}
-
-u32 rust_helper_readl_relaxed(const void __iomem *addr)
-{
-	return readl_relaxed(addr);
-}
-
+define_rust_mmio_read_helper(readb_relaxed, u8);
+define_rust_mmio_read_helper(readw_relaxed, u16);
+define_rust_mmio_read_helper(readl_relaxed, u32);
 #ifdef CONFIG_64BIT
-u64 rust_helper_readq_relaxed(const void __iomem *addr)
-{
-	return readq_relaxed(addr);
-}
+define_rust_mmio_read_helper(readq_relaxed, u64);
 #endif
 
-void rust_helper_writeb_relaxed(u8 value, void __iomem *addr)
-{
-	writeb_relaxed(value, addr);
-}
-
-void rust_helper_writew_relaxed(u16 value, void __iomem *addr)
-{
-	writew_relaxed(value, addr);
-}
-
-void rust_helper_writel_relaxed(u32 value, void __iomem *addr)
-{
-	writel_relaxed(value, addr);
-}
-
+define_rust_mmio_write_helper(writeb_relaxed, u8);
+define_rust_mmio_write_helper(writew_relaxed, u16);
+define_rust_mmio_write_helper(writel_relaxed, u32);
 #ifdef CONFIG_64BIT
-void rust_helper_writeq_relaxed(u64 value, void __iomem *addr)
-{
-	writeq_relaxed(value, addr);
-}
+define_rust_mmio_write_helper(writeq_relaxed, u64);
 #endif
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 02/11] rust: io: Replace Io with MMIo using IoAccess trait
  2025-05-09  3:15 [PATCH 00/11] rust: add support for Port io Andrew Ballance
  2025-05-09  3:15 ` [PATCH 01/11] rust: helpers: io: use macro to generate io accessor functions Andrew Ballance
@ 2025-05-09  3:15 ` Andrew Ballance
  2025-05-12 20:07   ` Bjorn Helgaas
  2025-05-09  3:15 ` [PATCH 03/11] rust: io: implement Debug for IoRaw and add some doctests Andrew Ballance
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Andrew Ballance @ 2025-05-09  3:15 UTC (permalink / raw)
  To: dakr, airlied, simona, akpm, ojeda, alex.gaynor, boqun.feng, gary,
	bjorn3_gh, benno.lossin, a.hindborg, aliceryhl, tmgross, gregkh,
	rafael, bhelgaas, kwilczynski, raag.jadav, andriy.shevchenko,
	arnd, me, andrewjballance, fujita.tomonori, daniel.almeida
  Cc: nouveau, dri-devel, linux-kernel, rust-for-linux, linux-pci

From: Fiona Behrens <me@kloenk.dev>

Replace the Io struct with a new MMIo struct that uses the different
traits (`IoAccess`, `IoAccess64`, `IoAccessRelaxed` and
`IoAccess64Relaxed).
This allows to later implement PortIo and a generic Io framework.

Signed-off-by: Fiona Behrens <me@kloenk.dev>
Co-developed-by: Andrew Ballance <andrewjballance@gmail.com>
Signed-off-by: Andrew Ballance <andrewjballance@gmail.com>
---
 rust/kernel/io.rs | 443 +++++++++++++++++++++++++++-------------------
 1 file changed, 259 insertions(+), 184 deletions(-)

diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 72d80a6f131e..19bbf802027c 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -7,6 +7,211 @@
 use crate::error::{code::EINVAL, Result};
 use crate::{bindings, build_assert};
 
+/// Private macro to define the [`IoAccess`] functions.
+macro_rules! define_io_access_function {
+    (@read_derived $(#[$attr:meta])* $name_unchecked:ident, $vis:vis $name:ident, $try_vis:vis $try_name:ident, $type_name:ty) => {
+    /// Read data from a give offset known at compile time.
+    ///
+    /// Bound checks are perfomed on compile time, hence if the offset is not known at compile
+    /// time, the build will fail.
+    $(#[$attr])*
+    #[inline]
+    $vis fn $name(&self, offset: usize) -> $type_name {
+        build_assert!(offset_valid::<$type_name>(offset, SIZE));
+
+        // SAFETY: offset checked to be valid above.
+        unsafe { self.$name_unchecked(offset) }
+    }
+
+    /// Read data from a given offset.
+    ///
+    /// Bound checks are performed on runtime, it fails if the offset (plus type size) is
+    /// out of bounds.
+    $(#[$attr])*
+    #[inline]
+    $try_vis fn $try_name(&self, offset: usize) -> Result<$type_name> {
+        if !(offset_valid::<$type_name>(offset, self.maxsize())) {
+            return Err(EINVAL);
+        }
+
+        // SAFETY: offset checked to be valid above.
+        Ok(unsafe { self.$name_unchecked(offset) })
+    }
+    };
+    (@read $(#[$attr:meta])* $name_unchecked:ident, $name:ident, $try_name:ident, $type_name:ty) => {
+    /// Read data from a given offset without doing any bound checks.
+    /// The offset is relative to the base address of Self.
+    ///
+    /// # Safety
+    ///
+    /// The offset has to be valid for self.
+    $(#[$attr])*
+    unsafe fn $name_unchecked(&self, offset: usize) -> $type_name;
+
+    define_io_access_function!(@read_derived $(#[$attr])* $name_unchecked, $name, $try_name, $type_name);
+    };
+    (@read $($(#[$attr:meta])* $name_unchecked:ident, $name:ident, $try_name:ident, $type_name:ty;)+) => {
+    $(
+        define_io_access_function!(@read $(#[$attr])* $name_unchecked, $name, $try_name, $type_name);
+    )*
+    };
+    (@write_derived $(#[$attr:meta])* $name_unchecked:ident, $vis:vis $name:ident, $try_vis:vis $try_name:ident, $type_name:ty) => {
+    /// Write data to a given offset known at compile time.
+    /// Bound checks are performed on compile time, hence if the offset is not known at compile
+    /// time, the build will fail.
+    $(#[$attr])*
+    #[inline]
+    $vis fn $name(&self, value: $type_name, offset: usize) {
+        build_assert!(offset_valid::<$type_name>(offset, SIZE));
+
+        // SAFETY: offset checked to be valid above.
+        unsafe { self.$name_unchecked(value, offset) }
+    }
+
+    /// Write data to a given offset.
+    ///
+    /// Bound checks are performed on runtime, it fails if the offset (plus the type size) is
+    /// out of bounds.
+    $(#[$attr])*
+        #[inline]
+    $try_vis fn $try_name(&self, value: $type_name, offset: usize) -> Result {
+        if !(offset_valid::<$type_name>(offset, self.maxsize())) {
+            return Err(EINVAL);
+        }
+
+        // SAFETY: offset checked to be valid above.
+        Ok(unsafe { self.$name_unchecked(value, offset) })
+    }
+    };
+    (@write $(#[$attr:meta])* $name_unchecked:ident, $name:ident, $try_name:ident, $type_name:ty) => {
+    /// Write data to a given offset without doing any bound checks.
+    /// The offset is relative to the base address of self.
+    ///
+    /// # Safety
+    ///
+    /// The offset has to be valid for Self.
+    $(#[$attr])*
+    unsafe fn $name_unchecked(&self, value: $type_name, offset: usize);
+
+    define_io_access_function!(@write_derived $(#[$attr])* $name_unchecked, $name, $try_name, $type_name);
+    };
+    (@write $($(#[$attr:meta])* $name_unchecked:ident, $name:ident, $try_name:ident, $type_name:ty;)+) => {
+    $(
+        define_io_access_function!(@write $(#[$attr])* $name_unchecked, $name, $try_name, $type_name);
+    )*
+    };
+}
+
+/// Private macro to generate accessor functions that call the correct C functions given as `fn_c`.
+///
+/// This Takes either `@read` or `@write` to generate a single read or write accessor function.
+///
+/// This also can take a list of read write pairs to generate both at the same time.
+macro_rules! impl_accessor_fn {
+    (@read $(#[$attr:meta])* $vis:vis $fn_rust:ident, $fn_c:ident, $type_name:ty) => {
+    $(#[$attr])*
+    $vis unsafe fn $fn_rust(&self, offset: usize) -> $type_name {
+        // SAFETY: by the safety requirement of the function `self.addr() + offset` is valid to read
+        // TODO: once MSRV is >= 1.79.0 replace `+` with `unchecked_add`
+        unsafe { bindings::$fn_c((self.addr() + offset) as _) as _ }
+    }
+    };
+    (@write $(#[$attr:meta])* $vis:vis $fn_rust:ident, $fn_c:ident, $type_name:ty) => {
+    $(#[$attr])*
+    $vis unsafe fn $fn_rust(&self, value: $type_name, offset: usize) {
+        // SAFETY:
+        // by the safety requirement of the function `self.addr() + offset` is valid to write
+        // TODO: once MSRV is >= 1.79.0 replace `+` with `unchecked_add`
+        unsafe { bindings::$fn_c(value, (self.addr() + offset) as _) as _ }
+    }
+    };
+    (
+    $(
+        $(#[$attr:meta])*
+        $vis_read:vis $fn_rust_read:ident, $fn_c_read:ident,
+        $vis_write:vis $fn_rust_write:ident, $fn_c_write:ident,
+        $type_name:ty $(;)?
+    )+
+    ) => {
+    $(
+        impl_accessor_fn!(@read $(#[$attr])* $vis_read $fn_rust_read, $fn_c_read, $type_name);
+        impl_accessor_fn!(@write $(#[$attr])* $vis_write $fn_rust_write, $fn_c_write, $type_name);
+    )+
+    };
+}
+
+/// Check if the offset is valid to still support the type U in the given size
+const fn offset_valid<U>(offset: usize, size: usize) -> bool {
+    let type_size = core::mem::size_of::<U>();
+    if let Some(end) = offset.checked_add(type_size) {
+        end <= size && offset % type_size == 0
+    } else {
+        false
+    }
+}
+
+/// Io Access functions.
+///
+/// # Safety
+///
+/// `SIZE` and `maxsize()` has to always be valid to add to the base address.
+pub unsafe trait IoAccess<const SIZE: usize = 0> {
+    /// Returns the maximum size of the accessed IO area.
+    fn maxsize(&self) -> usize;
+
+    /// Returns the base address of the accessed IO area.
+    fn addr(&self) -> usize;
+
+    define_io_access_function!(@read
+        read8_unchecked, read8, try_read8, u8;
+        read16_unchecked, read16, try_read16, u16;
+        read32_unchecked, read32, try_read32, u32;
+    );
+
+    define_io_access_function!(@write
+        write8_unchecked, write8, try_write8, u8;
+        write16_unchecked, write16, try_write16, u16;
+        write32_unchecked, write32, try_write32, u32;
+    );
+}
+
+/// Extending trait of [`IoAccess`] offering 64 bit functions.
+#[cfg(CONFIG_64BIT)]
+pub trait IoAccess64<const SIZE: usize = 0>: IoAccess<SIZE> {
+    define_io_access_function!(@read read64_unchecked, read64, try_read64, u64);
+    define_io_access_function!(@write write64_unchecked, write64, try_write64, u64);
+}
+
+/// Io Relaxed Access functions.
+///
+/// Similar to [`IoAccess`] but using relaxed memory boundries. For [`PortIo`] (when [`Io`]
+/// is a PortIo address) this just calls the functions from [`IoAccess`].
+pub trait IoAccessRelaxed<const SIZE: usize = 0>: IoAccess<SIZE> {
+    define_io_access_function!(@read
+        read8_relaxed_unchecked, read8_relaxed, try_read8_relaxed, u8;
+        read16_relaxed_unchecked, read16_relaxed, try_read16_relaxed, u16;
+        read32_relaxed_unchecked, read32_relaxed, try_read32_relaxed, u32;
+    );
+
+    define_io_access_function!(@write
+        write8_relaxed_unchecked, write8_relaxed, try_write8_relaxed, u8;
+        write16_relaxed_unchecked, write16_relaxed, try_write16_relaxed, u16;
+        write32_relaxed_unchecked, write32_relaxed, try_write32_relaxed, u32;
+    );
+}
+
+/// Extending trait of [`IoAccessRelaxed`] offering 64 bit functions.
+#[cfg(CONFIG_64BIT)]
+pub trait IoAccess64Relaxed<const SIZE: usize = 0>: IoAccess<SIZE> {
+    define_io_access_function!(@read
+        read64_relaxed_unchecked, read64_relaxed, try_read64_relaxed, u64;
+    );
+
+    define_io_access_function!(@write
+        write64_relaxed_unchecked, write64_relaxed, try_write64_relaxed, u64;
+    );
+}
+
 /// Raw representation of an MMIO region.
 ///
 /// By itself, the existence of an instance of this structure does not provide any guarantees that
@@ -43,218 +248,88 @@ pub fn maxsize(&self) -> usize {
     }
 }
 
-/// IO-mapped memory, starting at the base address @addr and spanning @maxlen bytes.
+/// IO-mapped memory, starting at the base address [`addr`] and spanning [`maxsize`] bytes.
 ///
 /// The creator (usually a subsystem / bus such as PCI) is responsible for creating the
-/// mapping, performing an additional region request etc.
-///
-/// # Invariant
-///
-/// `addr` is the start and `maxsize` the length of valid I/O mapped memory region of size
-/// `maxsize`.
-///
-/// # Examples
-///
-/// ```no_run
-/// # use kernel::{bindings, io::{Io, IoRaw}};
-/// # use core::ops::Deref;
+/// mapping, performing an additional region request, etc.
 ///
-/// // See also [`pci::Bar`] for a real example.
-/// struct IoMem<const SIZE: usize>(IoRaw<SIZE>);
+/// # Invariants
 ///
-/// impl<const SIZE: usize> IoMem<SIZE> {
-///     /// # Safety
-///     ///
-///     /// [`paddr`, `paddr` + `SIZE`) must be a valid MMIO region that is mappable into the CPUs
-///     /// virtual address space.
-///     unsafe fn new(paddr: usize) -> Result<Self>{
-///         // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
-///         // valid for `ioremap`.
-///         let addr = unsafe { bindings::ioremap(paddr as _, SIZE as _) };
-///         if addr.is_null() {
-///             return Err(ENOMEM);
-///         }
+/// [`addr`] is the start and [`maxsize`] the length of valid I/O mapped memory region of
+/// size [`maxsize`].
 ///
-///         Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
-///     }
-/// }
+/// [`addr`] is valid to access with the C [`read`]/[`write`] family of functions.
 ///
-/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
-///     fn drop(&mut self) {
-///         // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
-///         unsafe { bindings::iounmap(self.0.addr() as _); };
-///     }
-/// }
-///
-/// impl<const SIZE: usize> Deref for IoMem<SIZE> {
-///    type Target = Io<SIZE>;
-///
-///    fn deref(&self) -> &Self::Target {
-///         // SAFETY: The memory range stored in `self` has been properly mapped in `Self::new`.
-///         unsafe { Io::from_raw(&self.0) }
-///    }
-/// }
-///
-///# 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);
-/// assert!(iomem.try_write32(0x42, 0x0).is_ok());
-/// assert!(iomem.try_write32(0x42, 0x4).is_err());
-/// # Ok(())
-/// # }
-/// ```
+/// [`addr`]: IoAccess::addr
+/// [`maxsize`]: IoAccess::maxsize
+/// [`read`]: https://docs.kernel.org/driver-api/device-io.html#differences-between-i-o-access-functions
+/// [`write`]: https://docs.kernel.org/driver-api/device-io.html#differences-between-i-o-access-functions
 #[repr(transparent)]
-pub struct Io<const SIZE: usize = 0>(IoRaw<SIZE>);
-
-macro_rules! define_read {
-    ($(#[$attr:meta])* $name:ident, $try_name: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
-        /// time, the build will fail.
-        $(#[$attr])*
-        #[inline]
-        pub 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 _) }
-        }
-
-        /// Read IO data from a given offset.
-        ///
-        /// Bound checks are performed on runtime, it fails if the offset (plus the type size) is
-        /// out of bounds.
-        $(#[$attr])*
-        pub fn $try_name(&self, offset: usize) -> Result<$type_name> {
-            let addr = self.io_addr::<$type_name>(offset)?;
-
-            // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
-            Ok(unsafe { bindings::$c_fn(addr as _) })
-        }
-    };
-}
-
-macro_rules! define_write {
-    ($(#[$attr:meta])* $name:ident, $try_name:ident, $c_fn:ident <- $type_name:ty) => {
-        /// Write 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
-        /// 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);
-
-            // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
-            unsafe { bindings::$c_fn(value, addr as _, ) }
-        }
+pub struct MMIo<const SIZE: usize = 0>(IoRaw<SIZE>);
 
-        /// Write IO data from a given offset.
-        ///
-        /// Bound checks are performed on runtime, it fails if the offset (plus the type size) is
-        /// out of bounds.
-        $(#[$attr])*
-        pub fn $try_name(&self, value: $type_name, offset: usize) -> Result {
-            let addr = self.io_addr::<$type_name>(offset)?;
-
-            // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
-            unsafe { bindings::$c_fn(value, addr as _) }
-            Ok(())
-        }
-    };
-}
-
-impl<const SIZE: usize> Io<SIZE> {
-    /// Converts an `IoRaw` into an `Io` instance, providing the accessors to the MMIO mapping.
+impl<const SIZE: usize> MMIo<SIZE> {
+    /// Convert a [`IoRaw`] 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: &IoRaw<SIZE>) -> &Self {
-        // SAFETY: `Io` is a transparent wrapper around `IoRaw`.
-        unsafe { &*core::ptr::from_ref(raw).cast() }
-    }
-
-    /// Returns the base address of this mapping.
+    /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of
+    /// size `maxsize`.
     #[inline]
-    pub fn addr(&self) -> usize {
-        self.0.addr()
-    }
-
-    /// Returns the maximum size of this mapping.
-    #[inline]
-    pub fn maxsize(&self) -> usize {
-        self.0.maxsize()
+    pub unsafe fn from_raw(raw: IoRaw<SIZE>) -> Self {
+        Self(raw)
     }
 
+    /// Convert a ref to [`IoRaw`] 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`.
     #[inline]
-    const fn offset_valid<U>(offset: usize, size: usize) -> bool {
-        let type_size = core::mem::size_of::<U>();
-        if let Some(end) = offset.checked_add(type_size) {
-            end <= size && offset % type_size == 0
-        } else {
-            false
-        }
+    pub unsafe fn from_raw_ref(raw: &IoRaw<SIZE>) -> &Self {
+        // SAFETY: `MMIo` is a transparent wrapper around `IoRaw`.
+        unsafe { &*core::ptr::from_ref(raw).cast() }
     }
+}
 
+// SAFETY: as per invariant `raw` is valid
+unsafe impl<const SIZE: usize> IoAccess<SIZE> for MMIo<SIZE> {
     #[inline]
-    fn io_addr<U>(&self, offset: usize) -> Result<usize> {
-        if !Self::offset_valid::<U>(offset, self.maxsize()) {
-            return Err(EINVAL);
-        }
-
-        // Probably no need to check, since the safety requirements of `Self::new` guarantee that
-        // this can't overflow.
-        self.addr().checked_add(offset).ok_or(EINVAL)
+    fn maxsize(&self) -> usize {
+        self.0.maxsize()
     }
 
     #[inline]
-    fn io_addr_assert<U>(&self, offset: usize) -> usize {
-        build_assert!(Self::offset_valid::<U>(offset, SIZE));
-
-        self.addr() + offset
+    fn addr(&self) -> usize {
+        self.0.addr()
     }
 
-    define_read!(read8, try_read8, readb -> u8);
-    define_read!(read16, try_read16, readw -> u16);
-    define_read!(read32, try_read32, readl -> u32);
-    define_read!(
-        #[cfg(CONFIG_64BIT)]
-        read64,
-        try_read64,
-        readq -> u64
+    impl_accessor_fn!(
+        read8_unchecked, readb, write8_unchecked, writeb, u8;
+        read16_unchecked, readw, write16_unchecked, writew, u16;
+        read32_unchecked, readl, write32_unchecked, writel, u32;
     );
+}
 
-    define_read!(read8_relaxed, try_read8_relaxed, readb_relaxed -> u8);
-    define_read!(read16_relaxed, try_read16_relaxed, readw_relaxed -> u16);
-    define_read!(read32_relaxed, try_read32_relaxed, readl_relaxed -> u32);
-    define_read!(
-        #[cfg(CONFIG_64BIT)]
-        read64_relaxed,
-        try_read64_relaxed,
-        readq_relaxed -> u64
+#[cfg(CONFIG_64BIT)]
+impl<const SIZE: usize> IoAccess64<SIZE> for MMIo<SIZE> {
+    impl_accessor_fn!(
+        read64_unchecked, readq, write64_unchecked, writeq, u64;
     );
+}
 
-    define_write!(write8, try_write8, writeb <- u8);
-    define_write!(write16, try_write16, writew <- u16);
-    define_write!(write32, try_write32, writel <- u32);
-    define_write!(
-        #[cfg(CONFIG_64BIT)]
-        write64,
-        try_write64,
-        writeq <- u64
+impl<const SIZE: usize> IoAccessRelaxed<SIZE> for MMIo<SIZE> {
+    impl_accessor_fn!(
+        read8_relaxed_unchecked, readb_relaxed, write8_relaxed_unchecked, writeb_relaxed, u8;
+        read16_relaxed_unchecked, readw_relaxed, write16_relaxed_unchecked, writew_relaxed, u16;
+        read32_relaxed_unchecked, readl_relaxed, write32_relaxed_unchecked, writel_relaxed, u32;
     );
+}
 
-    define_write!(write8_relaxed, try_write8_relaxed, writeb_relaxed <- u8);
-    define_write!(write16_relaxed, try_write16_relaxed, writew_relaxed <- u16);
-    define_write!(write32_relaxed, try_write32_relaxed, writel_relaxed <- u32);
-    define_write!(
-        #[cfg(CONFIG_64BIT)]
-        write64_relaxed,
-        try_write64_relaxed,
-        writeq_relaxed <- u64
+#[cfg(CONFIG_64BIT)]
+impl<const SIZE: usize> IoAccess64Relaxed<SIZE> for MMIo<SIZE> {
+    impl_accessor_fn!(
+        read64_relaxed_unchecked, readq_relaxed, write64_relaxed_unchecked, writeq_relaxed, u64;
     );
 }
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 03/11] rust: io: implement Debug for IoRaw and add some doctests
  2025-05-09  3:15 [PATCH 00/11] rust: add support for Port io Andrew Ballance
  2025-05-09  3:15 ` [PATCH 01/11] rust: helpers: io: use macro to generate io accessor functions Andrew Ballance
  2025-05-09  3:15 ` [PATCH 02/11] rust: io: Replace Io with MMIo using IoAccess trait Andrew Ballance
@ 2025-05-09  3:15 ` Andrew Ballance
  2025-05-09  3:15 ` [PATCH 04/11] rust: io: add PortIo Andrew Ballance
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Andrew Ballance @ 2025-05-09  3:15 UTC (permalink / raw)
  To: dakr, airlied, simona, akpm, ojeda, alex.gaynor, boqun.feng, gary,
	bjorn3_gh, benno.lossin, a.hindborg, aliceryhl, tmgross, gregkh,
	rafael, bhelgaas, kwilczynski, raag.jadav, andriy.shevchenko,
	arnd, me, andrewjballance, fujita.tomonori, daniel.almeida
  Cc: nouveau, dri-devel, linux-kernel, rust-for-linux, linux-pci

From: Fiona Behrens <me@kloenk.dev>

Implement `Debug` for `kernel::io::IoRaw` which also outputs the const
generic SIZE as a field.

Add some doctests to `IoRaw::new` and `MMIo::from_raw(_ref)`.

Signed-off-by: Fiona Behrens <me@kloenk.dev>
Signed-off-by: Andrew Ballance <andrewjballance@gmail.com>
---
 rust/kernel/io.rs | 62 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 19bbf802027c..09440dd3e73b 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -227,6 +227,33 @@ pub struct IoRaw<const SIZE: usize = 0> {
 
 impl<const SIZE: usize> IoRaw<SIZE> {
     /// Returns a new `IoRaw` instance on success, an error otherwise.
+    ///
+    /// # Examples
+    ///
+    /// Const generic size 0, only allowing runtime checks:
+    /// ```
+    /// use kernel::io::IoRaw;
+    ///
+    /// let raw: IoRaw<0> = IoRaw::new(0xDEADBEEFC0DE, 8).unwrap();
+    /// # assert_eq!(raw.addr(), 0xDEADBEEFC0DE);
+    /// # assert_eq!(raw.maxsize(), 8);
+    /// ```
+    ///
+    /// Const generic size equals maxsize:
+    /// ```
+    /// use kernel::io::IoRaw;
+    ///
+    /// let raw: IoRaw<8> = IoRaw::new(0xDEADBEEFC0DE, 8).unwrap();
+    /// # assert_eq!(raw.addr(), 0xDEADBEEFC0DE);
+    /// # assert_eq!(raw.maxsize(), 8);
+    /// ```
+    ///
+    /// Const generic size bigger then maxsize:
+    /// ```
+    /// use kernel::io::IoRaw;
+    ///
+    /// IoRaw::<16>::new(0xDEADBEEFC0DE, 8).unwrap_err();
+    /// ```
     pub fn new(addr: usize, maxsize: usize) -> Result<Self> {
         if maxsize < SIZE {
             return Err(EINVAL);
@@ -248,6 +275,16 @@ pub fn maxsize(&self) -> usize {
     }
 }
 
+impl<const SIZE: usize> core::fmt::Debug for IoRaw<SIZE> {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        f.debug_struct("IoRaw")
+            .field("SIZE", &SIZE)
+            .field("addr", &self.addr)
+            .field("maxsize", &self.maxsize)
+            .finish()
+    }
+}
+
 /// IO-mapped memory, starting at the base address [`addr`] and spanning [`maxsize`] bytes.
 ///
 /// The creator (usually a subsystem / bus such as PCI) is responsible for creating the
@@ -264,6 +301,7 @@ pub fn maxsize(&self) -> usize {
 /// [`maxsize`]: IoAccess::maxsize
 /// [`read`]: https://docs.kernel.org/driver-api/device-io.html#differences-between-i-o-access-functions
 /// [`write`]: https://docs.kernel.org/driver-api/device-io.html#differences-between-i-o-access-functions
+#[derive(Debug)]
 #[repr(transparent)]
 pub struct MMIo<const SIZE: usize = 0>(IoRaw<SIZE>);
 
@@ -274,6 +312,18 @@ impl<const SIZE: usize> MMIo<SIZE> {
     ///
     /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of
     /// size `maxsize`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::io::{IoRaw, MMIo, IoAccess};
+    ///
+    /// let raw = IoRaw::<2>::new(0xDEADBEEFC0DE, 2).unwrap();
+    /// // SAFETY: test, value is not actually written to.
+    /// let mmio: MMIo<2> = unsafe { MMIo::from_raw(raw) };
+    /// # assert_eq!(0xDEADBEEFC0DE, mmio.addr());
+    /// # assert_eq!(2, mmio.maxsize());
+    /// ```
     #[inline]
     pub unsafe fn from_raw(raw: IoRaw<SIZE>) -> Self {
         Self(raw)
@@ -286,6 +336,18 @@ pub unsafe fn from_raw(raw: IoRaw<SIZE>) -> Self {
     ///
     /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of
     /// size `maxsize`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::io::{IoRaw, MMIo, IoAccess};
+    ///
+    /// let raw = IoRaw::<2>::new(0xDEADBEEFC0DE, 2).unwrap();
+    /// // SAFETY: test, value is not actually written to.
+    /// let mmio: &MMIo<2> = unsafe { MMIo::from_raw_ref(&raw) };
+    /// # assert_eq!(raw.addr(), mmio.addr());
+    /// # assert_eq!(raw.maxsize(), mmio.maxsize());
+    /// ```
     #[inline]
     pub unsafe fn from_raw_ref(raw: &IoRaw<SIZE>) -> &Self {
         // SAFETY: `MMIo` is a transparent wrapper around `IoRaw`.
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 04/11] rust: io: add PortIo
  2025-05-09  3:15 [PATCH 00/11] rust: add support for Port io Andrew Ballance
                   ` (2 preceding siblings ...)
  2025-05-09  3:15 ` [PATCH 03/11] rust: io: implement Debug for IoRaw and add some doctests Andrew Ballance
@ 2025-05-09  3:15 ` Andrew Ballance
  2025-05-09  6:05   ` Arnd Bergmann
  2025-05-13  6:09   ` kernel test robot
  2025-05-09  3:15 ` [PATCH 05/11] rust: io: add new Io type Andrew Ballance
                   ` (8 subsequent siblings)
  12 siblings, 2 replies; 23+ messages in thread
From: Andrew Ballance @ 2025-05-09  3:15 UTC (permalink / raw)
  To: dakr, airlied, simona, akpm, ojeda, alex.gaynor, boqun.feng, gary,
	bjorn3_gh, benno.lossin, a.hindborg, aliceryhl, tmgross, gregkh,
	rafael, bhelgaas, kwilczynski, raag.jadav, andriy.shevchenko,
	arnd, me, andrewjballance, fujita.tomonori, daniel.almeida
  Cc: nouveau, dri-devel, linux-kernel, rust-for-linux, linux-pci

From: Fiona Behrens <me@kloenk.dev>

Add `rust::io::PortIo` implementing the `IoAccess` trait.

Signed-off-by: Fiona Behrens <me@kloenk.dev>
Signed-off-by: Andrew Ballance <andrewjballance@gmail.com>
---
 rust/helpers/io.c | 20 +++++++++++
 rust/kernel/io.rs | 88 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+)

diff --git a/rust/helpers/io.c b/rust/helpers/io.c
index 525af02f209e..d439b61c672e 100644
--- a/rust/helpers/io.c
+++ b/rust/helpers/io.c
@@ -51,3 +51,23 @@ define_rust_mmio_write_helper(writel_relaxed, u32);
 #ifdef CONFIG_64BIT
 define_rust_mmio_write_helper(writeq_relaxed, u64);
 #endif
+
+#define define_rust_pio_read_helper(name, type)     \
+	type rust_helper_##name(unsigned long port) \
+	{                                           \
+		return name(port);                  \
+	}
+
+#define define_rust_pio_write_helper(name, type)                \
+	void rust_helper_##name(type value, unsigned long port) \
+	{                                                       \
+		name(value, port);                              \
+	}
+
+define_rust_pio_read_helper(inb, u8);
+define_rust_pio_read_helper(inw, u16);
+define_rust_pio_read_helper(inl, u32);
+
+define_rust_pio_write_helper(outb, u8);
+define_rust_pio_write_helper(outw, u16);
+define_rust_pio_write_helper(outl, u32);
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 09440dd3e73b..70621a016a87 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -395,3 +395,91 @@ impl<const SIZE: usize> IoAccess64Relaxed<SIZE> for MMIo<SIZE> {
         read64_relaxed_unchecked, readq_relaxed, write64_relaxed_unchecked, writeq_relaxed, u64;
     );
 }
+
+/// Port-IO, starting at the base address [`addr`] and spanning [`maxsize`] bytes.
+///
+/// The creator is responsible for performing an additional region request, etc.
+///
+/// # Invariants
+///
+/// [`addr`] is the start and [`maxsize`] the length of a valid port io region of size [`maxsize`].
+///
+/// [`addr`] is valid to access with the C [`in`]/[`out`] family of functions.
+///
+/// [`addr`]: IoAccess::addr
+/// [`maxsize`]: IoAccess::maxsize
+/// [`in`]: https://docs.kernel.org/driver-api/device-io.html#differences-between-i-o-access-functions
+/// [`out`]: https://docs.kernel.org/driver-api/device-io.html#differences-between-i-o-access-functions
+#[derive(Debug)]
+#[repr(transparent)]
+pub struct PortIo<const SIZE: usize = 0>(IoRaw<SIZE>);
+
+impl<const SIZE: usize> PortIo<SIZE> {
+    /// Convert a [`IoRaw`] into an [`PortIo`] instance, providing the accessors to the
+    /// PortIo mapping.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `addr` is the start of a valid Port I/O region of size `maxsize`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::io::{IoRaw, PortIo, IoAccess};
+    ///
+    /// let raw = IoRaw::<2>::new(0xDEADBEEFC0DE, 2).unwrap();
+    /// // SAFETY: test, value is not actually written to.
+    /// let pio: PortIo<2> = unsafe { PortIo::from_raw(raw) };
+    /// # assert_eq!(0xDEADBEEFC0DE, pio.addr());
+    /// # assert_eq!(2, pio.maxsize());
+    /// ```
+    #[inline]
+    pub unsafe fn from_raw(raw: IoRaw<SIZE>) -> Self {
+        Self(raw)
+    }
+
+    /// Convert a ref to [`IoRaw`] into an [`PortIo`] instance, providing the accessors to
+    /// the PortIo mapping.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of
+    /// size `maxsize`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::io::{IoRaw, PortIo, IoAccess};
+    ///
+    /// let raw = IoRaw::<2>::new(0xDEADBEEFC0DE, 2).unwrap();
+    /// // SAFETY: test, value is not actually written to.
+    /// let pio: &PortIo<2> = unsafe { PortIo::from_raw_ref(&raw) };
+    /// # assert_eq!(raw.addr(), pio.addr());
+    /// # assert_eq!(raw.maxsize(), pio.maxsize());
+    /// ```
+    #[inline]
+    pub unsafe fn from_raw_ref(raw: &IoRaw<SIZE>) -> &Self {
+        // SAFETY: `PortIo` is a transparent wrapper around `IoRaw`.
+        unsafe { &*core::ptr::from_ref(raw).cast() }
+    }
+}
+
+// SAFETY: as per invariant `raw` is valid
+unsafe impl<const SIZE: usize> IoAccess<SIZE> for PortIo<SIZE> {
+    #[inline]
+    fn maxsize(&self) -> usize {
+        self.0.maxsize()
+    }
+
+    #[inline]
+    fn addr(&self) -> usize {
+        self.0.addr()
+    }
+
+    #[rustfmt::skip]
+    impl_accessor_fn!(
+        read8_unchecked, inb, write8_unchecked, outb, u8;
+        read16_unchecked, inw, write16_unchecked, outw, u16;
+        read32_unchecked, inl, write32_unchecked, outl, u32;
+    );
+}
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 05/11] rust: io: add new Io type
  2025-05-09  3:15 [PATCH 00/11] rust: add support for Port io Andrew Ballance
                   ` (3 preceding siblings ...)
  2025-05-09  3:15 ` [PATCH 04/11] rust: io: add PortIo Andrew Ballance
@ 2025-05-09  3:15 ` Andrew Ballance
  2025-05-09  3:15 ` [PATCH 06/11] io: move PIO_OFFSET to linux/io.h Andrew Ballance
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Andrew Ballance @ 2025-05-09  3:15 UTC (permalink / raw)
  To: dakr, airlied, simona, akpm, ojeda, alex.gaynor, boqun.feng, gary,
	bjorn3_gh, benno.lossin, a.hindborg, aliceryhl, tmgross, gregkh,
	rafael, bhelgaas, kwilczynski, raag.jadav, andriy.shevchenko,
	arnd, me, andrewjballance, fujita.tomonori, daniel.almeida
  Cc: nouveau, dri-devel, linux-kernel, rust-for-linux, linux-pci

adds a new Io type that uses the C ioread/iowrite family of functions
and implements the IoAccess trait for it.

Signed-off-by: Andrew Ballance <andrewjballance@gmail.com>
---
 rust/helpers/io.c |  8 +++++
 rust/kernel/io.rs | 86 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+)

diff --git a/rust/helpers/io.c b/rust/helpers/io.c
index d439b61c672e..11c0c34f2eba 100644
--- a/rust/helpers/io.c
+++ b/rust/helpers/io.c
@@ -71,3 +71,11 @@ define_rust_pio_read_helper(inl, u32);
 define_rust_pio_write_helper(outb, u8);
 define_rust_pio_write_helper(outw, u16);
 define_rust_pio_write_helper(outl, u32);
+
+define_rust_mmio_read_helper(ioread8, u8);
+define_rust_mmio_read_helper(ioread16, u16);
+define_rust_mmio_read_helper(ioread32, u32);
+
+define_rust_mmio_write_helper(iowrite8, u8);
+define_rust_mmio_write_helper(iowrite16, u16);
+define_rust_mmio_write_helper(iowrite32, u32);
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 70621a016a87..3d8b6e731ce7 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -483,3 +483,89 @@ fn addr(&self) -> usize {
         read32_unchecked, inl, write32_unchecked, outl, u32;
     );
 }
+
+/// Io that can be either PortIo or MMIo,
+/// starting at the base address [`addr`] and spanning [`maxsize`] bytes.
+///
+/// The creator (usually a subsystem / bus such as PCI) is responsible for creating the
+/// mapping, performing an additional region request, etc.
+///
+/// # Invariants
+///
+/// [`addr`] is the start and [`maxsize`] the length of a valid io region of size [`maxsize`].
+///
+/// [`addr`] is valid to access with the C [`ioread`]/[`iowrite`] family of functions.
+///
+/// [`addr`]: IoAccess::addr
+/// [`maxsize`]: IoAccess::maxsize
+/// [`ioread`]: https://docs.kernel.org/driver-api/device-io.html#differences-between-i-o-access-functions
+/// [`iowrite`]: https://docs.kernel.org/driver-api/device-io.html#differences-between-i-o-access-functions
+#[derive(Debug)]
+#[repr(transparent)]
+pub struct Io<const SIZE: usize = 0>(IoRaw<SIZE>);
+
+impl<const SIZE: usize> Io<SIZE> {
+    /// Convert a [`IoRaw`] into an [`Io`] instance, providing the accessors to the
+    /// Io mapping.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `addr` is the start of a valid I/O region of size `maxsize`.
+    ///
+    /// ```
+    /// use kernel::io::{IoRaw, Io, IoAccess};
+    ///
+    /// let raw = IoRaw::<2>::new(0xDEADBEEFC0DE, 2).unwrap();
+    /// // SAFETY: test, value is not actually written to.
+    /// let io: Io<2> = unsafe { Io::from_raw(raw) };
+    /// # assert_eq!(0xDEADBEEFC0DE, io.addr());
+    /// # assert_eq!(2, io.maxsize());
+    /// ```
+    pub unsafe fn from_raw(raw: IoRaw<SIZE>) -> Self {
+        Self(raw)
+    }
+
+    /// Convert a ref to [`IoRaw`] into an [`Io`] instance, providing the accessors to
+    /// the Io mapping.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of
+    /// size `maxsize`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::io::{IoRaw, Io, IoAccess};
+    ///
+    /// let raw = IoRaw::<2>::new(0xDEADBEEFC0DE, 2).unwrap();
+    /// // SAFETY: test, value is not actually written to.
+    /// let io: &Io<2> = unsafe { Io::from_raw_ref(&raw) };
+    /// # assert_eq!(raw.addr(), io.addr());
+    /// # assert_eq!(raw.maxsize(), io.maxsize());
+    /// ```
+    #[inline]
+    pub unsafe fn from_raw_ref(raw: &IoRaw<SIZE>) -> &Self {
+        // SAFETY: `Io` is a transparent wrapper around `IoRaw`.
+        unsafe { &*core::ptr::from_ref(raw).cast() }
+    }
+}
+
+// SAFETY: as per invariant `raw` is valid
+unsafe impl<const SIZE: usize> IoAccess<SIZE> for Io<SIZE> {
+    #[inline]
+    fn addr(&self) -> usize {
+        self.0.addr()
+    }
+
+    #[inline]
+    fn maxsize(&self) -> usize {
+        self.0.maxsize()
+    }
+
+    impl_accessor_fn!(
+        read8_unchecked, ioread8, write8_unchecked, iowrite8, u8;
+        read16_unchecked, ioread16, write16_unchecked, iowrite16, u16;
+        read32_unchecked, ioread32, write32_unchecked, iowrite32, u32;
+    );
+}
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 06/11] io: move PIO_OFFSET to linux/io.h
  2025-05-09  3:15 [PATCH 00/11] rust: add support for Port io Andrew Ballance
                   ` (4 preceding siblings ...)
  2025-05-09  3:15 ` [PATCH 05/11] rust: io: add new Io type Andrew Ballance
@ 2025-05-09  3:15 ` Andrew Ballance
  2025-05-09  5:42   ` Arnd Bergmann
  2025-05-09 11:35   ` Andy Shevchenko
  2025-05-09  3:15 ` [PATCH 07/11] rust: io: add from_raw_cookie functions Andrew Ballance
                   ` (6 subsequent siblings)
  12 siblings, 2 replies; 23+ messages in thread
From: Andrew Ballance @ 2025-05-09  3:15 UTC (permalink / raw)
  To: dakr, airlied, simona, akpm, ojeda, alex.gaynor, boqun.feng, gary,
	bjorn3_gh, benno.lossin, a.hindborg, aliceryhl, tmgross, gregkh,
	rafael, bhelgaas, kwilczynski, raag.jadav, andriy.shevchenko,
	arnd, me, andrewjballance, fujita.tomonori, daniel.almeida
  Cc: nouveau, dri-devel, linux-kernel, rust-for-linux, linux-pci

From: Fiona Behrens <me@kloenk.dev>

Move the non arch specific PIO size to linux/io.h.

This allows rust to access `PIO_OFFSET`, `PIO_MASK` and
`PIO_RESERVED`. This is required to implement `IO_COND` in rust.

Signed-off-by: Fiona Behrens <me@kloenk.dev>
Signed-off-by: Andrew Ballance <andrewjballance@gmail.com>
---
 include/linux/io.h | 13 +++++++++++++
 lib/iomap.c        | 13 -------------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/linux/io.h b/include/linux/io.h
index 6a6bc4d46d0a..df032061544a 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -12,6 +12,19 @@
 #include <asm/io.h>
 #include <asm/page.h>
 
+#ifndef HAVE_ARCH_PIO_SIZE
+/*
+ * We encode the physical PIO addresses (0-0xffff) into the
+ * pointer by offsetting them with a constant (0x10000) and
+ * assuming that all the low addresses are always PIO. That means
+ * we can do some sanity checks on the low bits, and don't
+ * need to just take things for granted.
+ */
+#define PIO_OFFSET	0x10000UL
+#define PIO_MASK	0x0ffffUL
+#define PIO_RESERVED	0x40000UL
+#endif
+
 struct device;
 
 #ifndef __iowrite32_copy
diff --git a/lib/iomap.c b/lib/iomap.c
index a65717cd86f7..e13cfe77c32f 100644
--- a/lib/iomap.c
+++ b/lib/iomap.c
@@ -24,19 +24,6 @@
  * implementation and should do their own copy.
  */
 
-#ifndef HAVE_ARCH_PIO_SIZE
-/*
- * We encode the physical PIO addresses (0-0xffff) into the
- * pointer by offsetting them with a constant (0x10000) and
- * assuming that all the low addresses are always PIO. That means
- * we can do some sanity checks on the low bits, and don't
- * need to just take things for granted.
- */
-#define PIO_OFFSET	0x10000UL
-#define PIO_MASK	0x0ffffUL
-#define PIO_RESERVED	0x40000UL
-#endif
-
 static void bad_io_access(unsigned long port, const char *access)
 {
 	static int count = 10;
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 07/11] rust: io: add from_raw_cookie functions
  2025-05-09  3:15 [PATCH 00/11] rust: add support for Port io Andrew Ballance
                   ` (5 preceding siblings ...)
  2025-05-09  3:15 ` [PATCH 06/11] io: move PIO_OFFSET to linux/io.h Andrew Ballance
@ 2025-05-09  3:15 ` Andrew Ballance
  2025-05-09  5:45   ` Arnd Bergmann
  2025-05-09  3:15 ` [PATCH 08/11] rust: pci: make Bar generic over Io Andrew Ballance
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Andrew Ballance @ 2025-05-09  3:15 UTC (permalink / raw)
  To: dakr, airlied, simona, akpm, ojeda, alex.gaynor, boqun.feng, gary,
	bjorn3_gh, benno.lossin, a.hindborg, aliceryhl, tmgross, gregkh,
	rafael, bhelgaas, kwilczynski, raag.jadav, andriy.shevchenko,
	arnd, me, andrewjballance, fujita.tomonori, daniel.almeida
  Cc: nouveau, dri-devel, linux-kernel, rust-for-linux, linux-pci

adds a `from_raw_cookie` function to the IoAccess trait.

`from_raw_cookie` attempts to convert a iomem address that can be
accessed by the ioread/iowrite family of C functions into either
a `Io`, `PortIo` or `MMIo`.

This is done so that devices that know what type of Io they are at
compile time can give a hint about their type.

Suggested-by: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Andrew Ballance <andrewjballance@gmail.com>
---
 rust/kernel/io.rs | 104 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)

diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 3d8b6e731ce7..30892f2909a6 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -6,6 +6,62 @@
 
 use crate::error::{code::EINVAL, Result};
 use crate::{bindings, build_assert};
+use io_backend::*;
+
+/// `io_backend` is private and implements the config specific logic for
+/// `IoAccess::from_raw_cookie`.
+#[cfg(CONFIG_GENERIC_IOMAP)]
+mod io_backend {
+    // if generic_iomap is enabled copy the logic from IO_COND in `lib/iomap.c`
+
+    #[inline]
+    pub(super) fn is_mmio(addr: usize) -> bool {
+        addr >= bindings::PIO_RESERVED as usize
+    }
+
+    #[inline]
+    pub(super) fn is_portio(addr: usize) -> bool {
+        !is_mmio(addr) && addr > bindings::PIO_OFFSET as usize
+    }
+
+    #[inline]
+    pub(super) fn cookie_to_pio(addr: usize) -> usize {
+        addr & bindings::PIO_MASK as usize
+    }
+
+    #[inline]
+    pub(super) fn cookie_to_mmio(cookie: usize) -> usize {
+        cookie
+    }
+}
+#[cfg(not(CONFIG_GENERIC_IOMAP))]
+mod io_backend {
+    // for everyone who does not use generic iomap
+    // except for alpha and parisc, neither of which has a rust compiler,
+    // ioread/iowrite is defined in `include/asm-generic/io.h`.
+    //
+    // for these ioread/iowrite, maps to read/write.
+    // so allow any io to be converted  because they use the same backend
+    #[inline]
+    pub(super) fn is_mmio(_addr: usize) -> bool {
+        true
+    }
+
+    #[inline]
+    pub(super) fn is_portio(_addr: usize) -> bool {
+        false
+    }
+
+    #[inline]
+    pub(super) fn cookie_to_pio(cookie: usize) -> usize {
+        cookie
+    }
+
+    #[inline]
+    pub(super) fn cookie_to_mmio(cookie: usize) -> usize {
+        cookie
+    }
+}
 
 /// Private macro to define the [`IoAccess`] functions.
 macro_rules! define_io_access_function {
@@ -160,8 +216,18 @@ pub unsafe trait IoAccess<const SIZE: usize = 0> {
     fn maxsize(&self) -> usize;
 
     /// Returns the base address of the accessed IO area.
+    /// if `self` was created by ['from_raw_cookie'], `addr` might not be equal to the original
+    /// address.
     fn addr(&self) -> usize;
 
+    /// Attempts to create a `Self` from a [`IoRaw`].
+    ///
+    /// # Safety
+    /// `raw` should be a io cookie that can be accessed by the C `ioread`/`iowrite` functions
+    unsafe fn from_raw_cookie(raw: IoRaw<SIZE>) -> Result<Self>
+    where
+        Self: Sized;
+
     define_io_access_function!(@read
         read8_unchecked, read8, try_read8, u8;
         read16_unchecked, read16, try_read16, u16;
@@ -367,6 +433,19 @@ fn addr(&self) -> usize {
         self.0.addr()
     }
 
+    unsafe fn from_raw_cookie(mut raw: IoRaw<SIZE>) -> Result<Self>
+    where
+        Self: Sized,
+    {
+        if is_mmio(raw.addr()) {
+            // INVARIANT: `addr` is decoded so it should be ok to access with read/write
+            raw.addr = cookie_to_mmio(raw.addr());
+            Ok(Self(raw))
+        } else {
+            Err(EINVAL)
+        }
+    }
+
     impl_accessor_fn!(
         read8_unchecked, readb, write8_unchecked, writeb, u8;
         read16_unchecked, readw, write16_unchecked, writew, u16;
@@ -476,6 +555,19 @@ fn addr(&self) -> usize {
         self.0.addr()
     }
 
+    unsafe fn from_raw_cookie(mut raw: IoRaw<SIZE>) -> Result<Self>
+    where
+        Self: Sized,
+    {
+        if is_portio(raw.addr()) {
+            // INVARIANT: `addr` is decoded so it should be ok to access with in/out
+            raw.addr = cookie_to_pio(raw.addr());
+            Ok(Self(raw))
+        } else {
+            Err(EINVAL)
+        }
+    }
+
     #[rustfmt::skip]
     impl_accessor_fn!(
         read8_unchecked, inb, write8_unchecked, outb, u8;
@@ -563,6 +655,18 @@ fn maxsize(&self) -> usize {
         self.0.maxsize()
     }
 
+    unsafe fn from_raw_cookie(raw: IoRaw<SIZE>) -> Result<Self>
+    where
+        Self: Sized,
+    {
+        if is_mmio(raw.addr()) || is_portio(raw.addr()) {
+            // INVARIANT: `addr` is not touched so it should be able to be read with ioread/iowrite
+            Ok(Self(raw))
+        } else {
+            Err(EINVAL)
+        }
+    }
+
     impl_accessor_fn!(
         read8_unchecked, ioread8, write8_unchecked, iowrite8, u8;
         read16_unchecked, ioread16, write16_unchecked, iowrite16, u16;
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 08/11] rust: pci: make Bar generic over Io
  2025-05-09  3:15 [PATCH 00/11] rust: add support for Port io Andrew Ballance
                   ` (6 preceding siblings ...)
  2025-05-09  3:15 ` [PATCH 07/11] rust: io: add from_raw_cookie functions Andrew Ballance
@ 2025-05-09  3:15 ` Andrew Ballance
  2025-05-09  3:15 ` [PATCH 09/11] samples: rust: rust_driver_pci: update to use new bar and io api Andrew Ballance
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Andrew Ballance @ 2025-05-09  3:15 UTC (permalink / raw)
  To: dakr, airlied, simona, akpm, ojeda, alex.gaynor, boqun.feng, gary,
	bjorn3_gh, benno.lossin, a.hindborg, aliceryhl, tmgross, gregkh,
	rafael, bhelgaas, kwilczynski, raag.jadav, andriy.shevchenko,
	arnd, me, andrewjballance, fujita.tomonori, daniel.almeida
  Cc: nouveau, dri-devel, linux-kernel, rust-for-linux, linux-pci

renames `Bar` to `RawBar` and makes it generic over `IoAccess`.
a user can give a compile time suggestion when mapping a bar so
that the type of io can be known.

Suggested-by: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Andrew Ballance <andrewjballance@gmail.com>
---
 rust/kernel/pci.rs | 88 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 69 insertions(+), 19 deletions(-)

diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index c97d6d470b28..7e592db99073 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -11,8 +11,7 @@
     devres::Devres,
     driver,
     error::{to_result, Result},
-    io::Io,
-    io::IoRaw,
+    io::{Io, IoAccess, IoRaw, MMIo, PortIo},
     str::CStr,
     types::{ARef, ForeignOwnable, Opaque},
     ThisModule,
@@ -259,15 +258,25 @@ pub struct Device<Ctx: device::DeviceContext = device::Normal>(
 ///
 /// # Invariants
 ///
-/// `Bar` always holds an `IoRaw` inststance that holds a valid pointer to the start of the I/O
+/// `Bar` always holds an `I` inststance that holds a valid pointer to the start of the I/O
 /// memory mapped PCI bar and its size.
-pub struct Bar<const SIZE: usize = 0> {
+pub struct RawBar<const SIZE: usize = 0, I: IoAccess<SIZE> = Io<SIZE>> {
     pdev: ARef<Device>,
-    io: IoRaw<SIZE>,
+    original_ioptr: usize,
+    io: I,
     num: i32,
 }
 
-impl<const SIZE: usize> Bar<SIZE> {
+/// a pci bar that can be either PortIo or MMIo
+pub type IoBar<const SIZE: usize = 0> = RawBar<SIZE, Io<SIZE>>;
+
+/// a pci bar that maps a [`PortIo`].
+pub type MMIoBar<const SIZE: usize = 0> = RawBar<SIZE, MMIo<SIZE>>;
+
+/// a pci bar that maps a [`MMIo`].
+pub type PIoBar<const SIZE: usize = 0> = RawBar<SIZE, PortIo<SIZE>>;
+
+impl<const SIZE: usize, I: IoAccess<SIZE>> RawBar<SIZE, I> {
     fn new(pdev: &Device, num: u32, name: &CStr) -> Result<Self> {
         let len = pdev.resource_len(num)?;
         if len == 0 {
@@ -299,7 +308,22 @@ fn new(pdev: &Device, num: u32, name: &CStr) -> Result<Self> {
             return Err(ENOMEM);
         }
 
-        let io = match IoRaw::new(ioptr, len as usize) {
+        let raw = match IoRaw::new(ioptr, len as usize) {
+            Ok(io) => io,
+            Err(err) => {
+                // SAFETY:
+                // `pdev` is valid by the invariants of `Device`.
+                // `ioptr` is guaranteed to be the start of a valid I/O mapped memory region.
+                // `num` is checked for validity by a previous call to `Device::resource_len`.
+                unsafe { Self::do_release(pdev, ioptr, num) };
+                return Err(err);
+            }
+        };
+
+        // SAFETY:
+        // - `raw` is from `pci_iomap`
+        // - addresses from `pci_iomap` should be accesed through ioread/iowrite
+        let io = match unsafe { I::from_raw_cookie(raw) } {
             Ok(io) => io,
             Err(err) => {
                 // SAFETY:
@@ -311,8 +335,9 @@ fn new(pdev: &Device, num: u32, name: &CStr) -> Result<Self> {
             }
         };
 
-        Ok(Bar {
+        Ok(RawBar {
             pdev: pdev.into(),
+            original_ioptr: ioptr,
             io,
             num,
         })
@@ -334,29 +359,28 @@ unsafe fn do_release(pdev: &Device, ioptr: usize, num: i32) {
 
     fn release(&self) {
         // SAFETY: The safety requirements are guaranteed by the type invariant of `self.pdev`.
-        unsafe { Self::do_release(&self.pdev, self.io.addr(), self.num) };
+        unsafe { Self::do_release(&self.pdev, self.original_ioptr, self.num) };
     }
 }
 
-impl Bar {
+impl RawBar {
     fn index_is_valid(index: u32) -> bool {
         // A `struct pci_dev` owns an array of resources with at most `PCI_NUM_RESOURCES` entries.
         index < bindings::PCI_NUM_RESOURCES
     }
 }
 
-impl<const SIZE: usize> Drop for Bar<SIZE> {
+impl<const SIZE: usize, I: IoAccess<SIZE>> Drop for RawBar<SIZE, I> {
     fn drop(&mut self) {
         self.release();
     }
 }
 
-impl<const SIZE: usize> Deref for Bar<SIZE> {
-    type Target = Io<SIZE>;
+impl<const SIZE: usize, I: IoAccess<SIZE>> Deref for RawBar<SIZE, I> {
+    type Target = I;
 
     fn deref(&self) -> &Self::Target {
-        // SAFETY: By the type invariant of `Self`, the MMIO range in `self.io` is properly mapped.
-        unsafe { Io::from_raw(&self.io) }
+        &self.io
     }
 }
 
@@ -379,7 +403,7 @@ pub fn device_id(&self) -> u16 {
 
     /// Returns the size of the given PCI bar resource.
     pub fn resource_len(&self, bar: u32) -> Result<bindings::resource_size_t> {
-        if !Bar::index_is_valid(bar) {
+        if !RawBar::index_is_valid(bar) {
             return Err(EINVAL);
         }
 
@@ -395,17 +419,43 @@ pub fn iomap_region_sized<const SIZE: usize>(
         &self,
         bar: u32,
         name: &CStr,
-    ) -> Result<Devres<Bar<SIZE>>> {
-        let bar = Bar::<SIZE>::new(self, bar, name)?;
+    ) -> Result<Devres<IoBar<SIZE>>> {
+        let bar = RawBar::<SIZE, _>::new(self, bar, name)?;
         let devres = Devres::new(self.as_ref(), bar, GFP_KERNEL)?;
 
         Ok(devres)
     }
 
     /// Mapps an entire PCI-BAR after performing a region-request on it.
-    pub fn iomap_region(&self, bar: u32, name: &CStr) -> Result<Devres<Bar>> {
+    pub fn iomap_region(&self, bar: u32, name: &CStr) -> Result<Devres<IoBar>> {
         self.iomap_region_sized::<0>(bar, name)
     }
+
+    /// Maps an entire PCI-BAR after performing a region-request` where the
+    /// type of Io backend is known at compile time.
+    pub fn iomap_region_hint<I: IoAccess>(
+        &self,
+        bar: u32,
+        name: &CStr,
+    ) -> Result<Devres<RawBar<0, I>>> {
+        let bar = RawBar::<0, I>::new(self, bar, name)?;
+        let devres = Devres::new(self.as_ref(), bar, GFP_KERNEL)?;
+
+        Ok(devres)
+    }
+    /// Maps an entire PCI-BAR after performing a region-request` where the
+    /// type of Io backend is known at compile time. I/O operation bound checks
+    /// can be performed on compile time for offsets (plus the requested type size) < SIZE.
+    pub fn iomap_region_sized_hint<const SIZE: usize, I: IoAccess<SIZE>>(
+        &self,
+        bar: u32,
+        name: &CStr,
+    ) -> Result<Devres<RawBar<SIZE, I>>> {
+        let bar = RawBar::<SIZE, I>::new(self, bar, name)?;
+        let devres = Devres::new(self.as_ref(), bar, GFP_KERNEL)?;
+
+        Ok(devres)
+    }
 }
 
 impl Device<device::Core> {
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 09/11] samples: rust: rust_driver_pci: update to use new bar and io api
  2025-05-09  3:15 [PATCH 00/11] rust: add support for Port io Andrew Ballance
                   ` (7 preceding siblings ...)
  2025-05-09  3:15 ` [PATCH 08/11] rust: pci: make Bar generic over Io Andrew Ballance
@ 2025-05-09  3:15 ` Andrew Ballance
  2025-05-09  3:15 ` [PATCH 10/11] gpu: nova-core: update to use the " Andrew Ballance
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Andrew Ballance @ 2025-05-09  3:15 UTC (permalink / raw)
  To: dakr, airlied, simona, akpm, ojeda, alex.gaynor, boqun.feng, gary,
	bjorn3_gh, benno.lossin, a.hindborg, aliceryhl, tmgross, gregkh,
	rafael, bhelgaas, kwilczynski, raag.jadav, andriy.shevchenko,
	arnd, me, andrewjballance, fujita.tomonori, daniel.almeida
  Cc: nouveau, dri-devel, linux-kernel, rust-for-linux, linux-pci

update rust_driver_pci.rs to use the new bar and io api.

Signed-off-by: Andrew Ballance <andrewjballance@gmail.com>
---
 samples/rust/rust_driver_pci.rs | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 2bb260aebc9e..b645155142db 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -4,7 +4,9 @@
 //!
 //! To make this driver probe, QEMU must be run with `-device pci-testdev`.
 
-use kernel::{bindings, c_str, device::Core, devres::Devres, pci, prelude::*, types::ARef};
+use kernel::{
+    bindings, c_str, device::Core, devres::Devres, io::IoAccess, pci, prelude::*, types::ARef,
+};
 
 struct Regs;
 
@@ -16,7 +18,7 @@ impl Regs {
     const END: usize = 0x10;
 }
 
-type Bar0 = pci::Bar<{ Regs::END }>;
+type Bar0 = pci::IoBar<{ Regs::END }>;
 
 #[derive(Debug)]
 struct TestIndex(u8);
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 10/11] gpu: nova-core: update to use the new bar and io api
  2025-05-09  3:15 [PATCH 00/11] rust: add support for Port io Andrew Ballance
                   ` (8 preceding siblings ...)
  2025-05-09  3:15 ` [PATCH 09/11] samples: rust: rust_driver_pci: update to use new bar and io api Andrew Ballance
@ 2025-05-09  3:15 ` Andrew Ballance
  2025-05-09  3:15 ` [PATCH 11/11] rust: devres: fix doctest Andrew Ballance
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Andrew Ballance @ 2025-05-09  3:15 UTC (permalink / raw)
  To: dakr, airlied, simona, akpm, ojeda, alex.gaynor, boqun.feng, gary,
	bjorn3_gh, benno.lossin, a.hindborg, aliceryhl, tmgross, gregkh,
	rafael, bhelgaas, kwilczynski, raag.jadav, andriy.shevchenko,
	arnd, me, andrewjballance, fujita.tomonori, daniel.almeida
  Cc: nouveau, dri-devel, linux-kernel, rust-for-linux, linux-pci

updates nova-core to use the new Io and Bar api.

Signed-off-by: Andrew Ballance <andrewjballance@gmail.com>
---
 drivers/gpu/nova-core/driver.rs | 4 ++--
 drivers/gpu/nova-core/regs.rs   | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index a08fb6599267..42596ee2e07f 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -11,7 +11,7 @@ pub(crate) struct NovaCore {
 }
 
 const BAR0_SIZE: usize = 8;
-pub(crate) type Bar0 = pci::Bar<BAR0_SIZE>;
+pub(crate) type Bar0 = pci::MMIoBar<BAR0_SIZE>;
 
 kernel::pci_device_table!(
     PCI_TABLE,
@@ -33,7 +33,7 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> Result<Pin<KBox<Self
         pdev.enable_device_mem()?;
         pdev.set_master();
 
-        let bar = pdev.iomap_region_sized::<BAR0_SIZE>(0, c_str!("nova-core/bar0"))?;
+        let bar = pdev.iomap_region_sized_hint::<BAR0_SIZE, _>(0, c_str!("nova-core/bar0"))?;
 
         let this = KBox::pin_init(
             try_pin_init!(Self {
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index b1a25b86ef17..079c3d275a47 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 
 use crate::driver::Bar0;
+use kernel::io::IoAccess;
 
 // TODO
 //
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 11/11] rust: devres: fix doctest
  2025-05-09  3:15 [PATCH 00/11] rust: add support for Port io Andrew Ballance
                   ` (9 preceding siblings ...)
  2025-05-09  3:15 ` [PATCH 10/11] gpu: nova-core: update to use the " Andrew Ballance
@ 2025-05-09  3:15 ` Andrew Ballance
  2025-05-09  5:53 ` [PATCH 00/11] rust: add support for Port io Arnd Bergmann
  2025-05-13  8:32 ` Danilo Krummrich
  12 siblings, 0 replies; 23+ messages in thread
From: Andrew Ballance @ 2025-05-09  3:15 UTC (permalink / raw)
  To: dakr, airlied, simona, akpm, ojeda, alex.gaynor, boqun.feng, gary,
	bjorn3_gh, benno.lossin, a.hindborg, aliceryhl, tmgross, gregkh,
	rafael, bhelgaas, kwilczynski, raag.jadav, andriy.shevchenko,
	arnd, me, andrewjballance, fujita.tomonori, daniel.almeida
  Cc: nouveau, dri-devel, linux-kernel, rust-for-linux, linux-pci

fix a doc test to use the new Io api.

Signed-off-by: Andrew Ballance <andrewjballance@gmail.com>
---
 rust/kernel/devres.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index ddb1ce4a78d9..88d145821ca8 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -45,7 +45,7 @@ struct DevresInner<T> {
 /// # Example
 ///
 /// ```no_run
-/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::{Io, IoRaw}};
+/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::{Io, IoRaw, IoAccess}};
 /// # use core::ops::Deref;
 ///
 /// // See also [`pci::Bar`] for a real example.
@@ -80,7 +80,7 @@ struct DevresInner<T> {
 ///
 ///    fn deref(&self) -> &Self::Target {
 ///         // SAFETY: The memory range stored in `self` has been properly mapped in `Self::new`.
-///         unsafe { Io::from_raw(&self.0) }
+///         unsafe { Io::from_raw_ref(&self.0) }
 ///    }
 /// }
 /// # fn no_run() -> Result<(), Error> {
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 01/11] rust: helpers: io: use macro to generate io accessor functions
  2025-05-09  3:15 ` [PATCH 01/11] rust: helpers: io: use macro to generate io accessor functions Andrew Ballance
@ 2025-05-09  5:32   ` Arnd Bergmann
  2025-05-13  1:59   ` kernel test robot
  1 sibling, 0 replies; 23+ messages in thread
From: Arnd Bergmann @ 2025-05-09  5:32 UTC (permalink / raw)
  To: Andrew Ballance, Danilo Krummrich, Dave Airlie, Simona Vetter,
	Andrew Morton, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Greg Kroah-Hartman, Rafael J . Wysocki, bhelgaas,
	Krzysztof Wilczyński, Raag Jadav, Andy Shevchenko, me,
	FUJITA Tomonori, daniel.almeida
  Cc: nouveau@lists.freedesktop.org, dri-devel, linux-kernel,
	rust-for-linux, linux-pci

On Fri, May 9, 2025, at 05:15, Andrew Ballance wrote:
> +
> +define_rust_mmio_read_helper(readb, u8);
> +define_rust_mmio_read_helper(readw, u16);
> +define_rust_mmio_read_helper(readl, u32);

This makes it harder to grep for the definitions when trying
to follow the code flow. Can you find a way to have keep the actual
function body in source form, using the name of the generated
symbol?

     Arnd

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 06/11] io: move PIO_OFFSET to linux/io.h
  2025-05-09  3:15 ` [PATCH 06/11] io: move PIO_OFFSET to linux/io.h Andrew Ballance
@ 2025-05-09  5:42   ` Arnd Bergmann
  2025-05-09 11:35   ` Andy Shevchenko
  1 sibling, 0 replies; 23+ messages in thread
From: Arnd Bergmann @ 2025-05-09  5:42 UTC (permalink / raw)
  To: Andrew Ballance, Danilo Krummrich, Dave Airlie, Simona Vetter,
	Andrew Morton, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Greg Kroah-Hartman, Rafael J . Wysocki, bhelgaas,
	Krzysztof Wilczyński, Raag Jadav, Andy Shevchenko, me,
	FUJITA Tomonori, daniel.almeida
  Cc: nouveau@lists.freedesktop.org, dri-devel, linux-kernel,
	rust-for-linux, linux-pci

On Fri, May 9, 2025, at 05:15, Andrew Ballance wrote:
> From: Fiona Behrens <me@kloenk.dev>
>
> Move the non arch specific PIO size to linux/io.h.
>
> This allows rust to access `PIO_OFFSET`, `PIO_MASK` and
> `PIO_RESERVED`. This is required to implement `IO_COND` in rust.
>
> Signed-off-by: Fiona Behrens <me@kloenk.dev>
> Signed-off-by: Andrew Ballance <andrewjballance@gmail.com>

This puts an implementation detail of the x86 specific
iomap() code (unfortunately named "GENERIC_IOMAP" for historic
reasons) into common code. Please don't do that.

We still have a couple of users of GENERIC_IOMAP outside of
x86, but they all work in subtly different ways, and I've
been thinking about better ways to handle those. Ideally
the nonstandard iomap variants (x86, uml, powerpc/powernv,
m68k/q40) should just implement their own ioread/iowrite
helpers out-of-line like we do on alpha and parisc, while
everyone else just aliases them to readl/writel.

      Arnd

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 07/11] rust: io: add from_raw_cookie functions
  2025-05-09  3:15 ` [PATCH 07/11] rust: io: add from_raw_cookie functions Andrew Ballance
@ 2025-05-09  5:45   ` Arnd Bergmann
  0 siblings, 0 replies; 23+ messages in thread
From: Arnd Bergmann @ 2025-05-09  5:45 UTC (permalink / raw)
  To: Andrew Ballance, Danilo Krummrich, Dave Airlie, Simona Vetter,
	Andrew Morton, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Greg Kroah-Hartman, Rafael J . Wysocki, bhelgaas,
	Krzysztof Wilczyński, Raag Jadav, Andy Shevchenko, me,
	FUJITA Tomonori, daniel.almeida
  Cc: nouveau@lists.freedesktop.org, dri-devel, linux-kernel,
	rust-for-linux, linux-pci

On Fri, May 9, 2025, at 05:15, Andrew Ballance wrote:

> +}
> +#[cfg(not(CONFIG_GENERIC_IOMAP))]
> +mod io_backend {
> +    // for everyone who does not use generic iomap
> +    // except for alpha and parisc, neither of which has a rust compiler,
> +    // ioread/iowrite is defined in `include/asm-generic/io.h`.

I think you should special-case x86 here then. The GENERIC_IOMAP
variant is likely to need another hack for powernv to actually work.

Hopefully we won't have to support m68k/q40 here.

     Arnd

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 00/11] rust: add support for Port io
  2025-05-09  3:15 [PATCH 00/11] rust: add support for Port io Andrew Ballance
                   ` (10 preceding siblings ...)
  2025-05-09  3:15 ` [PATCH 11/11] rust: devres: fix doctest Andrew Ballance
@ 2025-05-09  5:53 ` Arnd Bergmann
  2025-05-13 15:15   ` Andrew Ballance
  2025-05-13  8:32 ` Danilo Krummrich
  12 siblings, 1 reply; 23+ messages in thread
From: Arnd Bergmann @ 2025-05-09  5:53 UTC (permalink / raw)
  To: Andrew Ballance, Danilo Krummrich, Dave Airlie, Simona Vetter,
	Andrew Morton, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Greg Kroah-Hartman, Rafael J . Wysocki, bhelgaas,
	Krzysztof Wilczyński, Raag Jadav, Andy Shevchenko, me,
	FUJITA Tomonori, daniel.almeida
  Cc: nouveau@lists.freedesktop.org, dri-devel, linux-kernel,
	rust-for-linux, linux-pci

On Fri, May 9, 2025, at 05:15, Andrew Ballance wrote:
> currently the rust `Io` type maps to the c read{b, w, l, q}/write{b, w, l, q}
> functions and have no support for port io.this is a problem for pci::Bar
> because the pointer returned by pci_iomap is expected to accessed with
> the ioread/iowrite api [0].
>
> this patch series splits the `Io` type into `Io`, `PortIo` and `MMIo`.and,
> updates pci::Bar, as suggested in the zulip[1], so that it is generic over
> Io and, a user can optionally give a compile time hint about the type of io. 

Can you describe here why you want to support both "Io" and "PortIo"
cases separately? I don't think we need to micro-optimize for
legacy ISA devices any more, so I'd hope the "Io" path would be
sufficient to cover the common outliers (ata, uart, vga, ipmi, ne2000)
that need the iomap indirection and also the legacy devices that only
need port I/O (floppy, x86 platform devices, ...).

Ideally we'd only need one set of I/O accessors at all, but I suspect
there are x86 specific drivers that actually need readl/writel to be
inline for performance when accessing on-chip registers rather than
slow PCIe registers.

      Arnd

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 04/11] rust: io: add PortIo
  2025-05-09  3:15 ` [PATCH 04/11] rust: io: add PortIo Andrew Ballance
@ 2025-05-09  6:05   ` Arnd Bergmann
  2025-05-13  6:09   ` kernel test robot
  1 sibling, 0 replies; 23+ messages in thread
From: Arnd Bergmann @ 2025-05-09  6:05 UTC (permalink / raw)
  To: Andrew Ballance, Danilo Krummrich, Dave Airlie, Simona Vetter,
	Andrew Morton, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Greg Kroah-Hartman, Rafael J . Wysocki, bhelgaas,
	Krzysztof Wilczyński, Raag Jadav, Andy Shevchenko, me,
	FUJITA Tomonori, daniel.almeida
  Cc: nouveau@lists.freedesktop.org, dri-devel, linux-kernel,
	rust-for-linux, linux-pci

On Fri, May 9, 2025, at 05:15, Andrew Ballance wrote:
> +
> +#define define_rust_pio_read_helper(name, type)     \
> +	type rust_helper_##name(unsigned long port) \
> +	{                                           \
> +		return name(port);                  \
> +	}
> +
> +#define define_rust_pio_write_helper(name, type)                \
> +	void rust_helper_##name(type value, unsigned long port) \
> +	{                                                       \
> +		name(value, port);                              \
> +	}
> +
> +define_rust_pio_read_helper(inb, u8);
> +define_rust_pio_read_helper(inw, u16);
> +define_rust_pio_read_helper(inl, u32);
> +
> +define_rust_pio_write_helper(outb, u8);
> +define_rust_pio_write_helper(outw, u16);
> +define_rust_pio_write_helper(outl, u32);

These have to be guarded with "#ifdef CONFIG_HAS_PIO", since
most modern machines no longer support PIO at all, even behind
PCI.

The option is still enabled by default for a number of
architectures that normally don't have PIO, but it should
eventually become optional on pretty much anything but
x86 and a few 1990s-era workstations that implement x86-style
on-board devices.

     Arnd

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 06/11] io: move PIO_OFFSET to linux/io.h
  2025-05-09  3:15 ` [PATCH 06/11] io: move PIO_OFFSET to linux/io.h Andrew Ballance
  2025-05-09  5:42   ` Arnd Bergmann
@ 2025-05-09 11:35   ` Andy Shevchenko
  1 sibling, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2025-05-09 11:35 UTC (permalink / raw)
  To: Andrew Ballance
  Cc: dakr, airlied, simona, akpm, ojeda, alex.gaynor, boqun.feng, gary,
	bjorn3_gh, benno.lossin, a.hindborg, aliceryhl, tmgross, gregkh,
	rafael, bhelgaas, kwilczynski, raag.jadav, arnd, me,
	fujita.tomonori, daniel.almeida, nouveau, dri-devel, linux-kernel,
	rust-for-linux, linux-pci

On Thu, May 08, 2025 at 10:15:19PM -0500, Andrew Ballance wrote:
> From: Fiona Behrens <me@kloenk.dev>
> 
> Move the non arch specific PIO size to linux/io.h.
> 
> This allows rust to access `PIO_OFFSET`, `PIO_MASK` and
> `PIO_RESERVED`. This is required to implement `IO_COND` in rust.

...

> +/*
> + * We encode the physical PIO addresses (0-0xffff) into the

I know this is the original text, but we have a chance to improve it a bit by
saying range more explicitly:

 * We encode the physical PIO addresses (0x0000-0xffff) into the

> + * pointer by offsetting them with a constant (0x10000) and
> + * assuming that all the low addresses are always PIO. That means
> + * we can do some sanity checks on the low bits, and don't
> + * need to just take things for granted.
> + */
> +#define PIO_OFFSET	0x10000UL
> +#define PIO_MASK	0x0ffffUL
> +#define PIO_RESERVED	0x40000UL
> +#endif


-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 02/11] rust: io: Replace Io with MMIo using IoAccess trait
  2025-05-09  3:15 ` [PATCH 02/11] rust: io: Replace Io with MMIo using IoAccess trait Andrew Ballance
@ 2025-05-12 20:07   ` Bjorn Helgaas
  0 siblings, 0 replies; 23+ messages in thread
From: Bjorn Helgaas @ 2025-05-12 20:07 UTC (permalink / raw)
  To: Andrew Ballance
  Cc: dakr, airlied, simona, akpm, ojeda, alex.gaynor, boqun.feng, gary,
	bjorn3_gh, benno.lossin, a.hindborg, aliceryhl, tmgross, gregkh,
	rafael, bhelgaas, kwilczynski, raag.jadav, andriy.shevchenko,
	arnd, me, fujita.tomonori, daniel.almeida, nouveau, dri-devel,
	linux-kernel, rust-for-linux, linux-pci

On Thu, May 08, 2025 at 10:15:15PM -0500, Andrew Ballance wrote:
> From: Fiona Behrens <me@kloenk.dev>
> 
> Replace the Io struct with a new MMIo struct that uses the different
> traits (`IoAccess`, `IoAccess64`, `IoAccessRelaxed` and
> `IoAccess64Relaxed).
> This allows to later implement PortIo and a generic Io framework.

Add blank line between paragraphs.

> +    /// Read data from a give offset known at compile time.

s/give/given/

> +    /// Bound checks are perfomed on compile time, hence if the offset is not known at compile
> +    /// time, the build will fail.

s/perfomed on/performed at/

> +    /// Bound checks are performed on runtime, it fails if the offset (plus type size) is
> +    /// out of bounds.

s/on runtime/at runtime/

> +/// This Takes either `@read` or `@write` to generate a single read or write accessor function.

s/This Takes/This takes/

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 01/11] rust: helpers: io: use macro to generate io accessor functions
  2025-05-09  3:15 ` [PATCH 01/11] rust: helpers: io: use macro to generate io accessor functions Andrew Ballance
  2025-05-09  5:32   ` Arnd Bergmann
@ 2025-05-13  1:59   ` kernel test robot
  1 sibling, 0 replies; 23+ messages in thread
From: kernel test robot @ 2025-05-13  1:59 UTC (permalink / raw)
  To: Andrew Ballance, dakr, airlied, simona, akpm, ojeda, alex.gaynor,
	boqun.feng, gary, bjorn3_gh, benno.lossin, a.hindborg, aliceryhl,
	tmgross, gregkh, rafael, bhelgaas, kwilczynski, raag.jadav,
	andriy.shevchenko, arnd, me, fujita.tomonori, daniel.almeida
  Cc: llvm, oe-kbuild-all, nouveau, dri-devel, linux-kernel,
	rust-for-linux, linux-pci

Hi Andrew,

kernel test robot noticed the following build errors:

[auto build test ERROR on 92a09c47464d040866cf2b4cd052bc60555185fb]

url:    https://github.com/intel-lab-lkp/linux/commits/Andrew-Ballance/rust-helpers-io-use-macro-to-generate-io-accessor-functions/20250509-111818
base:   92a09c47464d040866cf2b4cd052bc60555185fb
patch link:    https://lore.kernel.org/r/20250509031524.2604087-2-andrewjballance%40gmail.com
patch subject: [PATCH 01/11] rust: helpers: io: use macro to generate io accessor functions
config: x86_64-randconfig-073-20250512 (https://download.01.org/0day-ci/archive/20250513/202505130821.z5LcD77G-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
rustc: rustc 1.78.0 (9b00956e5 2024-04-29)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250513/202505130821.z5LcD77G-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505130821.z5LcD77G-lkp@intel.com/

All errors (new ones prefixed by >>):

   ***
   *** Rust bindings generator 'bindgen' < 0.69.5 together with libclang >= 19.1
   *** may not work due to a bug (https://github.com/rust-lang/rust-bindgen/pull/2824),
   *** unless patched (like Debian's).
   ***   Your bindgen version:  0.65.1
   ***   Your libclang version: 20.1.2
   ***
   ***
   *** Please see Documentation/rust/quick-start.rst for details
   *** on how to set up the Rust support.
   ***
>> error[E0425]: cannot find function `readb` in crate `bindings`
   --> rust/kernel/io.rs:221:36
   |
   221 |     define_read!(read8, try_read8, readb -> u8);
   |                                    ^^^^^ not found in `bindings`
--
>> error[E0425]: cannot find function `readw` in crate `bindings`
   --> rust/kernel/io.rs:222:38
   |
   222 |     define_read!(read16, try_read16, readw -> u16);
   |                                      ^^^^^ not found in `bindings`
--
>> error[E0425]: cannot find function `writel` in crate `bindings`
   --> rust/kernel/io.rs:243:41
   |
   243 |     define_write!(write32, try_write32, writel <- u32);
   |                                         ^^^^^^ not found in `bindings`
--
>> error[E0425]: cannot find function `writeq` in crate `bindings`
   --> rust/kernel/io.rs:248:9
   |
   248 |         writeq <- u64
   |         ^^^^^^ not found in `bindings`
--
>> error[E0425]: cannot find function `writeb_relaxed` in crate `bindings`
   --> rust/kernel/io.rs:251:55
   |
   251 |     define_write!(write8_relaxed, try_write8_relaxed, writeb_relaxed <- u8);
   |                                                       ^^^^^^^^^^^^^^ not found in `bindings`
--
>> error[E0425]: cannot find function `writew_relaxed` in crate `bindings`
   --> rust/kernel/io.rs:252:57
   |
   252 |     define_write!(write16_relaxed, try_write16_relaxed, writew_relaxed <- u16);
   |                                                         ^^^^^^^^^^^^^^ not found in `bindings`
--
>> error[E0425]: cannot find function `writel_relaxed` in crate `bindings`
   --> rust/kernel/io.rs:253:57
   |
   253 |     define_write!(write32_relaxed, try_write32_relaxed, writel_relaxed <- u32);
   |                                                         ^^^^^^^^^^^^^^ not found in `bindings`
--
>> error[E0425]: cannot find function `writeq_relaxed` in crate `bindings`
   --> rust/kernel/io.rs:258:9
   |
   258 |         writeq_relaxed <- u64
   |         ^^^^^^^^^^^^^^ not found in `bindings`
--
>> error[E0425]: cannot find function `readl` in crate `bindings`
   --> rust/kernel/io.rs:223:38
   |
   223 |     define_read!(read32, try_read32, readl -> u32);
   |                                      ^^^^^ not found in `bindings`
--
>> error[E0425]: cannot find function `readq` in crate `bindings`
   --> rust/kernel/io.rs:228:9
   |
   228 |         readq -> u64
   |         ^^^^^ not found in `bindings`
--
>> error[E0425]: cannot find function `readb_relaxed` in crate `bindings`
   --> rust/kernel/io.rs:231:52
   |
   231 |     define_read!(read8_relaxed, try_read8_relaxed, readb_relaxed -> u8);
   |                                                    ^^^^^^^^^^^^^ not found in `bindings`
..

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 04/11] rust: io: add PortIo
  2025-05-09  3:15 ` [PATCH 04/11] rust: io: add PortIo Andrew Ballance
  2025-05-09  6:05   ` Arnd Bergmann
@ 2025-05-13  6:09   ` kernel test robot
  1 sibling, 0 replies; 23+ messages in thread
From: kernel test robot @ 2025-05-13  6:09 UTC (permalink / raw)
  To: Andrew Ballance, dakr, airlied, simona, akpm, ojeda, alex.gaynor,
	boqun.feng, gary, bjorn3_gh, benno.lossin, a.hindborg, aliceryhl,
	tmgross, gregkh, rafael, bhelgaas, kwilczynski, raag.jadav,
	andriy.shevchenko, arnd, me, fujita.tomonori, daniel.almeida
  Cc: llvm, oe-kbuild-all, nouveau, dri-devel, linux-kernel,
	rust-for-linux, linux-pci

Hi Andrew,

kernel test robot noticed the following build errors:

[auto build test ERROR on 92a09c47464d040866cf2b4cd052bc60555185fb]

url:    https://github.com/intel-lab-lkp/linux/commits/Andrew-Ballance/rust-helpers-io-use-macro-to-generate-io-accessor-functions/20250509-111818
base:   92a09c47464d040866cf2b4cd052bc60555185fb
patch link:    https://lore.kernel.org/r/20250509031524.2604087-5-andrewjballance%40gmail.com
patch subject: [PATCH 04/11] rust: io: add PortIo
config: x86_64-randconfig-073-20250512 (https://download.01.org/0day-ci/archive/20250513/202505131314.ozIwFdR4-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
rustc: rustc 1.78.0 (9b00956e5 2024-04-29)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250513/202505131314.ozIwFdR4-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505131314.ozIwFdR4-lkp@intel.com/

All errors (new ones prefixed by >>):

>> error[E0425]: cannot find function `inb` in crate `bindings`
   --> rust/kernel/io.rs:481:26
   |
   481 |         read8_unchecked, inb, write8_unchecked, outb, u8;
   |                          ^^^ not found in `bindings`
--
>> error[E0425]: cannot find function `outb` in crate `bindings`
   --> rust/kernel/io.rs:481:49
   |
   481 |         read8_unchecked, inb, write8_unchecked, outb, u8;
   |                                                 ^^^^ not found in `bindings`
--
>> error[E0425]: cannot find function `inw` in crate `bindings`
   --> rust/kernel/io.rs:482:27
   |
   482 |         read16_unchecked, inw, write16_unchecked, outw, u16;
   |                           ^^^ not found in `bindings`
--
>> error[E0425]: cannot find function `outw` in crate `bindings`
   --> rust/kernel/io.rs:482:51
   |
   482 |         read16_unchecked, inw, write16_unchecked, outw, u16;
   |                                                   ^^^^ not found in `bindings`
--
>> error[E0425]: cannot find function `inl` in crate `bindings`
   --> rust/kernel/io.rs:483:27
   |
   483 |         read32_unchecked, inl, write32_unchecked, outl, u32;
   |                           ^^^ not found in `bindings`
--
>> error[E0425]: cannot find function `outl` in crate `bindings`
   --> rust/kernel/io.rs:483:51
   |
   483 |         read32_unchecked, inl, write32_unchecked, outl, u32;
   |                                                   ^^^^ not found in `bindings`

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 00/11] rust: add support for Port io
  2025-05-09  3:15 [PATCH 00/11] rust: add support for Port io Andrew Ballance
                   ` (11 preceding siblings ...)
  2025-05-09  5:53 ` [PATCH 00/11] rust: add support for Port io Arnd Bergmann
@ 2025-05-13  8:32 ` Danilo Krummrich
  12 siblings, 0 replies; 23+ messages in thread
From: Danilo Krummrich @ 2025-05-13  8:32 UTC (permalink / raw)
  To: Andrew Ballance
  Cc: airlied, simona, akpm, ojeda, alex.gaynor, boqun.feng, gary,
	bjorn3_gh, benno.lossin, a.hindborg, aliceryhl, tmgross, gregkh,
	rafael, bhelgaas, kwilczynski, raag.jadav, andriy.shevchenko,
	arnd, me, fujita.tomonori, daniel.almeida, nouveau, dri-devel,
	linux-kernel, rust-for-linux, linux-pci

Hi Andrew,

On Thu, May 08, 2025 at 10:15:13PM -0500, Andrew Ballance wrote:
> currently the rust `Io` type maps to the c read{b, w, l, q}/write{b, w, l, q}
> functions and have no support for port io.this is a problem for pci::Bar
> because the pointer returned by pci_iomap is expected to accessed with
> the ioread/iowrite api [0].
> 
> this patch series splits the `Io` type into `Io`, `PortIo` and `MMIo`.and,
> updates pci::Bar, as suggested in the zulip[1], so that it is generic over
> Io and, a user can optionally give a compile time hint about the type of io. 
> 
> Link: https://docs.kernel.org/6.11/driver-api/pci/pci.html#c.pci_iomap [0]
> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/.60IoRaw.60.20and.20.60usize.60/near/514788730 [1]
> 
> Andrew Ballance (6):
>   rust: io: add new Io type
>   rust: io: add from_raw_cookie functions
>   rust: pci: make Bar generic over Io
>   samples: rust: rust_driver_pci: update to use new bar and io api
>   gpu: nova-core: update to use the new bar and io api
>   rust: devres: fix doctest
> 
> Fiona Behrens (5):
>   rust: helpers: io: use macro to generate io accessor functions
>   rust: io: Replace Io with MMIo using IoAccess trait
>   rust: io: implement Debug for IoRaw and add some doctests
>   rust: io: add PortIo
>   io: move PIO_OFFSET to linux/io.h

Thanks for sending out the patch series.

I gave it a quick shot and the series breaks the build starting with patch 2. I
see that you have fixup commits later in the series, however in the kernel we
don't allow patches to intermediately introduce build failures, build warnings,
bugs, etc., see also [1]. You should still try to break things down logically as
good as possible.

From the current structure of your patches it seems to me that structure-wise
you should be good by going through them one by one and fix them up; your later
patches should become "noops" then. But feel free to re-organize things if you
feel that's not the best approach.

Can you please fix this up and resend right away? This should make the
subsequent review much easier.

[1] https://docs.kernel.org/process/submitting-patches.html#separate-your-changes

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 00/11] rust: add support for Port io
  2025-05-09  5:53 ` [PATCH 00/11] rust: add support for Port io Arnd Bergmann
@ 2025-05-13 15:15   ` Andrew Ballance
  0 siblings, 0 replies; 23+ messages in thread
From: Andrew Ballance @ 2025-05-13 15:15 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Danilo Krummrich, Dave Airlie, Simona Vetter, Andrew Morton,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Greg Kroah-Hartman, Rafael J . Wysocki, bhelgaas,
	Krzysztof Wilczyński, Raag Jadav, Andy Shevchenko, me,
	FUJITA Tomonori, daniel.almeida, nouveau@lists.freedesktop.org,
	dri-devel, linux-kernel, rust-for-linux, linux-pci

On Fri, May 09, 2025 at 07:53:31AM +0200, Arnd Bergmann wrote:
> Can you describe here why you want to support both "Io" and "PortIo"
> cases separately? I don't think we need to micro-optimize for
> legacy ISA devices any more, so I'd hope the "Io" path would be
> sufficient to cover the common outliers (ata, uart, vga, ipmi, ne2000)
> that need the iomap indirection and also the legacy devices that only
> need port I/O (floppy, x86 platform devices, ...).

Yeah, we probably don`t need the `PortIo` type and can rely on `Io` for
port io. I`ll remove it for the v2.

Best regards
Andrew Ballance

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2025-05-13 15:16 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-09  3:15 [PATCH 00/11] rust: add support for Port io Andrew Ballance
2025-05-09  3:15 ` [PATCH 01/11] rust: helpers: io: use macro to generate io accessor functions Andrew Ballance
2025-05-09  5:32   ` Arnd Bergmann
2025-05-13  1:59   ` kernel test robot
2025-05-09  3:15 ` [PATCH 02/11] rust: io: Replace Io with MMIo using IoAccess trait Andrew Ballance
2025-05-12 20:07   ` Bjorn Helgaas
2025-05-09  3:15 ` [PATCH 03/11] rust: io: implement Debug for IoRaw and add some doctests Andrew Ballance
2025-05-09  3:15 ` [PATCH 04/11] rust: io: add PortIo Andrew Ballance
2025-05-09  6:05   ` Arnd Bergmann
2025-05-13  6:09   ` kernel test robot
2025-05-09  3:15 ` [PATCH 05/11] rust: io: add new Io type Andrew Ballance
2025-05-09  3:15 ` [PATCH 06/11] io: move PIO_OFFSET to linux/io.h Andrew Ballance
2025-05-09  5:42   ` Arnd Bergmann
2025-05-09 11:35   ` Andy Shevchenko
2025-05-09  3:15 ` [PATCH 07/11] rust: io: add from_raw_cookie functions Andrew Ballance
2025-05-09  5:45   ` Arnd Bergmann
2025-05-09  3:15 ` [PATCH 08/11] rust: pci: make Bar generic over Io Andrew Ballance
2025-05-09  3:15 ` [PATCH 09/11] samples: rust: rust_driver_pci: update to use new bar and io api Andrew Ballance
2025-05-09  3:15 ` [PATCH 10/11] gpu: nova-core: update to use the " Andrew Ballance
2025-05-09  3:15 ` [PATCH 11/11] rust: devres: fix doctest Andrew Ballance
2025-05-09  5:53 ` [PATCH 00/11] rust: add support for Port io Arnd Bergmann
2025-05-13 15:15   ` Andrew Ballance
2025-05-13  8:32 ` Danilo Krummrich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).