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

On Thu Jun 25, 2026 at 8:11 AM JST, Gary Guo wrote:
> On Wed Jun 24, 2026 at 8:38 PM BST, David Laight wrote:
>> On Wed, 24 Jun 2026 23:59:56 +0900
>> "Alexandre Courbot" <[email protected]> wrote:
>>
>>> On Wed Jun 24, 2026 at 10:39 PM JST, David Laight wrote:
>>> > On Wed, 24 Jun 2026 21:40:52 +0900
>>> > "Alexandre Courbot" <[email protected]> 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 <[email protected]>
>>> >> > Cc: [email protected]
>>> >> > Signed-off-by: Zhi Wang <[email protected]>    
>>> >> 
>>> >> Suggested-by: Alexandre Courbot <[email protected]>
>>> >> Link: https://lore.kernel.org/all/[email protected]/
>>> >>   
>>> >> > ---
>>> >> >  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`?
>
> There're no value in having two versions with just slightly different promotion
> behaviour. Rust side can still use u16 by casting from unsigned int without
> having to expose both.

Indeed, that occurred to me only after I sent my reply. Since the
register this value originates from is 16 bits, we can justify that a
cast of the return to u16 won't lose any data.

Just making the C API return unsigned int is fine then.