Drivers might need to access PCI config space for querying capability
structures and access the registers inside the structures.

For Rust drivers need to access PCI config space, the Rust PCI abstraction
needs to support it in a way that upholds Rust's safety principles.

Introduce a `ConfigSpace` wrapper in Rust PCI abstraction to provide safe
accessors for PCI config space. The new type implements the `Io` trait and
`IoCapable<T>` for u8, u16, and u32 to share offset validation and
bound-checking logic with other I/O backends.

The `ConfigSpace` type uses marker types (`Normal` and `Extended`) to
represent configuration space sizes at the type level.

Cc: Alexandre Courbot <acourbot@nvidia.com>
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Gary Guo <gary@garyguo.net>
Cc: Joel Fernandes <joelagnelf@nvidia.com>
Signed-off-by: Zhi Wang <zhiw@nvidia.com>
---
 rust/kernel/pci.rs    |   7 +-
 rust/kernel/pci/io.rs | 167 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 172 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 82e128431f08..9020959ce0c7 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -40,7 +40,12 @@
     ClassMask,
     Vendor, //
 };
-pub use self::io::Bar;
+pub use self::io::{
+    Bar,
+    ConfigSpaceSize,
+    Extended,
+    Normal, //
+};
 pub use self::irq::{
     IrqType,
     IrqTypes,
diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs
index e3377397666e..39df41d0eaab 100644
--- a/rust/kernel/pci/io.rs
+++ b/rust/kernel/pci/io.rs
@@ -8,13 +8,149 @@
     device,
     devres::Devres,
     io::{
+        define_read,
+        define_write,
+        Io,
+        IoCapable,
+        IoKnownSize,
         Mmio,
         MmioRaw, //
     },
     prelude::*,
     sync::aref::ARef, //
 };
-use core::ops::Deref;
+use core::{
+    marker::PhantomData,
+    ops::Deref, //
+};
+
+/// Marker type for normal (256-byte) PCI configuration space.
+pub struct Normal;
+
+/// Marker type for extended (4096-byte) PCIe configuration space.
+pub struct Extended;
+
+/// Trait for PCI configuration space size markers.
+///
+/// This trait is implemented by [`Normal`] and [`Extended`] to provide
+/// compile-time knowledge of the configuration space size.
+pub trait ConfigSpaceSize {
+    /// The size of this configuration space in bytes.
+    const SIZE: usize;
+}
+
+impl ConfigSpaceSize for Normal {
+    const SIZE: usize = 256;
+}
+
+impl ConfigSpaceSize for Extended {
+    const SIZE: usize = 4096;
+}
+
+/// The PCI configuration space of a device.
+///
+/// Provides typed read and write accessors for configuration registers
+/// using the standard `pci_read_config_*` and `pci_write_config_*` helpers.
+///
+/// The generic parameter `S` indicates the maximum size of the configuration space.
+/// Use [`Normal`] for 256-byte legacy configuration space or [`Extended`] for
+/// 4096-byte PCIe extended configuration space (default).
+pub struct ConfigSpace<'a, S: ConfigSpaceSize = Extended> {
+    pub(crate) pdev: &'a Device<device::Bound>,
+    _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) };
+    };
+}
+
+// PCI configuration space supports 8, 16, and 32-bit accesses.
+impl<'a, S: ConfigSpaceSize> IoCapable<u8> for ConfigSpace<'a, S> {}
+impl<'a, S: ConfigSpaceSize> IoCapable<u16> for ConfigSpace<'a, S> {}
+impl<'a, S: ConfigSpaceSize> IoCapable<u32> for ConfigSpace<'a, S> {}
+
+impl<'a, S: ConfigSpaceSize> Io for ConfigSpace<'a, S> {
+    const MIN_SIZE: usize = S::SIZE;
+
+    /// Returns the base address of the I/O region. It is always 0 for configuration space.
+    #[inline]
+    fn addr(&self) -> usize {
+        0
+    }
+
+    /// Returns the maximum size of the configuration space.
+    #[inline]
+    fn maxsize(&self) -> usize {
+        self.pdev.cfg_size().map_or(0, |v| v)
+    }
+
+    // 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);
+}
+
+/// Marker trait indicating ConfigSpace has a known size at compile time.
+impl<'a, S: ConfigSpaceSize> IoKnownSize for ConfigSpace<'a, S> {}
 
 /// A PCI BAR to perform I/O-Operations on.
 ///
@@ -144,4 +280,33 @@ pub fn iomap_region<'a>(
     ) -> impl PinInit<Devres<Bar>, Error> + 'a {
         self.iomap_region_sized::<0>(bar, name)
     }
