All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/1] Rust PCI capability infrastructure and SR-IOV support
@ 2026-07-30 18:29 Zhi Wang
  2026-07-30 18:29 ` [PATCH v6 1/1] rust: pci: add extended capability " Zhi Wang
  0 siblings, 1 reply; 7+ messages in thread
From: Zhi Wang @ 2026-07-30 18:29 UTC (permalink / raw)
  To: rust-for-linux, linux-pci, linux-kernel
  Cc: dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, cjia,
	smitra, ankita, aniketa, kwankhede, targupta, kjaju, alkumar,
	acourbot, joelagnelf, jhubbard, zhiwang, Zhi Wang

This is a follow-up to v5 [8], reworked to address the Sashiko review
[9]. This version is based on drm-rust-next.

This patch has been used in the Boot GSP with vGPU enabled series [6].

The patch defines an ExtCapability trait that associates an extended
capability ID with its register layout. The generic
ConfigSpace::find_ext_capability() finder locates the capability, bounds
it at the next capability or the end of extended configuration space,
and projects the ConfigSpace view to the requested layout. This lets the
existing I/O projection and access macros operate on capability registers.

ExtSriovRegs provides the SR-IOV register layout.
ConfigSpace::read_vf_bar() decodes a VF memory BAR into ExtSriovVfBar,
which exposes its address and 64-bit status, plus the configuration-space
slot of the next logical BAR. ExtSriovCapability remains as a convenience
alias.

Changes since v5:
- Removed the unused ConfigSpace<Region<0>> offset() and size() inherent
  methods; ConfigSpace already provides size through the Io trait.
  (Sashiko, Zhi)
- Removed the doctest write to the SR-IOV NumVFs register, avoiding an
  example that bypasses PCI core SR-IOV state management. (Sashiko)
- Corrected Function Dependency Link to an 8-bit field followed by its
  reserved byte, matching the PCIe SR-IOV register layout. (Sashiko)

Changes since v4:
- Replaced the separate is_vf_bar_64bit() and read_vf_bar64() helpers
  with read_vf_bar(), returning a decoded ExtSriovVfBar. (Zhi)
- Moved memory BAR attribute stripping into the PCI abstraction and used
  named PCI attribute definitions rather than an open-coded mask. (Zhi)
- Exposed the next logical BAR slot so callers can walk mixed 32-bit and
  64-bit VF BAR layouts without duplicating slot arithmetic. (Zhi)
- Updated the doctest and PCI exports for the decoded BAR API. (Zhi)

Changes since v3:
- Replaced the custom ExtCapability<T> I/O wrapper with the existing
  ConfigSpace view infrastructure. (Alex)
- Reused ExtCapability as a trait carrying the capability ID, and made
  ConfigSpace::find_ext_capability() generic over register layouts.
  (Alex)
- Removed public cast_sized() and unused find_next_ext_capability().
  (Alex)
- Kept capability construction in the generic finder and documented
  calculate_ext_cap_size(). (Alex)
- Used PCI_SRIOV_NUM_BARS rather than a literal VF BAR count.
  (Alex, Zhi)
- Added is_vf_bar_64bit() and made read_vf_bar64() reject BARs that are
  not 64-bit memory BARs. (Alex, Zhi)
- Kept indexed VF BAR helpers because the Nova user accesses fixed BAR
  slots rather than iterating over them. (Alex)
- Adapted the implementation and doctest to the current ConfigSpace I/O
  APIs. (Zhi)

Changes since RFC v2:
- Hardened calculate_ext_cap_size() against corrupt capability lists.
  (Zhi)
- Added // INVARIANT: comments at all ExtCapability construction sites
  (make_ext_capability and cast_sized). (Zhi)
- Added #[inline] to small forwarding methods (find, read_vf_bar64).
  (Zhi)

Changes since RFC:
- Rebased on io_projection branch, using Gary's Io/IoCapable traits.
  (Gary)
- ExtCapability implements Io and delegates IoCapable to ConfigSpace
  instead of duplicating config read/write logic. (Gary)
- Dropped the fallible I/O patch (now upstream in this tree). (Zhi)
- Added Rust helper for PCI_EXT_CAP_NEXT() macro. (Zhi)
- Replaced raw `as` casts with From conversions where possible. (Zhi)
- Renamed SriovRegs/SriovCapability to ExtSriovRegs/ExtSriovCapability.
  (Zhi)

[1] https://lore.kernel.org/rust-for-linux/20260409185254.3869808-1-zhiw@nvidia.com/
[2] https://lore.kernel.org/rust-for-linux/DHRTUAF52GNI.1J98TSAG1LS6Q@nvidia.com/
[3] https://lore.kernel.org/rust-for-linux/DI2SL4G5INLY.2W1IFTR081ID3@nvidia.com/
[4] https://lore.kernel.org/rust-for-linux/20260225180449.1813833-1-zhiw@nvidia.com/
[5] https://lore.kernel.org/rust-for-linux/20260323153807.1360705-1-gary@kernel.org/
[6] https://lore.kernel.org/rust-for-linux/20260313165336.935771-1-zhiw@nvidia.com/
[7] https://lore.kernel.org/rust-for-linux/20260714165827.2937960-1-zhiw@nvidia.com/
[8] https://lore.kernel.org/rust-for-linux/20260730180349.771719-1-zhiw@nvidia.com/
[9] https://sashiko.dev/#/patchset/20260730180349.771719-2-zhiw@nvidia.com?part=1

Zhi Wang (1):
  rust: pci: add extended capability and SR-IOV support

 rust/helpers/pci.c     |   5 +
 rust/kernel/pci.rs     |   8 ++
 rust/kernel/pci/cap.rs | 236 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 249 insertions(+)
 create mode 100644 rust/kernel/pci/cap.rs


base-commit: 93b9511a3bba7f31d95502e5f912f0a476b0cf4a
-- 
2.53.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v6 1/1] rust: pci: add extended capability and SR-IOV support
  2026-07-30 18:29 [PATCH v6 0/1] Rust PCI capability infrastructure and SR-IOV support Zhi Wang
@ 2026-07-30 18:29 ` Zhi Wang
  2026-07-30 18:39   ` sashiko-bot
                     ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Zhi Wang @ 2026-07-30 18:29 UTC (permalink / raw)
  To: rust-for-linux, linux-pci, linux-kernel
  Cc: dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, cjia,
	smitra, ankita, aniketa, kwankhede, targupta, kjaju, alkumar,
	acourbot, joelagnelf, jhubbard, zhiwang, Zhi Wang

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 helper that
returns the address, width, and next configuration-space slot, using
PCI_SRIOV_NUM_BARS for the number of BAR slots. Since PCI_EXT_CAP_NEXT()
is a function-like macro, expose it through a Rust helper.

Link: https://lore.kernel.org/rust-for-linux/20260730180349.771719-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 | 236 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 249 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..08c044bedb70
--- /dev/null
+++ b/rust/kernel/pci/cap.rs
@@ -0,0 +1,236 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! PCI extended capability support.
+
+use super::{
+    io::ConfigSpaceBackend,
+    ConfigSpace,
+    Extended, //
+};
+use crate::{
+    bindings,
+    io::{
+        Io,
+        IoBackend,
+        Region, //
+    },
+    prelude::*,
+};
+
+/// Number of VF BAR register slots in an SR-IOV capability.
+// CAST: `PCI_SRIOV_NUM_BARS` is 6, which fits in `usize`.
+const NUM_VF_BARS: usize = bindings::PCI_SRIOV_NUM_BARS as usize;
+
+/// Attribute bits encoded in the low DWORD of a memory BAR.
+const VF_MEMORY_BAR_ATTRIBUTE_BITS: u32 = bindings::PCI_BASE_ADDRESS_SPACE
+    | bindings::PCI_BASE_ADDRESS_MEM_TYPE_MASK
+    | bindings::PCI_BASE_ADDRESS_MEM_PREFETCH;
+
+/// 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`.
+    Sriov = bindings::PCI_EXT_CAP_ID_SRIOV as u16,
+}
+
+impl ExtCapId {
+    fn as_raw(self) -> u16 {
+        self as u16
+    }
+}
+
+/// 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.
+    ///
+    /// # Examples
+    ///
+    /// ```no_run
+    /// use kernel::pci;
+    ///
+    /// fn probe_sriov(
+    ///     pdev: &pci::Device<kernel::device::Bound>,
+    /// ) -> Result<(), kernel::error::Error> {
+    ///     let sriov = pdev
+    ///         .config_space_extended()?
+    ///         .find_ext_capability::<pci::ExtSriovRegs>()?;
+    ///
+    ///     let total_vfs = kernel::io_read!(sriov, .total_vfs);
+    ///     let vf_offset = kernel::io_read!(sriov, .vf_offset);
+    ///     let bar0 = sriov.read_vf_bar(0)?;
+    ///     let bar1 = sriov.read_vf_bar(bar0.next_index())?;
+    ///
+    ///     Ok(())
+    /// }
+    /// ```
+    pub fn find_ext_capability<C: ExtCapability>(&self) -> Result<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 Err(ENODEV);
+        }
+
+        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>()
+    }
+
+    /// 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`. If its header
+    /// cannot be read, the capability is treated as the last one.
+    fn calculate_ext_cap_size(&self, offset: usize) -> usize {
+        let header = self.try_read32(offset).unwrap_or(0);
+        // 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;
+
+        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.
+    pub 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],
+    /// 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>;
+
+/// A decoded VF memory BAR.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct ExtSriovVfBar {
+    address: u64,
+    is_64bit: bool,
+    next_index: usize,
+}
+
+impl ExtSriovVfBar {
+    /// Returns the BAR address without PCI attribute bits.
+    #[inline]
+    pub fn address(&self) -> u64 {
+        self.address
+    }
+
+    /// Returns whether the BAR is 64-bit.
+    #[inline]
+    pub fn is_64bit(&self) -> bool {
+        self.is_64bit
+    }
+
+    /// Returns the configuration-space slot index of the next logical BAR.
+    #[inline]
+    pub fn next_index(&self) -> usize {
+        self.next_index
+    }
+}
+
+impl ConfigSpace<'_, ExtSriovRegs> {
+    /// Reads and decodes the VF memory BAR at configuration-space slot `bar_index`.
+    #[inline]
+    pub fn read_vf_bar(&self, bar_index: usize) -> Result<ExtSriovVfBar> {
+        if bar_index >= NUM_VF_BARS {
+            return Err(EINVAL);
+        }
+
+        let low = crate::io_read!(*self, .vf_bar[try: bar_index]);
+        if low & bindings::PCI_BASE_ADDRESS_SPACE != bindings::PCI_BASE_ADDRESS_SPACE_MEMORY {
+            return Err(EINVAL);
+        }
+
+        let is_64bit = low & bindings::PCI_BASE_ADDRESS_MEM_TYPE_MASK
+            == bindings::PCI_BASE_ADDRESS_MEM_TYPE_64;
+        let following_index = bar_index.checked_add(1).ok_or(EINVAL)?;
+
+        let (address, next_index) = if is_64bit {
+            if following_index >= NUM_VF_BARS {
+                return Err(EINVAL);
+            }
+
+            let high = crate::io_read!(*self, .vf_bar[try: following_index]);
+            (
+                (u64::from(high) << 32) | u64::from(low & !VF_MEMORY_BAR_ATTRIBUTE_BITS),
+                following_index.checked_add(1).ok_or(EINVAL)?,
+            )
+        } else {
+            (
+                u64::from(low & !VF_MEMORY_BAR_ATTRIBUTE_BITS),
+                following_index,
+            )
+        };
+
+        Ok(ExtSriovVfBar {
+            address,
+            is_64bit,
+            next_index,
+        })
+    }
+}
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v6 1/1] rust: pci: add extended capability and SR-IOV support
  2026-07-30 18:29 ` [PATCH v6 1/1] rust: pci: add extended capability " Zhi Wang
@ 2026-07-30 18:39   ` sashiko-bot
  2026-07-30 18:45   ` Gary Guo
  2026-07-31 10:35   ` Alexandre Courbot
  2 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-07-30 18:39 UTC (permalink / raw)
  To: Zhi Wang; +Cc: linux-pci, ojeda

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing `#[inline]` attribute on the `ExtCapId::as_raw` abstraction function.
--

--- Patch [1]: [PATCH v6 1/1] rust: pci: add extended capability and SR-IOV support ---
commit 69f27ed67b8e8dbe83e8e431269f12c739076818
Author: Zhi Wang <zhiw@nvidia.com>
Subject: rust: pci: add extended capability and SR-IOV support

This commit adds support for locating and projecting PCIe extended capabilities
in the Rust PCI abstraction. It defines a typed layout for the SR-IOV extended
capability and adds helpers to decode VF BARs.

Link: https://lore.kernel.org/rust-for-linux/20260730180349.771719-1-zhiw@nvidia.com/

> diff --git a/rust/kernel/pci/cap.rs b/rust/kernel/pci/cap.rs
> new file mode 100644
> index 0000000000000..08c044bedb706
> --- /dev/null
> +++ b/rust/kernel/pci/cap.rs

[ ... ]

> +/// 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`.
> +    Sriov = bindings::PCI_EXT_CAP_ID_SRIOV as u16,
> +}
> +
> +impl ExtCapId {
> +    fn as_raw(self) -> u16 {
> +        self as u16
> +    }
> +}

[Severity: Low]
This isn't a bug, but should this function be annotated with #[inline]?

The Rust subsystem guidelines specify that small functions or functions
forwarding to a binding call within abstractions should be annotated with
#[inline].

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730182954.783568-2-zhiw@nvidia.com?part=1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6 1/1] rust: pci: add extended capability and SR-IOV support
  2026-07-30 18:29 ` [PATCH v6 1/1] rust: pci: add extended capability " Zhi Wang
  2026-07-30 18:39   ` sashiko-bot
