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

["Gary Guo" <gary@garyguo.net>]

On Mon Aug 24, 2026 at 4:21 PM BST, Alexandre Courbot wrote:
> On Mon Aug 24, 2026 at 8:59 PM JST, Gary Guo wrote:
>> On Mon Aug 24, 2026 at 12:14 PM BST, Alexandre Courbot wrote:
>>> 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/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 }>());
>>>>
>>>> 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.
>>
>> Aesthetic and ergnomics is a big part of code, and is what make people like
>> Rust. In a world where less code is produced by human, I think it's especially
>> important that code remains easily human readable.
>>
>> You can probably already tell that I have very strong opinion about this.
>
> So do I, and the fact we both have opinions on the matter is irrelevant.
> "It looks horrible" is not an argument.

Ergnomics is a perfectly valid argument and often a key factor in design
decisions. Otherwise we'd make all functions carry extra argument indicating the
context they're in and would require sleeping functions to carry such token
types. We rejected that approach because it'll infect all functions and make
Rust code look horrible.

Everything is a trade-off. Whether to perform extra validation vs better
ergnomics is a genuine thing that needs deliberation. A outright dismissal of
the argument is itself a non-constructive argument.

>
> Safety and correctness are the very reason for using Rust, not that it
> looks better or is more ergonomic. It often does, it sometimes doesn't
> (see the hoops we have to jump through to cast a pointer for instance),
> and when it does, that is usually to improve correctness, not to
> compromise it. The reason we go through these lengths is to remove
> issues and footguns at build-time, and this is exactly what these
> helpers do because `as` is one of the footguns.

There's nothing unsafe or incorrect in using `as`. Your suggested code is no
more correct than Zhi's current version. You just added some extra check to some
constant that will never change, and we know will not produce error. My argument
is that that makes the syntax horrible and it's not a trade-off worth making.

When we create new abstractions, we try to make thing has minimal overhead.
Otherwise we'd be using a GC language and that'd solve memory safety issues.
Your abstraction is not zero-cost -- it sacrifices compilation-time, binary size
(for debug info) and it adds a significant cognitive overhead for writing code.

>
> They are just a temporary band-aid and won't be there forever anyway:
> once we have const `TryFrom`, we can make a bonfire with them, and I'll
> be happy to light the match myself.

Well, I'm happy converting the code once we have `const TryFrom`, and until
then, I'd rather keep `as`.

Best,
Gary