+
+    /// Returns the size of configuration space in bytes.
+    fn cfg_size(&self) -> Result<usize> {
+        // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
+        let size = unsafe { (*self.as_raw()).cfg_size };
+        match size {
+            256 | 4096 => Ok(size as usize),
+            _ => {
+                debug_assert!(false);
+                Err(EINVAL)
+            }
+        }
+    }
+
+    /// Return an initialized normal (256-byte) config space object.
+    pub fn config_space<'a>(&'a self) -> Result<ConfigSpace<'a, Normal>> {
+        Ok(ConfigSpace {
+            pdev: self,
+            _marker: PhantomData,
+        })
+    }
+
+    /// Return an initialized extended (4096-byte) config space object.
+    pub fn config_space_extended<'a>(&'a self) -> Result<ConfigSpace<'a, Extended>> {
+        Ok(ConfigSpace {
+            pdev: self,
+            _marker: PhantomData,
+        })
+    }
 }
-- 
2.51.0

On Wed Jan 21, 2026 at 8:22 PM GMT, Zhi Wang wrote:
> Drivers might need to access PCI config space for querying capability
> structures and access the registers inside the structures.
>
> For Rust drivers need to access PCI config space, the Rust PCI abstraction
> needs to support it in a way that upholds Rust's safety principles.
>
> Introduce a `ConfigSpace` wrapper in Rust PCI abstraction to provide safe
> accessors for PCI config space. The new type implements the `Io` trait and
> `IoCapable<T>` for u8, u16, and u32 to share offset validation and
> bound-checking logic with other I/O backends.
>
> The `ConfigSpace` type uses marker types (`Normal` and `Extended`) to
> represent configuration space sizes at the type level.
>
> Cc: Alexandre Courbot <acourbot@nvidia.com>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Cc: Gary Guo <gary@garyguo.net>
> Cc: Joel Fernandes <joelagnelf@nvidia.com>
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> ---
>  rust/kernel/pci.rs    |   7 +-
>  rust/kernel/pci/io.rs | 167 +++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 172 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 82e128431f08..9020959ce0c7 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -40,7 +40,12 @@
>      ClassMask,
>      Vendor, //
>  };
> -pub use self::io::Bar;
> +pub use self::io::{
> +    Bar,
> +    ConfigSpaceSize,
> +    Extended,
> +    Normal, //
> +};
>  pub use self::irq::{
>      IrqType,
>      IrqTypes,
> diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs
> index e3377397666e..39df41d0eaab 100644
> --- a/rust/kernel/pci/io.rs
> +++ b/rust/kernel/pci/io.rs
> @@ -8,13 +8,149 @@
>      device,
>      devres::Devres,
>      io::{
> +        define_read,
> +        define_write,
> +        Io,
> +        IoCapable,
> +        IoKnownSize,
>          Mmio,
>          MmioRaw, //
>      },
>      prelude::*,
>      sync::aref::ARef, //
>  };
> -use core::ops::Deref;
> +use core::{
> +    marker::PhantomData,
> +    ops::Deref, //
> +};
> +
> +/// Marker type for normal (256-byte) PCI configuration space.
> +pub struct Normal;
> +
> +/// Marker type for extended (4096-byte) PCIe configuration space.
> +pub struct Extended;
> +
> +/// Trait for PCI configuration space size markers.
> +///
> +/// This trait is implemented by [`Normal`] and [`Extended`] to provide
> +/// compile-time knowledge of the configuration space size.
> +pub trait ConfigSpaceSize {
> +    /// The size of this configuration space in bytes.
> +    const SIZE: usize;
> +}
> +
> +impl ConfigSpaceSize for Normal {
> +    const SIZE: usize = 256;
> +}
> +
> +impl ConfigSpaceSize for Extended {
> +    const SIZE: usize = 4096;
> +}
> +
> +/// The PCI configuration space of a device.
> +///
> +/// Provides typed read and write accessors for configuration registers
> +/// using the standard `pci_read_config_*` and `pci_write_config_*` helpers.
> +///
> +/// The generic parameter `S` indicates the maximum size of the configuration space.
> +/// Use [`Normal`] for 256-byte legacy configuration space or [`Extended`] for
> +/// 4096-byte PCIe extended configuration space (default).
> +pub struct ConfigSpace<'a, S: ConfigSpaceSize = Extended> {
> +    pub(crate) pdev: &'a Device<device::Bound>,
> +    _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) };
> +    };
> +}
> +
> +// PCI configuration space supports 8, 16, and 32-bit accesses.
> +impl<'a, S: ConfigSpaceSize> IoCapable<u8> for ConfigSpace<'a, S> {}
> +impl<'a, S: ConfigSpaceSize> IoCapable<u16> for ConfigSpace<'a, S> {}
> +impl<'a, S: ConfigSpaceSize> IoCapable<u32> for ConfigSpace<'a, S> {}
> +
> +impl<'a, S: ConfigSpaceSize> Io for ConfigSpace<'a, S> {
> +    const MIN_SIZE: usize = S::SIZE;
> +
> +    /// Returns the base address of the I/O region. It is always 0 for configuration space.
> +    #[inline]
> +    fn addr(&self) -> usize {
> +        0
> +    }
> +
> +    /// Returns the maximum size of the configuration space.
> +    #[inline]
> +    fn maxsize(&self) -> usize {
> +        self.pdev.cfg_size().map_or(0, |v| v)
> +    }
> +
> +    // 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);
> +}
> +
> +/// Marker trait indicating ConfigSpace has a known size at compile time.
> +impl<'a, S: ConfigSpaceSize> IoKnownSize for ConfigSpace<'a, S> {}
>  
>  /// A PCI BAR to perform I/O-Operations on.
>  ///
> @@ -144,4 +280,33 @@ pub fn iomap_region<'a>(
>      ) -> impl PinInit<Devres<Bar>, Error> + 'a {
>          self.iomap_region_sized::<0>(bar, name)
>      }
> +
> +    /// Returns the size of configuration space in bytes.
> +    fn cfg_size(&self) -> Result<usize> {
> +        // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
> +        let size = unsafe { (*self.as_raw()).cfg_size };
> +        match size {
> +            256 | 4096 => Ok(size as usize),
> +            _ => {
> +                debug_assert!(false);
> +                Err(EINVAL)
> +            }
> +        }
> +    }

