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

On Mon Aug 24, 2026 at 7:48 PM JST, Gary Guo wrote:
> On Mon Aug 24, 2026 at 9:12 AM BST, Alexandre Courbot wrote:
>> 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/[email protected]/
>>> Cc: Alexandre Courbot <[email protected]>
>>> Cc: Gary Guo <[email protected]>
>>> Signed-off-by: Zhi Wang <[email protected]>
>>> ---
>>>  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 }>());
>
> This looks horrible. I'd prefer `as`.

Looks are subjective (or so I like to tell myself), so let's focus on
what isn't.

What is not subjective is that you don't lose a single bit as long as
you use these functions, and can do your conversions without a `CAST`
comment.

These helpers were merged precisely for this use-case; if you want to
remove them and use `as` instead, that's of course open but also a
larger discussion than this patch.

>
>>> [snip]
>>>
>>> +
>>> +        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.
>
> `slot` is technically variable and you'd rely on optimization pass that is not
> const folding. I think `panic: ` is better.

Yeah no strong opinion on this. The use of a closure makes me a bit
unsure that the optimizer will catch the hint at every level of
optimization.