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

[Alexandre Courbot]

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@xxxxxxxxxx>
> Cc: linux-pci@xxxxxxxxxxxxxxx
> Signed-off-by: Zhi Wang <zhiw@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxx/

> +        // 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.