This method is only invoked from maxsize, which turns error into `0`. Do apart
from the debug assertion, the error code is pointless. I think this function
should just return `usize` as it's specified in the device (we should trust the
C side that the value is sensible).

The check, as Alex mentioned, need to be done when ConfigSpace is created in
the first place and is too late when you already hand out `Ok(ConfigSpace)`.

Best,
Gary

> +
> +    /// Return an initialized normal (256-byte) config space object.
> +    pub fn config_space<'a>(&'a self) -> Result<ConfigSpace<'a, Normal>> {
> +        Ok(ConfigSpace {
> +            pdev: self,
> +            _marker: PhantomData,
> +        })
> +    }
> +
> +    /// Return an initialized extended (4096-byte) config space object.
> +    pub fn config_space_extended<'a>(&'a self) -> Result<ConfigSpace<'a, Extended>> {
> +        Ok(ConfigSpace {
> +            pdev: self,
> +            _marker: PhantomData,
> +        })
> +    }
>  }

On Thu Jan 22, 2026 at 12:59 PM CET, Gary Guo wrote:
> On Wed Jan 21, 2026 at 8:22 PM GMT, Zhi Wang wrote:
>> +    /// Returns the size of configuration space in bytes.
>> +    fn cfg_size(&self) -> Result<usize> {
>> +        // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
>> +        let size = unsafe { (*self.as_raw()).cfg_size };
>> +        match size {
>> +            256 | 4096 => Ok(size as usize),
>> +            _ => {
>> +                debug_assert!(false);
>> +                Err(EINVAL)
>> +            }
>> +        }
>> +    }
>
> This method is only invoked from maxsize, which turns error into `0`. Do apart
> from the debug assertion, the error code is pointless. I think this function
> should just return `usize` as it's specified in the device (we should trust the
> C side that the value is sensible).

That seems reasonable, but I also think we should keep the enum ConfigSpaceSize
we had before and call the new trait ConfigSpaceKind instead, such that this
method becomes:

	fn cfg_size(&self) -> ConfigSpaceSize;

> The check, as Alex mentioned, need to be done when ConfigSpace is created in
> the first place and is too late when you already hand out `Ok(ConfigSpace)`.

We need the check for config_space_extended(), but not for config_space(), as it
represents the minimum size, i.e. it's always valid.

Here's a diff of what I think this should look like on top of this series.

(@Zhi: If we all agree on the diff and nothing else comes up you don't need to
resend. :)

diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 9020959ce0c7..1d1a253e5d5d 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -42,6 +42,7 @@
 };
 pub use self::io::{
     Bar,
+    ConfigSpaceKind,
     ConfigSpaceSize,
     Extended,
     Normal, //
diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs
index 39df41d0eaab..5dbdfe516418 100644
--- a/rust/kernel/pci/io.rs
+++ b/rust/kernel/pci/io.rs
@@ -24,6 +24,31 @@
     ops::Deref, //
 };

+/// Represents the size of a PCI configuration space.
+///
+/// PCI devices can have either a *normal* (legacy) configuration space of 256 bytes,
+/// or an *extended* configuration space of 4096 bytes as defined in the PCI Express
+/// specification.
+#[repr(usize)]
+#[derive(PartialEq)]
+pub enum ConfigSpaceSize {
+    /// 256-byte legacy PCI configuration space.
+    Normal = 256,
+
+    /// 4096-byte PCIe extended configuration space.
+    Extended = 4096,
+}
+
+impl ConfigSpaceSize {
+    /// Get the raw value of this enum.
+    #[inline(always)]
+    pub const fn into_raw(self) -> usize {
+        // CAST: PCI configuration space size is at most 4096 bytes, so the value always fits
+        // within `usize` without truncation or sign change.
+        self as usize
+    }
+}
+
 /// Marker type for normal (256-byte) PCI configuration space.
 pub struct Normal;

