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 to
share offset validation and bound-checking logic with others.

Cc: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Zhi Wang <zhiw@nvidia.com>
---
 rust/kernel/pci.rs | 148 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 142 insertions(+), 6 deletions(-)

diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 77a8eb39ad32..630660a7f05d 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -5,13 +5,31 @@
 //! C header: [`include/linux/pci.h`](srctree/include/linux/pci.h)
 
 use crate::{
-    bindings, container_of, device,
-    device_id::{RawDeviceId, RawDeviceIdIndex},
+    bindings,
+    container_of,
+    device,
+    device_id::{
+        RawDeviceId,
+        RawDeviceIdIndex
+    },
     devres::Devres,
     driver,
-    error::{from_result, to_result, Result},
-    io::{Mmio, MmioRaw},
-    irq::{self, IrqRequest},
+    error::{
+        from_result,
+        to_result,
+        Result
+    },
+    io::{
+        define_read,
+        define_write,
+        Io,
+        Mmio,
+        MmioRaw
+    },
+    irq::{
+        self,
+        IrqRequest
+    },
     str::CStr,
     sync::aref::ARef,
     types::Opaque,
@@ -20,7 +38,10 @@
 use core::{
     marker::PhantomData,
     ops::Deref,
-    ptr::{addr_of_mut, NonNull},
+    ptr::{
+        addr_of_mut,
+        NonNull
+    },
 };
 use kernel::prelude::*;
 
@@ -305,6 +326,83 @@ pub struct Device<Ctx: device::DeviceContext = device::Normal>(
     PhantomData<Ctx>,
 );
 
+/// Represents 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 const parameter `SIZE` can be used to indicate the
+/// maximum size of the configuration space (e.g. 256 bytes for legacy,
+/// 4096 bytes for extended config space). The actual size is obtained
+/// from the underlying `struct pci_dev` via [`Device::cfg_size`].
+pub struct ConfigSpace<'a, const SIZE: usize = { ConfigSpaceSize::Extended as usize }> {
+    pdev: &'a Device<device::Core>,
+}
+
+macro_rules! call_config_read {
+    (infallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr) => {{
+        let mut val: $ty = 0;
+        let ret = unsafe { bindings::$c_fn($self.pdev.as_raw(), $addr as i32, &mut val) };
+        match ret {
+            0 => val,
+            _ => !0,
+        }
+    }};
+
+    (fallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr) => {{
+        let mut val: $ty = 0;
+        let ret = unsafe { bindings::$c_fn($self.pdev.as_raw(), $addr as i32, &mut val) };
+        (ret == 0)
+            .then_some(Ok(val))
+            .unwrap_or_else(|| Err(Error::from_errno(ret)))
+    }};
+}
+
+macro_rules! call_config_write {
+    (infallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr, $value:expr) => {
+        let _ret = unsafe { bindings::$c_fn($self.pdev.as_raw(), $addr as i32, $value) };
+    };
+
+    (fallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr, $value:expr) => {{
+        let ret = unsafe { bindings::$c_fn($self.pdev.as_raw(), $addr as i32, $value) };
+        (ret == 0)
+            .then_some(Ok(()))
+            .unwrap_or_else(|| Err(Error::from_errno(ret)))
+    }};
+}
+
+impl<'a, const SIZE: usize> Io for ConfigSpace<'a, SIZE> {
+    const MIN_SIZE: usize = SIZE;
+
+    /// Returns the base address of this mapping.
+    #[inline]
+    fn addr(&self) -> usize {
+        0
+    }
+
+    /// Returns the maximum size of this mapping.
+    #[inline]
+    fn maxsize(&self) -> usize {
+        self.pdev.cfg_size().map_or(0, |v| v as usize)
+    }
+
+    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_read!(fallible, try_read8, call_config_read, pci_read_config_byte -> u8);
+    define_read!(fallible, try_read16, call_config_read, pci_read_config_word -> u16);
+    define_read!(fallible, try_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);
+
+    define_write!(fallible, try_write8, call_config_write, pci_write_config_byte <- u8);
+    define_write!(fallible, try_write16, call_config_write, pci_write_config_word <- u16);
+    define_write!(fallible, try_write32, call_config_write, pci_write_config_dword <- u32);
+}
+
 /// A PCI BAR to perform I/O-Operations on.
 ///
 /// # Invariants
@@ -418,6 +516,19 @@ fn as_raw(&self) -> *mut bindings::pci_dev {
     }
 }
 
