* [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; 8+ 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] 8+ 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:45 ` Gary Guo 2026-07-31 10:35 ` Alexandre Courbot 0 siblings, 2 replies; 8+ 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] 8+ 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:45 ` Gary Guo 2026-07-31 9:32 ` Zhi Wang 2026-07-31 9:57 ` Alexandre Courbot 2026-07-31 10:35 ` Alexandre Courbot 1 sibling, 2 replies; 8+ 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] 8+ 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; 8+ 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] 8+ 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 2026-07-31 12:38 ` Gary Guo 1 sibling, 1 reply; 8+ 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] 8+ messages in thread
* Re: [PATCH v6 1/1] rust: pci: add extended capability and SR-IOV support 2026-07-31 9:57 ` Alexandre Courbot @ 2026-07-31 12:38 ` Gary Guo 2026-07-31 12:52 ` Alexandre Courbot 0 siblings, 1 reply; 8+ messages in thread From: Gary Guo @ 2026-07-31 12:38 UTC (permalink / raw) To: Alexandre Courbot, 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 10:57 AM BST, Alexandre Courbot wrote: > 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/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`. `projection::OutOfBound` converts to `ERANGE` so it'll be different. Also, when this is converted to iterators, for the first case you'd probably want to just return `None` and for the second case log an error that the device is having out of spec capability. Best, Gary ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v6 1/1] rust: pci: add extended capability and SR-IOV support 2026-07-31 12:38 ` Gary Guo @ 2026-07-31 12:52 ` Alexandre Courbot 0 siblings, 0 replies; 8+ messages in thread From: Alexandre Courbot @ 2026-07-31 12:52 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 9:38 PM JST, Gary Guo wrote: > On Fri Jul 31, 2026 at 10:57 AM BST, Alexandre Courbot wrote: >> 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/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`. > > `projection::OutOfBound` converts to `ERANGE` so it'll be different. Also, when > this is converted to iterators, for the first case you'd probably want to just > return `None` and for the second case log an error that the device is having out > of spec capability. Yeah iterators are probably going to make this moot anyway. ^ permalink raw reply [flat|nested] 8+ 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:45 ` Gary Guo @ 2026-07-31 10:35 ` Alexandre Courbot 1 sibling, 0 replies; 8+ 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] 8+ messages in thread
end of thread, other threads:[~2026-07-31 12:52 UTC | newest] Thread overview: 8+ 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:45 ` Gary Guo 2026-07-31 9:32 ` Zhi Wang 2026-07-31 9:57 ` Alexandre Courbot 2026-07-31 12:38 ` Gary Guo 2026-07-31 12:52 ` Alexandre Courbot 2026-07-31 10:35 ` Alexandre Courbot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox