Re: [PATCH v7 5/6] rust: pci: add config space read/write support

["Alexandre Courbot" <acourbot@nvidia.com>]

On Wed Nov 19, 2025 at 8:21 PM JST, 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: Alexandre Courbot <acourbot@nvidia.com>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Cc: Joel Fernandes <joelagnelf@nvidia.com>
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> ---
>  rust/kernel/pci.rs    |  43 ++++++++++++++-
>  rust/kernel/pci/io.rs | 118 +++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 159 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 82e128431f08..f373413e8a84 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -40,7 +40,10 @@
>      ClassMask,
>      Vendor, //
>  };
> -pub use self::io::Bar;
> +pub use self::io::{
> +    Bar,
> +    ConfigSpace, //
> +};
>  pub use self::irq::{
>      IrqType,
>      IrqTypes,
> @@ -331,6 +334,30 @@ 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.
> +#[repr(usize)]
> +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 as_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

While correct, an even more solid guarantee is the fact that `self` is
itself represented by a `usize`.

> +    }
> +}
> +
>  impl Device {
>      /// Returns the PCI vendor ID as [`Vendor`].
>      ///
> @@ -427,6 +454,20 @@ 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),
> +            _ => {
> +                debug_assert!(false);
> +                Err(EINVAL)
> +            }

Should we implement `TryFrom` for `ConfigSpaceSize` and use it here?

(can't wait for the `TryFrom` derive macro to come and remove the need
for this kind of boilerplate!)

<snip>
> @@ -141,4 +243,18 @@ pub fn iomap_region<'a>(
>      ) -> impl PinInit<Devres<Bar>, Error> + 'a {
>          self.iomap_region_sized::<0>(bar, name)
>      }
> +
> +    /// Return an initialized config space object.
> +    pub fn config_space<'a>(
> +        &'a self,
> +    ) -> Result<ConfigSpace<'a, { ConfigSpaceSize::Normal.as_raw() }>> {
> +        Ok(ConfigSpace { pdev: self })
> +    }
> +
> +    /// Return an initialized config space object.
> +    pub fn config_space_extended<'a>(
> +        &'a self,
> +    ) -> Result<ConfigSpace<'a, { ConfigSpaceSize::Extended.as_raw() }>> {
> +        Ok(ConfigSpace { pdev: self })
> +    }
>  }

This still has the problem that one can create an extended config space
object even if the device's `cfg_size` is `256`. Both
`config_space` and `config_space_extended` should fail if `cfg_size`
returns an invalid value, and `config_space_extended` should also fail
if the returned size less than `ConfigSpaceSize::Extended`.