@ 2026-07-30 18:45   ` Gary Guo
  2026-07-31  9:32     ` Zhi Wang
  2026-07-31  9:57     ` Alexandre Courbot
  2026-07-31 10:35   ` Alexandre Courbot
  2 siblings, 2 replies; 7+ messages in thread
From: Gary Guo @ 2026-07-30 18:45 UTC (permalink / raw)
  To: Zhi Wang, rust-for-linux, linux-pci, linux-kernel
  Cc: dakr, aliceryhl, bhelgaas, kwilczynski, ojeda, boqun, gary,
	bjorn3_gh, lossin, a.hindborg, tmgross, markus.probst, cjia,
	smitra, ankita, aniketa, kwankhede, targupta, kjaju, alkumar,
	acourbot, joelagnelf, jhubbard, zhiwang

On Thu Jul 30, 2026 at 7:29 PM BST, 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 helper that
> returns the address, width, and next configuration-space slot, using
> PCI_SRIOV_NUM_BARS for the number of BAR slots. Since PCI_EXT_CAP_NEXT()
> is a function-like macro, expose it through a Rust helper.
>
> Link: https://lore.kernel.org/rust-for-linux/20260730180349.771719-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 | 236 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 249 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..08c044bedb70
> --- /dev/null
> +++ b/rust/kernel/pci/cap.rs
> @@ -0,0 +1,236 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! PCI extended capability support.
> +
> +use super::{
> +    io::ConfigSpaceBackend,
> +    ConfigSpace,
> +    Extended, //
> +};
> +use crate::{
> +    bindings,
> +    io::{
> +        Io,
> +        IoBackend,
> +        Region, //
> +    },
> +    prelude::*,
> +};
> +
> +/// Number of VF BAR register slots in an SR-IOV capability.
> +// CAST: `PCI_SRIOV_NUM_BARS` is 6, which fits in `usize`.
> +const NUM_VF_BARS: usize = bindings::PCI_SRIOV_NUM_BARS as usize;
> +
> +/// Attribute bits encoded in the low DWORD of a memory BAR.
> +const VF_MEMORY_BAR_ATTRIBUTE_BITS: u32 = bindings::PCI_BASE_ADDRESS_SPACE
> +    | bindings::PCI_BASE_ADDRESS_MEM_TYPE_MASK
> +    | bindings::PCI_BASE_ADDRESS_MEM_PREFETCH;
> +
> +/// 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`.
> +    Sriov = bindings::PCI_EXT_CAP_ID_SRIOV as u16,
> +}
> +
> +impl ExtCapId {
> +    fn as_raw(self) -> u16 {
> +        self as u16
> +    }
> +}
> +
> +/// 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.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```no_run
> +    /// use kernel::pci;
> +    ///
> +    /// fn probe_sriov(
> +    ///     pdev: &pci::Device<kernel::device::Bound>,
> +    /// ) -> Result<(), kernel::error::Error> {
> +    ///     let sriov = pdev
> +    ///         .config_space_extended()?
> +    ///         .find_ext_capability::<pci::ExtSriovRegs>()?;
> +    ///
> +    ///     let total_vfs = kernel::io_read!(sriov, .total_vfs);
> +    ///     let vf_offset = kernel::io_read!(sriov, .vf_offset);
> +    ///     let bar0 = sriov.read_vf_bar(0)?;
> +    ///     let bar1 = sriov.read_vf_bar(bar0.next_index())?;
> +    ///
> +    ///     Ok(())
> +    /// }
> +    /// ```
> +    pub fn find_ext_capability<C: ExtCapability>(&self) -> Result<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 Err(ENODEV);
> +        }
> +
> +        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)?;

