Re: [PATCH v2 1/7] PCI/IOV: Return u16 from pci_sriov_get_totalvfs()

[Alexandre Courbot]

On Wed Jun 24, 2026 at 10:39 PM JST, David Laight wrote:
> On Wed, 24 Jun 2026 21:40:52 +0900
> "Alexandre Courbot" <acourbot@xxxxxxxxxx> wrote:
>
>> On Tue Jun 23, 2026 at 4:43 AM JST, Zhi Wang wrote:
>> > pci_sriov_get_totalvfs() reports a VF count, not an errno-style
>> > status. It returns 0 when SR-IOV is unavailable or the device is not a
>> > PF, and otherwise returns the PF's driver_max_VFs value.
>> >
>> > driver_max_VFs is stored as a u16 in struct pci_sriov. It is derived
>> > from the SR-IOV TotalVFs field or from a driver-provided limit, so the
>> > implementation cannot return a negative value.
>> >
>> > Change the declaration, CONFIG_PCI_IOV stub, and implementation to
>> > return u16. Update callers to store the result in u16 variables, remove
>> > obsolete negative-value checks, and use unsigned format specifiers where
>> > needed.
>> >
>> > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
>> > Cc: linux-pci@xxxxxxxxxxxxxxx
>> > Signed-off-by: Zhi Wang <zhiw@xxxxxxxxxx>  
>> 
>> Suggested-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
>> Link: https://lore.kernel.org/all/DETDILPA1GFY.27WND0TEC5352@xxxxxxxxxx/
>> 
>> > ---
>> >  drivers/crypto/hisilicon/qm.c                   | 8 +++++---
>> >  drivers/crypto/intel/qat/qat_common/adf_sriov.c | 6 +++---
>> >  drivers/gpu/drm/xe/xe_sriov_pf.c                | 6 ++----
>> >  drivers/misc/genwqe/card_base.c                 | 6 ++----
>> >  drivers/net/ethernet/cavium/thunder/nic_main.c  | 2 +-
>> >  drivers/net/ethernet/emulex/benet/be_main.c     | 3 ++-
>> >  drivers/net/ethernet/mellanox/mlx5/core/sriov.c | 3 ++-
>> >  drivers/net/ethernet/sfc/ef10_sriov.c           | 2 +-  
>> 
>> I believe that you can avoid converting all these drivers in this patch.
>> The implicit `u16 -> int` conversion done by C should result in the
>> expected behavior, and it will be fewer Acked-by to collect.
>
> The generated code is also likely to be slightly better if the function
> return value is a 32bit value.
>
> Similarly you don't really want to do any kind of maths on local variables
> that aren't 32bit (or 64bit on 64bit builds).
>
> The fact that the domain of a value fits in 16 bits doesn't mean that
> it is better to use u16 - it is usually worse.
> Pretty much the only place u16 should be used is to reduce the size
> of structures.
>
> So it is probably correct to change the return type to unsigned int and
> remove the error return checks, but nothing else.

For C, I agree that unsigned int is the safest type.

Rust otoh does not do implicit integer promotion, and making it return a
`u16` carries useful range information. I wonder if we could have a
private `__pci_sriov_get_totalvfs` that returns a `u16`, make
`pci_sriov_get_totalvfs` promote it to an `unsigned int` and return it,
while the Rust bindings would invoke `__pci_sriov_get_totalvfs` so they
can expose a `u16`?