@@ -34,16 +59,16 @@
 ///
 /// This trait is implemented by [`Normal`] and [`Extended`] to provide
 /// compile-time knowledge of the configuration space size.
-pub trait ConfigSpaceSize {
+pub trait ConfigSpaceKind {
     /// The size of this configuration space in bytes.
     const SIZE: usize;
 }

-impl ConfigSpaceSize for Normal {
+impl ConfigSpaceKind for Normal {
     const SIZE: usize = 256;
 }

-impl ConfigSpaceSize for Extended {
+impl ConfigSpaceKind for Extended {
     const SIZE: usize = 4096;
 }

@@ -55,7 +80,7 @@ impl ConfigSpaceSize for Extended {
 /// The generic parameter `S` indicates the maximum size of the configuration space.
 /// Use [`Normal`] for 256-byte legacy configuration space or [`Extended`] for
 /// 4096-byte PCIe extended configuration space (default).
-pub struct ConfigSpace<'a, S: ConfigSpaceSize = Extended> {
+pub struct ConfigSpace<'a, S: ConfigSpaceKind = Extended> {
     pub(crate) pdev: &'a Device<device::Bound>,
     _marker: PhantomData<S>,
 }
@@ -118,11 +143,11 @@ macro_rules! call_config_write {
 }

 // PCI configuration space supports 8, 16, and 32-bit accesses.
-impl<'a, S: ConfigSpaceSize> IoCapable<u8> for ConfigSpace<'a, S> {}
-impl<'a, S: ConfigSpaceSize> IoCapable<u16> for ConfigSpace<'a, S> {}
-impl<'a, S: ConfigSpaceSize> IoCapable<u32> for ConfigSpace<'a, S> {}
+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<'a, S: ConfigSpaceSize> Io for ConfigSpace<'a, S> {
+impl<'a, S: ConfigSpaceKind> Io for ConfigSpace<'a, S> {
     const MIN_SIZE: usize = S::SIZE;

     /// Returns the base address of the I/O region. It is always 0 for configuration space.
@@ -134,7 +159,7 @@ fn addr(&self) -> usize {
     /// Returns the maximum size of the configuration space.
     #[inline]
     fn maxsize(&self) -> usize {
-        self.pdev.cfg_size().map_or(0, |v| v)
+        self.pdev.cfg_size().into_raw()
     }

     // PCI configuration space does not support fallible operations.
@@ -150,7 +175,7 @@ fn maxsize(&self) -> usize {
 }

 /// Marker trait indicating ConfigSpace has a known size at compile time.
-impl<'a, S: ConfigSpaceSize> IoKnownSize for ConfigSpace<'a, S> {}
+impl<'a, S: ConfigSpaceKind> IoKnownSize for ConfigSpace<'a, S> {}

 /// A PCI BAR to perform I/O-Operations on.
 ///
@@ -281,29 +306,35 @@ pub fn iomap_region<'a>(
         self.iomap_region_sized::<0>(bar, name)
     }

-    /// Returns the size of configuration space in bytes.
-    fn cfg_size(&self) -> Result<usize> {
+    /// Returns the size of configuration space.
+    fn cfg_size(&self) -> ConfigSpaceSize {
         // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
         let size = unsafe { (*self.as_raw()).cfg_size };
         match size {
-            256 | 4096 => Ok(size as usize),
+            256 => ConfigSpaceSize::Normal,
+            4096 => ConfigSpaceSize::Extended,
             _ => {
-                debug_assert!(false);
-                Err(EINVAL)
+                // PANIC: The PCI subsystem only ever reports the configuration space size as either
+                // `ConfigSpaceSize::Normal` or `ConfigSpaceSize::Extended`.
+                unreachable!();
             }
         }
     }

     /// Return an initialized normal (256-byte) config space object.
-    pub fn config_space<'a>(&'a self) -> Result<ConfigSpace<'a, Normal>> {
-        Ok(ConfigSpace {
+    pub fn config_space<'a>(&'a self) -> ConfigSpace<'a, Normal> {
+        ConfigSpace {
             pdev: self,
             _marker: PhantomData,
-        })
+        }
     }

     /// Return an initialized extended (4096-byte) config space object.
     pub fn config_space_extended<'a>(&'a self) -> Result<ConfigSpace<'a, Extended>> {
+        if self.cfg_size() != ConfigSpaceSize::Extended {
+            return Err(EINVAL);
+        }
+
         Ok(ConfigSpace {
             pdev: self,
             _marker: PhantomData,
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 1bc5bd1a8df5..8eea79e858a2 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -67,8 +67,8 @@ fn testdev(index: &TestIndex, bar: &Bar0) -> Result<u32> {
         Ok(bar.read32(Regs::COUNT))
     }

-    fn config_space(pdev: &pci::Device<Bound>) -> Result {
-        let config = pdev.config_space()?;
+    fn config_space(pdev: &pci::Device<Bound>) {
+        let config = pdev.config_space();

         // TODO: use the register!() macro for defining PCI configuration space registers once it
         // has been move out of nova-core.
@@ -89,8 +89,6 @@ fn config_space(pdev: &pci::Device<Bound>) -> Result {
             "pci-testdev config space read32 BAR 0: {:x}\n",
             config.read32(0x10)
         );
-
-        Ok(())
     }
 }

@@ -123,7 +121,7 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Er
                         "pci-testdev data-match count: {}\n",
                         Self::testdev(info, bar)?
                     );
-                    Self::config_space(pdev)?;
+                    Self::config_space(pdev);
                 },
                 pdev: pdev.into(),
             }))

On Thu Jan 22, 2026 at 12:40 PM GMT, Danilo Krummrich wrote:
> On Thu Jan 22, 2026 at 12:59 PM CET, Gary Guo wrote:
>> On Wed Jan 21, 2026 at 8:22 PM GMT, Zhi Wang wrote:
>>> +    /// Returns the size of configuration space in bytes.
>>> +    fn cfg_size(&self) -> Result<usize> {
>>> +        // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
>>> +        let size = unsafe { (*self.as_raw()).cfg_size };
>>> +        match size {
>>> +            256 | 4096 => Ok(size as usize),
>>> +            _ => {
>>> +                debug_assert!(false);
>>> +                Err(EINVAL)
>>> +            }
>>> +        }
>>> +    }
>>
>> This method is only invoked from maxsize, which turns error into `0`. Do apart
>> from the debug assertion, the error code is pointless. I think this function
>> should just return `usize` as it's specified in the device (we should trust the
>> C side that the value is sensible).
>
> That seems reasonable, but I also think we should keep the enum ConfigSpaceSize
> we had before and call the new trait ConfigSpaceKind instead, such that this
> method becomes:
>
> 	fn cfg_size(&self) -> ConfigSpaceSize;
>
>> The check, as Alex mentioned, need to be done when ConfigSpace is created in
>> the first place and is too late when you already hand out `Ok(ConfigSpace)`.
>
> We need the check for config_space_extended(), but not for config_space(), as it
> represents the minimum size, i.e. it's always valid.
>
> Here's a diff of what I think this should look like on top of this series.

The proposal looks good to me. Some comments below.

Reviewed-by: Gary Guo <gary@garyguo.net>

>
> (@Zhi: If we all agree on the diff and nothing else comes up you don't need to
> resend. :)
>
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 9020959ce0c7..1d1a253e5d5d 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -42,6 +42,7 @@
>  };
>  pub use self::io::{
>      Bar,
> +    ConfigSpaceKind,
>      ConfigSpaceSize,
>      Extended,
>      Normal, //
> diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs
> index 39df41d0eaab..5dbdfe516418 100644
> --- a/rust/kernel/pci/io.rs
> +++ b/rust/kernel/pci/io.rs
> @@ -24,6 +24,31 @@
>      ops::Deref, //
>  };
>
> +/// Represents the size of a PCI configuration space.
> +///
> +/// PCI devices can have either a *normal* (legacy) configuration space of 256 bytes,
> +/// or an *extended* configuration space of 4096 bytes as defined in the PCI Express
> +/// specification.
> +#[repr(usize)]
> +#[derive(PartialEq)]

When `PartialEq` is derived, I would also derive `Eq` unless there's no
reflexivity in comparison.

