The previous Io<SIZE> type combined both the generic I/O access helpers
and MMIO implementation details in a single struct.
To establish a cleaner layering between the I/O interface and its concrete
backends, paving the way for supporting additional I/O mechanisms in the
future, Io<SIZE> need to be factored.
Factor the common helpers into a new Io trait, and move the MMIO-specific
logic into a dedicated Mmio<SIZE> type implementing that trait. Rename the
IoRaw to MmioRaw and update the bus MMIO implementations to use MmioRaw.
No functional change intended.
Cc: Alexandre Courbot <acourbot@nvidia.com>
Cc: Bjorn Helgaas <helgaas@kernel.org>
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Zhi Wang <zhiw@nvidia.com>
---
drivers/gpu/nova-core/regs/macros.rs | 90 +++++----
drivers/gpu/nova-core/vbios.rs | 1 +
rust/kernel/devres.rs | 14 +-
rust/kernel/io.rs | 264 ++++++++++++++++++++-------
rust/kernel/io/mem.rs | 16 +-
rust/kernel/io/poll.rs | 8 +-
rust/kernel/pci/io.rs | 12 +-
samples/rust/rust_driver_pci.rs | 2 +
8 files changed, 277 insertions(+), 130 deletions(-)
diff --git a/drivers/gpu/nova-core/regs/macros.rs b/drivers/gpu/nova-core/regs/macros.rs
index 8058e1696df9..39b1069a3429 100644
--- a/drivers/gpu/nova-core/regs/macros.rs
+++ b/drivers/gpu/nova-core/regs/macros.rs
@@ -608,16 +608,18 @@ impl $name {
/// Read the register from its address in `io`.
#[inline(always)]
- pub(crate) fn read<const SIZE: usize, T>(io: &T) -> Self where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ pub(crate) fn read<T, I>(io: &T) -> Self where
+ T: ::core::ops::Deref<Target = I>,
+ I: ::kernel::io::IoInfallible,
{
Self(io.read32($offset))
}
/// Write the value contained in `self` to the register address in `io`.
#[inline(always)]
- pub(crate) fn write<const SIZE: usize, T>(self, io: &T) where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ pub(crate) fn write<T, I>(self, io: &T) where
+ T: ::core::ops::Deref<Target = I>,
+ I: ::kernel::io::IoInfallible,
{
io.write32(self.0, $offset)
}
@@ -625,11 +627,12 @@ pub(crate) fn write<const SIZE: usize, T>(self, io: &T) where
/// Read the register from its address in `io` and run `f` on its value to obtain a new
/// value to write back.
#[inline(always)]
- pub(crate) fn alter<const SIZE: usize, T, F>(
+ pub(crate) fn alter<T, I, F>(
io: &T,
f: F,
) where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = I>,
+ I: ::kernel::io::IoInfallible,
F: ::core::ops::FnOnce(Self) -> Self,
{
let reg = f(Self::read(io));
@@ -647,12 +650,13 @@ impl $name {
/// Read the register from `io`, using the base address provided by `base` and adding
/// the register's offset to it.
#[inline(always)]
- pub(crate) fn read<const SIZE: usize, T, B>(
+ pub(crate) fn read<T, I, B>(
io: &T,
#[allow(unused_variables)]
base: &B,
) -> Self where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = I>,
+ I: ::kernel::io::IoInfallible,
B: crate::regs::macros::RegisterBase<$base>,
{
const OFFSET: usize = $name::OFFSET;
@@ -667,13 +671,14 @@ pub(crate) fn read<const SIZE: usize, T, B>(
/// Write the value contained in `self` to `io`, using the base address provided by
/// `base` and adding the register's offset to it.
#[inline(always)]
- pub(crate) fn write<const SIZE: usize, T, B>(
+ pub(crate) fn write<T, I, B>(
self,
io: &T,
#[allow(unused_variables)]
base: &B,
) where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = I>,
+ I: ::kernel::io::IoInfallible,
B: crate::regs::macros::RegisterBase<$base>,
{
const OFFSET: usize = $name::OFFSET;
@@ -688,12 +693,13 @@ pub(crate) fn write<const SIZE: usize, T, B>(
/// the register's offset to it, then run `f` on its value to obtain a new value to
/// write back.
#[inline(always)]
- pub(crate) fn alter<const SIZE: usize, T, B, F>(
+ pub(crate) fn alter<T, I, B, F>(
io: &T,
base: &B,
f: F,
) where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = I>,
+ I: ::kernel::io::IoInfallible,
B: crate::regs::macros::RegisterBase<$base>,
F: ::core::ops::FnOnce(Self) -> Self,
{
@@ -713,11 +719,12 @@ impl $name {
/// Read the array register at index `idx` from its address in `io`.
#[inline(always)]
- pub(crate) fn read<const SIZE: usize, T>(
+ pub(crate) fn read<T, I>(
io: &T,
idx: usize,
) -> Self where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = I>,
+ I: ::kernel::io::IoInfallible,
{
build_assert!(idx < Self::SIZE);
@@ -729,12 +736,13 @@ pub(crate) fn read<const SIZE: usize, T>(
/// Write the value contained in `self` to the array register with index `idx` in `io`.
#[inline(always)]
- pub(crate) fn write<const SIZE: usize, T>(
+ pub(crate) fn write<T, I>(
self,
io: &T,
idx: usize
) where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = I>,
+ I: ::kernel::io::IoInfallible,
{
build_assert!(idx < Self::SIZE);
@@ -746,12 +754,13 @@ pub(crate) fn write<const SIZE: usize, T>(
/// Read the array register at index `idx` in `io` and run `f` on its value to obtain a
/// new value to write back.
#[inline(always)]
- pub(crate) fn alter<const SIZE: usize, T, F>(
+ pub(crate) fn alter<T, I, F>(
io: &T,
idx: usize,
f: F,
) where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = I>,
+ I: ::kernel::io::IoInfallible,
F: ::core::ops::FnOnce(Self) -> Self,
{
let reg = f(Self::read(io, idx));
@@ -763,11 +772,12 @@ pub(crate) fn alter<const SIZE: usize, T, F>(
/// The validity of `idx` is checked at run-time, and `EINVAL` is returned is the
/// access was out-of-bounds.
#[inline(always)]
- pub(crate) fn try_read<const SIZE: usize, T>(
+ pub(crate) fn try_read<T, I>(
io: &T,
idx: usize,
) -> ::kernel::error::Result<Self> where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = I>,
+ I: ::kernel::io::IoInfallible,
{
if idx < Self::SIZE {
Ok(Self::read(io, idx))
@@ -781,12 +791,13 @@ pub(crate) fn try_read<const SIZE: usize, T>(
/// The validity of `idx` is checked at run-time, and `EINVAL` is returned is the
/// access was out-of-bounds.
#[inline(always)]
- pub(crate) fn try_write<const SIZE: usize, T>(
+ pub(crate) fn try_write<T, I>(
self,
io: &T,
idx: usize,
) -> ::kernel::error::Result where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = I>,
+ I: ::kernel::io::IoInfallible,
{
if idx < Self::SIZE {
Ok(self.write(io, idx))
@@ -801,12 +812,13 @@ pub(crate) fn try_write<const SIZE: usize, T>(
/// The validity of `idx` is checked at run-time, and `EINVAL` is returned is the
/// access was out-of-bounds.
#[inline(always)]
- pub(crate) fn try_alter<const SIZE: usize, T, F>(
+ pub(crate) fn try_alter<T, I, F>(
io: &T,
idx: usize,
f: F,
) -> ::kernel::error::Result where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = I>,
+ I: ::kernel::io::IoInfallible,
F: ::core::ops::FnOnce(Self) -> Self,
{
if idx < Self::SIZE {
@@ -832,13 +844,14 @@ impl $name {
/// Read the array register at index `idx` from `io`, using the base address provided
/// by `base` and adding the register's offset to it.
#[inline(always)]
- pub(crate) fn read<const SIZE: usize, T, B>(
+ pub(crate) fn read<T, I, B>(
io: &T,
#[allow(unused_variables)]
base: &B,
idx: usize,
) -> Self where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = I>,
+ I: ::kernel::io::IoInfallible,
B: crate::regs::macros::RegisterBase<$base>,
{
build_assert!(idx < Self::SIZE);
@@ -853,14 +866,15 @@ pub(crate) fn read<const SIZE: usize, T, B>(
/// Write the value contained in `self` to `io`, using the base address provided by
/// `base` and adding the offset of array register `idx` to it.
#[inline(always)]
- pub(crate) fn write<const SIZE: usize, T, B>(
+ pub(crate) fn write<T, I, B>(
self,
io: &T,
#[allow(unused_variables)]
base: &B,
idx: usize
) where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = I>,
+ I: ::kernel::io::IoInfallible,
B: crate::regs::macros::RegisterBase<$base>,
{
build_assert!(idx < Self::SIZE);
@@ -875,13 +889,14 @@ pub(crate) fn write<const SIZE: usize, T, B>(
/// by `base` and adding the register's offset to it, then run `f` on its value to
/// obtain a new value to write back.
#[inline(always)]
- pub(crate) fn alter<const SIZE: usize, T, B, F>(
+ pub(crate) fn alter<T, I, B, F>(
io: &T,
base: &B,
idx: usize,
f: F,
) where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = I>,
+ I: ::kernel::io::IoInfallible,
B: crate::regs::macros::RegisterBase<$base>,
F: ::core::ops::FnOnce(Self) -> Self,
{
@@ -895,12 +910,13 @@ pub(crate) fn alter<const SIZE: usize, T, B, F>(
/// The validity of `idx` is checked at run-time, and `EINVAL` is returned is the
/// access was out-of-bounds.
#[inline(always)]
- pub(crate) fn try_read<const SIZE: usize, T, B>(
+ pub(crate) fn try_read<T, I, B>(
io: &T,
base: &B,
idx: usize,
) -> ::kernel::error::Result<Self> where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = I>,
+ I: ::kernel::io::IoInfallible,
B: crate::regs::macros::RegisterBase<$base>,
{
if idx < Self::SIZE {
@@ -916,13 +932,14 @@ pub(crate) fn try_read<const SIZE: usize, T, B>(
/// The validity of `idx` is checked at run-time, and `EINVAL` is returned is the
/// access was out-of-bounds.
#[inline(always)]
- pub(crate) fn try_write<const SIZE: usize, T, B>(
+ pub(crate) fn try_write<T, I, B>(
self,
io: &T,
base: &B,
idx: usize,
) -> ::kernel::error::Result where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = I>,
+ I: ::kernel::io::IoInfallible,
B: crate::regs::macros::RegisterBase<$base>,
{
if idx < Self::SIZE {
@@ -939,13 +956,14 @@ pub(crate) fn try_write<const SIZE: usize, T, B>(
/// The validity of `idx` is checked at run-time, and `EINVAL` is returned is the
/// access was out-of-bounds.
#[inline(always)]
- pub(crate) fn try_alter<const SIZE: usize, T, B, F>(
+ pub(crate) fn try_alter<T, I, B, F>(
io: &T,
base: &B,
idx: usize,
f: F,
) -> ::kernel::error::Result where
- T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,
+ T: ::core::ops::Deref<Target = I>,
+ I: ::kernel::io::IoInfallible,
B: crate::regs::macros::RegisterBase<$base>,
F: ::core::ops::FnOnce(Self) -> Self,
{
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 71fbe71b84db..7a0121ab9b09 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -8,6 +8,7 @@
use core::convert::TryFrom;
use kernel::device;
use kernel::error::Result;
+use kernel::io::IoFallible;
use kernel::prelude::*;
use kernel::ptr::{Alignable, Alignment};
use kernel::types::ARef;
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 3376c7090ccd..1dc7cb30d9f2 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -75,14 +75,15 @@ struct Inner<T: Send> {
/// # },
/// # devres::Devres,
/// # io::{
-/// # Io,
-/// # IoRaw, //
+/// # IoInfallible,
+/// # Mmio,
+/// # MmioRaw, //
/// # }, //
/// # };
/// # use core::ops::Deref;
///
/// // See also [`pci::Bar`] for a real example.
-/// struct IoMem<const SIZE: usize>(IoRaw<SIZE>);
+/// struct IoMem<const SIZE: usize>(MmioRaw<SIZE>);
///
/// impl<const SIZE: usize> IoMem<SIZE> {
/// /// # Safety
@@ -97,7 +98,7 @@ struct Inner<T: Send> {
/// return Err(ENOMEM);
/// }
///
-/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
+/// Ok(IoMem(MmioRaw::new(addr as usize, SIZE)?))
/// }
/// }
///
@@ -109,11 +110,11 @@ struct Inner<T: Send> {
/// }
///
/// impl<const SIZE: usize> Deref for IoMem<SIZE> {
-/// type Target = Io<SIZE>;
+/// type Target = Mmio<SIZE>;
///
/// fn deref(&self) -> &Self::Target {
/// // SAFETY: The memory range stored in `self` has been properly mapped in `Self::new`.
-/// unsafe { Io::from_raw(&self.0) }
+/// unsafe { Mmio::from_raw(&self.0) }
/// }
/// }
/// # fn no_run(dev: &Device<Bound>) -> Result<(), Error> {
@@ -259,6 +260,7 @@ pub fn device(&self) -> &Device {
/// # use kernel::{
/// # device::Core,
/// # devres::Devres,
+ /// # io::IoInfallible,
/// # pci, //
/// # };
///
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 5af04c3b807b..4d98d431b523 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -20,16 +20,16 @@
/// By itself, the existence of an instance of this structure does not provide any guarantees that
/// the represented MMIO region does exist or is properly mapped.
///
-/// Instead, the bus specific MMIO implementation must convert this raw representation into an `Io`
-/// instance providing the actual memory accessors. Only by the conversion into an `Io` structure
-/// any guarantees are given.
-pub struct IoRaw<const SIZE: usize = 0> {
+/// Instead, the bus specific MMIO implementation must convert this raw representation into an
+/// `Mmio` instance providing the actual memory accessors. Only by the conversion into an `Mmio`
+/// structure any guarantees are given.
+pub struct MmioRaw<const SIZE: usize = 0> {
addr: usize,
maxsize: usize,
}
-impl<const SIZE: usize> IoRaw<SIZE> {
- /// Returns a new `IoRaw` instance on success, an error otherwise.
+impl<const SIZE: usize> MmioRaw<SIZE> {
+ /// Returns a new `MmioRaw` instance on success, an error otherwise.
pub fn new(addr: usize, maxsize: usize) -> Result<Self> {
if maxsize < SIZE {
return Err(EINVAL);
@@ -68,14 +68,16 @@ pub fn maxsize(&self) -> usize {
/// # bindings,
/// # ffi::c_void,
/// # io::{
-/// # Io,
-/// # IoRaw, //
+/// # IoFallible,
+/// # IoInfallible,
+/// # Mmio,
+/// # MmioRaw, //
/// # }, //
/// # };
/// # use core::ops::Deref;
///
/// // See also [`pci::Bar`] for a real example.
-/// struct IoMem<const SIZE: usize>(IoRaw<SIZE>);
+/// struct IoMem<const SIZE: usize>(MmioRaw<SIZE>);
///
/// impl<const SIZE: usize> IoMem<SIZE> {
/// /// # Safety
@@ -90,7 +92,7 @@ pub fn maxsize(&self) -> usize {
/// return Err(ENOMEM);
/// }
///
-/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
+/// Ok(IoMem(MmioRaw::new(addr as usize, SIZE)?))
/// }
/// }
///
@@ -102,11 +104,11 @@ pub fn maxsize(&self) -> usize {
/// }
///
/// impl<const SIZE: usize> Deref for IoMem<SIZE> {
-/// type Target = Io<SIZE>;
+/// type Target = Mmio<SIZE>;
///
/// fn deref(&self) -> &Self::Target {
/// // SAFETY: The memory range stored in `self` has been properly mapped in `Self::new`.
-/// unsafe { Io::from_raw(&self.0) }
+/// unsafe { Mmio::from_raw(&self.0) }
/// }
/// }
///
@@ -120,29 +122,31 @@ pub fn maxsize(&self) -> usize {
/// # }
/// ```
#[repr(transparent)]
-pub struct Io<const SIZE: usize = 0>(IoRaw<SIZE>);
+pub struct Mmio<const SIZE: usize = 0>(MmioRaw<SIZE>);
macro_rules! define_read {
- ($(#[$attr:meta])* $name:ident, $try_name:ident, $c_fn:ident -> $type_name:ty) => {
+ (infallible, $(#[$attr:meta])* $vis:vis $name:ident, $c_fn:ident -> $type_name:ty) => {
/// Read IO data from a given offset known at compile time.
///
/// Bound checks are performed on compile time, hence if the offset is not known at compile
/// time, the build will fail.
$(#[$attr])*
#[inline]
- pub fn $name(&self, offset: usize) -> $type_name {
+ $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 MMIO operations.
unsafe { bindings::$c_fn(addr as *const c_void) }
}
+ };
+ (fallible, $(#[$attr:meta])* $vis:vis $try_name: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])*
- pub fn $try_name(&self, offset: usize) -> Result<$type_name> {
+ $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 MMIO operations.
@@ -152,26 +156,28 @@ pub fn $try_name(&self, offset: usize) -> Result<$type_name> {
}
macro_rules! define_write {
- ($(#[$attr:meta])* $name:ident, $try_name:ident, $c_fn:ident <- $type_name:ty) => {
+ (infallible, $(#[$attr:meta])* $vis:vis $name:ident, $c_fn:ident <- $type_name:ty) => {
/// Write IO data from a given offset known at compile time.
///
/// Bound checks are performed on compile time, hence if the offset is not known at compile
/// time, the build will fail.
$(#[$attr])*
#[inline]
- pub fn $name(&self, value: $type_name, offset: usize) {
+ $vis fn $name(&self, value: $type_name, offset: usize) {
let addr = self.io_addr_assert::<$type_name>(offset);
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
unsafe { bindings::$c_fn(value, addr as *mut c_void) }
}
+ };
+ (fallible, $(#[$attr:meta])* $vis:vis $try_name: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])*
- pub fn $try_name(&self, value: $type_name, offset: usize) -> Result {
+ $vis fn $try_name(&self, value: $type_name, offset: usize) -> Result {
let addr = self.io_addr::<$type_name>(offset)?;
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
@@ -181,43 +187,38 @@ pub fn $try_name(&self, value: $type_name, offset: usize) -> Result {
};
}
-impl<const SIZE: usize> Io<SIZE> {
- /// Converts an `IoRaw` into an `Io` instance, providing the accessors to the MMIO mapping.
- ///
- /// # Safety
- ///
- /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of size
- /// `maxsize`.
- pub unsafe fn from_raw(raw: &IoRaw<SIZE>) -> &Self {
- // SAFETY: `Io` is a transparent wrapper around `IoRaw`.
- unsafe { &*core::ptr::from_ref(raw).cast() }
+/// Checks whether an access of type `U` at the given `offset`
+/// is valid within this region.
+#[inline]
+const fn offset_valid<U>(offset: usize, size: usize) -> bool {
+ let type_size = core::mem::size_of::<U>();
+ if let Some(end) = offset.checked_add(type_size) {
+ end <= size && offset % type_size == 0
+ } else {
+ false
}
+}
+
+/// Represents a region of I/O space of a fixed size.
+///
+/// Provides common helpers for offset validation and address
+/// calculation on top of a base address and maximum size.
+///
+pub trait Io {
+ /// Minimum usable size of this region.
+ const MIN_SIZE: usize;
/// Returns the base address of this mapping.
- #[inline]
- pub fn addr(&self) -> usize {
- self.0.addr()
- }
+ fn addr(&self) -> usize;
/// Returns the maximum size of this mapping.
- #[inline]
- pub fn maxsize(&self) -> usize {
- self.0.maxsize()
- }
-
- #[inline]
- const fn offset_valid<U>(offset: usize, size: usize) -> bool {
- let type_size = core::mem::size_of::<U>();
- if let Some(end) = offset.checked_add(type_size) {
- end <= size && offset % type_size == 0
- } else {
- false
- }
- }
+ fn maxsize(&self) -> usize;
+ /// Returns the absolute I/O address for a given `offset`,
+ /// performing runtime bound checks.
#[inline]
fn io_addr<U>(&self, offset: usize) -> Result<usize> {
- if !Self::offset_valid::<U>(offset, self.maxsize()) {
+ if !offset_valid::<U>(offset, self.maxsize()) {
return Err(EINVAL);
}
@@ -226,50 +227,173 @@ fn io_addr<U>(&self, offset: usize) -> Result<usize> {
self.addr().checked_add(offset).ok_or(EINVAL)
}
+ /// Returns the absolute I/O address for a given `offset`,
+ /// performing compile-time bound checks.
#[inline]
fn io_addr_assert<U>(&self, offset: usize) -> usize {
- build_assert!(Self::offset_valid::<U>(offset, SIZE));
+ build_assert!(offset_valid::<U>(offset, Self::MIN_SIZE));
self.addr() + offset
}
+}
+
+/// Types implementing this trait (e.g. MMIO BARs or PCI config
+/// regions) can share the same Infallible accessors.
+pub trait IoInfallible: Io {
+ /// Infallible 8-bit read with compile-time bounds check.
+ fn read8(&self, offset: usize) -> u8;
+
+ /// Infallible 16-bit read with compile-time bounds check.
+ fn read16(&self, offset: usize) -> u16;
+
+ /// Infallible 32-bit read with compile-time bounds check.
+ fn read32(&self, offset: usize) -> u32;
+
+ /// Infallible 8-bit write with compile-time bounds check.
+ fn write8(&self, value: u8, offset: usize);
+
+ /// Infallible 16-bit write with compile-time bounds check.
+ fn write16(&self, value: u16, offset: usize);
+
+ /// Infallible 32-bit write with compile-time bounds check.
+ fn write32(&self, value: u32, offset: usize);
+}
+
+/// Types implementing this trait (e.g. MMIO BARs or PCI config
+/// regions) can share the same Fallible accessors.
+pub trait IoFallible: Io {
+ /// Fallible 8-bit read with runtime bounds check.
+ fn try_read8(&self, offset: usize) -> Result<u8>;
+
+ /// Fallible 16-bit read with runtime bounds check.
+ fn try_read16(&self, offset: usize) -> Result<u16>;
+
+ /// Fallible 32-bit read with runtime bounds check.
+ fn try_read32(&self, offset: usize) -> Result<u32>;
+
+ /// Fallible 8-bit write with runtime bounds check.
+ fn try_write8(&self, value: u8, offset: usize) -> Result;
+
+ /// Fallible 16-bit write with runtime bounds check.
+ fn try_write16(&self, value: u16, offset: usize) -> Result;
+
+ /// Fallible 32-bit write with runtime bounds check.
+ fn try_write32(&self, value: u32, offset: usize) -> Result;
+}
+
+impl<const SIZE: usize> Io for Mmio<SIZE> {
+ const MIN_SIZE: usize = 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()
+ }
+}
+
+impl<const SIZE: usize> IoInfallible for Mmio<SIZE> {
+ define_read!(infallible, read8, readb -> u8);
+ define_read!(infallible, read16, readw -> u16);
+ define_read!(infallible, read32, readl -> u32);
+
+ define_write!(infallible, write8, writeb <- u8);
+ define_write!(infallible, write16, writew <- u16);
+ define_write!(infallible, write32, writel <- u32);
+}
+
+impl<const SIZE: usize> IoFallible for Mmio<SIZE> {
+ define_read!(fallible, try_read8, readb -> u8);
+ define_read!(fallible, try_read16, readw -> u16);
+ define_read!(fallible, try_read32, readl -> u32);
+
+ define_write!(fallible, try_write8, writeb <- u8);
+ define_write!(fallible, try_write16, writew <- u16);
+ define_write!(fallible, try_write32, writel <- u32);
+}
+
+impl<const SIZE: usize> Mmio<SIZE> {
+ /// Converts an `MmioRaw` into an `Mmio` instance, providing the accessors to the MMIO mapping.
+ ///
+ /// # Safety
+ ///
+ /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of size
+ /// `maxsize`.
+ pub unsafe fn from_raw(raw: &MmioRaw<SIZE>) -> &Self {
+ // SAFETY: `Mmio` is a transparent wrapper around `MmioRaw`.
+ unsafe { &*core::ptr::from_ref(raw).cast() }
+ }
- define_read!(read8, try_read8, readb -> u8);
- define_read!(read16, try_read16, readw -> u16);
- define_read!(read32, try_read32, readl -> u32);
define_read!(
+ infallible,
#[cfg(CONFIG_64BIT)]
- read64,
- try_read64,
+ pub read64,
readq -> u64
);
- define_read!(read8_relaxed, try_read8_relaxed, readb_relaxed -> u8);
- define_read!(read16_relaxed, try_read16_relaxed, readw_relaxed -> u16);
- define_read!(read32_relaxed, try_read32_relaxed, readl_relaxed -> u32);
+ define_write!(
+ infallible,
+ #[cfg(CONFIG_64BIT)]
+ pub write64,
+ writeq <- u64
+ );
+
define_read!(
+ fallible,
#[cfg(CONFIG_64BIT)]
- read64_relaxed,
- try_read64_relaxed,
- readq_relaxed -> u64
+ pub try_read64,
+ readq -> u64
);
- define_write!(write8, try_write8, writeb <- u8);
- define_write!(write16, try_write16, writew <- u16);
- define_write!(write32, try_write32, writel <- u32);
define_write!(
+ fallible,
#[cfg(CONFIG_64BIT)]
- write64,
- try_write64,
+ pub try_write64,
writeq <- u64
);
- define_write!(write8_relaxed, try_write8_relaxed, writeb_relaxed <- u8);
- define_write!(write16_relaxed, try_write16_relaxed, writew_relaxed <- u16);
- define_write!(write32_relaxed, try_write32_relaxed, writel_relaxed <- u32);
+ define_read!(infallible, pub read8_relaxed, readb_relaxed -> u8);
+ define_read!(infallible, pub read16_relaxed, readw_relaxed -> u16);
+ define_read!(infallible, pub read32_relaxed, readl_relaxed -> u32);
+ define_read!(
+ infallible,
+ #[cfg(CONFIG_64BIT)]
+ pub read64_relaxed,
+ readq_relaxed -> u64
+ );
+
+ define_read!(fallible, pub try_read8_relaxed, readb_relaxed -> u8);
+ define_read!(fallible, pub try_read16_relaxed, readw_relaxed -> u16);
+ define_read!(fallible, pub try_read32_relaxed, readl_relaxed -> u32);
+ define_read!(
+ fallible,
+ #[cfg(CONFIG_64BIT)]
+ pub try_read64_relaxed,
+ readq_relaxed -> u64
+ );
+
+ define_write!(infallible, pub write8_relaxed, writeb_relaxed <- u8);
+ define_write!(infallible, pub write16_relaxed, writew_relaxed <- u16);
+ define_write!(infallible, pub write32_relaxed, writel_relaxed <- u32);
+ define_write!(
+ infallible,
+ #[cfg(CONFIG_64BIT)]
+ pub write64_relaxed,
+ writeq_relaxed <- u64
+ );
+
+ define_write!(fallible, pub try_write8_relaxed, writeb_relaxed <- u8);
+ define_write!(fallible, pub try_write16_relaxed, writew_relaxed <- u16);
+ define_write!(fallible, pub try_write32_relaxed, writel_relaxed <- u32);
define_write!(
+ fallible,
#[cfg(CONFIG_64BIT)]
- write64_relaxed,
- try_write64_relaxed,
+ pub try_write64_relaxed,
writeq_relaxed <- u64
);
}
diff --git a/rust/kernel/io/mem.rs b/rust/kernel/io/mem.rs
index b03b82cd531b..5dcd7c901427 100644
--- a/rust/kernel/io/mem.rs
+++ b/rust/kernel/io/mem.rs
@@ -17,8 +17,8 @@
Region,
Resource, //
},
- Io,
- IoRaw, //
+ Mmio,
+ MmioRaw, //
},
prelude::*,
};
@@ -203,7 +203,7 @@ pub fn new<'a>(io_request: IoRequest<'a>) -> impl PinInit<Devres<Self>, Error> +
}
impl<const SIZE: usize> Deref for ExclusiveIoMem<SIZE> {
- type Target = Io<SIZE>;
+ type Target = Mmio<SIZE>;
fn deref(&self) -> &Self::Target {
&self.iomem
@@ -217,10 +217,10 @@ fn deref(&self) -> &Self::Target {
///
/// # Invariants
///
-/// [`IoMem`] always holds an [`IoRaw`] instance that holds a valid pointer to the
+/// [`IoMem`] always holds an [`MmioRaw`] instance that holds a valid pointer to the
/// start of the I/O memory mapped region.
pub struct IoMem<const SIZE: usize = 0> {
- io: IoRaw<SIZE>,
+ io: MmioRaw<SIZE>,
}
impl<const SIZE: usize> IoMem<SIZE> {
@@ -255,7 +255,7 @@ fn ioremap(resource: &Resource) -> Result<Self> {
return Err(ENOMEM);
}
- let io = IoRaw::new(addr as usize, size)?;
+ let io = MmioRaw::new(addr as usize, size)?;
let io = IoMem { io };
Ok(io)
@@ -278,10 +278,10 @@ fn drop(&mut self) {
}
impl<const SIZE: usize> Deref for IoMem<SIZE> {
- type Target = Io<SIZE>;
+ type Target = Mmio<SIZE>;
fn deref(&self) -> &Self::Target {
// SAFETY: Safe as by the invariant of `IoMem`.
- unsafe { Io::from_raw(&self.io) }
+ unsafe { Mmio::from_raw(&self.io) }
}
}
diff --git a/rust/kernel/io/poll.rs b/rust/kernel/io/poll.rs
index b1a2570364f4..543a4b7cea0d 100644
--- a/rust/kernel/io/poll.rs
+++ b/rust/kernel/io/poll.rs
@@ -45,12 +45,12 @@
/// # Examples
///
/// ```no_run
-/// use kernel::io::{Io, poll::read_poll_timeout};
+/// use kernel::io::{IoFallible, Mmio, poll::read_poll_timeout};
/// use kernel::time::Delta;
///
/// const HW_READY: u16 = 0x01;
///
-/// fn wait_for_hardware<const SIZE: usize>(io: &Io<SIZE>) -> Result {
+/// fn wait_for_hardware<const SIZE: usize>(io: &Mmio<SIZE>) -> Result {
/// read_poll_timeout(
/// // The `op` closure reads the value of a specific status register.
/// || io.try_read16(0x1000),
@@ -128,12 +128,12 @@ pub fn read_poll_timeout<Op, Cond, T>(
/// # Examples
///
/// ```no_run
-/// use kernel::io::{poll::read_poll_timeout_atomic, Io};
+/// use kernel::io::{poll::read_poll_timeout_atomic, IoFallible, Mmio};
/// use kernel::time::Delta;
///
/// const HW_READY: u16 = 0x01;
///
-/// fn wait_for_hardware<const SIZE: usize>(io: &Io<SIZE>) -> Result {
+/// fn wait_for_hardware<const SIZE: usize>(io: &Mmio<SIZE>) -> Result {
/// read_poll_timeout_atomic(
/// // The `op` closure reads the value of a specific status register.
/// || io.try_read16(0x1000),
diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs
index 0d55c3139b6f..2bbb3261198d 100644
--- a/rust/kernel/pci/io.rs
+++ b/rust/kernel/pci/io.rs
@@ -8,8 +8,8 @@
device,
devres::Devres,
io::{
- Io,
- IoRaw, //
+ Mmio,
+ MmioRaw, //
},
prelude::*,
sync::aref::ARef, //
@@ -24,7 +24,7 @@
/// memory mapped PCI BAR and its size.
pub struct Bar<const SIZE: usize = 0> {
pdev: ARef<Device>,
- io: IoRaw<SIZE>,
+ io: MmioRaw<SIZE>,
num: i32,
}
@@ -60,7 +60,7 @@ pub(super) fn new(pdev: &Device, num: u32, name: &CStr) -> Result<Self> {
return Err(ENOMEM);
}
- let io = match IoRaw::new(ioptr, len as usize) {
+ let io = match MmioRaw::new(ioptr, len as usize) {
Ok(io) => io,
Err(err) => {
// SAFETY:
@@ -114,11 +114,11 @@ fn drop(&mut self) {
}
impl<const SIZE: usize> Deref for Bar<SIZE> {
- type Target = Io<SIZE>;
+ type Target = Mmio<SIZE>;
fn deref(&self) -> &Self::Target {
// SAFETY: By the type invariant of `Self`, the MMIO range in `self.io` is properly mapped.
- unsafe { Io::from_raw(&self.io) }
+ unsafe { Mmio::from_raw(&self.io) }
}
}
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index ee6248b8cda5..74b93ca7c338 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -8,6 +8,8 @@
c_str,
device::Core,
devres::Devres,
+ io::IoFallible,
+ io::IoInfallible,
pci,
prelude::*,
sync::aref::ARef, //
--
2.51.0On Mon, Nov 10, 2025 at 10:41:16PM +0200, Zhi Wang wrote: > The previous Io<SIZE> type combined both the generic I/O access helpers > and MMIO implementation details in a single struct. > > To establish a cleaner layering between the I/O interface and its concrete > backends, paving the way for supporting additional I/O mechanisms in the > future, Io<SIZE> need to be factored. > > Factor the common helpers into a new Io trait, and move the MMIO-specific > logic into a dedicated Mmio<SIZE> type implementing that trait. Rename the > IoRaw to MmioRaw and update the bus MMIO implementations to use MmioRaw. > > No functional change intended. > > Cc: Alexandre Courbot <acourbot@nvidia.com> > Cc: Bjorn Helgaas <helgaas@kernel.org> > Cc: Danilo Krummrich <dakr@kernel.org> > Cc: John Hubbard <jhubbard@nvidia.com> > Signed-off-by: Zhi Wang <zhiw@nvidia.com> This defines three traits: * Io * IoInfallible: Io * IoFallible: Io This particular split says that there are going to be cases where we implement IoInfallible only, cases where we implement IoFallible only, and maybe cases where we implement both. And the distiction between them is whether the bounds check is runtime or compile-time. But this doesn't make much sense to me. Surely any Io resource that can provide compile-time checked io can also provide runtime-checked io, so maybe IoFallible should extend IoInfallible? And why are these separate traits at all? Why not support both compile-time and runtime-checked IO always? I noticed also that the trait does not have methods for 64-bit writes, and that these are left as inherent methods on Mmio. The traits that would make sense to me are these: * Io * Io64: Io where Io provides everything the three traits you have now provides, and Io64 provides the 64-bit operations. That way, everything needs to support operations of various sizes with both compile-time and runtime-checked bounds, but types may opt-in to providing 64-bit ops. Thoughts? Alice
On 11/14/25 4:58 AM, Alice Ryhl wrote: > On Mon, Nov 10, 2025 at 10:41:16PM +0200, Zhi Wang wrote: ... > This defines three traits: > > * Io > * IoInfallible: Io > * IoFallible: Io > > This particular split says that there are going to be cases where we > implement IoInfallible only, cases where we implement IoFallible only, > and maybe cases where we implement both. > > And the distiction between them is whether the bounds check is runtime > or compile-time. > > But this doesn't make much sense to me. Surely any Io resource that can > provide compile-time checked io can also provide runtime-checked io, so > maybe IoFallible should extend IoInfallible? IO is generally something that can fail, so this whole idea of infallible IO is making me uneasy. I understand that we're trying to wrap it up into a bound device, but bound devices are all about whether or not the driver lifetime is OK, not so much about IO. For PCIe, it is still possible for the device to fall off of the bus, and in that case you'll usually see 0xFFFF_FFFF returned from PCIe reads. The Open RM driver has sprinkled around checks for this value (not fun, I know), and Danilo hinted elsewhere that bound-ness requires not getting these, so maybe that suffices. But it means that Rust will be "interesting" here, because falling off the bus means that there will be a time window in which the IO is, in fact, fallible. Other IO subsystems can also get IO errors, too. I wonder if we should just provide IoFallible? (It could check for the 0xFFFF_FFFF case, for example, which is helpful to simplify the caller.) Again, it feels *really* odd to claim infallibility on something that, almost (but not quite) by it's very nature is going to generate errors at times. > > And why are these separate traits at all? Why not support both > compile-time and runtime-checked IO always? > > I noticed also that the trait does not have methods for 64-bit writes, > and that these are left as inherent methods on Mmio. > > The traits that would make sense to me are these: > > * Io > * Io64: Io > > where Io provides everything the three traits you have now provides, and > Io64 provides the 64-bit operations. That way, everything needs to > support operations of various sizes with both compile-time and > runtime-checked bounds, but types may opt-in to providing 64-bit ops. > > Thoughts? > > Alice thanks, -- John Hubbard
On Tue Nov 18, 2025 at 11:44 AM NZDT, John Hubbard wrote: > IO is generally something that can fail, so this whole idea of infallible > IO is making me uneasy. > > I understand that we're trying to wrap it up into a bound device, but > bound devices are all about whether or not the driver lifetime is OK, > not so much about IO. That is correct, device context states are about driver lifetime. However, it is at least related, see below. > For PCIe, it is still possible for the device to fall off of the bus, and > in that case you'll usually see 0xFFFF_FFFF returned from PCIe reads. The > Open RM driver has sprinkled around checks for this value (not fun, I > know), and Danilo hinted elsewhere that bound-ness requires not getting > these, so maybe that suffices. But it means that Rust will be "interesting" > here, because falling off the bus means that there will be a time window in > which the IO is, in fact, fallible. The PCI configuration space accessors indeed check a flag that is set when the device falls off the bus. However, it is not sufficient, since you still have a period of time when the device fell off the bus where the flag isn't set yet and the I/O accessor may still be used concurrently. (If you look at C drivers you will note that almost none of the drivers actually check the return value of the configuration space accessors; needless to say MMIO ones don't even have the flag.) Because of that, there is not a point in making all the I/O accessors fallible, because you'd have to deal with false negatives anyways, i.e. check the read value for plausibility, because the device could already be gone, while the flag is not set yet. Additionally, when the device fell off the bus the driver core will unbind the driver, so the period where fallability would serve at least some purpose would be very short anyways. Instead, drivers have to be designed to be robust enough to deal with broken data read from the bus. > Other IO subsystems can also get IO errors, too. > > I wonder if we should just provide IoFallible? (It could check for the > 0xFFFF_FFFF case, for example, which is helpful to simplify the caller.) For some registers this could be an expected value, plus a device can fall off the bus during a read was well, leaving you with broken data. I don't think trying to make all I/O operations fallible is the way to go, it's just unreliable to detect in the generic layer. Instead, drivers should perform a plausibility check on the read values (which they have to do anyways).
On 11/18/25 1:18 PM, Danilo Krummrich wrote: > On Tue Nov 18, 2025 at 11:44 AM NZDT, John Hubbard wrote: ... >> Other IO subsystems can also get IO errors, too. >> >> I wonder if we should just provide IoFallible? (It could check for the >> 0xFFFF_FFFF case, for example, which is helpful to simplify the caller.) > > For some registers this could be an expected value, plus a device can fall off > the bus during a read was well, leaving you with broken data. > > I don't think trying to make all I/O operations fallible is the way to go, it's > just unreliable to detect in the generic layer. Instead, drivers should perform > a plausibility check on the read values (which they have to do anyways). OK, I feel more comfortable with the approach now, thanks. In return, I'll resist making jokes about calling it IoMostlyFallible. You're welcome. haha :) thanks, -- John Hubbard
On Fri, 14 Nov 2025 12:58:31 +0000 Alice Ryhl <aliceryhl@google.com> wrote: > On Mon, Nov 10, 2025 at 10:41:16PM +0200, Zhi Wang wrote: > > The previous Io<SIZE> type combined both the generic I/O access > > helpers and MMIO implementation details in a single struct. > > > > To establish a cleaner layering between the I/O interface and its > > concrete backends, paving the way for supporting additional I/O > > mechanisms in the future, Io<SIZE> need to be factored. > > > > Factor the common helpers into a new Io trait, and move the > > MMIO-specific logic into a dedicated Mmio<SIZE> type implementing > > that trait. Rename the IoRaw to MmioRaw and update the bus MMIO > > implementations to use MmioRaw. > > > > No functional change intended. > > > > Cc: Alexandre Courbot <acourbot@nvidia.com> > > Cc: Bjorn Helgaas <helgaas@kernel.org> > > Cc: Danilo Krummrich <dakr@kernel.org> > > Cc: John Hubbard <jhubbard@nvidia.com> > > Signed-off-by: Zhi Wang <zhiw@nvidia.com> > > This defines three traits: > > * Io > * IoInfallible: Io > * IoFallible: Io > > This particular split says that there are going to be cases where we > implement IoInfallible only, cases where we implement IoFallible only, > and maybe cases where we implement both. > > And the distiction between them is whether the bounds check is runtime > or compile-time. > > But this doesn't make much sense to me. Surely any Io resource that > can provide compile-time checked io can also provide runtime-checked > io, so maybe IoFallible should extend IoInfallible? > > And why are these separate traits at all? Why not support both > compile-time and runtime-checked IO always? > Hi Alice: Thanks for comments. I did have a version that PCI configuration space only have fallible accessors because I thought the device can be unplugged or a VF might fail its FLR and get unresponsive, so the driver may need to check the return all the time. And Danilo's comments were let's have the infallible accessors for PCI configuration space and add them later if some driver needs it. [1] I am open to either options. like have both or having infallibles first and fallibles later. > I noticed also that the trait does not have methods for 64-bit writes, > and that these are left as inherent methods on Mmio. > > The traits that would make sense to me are these: > > * Io > * Io64: Io > Hehe. I had the same idea here [2]: > Io trait - Main trait + 32-bit access > | > | -- Common address/bound checks > | > | (accessor traits) > | -- Io Fallible trait - (MMIO backend implements) > | -- Io Infallible trait - (MMIO/ConfigSpace backend implements this) > | > | -- Io64 trait - For backend supports 64 bit access > | (accessor traits) > | -- Io64 Faillable trait (MMIO backend implements this) > | -- Io64 Infallible trait (MMIO backend implements this) I am struggling with how many IO backends actually need 64bit read/write other than MMIO backend. E.g. SPI, I2C. The conclusion we had so far was we can add it at any time when someone need it. If we think this is necessary, I can add it in the next spin. [1] https://lore.kernel.org/all/DE00WIVOSYC2.2CAGWUYWE6FZ@kernel.org/ [2] https://lore.kernel.org/all/e7a75899-be93-4f0f-9c9f-0d63d03f4806@kernel.org/ > where Io provides everything the three traits you have now provides, > and Io64 provides the 64-bit operations. That way, everything needs to > support operations of various sizes with both compile-time and > runtime-checked bounds, but types may opt-in to providing 64-bit ops. > > Thoughts? > > Alice
On Sat Nov 15, 2025 at 4:27 AM AEDT, Zhi Wang wrote: > On Fri, 14 Nov 2025 12:58:31 +0000 > Alice Ryhl <aliceryhl@google.com> wrote: >> This defines three traits: >> >> * Io >> * IoInfallible: Io >> * IoFallible: Io >> >> This particular split says that there are going to be cases where we >> implement IoInfallible only, cases where we implement IoFallible only, >> and maybe cases where we implement both. >> >> And the distiction between them is whether the bounds check is runtime >> or compile-time. >> >> But this doesn't make much sense to me. Surely any Io resource that >> can provide compile-time checked io can also provide runtime-checked >> io, so maybe IoFallible should extend IoInfallible? Yeah, though I did like that with this split we can enforce one or the other. E.g. in the case of the PCI configuration space we can always assert the expected size at compile time and drivers should not have to deal with runtime offsets either. Hence, with this split we can avoid that drivers implement unnecessary runtime checks. Either is fine with me though. >> And why are these separate traits at all? Why not support both >> compile-time and runtime-checked IO always? >> > > Hi Alice: > > Thanks for comments. I did have a version that PCI configuration space > only have fallible accessors because I thought the device can be > unplugged or a VF might fail its FLR and get unresponsive, so the driver > may need to check the return all the time. And Danilo's comments were > let's have the infallible accessors for PCI configuration space and add > them later if some driver needs it. [1] Yeah, that's the same with MMIO accesses as well, yet we don't check all the time. Actually, when a device falls from the bus, the error state is not available immediately either and hence drivers have to be robust enough to deal with it anyway. Adding in that a driver is also unbound as soon as the device falling from the bus is detected, there is not much value in those checks.
On Fri, Nov 14, 2025 at 12:37 PM Zhi Wang <zhiw@nvidia.com> wrote: > > On Fri, 14 Nov 2025 12:58:31 +0000 > Alice Ryhl <aliceryhl@google.com> wrote: > > > On Mon, Nov 10, 2025 at 10:41:16PM +0200, Zhi Wang wrote: > > > The previous Io<SIZE> type combined both the generic I/O access > > > helpers and MMIO implementation details in a single struct. > > > > > > To establish a cleaner layering between the I/O interface and its > > > concrete backends, paving the way for supporting additional I/O > > > mechanisms in the future, Io<SIZE> need to be factored. > > > > > > Factor the common helpers into a new Io trait, and move the > > > MMIO-specific logic into a dedicated Mmio<SIZE> type implementing > > > that trait. Rename the IoRaw to MmioRaw and update the bus MMIO > > > implementations to use MmioRaw. > > > > > > No functional change intended. > > > > > > Cc: Alexandre Courbot <acourbot@nvidia.com> > > > Cc: Bjorn Helgaas <helgaas@kernel.org> > > > Cc: Danilo Krummrich <dakr@kernel.org> > > > Cc: John Hubbard <jhubbard@nvidia.com> > > > Signed-off-by: Zhi Wang <zhiw@nvidia.com> > > > > This defines three traits: > > > > * Io > > * IoInfallible: Io > > * IoFallible: Io > > > > This particular split says that there are going to be cases where we > > implement IoInfallible only, cases where we implement IoFallible only, > > and maybe cases where we implement both. > > > > And the distiction between them is whether the bounds check is runtime > > or compile-time. > > > > But this doesn't make much sense to me. Surely any Io resource that > > can provide compile-time checked io can also provide runtime-checked > > io, so maybe IoFallible should extend IoInfallible? > > > > And why are these separate traits at all? Why not support both > > compile-time and runtime-checked IO always? > > > > Hi Alice: > > Thanks for comments. I did have a version that PCI configuration space > only have fallible accessors because I thought the device can be > unplugged or a VF might fail its FLR and get unresponsive, so the driver > may need to check the return all the time. And Danilo's comments were > let's have the infallible accessors for PCI configuration space and add > them later if some driver needs it. [1] > > I am open to either options. like have both or having infallibles first > and fallibles later. What about using an associated Err type? In the infallible case, it would be `core::convert::Infallible`. It would be slightly more ergonomic if associated type defaults were stable[0], though. [0] https://github.com/rust-lang/rust/issues/29661
On Fri, 14 Nov 2025 13:53:01 -0500 Tamir Duberstein <tamird@gmail.com> wrote: > On Fri, Nov 14, 2025 at 12:37 PM Zhi Wang <zhiw@nvidia.com> wrote: > > snip > > What about using an associated Err type? In the infallible case, it > would be `core::convert::Infallible`. It would be slightly more > ergonomic if associated type defaults were stable[0], though. > > [0] https://github.com/rust-lang/rust/issues/29661 Thanks, Tamir. From Alice's and Danilo’s discussion, it seems we’ll keep the fallible and infallible traits separate for now. Still, I really like the idea of an associated Err type. Thanks for the trick. :) Z.
Hi Zhi, On Tue Nov 11, 2025 at 5:41 AM JST, Zhi Wang wrote: > The previous Io<SIZE> type combined both the generic I/O access helpers > and MMIO implementation details in a single struct. > > To establish a cleaner layering between the I/O interface and its concrete > backends, paving the way for supporting additional I/O mechanisms in the > future, Io<SIZE> need to be factored. > > Factor the common helpers into a new Io trait, and move the MMIO-specific > logic into a dedicated Mmio<SIZE> type implementing that trait. Rename the > IoRaw to MmioRaw and update the bus MMIO implementations to use MmioRaw. > > No functional change intended. > > Cc: Alexandre Courbot <acourbot@nvidia.com> > Cc: Bjorn Helgaas <helgaas@kernel.org> > Cc: Danilo Krummrich <dakr@kernel.org> > Cc: John Hubbard <jhubbard@nvidia.com> > Signed-off-by: Zhi Wang <zhiw@nvidia.com> This is looking quite good to me! I tried really hard to find stuff to comment about, but couldn't. :) Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
© 2016 - 2026 Red Hat, Inc.