+/// 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.
+pub enum ConfigSpaceSize {
+    /// 256-byte legacy PCI configuration space.
+    Normal = 256,
+
+    /// 4096-byte PCIe extended configuration space.
+    Extended = 4096,
+}
+
 impl Device {
     /// Returns the PCI vendor ID as [`Vendor`].
     ///
@@ -514,6 +625,17 @@ pub fn pci_class(&self) -> Class {
         // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
         Class::from_raw(unsafe { (*self.as_raw()).class })
     }
+
+    /// Returns the size of configuration space.
+    fn cfg_size(&self) -> Result<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 => Ok(ConfigSpaceSize::Normal),
+            4096 => Ok(ConfigSpaceSize::Extended),
+            _ => Err(EINVAL),
+        }
+    }
 }
 
 impl Device<device::Bound> {
@@ -591,6 +713,20 @@ pub fn set_master(&self) {
         // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
         unsafe { bindings::pci_set_master(self.as_raw()) };
     }
+
+    /// Return an initialized config space object.
+    pub fn config_space<'a>(
+        &'a self,
+    ) -> Result<ConfigSpace<'a, { ConfigSpaceSize::Normal as usize }>> {
+        Ok(ConfigSpace { pdev: self })
+    }
+
+    /// Return an initialized config space object.
+    pub fn config_space_exteneded<'a>(
+        &'a self,
+    ) -> Result<ConfigSpace<'a, { ConfigSpaceSize::Extended as usize }>> {
+        Ok(ConfigSpace { pdev: self })
+    }
 }
 
 // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
-- 
2.51.0

On Tue Nov 4, 2025 at 3:27 PM CET, Zhi Wang wrote:
> +/// Represents 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 const parameter `SIZE` can be used to indicate the
> +/// maximum size of the configuration space (e.g. 256 bytes for legacy,
> +/// 4096 bytes for extended config space). The actual size is obtained
> +/// from the underlying `struct pci_dev` via [`Device::cfg_size`].
> +pub struct ConfigSpace<'a, const SIZE: usize = { ConfigSpaceSize::Extended as usize }> {
> +    pdev: &'a Device<device::Core>,
> +}

Device<Bound> should be enough, we don't need to require the core context.

On Tue Nov 4, 2025 at 3:27 PM CET, 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 to
> share offset validation and bound-checking logic with others.
>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> ---
>  rust/kernel/pci.rs | 148 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 142 insertions(+), 6 deletions(-)
>
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 77a8eb39ad32..630660a7f05d 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -5,13 +5,31 @@
>  //! C header: [`include/linux/pci.h`](srctree/include/linux/pci.h)
>  
>  use crate::{
> -    bindings, container_of, device,
> -    device_id::{RawDeviceId, RawDeviceIdIndex},
> +    bindings,
> +    container_of,
> +    device,
> +    device_id::{
> +        RawDeviceId,
> +        RawDeviceIdIndex
> +    },
>      devres::Devres,
>      driver,
> -    error::{from_result, to_result, Result},
> -    io::{Mmio, MmioRaw},
> -    irq::{self, IrqRequest},
> +    error::{
> +        from_result,
> +        to_result,
> +        Result
> +    },
> +    io::{
> +        define_read,
> +        define_write,
> +        Io,
> +        Mmio,
> +        MmioRaw
> +    },
> +    irq::{
> +        self,
> +        IrqRequest
> +    },
>      str::CStr,
>      sync::aref::ARef,
>      types::Opaque,
> @@ -20,7 +38,10 @@
>  use core::{
>      marker::PhantomData,
>      ops::Deref,
> -    ptr::{addr_of_mut, NonNull},
> +    ptr::{
> +        addr_of_mut,
> +        NonNull
> +    },
>  };

Please add a comma and use `//` at the end of the last element of a block as
described in [1]. Otherwise rustfmt will change it up again.

I think most of the diff is not related to this patch. I think it would be good
to convert the whole module and then add further changes.

I just did such a cleanup for the I/O module [1], I can send another patch for
all the PCI stuff.

[1] https://docs.kernel.org/rust/coding-guidelines.html#imports
[2] https://lore.kernel.org/lkml/20251104133301.59402-1-dakr@kernel.org/

>  use kernel::prelude::*;
>  
> @@ -305,6 +326,83 @@ pub struct Device<Ctx: device::DeviceContext = device::Normal>(
>      PhantomData<Ctx>,
>  );
>  
> +/// Represents 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 const parameter `SIZE` can be used to indicate the
> +/// maximum size of the configuration space (e.g. 256 bytes for legacy,
> +/// 4096 bytes for extended config space). The actual size is obtained
> +/// from the underlying `struct pci_dev` via [`Device::cfg_size`].
> +pub struct ConfigSpace<'a, const SIZE: usize = { ConfigSpaceSize::Extended as usize }> {
> +    pdev: &'a Device<device::Core>,
> +}
> +
> +macro_rules! call_config_read {
> +    (infallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr) => {{
> +        let mut val: $ty = 0;
> +        let ret = unsafe { bindings::$c_fn($self.pdev.as_raw(), $addr as i32, &mut val) };
> +        match ret {
> +            0 => val,
> +            _ => !0,
> +        }

I think the match is not needed, the value should already be set accordingly.

> +    }};
> +/// 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.
> +pub enum ConfigSpaceSize {

I think this should be [repr(usize)].

> +    /// 256-byte legacy PCI configuration space.
> +    Normal = 256,
> +
> +    /// 4096-byte PCIe extended configuration space.
> +    Extended = 4096,
> +}
> +
>  impl Device {
>      /// Returns the PCI vendor ID as [`Vendor`].
>      ///
> @@ -514,6 +625,17 @@ pub fn pci_class(&self) -> Class {
>          // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
>          Class::from_raw(unsafe { (*self.as_raw()).class })
>      }
> +
> +    /// Returns the size of configuration space.
> +    fn cfg_size(&self) -> Result<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 => Ok(ConfigSpaceSize::Normal),
> +            4096 => Ok(ConfigSpaceSize::Extended),
> +            _ => Err(EINVAL),

I'd add a debug_assert!() in this case.

> +        }
> +    }
>  }
>  
>  impl Device<device::Bound> {
> @@ -591,6 +713,20 @@ pub fn set_master(&self) {
>          // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
>          unsafe { bindings::pci_set_master(self.as_raw()) };
>      }
> +
> +    /// Return an initialized config space object.
> +    pub fn config_space<'a>(
> +        &'a self,
> +    ) -> Result<ConfigSpace<'a, { ConfigSpaceSize::Normal as usize }>> {