> +pub enum ConfigSpaceSize {
> +    /// 256-byte legacy PCI configuration space.
> +    Normal = 256,
> +
> +    /// 4096-byte PCIe extended configuration space.
> +    Extended = 4096,
> +}
> +
> +impl ConfigSpaceSize {
> +    /// Get the raw value of this enum.
> +    #[inline(always)]
> +    pub const fn into_raw(self) -> usize {
> +        // CAST: PCI configuration space size is at most 4096 bytes, so the value always fits
> +        // within `usize` without truncation or sign change.
> +        self as usize
> +    }
> +}
> +
>  /// Marker type for normal (256-byte) PCI configuration space.
>  pub struct Normal;
>
> @@ -34,16 +59,16 @@
>  ///
>  /// This trait is implemented by [`Normal`] and [`Extended`] to provide
>  /// compile-time knowledge of the configuration space size.
> -pub trait ConfigSpaceSize {
> +pub trait ConfigSpaceKind {
>      /// The size of this configuration space in bytes.
>      const SIZE: usize;
>  }
>
> -impl ConfigSpaceSize for Normal {
> +impl ConfigSpaceKind for Normal {
>      const SIZE: usize = 256;
>  }
>
> -impl ConfigSpaceSize for Extended {
> +impl ConfigSpaceKind for Extended {
>      const SIZE: usize = 4096;
>  }
>
> @@ -55,7 +80,7 @@ impl ConfigSpaceSize for Extended {
>  /// The generic parameter `S` indicates the maximum size of the configuration space.
>  /// Use [`Normal`] for 256-byte legacy configuration space or [`Extended`] for
>  /// 4096-byte PCIe extended configuration space (default).
> -pub struct ConfigSpace<'a, S: ConfigSpaceSize = Extended> {
> +pub struct ConfigSpace<'a, S: ConfigSpaceKind = Extended> {
>      pub(crate) pdev: &'a Device<device::Bound>,
>      _marker: PhantomData<S>,
>  }
> @@ -118,11 +143,11 @@ macro_rules! call_config_write {
>  }
>
>  // PCI configuration space supports 8, 16, and 32-bit accesses.
> -impl<'a, S: ConfigSpaceSize> IoCapable<u8> for ConfigSpace<'a, S> {}
> -impl<'a, S: ConfigSpaceSize> IoCapable<u16> for ConfigSpace<'a, S> {}
> -impl<'a, S: ConfigSpaceSize> IoCapable<u32> for ConfigSpace<'a, S> {}
> +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<'a, S: ConfigSpaceSize> Io for ConfigSpace<'a, S> {
> +impl<'a, S: ConfigSpaceKind> Io for ConfigSpace<'a, S> {
>      const MIN_SIZE: usize = S::SIZE;
>
>      /// Returns the base address of the I/O region. It is always 0 for configuration space.
> @@ -134,7 +159,7 @@ fn addr(&self) -> usize {
>      /// Returns the maximum size of the configuration space.
>      #[inline]
>      fn maxsize(&self) -> usize {
> -        self.pdev.cfg_size().map_or(0, |v| v)
> +        self.pdev.cfg_size().into_raw()
>      }
>
>      // PCI configuration space does not support fallible operations.
> @@ -150,7 +175,7 @@ fn maxsize(&self) -> usize {
>  }
>
>  /// Marker trait indicating ConfigSpace has a known size at compile time.
> -impl<'a, S: ConfigSpaceSize> IoKnownSize for ConfigSpace<'a, S> {}
> +impl<'a, S: ConfigSpaceKind> IoKnownSize for ConfigSpace<'a, S> {}
>
>  /// A PCI BAR to perform I/O-Operations on.
>  ///
> @@ -281,29 +306,35 @@ pub fn iomap_region<'a>(
>          self.iomap_region_sized::<0>(bar, name)
>      }
>
> -    /// Returns the size of configuration space in bytes.
> -    fn cfg_size(&self) -> Result<usize> {
> +    /// Returns the size of configuration space.
> +    fn cfg_size(&self) -> ConfigSpaceSize {

If you keep this `fn` instead of `pub fn`, then the `ConfigSpaceSize` type being
`pub` is not very useful as it cannot be invoked by the user.

>          // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
>          let size = unsafe { (*self.as_raw()).cfg_size };
>          match size {
> -            256 | 4096 => Ok(size as usize),
> +            256 => ConfigSpaceSize::Normal,
> +            4096 => ConfigSpaceSize::Extended,
>              _ => {
> -                debug_assert!(false);
> -                Err(EINVAL)
> +                // PANIC: The PCI subsystem only ever reports the configuration space size as either
> +                // `ConfigSpaceSize::Normal` or `ConfigSpaceSize::Extended`.
> +                unreachable!();
>              }
>          }
>      }
>
>      /// Return an initialized normal (256-byte) config space object.
> -    pub fn config_space<'a>(&'a self) -> Result<ConfigSpace<'a, Normal>> {
> -        Ok(ConfigSpace {
> +    pub fn config_space<'a>(&'a self) -> ConfigSpace<'a, Normal> {

Nice. Less failing path = good.

> +        ConfigSpace {
>              pdev: self,
>              _marker: PhantomData,
> -        })
> +        }
>      }
>
>      /// Return an initialized extended (4096-byte) config space object.
>      pub fn config_space_extended<'a>(&'a self) -> Result<ConfigSpace<'a, Extended>> {
> +        if self.cfg_size() != ConfigSpaceSize::Extended {
> +            return Err(EINVAL);
> +        }
> +
>          Ok(ConfigSpace {
>              pdev: self,
>              _marker: PhantomData,
> diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
> index 1bc5bd1a8df5..8eea79e858a2 100644
> --- a/samples/rust/rust_driver_pci.rs
> +++ b/samples/rust/rust_driver_pci.rs
> @@ -67,8 +67,8 @@ fn testdev(index: &TestIndex, bar: &Bar0) -> Result<u32> {
>          Ok(bar.read32(Regs::COUNT))
>      }
>
> -    fn config_space(pdev: &pci::Device<Bound>) -> Result {
> -        let config = pdev.config_space()?;
> +    fn config_space(pdev: &pci::Device<Bound>) {
> +        let config = pdev.config_space();
>
>          // TODO: use the register!() macro for defining PCI configuration space registers once it
>          // has been move out of nova-core.
> @@ -89,8 +89,6 @@ fn config_space(pdev: &pci::Device<Bound>) -> Result {
>              "pci-testdev config space read32 BAR 0: {:x}\n",
>              config.read32(0x10)
>          );
> -
> -        Ok(())
>      }
>  }
>
> @@ -123,7 +121,7 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Er
>                          "pci-testdev data-match count: {}\n",
>                          Self::testdev(info, bar)?
>                      );
> -                    Self::config_space(pdev)?;
> +                    Self::config_space(pdev);
>                  },
>                  pdev: pdev.into(),
>              }))

On Thu, 22 Jan 2026 13:40:22 +0100
"Danilo Krummrich" <dakr@kernel.org> wrote:

> On Thu Jan 22, 2026 at 12:59 PM CET, Gary Guo wrote:
> > On Wed Jan 21, 2026 at 8:22 PM GMT, Zhi Wang wrote:

> (@Zhi: If we all agree on the diff and nothing else comes up you don't
> need to resend. :)
> 

Hi Danilo:

Thanks so much for this. I went through the code. It looks good to me.

Z.

> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 9020959ce0c7..1d1a253e5d5d 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -42,6 +42,7 @@
>  };
>  pub use self::io::{
>      Bar,
> +    ConfigSpaceKind,
>      ConfigSpaceSize,
>      Extended,
>      Normal, //
> diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs
> index 39df41d0eaab..5dbdfe516418 100644
> --- a/rust/kernel/pci/io.rs
> +++ b/rust/kernel/pci/io.rs
> @@ -24,6 +24,31 @@
>      ops::Deref, //
>  };
> 
> +/// Represents the size of a PCI configuration space.
> +///
> +/// PCI devices can have either a *normal* (legacy) configuration space
> of 256 bytes, +/// or an *extended* configuration space of 4096 bytes as
> defined in the PCI Express +/// specification.
> +#[repr(usize)]
> +#[derive(PartialEq)]
> +pub enum ConfigSpaceSize {
> +    /// 256-byte legacy PCI configuration space.
> +    Normal = 256,
> +
> +    /// 4096-byte PCIe extended configuration space.
> +    Extended = 4096,
> +}
> +
> +impl ConfigSpaceSize {
> +    /// Get the raw value of this enum.
> +    #[inline(always)]
> +    pub const fn into_raw(self) -> usize {
> +        // CAST: PCI configuration space size is at most 4096 bytes, so
> the value always fits
> +        // within `usize` without truncation or sign change.
> +        self as usize
> +    }
> +}
> +
>  /// Marker type for normal (256-byte) PCI configuration space.
>  pub struct Normal;
> 
> @@ -34,16 +59,16 @@
>  ///
>  /// This trait is implemented by [`Normal`] and [`Extended`] to provide
>  /// compile-time knowledge of the configuration space size.
> -pub trait ConfigSpaceSize {
> +pub trait ConfigSpaceKind {
>      /// The size of this configuration space in bytes.
>      const SIZE: usize;
>  }
> 
> -impl ConfigSpaceSize for Normal {
> +impl ConfigSpaceKind for Normal {
>      const SIZE: usize = 256;
>  }
> 
> -impl ConfigSpaceSize for Extended {
> +impl ConfigSpaceKind for Extended {
>      const SIZE: usize = 4096;
>  }
> 
> @@ -55,7 +80,7 @@ impl ConfigSpaceSize for Extended {
>  /// The generic parameter `S` indicates the maximum size of the
> configuration space. /// Use [`Normal`] for 256-byte legacy
> configuration space or [`Extended`] for /// 4096-byte PCIe extended
> configuration space (default). -pub struct ConfigSpace<'a, S:
> ConfigSpaceSize = Extended> { +pub struct ConfigSpace<'a, S:
> ConfigSpaceKind = Extended> { pub(crate) pdev: &'a Device<device::Bound>,
>      _marker: PhantomData<S>,
>  }
> @@ -118,11 +143,11 @@ macro_rules! call_config_write {
>  }
> 
>  // PCI configuration space supports 8, 16, and 32-bit accesses.
> -impl<'a, S: ConfigSpaceSize> IoCapable<u8> for ConfigSpace<'a, S> {}
> -impl<'a, S: ConfigSpaceSize> IoCapable<u16> for ConfigSpace<'a, S> {}
> -impl<'a, S: ConfigSpaceSize> IoCapable<u32> for ConfigSpace<'a, S> {}
> +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<'a, S: ConfigSpaceSize> Io for ConfigSpace<'a, S> {
> +impl<'a, S: ConfigSpaceKind> Io for ConfigSpace<'a, S> {
>      const MIN_SIZE: usize = S::SIZE;
> 
>      /// Returns the base address of the I/O region. It is always 0 for
> configuration space. @@ -134,7 +159,7 @@ fn addr(&self) -> usize {
>      /// Returns the maximum size of the configuration space.
>      #[inline]
>      fn maxsize(&self) -> usize {
> -        self.pdev.cfg_size().map_or(0, |v| v)
> +        self.pdev.cfg_size().into_raw()
>      }
> 
>      // PCI configuration space does not support fallible operations.
> @@ -150,7 +175,7 @@ fn maxsize(&self) -> usize {
>  }
> 
>  /// Marker trait indicating ConfigSpace has a known size at compile
> time. -impl<'a, S: ConfigSpaceSize> IoKnownSize for ConfigSpace<'a, S> {}
> +impl<'a, S: ConfigSpaceKind> IoKnownSize for ConfigSpace<'a, S> {}
> 
>  /// A PCI BAR to perform I/O-Operations on.
>  ///
> @@ -281,29 +306,35 @@ pub fn iomap_region<'a>(
>          self.iomap_region_sized::<0>(bar, name)
>      }
> 
> -    /// Returns the size of configuration space in bytes.
> -    fn cfg_size(&self) -> Result<usize> {
> +    /// Returns the size of configuration space.
> +    fn cfg_size(&self) -> ConfigSpaceSize {
>          // SAFETY: `self.as_raw` is a valid pointer to a `struct
> pci_dev`. let size = unsafe { (*self.as_raw()).cfg_size };
>          match size {
> -            256 | 4096 => Ok(size as usize),
> +            256 => ConfigSpaceSize::Normal,
> +            4096 => ConfigSpaceSize::Extended,
>              _ => {
> -                debug_assert!(false);
> -                Err(EINVAL)
> +                // PANIC: The PCI subsystem only ever reports the
> configuration space size as either
> +                // `ConfigSpaceSize::Normal` or
> `ConfigSpaceSize::Extended`.
> +                unreachable!();
>              }
>          }
>      }
> 
>      /// Return an initialized normal (256-byte) config space object.
> -    pub fn config_space<'a>(&'a self) -> Result<ConfigSpace<'a,
> Normal>> {
> -        Ok(ConfigSpace {
> +    pub fn config_space<'a>(&'a self) -> ConfigSpace<'a, Normal> {
> +        ConfigSpace {
>              pdev: self,
>              _marker: PhantomData,
> -        })
> +        }
>      }
> 
>      /// Return an initialized extended (4096-byte) config space object.
>      pub fn config_space_extended<'a>(&'a self) ->
> Result<ConfigSpace<'a, Extended>> {
> +        if self.cfg_size() != ConfigSpaceSize::Extended {
> +            return Err(EINVAL);
> +        }
> +
>          Ok(ConfigSpace {
>              pdev: self,
>              _marker: PhantomData,
> diff --git a/samples/rust/rust_driver_pci.rs
> b/samples/rust/rust_driver_pci.rs index 1bc5bd1a8df5..8eea79e858a2 100644
> --- a/samples/rust/rust_driver_pci.rs
> +++ b/samples/rust/rust_driver_pci.rs
> @@ -67,8 +67,8 @@ fn testdev(index: &TestIndex, bar: &Bar0) ->
> Result<u32> { Ok(bar.read32(Regs::COUNT))
>      }
> 
> -    fn config_space(pdev: &pci::Device<Bound>) -> Result {
> -        let config = pdev.config_space()?;
> +    fn config_space(pdev: &pci::Device<Bound>) {
> +        let config = pdev.config_space();
> 
>          // TODO: use the register!() macro for defining PCI
> configuration space registers once it // has been move out of nova-core.
> @@ -89,8 +89,6 @@ fn config_space(pdev: &pci::Device<Bound>) -> Result {
>              "pci-testdev config space read32 BAR 0: {:x}\n",
>              config.read32(0x10)
>          );
> -
> -        Ok(())
>      }
>  }
> 
> @@ -123,7 +121,7 @@ fn probe(pdev: &pci::Device<Core>, info:
> &Self::IdInfo) -> impl PinInit<Self, Er "pci-testdev data-match count:
> {}\n", Self::testdev(info, bar)?
>                      );
> -                    Self::config_space(pdev)?;
> +                    Self::config_space(pdev);
>                  },
>                  pdev: pdev.into(),
>              }))
>

On Thu Jan 22, 2026 at 5:22 AM JST, Zhi Wang wrote:
<snip>
> +    /// Return an initialized normal (256-byte) config space object.
> +    pub fn config_space<'a>(&'a self) -> Result<ConfigSpace<'a, Normal>> {
> +        Ok(ConfigSpace {
> +            pdev: self,
> +            _marker: PhantomData,
> +        })
> +    }
> +
> +    /// Return an initialized extended (4096-byte) config space object.
> +    pub fn config_space_extended<'a>(&'a self) -> Result<ConfigSpace<'a, Extended>> {
> +        Ok(ConfigSpace {
> +            pdev: self,
> +            _marker: PhantomData,
> +        })
> +    }

Why are we returning a `Result` given that none of these methods can
ever fail? I mentioned that in earlier revisions too [1], but it looks
like we should invoke `cfg_size` to validate the size first, lest we
return a `ConfigSpace` that is larger than what is actually supported.

[1] https://lore.kernel.org/all/DEHUBNKNSNXH.14A4OGQKY5KZR@nvidia.com/#t