Re: [PATCH 1/9] rust: pci: expose sriov_get_totalvfs() helper

[Zhi Wang <zhiw@nvidia.com>]

On Fri, 05 Jun 2026 23:08:38 +0900
"Alexandre Courbot" <acourbot@nvidia.com> wrote:

> On Thu Jun 4, 2026 at 8:43 PM JST, Zhi Wang wrote:
> > Add a wrapper for the `pci_sriov_get_totalvfs()` helper, allowing
> > drivers to query the number of total SR-IOV virtual functions a PCI
> > device supports.
> >
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: linux-pci@vger.kernel.org
> > Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> > ---
> >  rust/kernel/pci.rs | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> > index 5071cae6543f..d04e5f6841f2 100644
> > --- a/rust/kernel/pci.rs
> > +++ b/rust/kernel/pci.rs
> > @@ -450,6 +450,18 @@ 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 total number of VFs, or `Err(ENODEV)` if none
> > available.
> > +    pub fn sriov_get_totalvfs(&self) -> Result<i32> {
> 
> I mentioned it in a previous review [1], but I think there is an
> opportunity to improve the C API (and by transition the Rust one) by
> making it return a `u16`, which is the type the number of total VFs is
> ultimately stored in anyway.
> 
> IIRC there is also no need to even update any caller, just changing
> the prototype of `pci_sriov_get_totalvfs` would be enough as callers
> ultimately compare the return value against u16s. So if anything it
> would make the C API more sound.
> 
> [1] https://lore.kernel.org/all/DETDILPA1GFY.27WND0TEC5352@nvidia.com/
> 
> > +        // SAFETY: `self.as_raw()` is a valid pointer to a `struct
> > pci_dev`.
> > +        let vfs = unsafe {
> > bindings::pci_sriov_get_totalvfs(self.as_raw()) }; +
> > +        if vfs == 0 {
> > +            return Err(ENODEV);
> > +        }
> 
> Having 0 VFs does not necessarily look like an error - it's quite a
> valid answer to the question "how many VFs do we have?" which this
> method tries to answer. In patch 7 you even do `unwrap_or(0)` on the
> result of this method. So unless there is a good reason to treat this
> as an error, maybe we can just return a `u16` here.

I will add it in the next re-spin. Sorry that I missed it.