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

On Thu, 13 Aug 2026 15:56:41 +0200
"Danilo Krummrich" <[email protected]> wrote:

Hi Danilo:

I have updated the v8 to address all the comments except the io
projection part. Regarding to Gary's new Io project patch, I would send
another re-spin after they are on drm-rust-next. Thanks for the review.

Z.


> On Tue Aug 4, 2026 at 6:16 PM CEST, Zhi Wang wrote:
> > +/// Number of VF BAR register slots in an SR-IOV capability.
> > +// CAST: `PCI_SRIOV_NUM_BARS` is 6, which fits in `usize`.
> 
> I think the more relevant point is that it is a constant coming from
> the PCIe spec. (Plus, more SR-IOV BARs than addressable bytes
> wouldn't make a lot of sense anyway. :)
> 
> > +const NUM_VF_BARS: usize = bindings::PCI_SRIOV_NUM_BARS as usize;
> > +
> > +/// PCI extended capability IDs.
> > +#[repr(u16)]
> > +#[derive(Debug, Clone, Copy, PartialEq, Eq)]
> > +pub enum ExtCapId {
> > +    /// Single Root I/O Virtualization.
> > +    // CAST: `PCI_EXT_CAP_ID_SRIOV` is `0x10`, which fits in `u16`.
> 
> Same here, I'd mention that it is a constant from the spec.
> 
> > +    Sriov = bindings::PCI_EXT_CAP_ID_SRIOV as u16,
> > +}
> 
> [...]
> 
> > +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.
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// ```no_run
> > +    /// use kernel::pci;
> > +    ///
> > +    /// fn probe_sriov(
> > +    ///     pdev: &pci::Device<kernel::device::Bound>,
> > +    /// ) -> Result<(), kernel::error::Error> {
> 
> Just Result; please also import kernel::device::Bound for readability.
> 
> io_read!() should be imported from kernel::io::.
> 
> > +/// SR-IOV register layout per PCIe spec (64 bytes starting at cap
> > offset). +#[repr(C)]
> > +#[derive(FromBytes, IntoBytes)]
> > +pub struct ExtSriovRegs {
> > +    /// Extended capability header.
> > +    pub header: u32,
> > +    /// SR-IOV capabilities.
> > +    pub cap: u32,
> > +    /// SR-IOV control.
> > +    pub ctrl: u16,
> > +    /// SR-IOV status.
> > +    pub status: u16,
> 
> This is not a raw value, but a bitfield and should be represented as
> such (might be true for some other fields as well).
> 
> I think for this to work it needs Gary's recent I/O series though; I
> can merge both together next cycle and share with drm-rust.
> 
> > +    /// 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],
> > +    /// VF migration state array offset.
> > +    pub migration_state: u32,
> > +}