Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH v2 1/7] PCI/IOV: Return u16 from pci_sriov_get_totalvfs()
       [not found] <20260622194353.1308872-1-zhiw@nvidia.com>
@ 2026-06-22 19:43 ` Zhi Wang
  2026-06-24 12:40   ` Alexandre Courbot
  2026-06-22 19:43 ` [PATCH v2 2/7] rust: pci: Add sriov_get_totalvfs() helper Zhi Wang
  1 sibling, 1 reply; 6+ messages in thread
From: Zhi Wang @ 2026-06-22 19:43 UTC (permalink / raw)
  To: dakr, acourbot
  Cc: airlied, simona, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross, jhubbard, ecourtney,
	joelagnelf, apopple, cjia, smitra, kjaju, alkumar, ankita,
	aniketa, kwankhede, targupta, nova-gpu, linux-kernel, zhiwang,
	Zhi Wang, Bjorn Helgaas, linux-pci

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@google.com>
Cc: linux-pci@vger.kernel.org
Signed-off-by: Zhi Wang <zhiw@nvidia.com>
---
 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 +-
 drivers/pci/iov.c                               | 2 +-
 drivers/vdpa/octeon_ep/octep_vdpa_main.c        | 4 ++--
 include/linux/pci.h                             | 4 ++--
 11 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/crypto/hisilicon/qm.c b/drivers/crypto/hisilicon/qm.c
index 3ca47e2a9719..13b45fcb582b 100644
--- a/drivers/crypto/hisilicon/qm.c
+++ b/drivers/crypto/hisilicon/qm.c
@@ -3878,7 +3878,8 @@ static int qm_func_shaper_enable(struct hisi_qm *qm, u32 fun_index, u32 qos)
 	struct device *dev = &qm->pdev->dev;
 	struct qm_shaper_factor t_factor;
 	u32 ir = qos * QM_QOS_RATE;
-	int ret, total_vfs, i;
+	u16 total_vfs;
+	int ret, i;
 
 	total_vfs = pci_sriov_get_totalvfs(qm->pdev);
 	if (fun_index > total_vfs)
@@ -4188,7 +4189,8 @@ static void hisi_qm_init_vf_qos(struct hisi_qm *qm, int total_func)
 int hisi_qm_sriov_enable(struct pci_dev *pdev, int max_vfs)
 {
 	struct hisi_qm *qm = pci_get_drvdata(pdev);
-	int pre_existing_vfs, num_vfs, total_vfs, ret;
+	int pre_existing_vfs, num_vfs, ret;
+	u16 total_vfs;
 
 	ret = qm_pm_get_sync(qm);
 	if (ret)
@@ -4203,7 +4205,7 @@ int hisi_qm_sriov_enable(struct pci_dev *pdev, int max_vfs)
 	}
 
 	if (max_vfs > total_vfs) {
-		pci_err(pdev, "%d VFs is more than total VFs %d!\n", max_vfs, total_vfs);
+		pci_err(pdev, "%d VFs is more than total VFs %u!\n", max_vfs, total_vfs);
 		ret = -ERANGE;
 		goto err_put_sync;
 	}
diff --git a/drivers/crypto/intel/qat/qat_common/adf_sriov.c b/drivers/crypto/intel/qat/qat_common/adf_sriov.c
index 8bf0fe1fcb4d..317b80c1772f 100644
--- a/drivers/crypto/intel/qat/qat_common/adf_sriov.c
+++ b/drivers/crypto/intel/qat/qat_common/adf_sriov.c
@@ -52,7 +52,7 @@ void adf_schedule_vf2pf_handler(struct adf_accel_vf_info *vf_info)
 static int adf_enable_sriov(struct adf_accel_dev *accel_dev)
 {
 	struct pci_dev *pdev = accel_to_pci_dev(accel_dev);
-	int totalvfs = pci_sriov_get_totalvfs(pdev);
+	u16 totalvfs = pci_sriov_get_totalvfs(pdev);
 	struct adf_hw_device_data *hw_data = accel_dev->hw_device;
 	struct adf_accel_vf_info *vf_info;
 	int i;
@@ -148,7 +148,7 @@ static int adf_do_disable_sriov(struct adf_accel_dev *accel_dev)
 static int adf_do_enable_sriov(struct adf_accel_dev *accel_dev)
 {
 	struct pci_dev *pdev = accel_to_pci_dev(accel_dev);
-	int totalvfs = pci_sriov_get_totalvfs(pdev);
+	u16 totalvfs = pci_sriov_get_totalvfs(pdev);
 	unsigned long val;
 	int ret;
 
@@ -237,7 +237,7 @@ void adf_reenable_sriov(struct adf_accel_dev *accel_dev)
 void adf_disable_sriov(struct adf_accel_dev *accel_dev)
 {
 	struct adf_hw_device_data *hw_data = accel_dev->hw_device;
-	int totalvfs = pci_sriov_get_totalvfs(accel_to_pci_dev(accel_dev));
+	u16 totalvfs = pci_sriov_get_totalvfs(accel_to_pci_dev(accel_dev));
 	struct adf_accel_vf_info *vf;
 	int i;
 
diff --git a/drivers/gpu/drm/xe/xe_sriov_pf.c b/drivers/gpu/drm/xe/xe_sriov_pf.c
index 33bd754d138f..1a1c4978e5f3 100644
--- a/drivers/gpu/drm/xe/xe_sriov_pf.c
+++ b/drivers/gpu/drm/xe/xe_sriov_pf.c
@@ -58,10 +58,8 @@ bool xe_sriov_pf_readiness(struct xe_device *xe)
 {
 	struct device *dev = xe->drm.dev;
 	struct pci_dev *pdev = to_pci_dev(dev);
-	int totalvfs = pci_sriov_get_totalvfs(pdev);
-	int newlimit = min_t(u16, wanted_max_vfs(xe), totalvfs);
-
-	xe_assert(xe, totalvfs <= U16_MAX);
+	u16 totalvfs = pci_sriov_get_totalvfs(pdev);
+	u16 newlimit = min_t(u16, wanted_max_vfs(xe), totalvfs);
 
 	if (!dev_is_pf(dev))
 		return false;
diff --git a/drivers/misc/genwqe/card_base.c b/drivers/misc/genwqe/card_base.c
index 86bfa82723ff..259d2f9c9416 100644
--- a/drivers/misc/genwqe/card_base.c
+++ b/drivers/misc/genwqe/card_base.c
@@ -360,10 +360,10 @@ static bool genwqe_setup_vf_jtimer(struct genwqe_dev *cd)
 	unsigned int vf;
 	u32 T = genwqe_T_psec(cd);
 	u64 x;
-	int totalvfs;
+	u16 totalvfs;
 
 	totalvfs = pci_sriov_get_totalvfs(pci_dev);
-	if (totalvfs <= 0)
+	if (!totalvfs)
 		return false;
 
 	for (vf = 0; vf < totalvfs; vf++) {
@@ -1132,8 +1132,6 @@ static int genwqe_pci_setup(struct genwqe_dev *cd)
 	}
 
 	cd->num_vfs = pci_sriov_get_totalvfs(pci_dev);
-	if (cd->num_vfs < 0)
-		cd->num_vfs = 0;
 
 	err = genwqe_read_ids(cd);
 	if (err)
diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c
index 0ec65ec634df..33faa40381d3 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -98,7 +98,7 @@ static u64 nic_reg_read(struct nicpf *nic, u64 offset)
 /* PF -> VF mailbox communication APIs */
 static void nic_enable_mbx_intr(struct nicpf *nic)
 {
-	int vf_cnt = pci_sriov_get_totalvfs(nic->pdev);
+	u16 vf_cnt = pci_sriov_get_totalvfs(nic->pdev);
 
 #define INTR_MASK(vfs) ((vfs < 64) ? (BIT_ULL(vfs) - 1) : (~0ull))
 
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index ed302f5ec476..21cc5113ab92 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -4441,7 +4441,8 @@ static void be_calculate_pf_pool_rss_tables(struct be_adapter *adapter)
 static int be_get_sriov_config(struct be_adapter *adapter)
 {
 	struct be_resources res = {0};
-	int max_vfs, old_vfs;
+	u16 max_vfs;
+	int old_vfs;
 
 	be_cmd_get_profile_config(adapter, &res, NULL, ACTIVE_PROFILE_TYPE,
 				  RESOURCE_LIMITS, 0);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sriov.c b/drivers/net/ethernet/mellanox/mlx5/core/sriov.c
index bf6f631cf2ce..7087f1a4d364 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/sriov.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/sriov.c
@@ -298,7 +298,8 @@ int mlx5_sriov_init(struct mlx5_core_dev *dev)
 {
 	struct mlx5_core_sriov *sriov = &dev->priv.sriov;
 	struct pci_dev *pdev = dev->pdev;
-	int total_vfs, i;
+	u16 total_vfs;
+	int i;
 
 	if (!mlx5_core_is_pf(dev))
 		return 0;
diff --git a/drivers/net/ethernet/sfc/ef10_sriov.c b/drivers/net/ethernet/sfc/ef10_sriov.c
index f98f1707d1a9..5b339b055761 100644
--- a/drivers/net/ethernet/sfc/ef10_sriov.c
+++ b/drivers/net/ethernet/sfc/ef10_sriov.c
@@ -263,7 +263,7 @@ int efx_ef10_vswitching_probe_pf(struct efx_nic *efx)
 	struct net_device *net_dev = efx->net_dev;
 	int rc;
 
-	if (pci_sriov_get_totalvfs(efx->pci_dev) <= 0) {
+	if (!pci_sriov_get_totalvfs(efx->pci_dev)) {
 		/* vswitch not needed as we have no VFs */
 		efx_ef10_vadaptor_alloc_set_features(efx);
 		return 0;
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 91ac4e37ecb9..db9828bfff52 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -1277,7 +1277,7 @@ EXPORT_SYMBOL_GPL(pci_sriov_set_totalvfs);
  * SRIOV capability value of TotalVFs or the value of driver_max_VFs
  * if the driver reduced it.  Otherwise 0.
  */
-int pci_sriov_get_totalvfs(struct pci_dev *dev)
+u16 pci_sriov_get_totalvfs(struct pci_dev *dev)
 {
 	if (!dev->is_physfn)
 		return 0;
diff --git a/drivers/vdpa/octeon_ep/octep_vdpa_main.c b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
index 31a02e7fd7f2..4b032b757ee9 100644
--- a/drivers/vdpa/octeon_ep/octep_vdpa_main.c
+++ b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
@@ -796,13 +796,13 @@ static int octep_vdpa_pf_setup(struct octep_pf *octpf)
 {
 	u8 __iomem *addr = octpf->base[OCTEP_HW_MBOX_BAR];
 	struct pci_dev *pdev = octpf->pdev;
-	int totalvfs;
+	u16 totalvfs;
 	size_t len;
 	u64 val;
 
 	totalvfs = pci_sriov_get_totalvfs(pdev);
 	if (unlikely(!totalvfs)) {
-		dev_info(&pdev->dev, "Total VFs are %d in PF sriov configuration\n", totalvfs);
+		dev_info(&pdev->dev, "Total VFs are %u in PF sriov configuration\n", totalvfs);
 		return 0;
 	}
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 2c4454583c11..1994a88afbc2 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2538,7 +2538,7 @@ void pci_iov_remove_virtfn(struct pci_dev *dev, int id);
 int pci_num_vf(struct pci_dev *dev);
 int pci_vfs_assigned(struct pci_dev *dev);
 int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs);
-int pci_sriov_get_totalvfs(struct pci_dev *dev);
+u16 pci_sriov_get_totalvfs(struct pci_dev *dev);
 int pci_sriov_configure_simple(struct pci_dev *dev, int nr_virtfn);
 resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno);
 int pci_iov_vf_bar_set_size(struct pci_dev *dev, int resno, int size);
@@ -2590,7 +2590,7 @@ static inline int pci_vfs_assigned(struct pci_dev *dev)
 { return 0; }
 static inline int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs)
 { return 0; }
-static inline int pci_sriov_get_totalvfs(struct pci_dev *dev)
+static inline u16 pci_sriov_get_totalvfs(struct pci_dev *dev)
 { return 0; }
 #define pci_sriov_configure_simple	NULL
 static inline resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno)
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/7] rust: pci: Add sriov_get_totalvfs() helper
       [not found] <20260622194353.1308872-1-zhiw@nvidia.com>
  2026-06-22 19:43 ` [PATCH v2 1/7] PCI/IOV: Return u16 from pci_sriov_get_totalvfs() Zhi Wang
