* [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait
@ 2026-02-06 6:00 Alexandre Courbot
2026-02-06 6:00 ` [PATCH v2 1/6] " Alexandre Courbot
` (9 more replies)
0 siblings, 10 replies; 25+ messages in thread
From: Alexandre Courbot @ 2026-02-06 6:00 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Lyude Paul, Eliot Courtney, Alexandre Courbot
`IoCapable<T>` is currently used as a marker trait to signal that the
methods of the `Io` trait corresponding to `T` have been overridden by
the implementor (the default implementations triggering a build-time
error).
This goes against the DRY principle and separates the signaling of the
capability from its implementation, making it possible to forget a step
while implementing a new `Io`.
Another undesirable side-effect is that it makes the implementation of
I/O backends boilerplate-y and convoluted: currently this is done using
two levels of imbricated macros that generate unsafe code.
This patchset fixes these issues by turning `IoCapable` into a
functional trait including the raw implementation of the I/O
accessors for `T` using unsafe methods that work with an arbitrary
address, and making the default methods of `Io` call into these
implementations after checking the bounds.
This makes overriding these accessors on all I/O backends unneeded,
resulting in a net -90 LoCs while avoiding a violation of the DRY
principle and reducing (and simplifying) the use of macros generating
unsafe code.
Patch 1 adds the `io_read` and `io_write` unsafe methods to `IoCapable`,
provides the required implementations for `Mmio` and `pci::ConfigSpace`,
and make the default I/O accessors of `Io` call into them instead of
failing.
Patches 2 to 4 get rid of the `_relaxed` variants we had in `Mmio`,
since these are not usable in code generic against `Io` and makes use of
the macros we want to remove. They are replaced by a `RelaxedMmio`
wrapper type that implements the required `IoCapable`s and is thus
usable in generic code.
Patches 5 and 6 remove the overloaded implementations of the `Io`
methods for `pci::ConfigSpace` and `Mmio`, respectively, while also
deleting the macros that have become unused.
There is more work coming on top of this patchset (notably the
`register!` macro with proper I/O), but I wanted to send this work first
as it stands on its own IMHO and is more digestible from a review
perspective.
The base for this patchset is `driver-core-testing`.
Cc: Zhi Wang <zhiw@nvidia.com>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Eliot Courtney <ecourtney@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
Changes in v2:
- Turn `RelaxedMmio` into an actual wrapper type and make it available
through a `Mmio::relaxed()` method.
- Link to v1: https://patch.msgid.link/20260202-io-v1-0-9bb2177d23be@nvidia.com
---
Alexandre Courbot (6):
rust: io: turn IoCapable into a functional trait
rust: io: mem: use non-relaxed I/O ops in examples
rust: io: provide Mmio relaxed ops through a wrapper type
rust: io: remove legacy relaxed accessors of Mmio
rust: pci: io: remove overloaded Io methods of ConfigSpace
rust: io: remove overloaded Io methods of Mmio
rust/kernel/io.rs | 435 ++++++++++++++++++++++----------------------------
rust/kernel/io/mem.rs | 10 +-
rust/kernel/pci/io.rs | 99 ++++--------
3 files changed, 227 insertions(+), 317 deletions(-)
---
base-commit: f55ae0bfa00e446ea751d09f468daeafc303e03f
change-id: 20260202-io-81fd368f7565
Best regards,
--
Alexandre Courbot <acourbot@nvidia.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 1/6] rust: io: turn IoCapable into a functional trait
2026-02-06 6:00 [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait Alexandre Courbot
@ 2026-02-06 6:00 ` Alexandre Courbot
2026-02-06 20:29 ` lyude
2026-02-06 6:00 ` [PATCH v2 2/6] rust: io: mem: use non-relaxed I/O ops in examples Alexandre Courbot
` (8 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Alexandre Courbot @ 2026-02-06 6:00 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Lyude Paul, Eliot Courtney, Alexandre Courbot
`IoCapable<T>` is currently used as a marker trait to signal that the
methods of the `Io` trait corresponding to `T` have been overridden by
the implementor (the default implementations triggering a build-time
error).
This goes against the DRY principle and separates the signaling of the
capability from its implementation, making it possible to forget a step
while implementing a new `Io`.
Another undesirable side-effect is that it makes the implementation of
I/O backends boilerplate-y and convoluted: currently this is done using
two levels of imbricated macros that generate unsafe code.
Fix these issues by turning `IoCapable` into a functional trait that
includes the raw implementation of the I/O access for `T` using
unsafe methods that work with an arbitrary address.
This allows us to turn the default methods of `Io` into regular methods
that check the passed offset, turn it into an address, and call into the
corresponding `IoCapable` functions, removing the need to overload them
at all.
`IoCapable` must still be implemented for all supported primitive types,
which is still done more concisely using a macro, but this macro becomes
much simpler and does not require calling into another one.
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Acked-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
rust/kernel/io.rs | 169 ++++++++++++++++++++++++++++++++++++++------------
rust/kernel/pci/io.rs | 37 ++++++++++-
2 files changed, 163 insertions(+), 43 deletions(-)
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index c1cca7b438c3..dc894a45bbcc 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -273,14 +273,29 @@ const fn offset_valid<U>(offset: usize, size: usize) -> bool {
}
}
-/// Marker trait indicating that an I/O backend supports operations of a certain type.
+/// Trait indicating that an I/O backend supports operations of a certain type and providing an
+/// implementation for these operations.
///
/// Different I/O backends can implement this trait to expose only the operations they support.
///
/// For example, a PCI configuration space may implement `IoCapable<u8>`, `IoCapable<u16>`,
/// and `IoCapable<u32>`, but not `IoCapable<u64>`, while an MMIO region on a 64-bit
/// system might implement all four.
-pub trait IoCapable<T> {}
+pub trait IoCapable<T> {
+ /// Performs an I/O read of type `T` at `address` and returns the result.
+ ///
+ /// # Safety
+ ///
+ /// The range `[address..address + size_of::<T>()]` must be within the bounds of `Self`.
+ unsafe fn io_read(&self, address: usize) -> T;
+
+ /// Performs an I/O write of `value` at `address`.
+ ///
+ /// # Safety
+ ///
+ /// The range `[address..address + size_of::<T>()]` must be within the bounds of `Self`.
+ unsafe fn io_write(&self, value: T, address: usize);
+}
/// Types implementing this trait (e.g. MMIO BARs or PCI config regions)
/// can perform I/O operations on regions of memory.
@@ -322,146 +337,198 @@ fn io_addr<U>(&self, offset: usize) -> Result<usize> {
/// Fallible 8-bit read with runtime bounds check.
#[inline(always)]
- fn try_read8(&self, _offset: usize) -> Result<u8>
+ fn try_read8(&self, offset: usize) -> Result<u8>
where
Self: IoCapable<u8>,
{
- build_error!("Backend does not support fallible 8-bit read")
+ let address = self.io_addr::<u8>(offset)?;
+
+ // SAFETY: `address` has been validated by `io_addr`.
+ Ok(unsafe { self.io_read(address) })
}
/// Fallible 16-bit read with runtime bounds check.
#[inline(always)]
- fn try_read16(&self, _offset: usize) -> Result<u16>
+ fn try_read16(&self, offset: usize) -> Result<u16>
where
Self: IoCapable<u16>,
{
- build_error!("Backend does not support fallible 16-bit read")
+ let address = self.io_addr::<u16>(offset)?;
+
+ // SAFETY: `address` has been validated by `io_addr`.
+ Ok(unsafe { self.io_read(address) })
}
/// Fallible 32-bit read with runtime bounds check.
#[inline(always)]
- fn try_read32(&self, _offset: usize) -> Result<u32>
+ fn try_read32(&self, offset: usize) -> Result<u32>
where
Self: IoCapable<u32>,
{
- build_error!("Backend does not support fallible 32-bit read")
+ let address = self.io_addr::<u32>(offset)?;
+
+ // SAFETY: `address` has been validated by `io_addr`.
+ Ok(unsafe { self.io_read(address) })
}
/// Fallible 64-bit read with runtime bounds check.
#[inline(always)]
- fn try_read64(&self, _offset: usize) -> Result<u64>
+ fn try_read64(&self, offset: usize) -> Result<u64>
where
Self: IoCapable<u64>,
{
- build_error!("Backend does not support fallible 64-bit read")
+ let address = self.io_addr::<u64>(offset)?;
+
+ // SAFETY: `address` has been validated by `io_addr`.
+ Ok(unsafe { self.io_read(address) })
}
/// Fallible 8-bit write with runtime bounds check.
#[inline(always)]
- fn try_write8(&self, _value: u8, _offset: usize) -> Result
+ fn try_write8(&self, value: u8, offset: usize) -> Result
where
Self: IoCapable<u8>,
{
- build_error!("Backend does not support fallible 8-bit write")
+ let address = self.io_addr::<u8>(offset)?;
+
+ // SAFETY: `address` has been validated by `io_addr`.
+ unsafe { self.io_write(value, address) };
+ Ok(())
}
/// Fallible 16-bit write with runtime bounds check.
#[inline(always)]
- fn try_write16(&self, _value: u16, _offset: usize) -> Result
+ fn try_write16(&self, value: u16, offset: usize) -> Result
where
Self: IoCapable<u16>,
{
- build_error!("Backend does not support fallible 16-bit write")
+ let address = self.io_addr::<u16>(offset)?;
+
+ // SAFETY: `address` has been validated by `io_addr`.
+ unsafe { self.io_write(value, address) };
+ Ok(())
}
/// Fallible 32-bit write with runtime bounds check.
#[inline(always)]
- fn try_write32(&self, _value: u32, _offset: usize) -> Result
+ fn try_write32(&self, value: u32, offset: usize) -> Result
where
Self: IoCapable<u32>,
{
- build_error!("Backend does not support fallible 32-bit write")
+ let address = self.io_addr::<u32>(offset)?;
+
+ // SAFETY: `address` has been validated by `io_addr`.
+ unsafe { self.io_write(value, address) };
+ Ok(())
}
/// Fallible 64-bit write with runtime bounds check.
#[inline(always)]
- fn try_write64(&self, _value: u64, _offset: usize) -> Result
+ fn try_write64(&self, value: u64, offset: usize) -> Result
where
Self: IoCapable<u64>,
{
- build_error!("Backend does not support fallible 64-bit write")
+ let address = self.io_addr::<u64>(offset)?;
+
+ // SAFETY: `address` has been validated by `io_addr`.
+ unsafe { self.io_write(value, address) };
+ Ok(())
}
/// Infallible 8-bit read with compile-time bounds check.
#[inline(always)]
- fn read8(&self, _offset: usize) -> u8
+ fn read8(&self, offset: usize) -> u8
where
Self: IoKnownSize + IoCapable<u8>,
{
- build_error!("Backend does not support infallible 8-bit read")
+ let address = self.io_addr_assert::<u8>(offset);
+
+ // SAFETY: `address` has been validated by `io_addr_assert`.
+ unsafe { self.io_read(address) }
}
/// Infallible 16-bit read with compile-time bounds check.
#[inline(always)]
- fn read16(&self, _offset: usize) -> u16
+ fn read16(&self, offset: usize) -> u16
where
Self: IoKnownSize + IoCapable<u16>,
{
- build_error!("Backend does not support infallible 16-bit read")
+ let address = self.io_addr_assert::<u16>(offset);
+
+ // SAFETY: `address` has been validated by `io_addr_assert`.
+ unsafe { self.io_read(address) }
}
/// Infallible 32-bit read with compile-time bounds check.
#[inline(always)]
- fn read32(&self, _offset: usize) -> u32
+ fn read32(&self, offset: usize) -> u32
where
Self: IoKnownSize + IoCapable<u32>,
{
- build_error!("Backend does not support infallible 32-bit read")
+ let address = self.io_addr_assert::<u32>(offset);
+
+ // SAFETY: `address` has been validated by `io_addr_assert`.
+ unsafe { self.io_read(address) }
}
/// Infallible 64-bit read with compile-time bounds check.
#[inline(always)]
- fn read64(&self, _offset: usize) -> u64
+ fn read64(&self, offset: usize) -> u64
where
Self: IoKnownSize + IoCapable<u64>,
{
- build_error!("Backend does not support infallible 64-bit read")
+ let address = self.io_addr_assert::<u64>(offset);
+
+ // SAFETY: `address` has been validated by `io_addr_assert`.
+ unsafe { self.io_read(address) }
}
/// Infallible 8-bit write with compile-time bounds check.
#[inline(always)]
- fn write8(&self, _value: u8, _offset: usize)
+ fn write8(&self, value: u8, offset: usize)
where
Self: IoKnownSize + IoCapable<u8>,
{
- build_error!("Backend does not support infallible 8-bit write")
+ let address = self.io_addr_assert::<u8>(offset);
+
+ // SAFETY: `address` has been validated by `io_addr_assert`.
+ unsafe { self.io_write(value, address) }
}
/// Infallible 16-bit write with compile-time bounds check.
#[inline(always)]
- fn write16(&self, _value: u16, _offset: usize)
+ fn write16(&self, value: u16, offset: usize)
where
Self: IoKnownSize + IoCapable<u16>,
{
- build_error!("Backend does not support infallible 16-bit write")
+ let address = self.io_addr_assert::<u16>(offset);
+
+ // SAFETY: `address` has been validated by `io_addr_assert`.
+ unsafe { self.io_write(value, address) }
}
/// Infallible 32-bit write with compile-time bounds check.
#[inline(always)]
- fn write32(&self, _value: u32, _offset: usize)
+ fn write32(&self, value: u32, offset: usize)
where
Self: IoKnownSize + IoCapable<u32>,
{
- build_error!("Backend does not support infallible 32-bit write")
+ let address = self.io_addr_assert::<u32>(offset);
+
+ // SAFETY: `address` has been validated by `io_addr_assert`.
+ unsafe { self.io_write(value, address) }
}
/// Infallible 64-bit write with compile-time bounds check.
#[inline(always)]
- fn write64(&self, _value: u64, _offset: usize)
+ fn write64(&self, value: u64, offset: usize)
where
Self: IoKnownSize + IoCapable<u64>,
{
- build_error!("Backend does not support infallible 64-bit write")
+ let address = self.io_addr_assert::<u64>(offset);
+
+ // SAFETY: `address` has been validated by `io_addr_assert`.
+ unsafe { self.io_write(value, address) }
}
}
@@ -487,14 +554,36 @@ fn io_addr_assert<U>(&self, offset: usize) -> usize {
}
}
-// MMIO regions support 8, 16, and 32-bit accesses.
-impl<const SIZE: usize> IoCapable<u8> for Mmio<SIZE> {}
-impl<const SIZE: usize> IoCapable<u16> for Mmio<SIZE> {}
-impl<const SIZE: usize> IoCapable<u32> for Mmio<SIZE> {}
+/// Implements [`IoCapable`] on `$mmio` for `$ty` using `$read_fn` and `$write_fn`.
+macro_rules! impl_mmio_io_capable {
+ ($mmio:ident, $(#[$attr:meta])* $ty:ty, $read_fn:ident, $write_fn:ident) => {
+ $(#[$attr])*
+ impl<const SIZE: usize> IoCapable<$ty> for $mmio<SIZE> {
+ unsafe fn io_read(&self, address: usize) -> $ty {
+ // SAFETY: By the trait invariant `address` is a valid address for MMIO operations.
+ unsafe { bindings::$read_fn(address as *const c_void) }
+ }
+ unsafe fn io_write(&self, value: $ty, address: usize) {
+ // SAFETY: By the trait invariant `address` is a valid address for MMIO operations.
+ unsafe { bindings::$write_fn(value, address as *mut c_void) }
+ }
+ }
+ };
+}
+
+// MMIO regions support 8, 16, and 32-bit accesses.
+impl_mmio_io_capable!(Mmio, u8, readb, writeb);
+impl_mmio_io_capable!(Mmio, u16, readw, writew);
+impl_mmio_io_capable!(Mmio, u32, readl, writel);
// MMIO regions on 64-bit systems also support 64-bit accesses.
-#[cfg(CONFIG_64BIT)]
-impl<const SIZE: usize> IoCapable<u64> for Mmio<SIZE> {}
+impl_mmio_io_capable!(
+ Mmio,
+ #[cfg(CONFIG_64BIT)]
+ u64,
+ readq,
+ writeq
+);
impl<const SIZE: usize> Io for Mmio<SIZE> {
/// Returns the base address of this mapping.
diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs
index 6ca4cf75594c..8c8aab2e3f22 100644
--- a/rust/kernel/pci/io.rs
+++ b/rust/kernel/pci/io.rs
@@ -142,10 +142,41 @@ macro_rules! call_config_write {
};
}
+/// Implements [`IoCapable`] on [`ConfigSpace`] for `$ty` using `$read_fn` and `$write_fn`.
+macro_rules! impl_config_space_io_capable {
+ ($ty:ty, $read_fn:ident, $write_fn:ident) => {
+ impl<'a, S: ConfigSpaceKind> IoCapable<$ty> for ConfigSpace<'a, S> {
+ unsafe fn io_read(&self, address: usize) -> $ty {
+ let mut val: $ty = 0;
+
+ // Return value from C function is ignored in infallible accessors.
+ let _ret =
+ // SAFETY: By the type invariant `self.pdev` is a valid address.
+ // CAST: The offset is cast to `i32` because the C functions expect a 32-bit
+ // signed offset parameter. PCI configuration space size is at most 4096 bytes,
+ // so the value always fits within `i32` without truncation or sign change.
+ unsafe { bindings::$read_fn(self.pdev.as_raw(), address as i32, &mut val) };
+
+ val
+ }
+
+ unsafe fn io_write(&self, value: $ty, address: usize) {
+ // Return value from C function is ignored in infallible accessors.
+ let _ret =
+ // SAFETY: By the type invariant `self.pdev` is a valid address.
+ // CAST: The offset is cast to `i32` because the C functions expect a 32-bit
+ // signed offset parameter. PCI configuration space size is at most 4096 bytes,
+ // so the value always fits within `i32` without truncation or sign change.
+ unsafe { bindings::$write_fn(self.pdev.as_raw(), address as i32, value) };
+ }
+ }
+ };
+}
+
// PCI configuration space supports 8, 16, and 32-bit accesses.
-impl<'a, S: ConfigSpaceKind> IoCapable<u8> for ConfigSpace<'a, S> {}
-impl<'a, S: ConfigSpaceKind> IoCapable<u16> for ConfigSpace<'a, S> {}
-impl<'a, S: ConfigSpaceKind> IoCapable<u32> for ConfigSpace<'a, S> {}
+impl_config_space_io_capable!(u8, pci_read_config_byte, pci_write_config_byte);
+impl_config_space_io_capable!(u16, pci_read_config_word, pci_write_config_word);
+impl_config_space_io_capable!(u32, pci_read_config_dword, pci_write_config_dword);
impl<'a, S: ConfigSpaceKind> Io for ConfigSpace<'a, S> {
/// Returns the base address of the I/O region. It is always 0 for configuration space.
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 2/6] rust: io: mem: use non-relaxed I/O ops in examples
2026-02-06 6:00 [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait Alexandre Courbot
2026-02-06 6:00 ` [PATCH v2 1/6] " Alexandre Courbot
@ 2026-02-06 6:00 ` Alexandre Courbot
2026-02-06 6:00 ` [PATCH v2 3/6] rust: io: provide Mmio relaxed ops through a wrapper type Alexandre Courbot
` (7 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Alexandre Courbot @ 2026-02-06 6:00 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Lyude Paul, Eliot Courtney, Alexandre Courbot
The `_relaxed` I/O variant methods are about to be replaced by a wrapper
type exposing this access pattern with the regular methods of the `Io`
trait. Thus replace the examples to use the regular I/O methods.
Since these are examples, we want them to use the most standard ops
anyway, and the relaxed variants were but an addition that was
MMIO-specific.
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Acked-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
rust/kernel/io/mem.rs | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs
index 620022cff401..7dc78d547f7a 100644
--- a/rust/kernel/io/mem.rs
+++ b/rust/kernel/io/mem.rs
@@ -54,6 +54,7 @@ pub(crate) unsafe fn new(device: &'a Device<Bound>, resource: &'a Resource) -> S
/// use kernel::{
/// bindings,
/// device::Core,
+ /// io::Io,
/// of,
/// platform,
/// };
@@ -78,9 +79,9 @@ pub(crate) unsafe fn new(device: &'a Device<Bound>, resource: &'a Resource) -> S
/// let io = iomem.access(pdev.as_ref())?;
///
/// // Read and write a 32-bit value at `offset`.
- /// let data = io.read32_relaxed(offset);
+ /// let data = io.read32(offset);
///
- /// io.write32_relaxed(data, offset);
+ /// io.write32(data, offset);
///
/// # Ok(SampleDriver)
/// }
@@ -117,6 +118,7 @@ pub fn iomap_exclusive_sized<const SIZE: usize>(
/// use kernel::{
/// bindings,
/// device::Core,
+ /// io::Io,
/// of,
/// platform,
/// };
@@ -141,9 +143,9 @@ pub fn iomap_exclusive_sized<const SIZE: usize>(
///
/// let io = iomem.access(pdev.as_ref())?;
///
- /// let data = io.try_read32_relaxed(offset)?;
+ /// let data = io.try_read32(offset)?;
///
- /// io.try_write32_relaxed(data, offset)?;
+ /// io.try_write32(data, offset)?;
///
/// # Ok(SampleDriver)
/// }
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 3/6] rust: io: provide Mmio relaxed ops through a wrapper type
2026-02-06 6:00 [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait Alexandre Courbot
2026-02-06 6:00 ` [PATCH v2 1/6] " Alexandre Courbot
2026-02-06 6:00 ` [PATCH v2 2/6] rust: io: mem: use non-relaxed I/O ops in examples Alexandre Courbot
@ 2026-02-06 6:00 ` Alexandre Courbot
2026-02-06 16:09 ` Daniel Almeida
2026-02-06 6:00 ` [PATCH v2 4/6] rust: io: remove legacy relaxed accessors of Mmio Alexandre Courbot
` (6 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Alexandre Courbot @ 2026-02-06 6:00 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Lyude Paul, Eliot Courtney, Alexandre Courbot
Relaxed I/O accessors for `Mmio` are currently implemented as an extra
set of methods that mirror the ones defined in `Io`, but with the
`_relaxed` suffix.
This makes these methods impossible to use with generic code, which is a
highly plausible proposition now that we have the `Io` trait.
Address this by adding a new `RelaxedMmio` wrapper type for `Mmio` that
provides its own `IoCapable` implementations relying on the relaxed C
accessors. This makes it possible to use relaxed operations on a `Mmio`
simply by wrapping it, and to use `RelaxedMmio` in code generic against
`Io`.
Acked-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
rust/kernel/io.rs | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index dc894a45bbcc..d5d6e9501453 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -695,3 +695,65 @@ pub unsafe fn from_raw(raw: &MmioRaw<SIZE>) -> &Self {
call_mmio_write(writeq_relaxed) <- u64
);
}
+
+/// [`Mmio`] wrapper using relaxed accessors.
+///
+/// This type provides an implementation of [`Io`] that uses relaxed I/O MMIO operands instead of
+/// the regular ones.
+///
+/// See [`Mmio::relaxed`] for a usage example.
+#[repr(transparent)]
+pub struct RelaxedMmio<const SIZE: usize = 0>(Mmio<SIZE>);
+
+impl<const SIZE: usize> Io for RelaxedMmio<SIZE> {
+ #[inline]
+ fn addr(&self) -> usize {
+ self.0.addr()
+ }
+
+ #[inline]
+ fn maxsize(&self) -> usize {
+ self.0.maxsize()
+ }
+}
+
+impl<const SIZE: usize> IoKnownSize for RelaxedMmio<SIZE> {
+ const MIN_SIZE: usize = SIZE;
+}
+
+impl<const SIZE: usize> Mmio<SIZE> {
+ /// Returns a [`RelaxedMmio`] reference that performs relaxed I/O operations.
+ ///
+ /// Relaxed accessors do not provide ordering guarantees with respect to DMA or memory accesses
+ /// and can be used when such ordering is not required.
+ ///
+ /// # Examples
+ ///
+ /// ```no_run
+ /// use kernel::io::{Io, Mmio, RelaxedMmio};
+ ///
+ /// fn do_io(io: &Mmio<0x100>) {
+ /// // The access is performed using `readl_relaxed` instead of `readl`.
+ /// let v = io.relaxed().read32(0x10);
+ /// }
+ ///
+ /// ```
+ pub fn relaxed(&self) -> &RelaxedMmio<SIZE> {
+ // SAFETY: `RelaxedMmio` is `#[repr(transparent)]` over `Mmio`, so `Mmio<SIZE>` and
+ // `RelaxedMmio<SIZE>` have identical layout.
+ unsafe { core::mem::transmute(self) }
+ }
+}
+
+// MMIO regions support 8, 16, and 32-bit accesses.
+impl_mmio_io_capable!(RelaxedMmio, u8, readb_relaxed, writeb_relaxed);
+impl_mmio_io_capable!(RelaxedMmio, u16, readw_relaxed, writew_relaxed);
+impl_mmio_io_capable!(RelaxedMmio, u32, readl_relaxed, writel_relaxed);
+// MMIO regions on 64-bit systems also support 64-bit accesses.
+impl_mmio_io_capable!(
+ RelaxedMmio,
+ #[cfg(CONFIG_64BIT)]
+ u64,
+ readq_relaxed,
+ writeq_relaxed
+);
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 4/6] rust: io: remove legacy relaxed accessors of Mmio
2026-02-06 6:00 [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait Alexandre Courbot
` (2 preceding siblings ...)
2026-02-06 6:00 ` [PATCH v2 3/6] rust: io: provide Mmio relaxed ops through a wrapper type Alexandre Courbot
@ 2026-02-06 6:00 ` Alexandre Courbot
2026-02-06 6:00 ` [PATCH v2 5/6] rust: pci: io: remove overloaded Io methods of ConfigSpace Alexandre Courbot
` (5 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Alexandre Courbot @ 2026-02-06 6:00 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Lyude Paul, Eliot Courtney, Alexandre Courbot
The relaxed access functionality is now provided by the `RelaxedMmio`
wrapper type, and we don't have any user of the legacy methods left.
Remove them.
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Acked-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
rust/kernel/io.rs | 40 ----------------------------------------
1 file changed, 40 deletions(-)
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index d5d6e9501453..06856d6a9b46 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -654,46 +654,6 @@ pub unsafe fn from_raw(raw: &MmioRaw<SIZE>) -> &Self {
// SAFETY: `Mmio` is a transparent wrapper around `MmioRaw`.
unsafe { &*core::ptr::from_ref(raw).cast() }
}
-
- define_read!(infallible, pub read8_relaxed, call_mmio_read(readb_relaxed) -> u8);
- define_read!(infallible, pub read16_relaxed, call_mmio_read(readw_relaxed) -> u16);
- define_read!(infallible, pub read32_relaxed, call_mmio_read(readl_relaxed) -> u32);
- define_read!(
- infallible,
- #[cfg(CONFIG_64BIT)]
- pub read64_relaxed,
- call_mmio_read(readq_relaxed) -> u64
- );
-
- define_read!(fallible, pub try_read8_relaxed, call_mmio_read(readb_relaxed) -> u8);
- define_read!(fallible, pub try_read16_relaxed, call_mmio_read(readw_relaxed) -> u16);
- define_read!(fallible, pub try_read32_relaxed, call_mmio_read(readl_relaxed) -> u32);
- define_read!(
- fallible,
- #[cfg(CONFIG_64BIT)]
- pub try_read64_relaxed,
- call_mmio_read(readq_relaxed) -> u64
- );
-
- define_write!(infallible, pub write8_relaxed, call_mmio_write(writeb_relaxed) <- u8);
- define_write!(infallible, pub write16_relaxed, call_mmio_write(writew_relaxed) <- u16);
- define_write!(infallible, pub write32_relaxed, call_mmio_write(writel_relaxed) <- u32);
- define_write!(
- infallible,
- #[cfg(CONFIG_64BIT)]
- pub write64_relaxed,
- call_mmio_write(writeq_relaxed) <- u64
- );
-
- define_write!(fallible, pub try_write8_relaxed, call_mmio_write(writeb_relaxed) <- u8);
- define_write!(fallible, pub try_write16_relaxed, call_mmio_write(writew_relaxed) <- u16);
- define_write!(fallible, pub try_write32_relaxed, call_mmio_write(writel_relaxed) <- u32);
- define_write!(
- fallible,
- #[cfg(CONFIG_64BIT)]
- pub try_write64_relaxed,
- call_mmio_write(writeq_relaxed) <- u64
- );
}
/// [`Mmio`] wrapper using relaxed accessors.
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 5/6] rust: pci: io: remove overloaded Io methods of ConfigSpace
2026-02-06 6:00 [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait Alexandre Courbot
` (3 preceding siblings ...)
2026-02-06 6:00 ` [PATCH v2 4/6] rust: io: remove legacy relaxed accessors of Mmio Alexandre Courbot
@ 2026-02-06 6:00 ` Alexandre Courbot
2026-02-06 6:00 ` [PATCH v2 6/6] rust: io: remove overloaded Io methods of Mmio Alexandre Courbot
` (4 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Alexandre Courbot @ 2026-02-06 6:00 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Lyude Paul, Eliot Courtney, Alexandre Courbot
Since `ConfigSpace` now has the relevant implementations of `IoCapable`,
the default methods of `Io` can be used in place of the overloaded ones.
Remove them as well as the macros generating them.
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Acked-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
rust/kernel/io.rs | 3 ---
rust/kernel/pci/io.rs | 70 ---------------------------------------------------
2 files changed, 73 deletions(-)
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 06856d6a9b46..9874edda2b28 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -226,8 +226,6 @@ macro_rules! define_read {
}
};
}
-pub(crate) use define_read;
-
macro_rules! define_write {
(infallible, $(#[$attr:meta])* $vis:vis $name:ident, $call_macro:ident($c_fn:ident) <-
$type_name:ty) => {
@@ -259,7 +257,6 @@ macro_rules! define_write {
}
};
}
-pub(crate) use define_write;
/// Checks whether an access of type `U` at the given `offset`
/// is valid within this region.
diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs
index 8c8aab2e3f22..ae78676c927f 100644
--- a/rust/kernel/pci/io.rs
+++ b/rust/kernel/pci/io.rs
@@ -8,8 +8,6 @@
device,
devres::Devres,
io::{
- define_read,
- define_write,
Io,
IoCapable,
IoKnownSize,
@@ -85,63 +83,6 @@ pub struct ConfigSpace<'a, S: ConfigSpaceKind = Extended> {
_marker: PhantomData<S>,
}
-/// Internal helper macros used to invoke C PCI configuration space read functions.
-///
-/// This macro is intended to be used by higher-level PCI configuration space access macros
-/// (define_read) and provides a unified expansion for infallible vs. fallible read semantics. It
-/// emits a direct call into the corresponding C helper and performs the required cast to the Rust
-/// return type.
-///
-/// # Parameters
-///
-/// * `$c_fn` – The C function performing the PCI configuration space write.
-/// * `$self` – The I/O backend object.
-/// * `$ty` – The type of the value to read.
-/// * `$addr` – The PCI configuration space offset to read.
-///
-/// This macro does not perform any validation; all invariants must be upheld by the higher-level
-/// abstraction invoking it.
-macro_rules! call_config_read {
- (infallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr) => {{
- let mut val: $ty = 0;
- // SAFETY: By the type invariant `$self.pdev` is a valid address.
- // CAST: The offset is cast to `i32` because the C functions expect a 32-bit signed offset
- // parameter. PCI configuration space size is at most 4096 bytes, so the value always fits
- // within `i32` without truncation or sign change.
- // Return value from C function is ignored in infallible accessors.
- let _ret = unsafe { bindings::$c_fn($self.pdev.as_raw(), $addr as i32, &mut val) };
- val
- }};
-}
-
-/// Internal helper macros used to invoke C PCI configuration space write functions.
-///
-/// This macro is intended to be used by higher-level PCI configuration space access macros
-/// (define_write) and provides a unified expansion for infallible vs. fallible read semantics. It
-/// emits a direct call into the corresponding C helper and performs the required cast to the Rust
-/// return type.
-///
-/// # Parameters
-///
-/// * `$c_fn` – The C function performing the PCI configuration space write.
-/// * `$self` – The I/O backend object.
-/// * `$ty` – The type of the written value.
-/// * `$addr` – The configuration space offset to write.
-/// * `$value` – The value to write.
-///
-/// This macro does not perform any validation; all invariants must be upheld by the higher-level
-/// abstraction invoking it.
-macro_rules! call_config_write {
- (infallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr, $value:expr) => {
- // SAFETY: By the type invariant `$self.pdev` is a valid address.
- // CAST: The offset is cast to `i32` because the C functions expect a 32-bit signed offset
- // parameter. PCI configuration space size is at most 4096 bytes, so the value always fits
- // within `i32` without truncation or sign change.
- // Return value from C function is ignored in infallible accessors.
- let _ret = unsafe { bindings::$c_fn($self.pdev.as_raw(), $addr as i32, $value) };
- };
-}
-
/// Implements [`IoCapable`] on [`ConfigSpace`] for `$ty` using `$read_fn` and `$write_fn`.
macro_rules! impl_config_space_io_capable {
($ty:ty, $read_fn:ident, $write_fn:ident) => {
@@ -190,17 +131,6 @@ fn addr(&self) -> usize {
fn maxsize(&self) -> usize {
self.pdev.cfg_size().into_raw()
}
-
- // PCI configuration space does not support fallible operations.
- // The default implementations from the Io trait are not used.
-
- define_read!(infallible, read8, call_config_read(pci_read_config_byte) -> u8);
- define_read!(infallible, read16, call_config_read(pci_read_config_word) -> u16);
- define_read!(infallible, read32, call_config_read(pci_read_config_dword) -> u32);
-
- define_write!(infallible, write8, call_config_write(pci_write_config_byte) <- u8);
- define_write!(infallible, write16, call_config_write(pci_write_config_word) <- u16);
- define_write!(infallible, write32, call_config_write(pci_write_config_dword) <- u32);
}
impl<'a, S: ConfigSpaceKind> IoKnownSize for ConfigSpace<'a, S> {
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 6/6] rust: io: remove overloaded Io methods of Mmio
2026-02-06 6:00 [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait Alexandre Courbot
` (4 preceding siblings ...)
2026-02-06 6:00 ` [PATCH v2 5/6] rust: pci: io: remove overloaded Io methods of ConfigSpace Alexandre Courbot
@ 2026-02-06 6:00 ` Alexandre Courbot
2026-02-06 15:02 ` [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait Gary Guo
` (3 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Alexandre Courbot @ 2026-02-06 6:00 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Lyude Paul, Eliot Courtney, Alexandre Courbot
Since `Mmio` now has the relevant implementations of `IoCapable`, the
default methods of `Io` can be used in place of the overloaded ones.
Remove them as well as the macros generating them.
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Acked-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
rust/kernel/io.rs | 161 ------------------------------------------------------
1 file changed, 161 deletions(-)
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 9874edda2b28..b150743ffa4f 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -137,127 +137,6 @@ pub fn maxsize(&self) -> usize {
#[repr(transparent)]
pub struct Mmio<const SIZE: usize = 0>(MmioRaw<SIZE>);
-/// Internal helper macros used to invoke C MMIO read functions.
-///
-/// This macro is intended to be used by higher-level MMIO access macros (define_read) and provides
-/// a unified expansion for infallible vs. fallible read semantics. It emits a direct call into the
-/// corresponding C helper and performs the required cast to the Rust return type.
-///
-/// # Parameters
-///
-/// * `$c_fn` – The C function performing the MMIO read.
-/// * `$self` – The I/O backend object.
-/// * `$ty` – The type of the value to be read.
-/// * `$addr` – The MMIO address to read.
-///
-/// This macro does not perform any validation; all invariants must be upheld by the higher-level
-/// abstraction invoking it.
-macro_rules! call_mmio_read {
- (infallible, $c_fn:ident, $self:ident, $type:ty, $addr:expr) => {
- // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- unsafe { bindings::$c_fn($addr as *const c_void) as $type }
- };
-
- (fallible, $c_fn:ident, $self:ident, $type:ty, $addr:expr) => {{
- // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- Ok(unsafe { bindings::$c_fn($addr as *const c_void) as $type })
- }};
-}
-
-/// Internal helper macros used to invoke C MMIO write functions.
-///
-/// This macro is intended to be used by higher-level MMIO access macros (define_write) and provides
-/// a unified expansion for infallible vs. fallible write semantics. It emits a direct call into the
-/// corresponding C helper and performs the required cast to the Rust return type.
-///
-/// # Parameters
-///
-/// * `$c_fn` – The C function performing the MMIO write.
-/// * `$self` – The I/O backend object.
-/// * `$ty` – The type of the written value.
-/// * `$addr` – The MMIO address to write.
-/// * `$value` – The value to write.
-///
-/// This macro does not perform any validation; all invariants must be upheld by the higher-level
-/// abstraction invoking it.
-macro_rules! call_mmio_write {
- (infallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr, $value:expr) => {
- // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- unsafe { bindings::$c_fn($value, $addr as *mut c_void) }
- };
-
- (fallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr, $value:expr) => {{
- // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- unsafe { bindings::$c_fn($value, $addr as *mut c_void) };
- Ok(())
- }};
-}
-
-macro_rules! define_read {
- (infallible, $(#[$attr:meta])* $vis:vis $name:ident, $call_macro: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])*
- // Always inline to optimize out error path of `io_addr_assert`.
- #[inline(always)]
- $vis 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 IO operations.
- $call_macro!(infallible, $c_fn, self, $type_name, addr)
- }
- };
-
- (fallible, $(#[$attr:meta])* $vis:vis $try_name:ident, $call_macro:ident($c_fn:ident) ->
- $type_name:ty) => {
- /// 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])*
- $vis 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 IO operations.
- $call_macro!(fallible, $c_fn, self, $type_name, addr)
- }
- };
-}
-macro_rules! define_write {
- (infallible, $(#[$attr:meta])* $vis:vis $name:ident, $call_macro: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])*
- // Always inline to optimize out error path of `io_addr_assert`.
- #[inline(always)]
- $vis fn $name(&self, value: $type_name, offset: usize) {
- let addr = self.io_addr_assert::<$type_name>(offset);
-
- $call_macro!(infallible, $c_fn, self, $type_name, addr, value);
- }
- };
-
- (fallible, $(#[$attr:meta])* $vis:vis $try_name:ident, $call_macro:ident($c_fn:ident) <-
- $type_name:ty) => {
- /// 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])*
- $vis fn $try_name(&self, value: $type_name, offset: usize) -> Result {
- let addr = self.io_addr::<$type_name>(offset)?;
-
- $call_macro!(fallible, $c_fn, self, $type_name, addr, value)
- }
- };
-}
-
/// Checks whether an access of type `U` at the given `offset`
/// is valid within this region.
#[inline]
@@ -594,46 +473,6 @@ fn addr(&self) -> usize {
fn maxsize(&self) -> usize {
self.0.maxsize()
}
-
- define_read!(fallible, try_read8, call_mmio_read(readb) -> u8);
- define_read!(fallible, try_read16, call_mmio_read(readw) -> u16);
- define_read!(fallible, try_read32, call_mmio_read(readl) -> u32);
- define_read!(
- fallible,
- #[cfg(CONFIG_64BIT)]
- try_read64,
- call_mmio_read(readq) -> u64
- );
-
- define_write!(fallible, try_write8, call_mmio_write(writeb) <- u8);
- define_write!(fallible, try_write16, call_mmio_write(writew) <- u16);
- define_write!(fallible, try_write32, call_mmio_write(writel) <- u32);
- define_write!(
- fallible,
- #[cfg(CONFIG_64BIT)]
- try_write64,
- call_mmio_write(writeq) <- u64
- );
-
- define_read!(infallible, read8, call_mmio_read(readb) -> u8);
- define_read!(infallible, read16, call_mmio_read(readw) -> u16);
- define_read!(infallible, read32, call_mmio_read(readl) -> u32);
- define_read!(
- infallible,
- #[cfg(CONFIG_64BIT)]
- read64,
- call_mmio_read(readq) -> u64
- );
-
- define_write!(infallible, write8, call_mmio_write(writeb) <- u8);
- define_write!(infallible, write16, call_mmio_write(writew) <- u16);
- define_write!(infallible, write32, call_mmio_write(writel) <- u32);
- define_write!(
- infallible,
- #[cfg(CONFIG_64BIT)]
- write64,
- call_mmio_write(writeq) <- u64
- );
}
impl<const SIZE: usize> IoKnownSize for Mmio<SIZE> {
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait
2026-02-06 6:00 [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait Alexandre Courbot
` (5 preceding siblings ...)
2026-02-06 6:00 ` [PATCH v2 6/6] rust: io: remove overloaded Io methods of Mmio Alexandre Courbot
@ 2026-02-06 15:02 ` Gary Guo
2026-02-06 19:12 ` Danilo Krummrich
` (2 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Gary Guo @ 2026-02-06 15:02 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, Daniel Almeida,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Lyude Paul, Eliot Courtney
On Fri Feb 6, 2026 at 6:00 AM GMT, Alexandre Courbot wrote:
> `IoCapable<T>` is currently used as a marker trait to signal that the
> methods of the `Io` trait corresponding to `T` have been overridden by
> the implementor (the default implementations triggering a build-time
> error).
>
> This goes against the DRY principle and separates the signaling of the
> capability from its implementation, making it possible to forget a step
> while implementing a new `Io`.
>
> Another undesirable side-effect is that it makes the implementation of
> I/O backends boilerplate-y and convoluted: currently this is done using
> two levels of imbricated macros that generate unsafe code.
>
> This patchset fixes these issues by turning `IoCapable` into a
> functional trait including the raw implementation of the I/O
> accessors for `T` using unsafe methods that work with an arbitrary
> address, and making the default methods of `Io` call into these
> implementations after checking the bounds.
>
> This makes overriding these accessors on all I/O backends unneeded,
> resulting in a net -90 LoCs while avoiding a violation of the DRY
> principle and reducing (and simplifying) the use of macros generating
> unsafe code.
>
> Patch 1 adds the `io_read` and `io_write` unsafe methods to `IoCapable`,
> provides the required implementations for `Mmio` and `pci::ConfigSpace`,
> and make the default I/O accessors of `Io` call into them instead of
> failing.
>
> Patches 2 to 4 get rid of the `_relaxed` variants we had in `Mmio`,
> since these are not usable in code generic against `Io` and makes use of
> the macros we want to remove. They are replaced by a `RelaxedMmio`
> wrapper type that implements the required `IoCapable`s and is thus
> usable in generic code.
>
> Patches 5 and 6 remove the overloaded implementations of the `Io`
> methods for `pci::ConfigSpace` and `Mmio`, respectively, while also
> deleting the macros that have become unused.
>
> There is more work coming on top of this patchset (notably the
> `register!` macro with proper I/O), but I wanted to send this work first
> as it stands on its own IMHO and is more digestible from a review
> perspective.
>
> The base for this patchset is `driver-core-testing`.
>
> Cc: Zhi Wang <zhiw@nvidia.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Eliot Courtney <ecourtney@nvidia.com>
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
For the whole series:
Reviewed-by: Gary Guo <gary@garyguo.net>
> ---
> Changes in v2:
> - Turn `RelaxedMmio` into an actual wrapper type and make it available
> through a `Mmio::relaxed()` method.
> - Link to v1: https://patch.msgid.link/20260202-io-v1-0-9bb2177d23be@nvidia.com
>
> ---
> Alexandre Courbot (6):
> rust: io: turn IoCapable into a functional trait
> rust: io: mem: use non-relaxed I/O ops in examples
> rust: io: provide Mmio relaxed ops through a wrapper type
> rust: io: remove legacy relaxed accessors of Mmio
> rust: pci: io: remove overloaded Io methods of ConfigSpace
> rust: io: remove overloaded Io methods of Mmio
>
> rust/kernel/io.rs | 435 ++++++++++++++++++++++----------------------------
> rust/kernel/io/mem.rs | 10 +-
> rust/kernel/pci/io.rs | 99 ++++--------
> 3 files changed, 227 insertions(+), 317 deletions(-)
> ---
> base-commit: f55ae0bfa00e446ea751d09f468daeafc303e03f
> change-id: 20260202-io-81fd368f7565
>
> Best regards,
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 3/6] rust: io: provide Mmio relaxed ops through a wrapper type
2026-02-06 6:00 ` [PATCH v2 3/6] rust: io: provide Mmio relaxed ops through a wrapper type Alexandre Courbot
@ 2026-02-06 16:09 ` Daniel Almeida
0 siblings, 0 replies; 25+ messages in thread
From: Daniel Almeida @ 2026-02-06 16:09 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Bjorn Helgaas, Krzysztof Wilczyński,
driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Lyude Paul, Eliot Courtney
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait
2026-02-06 6:00 [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait Alexandre Courbot
` (6 preceding siblings ...)
2026-02-06 15:02 ` [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait Gary Guo
@ 2026-02-06 19:12 ` Danilo Krummrich
2026-02-06 19:48 ` lyude
2026-03-15 0:56 ` Danilo Krummrich
9 siblings, 0 replies; 25+ messages in thread
From: Danilo Krummrich @ 2026-02-06 19:12 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Alice Ryhl, Daniel Almeida, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Bjorn Helgaas, Krzysztof Wilczyński,
driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Lyude Paul, Eliot Courtney
On Fri Feb 6, 2026 at 7:00 AM CET, Alexandre Courbot wrote:
> Alexandre Courbot (6):
> rust: io: turn IoCapable into a functional trait
> rust: io: mem: use non-relaxed I/O ops in examples
> rust: io: provide Mmio relaxed ops through a wrapper type
> rust: io: remove legacy relaxed accessors of Mmio
> rust: pci: io: remove overloaded Io methods of ConfigSpace
> rust: io: remove overloaded Io methods of Mmio
I will queue this up once -rc1 is out.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait
2026-02-06 6:00 [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait Alexandre Courbot
` (7 preceding siblings ...)
2026-02-06 19:12 ` Danilo Krummrich
@ 2026-02-06 19:48 ` lyude
2026-02-12 12:28 ` Alexandre Courbot
2026-03-15 0:56 ` Danilo Krummrich
9 siblings, 1 reply; 25+ messages in thread
From: lyude @ 2026-02-06 19:48 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, Daniel Almeida,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Eliot Courtney
A thought that crossed my mind just now with this patch series while I
was working on converting iosys_map over to it: shouldn't we have some
unit tests for confirming runtime bounds checking works as well?
On Fri, 2026-02-06 at 15:00 +0900, Alexandre Courbot wrote:
> `IoCapable<T>` is currently used as a marker trait to signal that the
> methods of the `Io` trait corresponding to `T` have been overridden
> by
> the implementor (the default implementations triggering a build-time
> error).
>
> This goes against the DRY principle and separates the signaling of
> the
> capability from its implementation, making it possible to forget a
> step
> while implementing a new `Io`.
>
> Another undesirable side-effect is that it makes the implementation
> of
> I/O backends boilerplate-y and convoluted: currently this is done
> using
> two levels of imbricated macros that generate unsafe code.
>
> This patchset fixes these issues by turning `IoCapable` into a
> functional trait including the raw implementation of the I/O
> accessors for `T` using unsafe methods that work with an arbitrary
> address, and making the default methods of `Io` call into these
> implementations after checking the bounds.
>
> This makes overriding these accessors on all I/O backends unneeded,
> resulting in a net -90 LoCs while avoiding a violation of the DRY
> principle and reducing (and simplifying) the use of macros generating
> unsafe code.
>
> Patch 1 adds the `io_read` and `io_write` unsafe methods to
> `IoCapable`,
> provides the required implementations for `Mmio` and
> `pci::ConfigSpace`,
> and make the default I/O accessors of `Io` call into them instead of
> failing.
>
> Patches 2 to 4 get rid of the `_relaxed` variants we had in `Mmio`,
> since these are not usable in code generic against `Io` and makes use
> of
> the macros we want to remove. They are replaced by a `RelaxedMmio`
> wrapper type that implements the required `IoCapable`s and is thus
> usable in generic code.
>
> Patches 5 and 6 remove the overloaded implementations of the `Io`
> methods for `pci::ConfigSpace` and `Mmio`, respectively, while also
> deleting the macros that have become unused.
>
> There is more work coming on top of this patchset (notably the
> `register!` macro with proper I/O), but I wanted to send this work
> first
> as it stands on its own IMHO and is more digestible from a review
> perspective.
>
> The base for this patchset is `driver-core-testing`.
>
> Cc: Zhi Wang <zhiw@nvidia.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Eliot Courtney <ecourtney@nvidia.com>
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> Changes in v2:
> - Turn `RelaxedMmio` into an actual wrapper type and make it
> available
> through a `Mmio::relaxed()` method.
> - Link to v1:
> https://patch.msgid.link/20260202-io-v1-0-9bb2177d23be@nvidia.com
>
> ---
> Alexandre Courbot (6):
> rust: io: turn IoCapable into a functional trait
> rust: io: mem: use non-relaxed I/O ops in examples
> rust: io: provide Mmio relaxed ops through a wrapper type
> rust: io: remove legacy relaxed accessors of Mmio
> rust: pci: io: remove overloaded Io methods of ConfigSpace
> rust: io: remove overloaded Io methods of Mmio
>
> rust/kernel/io.rs | 435 ++++++++++++++++++++++------------------
> ----------
> rust/kernel/io/mem.rs | 10 +-
> rust/kernel/pci/io.rs | 99 ++++--------
> 3 files changed, 227 insertions(+), 317 deletions(-)
> ---
> base-commit: f55ae0bfa00e446ea751d09f468daeafc303e03f
> change-id: 20260202-io-81fd368f7565
>
> Best regards,
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] rust: io: turn IoCapable into a functional trait
2026-02-06 6:00 ` [PATCH v2 1/6] " Alexandre Courbot
@ 2026-02-06 20:29 ` lyude
2026-02-12 12:04 ` Alexandre Courbot
0 siblings, 1 reply; 25+ messages in thread
From: lyude @ 2026-02-06 20:29 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, Daniel Almeida,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Bjorn Helgaas,
Krzysztof Wilczyński
Cc: driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Eliot Courtney
On Fri, 2026-02-06 at 15:00 +0900, Alexandre Courbot wrote:
> `IoCapable<T>` is currently used as a marker trait to signal that the
> methods of the `Io` trait corresponding to `T` have been overridden
> by
> the implementor (the default implementations triggering a build-time
> error).
>
> This goes against the DRY principle and separates the signaling of
> the
> capability from its implementation, making it possible to forget a
> step
> while implementing a new `Io`.
I realized another thing that might be missing from this series while
porting the iosys_map code over: it seems like IoCapable is fine being
a safe trait, but Io and IoKnownSize both seem like they should
actually be unsafe traits. The main reason being that IoCapable doesn't
actually provide any methods that provide a guarantee of being able to
read/write from the IO space, but Io does - and IoKnownSize is making
the guarantee the IO space is at least of size IoKnownSize::MIN_SIZE.
>
> Another undesirable side-effect is that it makes the implementation
> of
> I/O backends boilerplate-y and convoluted: currently this is done
> using
> two levels of imbricated macros that generate unsafe code.
>
> Fix these issues by turning `IoCapable` into a functional trait that
> includes the raw implementation of the I/O access for `T` using
> unsafe methods that work with an arbitrary address.
>
> This allows us to turn the default methods of `Io` into regular
> methods
> that check the passed offset, turn it into an address, and call into
> the
> corresponding `IoCapable` functions, removing the need to overload
> them
> at all.
>
> `IoCapable` must still be implemented for all supported primitive
> types,
> which is still done more concisely using a macro, but this macro
> becomes
> much simpler and does not require calling into another one.
>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Acked-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> rust/kernel/io.rs | 169 ++++++++++++++++++++++++++++++++++++++--
> ----------
> rust/kernel/pci/io.rs | 37 ++++++++++-
> 2 files changed, 163 insertions(+), 43 deletions(-)
>
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index c1cca7b438c3..dc894a45bbcc 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -273,14 +273,29 @@ const fn offset_valid<U>(offset: usize, size:
> usize) -> bool {
> }
> }
>
> -/// Marker trait indicating that an I/O backend supports operations
> of a certain type.
> +/// Trait indicating that an I/O backend supports operations of a
> certain type and providing an
> +/// implementation for these operations.
> ///
> /// Different I/O backends can implement this trait to expose only
> the operations they support.
> ///
> /// For example, a PCI configuration space may implement
> `IoCapable<u8>`, `IoCapable<u16>`,
> /// and `IoCapable<u32>`, but not `IoCapable<u64>`, while an MMIO
> region on a 64-bit
> /// system might implement all four.
> -pub trait IoCapable<T> {}
> +pub trait IoCapable<T> {
> + /// Performs an I/O read of type `T` at `address` and returns
> the result.
> + ///
> + /// # Safety
> + ///
> + /// The range `[address..address + size_of::<T>()]` must be
> within the bounds of `Self`.
> + unsafe fn io_read(&self, address: usize) -> T;
> +
> + /// Performs an I/O write of `value` at `address`.
> + ///
> + /// # Safety
> + ///
> + /// The range `[address..address + size_of::<T>()]` must be
> within the bounds of `Self`.
> + unsafe fn io_write(&self, value: T, address: usize);
> +}
>
> /// Types implementing this trait (e.g. MMIO BARs or PCI config
> regions)
> /// can perform I/O operations on regions of memory.
> @@ -322,146 +337,198 @@ fn io_addr<U>(&self, offset: usize) ->
> Result<usize> {
>
> /// Fallible 8-bit read with runtime bounds check.
> #[inline(always)]
> - fn try_read8(&self, _offset: usize) -> Result<u8>
> + fn try_read8(&self, offset: usize) -> Result<u8>
> where
> Self: IoCapable<u8>,
> {
> - build_error!("Backend does not support fallible 8-bit read")
> + let address = self.io_addr::<u8>(offset)?;
> +
> + // SAFETY: `address` has been validated by `io_addr`.
> + Ok(unsafe { self.io_read(address) })
> }
>
> /// Fallible 16-bit read with runtime bounds check.
> #[inline(always)]
> - fn try_read16(&self, _offset: usize) -> Result<u16>
> + fn try_read16(&self, offset: usize) -> Result<u16>
> where
> Self: IoCapable<u16>,
> {
> - build_error!("Backend does not support fallible 16-bit
> read")
> + let address = self.io_addr::<u16>(offset)?;
> +
> + // SAFETY: `address` has been validated by `io_addr`.
> + Ok(unsafe { self.io_read(address) })
> }
>
> /// Fallible 32-bit read with runtime bounds check.
> #[inline(always)]
> - fn try_read32(&self, _offset: usize) -> Result<u32>
> + fn try_read32(&self, offset: usize) -> Result<u32>
> where
> Self: IoCapable<u32>,
> {
> - build_error!("Backend does not support fallible 32-bit
> read")
> + let address = self.io_addr::<u32>(offset)?;
> +
> + // SAFETY: `address` has been validated by `io_addr`.
> + Ok(unsafe { self.io_read(address) })
> }
>
> /// Fallible 64-bit read with runtime bounds check.
> #[inline(always)]
> - fn try_read64(&self, _offset: usize) -> Result<u64>
> + fn try_read64(&self, offset: usize) -> Result<u64>
> where
> Self: IoCapable<u64>,
> {
> - build_error!("Backend does not support fallible 64-bit
> read")
> + let address = self.io_addr::<u64>(offset)?;
> +
> + // SAFETY: `address` has been validated by `io_addr`.
> + Ok(unsafe { self.io_read(address) })
> }
>
> /// Fallible 8-bit write with runtime bounds check.
> #[inline(always)]
> - fn try_write8(&self, _value: u8, _offset: usize) -> Result
> + fn try_write8(&self, value: u8, offset: usize) -> Result
> where
> Self: IoCapable<u8>,
> {
> - build_error!("Backend does not support fallible 8-bit
> write")
> + let address = self.io_addr::<u8>(offset)?;
> +
> + // SAFETY: `address` has been validated by `io_addr`.
> + unsafe { self.io_write(value, address) };
> + Ok(())
> }
>
> /// Fallible 16-bit write with runtime bounds check.
> #[inline(always)]
> - fn try_write16(&self, _value: u16, _offset: usize) -> Result
> + fn try_write16(&self, value: u16, offset: usize) -> Result
> where
> Self: IoCapable<u16>,
> {
> - build_error!("Backend does not support fallible 16-bit
> write")
> + let address = self.io_addr::<u16>(offset)?;
> +
> + // SAFETY: `address` has been validated by `io_addr`.
> + unsafe { self.io_write(value, address) };
> + Ok(())
> }
>
> /// Fallible 32-bit write with runtime bounds check.
> #[inline(always)]
> - fn try_write32(&self, _value: u32, _offset: usize) -> Result
> + fn try_write32(&self, value: u32, offset: usize) -> Result
> where
> Self: IoCapable<u32>,
> {
> - build_error!("Backend does not support fallible 32-bit
> write")
> + let address = self.io_addr::<u32>(offset)?;
> +
> + // SAFETY: `address` has been validated by `io_addr`.
> + unsafe { self.io_write(value, address) };
> + Ok(())
> }
>
> /// Fallible 64-bit write with runtime bounds check.
> #[inline(always)]
> - fn try_write64(&self, _value: u64, _offset: usize) -> Result
> + fn try_write64(&self, value: u64, offset: usize) -> Result
> where
> Self: IoCapable<u64>,
> {
> - build_error!("Backend does not support fallible 64-bit
> write")
> + let address = self.io_addr::<u64>(offset)?;
> +
> + // SAFETY: `address` has been validated by `io_addr`.
> + unsafe { self.io_write(value, address) };
> + Ok(())
> }
>
> /// Infallible 8-bit read with compile-time bounds check.
> #[inline(always)]
> - fn read8(&self, _offset: usize) -> u8
> + fn read8(&self, offset: usize) -> u8
> where
> Self: IoKnownSize + IoCapable<u8>,
> {
> - build_error!("Backend does not support infallible 8-bit
> read")
> + let address = self.io_addr_assert::<u8>(offset);
> +
> + // SAFETY: `address` has been validated by `io_addr_assert`.
> + unsafe { self.io_read(address) }
> }
>
> /// Infallible 16-bit read with compile-time bounds check.
> #[inline(always)]
> - fn read16(&self, _offset: usize) -> u16
> + fn read16(&self, offset: usize) -> u16
> where
> Self: IoKnownSize + IoCapable<u16>,
> {
> - build_error!("Backend does not support infallible 16-bit
> read")
> + let address = self.io_addr_assert::<u16>(offset);
> +
> + // SAFETY: `address` has been validated by `io_addr_assert`.
> + unsafe { self.io_read(address) }
> }
>
> /// Infallible 32-bit read with compile-time bounds check.
> #[inline(always)]
> - fn read32(&self, _offset: usize) -> u32
> + fn read32(&self, offset: usize) -> u32
> where
> Self: IoKnownSize + IoCapable<u32>,
> {
> - build_error!("Backend does not support infallible 32-bit
> read")
> + let address = self.io_addr_assert::<u32>(offset);
> +
> + // SAFETY: `address` has been validated by `io_addr_assert`.
> + unsafe { self.io_read(address) }
> }
>
> /// Infallible 64-bit read with compile-time bounds check.
> #[inline(always)]
> - fn read64(&self, _offset: usize) -> u64
> + fn read64(&self, offset: usize) -> u64
> where
> Self: IoKnownSize + IoCapable<u64>,
> {
> - build_error!("Backend does not support infallible 64-bit
> read")
> + let address = self.io_addr_assert::<u64>(offset);
> +
> + // SAFETY: `address` has been validated by `io_addr_assert`.
> + unsafe { self.io_read(address) }
> }
>
> /// Infallible 8-bit write with compile-time bounds check.
> #[inline(always)]
> - fn write8(&self, _value: u8, _offset: usize)
> + fn write8(&self, value: u8, offset: usize)
> where
> Self: IoKnownSize + IoCapable<u8>,
> {
> - build_error!("Backend does not support infallible 8-bit
> write")
> + let address = self.io_addr_assert::<u8>(offset);
> +
> + // SAFETY: `address` has been validated by `io_addr_assert`.
> + unsafe { self.io_write(value, address) }
> }
>
> /// Infallible 16-bit write with compile-time bounds check.
> #[inline(always)]
> - fn write16(&self, _value: u16, _offset: usize)
> + fn write16(&self, value: u16, offset: usize)
> where
> Self: IoKnownSize + IoCapable<u16>,
> {
> - build_error!("Backend does not support infallible 16-bit
> write")
> + let address = self.io_addr_assert::<u16>(offset);
> +
> + // SAFETY: `address` has been validated by `io_addr_assert`.
> + unsafe { self.io_write(value, address) }
> }
>
> /// Infallible 32-bit write with compile-time bounds check.
> #[inline(always)]
> - fn write32(&self, _value: u32, _offset: usize)
> + fn write32(&self, value: u32, offset: usize)
> where
> Self: IoKnownSize + IoCapable<u32>,
> {
> - build_error!("Backend does not support infallible 32-bit
> write")
> + let address = self.io_addr_assert::<u32>(offset);
> +
> + // SAFETY: `address` has been validated by `io_addr_assert`.
> + unsafe { self.io_write(value, address) }
> }
>
> /// Infallible 64-bit write with compile-time bounds check.
> #[inline(always)]
> - fn write64(&self, _value: u64, _offset: usize)
> + fn write64(&self, value: u64, offset: usize)
> where
> Self: IoKnownSize + IoCapable<u64>,
> {
> - build_error!("Backend does not support infallible 64-bit
> write")
> + let address = self.io_addr_assert::<u64>(offset);
> +
> + // SAFETY: `address` has been validated by `io_addr_assert`.
> + unsafe { self.io_write(value, address) }
> }
> }
>
> @@ -487,14 +554,36 @@ fn io_addr_assert<U>(&self, offset: usize) ->
> usize {
> }
> }
>
> -// MMIO regions support 8, 16, and 32-bit accesses.
> -impl<const SIZE: usize> IoCapable<u8> for Mmio<SIZE> {}
> -impl<const SIZE: usize> IoCapable<u16> for Mmio<SIZE> {}
> -impl<const SIZE: usize> IoCapable<u32> for Mmio<SIZE> {}
> +/// Implements [`IoCapable`] on `$mmio` for `$ty` using `$read_fn`
> and `$write_fn`.
> +macro_rules! impl_mmio_io_capable {
> + ($mmio:ident, $(#[$attr:meta])* $ty:ty, $read_fn:ident,
> $write_fn:ident) => {
> + $(#[$attr])*
> + impl<const SIZE: usize> IoCapable<$ty> for $mmio<SIZE> {
> + unsafe fn io_read(&self, address: usize) -> $ty {
> + // SAFETY: By the trait invariant `address` is a
> valid address for MMIO operations.
> + unsafe { bindings::$read_fn(address as *const
> c_void) }
> + }
>
> + unsafe fn io_write(&self, value: $ty, address: usize) {
> + // SAFETY: By the trait invariant `address` is a
> valid address for MMIO operations.
> + unsafe { bindings::$write_fn(value, address as *mut
> c_void) }
> + }
> + }
> + };
> +}
> +
> +// MMIO regions support 8, 16, and 32-bit accesses.
> +impl_mmio_io_capable!(Mmio, u8, readb, writeb);
> +impl_mmio_io_capable!(Mmio, u16, readw, writew);
> +impl_mmio_io_capable!(Mmio, u32, readl, writel);
> // MMIO regions on 64-bit systems also support 64-bit accesses.
> -#[cfg(CONFIG_64BIT)]
> -impl<const SIZE: usize> IoCapable<u64> for Mmio<SIZE> {}
> +impl_mmio_io_capable!(
> + Mmio,
> + #[cfg(CONFIG_64BIT)]
> + u64,
> + readq,
> + writeq
> +);
>
> impl<const SIZE: usize> Io for Mmio<SIZE> {
> /// Returns the base address of this mapping.
> diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs
> index 6ca4cf75594c..8c8aab2e3f22 100644
> --- a/rust/kernel/pci/io.rs
> +++ b/rust/kernel/pci/io.rs
> @@ -142,10 +142,41 @@ macro_rules! call_config_write {
> };
> }
>
> +/// Implements [`IoCapable`] on [`ConfigSpace`] for `$ty` using
> `$read_fn` and `$write_fn`.
> +macro_rules! impl_config_space_io_capable {
> + ($ty:ty, $read_fn:ident, $write_fn:ident) => {
> + impl<'a, S: ConfigSpaceKind> IoCapable<$ty> for
> ConfigSpace<'a, S> {
> + unsafe fn io_read(&self, address: usize) -> $ty {
> + let mut val: $ty = 0;
> +
> + // Return value from C function is ignored in
> infallible accessors.
> + let _ret =
> + // SAFETY: By the type invariant `self.pdev` is
> a valid address.
> + // CAST: The offset is cast to `i32` because the
> C functions expect a 32-bit
> + // signed offset parameter. PCI configuration
> space size is at most 4096 bytes,
> + // so the value always fits within `i32` without
> truncation or sign change.
> + unsafe { bindings::$read_fn(self.pdev.as_raw(),
> address as i32, &mut val) };
> +
> + val
> + }
> +
> + unsafe fn io_write(&self, value: $ty, address: usize) {
> + // Return value from C function is ignored in
> infallible accessors.
> + let _ret =
> + // SAFETY: By the type invariant `self.pdev` is
> a valid address.
> + // CAST: The offset is cast to `i32` because the
> C functions expect a 32-bit
> + // signed offset parameter. PCI configuration
> space size is at most 4096 bytes,
> + // so the value always fits within `i32` without
> truncation or sign change.
> + unsafe { bindings::$write_fn(self.pdev.as_raw(),
> address as i32, value) };
> + }
> + }
> + };
> +}
> +
> // PCI configuration space supports 8, 16, and 32-bit accesses.
> -impl<'a, S: ConfigSpaceKind> IoCapable<u8> for ConfigSpace<'a, S> {}
> -impl<'a, S: ConfigSpaceKind> IoCapable<u16> for ConfigSpace<'a, S>
> {}
> -impl<'a, S: ConfigSpaceKind> IoCapable<u32> for ConfigSpace<'a, S>
> {}
> +impl_config_space_io_capable!(u8, pci_read_config_byte,
> pci_write_config_byte);
> +impl_config_space_io_capable!(u16, pci_read_config_word,
> pci_write_config_word);
> +impl_config_space_io_capable!(u32, pci_read_config_dword,
> pci_write_config_dword);
>
> impl<'a, S: ConfigSpaceKind> Io for ConfigSpace<'a, S> {
> /// Returns the base address of the I/O region. It is always 0
> for configuration space.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] rust: io: turn IoCapable into a functional trait
2026-02-06 20:29 ` lyude
@ 2026-02-12 12:04 ` Alexandre Courbot
2026-02-12 12:23 ` Danilo Krummrich
2026-02-12 14:11 ` Gary Guo
0 siblings, 2 replies; 25+ messages in thread
From: Alexandre Courbot @ 2026-02-12 12:04 UTC (permalink / raw)
To: lyude, Danilo Krummrich, Alice Ryhl, Daniel Almeida
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Bjorn Helgaas,
Krzysztof Wilczyński, driver-core, rust-for-linux,
linux-kernel, linux-pci, Zhi Wang, Eliot Courtney
Hi Lyude,
On Sat Feb 7, 2026 at 5:29 AM JST, lyude wrote:
> On Fri, 2026-02-06 at 15:00 +0900, Alexandre Courbot wrote:
>> `IoCapable<T>` is currently used as a marker trait to signal that the
>> methods of the `Io` trait corresponding to `T` have been overridden
>> by
>> the implementor (the default implementations triggering a build-time
>> error).
>>
>> This goes against the DRY principle and separates the signaling of
>> the
>> capability from its implementation, making it possible to forget a
>> step
>> while implementing a new `Io`.
>
> I realized another thing that might be missing from this series while
> porting the iosys_map code over: it seems like IoCapable is fine being
> a safe trait, but Io and IoKnownSize both seem like they should
> actually be unsafe traits. The main reason being that IoCapable doesn't
> actually provide any methods that provide a guarantee of being able to
> read/write from the IO space, but Io does - and IoKnownSize is making
> the guarantee the IO space is at least of size IoKnownSize::MIN_SIZE.
Mmm I think you are correct. For instance, nothing prevents someone from
implementing `Io` with bogus `addr()` and `maxsize()` methods, which
would trigger undefined behavior with purely safe code.
Danilo/Alice/Daniel: I suppose we want this before this series gets
merged? If so I will respin.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] rust: io: turn IoCapable into a functional trait
2026-02-12 12:04 ` Alexandre Courbot
@ 2026-02-12 12:23 ` Danilo Krummrich
2026-02-12 14:11 ` Gary Guo
1 sibling, 0 replies; 25+ messages in thread
From: Danilo Krummrich @ 2026-02-12 12:23 UTC (permalink / raw)
To: Alexandre Courbot
Cc: lyude, Alice Ryhl, Daniel Almeida, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Bjorn Helgaas, Krzysztof Wilczyński,
driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Eliot Courtney
On Thu Feb 12, 2026 at 1:04 PM CET, Alexandre Courbot wrote:
> Danilo/Alice/Daniel: I suppose we want this before this series gets
> merged? If so I will respin.
I think the corresponding fixes should not be part of this series, as they are
independent.
And except for patch 3 that implements another I/O backend, they do not affect
this series, right?
So, for me it would be good enough if you post the fixes (if you want to hack
them up) and paste a diff for patch 3, as it should be trivial. I.e. no need to
resend the whole series.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait
2026-02-06 19:48 ` lyude
@ 2026-02-12 12:28 ` Alexandre Courbot
2026-02-12 14:40 ` Daniel Almeida
0 siblings, 1 reply; 25+ messages in thread
From: Alexandre Courbot @ 2026-02-12 12:28 UTC (permalink / raw)
To: lyude
Cc: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Bjorn Helgaas,
Krzysztof Wilczyński, driver-core, rust-for-linux,
linux-kernel, linux-pci, Zhi Wang, Eliot Courtney
On Sat Feb 7, 2026 at 4:48 AM JST, lyude wrote:
> A thought that crossed my mind just now with this patch series while I
> was working on converting iosys_map over to it: shouldn't we have some
> unit tests for confirming runtime bounds checking works as well?
I guess we would need a mock I/O type for that - is that what you had in
mind, or did I miss your idea?
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] rust: io: turn IoCapable into a functional trait
2026-02-12 12:04 ` Alexandre Courbot
2026-02-12 12:23 ` Danilo Krummrich
@ 2026-02-12 14:11 ` Gary Guo
2026-02-12 14:52 ` Danilo Krummrich
1 sibling, 1 reply; 25+ messages in thread
From: Gary Guo @ 2026-02-12 14:11 UTC (permalink / raw)
To: Alexandre Courbot, lyude, Danilo Krummrich, Alice Ryhl,
Daniel Almeida
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Bjorn Helgaas,
Krzysztof Wilczyński, driver-core, rust-for-linux,
linux-kernel, linux-pci, Zhi Wang, Eliot Courtney
On Thu Feb 12, 2026 at 8:04 PM CST, Alexandre Courbot wrote:
> Hi Lyude,
>
> On Sat Feb 7, 2026 at 5:29 AM JST, lyude wrote:
>> On Fri, 2026-02-06 at 15:00 +0900, Alexandre Courbot wrote:
>>> `IoCapable<T>` is currently used as a marker trait to signal that the
>>> methods of the `Io` trait corresponding to `T` have been overridden
>>> by
>>> the implementor (the default implementations triggering a build-time
>>> error).
>>>
>>> This goes against the DRY principle and separates the signaling of
>>> the
>>> capability from its implementation, making it possible to forget a
>>> step
>>> while implementing a new `Io`.
>>
>> I realized another thing that might be missing from this series while
>> porting the iosys_map code over: it seems like IoCapable is fine being
>> a safe trait, but Io and IoKnownSize both seem like they should
>> actually be unsafe traits. The main reason being that IoCapable doesn't
>> actually provide any methods that provide a guarantee of being able to
>> read/write from the IO space, but Io does - and IoKnownSize is making
>> the guarantee the IO space is at least of size IoKnownSize::MIN_SIZE.
>
> Mmm I think you are correct. For instance, nothing prevents someone from
> implementing `Io` with bogus `addr()` and `maxsize()` methods, which
> would trigger undefined behavior with purely safe code.
They can, but the `Io` trait just passes the wrong address to the `IoCapable`
trait, and nothing horrible can happen without doing things unsafely inside
`IoCapable` impl, which is controlled by the user who implements `Io`. It looks
to me that unsafe code is still needed to do bogus things.
Can you give a more detailed example on how safe code can cause issue with a
safe `Io` trait?
Best,
Gary
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait
2026-02-12 12:28 ` Alexandre Courbot
@ 2026-02-12 14:40 ` Daniel Almeida
0 siblings, 0 replies; 25+ messages in thread
From: Daniel Almeida @ 2026-02-12 14:40 UTC (permalink / raw)
To: Alexandre Courbot
Cc: lyude, Danilo Krummrich, Alice Ryhl, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Bjorn Helgaas, Krzysztof Wilczyński,
driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Eliot Courtney
> On 12 Feb 2026, at 09:29, Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> On Sat Feb 7, 2026 at 4:48 AM JST, lyude wrote:
>> A thought that crossed my mind just now with this patch series while I
>> was working on converting iosys_map over to it: shouldn't we have some
>> unit tests for confirming runtime bounds checking works as well?
>
> I guess we would need a mock I/O type for that - is that what you had in
> mind, or did I miss your idea?
>
This is probably yet another use case for the “system memory” backend we’ve been discussing recently.
— Daniel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] rust: io: turn IoCapable into a functional trait
2026-02-12 14:11 ` Gary Guo
@ 2026-02-12 14:52 ` Danilo Krummrich
2026-02-16 11:51 ` Alexandre Courbot
0 siblings, 1 reply; 25+ messages in thread
From: Danilo Krummrich @ 2026-02-12 14:52 UTC (permalink / raw)
To: Gary Guo
Cc: Alexandre Courbot, lyude, Alice Ryhl, Daniel Almeida,
Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Bjorn Helgaas,
Krzysztof Wilczyński, driver-core, rust-for-linux,
linux-kernel, linux-pci, Zhi Wang, Eliot Courtney
On Thu Feb 12, 2026 at 3:11 PM CET, Gary Guo wrote:
> They can, but the `Io` trait just passes the wrong address to the `IoCapable`
> trait, and nothing horrible can happen without doing things unsafely inside
> `IoCapable` impl, which is controlled by the user who implements `Io`. It looks
> to me that unsafe code is still needed to do bogus things.
I think what you mean is that the invariant of `addr` and `maxsize` being valid
is on the implementing type of `Io`, e.g. `MmioRaw` and `Mmio`. The same applies
to IoKnownSize::MIN_SIZE.
To me this seems like a valid way of arguing.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] rust: io: turn IoCapable into a functional trait
2026-02-12 14:52 ` Danilo Krummrich
@ 2026-02-16 11:51 ` Alexandre Courbot
2026-02-16 12:08 ` Danilo Krummrich
0 siblings, 1 reply; 25+ messages in thread
From: Alexandre Courbot @ 2026-02-16 11:51 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Gary Guo, lyude, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Bjorn Helgaas, Krzysztof Wilczyński,
driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Eliot Courtney
On Thu Feb 12, 2026 at 11:52 PM JST, Danilo Krummrich wrote:
> On Thu Feb 12, 2026 at 3:11 PM CET, Gary Guo wrote:
>> They can, but the `Io` trait just passes the wrong address to the `IoCapable`
>> trait, and nothing horrible can happen without doing things unsafely inside
>> `IoCapable` impl, which is controlled by the user who implements `Io`. It looks
>> to me that unsafe code is still needed to do bogus things.
>
> I think what you mean is that the invariant of `addr` and `maxsize` being valid
> is on the implementing type of `Io`, e.g. `MmioRaw` and `Mmio`. The same applies
> to IoKnownSize::MIN_SIZE.
>
> To me this seems like a valid way of arguing.
But still, using only safe code an implementor of `Io` can lie about
this safety statement:
// SAFETY: `address` has been validated by `io_addr`.
Ok(unsafe { self.io_read(address) })
Granted, the same person will likely have written the `IoCapable`
implementations, but its safety requirements cannot be fulfilled unless
the caller also guarantees that the offsets it passes are valid, which
the type system alone cannot guarantee - thus the need to make `Io` and
`IoKnownSize` unsafe IMHO.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] rust: io: turn IoCapable into a functional trait
2026-02-16 11:51 ` Alexandre Courbot
@ 2026-02-16 12:08 ` Danilo Krummrich
2026-02-16 13:27 ` Alexandre Courbot
0 siblings, 1 reply; 25+ messages in thread
From: Danilo Krummrich @ 2026-02-16 12:08 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Gary Guo, lyude, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Bjorn Helgaas, Krzysztof Wilczyński,
driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Eliot Courtney
On Mon Feb 16, 2026 at 12:51 PM CET, Alexandre Courbot wrote:
> On Thu Feb 12, 2026 at 11:52 PM JST, Danilo Krummrich wrote:
>> On Thu Feb 12, 2026 at 3:11 PM CET, Gary Guo wrote:
>>> They can, but the `Io` trait just passes the wrong address to the `IoCapable`
>>> trait, and nothing horrible can happen without doing things unsafely inside
>>> `IoCapable` impl, which is controlled by the user who implements `Io`. It looks
>>> to me that unsafe code is still needed to do bogus things.
>>
>> I think what you mean is that the invariant of `addr` and `maxsize` being valid
>> is on the implementing type of `Io`, e.g. `MmioRaw` and `Mmio`. The same applies
>> to IoKnownSize::MIN_SIZE.
>>
>> To me this seems like a valid way of arguing.
>
> But still, using only safe code an implementor of `Io` can lie about
> this safety statement:
>
> // SAFETY: `address` has been validated by `io_addr`.
> Ok(unsafe { self.io_read(address) })
>
> Granted, the same person will likely have written the `IoCapable`
> implementations, but its safety requirements cannot be fulfilled unless
> the caller also guarantees that the offsets it passes are valid, which
> the type system alone cannot guarantee - thus the need to make `Io` and
> `IoKnownSize` unsafe IMHO.
Hm...the implementor of Io and IoCapable has to justify in the implementation of
IoCapable, i.e. in io_read() and io_write() that the address is in fact correct.
The implementor can't justify this if the address or offset can be bogus in
their implementation of Io.
So, considering that, it looks to me that we don't even need io_read() and
io_write() to be unsafe in the first place?
I.e. we are only passing through values in generic implementation.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] rust: io: turn IoCapable into a functional trait
2026-02-16 12:08 ` Danilo Krummrich
@ 2026-02-16 13:27 ` Alexandre Courbot
2026-02-16 17:04 ` Danilo Krummrich
0 siblings, 1 reply; 25+ messages in thread
From: Alexandre Courbot @ 2026-02-16 13:27 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Gary Guo, lyude, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Bjorn Helgaas, Krzysztof Wilczyński,
driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Eliot Courtney
On Mon Feb 16, 2026 at 9:08 PM JST, Danilo Krummrich wrote:
> On Mon Feb 16, 2026 at 12:51 PM CET, Alexandre Courbot wrote:
>> On Thu Feb 12, 2026 at 11:52 PM JST, Danilo Krummrich wrote:
>>> On Thu Feb 12, 2026 at 3:11 PM CET, Gary Guo wrote:
>>>> They can, but the `Io` trait just passes the wrong address to the `IoCapable`
>>>> trait, and nothing horrible can happen without doing things unsafely inside
>>>> `IoCapable` impl, which is controlled by the user who implements `Io`. It looks
>>>> to me that unsafe code is still needed to do bogus things.
>>>
>>> I think what you mean is that the invariant of `addr` and `maxsize` being valid
>>> is on the implementing type of `Io`, e.g. `MmioRaw` and `Mmio`. The same applies
>>> to IoKnownSize::MIN_SIZE.
>>>
>>> To me this seems like a valid way of arguing.
>>
>> But still, using only safe code an implementor of `Io` can lie about
>> this safety statement:
>>
>> // SAFETY: `address` has been validated by `io_addr`.
>> Ok(unsafe { self.io_read(address) })
>>
>> Granted, the same person will likely have written the `IoCapable`
>> implementations, but its safety requirements cannot be fulfilled unless
>> the caller also guarantees that the offsets it passes are valid, which
>> the type system alone cannot guarantee - thus the need to make `Io` and
>> `IoKnownSize` unsafe IMHO.
>
> Hm...the implementor of Io and IoCapable has to justify in the implementation of
> IoCapable, i.e. in io_read() and io_write() that the address is in fact correct.
It doesn't - here is the implementation of Io for Mmio:
impl<const SIZE: usize> Io for Mmio<SIZE> {
/// Returns the base address of this mapping.
#[inline]
fn addr(&self) -> usize {
self.0.addr()
}
/// Returns the maximum size of this mapping.
#[inline]
fn maxsize(&self) -> usize {
self.0.maxsize()
}
}
Now what prevents me from doing this:
impl<const SIZE: usize> Io for YoloMmio<SIZE> {
fn addr(&self) -> usize {
self.0.addr()
}
fn maxsize(&self) -> usize {
self.0.maxsize() + 0x10000
}
}
With that, I have allowed callers to invoke the unsafe methods of
`IoCapable` on an extra 0x10000 bytes of I/O I don't own, without any
unsafe code.
> So, considering that, it looks to me that we don't even need io_read() and
> io_write() to be unsafe in the first place?
>
> I.e. we are only passing through values in generic implementation.
Possibly? Although `io_read` and `io_write` typically call unsafe C
methods though, so they would have to always maintain *their* safety
requirements if we go that way (instead of delegating that to `Io`
through the `IoCapable` implementation's safety statement).
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] rust: io: turn IoCapable into a functional trait
2026-02-16 13:27 ` Alexandre Courbot
@ 2026-02-16 17:04 ` Danilo Krummrich
2026-02-17 1:36 ` Alexandre Courbot
0 siblings, 1 reply; 25+ messages in thread
From: Danilo Krummrich @ 2026-02-16 17:04 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Gary Guo, lyude, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Bjorn Helgaas, Krzysztof Wilczyński,
driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Eliot Courtney
On Mon Feb 16, 2026 at 2:27 PM CET, Alexandre Courbot wrote:
> It doesn't - here is the implementation of Io for Mmio:
>
> impl<const SIZE: usize> Io for Mmio<SIZE> {
> /// Returns the base address of this mapping.
> #[inline]
> fn addr(&self) -> usize {
> self.0.addr()
> }
>
> /// Returns the maximum size of this mapping.
> #[inline]
> fn maxsize(&self) -> usize {
> self.0.maxsize()
> }
> }
>
> Now what prevents me from doing this:
>
> impl<const SIZE: usize> Io for YoloMmio<SIZE> {
> fn addr(&self) -> usize {
> self.0.addr()
> }
>
> fn maxsize(&self) -> usize {
> self.0.maxsize() + 0x10000
> }
> }
>
> With that, I have allowed callers to invoke the unsafe methods of
> `IoCapable` on an extra 0x10000 bytes of I/O I don't own, without any
> unsafe code.
I don't think you did, as you only present half of your counter example; you
left out the IoCapable part.
I.e. with what you have above cannot uphold the safety justification in the
corresponding IoCapable implementation:
This is the invariant on struct Mmio:
/// # Invariant
///
/// `addr` is the start and `maxsize` the length of valid I/O mapped memory region of size
/// `maxsize`.
And in impl_mmio_io_capable!() you refer to this invariant:
macro_rules! impl_mmio_io_capable {
($mmio:ident, $(#[$attr:meta])* $ty:ty, $read_fn:ident, $write_fn:ident) => {
$(#[$attr])*
impl<const SIZE: usize> IoCapable<$ty> for $mmio<SIZE> {
unsafe fn io_read(&self, address: usize) -> $ty {
// SAFETY: By the trait invariant `address` is a valid address for MMIO operations.
unsafe { bindings::$read_fn(address as *const c_void) }
}
unsafe fn io_write(&self, value: $ty, address: usize) {
// SAFETY: By the trait invariant `address` is a valid address for MMIO operations.
unsafe { bindings::$write_fn(value, address as *mut c_void) }
}
}
};
}
But your YoloMmio implementation doesn't provide this invariant (because it
can't).
So, how do you justify the unsafe call to bindings::$write_fn and
bindings::$read_fn now that you have to call impl_mmio_io_capable!() for
YoloMmio?
Again, you can't justify it, which proves that it doesn't matter what YoloMmio
returns, it matters how you can justify it in io_read() and io_write().
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] rust: io: turn IoCapable into a functional trait
2026-02-16 17:04 ` Danilo Krummrich
@ 2026-02-17 1:36 ` Alexandre Courbot
2026-02-17 11:17 ` Danilo Krummrich
0 siblings, 1 reply; 25+ messages in thread
From: Alexandre Courbot @ 2026-02-17 1:36 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Gary Guo, lyude, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Bjorn Helgaas, Krzysztof Wilczyński,
driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Eliot Courtney
On Tue Feb 17, 2026 at 2:04 AM JST, Danilo Krummrich wrote:
> On Mon Feb 16, 2026 at 2:27 PM CET, Alexandre Courbot wrote:
>> It doesn't - here is the implementation of Io for Mmio:
>>
>> impl<const SIZE: usize> Io for Mmio<SIZE> {
>> /// Returns the base address of this mapping.
>> #[inline]
>> fn addr(&self) -> usize {
>> self.0.addr()
>> }
>>
>> /// Returns the maximum size of this mapping.
>> #[inline]
>> fn maxsize(&self) -> usize {
>> self.0.maxsize()
>> }
>> }
>>
>> Now what prevents me from doing this:
>>
>> impl<const SIZE: usize> Io for YoloMmio<SIZE> {
>> fn addr(&self) -> usize {
>> self.0.addr()
>> }
>>
>> fn maxsize(&self) -> usize {
>> self.0.maxsize() + 0x10000
>> }
>> }
>>
>> With that, I have allowed callers to invoke the unsafe methods of
>> `IoCapable` on an extra 0x10000 bytes of I/O I don't own, without any
>> unsafe code.
>
> I don't think you did, as you only present half of your counter example; you
> left out the IoCapable part.
>
> I.e. with what you have above cannot uphold the safety justification in the
> corresponding IoCapable implementation:
>
> This is the invariant on struct Mmio:
>
> /// # Invariant
> ///
> /// `addr` is the start and `maxsize` the length of valid I/O mapped memory region of size
> /// `maxsize`.
>
> And in impl_mmio_io_capable!() you refer to this invariant:
>
> macro_rules! impl_mmio_io_capable {
> ($mmio:ident, $(#[$attr:meta])* $ty:ty, $read_fn:ident, $write_fn:ident) => {
> $(#[$attr])*
> impl<const SIZE: usize> IoCapable<$ty> for $mmio<SIZE> {
> unsafe fn io_read(&self, address: usize) -> $ty {
> // SAFETY: By the trait invariant `address` is a valid address for MMIO operations.
> unsafe { bindings::$read_fn(address as *const c_void) }
> }
>
> unsafe fn io_write(&self, value: $ty, address: usize) {
> // SAFETY: By the trait invariant `address` is a valid address for MMIO operations.
> unsafe { bindings::$write_fn(value, address as *mut c_void) }
> }
> }
> };
> }
>
> But your YoloMmio implementation doesn't provide this invariant (because it
> can't).
>
> So, how do you justify the unsafe call to bindings::$write_fn and
> bindings::$read_fn now that you have to call impl_mmio_io_capable!() for
> YoloMmio?
>
> Again, you can't justify it, which proves that it doesn't matter what YoloMmio
> returns, it matters how you can justify it in io_read() and io_write().
Ah, I think I finally get it now. `Io` and `IoKnownSize` can piggyback
on the `IoCapable` safety requirement, because all the unsafe code
requires an `IoCapable` implementation anyway. I guess that's sound
indeed.
I don't think the same can apply to `io_read` and `io_write` though -
`IoCapable` is public, and so anyone could call them with an arbitrary
argument, e.g.
let mmio: &Mmio<0x100>;
...
IoCapable::<u8>::io_read(mmio, usize::MAX);
Here we have no way of making the callers enforce the internal safety
requirement of these methods, thus we need to keep them unsafe.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 1/6] rust: io: turn IoCapable into a functional trait
2026-02-17 1:36 ` Alexandre Courbot
@ 2026-02-17 11:17 ` Danilo Krummrich
0 siblings, 0 replies; 25+ messages in thread
From: Danilo Krummrich @ 2026-02-17 11:17 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Gary Guo, lyude, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Bjorn Helgaas, Krzysztof Wilczyński,
driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Eliot Courtney
On Tue Feb 17, 2026 at 2:36 AM CET, Alexandre Courbot wrote:
> Ah, I think I finally get it now. `Io` and `IoKnownSize` can piggyback
> on the `IoCapable` safety requirement, because all the unsafe code
> requires an `IoCapable` implementation anyway. I guess that's sound
> indeed.
>
> I don't think the same can apply to `io_read` and `io_write` though -
> `IoCapable` is public, and so anyone could call them with an arbitrary
> argument, e.g.
>
> let mmio: &Mmio<0x100>;
> ...
> IoCapable::<u8>::io_read(mmio, usize::MAX);
>
> Here we have no way of making the callers enforce the internal safety
> requirement of these methods, thus we need to keep them unsafe.
Agreed.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait
2026-02-06 6:00 [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait Alexandre Courbot
` (8 preceding siblings ...)
2026-02-06 19:48 ` lyude
@ 2026-03-15 0:56 ` Danilo Krummrich
9 siblings, 0 replies; 25+ messages in thread
From: Danilo Krummrich @ 2026-03-15 0:56 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Alice Ryhl, Daniel Almeida, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Bjorn Helgaas, Krzysztof Wilczyński,
driver-core, rust-for-linux, linux-kernel, linux-pci, Zhi Wang,
Lyude Paul, Eliot Courtney
On Fri Feb 6, 2026 at 7:00 AM CET, Alexandre Courbot wrote:
I've now picked this up in the topic/io topic branch [1]; it will soon be merged
into driver-core-next and drm-rust-next, thanks!
> Alexandre Courbot (6):
> rust: io: turn IoCapable into a functional trait
> rust: io: mem: use non-relaxed I/O ops in examples
> rust: io: provide Mmio relaxed ops through a wrapper type
[ Use kernel import style in examples. - Danilo ]
> rust: io: remove legacy relaxed accessors of Mmio
> rust: pci: io: remove overloaded Io methods of ConfigSpace
> rust: io: remove overloaded Io methods of Mmio
[1] https://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git/log/?h=topic/io
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2026-03-15 0:57 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-06 6:00 [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait Alexandre Courbot
2026-02-06 6:00 ` [PATCH v2 1/6] " Alexandre Courbot
2026-02-06 20:29 ` lyude
2026-02-12 12:04 ` Alexandre Courbot
2026-02-12 12:23 ` Danilo Krummrich
2026-02-12 14:11 ` Gary Guo
2026-02-12 14:52 ` Danilo Krummrich
2026-02-16 11:51 ` Alexandre Courbot
2026-02-16 12:08 ` Danilo Krummrich
2026-02-16 13:27 ` Alexandre Courbot
2026-02-16 17:04 ` Danilo Krummrich
2026-02-17 1:36 ` Alexandre Courbot
2026-02-17 11:17 ` Danilo Krummrich
2026-02-06 6:00 ` [PATCH v2 2/6] rust: io: mem: use non-relaxed I/O ops in examples Alexandre Courbot
2026-02-06 6:00 ` [PATCH v2 3/6] rust: io: provide Mmio relaxed ops through a wrapper type Alexandre Courbot
2026-02-06 16:09 ` Daniel Almeida
2026-02-06 6:00 ` [PATCH v2 4/6] rust: io: remove legacy relaxed accessors of Mmio Alexandre Courbot
2026-02-06 6:00 ` [PATCH v2 5/6] rust: pci: io: remove overloaded Io methods of ConfigSpace Alexandre Courbot
2026-02-06 6:00 ` [PATCH v2 6/6] rust: io: remove overloaded Io methods of Mmio Alexandre Courbot
2026-02-06 15:02 ` [PATCH v2 0/6] rust: io: turn IoCapable into a functional trait Gary Guo
2026-02-06 19:12 ` Danilo Krummrich
2026-02-06 19:48 ` lyude
2026-02-12 12:28 ` Alexandre Courbot
2026-02-12 14:40 ` Daniel Almeida
2026-03-15 0:56 ` Danilo Krummrich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox