Re: [PATCH v3 2/8] rust: pci: add sriov_get_totalvfs() helper

Zhi Wang <[email protected]> Thu, 9 Jul 2026 17:24:27 +0300
Newsgroups dev.linux.lists.nova-gpu,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci
Message-ID <20260709172427.5e9fa20a@inno-dell>
On Wed, 08 Jul 2026 23:13:08 +0900
"Alexandre Courbot" <[email protected]> wrote:

> On Wed Jul 8, 2026 at 11:03 PM JST, Alexandre Courbot wrote:
> > On Wed Jul 1, 2026 at 3:26 PM JST, Zhi Wang wrote:
> >> Expose pci_sriov_get_totalvfs() to Rust PCI drivers so they can
> >> query how many SR-IOV VFs a device supports.
> >>
> >> Cc: Alexandre Courbot <[email protected]>
> >> Cc: Bjorn Helgaas <[email protected]>
> >> Cc: David Laight <[email protected]>
> >> Cc: Gary Guo <[email protected]>
> >> Cc: [email protected]
> >> Link:
> >> https://lore.kernel.org/all/[email protected]/
> >> Signed-off-by: Zhi Wang <[email protected]> ---
> >>  rust/kernel/pci.rs | 11 +++++++++++
> >>  1 file changed, 11 insertions(+)
> >>
> >> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> >> index 5071cae6543f..21c51981c02e 100644
> >> --- a/rust/kernel/pci.rs
> >> +++ b/rust/kernel/pci.rs
> >> @@ -450,6 +450,17 @@ pub fn pci_class(&self) -> Class {
> >>          // SAFETY: `self.as_raw` is a valid pointer to a `struct
> >> pci_dev`. Class::from_raw(unsafe { (*self.as_raw()).class })
> >>      }
> >> +
> >> +    /// Returns the total number of VFs, or 0 if SR-IOV is not
> >> available.
> >> +    #[inline]
> >> +    pub fn sriov_get_totalvfs(&self) -> u16 {
> >> +        // SAFETY: `self.as_raw()` is a valid pointer to a
> >> `struct pci_dev`.
> >> +        let total_vfs = unsafe {
> >> bindings::pci_sriov_get_totalvfs(self.as_raw()) }; +
> >> +        // CAST: The C helper returns `unsigned int`, but the
> >> value originates
> >> +        // from TotalVFs/driver_max_VFs, so this cast cannot
> >> truncate.
> >
> > nit: "from TotalVFs/driver_max_VFs (which are defined as `u16`),
> > ..."
> >
> > With that,
> >
> > Reviewed-by: Alexandre Courbot <[email protected]>
> 
> ... with an additional item for thought: in a previous revision [1], I
> had raised the question of making the return type
> `Option<NonZero<u16>>`, as I think it better encodes "0 if SR-IOV is
> not available" - callers are then forced to consider the dichotomy of
> the result, and not interpret a particular value as having a special
> meaning.
> 
> OTOH, for all practical purposes 0 VFs also seems to be strictly
> equivalent to "SR-IOV is not available", so please take this as a
> nit to consider, not a blocker.
> 
> [1]
> https://lore.kernel.org/rust-for-linux/[email protected]/

Nice advice. I will address it in v4. :)