Let's add as_raw() to ConfigSpaceSize and use that instead.

> +        Ok(ConfigSpace { pdev: self })
> +    }
> +
> +    /// Return an initialized config space object.
> +    pub fn config_space_exteneded<'a>(
> +        &'a self,
> +    ) -> Result<ConfigSpace<'a, { ConfigSpaceSize::Extended as usize }>> {
> +        Ok(ConfigSpace { pdev: self })
> +    }
>  }
>  
>  // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
> -- 
> 2.51.0

On Tue, 04 Nov 2025 16:36:35 +0100
"Danilo Krummrich" <dakr@kernel.org> wrote:

> On Tue Nov 4, 2025 at 3:27 PM CET, 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 to share offset validation and
> > bound-checking logic with others.
> >
> > Cc: Danilo Krummrich <dakr@kernel.org>
> > Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> > ---
> >  rust/kernel/pci.rs | 148
> > +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 142
> > insertions(+), 6 deletions(-)
> >
> > diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> > index 77a8eb39ad32..630660a7f05d 100644
> > --- a/rust/kernel/pci.rs
> > +++ b/rust/kernel/pci.rs
> > @@ -5,13 +5,31 @@
> >  //! C header: [`include/linux/pci.h`](srctree/include/linux/pci.h)
> >  
> >  use crate::{
> > -    bindings, container_of, device,
> > -    device_id::{RawDeviceId, RawDeviceIdIndex},
> > +    bindings,
> > +    container_of,
> > +    device,
> > +    device_id::{
> > +        RawDeviceId,
> > +        RawDeviceIdIndex
> > +    },
> >      devres::Devres,
> >      driver,
> > -    error::{from_result, to_result, Result},
> > -    io::{Mmio, MmioRaw},
> > -    irq::{self, IrqRequest},
> > +    error::{
> > +        from_result,
> > +        to_result,
> > +        Result
> > +    },
> > +    io::{
> > +        define_read,
> > +        define_write,
> > +        Io,
> > +        Mmio,
> > +        MmioRaw
> > +    },
> > +    irq::{
> > +        self,
> > +        IrqRequest
> > +    },
> >      str::CStr,
> >      sync::aref::ARef,
> >      types::Opaque,
> > @@ -20,7 +38,10 @@
> >  use core::{
> >      marker::PhantomData,
> >      ops::Deref,
> > -    ptr::{addr_of_mut, NonNull},
> > +    ptr::{
> > +        addr_of_mut,
> > +        NonNull
> > +    },
> >  };
> 
> Please add a comma and use `//` at the end of the last element of a
> block as described in [1]. Otherwise rustfmt will change it up again.
> 

I see.

> I think most of the diff is not related to this patch. I think it
> would be good to convert the whole module and then add further
> changes.
> 
> I just did such a cleanup for the I/O module [1], I can send another
> patch for all the PCI stuff.
> 

I can rebase this series on top of yours once your sent them out. 

> [1] https://docs.kernel.org/rust/coding-guidelines.html#imports
> [2]
> https://lore.kernel.org/lkml/20251104133301.59402-1-dakr@kernel.org/
> 
> >  use kernel::prelude::*;
> >  
> > @@ -305,6 +326,83 @@ pub struct Device<Ctx: device::DeviceContext =
> > device::Normal>( PhantomData<Ctx>,
> >  );
> >  
> > +/// Represents 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 const parameter `SIZE` can be used to indicate the
> > +/// maximum size of the configuration space (e.g. 256 bytes for
> > legacy, +/// 4096 bytes for extended config space). The actual size
> > is obtained +/// from the underlying `struct pci_dev` via
> > [`Device::cfg_size`]. +pub struct ConfigSpace<'a, const SIZE: usize
> > = { ConfigSpaceSize::Extended as usize }> {
> > +    pdev: &'a Device<device::Core>,
> > +}
> > +
> > +macro_rules! call_config_read {
> > +    (infallible, $c_fn:ident, $self:ident, $ty:ty, $addr:expr) =>
> > {{
> > +        let mut val: $ty = 0;
> > +        let ret = unsafe { bindings::$c_fn($self.pdev.as_raw(),
> > $addr as i32, &mut val) };
> > +        match ret {
> > +            0 => val,
> > +            _ => !0,
> > +        }
> 
> I think the match is not needed, the value should already be set
> accordingly.
> 
> > +    }};
> > +/// 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.
> > +pub enum ConfigSpaceSize {
> 
> I think this should be [repr(usize)].
> 
> > +    /// 256-byte legacy PCI configuration space.
> > +    Normal = 256,
> > +
> > +    /// 4096-byte PCIe extended configuration space.
> > +    Extended = 4096,
> > +}
> > +
> >  impl Device {
> >      /// Returns the PCI vendor ID as [`Vendor`].
> >      ///
> > @@ -514,6 +625,17 @@ pub fn pci_class(&self) -> Class {
> >          // SAFETY: `self.as_raw` is a valid pointer to a `struct
> > pci_dev`. Class::from_raw(unsafe { (*self.as_raw()).class })
> >      }
> > +
> > +    /// Returns the size of configuration space.
> > +    fn cfg_size(&self) -> Result<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 => Ok(ConfigSpaceSize::Normal),
> > +            4096 => Ok(ConfigSpaceSize::Extended),
> > +            _ => Err(EINVAL),
> 
> I'd add a debug_assert!() in this case.
> 
> > +        }
> > +    }
> >  }
> >  
> >  impl Device<device::Bound> {
> > @@ -591,6 +713,20 @@ pub fn set_master(&self) {
> >          // SAFETY: `self.as_raw` is guaranteed to be a pointer to
> > a valid `struct pci_dev`. unsafe {
> > bindings::pci_set_master(self.as_raw()) }; }
> > +
> > +    /// Return an initialized config space object.
> > +    pub fn config_space<'a>(
> > +        &'a self,
> > +    ) -> Result<ConfigSpace<'a, { ConfigSpaceSize::Normal as usize
> > }>> {
> 
> Let's add as_raw() to ConfigSpaceSize and use that instead.
> 
> > +        Ok(ConfigSpace { pdev: self })
> > +    }
> > +
> > +    /// Return an initialized config space object.
> > +    pub fn config_space_exteneded<'a>(
> > +        &'a self,
> > +    ) -> Result<ConfigSpace<'a, { ConfigSpaceSize::Extended as
> > usize }>> {
> > +        Ok(ConfigSpace { pdev: self })
> > +    }
> >  }
> >  
> >  // SAFETY: `Device` is a transparent wrapper of a type that
> > doesn't depend on `Device`'s generic -- 
> > 2.51.0
>