Re: [PATCH v8 1/1] rust: pci: add extended capability and SR-IOV support

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

On Tue Aug 18, 2026 at 5:46 PM JST, Zhi Wang wrote:
> Rust PCI drivers have no typed interface for locating and accessing PCIe
> extended capabilities.
>
> The SR-IOV extended capability describes VF topology and VF BARs. Expose
> this information through the Rust PCI abstraction so drivers can use the
> existing typed configuration-space accessors instead of raw bindings.
>
> Define ExtCapability to associate a capability ID with a register layout,
> and add ConfigSpace::find_ext_capability() to locate and project that
> layout. Bound the view at the next capability or the end of extended
> configuration space. Add ExtSriovRegs and a decoded VF BAR iterator that
> reads and validates all six VF BAR register slots up front, yields decoded
> BAR addresses and widths in logical order, and keeps the raw
> configuration-space slot advancement internal. Since PCI_EXT_CAP_NEXT() is
> a function-like macro, expose it through a Rust helper.
>
> Link: https://lore.kernel.org/rust-for-linux/20260804161612.776752-1-zhiw@nvidia.com/
> Cc: Alexandre Courbot <acourbot@nvidia.com>
> Cc: Gary Guo <gary@garyguo.net>
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> ---
>  rust/helpers/pci.c     |   5 +
>  rust/kernel/pci.rs     |   8 +
>  rust/kernel/pci/cap.rs | 329 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 342 insertions(+)
>  create mode 100644 rust/kernel/pci/cap.rs
>
> diff --git a/rust/helpers/pci.c b/rust/helpers/pci.c
> index 4ebf256dff23..b946b14d79e4 100644
> --- a/rust/helpers/pci.c
> +++ b/rust/helpers/pci.c
> @@ -24,6 +24,11 @@ __rust_helper bool rust_helper_dev_is_pci(const struct device *dev)
>  	return dev_is_pci(dev);
>  }
>  
> +__rust_helper u32 rust_helper_pci_ext_cap_next(u32 header)
> +{
> +	return PCI_EXT_CAP_NEXT(header);
> +}
> +
>  #ifndef CONFIG_PCI_IOV
>  __rust_helper unsigned int
>  rust_helper_pci_sriov_get_totalvfs(struct pci_dev *pdev)
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 9f19ccd5905c..008c2770a3f3 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -32,10 +32,18 @@
>      },
>  };
>  
> +mod cap;
>  mod id;
>  mod io;
>  mod irq;
>  
> +pub use self::cap::{
> +    ExtCapId,
> +    ExtCapability,
> +    ExtSriovCapability,
> +    ExtSriovRegs,
> +    ExtSriovVfBar, //
> +};
>  pub use self::id::{
>      Class,
>      ClassMask,
> diff --git a/rust/kernel/pci/cap.rs b/rust/kernel/pci/cap.rs
> new file mode 100644
> index 000000000000..ddb3fd73e195
> --- /dev/null
> +++ b/rust/kernel/pci/cap.rs
> @@ -0,0 +1,329 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! PCI extended capability support.
> +
> +use super::{
> +    io::ConfigSpaceBackend,
> +    ConfigSpace,
> +    Extended, //
> +};

Let's merge this block with the one below, i.e. using `crate::pci`?

> +use crate::{
> +    bindings,
> +    io::{
> +        Io,
> +        IoBackend,
> +        Region, //
> +    },
> +    num::Bounded,
> +    prelude::*,
> +};
> +
> +/// Number of VF BAR register slots in an SR-IOV capability.
> +// CAST: `PCI_SRIOV_NUM_BARS` is the PCIe-specified number of VF BAR register slots and fits in
> +// `usize`.
> +const NUM_VF_BARS: usize = bindings::PCI_SRIOV_NUM_BARS as usize;

The infallible casts module is now available in `master`. If you import
`crate::num::casts` you can now turn this into

    const NUM_VF_BARS: usize = casts::u32_as_usize(bindings::PCI_SRIOV_NUM_BARS);

and remove the `CAST` comment.

> +
> +/// PCI extended capability IDs.
> +#[repr(transparent)]
> +#[derive(Debug, Clone, Copy, PartialEq, Eq)]
> +pub struct ExtCapId(u16);
> +
> +impl ExtCapId {
> +    /// Single Root I/O Virtualization.
> +    // CAST: PCI extended capability IDs are 16-bit values defined by the PCIe specification.
> +    pub const SRIOV: Self = Self(bindings::PCI_EXT_CAP_ID_SRIOV as u16);

Same here, the `CAST` comment can be removed if you turn this line into

    pub const SRIOV: Self = Self(casts::u32_into_u16::<{ bindings::PCI_EXT_CAP_ID_SRIOV }>());

> +
> +    /// Creates an extended capability ID from its raw PCIe value.
> +    #[inline]
> +    pub const fn new(id: u16) -> Self {

For symmetry with `as_raw`, should this be `from_raw`? The other PCI
types (e.g. Class and Vendor) also use this naming pattern.

> +        Self(id)
> +    }
> +
> +    /// Returns the raw PCIe extended capability ID.
> +    #[inline]
> +    const fn as_raw(self) -> u16 {
> +        self.0
> +    }

... and for symmetry as well, let's make this `pub`. :)

> +}
> +
> +/// A typed PCI extended capability register layout.
> +///
> +/// Implementors describe the register layout of one extended capability. The layout must start at
> +/// the extended capability header, and [`Self::ID`] must identify that layout.
> +pub trait ExtCapability: FromBytes + IntoBytes {
> +    /// PCI extended capability ID for this register layout.
> +    const ID: ExtCapId;
> +}
> +
> +impl<'a> ConfigSpace<'a, Extended> {
> +    /// Finds and projects an extended capability into its typed register layout.
> +    ///
> +    /// Returns [`None`] if the device does not implement the capability.
> +    ///
> +    /// Returns an error if the capability is present but its register span is too small or
> +    /// insufficiently aligned for `C`.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```no_run
> +    /// use kernel::{
> +    ///     device::Bound,
> +    ///     io::io_read,
> +    ///     pci,
> +    ///     prelude::*,
> +    /// };
> +    ///
> +    /// fn probe_sriov(pdev: &pci::Device<Bound>) -> Result {
> +    ///     let Some(sriov) = pdev
> +    ///         .config_space_extended()?
> +    ///         .find_ext_capability::<pci::ExtSriovRegs>()?
> +    ///     else {
> +    ///         return Ok(());
> +    ///     };
> +    ///
> +    ///     let total_vfs = io_read!(sriov, .total_vfs);
> +    ///     let vf_offset = io_read!(sriov, .vf_offset);
> +    ///     let mut vf_bars = sriov.vf_bars()?;
> +    ///     let bar0 = vf_bars.next().ok_or(EINVAL)?;
> +    ///     let bar1 = vf_bars.next().ok_or(EINVAL)?;
> +    ///     let bar2 = vf_bars.next().ok_or(EINVAL)?;
> +    ///
> +    ///     Ok(())
> +    /// }
> +    /// ```
> +    pub fn find_ext_capability<C: ExtCapability>(&self) -> Result<Option<ConfigSpace<'a, C>>> {
> +        let offset = usize::from(
> +            // SAFETY: `self.pdev` is valid by the type invariant of `ConfigSpace`.
> +            unsafe {
> +                bindings::pci_find_ext_capability(self.pdev.as_raw(), i32::from(C::ID.as_raw()))
> +            },
> +        );
> +
> +        if offset == 0 {
> +            return Ok(None);
> +        }
> +
> +        let size = self.calculate_ext_cap_size(offset)?;
> +
> +        let base = ConfigSpaceBackend::as_ptr(*self)
> +            .cast::<u8>()
> +            .wrapping_add(offset);
> +        let ptr = Region::<0>::ptr_try_from_raw_parts_mut(base, size)?;
> +
> +        // SAFETY: `offset` was returned by `pci_find_ext_capability`, and
> +        // `calculate_ext_cap_size` bounds `ptr` at the next capability or the end of the extended
> +        // configuration space. `ptr_try_from_raw_parts_mut` verified the region layout.
> +        let capability = unsafe { ConfigSpaceBackend::project_view(*self, ptr) };
> +
> +        capability.try_cast::<C>().map(Some)
> +    }
> +
> +    /// Calculates the size of the extended capability at `offset`.
> +    ///
> +    /// The capability extends to the next extended capability, or to the end of the extended
> +    /// configuration space if it is the last one. `offset` must be a DWORD-aligned offset within
> +    /// the extended configuration space returned by `pci_find_ext_capability`. Returns an error if
> +    /// the capability header is outside the extended configuration space.
> +    fn calculate_ext_cap_size(&self, offset: usize) -> Result<usize> {
> +        let header = self.try_read32(offset)?;
> +        // SAFETY: Pure bit manipulation, no preconditions.
> +        // CAST: The next-cap pointer is a 12-bit field (max 0xFFC), always fits in `usize`.
> +        let next = unsafe { bindings::pci_ext_cap_next(header) } as usize;

This `CAST` as well can be removed:

    let next = casts::u32_as_usize(unsafe { bindings::pci_ext_cap_next(header) });

> +
> +        Ok(if next > offset {
> +            next - offset
> +        } else {
> +            self.size() - offset
> +        })
> +    }
> +}
> +
> +/// SR-IOV register layout per PCIe spec (64 bytes starting at cap offset).
> +#[repr(C)]
> +#[derive(FromBytes, IntoBytes)]
> +pub struct ExtSriovRegs {
> +    /// Extended capability header.
> +    _header: u32,
> +    /// SR-IOV capabilities.
> +    pub cap: u32,
> +    /// SR-IOV control.
> +    pub ctrl: u16,
> +    /// SR-IOV status.
> +    pub status: u16,
> +    /// Initial VFs.
> +    pub initial_vfs: u16,
> +    /// Total VFs.
> +    pub total_vfs: u16,
> +    /// Number of VFs.
> +    pub num_vfs: u16,
> +    /// Function dependency link.
> +    pub func_dep_link: u8,
> +    _reserved_0: u8,
> +    /// First VF offset.
> +    pub vf_offset: u16,
> +    /// VF stride.
> +    pub vf_stride: u16,
> +    _reserved_1: u16,
> +    /// VF device ID.
> +    pub vf_device_id: u16,
> +    /// Supported page sizes.
> +    pub supported_page_sizes: u32,
> +    /// System page size.
> +    pub system_page_size: u32,
> +    /// VF BARs (BAR0–BAR5).
> +    pub vf_bar: [u32; NUM_VF_BARS],

Now that we have an iterator method, we can make this member private.
I'd even say we should as making this public enables the kinds of
invalid accesses we built the iterator to avoid.

> +    /// VF migration state array offset.
> +    pub migration_state: u32,
> +}
> +
> +impl ExtCapability for ExtSriovRegs {
> +    const ID: ExtCapId = ExtCapId::SRIOV;
> +}
> +
> +/// A typed view of an SR-IOV extended capability.
> +pub type ExtSriovCapability<'a> = ConfigSpace<'a, ExtSriovRegs>;
> +
> +#[derive(Debug, Clone, Copy, PartialEq, Eq)]
> +enum VfBarMemoryType {
> +    Bits32,
> +    Bits64,
> +}
> +
> +impl TryFrom<Bounded<u32, 2>> for VfBarMemoryType {
> +    type Error = Error;
> +
> +    fn try_from(value: Bounded<u32, 2>) -> Result<Self> {
> +        match value.get() {
> +            0b00 => Ok(Self::Bits32),
> +            0b10 => Ok(Self::Bits64),
> +            _ => Err(EINVAL),
> +        }
> +    }
> +}
> +
> +impl From<VfBarMemoryType> for Bounded<u32, 2> {
> +    fn from(value: VfBarMemoryType) -> Self {
> +        match value {
> +            VfBarMemoryType::Bits32 => Self::new::<0b00>(),
> +            VfBarMemoryType::Bits64 => Self::new::<0b10>(),
> +        }
> +    }
> +}
> +
> +crate::bitfield! {
> +    /// Low DWORD of an SR-IOV VF BAR.
> +    struct VfBarLow(u32) {
> +        /// Base address bits 31:4.
> +        31:4 address;
> +        /// Whether the address range is prefetchable.
> +        3:3 prefetchable => bool;
> +        /// Memory BAR type.
> +        2:1 memory_type ?=> VfBarMemoryType;
> +        /// Whether this is an I/O-space BAR.
> +        0:0 io_space => bool;
> +    }
> +}
> +
> +/// A decoded VF BAR register encoding.
> +#[derive(Debug, Clone, Copy, PartialEq, Eq)]
> +pub struct ExtSriovVfBar {
> +    /// The BAR address without PCI attribute bits.
> +    pub address: u64,
> +
> +    /// Whether the BAR is 64-bit.
> +    pub is_64bit: bool,
> +}
> +
> +/// Iterator over decoded VF BAR register encodings.
> +///
> +/// A 32-bit memory BAR encoding uses one register. A 64-bit memory BAR encoding uses that register
> +/// for bits 31:0 and the immediately following register for bits 63:32.
> +///
> +/// # Invariants
> +///
> +/// - `next_bar <= bar_count <= NUM_VF_BARS`.
> +/// - Entries before `bar_count` contain decoded VF BARs in logical order.
> +struct ExtSriovVfBars {
> +    bars: [ExtSriovVfBar; NUM_VF_BARS],
> +    bar_count: usize,
> +    next_bar: usize,
> +}
> +
> +impl ExtSriovVfBars {
> +    fn new(slots: [u32; NUM_VF_BARS]) -> Result<Self> {
> +        let mut bars = [ExtSriovVfBar {
> +            address: 0,
> +            is_64bit: false,
> +        }; NUM_VF_BARS];
> +        let mut bar_count = 0;
> +        let mut config_slot = 0;
> +
> +        while config_slot < NUM_VF_BARS {
> +            let low = VfBarLow::from(slots[config_slot]);
> +
> +            if low.io_space() {
> +                return Err(EINVAL);
> +            }

A comment would be appreciated for those not familiar with the PCI spec.
:)

> +
> +            let is_64bit = low.memory_type()? == VfBarMemoryType::Bits64;
> +            let low_address = u64::from(low.address()) << VfBarLow::ADDRESS_SHIFT;
> +
> +            let address = if is_64bit {
> +                if config_slot + 1 >= NUM_VF_BARS {
> +                    return Err(EINVAL);
> +                }
> +
> +                let high = slots[config_slot + 1];
> +                config_slot += 2;
> +                (u64::from(high) << 32) | low_address
> +            } else {
> +                config_slot += 1;
> +                low_address
> +            };
> +
> +            bars[bar_count] = ExtSriovVfBar { address, is_64bit };
> +            bar_count += 1;
> +        }

This looks a lot like C code - double counters in particular are
error-prone. We can make this a bit more idiomatic. I sense you didn't
use iterators because 64-bit entries take two slots, but you can use
this trick:

    // Store a mutable iterator.
    let mut slots = slots.into_iter();

    // Note the early conversion to `VfBarLow`
    while let Some(low) = slots.next().map(VfBarLow::from) {
        ...

        let bar = match low.memory_type()? {
            VfBarMemoryType::Bits64 => ExtSriovVfBar {
                // We read the second slot here.
                address: (u64::from(slots.next().ok_or(EINVAL)?) << 32) | low_address,
                is_64bit: true,
            },
            VfBarMemoryType::Bits32 => ExtSriovVfBar {
                address: low_address,
                is_64bit: false,
            },
        };

        ...
    }

... but we can go a bit further, please read along.

> +
> +        Ok(Self {
> +            bars,
> +            bar_count,
> +            next_bar: 0,
> +        })
> +    }
> +}
> +
> +impl Iterator for ExtSriovVfBars {
> +    type Item = ExtSriovVfBar;
> +
> +    fn next(&mut self) -> Option<Self::Item> {
> +        if self.next_bar >= self.bar_count {
> +            return None;
> +        }
> +
> +        let bar = self.bars[self.next_bar];
> +        self.next_bar += 1;
> +        Some(bar)
> +    }
> +}
> +
> +impl ExtSriovCapability<'_> {
> +    /// Returns an iterator over decoded VF BAR register encodings.
> +    ///
> +    /// All six raw VF BAR register slots are read and decoded up front. A 32-bit encoding yields
> +    /// one entry; a 64-bit encoding combines two slots into one entry.
> +    ///
> +    /// A zero-valued low DWORD is yielded as a 32-bit BAR at address zero; this method does not
> +    /// probe whether a BAR is implemented.
> +    ///
> +    /// Returns [`EINVAL`] and logs an error if a BAR low DWORD does not encode a 32-bit or 64-bit
> +    /// memory BAR, or if a 64-bit encoding has no upper DWORD.
> +    pub fn vf_bars(&self) -> Result<impl Iterator<Item = ExtSriovVfBar>> {

Since `ExtSriovVfBars` is private and we are returning an `impl`, I
think we can get rid of it altogether. Combining with my suggestion from
above, here is an alternative version of this method:

    pub fn vf_bars(&self) -> Result<impl Iterator<Item = ExtSriovVfBar>> {
        let slots: [u32; NUM_VF_BARS] =
            core::array::from_fn(|slot| crate::io_read!(*self, .vf_bar[panic: slot]));
        let mut slots = slots.into_iter();
        let mut bars = [None; NUM_VF_BARS];
        let mut count = 0;

        while let Some(low) = slots.next().map(VfBarLow::from) {
            if low.io_space() {
                return Err(EINVAL);
            }

            let low_address = u64::from(low.address()) << VfBarLow::ADDRESS_SHIFT;
            let bar = match low.memory_type()? {
                VfBarMemoryType::Bits64 => ExtSriovVfBar {
                    address: (u64::from(slots.next().ok_or(EINVAL)?) << 32) | low_address,
                    is_64bit: true,
                },
                VfBarMemoryType::Bits32 => ExtSriovVfBar {
                    address: low_address,
                    is_64bit: false,
                },
            };

            bars[count] = Some(bar);
            count += 1;
        }

        Ok(bars.into_iter().flatten())
    }

With this you don't need `ExtSriovVfBars` at all, which removes a bit
(almost 50 LoCs!) of code.

> +        let slots: [u32; NUM_VF_BARS] =
> +            core::array::from_fn(|slot| crate::io_read!(*self, .vf_bar[panic: slot]));

Can't this be `build:`? `vf_bar` is sized by `NUM_VF_BARS`, and so is
the result, so I'd assume the optimizer can infer this. Not that `panic:`
is problematic here but I wonder whether you chose this because you hit
an issue. To reiterate, I'm fine with `panic:` here, as long as the
alternative has been considered.