@ 2026-06-22 19:43 ` Zhi Wang
  1 sibling, 0 replies; 6+ messages in thread
From: Zhi Wang @ 2026-06-22 19:43 UTC (permalink / raw)
  To: dakr, acourbot
  Cc: airlied, simona, ojeda, alex.gaynor, boqun.feng, gary, bjorn3_gh,
	lossin, a.hindborg, aliceryhl, tmgross, jhubbard, ecourtney,
	joelagnelf, apopple, cjia, smitra, kjaju, alkumar, ankita,
	aniketa, kwankhede, targupta, nova-gpu, linux-kernel, zhiwang,
	Zhi Wang, Bjorn Helgaas, linux-pci

Expose pci_sriov_get_totalvfs() to Rust PCI drivers so they can query
how many SR-IOV VFs a device supports.

The C helper now returns u16 and reports 0 when SR-IOV is unavailable
or the device is not a PF, so mirror that API directly instead of
modeling 0 VFs as an error.

Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Signed-off-by: Zhi Wang <zhiw@nvidia.com>
---
 rust/kernel/pci.rs | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 5071cae6543f..adcb99fd0a2e 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -450,6 +450,12 @@ 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.
+    pub fn sriov_get_totalvfs(&self) -> u16 {
+        // SAFETY: `self.as_raw()` is a valid pointer to a `struct pci_dev`.
+        unsafe { bindings::pci_sriov_get_totalvfs(self.as_raw()) }
+    }
 }
 
 impl<'a> Device<device::Core<'a>> {
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/7] PCI/IOV: Return u16 from pci_sriov_get_totalvfs()
  2026-06-22 19:43 ` [PATCH v2 1/7] PCI/IOV: Return u16 from pci_sriov_get_totalvfs() Zhi Wang
@ 2026-06-24 12:40   ` Alexandre Courbot
  2026-06-24 13:39     ` David Laight
  0 siblings, 1 reply; 6+ messages in thread
From: Alexandre Courbot @ 2026-06-24 12:40 UTC (permalink / raw)
  To: Zhi Wang
  Cc: dakr, airlied, simona, ojeda, alex.gaynor, boqun.feng, gary,
	bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
	ecourtney, joelagnelf, apopple, cjia, smitra, kjaju, alkumar,
	ankita, aniketa, kwankhede, targupta, nova-gpu, linux-kernel,
	zhiwang, Bjorn Helgaas, linux-pci

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@google.com>
> Cc: linux-pci@vger.kernel.org
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>

Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
Link: https://lore.kernel.org/all/DETDILPA1GFY.27WND0TEC5352@nvidia.com/

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

I.e. just updating drivers/pci/iov.c and include/linux/pci.h should be
sufficient as the first step. Specific drivers can then be updated using
separate patches that will be easier to merge individually, if you want
to do so.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/7] PCI/IOV: Return u16 from pci_sriov_get_totalvfs()
  2026-06-24 12:40   ` Alexandre Courbot
@ 2026-06-24 13:39     ` David Laight
  2026-06-24 14:59       ` Alexandre Courbot
  0 siblings, 1 reply; 6+ messages in thread
From: David Laight @ 2026-06-24 13:39 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: Zhi Wang, dakr, airlied, simona, ojeda, alex.gaynor, boqun.feng,
	gary, bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
	ecourtney, joelagnelf, apopple, cjia, smitra, kjaju, alkumar,
	ankita, aniketa, kwankhede, targupta, nova-gpu, linux-kernel,
	zhiwang, Bjorn Helgaas, linux-pci

On Wed, 24 Jun 2026 21:40:52 +0900
"Alexandre Courbot" <acourbot@nvidia.com> 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@google.com>
> > Cc: linux-pci@vger.kernel.org
> > Signed-off-by: Zhi Wang <zhiw@nvidia.com>  
> 
> Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
> Link: https://lore.kernel.org/all/DETDILPA1GFY.27WND0TEC5352@nvidia.com/
> 
> > ---
> >  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.

	David

> 
> I.e. just updating drivers/pci/iov.c and include/linux/pci.h should be
> sufficient as the first step. Specific drivers can then be updated using
> separate patches that will be easier to merge individually, if you want
> to do so.
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/7] PCI/IOV: Return u16 from pci_sriov_get_totalvfs()
  2026-06-24 13:39     ` David Laight