The signature should be 

    Result<Option<...>>

where the result is usually handled via `?` and `None` needs to be handled
explicitly, rather than matching on ENODEV.

> +
> +        // 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>()
> +    }
> +
> +    /// 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`. If its header
> +    /// cannot be read, the capability is treated as the last one.
> +    fn calculate_ext_cap_size(&self, offset: usize) -> usize {
> +        let header = self.try_read32(offset).unwrap_or(0);
> +        // 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;
> +
> +        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.
> +    pub 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],
> +    /// 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>;
> +
> +/// A decoded VF memory BAR.
> +#[derive(Debug, Clone, Copy, PartialEq, Eq)]
> +pub struct ExtSriovVfBar {
> +    address: u64,
> +    is_64bit: bool,
> +    next_index: usize,
> +}
> +
> +impl ExtSriovVfBar {
> +    /// Returns the BAR address without PCI attribute bits.
> +    #[inline]
> +    pub fn address(&self) -> u64 {
> +        self.address
> +    }
> +
> +    /// Returns whether the BAR is 64-bit.
> +    #[inline]
> +    pub fn is_64bit(&self) -> bool {
> +        self.is_64bit
> +    }
> +
> +    /// Returns the configuration-space slot index of the next logical BAR.
> +    #[inline]
> +    pub fn next_index(&self) -> usize {
> +        self.next_index
> +    }
> +}
> +
> +impl ConfigSpace<'_, ExtSriovRegs> {
> +    /// Reads and decodes the VF memory BAR at configuration-space slot `bar_index`.
> +    #[inline]
> +    pub fn read_vf_bar(&self, bar_index: usize) -> Result<ExtSriovVfBar> {

Do you expect people to pass in a random index instead of 0 or next_index? If
not, this should be an iterator. Otherwise it'd be possible to index into high
part of 64-bit address.

> +        if bar_index >= NUM_VF_BARS {
> +            return Err(EINVAL);
> +        }
> +
> +        let low = crate::io_read!(*self, .vf_bar[try: bar_index]);

Given the bound checking above I'd use `panic: ` here.

> +        if low & bindings::PCI_BASE_ADDRESS_SPACE != bindings::PCI_BASE_ADDRESS_SPACE_MEMORY {
> +            return Err(EINVAL);
> +        }
> +
> +        let is_64bit = low & bindings::PCI_BASE_ADDRESS_MEM_TYPE_MASK
> +            == bindings::PCI_BASE_ADDRESS_MEM_TYPE_64;
> +        let following_index = bar_index.checked_add(1).ok_or(EINVAL)?;

Just use operator here as it cannot overflow.

> +
> +        let (address, next_index) = if is_64bit {
> +            if following_index >= NUM_VF_BARS {
> +                return Err(EINVAL);
> +            }
> +
> +            let high = crate::io_read!(*self, .vf_bar[try: following_index]);

Same here.

> +            (
> +                (u64::from(high) << 32) | u64::from(low & !VF_MEMORY_BAR_ATTRIBUTE_BITS),
> +                following_index.checked_add(1).ok_or(EINVAL)?,

Same here.

Best,
Gary

> +            )
> +        } else {
> +            (
> +                u64::from(low & !VF_MEMORY_BAR_ATTRIBUTE_BITS),
> +                following_index,
> +            )
> +        };
> +
> +        Ok(ExtSriovVfBar {
> +            address,
> +            is_64bit,
> +            next_index,
> +        })
> +    }
> +}



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6 1/1] rust: pci: add extended capability and SR-IOV support
  2026-07-30 18:45   ` Gary Guo
@ 2026-07-31  9:32     ` Zhi Wang
  2026-07-31  9:57     ` Alexandre Courbot
  1 sibling, 0 replies; 7+ messages in thread
From: Zhi Wang @ 2026-07-31  9:32 UTC (permalink / raw)
  To: Gary Guo
  Cc: rust-for-linux, linux-pci, linux-kernel, dakr, aliceryhl,
	bhelgaas, kwilczynski, ojeda, boqun, bjorn3_gh, lossin,
	a.hindborg, tmgross, markus.probst, cjia, smitra, ankita, aniketa,
	kwankhede, targupta, kjaju, alkumar, acourbot, joelagnelf,
	jhubbard, zhiwang

On Thu, 30 Jul 2026 19:45:05 +0100
"Gary Guo" <gary@garyguo.net> wrote:

snip

> > +
> > +impl ConfigSpace<'_, ExtSriovRegs> {
> > +    /// Reads and decodes the VF memory BAR at configuration-space
> > slot `bar_index`.
> > +    #[inline]
> > +    pub fn read_vf_bar(&self, bar_index: usize) ->
> > Result<ExtSriovVfBar> {
> 
> Do you expect people to pass in a random index instead of 0 or
> next_index? If not, this should be an iterator. Otherwise it'd be
> possible to index into high part of 64-bit address.
> 

Yes, that is exactly what I want to avoid. Looking at the driver again,
it does not need random access, and I was thinking to make the
interface more convenient for the driver and avoid ambiguous numbers
from driver. Alex also suggested an iterator in an earlier review. So
having an interator makes more sense. I will rework this into an
iterator in the next re-spin.

Z.

> > +        if bar_index >= NUM_VF_BARS {
> > +            return Err(EINVAL);
> > +        }
> > +
> > +        let low = crate::io_read!(*self, .vf_bar[try: bar_index]);
> 
> Given the bound checking above I'd use `panic: ` here.
> 
> > +        if low & bindings::PCI_BASE_ADDRESS_SPACE !=
> > bindings::PCI_BASE_ADDRESS_SPACE_MEMORY {
> > +            return Err(EINVAL);
> > +        }
> > +
> > +        let is_64bit = low &
> > bindings::PCI_BASE_ADDRESS_MEM_TYPE_MASK
> > +            == bindings::PCI_BASE_ADDRESS_MEM_TYPE_64;
> > +        let following_index =
> > bar_index.checked_add(1).ok_or(EINVAL)?;
> 
> Just use operator here as it cannot overflow.
> 
> > +
> > +        let (address, next_index) = if is_64bit {
> > +            if following_index >= NUM_VF_BARS {
> > +                return Err(EINVAL);
> > +            }
> > +
> > +            let high = crate::io_read!(*self, .vf_bar[try:
> > following_index]);
> 
> Same here.
> 
> > +            (
> > +                (u64::from(high) << 32) | u64::from(low &
> > !VF_MEMORY_BAR_ATTRIBUTE_BITS),
> > +                following_index.checked_add(1).ok_or(EINVAL)?,
> 
> Same here.
> 
> Best,
> Gary
> 
> > +            )
> > +        } else {
> > +            (
> > +                u64::from(low & !VF_MEMORY_BAR_ATTRIBUTE_BITS),
> > +                following_index,
> > +            )
> > +        };
> > +
> > +        Ok(ExtSriovVfBar {
> > +            address,
> > +            is_64bit,
> > +            next_index,
> > +        })
> > +    }
> > +}
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6 1/1] rust: pci: add extended capability and SR-IOV support
  2026-07-30 18:45   ` Gary Guo
  2026-07-31  9:32     ` Zhi Wang
@ 2026-07-31  9:57     ` Alexandre Courbot
  1 sibling, 0 replies; 7+ messages in thread
From: Alexandre Courbot @ 2026-07-31  9:57 UTC (permalink / raw)
  To: Gary Guo
  Cc: Zhi Wang, rust-for-linux, linux-pci, linux-kernel, dakr,
	aliceryhl, bhelgaas, kwilczynski, ojeda, boqun, bjorn3_gh, lossin,
	a.hindborg, tmgross, markus.probst, cjia, smitra, ankita, aniketa,
	kwankhede, targupta, kjaju, alkumar, joelagnelf, jhubbard,
	zhiwang

On Fri Jul 31, 2026 at 3:45 AM JST, Gary Guo wrote:
> On Thu Jul 30, 2026 at 7:29 PM BST, 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 helper that
>> returns the address, width, and next configuration-space slot, using
>> PCI_SRIOV_NUM_BARS for the number of BAR slots. Since PCI_EXT_CAP_NEXT()
>> is a function-like macro, expose it through a Rust helper.
>>
>> Link: https://lore.kernel.org/rust-for-linux/20260730180349.771719-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 | 236 +++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 249 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..08c044bedb70
>> --- /dev/null
>> +++ b/rust/kernel/pci/cap.rs
>> @@ -0,0 +1,236 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! PCI extended capability support.
>> +
>> +use super::{
>> +    io::ConfigSpaceBackend,
>> +    ConfigSpace,
>> +    Extended, //
>> +};
>> +use crate::{
>> +    bindings,
>> +    io::{
>> +        Io,
>> +        IoBackend,
>> +        Region, //
>> +    },
>> +    prelude::*,
>> +};
>> +
>> +/// Number of VF BAR register slots in an SR-IOV capability.
>> +// CAST: `PCI_SRIOV_NUM_BARS` is 6, which fits in `usize`.
>> +const NUM_VF_BARS: usize = bindings::PCI_SRIOV_NUM_BARS as usize;
>> +
>> +/// Attribute bits encoded in the low DWORD of a memory BAR.
>> +const VF_MEMORY_BAR_ATTRIBUTE_BITS: u32 = bindings::PCI_BASE_ADDRESS_SPACE
>> +    | bindings::PCI_BASE_ADDRESS_MEM_TYPE_MASK
>> +    | bindings::PCI_BASE_ADDRESS_MEM_PREFETCH;
>> +
>> +/// 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`.
>> +    Sriov = bindings::PCI_EXT_CAP_ID_SRIOV as u16,
>> +}
>> +
>> +impl ExtCapId {
>> +    fn as_raw(self) -> u16 {
>> +        self as u16
>> +    }
>> +}
>> +
>> +/// 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.
>> +    ///
>> +    /// # Examples
>> +    ///
>> +    /// ```no_run
>> +    /// use kernel::pci;
>> +    ///
>> +    /// fn probe_sriov(
>> +    ///     pdev: &pci::Device<kernel::device::Bound>,
>> +    /// ) -> Result<(), kernel::error::Error> {
>> +    ///     let sriov = pdev
>> +    ///         .config_space_extended()?
>> +    ///         .find_ext_capability::<pci::ExtSriovRegs>()?;
>> +    ///
>> +    ///     let total_vfs = kernel::io_read!(sriov, .total_vfs);
>> +    ///     let vf_offset = kernel::io_read!(sriov, .vf_offset);
>> +    ///     let bar0 = sriov.read_vf_bar(0)?;
>> +    ///     let bar1 = sriov.read_vf_bar(bar0.next_index())?;
>> +    ///
>> +    ///     Ok(())
>> +    /// }
>> +    /// ```
>> +    pub fn find_ext_capability<C: ExtCapability>(&self) -> Result<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 Err(ENODEV);
>> +        }
>> +
>> +        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)?;
>
> The signature should be 
>
>     Result<Option<...>>
>
> where the result is usually handled via `?` and `None` needs to be handled
> explicitly, rather than matching on ENODEV.
>
>> +
>> +        // 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>()
>> +    }
>> +
>> +    /// 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`. If its header
>> +    /// cannot be read, the capability is treated as the last one.
>> +    fn calculate_ext_cap_size(&self, offset: usize) -> usize {
>> +        let header = self.try_read32(offset).unwrap_or(0);
>> +        // 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;
>> +
>> +        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.
>> +    pub 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],
>> +    /// 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>;
>> +
>> +/// A decoded VF memory BAR.
>> +#[derive(Debug, Clone, Copy, PartialEq, Eq)]
>> +pub struct ExtSriovVfBar {
>> +    address: u64,
>> +    is_64bit: bool,
>> +    next_index: usize,
>> +}
>> +
>> +impl ExtSriovVfBar {
>> +    /// Returns the BAR address without PCI attribute bits.
>> +    #[inline]
>> +    pub fn address(&self) -> u64 {
>> +        self.address
>> +    }
>> +
>> +    /// Returns whether the BAR is 64-bit.
>> +    #[inline]
>> +    pub fn is_64bit(&self) -> bool {
>> +        self.is_64bit
>> +    }
>> +
>> +    /// Returns the configuration-space slot index of the next logical BAR.
>> +    #[inline]
>> +    pub fn next_index(&self) -> usize {
>> +        self.next_index
>> +    }
>> +}
>> +
>> +impl ConfigSpace<'_, ExtSriovRegs> {
>> +    /// Reads and decodes the VF memory BAR at configuration-space slot `bar_index`.
>> +    #[inline]
>> +    pub fn read_vf_bar(&self, bar_index: usize) -> Result<ExtSriovVfBar> {
>
> Do you expect people to pass in a random index instead of 0 or next_index? If
> not, this should be an iterator. Otherwise it'd be possible to index into high
> part of 64-bit address.

I made a similar suggestion on patch 3 [1], and I think Zhi kept the
index for ergonomic reasons. But given that not all indices within range
are valid, an iterator indeed sounds safer.

[1] https://lore.kernel.org/all/DHRTUAF52GNI.1J98TSAG1LS6Q@nvidia.com/

>
>> +        if bar_index >= NUM_VF_BARS {
>> +            return Err(EINVAL);
>> +        }
>> +
>> +        let low = crate::io_read!(*self, .vf_bar[try: bar_index]);
>
> Given the bound checking above I'd use `panic: ` here.

Or better, one could just remove the `if bar_index >= ...` block, and
keep the `try:`. That way it will be used for what is was designed for,
the bounds checking will be done against the actual size of the array
and not a constant that also happens to be used as the array size, and a
dedicated error code will be returned instead of the ubiquitous
`EINVAL`.

>
>> +        if low & bindings::PCI_BASE_ADDRESS_SPACE != bindings::PCI_BASE_ADDRESS_SPACE_MEMORY {
>> +            return Err(EINVAL);
>> +        }
>> +
>> +        let is_64bit = low & bindings::PCI_BASE_ADDRESS_MEM_TYPE_MASK
>> +            == bindings::PCI_BASE_ADDRESS_MEM_TYPE_64;
>> +        let following_index = bar_index.checked_add(1).ok_or(EINVAL)?;
>
> Just use operator here as it cannot overflow.
>
>> +
>> +        let (address, next_index) = if is_64bit {
>> +            if following_index >= NUM_VF_BARS {
>> +                return Err(EINVAL);
>> +            }
>> +
>> +            let high = crate::io_read!(*self, .vf_bar[try: following_index]);
>
> Same here.

And here as well removing the `if` block should do the trick.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6 1/1] rust: pci: add extended capability and SR-IOV support
  2026-07-30 18:29 ` [PATCH v6 1/1] rust: pci: add extended capability " Zhi Wang
  2026-07-30 18:39   ` sashiko-bot
  2026-07-30 18:45   ` Gary Guo
@ 2026-07-31 10:35   ` Alexandre Courbot
  2 siblings, 0 replies; 7+ messages in thread
From: Alexandre Courbot @ 2026-07-31 10:35 UTC (permalink / raw)
  To: Zhi Wang
  Cc: rust-for-linux, linux-pci, linux-kernel, dakr, aliceryhl,
	bhelgaas, kwilczynski, ojeda, boqun, gary, bjorn3_gh, lossin,
	a.hindborg, tmgross, markus.probst, cjia, smitra, ankita, aniketa,
	kwankhede, targupta, kjaju, alkumar, joelagnelf, jhubbard,
	zhiwang

On Fri Jul 31, 2026 at 3:29 AM JST, Zhi Wang wrote:
<...>
> diff --git a/rust/kernel/pci/cap.rs b/rust/kernel/pci/cap.rs
> new file mode 100644
> index 000000000000..08c044bedb70
> --- /dev/null
> +++ b/rust/kernel/pci/cap.rs
> @@ -0,0 +1,236 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! PCI extended capability support.
> +
> +use super::{
> +    io::ConfigSpaceBackend,
> +    ConfigSpace,
> +    Extended, //
> +};
> +use crate::{
> +    bindings,
> +    io::{
> +        Io,
> +        IoBackend,
> +        Region, //
> +    },
> +    prelude::*,
> +};
> +
> +/// Number of VF BAR register slots in an SR-IOV capability.
> +// CAST: `PCI_SRIOV_NUM_BARS` is 6, which fits in `usize`.
> +const NUM_VF_BARS: usize = bindings::PCI_SRIOV_NUM_BARS as usize;
> +
> +/// Attribute bits encoded in the low DWORD of a memory BAR.
> +const VF_MEMORY_BAR_ATTRIBUTE_BITS: u32 = bindings::PCI_BASE_ADDRESS_SPACE
> +    | bindings::PCI_BASE_ADDRESS_MEM_TYPE_MASK
> +    | bindings::PCI_BASE_ADDRESS_MEM_PREFETCH;

This is only used in `read_vf_bar` and can be local to it.

> +
> +/// 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`.
> +    Sriov = bindings::PCI_EXT_CAP_ID_SRIOV as u16,
> +}
> +
> +impl ExtCapId {
> +    fn as_raw(self) -> u16 {
> +        self as u16
> +    }
> +}
> +
> +/// 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.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```no_run
> +    /// use kernel::pci;
> +    ///
> +    /// fn probe_sriov(
> +    ///     pdev: &pci::Device<kernel::device::Bound>,
> +    /// ) -> Result<(), kernel::error::Error> {
> +    ///     let sriov = pdev
> +    ///         .config_space_extended()?
> +    ///         .find_ext_capability::<pci::ExtSriovRegs>()?;
> +    ///
> +    ///     let total_vfs = kernel::io_read!(sriov, .total_vfs);
> +    ///     let vf_offset = kernel::io_read!(sriov, .vf_offset);
> +    ///     let bar0 = sriov.read_vf_bar(0)?;
> +    ///     let bar1 = sriov.read_vf_bar(bar0.next_index())?;
> +    ///
> +    ///     Ok(())
> +    /// }
> +    /// ```
> +    pub fn find_ext_capability<C: ExtCapability>(&self) -> Result<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 Err(ENODEV);
> +        }
> +
> +        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>()
> +    }
> +
> +    /// 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`. If its header
> +    /// cannot be read, the capability is treated as the last one.
> +    fn calculate_ext_cap_size(&self, offset: usize) -> usize {
> +        let header = self.try_read32(offset).unwrap_or(0);

The special behavior on invalid index is intriguing - is this part of
the PCI specification? Or can it return an error if `offset` is out of
bounds?

If we can, I'd probably prefer that as the fallback path relies on
`pci_ext_cap_next` to leave the `0` value untouched, which is not super
obvious. Or if we keep the current behavior, let's at least document
this fallback path a bit more.

> +        // 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;
> +
> +        if next > offset {
> +            next - offset
> +        } else {
> +            (*self).size() - offset

The `try_read32(offset)` above failing means that `offset + 4 >=
(*self).size()`, so this will either underflow, or return a bogus size.
Which is another argument in favor of handling the special case with an
error if the current behavior is not warranted by the spec.

Note also that this can just be `self.size() - offset`, but it's fine if
you want to keep the deref explicit.

> +        }
> +    }
> +}
> +
> +/// 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,
> +    /// 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,
> +}

Side-note: It would be interesting if we could end up representing every
I/O space this way, although padding and keeping the fields offsets
visible would make this challenging. But if we can eventually generalize
this, then I guess we can retire the `register!` macro. :)

> +
> +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>;
> +
> +/// A decoded VF memory BAR.
> +#[derive(Debug, Clone, Copy, PartialEq, Eq)]
> +pub struct ExtSriovVfBar {
> +    address: u64,
> +    is_64bit: bool,
> +    next_index: usize,
> +}
> +
> +impl ExtSriovVfBar {
> +    /// Returns the BAR address without PCI attribute bits.
> +    #[inline]
> +    pub fn address(&self) -> u64 {
> +        self.address
> +    }
> +
> +    /// Returns whether the BAR is 64-bit.
> +    #[inline]
> +    pub fn is_64bit(&self) -> bool {
> +        self.is_64bit
> +    }
> +
> +    /// Returns the configuration-space slot index of the next logical BAR.
> +    #[inline]
> +    pub fn next_index(&self) -> usize {
> +        self.next_index
> +    }
> +}

How about making all the members public (and documenting them)? We won't
be working with mutable values, and that way we can remove this entire
impl block.

> +
> +impl ConfigSpace<'_, ExtSriovRegs> {

Since you have a typed variant defined, let this be

    impl ExtSriovCapability<'_> {

> +    /// Reads and decodes the VF memory BAR at configuration-space slot `bar_index`.
> +    #[inline]
> +    pub fn read_vf_bar(&self, bar_index: usize) -> Result<ExtSriovVfBar> {
> +        if bar_index >= NUM_VF_BARS {
> +            return Err(EINVAL);
> +        }
> +
> +        let low = crate::io_read!(*self, .vf_bar[try: bar_index]);
> +        if low & bindings::PCI_BASE_ADDRESS_SPACE != bindings::PCI_BASE_ADDRESS_SPACE_MEMORY {
> +            return Err(EINVAL);
> +        }
> +
> +        let is_64bit = low & bindings::PCI_BASE_ADDRESS_MEM_TYPE_MASK
> +            == bindings::PCI_BASE_ADDRESS_MEM_TYPE_64;
> +        let following_index = bar_index.checked_add(1).ok_or(EINVAL)?;

Looks like these BAR registers should be bitfields? The `vf_bar` member
will need to stay a regular `u32` because we don't know ahead of time
whether a given entry is the high part of a 64-bit BAR or not, but once
we read the value we could create a typed bitfield from it and avoid
doing the shifting/masking magic ourselves.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-07-31 10:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-30 18:29 [PATCH v6 0/1] Rust PCI capability infrastructure and SR-IOV support Zhi Wang
2026-07-30 18:29 ` [PATCH v6 1/1] rust: pci: add extended capability " Zhi Wang
2026-07-30 18:39   ` sashiko-bot
2026-07-30 18:45   ` Gary Guo
2026-07-31  9:32     ` Zhi Wang
2026-07-31  9:57     ` Alexandre Courbot
2026-07-31 10:35   ` Alexandre Courbot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.