@ 2026-06-24 14:59       ` Alexandre Courbot
  2026-06-24 19:38         ` David Laight
  0 siblings, 1 reply; 6+ messages in thread
From: Alexandre Courbot @ 2026-06-24 14:59 UTC (permalink / raw)
  To: David Laight
  Cc: Zhi Wang, dakr, airlied, simona, ojeda, alex.gaynor, boqun.feng,
	gary, bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
	ecourtney, joelagnelf, apopple, cjia, smitra, kjaju, alkumar,
	ankita, aniketa, kwankhede, targupta, nova-gpu, linux-kernel,
	zhiwang, Bjorn Helgaas, linux-pci

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@nvidia.com> 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@google.com>
>> > Cc: linux-pci@vger.kernel.org
>> > Signed-off-by: Zhi Wang <zhiw@nvidia.com>  
>> 
>> Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
>> Link: https://lore.kernel.org/all/DETDILPA1GFY.27WND0TEC5352@nvidia.com/
>> 
>> > ---
>> >  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`?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/7] PCI/IOV: Return u16 from pci_sriov_get_totalvfs()
  2026-06-24 14:59       ` Alexandre Courbot
@ 2026-06-24 19:38         ` David Laight
  0 siblings, 0 replies; 6+ messages in thread
From: David Laight @ 2026-06-24 19:38 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: Zhi Wang, dakr, airlied, simona, ojeda, alex.gaynor, boqun.feng,
	gary, bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, jhubbard,
	ecourtney, joelagnelf, apopple, cjia, smitra, kjaju, alkumar,
	ankita, aniketa, kwankhede, targupta, nova-gpu, linux-kernel,
	zhiwang, Bjorn Helgaas, linux-pci

On Wed, 24 Jun 2026 23:59:56 +0900
"Alexandre Courbot" <acourbot@nvidia.com> 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" <acourbot@nvidia.com> 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@google.com>
> >> > Cc: linux-pci@vger.kernel.org
> >> > Signed-off-by: Zhi Wang <zhiw@nvidia.com>    
> >> 
> >> Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
> >> Link: https://lore.kernel.org/all/DETDILPA1GFY.27WND0TEC5352@nvidia.com/
> >>   
> >> > ---
> >> >  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`?

That is getting silly, something will end up doing the masking to police
the 16bit value - it isn't just the C rules, the ABI pass/return u16 in
in 32bit registers (even on x86) so one side (or both) has to mask the
value.

Your rust code has the same problem. Add 1 to a u16 variable and code has
to be added to mask the result to 16 bits.
You won't see it on x86 because it has 16bit alu operations, about the
only other cpu eoth them is m68k (I'm not sure about s390 - it is based
on a very old instructions set).

	David

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-24 19:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260622194353.1308872-1-zhiw@nvidia.com>
2026-06-22 19:43 ` [PATCH v2 1/7] PCI/IOV: Return u16 from pci_sriov_get_totalvfs() Zhi Wang
2026-06-24 12:40   ` Alexandre Courbot
2026-06-24 13:39     ` David Laight
2026-06-24 14:59       ` Alexandre Courbot
2026-06-24 19:38         ` David Laight
2026-06-22 19:43 ` [PATCH v2 2/7] rust: pci: Add sriov_get_totalvfs() helper Zhi Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox