From: Leon Romanovsky <leon@kernel.org> Date: 2021-01-14 10:32:32
From: Leon Romanovsky <leonro@nvidia.com>
Changelog
v2:
* Patch 1:
* Renamed vf_msix_vec sysfs knob to be sriov_vf_msix_count
* Added PF and VF device locks during set MSI-X call to protect from parallel
driver bind/unbind operations.
* Removed extra checks when reading sriov_vf_msix, because users will
be able to distinguish between supported/not supported by looking on
sriov_vf_total_msix count.
* Changed all occurrences of "numb" to be "count"
* Changed returned error from EOPNOTSUPP to be EBUSY if user tries to set
MSI-X count after driver already bound to the VF.
* Added extra comment in pci_set_msix_vec_count() to emphasize that driver
should not be bound.
* Patch 2:
* Chaged vf_total_msix from int to be u32 and updated function signatures
accordingly.
* Improved patch title
v1: https://lore.kernel.org/linux-pci/20210110150727.1965295-1-leon@kernel.org
* Improved wording and commit messages of first PCI patch
* Added extra PCI patch to provide total number of MSI-X vectors
* Prohibited read of vf_msix_vec sysfs file if driver doesn't support write
* Removed extra function definition in pci.h
v0: https://lore.kernel.org/linux-pci/20210103082440.34994-1-leon@kernel.org
--------------------------------------------------------------------
Hi,
The number of MSI-X vectors is PCI property visible through lspci, that
field is read-only and configured by the device.
The static assignment of an amount of MSI-X vectors doesn't allow utilize
the newly created VF because it is not known to the device the future load
and configuration where that VF will be used.
The VFs are created on the hypervisor and forwarded to the VMs that have
different properties (for example number of CPUs).
To overcome the inefficiency in the spread of such MSI-X vectors, we
allow the kernel to instruct the device with the needed number of such
vectors, before VF is initialized and bounded to the driver.
Before this series:
[root@server ~]# lspci -vs 0000:08:00.2
08:00.2 Ethernet controller: Mellanox Technologies MT27800 Family [ConnectX-5 Virtual Function]
....
Capabilities: [9c] MSI-X: Enable- Count=12 Masked-
Configuration script:
1. Start fresh
echo 0 > /sys/bus/pci/devices/0000\:08\:00.0/sriov_numvfs
modprobe -q -r mlx5_ib mlx5_core
2. Ensure that driver doesn't run and it is safe to change MSI-X
echo 0 > /sys/bus/pci/devices/0000\:08\:00.0/sriov_drivers_autoprobe
3. Load driver for the PF
modprobe mlx5_core
4. Configure one of the VFs with new number
echo 2 > /sys/bus/pci/devices/0000\:08\:00.0/sriov_numvfs
echo 21 > /sys/bus/pci/devices/0000\:08\:00.2/sriov_vf_msix_count
After this series:
[root@server ~]# lspci -vs 0000:08:00.2
08:00.2 Ethernet controller: Mellanox Technologies MT27800 Family [ConnectX-5 Virtual Function]
....
Capabilities: [9c] MSI-X: Enable- Count=21 Masked-
Thanks
Leon Romanovsky (5):
PCI: Add sysfs callback to allow MSI-X table size change of SR-IOV VFs
PCI: Add SR-IOV sysfs entry to read total number of dynamic MSI-X
vectors
net/mlx5: Add dynamic MSI-X capabilities bits
net/mlx5: Dynamically assign MSI-X vectors count
net/mlx5: Allow to the users to configure number of MSI-X vectors
Documentation/ABI/testing/sysfs-bus-pci | 34 +++++++
.../net/ethernet/mellanox/mlx5/core/main.c | 5 ++
.../ethernet/mellanox/mlx5/core/mlx5_core.h | 6 ++
.../net/ethernet/mellanox/mlx5/core/pci_irq.c | 62 +++++++++++++
.../net/ethernet/mellanox/mlx5/core/sriov.c | 52 ++++++++++-
drivers/pci/iov.c | 89 +++++++++++++++++++
drivers/pci/msi.c | 47 ++++++++++
drivers/pci/pci-sysfs.c | 1 +
drivers/pci/pci.h | 5 ++
include/linux/mlx5/mlx5_ifc.h | 11 ++-
include/linux/pci.h | 5 ++
11 files changed, 314 insertions(+), 3 deletions(-)
--
2.29.2
From: Leon Romanovsky <leon@kernel.org> Date: 2021-01-14 10:33:00
From: Leon Romanovsky <leonro@nvidia.com>
Some SR-IOV capable devices provide an ability to configure specific
number of MSI-X vectors on their VF prior driver is probed on that VF.
In order to make management easy, provide new read-only sysfs file that
returns a total number of possible to configure MSI-X vectors.
cat /sys/bus/pci/devices/.../sriov_vf_total_msix
= 0 - feature is not supported
> 0 - total number of MSI-X vectors to consume by the VFs
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
Documentation/ABI/testing/sysfs-bus-pci | 14 +++++++++++
drivers/pci/iov.c | 31 +++++++++++++++++++++++++
drivers/pci/pci.h | 3 +++
include/linux/pci.h | 2 ++
4 files changed, 50 insertions(+)
@@ -395,3 +395,17 @@ Description: The file is writable if the PF is bound to a driver that set sriov_vf_total_msix > 0 and there is no driver bound to the VF.++What: /sys/bus/pci/devices/.../sriov_vf_total_msix+Date: January 2021+Contact: Leon Romanovsky <leonro@nvidia.com>+Description:+ This file is associated with the SR-IOV PFs.+ It returns a total number of possible to configure MSI-X+ vectors on the enabled VFs.++ The values returned are:+ * > 0 - this will be total number possible to consume by VFs,+ * = 0 - feature is not supported++ If no SR-IOV VFs are enabled, this value will return 0.
@@ -327,6 +327,9 @@ struct pci_sriov {u16subsystem_device;/* VF subsystem device */resource_size_tbarsz[PCI_SRIOV_NUM_BARS];/* VF BAR size */booldrivers_autoprobe;/* Auto probing of VFs by driver */+u32vf_total_msix;/* Total number of MSI-X vectors the VFs+*canconsume+*/};/**
@@ -2075,6 +2075,7 @@ int pci_sriov_get_totalvfs(struct pci_dev *dev);intpci_sriov_configure_simple(structpci_dev*dev,intnr_virtfn);resource_size_tpci_iov_resource_size(structpci_dev*dev,intresno);voidpci_vf_drivers_autoprobe(structpci_dev*dev,boolprobe);+voidpci_sriov_set_vf_total_msix(structpci_dev*dev,u32count);/* Arch may override these (weak) */intpcibios_sriov_enable(structpci_dev*pdev,u16num_vfs);
From: Leon Romanovsky <leon@kernel.org> Date: 2021-01-14 10:33:00
From: Leon Romanovsky <leonro@nvidia.com>
The number of MSI-X vectors is PCI property visible through lspci, that
field is read-only and configured by the device. The static assignment
of an amount of MSI-X vectors doesn't allow utilize the newly created
VF because it is not known to the device the future load and configuration
where that VF will be used.
To overcome the inefficiency in the spread of such MSI-X vectors, we
allow the kernel to instruct the device with the needed number of such
vectors.
Such change immediately increases the amount of MSI-X vectors for the
system with @ VFs from 12 vectors per-VF, to be 32 vectors per-VF.
Before this patch:
[root@server ~]# lspci -vs 0000:08:00.2
08:00.2 Ethernet controller: Mellanox Technologies MT27800 Family [ConnectX-5 Virtual Function]
....
Capabilities: [9c] MSI-X: Enable- Count=12 Masked-
After this patch:
[root@server ~]# lspci -vs 0000:08:00.2
08:00.2 Ethernet controller: Mellanox Technologies MT27800 Family [ConnectX-5 Virtual Function]
....
Capabilities: [9c] MSI-X: Enable- Count=32 Masked-
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
.../net/ethernet/mellanox/mlx5/core/main.c | 4 ++
.../ethernet/mellanox/mlx5/core/mlx5_core.h | 5 ++
.../net/ethernet/mellanox/mlx5/core/pci_irq.c | 62 +++++++++++++++++++
.../net/ethernet/mellanox/mlx5/core/sriov.c | 14 ++++-
4 files changed, 83 insertions(+), 2 deletions(-)
@@ -172,6 +172,11 @@ int mlx5_irq_attach_nb(struct mlx5_irq_table *irq_table, int vecidx,structnotifier_block*nb);intmlx5_irq_detach_nb(structmlx5_irq_table*irq_table,intvecidx,structnotifier_block*nb);++intmlx5_set_msix_vec_count(structmlx5_core_dev*dev,intdevfn,+intmsix_vec_count);+intmlx5_get_default_msix_vec_count(structmlx5_core_dev*dev,intnum_vfs);+structcpumask*mlx5_irq_get_affinity_mask(structmlx5_irq_table*irq_table,intvecidx);structcpu_rmap*mlx5_irq_get_rmap(structmlx5_irq_table*table);
@@ -71,8 +71,7 @@ static int sriov_restore_guids(struct mlx5_core_dev *dev, int vf)staticintmlx5_device_enable_sriov(structmlx5_core_dev*dev,intnum_vfs){structmlx5_core_sriov*sriov=&dev->priv.sriov;-interr;-intvf;+interr,vf,num_msix_count;if(!MLX5_ESWITCH_MANAGER(dev))gotoenable_vfs_hca;
@@ -85,12 +84,23 @@ static int mlx5_device_enable_sriov(struct mlx5_core_dev *dev, int num_vfs)}enable_vfs_hca:+num_msix_count=mlx5_get_default_msix_vec_count(dev,num_vfs);for(vf=0;vf<num_vfs;vf++){err=mlx5_core_enable_hca(dev,vf+1);if(err){mlx5_core_warn(dev,"failed to enable VF %d (%d)\n",vf,err);continue;}++err=mlx5_set_msix_vec_count(dev,vf+1,num_msix_count);+if(err){+mlx5_core_warn(+dev,+"failed to set MSI-X vector counts VF %d, err %d\n",+vf,err);+continue;+}+sriov->vfs_ctx[vf].enabled=1;if(MLX5_CAP_GEN(dev,port_type)==MLX5_CAP_PORT_TYPE_IB){err=sriov_restore_guids(dev,vf);
From: Leon Romanovsky <leon@kernel.org> Date: 2021-01-14 10:33:01
From: Leon Romanovsky <leonro@nvidia.com>
These new fields declare the number of MSI-X vectors that is
possible to allocate on the VF through PF configuration.
Value must be in range defined by min_dynamic_vf_msix_table_size
and max_dynamic_vf_msix_table_size.
The driver should continue to query its MSI-X table through PCI
configuration header.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
include/linux/mlx5/mlx5_ifc.h | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
@@ -59,7 +59,7 @@ int mlx5_get_default_msix_vec_count(struct mlx5_core_dev *dev, int num_vfs){intnum_vf_msix,min_msix,max_msix;-num_vf_msix=MLX5_CAP_GEN(dev,num_total_dynamic_vf_msix);+num_vf_msix=MLX5_CAP_GEN_MAX(dev,num_total_dynamic_vf_msix);if(!num_vf_msix)return0;
@@ -83,7 +83,7 @@ int mlx5_set_msix_vec_count(struct mlx5_core_dev *dev, int function_id,void*hca_cap,*cap;intret;-num_vf_msix=MLX5_CAP_GEN(dev,num_total_dynamic_vf_msix);+num_vf_msix=MLX5_CAP_GEN_MAX(dev,num_total_dynamic_vf_msix);if(!num_vf_msix)return0;
@@ -153,6 +154,8 @@ static int mlx5_sriov_enable(struct pci_dev *pdev, int num_vfs)returnerr;}+num_vf_msix=MLX5_CAP_GEN_MAX(dev,num_total_dynamic_vf_msix);+pci_sriov_set_vf_total_msix(pdev,num_vf_msix);err=pci_enable_sriov(pdev,num_vfs);if(err){mlx5_core_warn(dev,"pci_enable_sriov failed : %d\n",err);
@@ -188,6 +191,41 @@ int mlx5_core_sriov_configure(struct pci_dev *pdev, int num_vfs)returnerr?err:num_vfs;}+intmlx5_core_sriov_set_msix_vec_count(structpci_dev*vf,intmsix_vec_count)+{+structpci_dev*pf=pci_physfn(vf);+structmlx5_core_sriov*sriov;+structmlx5_core_dev*dev;+intnum_vf_msix,id;++dev=pci_get_drvdata(pf);+num_vf_msix=MLX5_CAP_GEN_MAX(dev,num_total_dynamic_vf_msix);+if(!num_vf_msix)+return-EOPNOTSUPP;++if(!msix_vec_count)+msix_vec_count=+mlx5_get_default_msix_vec_count(dev,pci_num_vf(pf));++sriov=&dev->priv.sriov;++/* Reversed translation of PCI VF function number to the internal+*function_id,whichexistsinthenameofvirtfnsymlink.+*/+for(id=0;id<pci_num_vf(pf);id++){+if(!sriov->vfs_ctx[id].enabled)+continue;++if(vf->devfn==pci_iov_virtfn_devfn(pf,id))+break;+}++if(id==pci_num_vf(pf)||!sriov->vfs_ctx[id].enabled)+return-EINVAL;++returnmlx5_set_msix_vec_count(dev,id+1,msix_vec_count);+}+intmlx5_sriov_attach(structmlx5_core_dev*dev){if(!mlx5_core_is_pf(dev)||!pci_num_vf(dev->pdev))
From: Leon Romanovsky <leon@kernel.org> Date: 2021-01-14 10:33:10
From: Leon Romanovsky <leonro@nvidia.com>
Extend PCI sysfs interface with a new callback that allows configure
the number of MSI-X vectors for specific SR-IO VF. This is needed
to optimize the performance of newly bound devices by allocating
the number of vectors based on the administrator knowledge of targeted VM.
This function is applicable for SR-IOV VF because such devices allocate
their MSI-X table before they will run on the VMs and HW can't guess the
right number of vectors, so the HW allocates them statically and equally.
The newly added /sys/bus/pci/devices/.../sriov_vf_msix_count file will be seen
for the VFs and it is writable as long as a driver is not bounded to the VF.
The values accepted are:
* > 0 - this will be number reported by the VF's MSI-X capability
* < 0 - not valid
* = 0 - will reset to the device default value
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
Documentation/ABI/testing/sysfs-bus-pci | 20 +++++++++
drivers/pci/iov.c | 58 +++++++++++++++++++++++++
drivers/pci/msi.c | 47 ++++++++++++++++++++
drivers/pci/pci-sysfs.c | 1 +
drivers/pci/pci.h | 2 +
include/linux/pci.h | 3 ++
6 files changed, 131 insertions(+)
@@ -375,3 +375,23 @@ Description: The value comes from the PCI kernel device state and can be one of: "unknown", "error", "D0", D1", "D2", "D3hot", "D3cold". The file is read only.++What: /sys/bus/pci/devices/.../sriov_vf_msix_count+Date: December 2020+Contact: Leon Romanovsky <leonro@nvidia.com>+Description:+ This file is associated with the SR-IOV VFs.+ It allows configuration of the number of MSI-X vectors for+ the VF. This is needed to optimize performance of newly bound+ devices by allocating the number of vectors based on the+ administrator knowledge of targeted VM.++ The values accepted are:+ * > 0 - this will be number reported by the VF's MSI-X+ capability+ * < 0 - not valid+ * = 0 - will reset to the device default value++ The file is writable if the PF is bound to a driver that+ set sriov_vf_total_msix > 0 and there is no driver bound+ to the VF.
@@ -31,6 +31,7 @@ int pci_iov_virtfn_devfn(struct pci_dev *dev, int vf_id)return(dev->devfn+dev->sriov->offset+dev->sriov->stride*vf_id)&0xff;}+EXPORT_SYMBOL(pci_iov_virtfn_devfn);/**PerSR-IOVspecsec3.3.10and3.3.11,FirstVFOffsetandVFStridemay
From: Jakub Kicinski <kuba@kernel.org> Date: 2021-01-14 17:52:39
On Thu, 14 Jan 2021 12:31:35 +0200 Leon Romanovsky wrote:
The number of MSI-X vectors is PCI property visible through lspci, that
field is read-only and configured by the device.
The static assignment of an amount of MSI-X vectors doesn't allow utilize
the newly created VF because it is not known to the device the future load
and configuration where that VF will be used.
The VFs are created on the hypervisor and forwarded to the VMs that have
different properties (for example number of CPUs).
To overcome the inefficiency in the spread of such MSI-X vectors, we
allow the kernel to instruct the device with the needed number of such
vectors, before VF is initialized and bounded to the driver.
From: Alex Williamson <hidden> Date: 2021-01-15 00:07:11
On Thu, 14 Jan 2021 12:31:37 +0200
Leon Romanovsky [off-list ref] wrote:
quoted hunk
From: Leon Romanovsky <leonro@nvidia.com>
Some SR-IOV capable devices provide an ability to configure specific
number of MSI-X vectors on their VF prior driver is probed on that VF.
In order to make management easy, provide new read-only sysfs file that
returns a total number of possible to configure MSI-X vectors.
cat /sys/bus/pci/devices/.../sriov_vf_total_msix
= 0 - feature is not supported
> 0 - total number of MSI-X vectors to consume by the VFs
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
Documentation/ABI/testing/sysfs-bus-pci | 14 +++++++++++
drivers/pci/iov.c | 31 +++++++++++++++++++++++++
drivers/pci/pci.h | 3 +++
include/linux/pci.h | 2 ++
4 files changed, 50 insertions(+)
@@ -395,3 +395,17 @@ Description: The file is writable if the PF is bound to a driver that set sriov_vf_total_msix > 0 and there is no driver bound to the VF.++What: /sys/bus/pci/devices/.../sriov_vf_total_msix+Date: January 2021+Contact: Leon Romanovsky <leonro@nvidia.com>+Description:+ This file is associated with the SR-IOV PFs.+ It returns a total number of possible to configure MSI-X+ vectors on the enabled VFs.++ The values returned are:+ * > 0 - this will be total number possible to consume by VFs,+ * = 0 - feature is not supported
As with previous, why expose it if not supported?
This seems pretty challenging for userspace to use; aiui they would
need to iterate all the VFs to learn how many vectors are already
allocated, subtract that number from this value, all while hoping they
aren't racing someone else doing the same. Would it be more useful if
this reported the number of surplus vectors available?
How would a per VF limit be exposed? Do we expect users to know the
absolutely MSI-X vector limit or the device specific limit? Thanks,
Alex
quoted hunk
+
+ If no SR-IOV VFs are enabled, this value will return 0.
@@ -327,6 +327,9 @@ struct pci_sriov {u16subsystem_device;/* VF subsystem device */resource_size_tbarsz[PCI_SRIOV_NUM_BARS];/* VF BAR size */booldrivers_autoprobe;/* Auto probing of VFs by driver */+u32vf_total_msix;/* Total number of MSI-X vectors the VFs+*canconsume+*/};/**
@@ -2075,6 +2075,7 @@ int pci_sriov_get_totalvfs(struct pci_dev *dev);intpci_sriov_configure_simple(structpci_dev*dev,intnr_virtfn);resource_size_tpci_iov_resource_size(structpci_dev*dev,intresno);voidpci_vf_drivers_autoprobe(structpci_dev*dev,boolprobe);+voidpci_sriov_set_vf_total_msix(structpci_dev*dev,u32count);/* Arch may override these (weak) */intpcibios_sriov_enable(structpci_dev*pdev,u16num_vfs);
From: Alex Williamson <hidden> Date: 2021-01-15 00:07:17
On Thu, 14 Jan 2021 12:31:36 +0200
Leon Romanovsky [off-list ref] wrote:
quoted hunk
From: Leon Romanovsky <leonro@nvidia.com>
Extend PCI sysfs interface with a new callback that allows configure
the number of MSI-X vectors for specific SR-IO VF. This is needed
to optimize the performance of newly bound devices by allocating
the number of vectors based on the administrator knowledge of targeted VM.
This function is applicable for SR-IOV VF because such devices allocate
their MSI-X table before they will run on the VMs and HW can't guess the
right number of vectors, so the HW allocates them statically and equally.
The newly added /sys/bus/pci/devices/.../sriov_vf_msix_count file will be seen
for the VFs and it is writable as long as a driver is not bounded to the VF.
The values accepted are:
* > 0 - this will be number reported by the VF's MSI-X capability
* < 0 - not valid
* = 0 - will reset to the device default value
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
Documentation/ABI/testing/sysfs-bus-pci | 20 +++++++++
drivers/pci/iov.c | 58 +++++++++++++++++++++++++
drivers/pci/msi.c | 47 ++++++++++++++++++++
drivers/pci/pci-sysfs.c | 1 +
drivers/pci/pci.h | 2 +
include/linux/pci.h | 3 ++
6 files changed, 131 insertions(+)
@@ -375,3 +375,23 @@ Description: The value comes from the PCI kernel device state and can be one of: "unknown", "error", "D0", D1", "D2", "D3hot", "D3cold". The file is read only.++What: /sys/bus/pci/devices/.../sriov_vf_msix_count+Date: December 2020+Contact: Leon Romanovsky <leonro@nvidia.com>+Description:+ This file is associated with the SR-IOV VFs.+ It allows configuration of the number of MSI-X vectors for+ the VF. This is needed to optimize performance of newly bound+ devices by allocating the number of vectors based on the+ administrator knowledge of targeted VM.++ The values accepted are:+ * > 0 - this will be number reported by the VF's MSI-X+ capability+ * < 0 - not valid+ * = 0 - will reset to the device default value++ The file is writable if the PF is bound to a driver that+ set sriov_vf_total_msix > 0 and there is no driver bound+ to the VF.
@@ -31,6 +31,7 @@ int pci_iov_virtfn_devfn(struct pci_dev *dev, int vf_id)return(dev->devfn+dev->sriov->offset+dev->sriov->stride*vf_id)&0xff;}+EXPORT_SYMBOL(pci_iov_virtfn_devfn);/**PerSR-IOVspecsec3.3.10and3.3.11,FirstVFOffsetandVFStridemay
Wouldn't it be cleaner to also hide this on VFs where
pci_msix_vec_count() returns an error or where the PF driver doesn't
implement .sriov_set_msix_vec_count()? IOW, expose it only where it
could actually work.
@@ -991,6 +991,53 @@ int pci_msix_vec_count(struct pci_dev *dev)}EXPORT_SYMBOL(pci_msix_vec_count);+/**+*pci_set_msix_vec_count-changethereportednumberofMSI-Xvectors+*ThisfunctionisapplicableforSR-IOVVFbecausesuchdevicesallocate+*theirMSI-XtablebeforetheywillrunontheVMsandHWcan'tguessthe+*rightnumberofvectors,sotheHWallocatesthemstaticallyandequally.
Nit, this is an assumption of the VF usage and conjecture of the
implementation.
+ * @dev: VF device that is going to be changed
+ * @count amount of MSI-X vectors
+ **/
+int pci_set_msix_vec_count(struct pci_dev *dev, int count)
pci_vf_set_msix_vec_count()? Long, I know, but if it's limited to VFs
name it accordingly. Thanks,
Alex
quoted hunk
+{
+ struct pci_dev *pdev = pci_physfn(dev);
+ int ret;
+
+ if (!dev->msix_cap || !pdev->msix_cap || count < 0)
+ /*
+ * We don't support negative numbers for now,
+ * but maybe in the future it will make sense.
+ */
+ return -EINVAL;
+
+ device_lock(&pdev->dev);
+ if (!pdev->driver || !pdev->driver->sriov_set_msix_vec_count) {
+ ret = -EOPNOTSUPP;
+ goto err_pdev;
+ }
+
+ device_lock(&dev->dev);
+ if (dev->driver) {
+ /*
+ * Driver already probed this VF and configured itself
+ * based on previously configured (or default) MSI-X vector
+ * count. It is too late to change this field for this
+ * specific VF.
+ */
+ ret = -EBUSY;
+ goto err_dev;
+ }
+
+ ret = pdev->driver->sriov_set_msix_vec_count(dev, count);
+
+err_dev:
+ device_unlock(&dev->dev);
+err_pdev:
+ device_unlock(&pdev->dev);
+ return ret;
+}
+
static int __pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries,
int nvec, struct irq_affinity *affd, int flags)
{
From: Leon Romanovsky <leon@kernel.org> Date: 2021-01-16 08:24:33
On Thu, Jan 14, 2021 at 05:05:43PM -0700, Alex Williamson wrote:
On Thu, 14 Jan 2021 12:31:36 +0200
Leon Romanovsky [off-list ref] wrote:
quoted
From: Leon Romanovsky <leonro@nvidia.com>
Extend PCI sysfs interface with a new callback that allows configure
the number of MSI-X vectors for specific SR-IO VF. This is needed
to optimize the performance of newly bound devices by allocating
the number of vectors based on the administrator knowledge of targeted VM.
This function is applicable for SR-IOV VF because such devices allocate
their MSI-X table before they will run on the VMs and HW can't guess the
right number of vectors, so the HW allocates them statically and equally.
The newly added /sys/bus/pci/devices/.../sriov_vf_msix_count file will be seen
for the VFs and it is writable as long as a driver is not bounded to the VF.
The values accepted are:
* > 0 - this will be number reported by the VF's MSI-X capability
* < 0 - not valid
* = 0 - will reset to the device default value
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
Documentation/ABI/testing/sysfs-bus-pci | 20 +++++++++
drivers/pci/iov.c | 58 +++++++++++++++++++++++++
drivers/pci/msi.c | 47 ++++++++++++++++++++
drivers/pci/pci-sysfs.c | 1 +
drivers/pci/pci.h | 2 +
include/linux/pci.h | 3 ++
6 files changed, 131 insertions(+)
@@ -375,3 +375,23 @@ Description: The value comes from the PCI kernel device state and can be one of: "unknown", "error", "D0", D1", "D2", "D3hot", "D3cold". The file is read only.++What: /sys/bus/pci/devices/.../sriov_vf_msix_count+Date: December 2020+Contact: Leon Romanovsky <leonro@nvidia.com>+Description:+ This file is associated with the SR-IOV VFs.+ It allows configuration of the number of MSI-X vectors for+ the VF. This is needed to optimize performance of newly bound+ devices by allocating the number of vectors based on the+ administrator knowledge of targeted VM.++ The values accepted are:+ * > 0 - this will be number reported by the VF's MSI-X+ capability+ * < 0 - not valid+ * = 0 - will reset to the device default value++ The file is writable if the PF is bound to a driver that+ set sriov_vf_total_msix > 0 and there is no driver bound+ to the VF.
@@ -31,6 +31,7 @@ int pci_iov_virtfn_devfn(struct pci_dev *dev, int vf_id)return(dev->devfn+dev->sriov->offset+dev->sriov->stride*vf_id)&0xff;}+EXPORT_SYMBOL(pci_iov_virtfn_devfn);/**PerSR-IOVspecsec3.3.10and3.3.11,FirstVFOffsetandVFStridemay
Wouldn't it be cleaner to also hide this on VFs where
pci_msix_vec_count() returns an error or where the PF driver doesn't
implement .sriov_set_msix_vec_count()? IOW, expose it only where it
could actually work.
I wasn't sure about the policy in PCI/core, but sure will change.
@@ -991,6 +991,53 @@ int pci_msix_vec_count(struct pci_dev *dev)}EXPORT_SYMBOL(pci_msix_vec_count);+/**+*pci_set_msix_vec_count-changethereportednumberofMSI-Xvectors+*ThisfunctionisapplicableforSR-IOVVFbecausesuchdevicesallocate+*theirMSI-XtablebeforetheywillrunontheVMsandHWcan'tguessthe+*rightnumberofvectors,sotheHWallocatesthemstaticallyandequally.
Nit, this is an assumption of the VF usage and conjecture of the
implementation.
This is one of the possible implementations.
quoted
+ * @dev: VF device that is going to be changed
+ * @count amount of MSI-X vectors
+ **/
+int pci_set_msix_vec_count(struct pci_dev *dev, int count)
pci_vf_set_msix_vec_count()? Long, I know, but if it's limited to VFs
name it accordingly. Thanks,
I'll do.
Alex
quoted
+{
+ struct pci_dev *pdev = pci_physfn(dev);
+ int ret;
+
+ if (!dev->msix_cap || !pdev->msix_cap || count < 0)
+ /*
+ * We don't support negative numbers for now,
+ * but maybe in the future it will make sense.
+ */
+ return -EINVAL;
+
+ device_lock(&pdev->dev);
+ if (!pdev->driver || !pdev->driver->sriov_set_msix_vec_count) {
+ ret = -EOPNOTSUPP;
+ goto err_pdev;
+ }
+
+ device_lock(&dev->dev);
+ if (dev->driver) {
+ /*
+ * Driver already probed this VF and configured itself
+ * based on previously configured (or default) MSI-X vector
+ * count. It is too late to change this field for this
+ * specific VF.
+ */
+ ret = -EBUSY;
+ goto err_dev;
+ }
+
+ ret = pdev->driver->sriov_set_msix_vec_count(dev, count);
+
+err_dev:
+ device_unlock(&dev->dev);
+err_pdev:
+ device_unlock(&pdev->dev);
+ return ret;
+}
+
static int __pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries,
int nvec, struct irq_affinity *affd, int flags)
{
From: Leon Romanovsky <leon@kernel.org> Date: 2021-01-16 08:38:01
On Thu, Jan 14, 2021 at 05:05:36PM -0700, Alex Williamson wrote:
On Thu, 14 Jan 2021 12:31:37 +0200
Leon Romanovsky [off-list ref] wrote:
quoted
From: Leon Romanovsky <leonro@nvidia.com>
Some SR-IOV capable devices provide an ability to configure specific
number of MSI-X vectors on their VF prior driver is probed on that VF.
In order to make management easy, provide new read-only sysfs file that
returns a total number of possible to configure MSI-X vectors.
cat /sys/bus/pci/devices/.../sriov_vf_total_msix
= 0 - feature is not supported
> 0 - total number of MSI-X vectors to consume by the VFs
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
Documentation/ABI/testing/sysfs-bus-pci | 14 +++++++++++
drivers/pci/iov.c | 31 +++++++++++++++++++++++++
drivers/pci/pci.h | 3 +++
include/linux/pci.h | 2 ++
4 files changed, 50 insertions(+)
@@ -395,3 +395,17 @@ Description: The file is writable if the PF is bound to a driver that set sriov_vf_total_msix > 0 and there is no driver bound to the VF.++What: /sys/bus/pci/devices/.../sriov_vf_total_msix+Date: January 2021+Contact: Leon Romanovsky <leonro@nvidia.com>+Description:+ This file is associated with the SR-IOV PFs.+ It returns a total number of possible to configure MSI-X+ vectors on the enabled VFs.++ The values returned are:+ * > 0 - this will be total number possible to consume by VFs,+ * = 0 - feature is not supported
As with previous, why expose it if not supported?
It is much simpler to the users implement logic that operates
accordingly to this value instead of relying on exist/not-exist and
anyway handle 0 to be on the safe side.
This seems pretty challenging for userspace to use; aiui they would
need to iterate all the VFs to learn how many vectors are already
allocated, subtract that number from this value, all while hoping they
aren't racing someone else doing the same. Would it be more useful if
this reported the number of surplus vectors available?
Only privileged users are allowed to do it, so it is unlikely that we
will have more than one entity which manages PFs/VFs assignments.
Users already count number of CPUs they give to the VMs, so counting
resources is not new to them.
I didn't count in the kernel because it will require from users to
understand and treat "0" differently to understand that the pool is
depleted. So they will need to count max size of the pool anyway.
Unless we want to have two knobs, one of max and another for current,
they will count. The thing is that users will count anyway and won't
use the current value. It gives nothing.
How would a per VF limit be exposed? Do we expect users to know the
absolutely MSI-X vector limit or the device specific limit? Thanks,
At this stage yes, we can discuss it later when the need will arise.
Thanks
From: Leon Romanovsky <leon@kernel.org> Date: 2021-01-17 05:47:41
On Thu, Jan 14, 2021 at 09:51:28AM -0800, Jakub Kicinski wrote:
On Thu, 14 Jan 2021 12:31:35 +0200 Leon Romanovsky wrote:
quoted
The number of MSI-X vectors is PCI property visible through lspci, that
field is read-only and configured by the device.
The static assignment of an amount of MSI-X vectors doesn't allow utilize
the newly created VF because it is not known to the device the future load
and configuration where that VF will be used.
The VFs are created on the hypervisor and forwarded to the VMs that have
different properties (for example number of CPUs).
To overcome the inefficiency in the spread of such MSI-X vectors, we
allow the kernel to instruct the device with the needed number of such
vectors, before VF is initialized and bounded to the driver.
From: Leon Romanovsky <leon@kernel.org> Date: 2021-01-17 07:06:00
On Sat, Jan 16, 2021 at 10:23:31AM +0200, Leon Romanovsky wrote:
On Thu, Jan 14, 2021 at 05:05:43PM -0700, Alex Williamson wrote:
quoted
On Thu, 14 Jan 2021 12:31:36 +0200
Leon Romanovsky [off-list ref] wrote:
quoted
From: Leon Romanovsky <leonro@nvidia.com>
Extend PCI sysfs interface with a new callback that allows configure
the number of MSI-X vectors for specific SR-IO VF. This is needed
to optimize the performance of newly bound devices by allocating
the number of vectors based on the administrator knowledge of targeted VM.
This function is applicable for SR-IOV VF because such devices allocate
their MSI-X table before they will run on the VMs and HW can't guess the
right number of vectors, so the HW allocates them statically and equally.
The newly added /sys/bus/pci/devices/.../sriov_vf_msix_count file will be seen
for the VFs and it is writable as long as a driver is not bounded to the VF.
The values accepted are:
* > 0 - this will be number reported by the VF's MSI-X capability
* < 0 - not valid
* = 0 - will reset to the device default value
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
Documentation/ABI/testing/sysfs-bus-pci | 20 +++++++++
drivers/pci/iov.c | 58 +++++++++++++++++++++++++
drivers/pci/msi.c | 47 ++++++++++++++++++++
drivers/pci/pci-sysfs.c | 1 +
drivers/pci/pci.h | 2 +
include/linux/pci.h | 3 ++
6 files changed, 131 insertions(+)
Wouldn't it be cleaner to also hide this on VFs where
pci_msix_vec_count() returns an error or where the PF driver doesn't
implement .sriov_set_msix_vec_count()? IOW, expose it only where it
could actually work.
I wasn't sure about the policy in PCI/core, but sure will change.
I ended adding checks of msix_cap, but can't check .sriov_set_msix_vec_count.
The latter will require to hold device_lock on PF that can disappear later, it
is too racy.
Thanks
From: Leon Romanovsky <leon@kernel.org> Date: 2021-01-17 07:26:55
On Sun, Jan 17, 2021 at 07:44:09AM +0200, Leon Romanovsky wrote:
On Thu, Jan 14, 2021 at 09:51:28AM -0800, Jakub Kicinski wrote:
quoted
On Thu, 14 Jan 2021 12:31:35 +0200 Leon Romanovsky wrote:
quoted
The number of MSI-X vectors is PCI property visible through lspci, that
field is read-only and configured by the device.
The static assignment of an amount of MSI-X vectors doesn't allow utilize
the newly created VF because it is not known to the device the future load
and configuration where that VF will be used.
The VFs are created on the hypervisor and forwarded to the VMs that have
different properties (for example number of CPUs).
To overcome the inefficiency in the spread of such MSI-X vectors, we
allow the kernel to instruct the device with the needed number of such
vectors, before VF is initialized and bounded to the driver.
Thanks Jakub,
I'll add kdocs to internal mlx5 functions.
IMHO, they are useless.
At the end, it looks like CI false alarm.
drivers/net/ethernet/mellanox/mlx5/core/pci_irq.c:81: warning: Function parameter or member 'dev' not described in 'mlx5_set_msix_vec_count'
drivers/net/ethernet/mellanox/mlx5/core/pci_irq.c:81: warning: Function parameter or member 'function_id' not described in 'mlx5_set_msix_vec_count'
drivers/net/ethernet/mellanox/mlx5/core/pci_irq.c:81: warning: Function parameter or member 'msix_vec_count' not described in 'mlx5_set_msix_vec_count'
New warnings added
The function mlx5_set_msix_vec_count() is documented.
+/**
+ * mlx5_set_msix_vec_count() - Set dynamically allocated MSI-X to the VF
+ * @dev - PF to work on
+ * @function_id - internal PCI VF function id
+ * @msix_vec_count - Number of MSI-X to set
+ **/
+int mlx5_set_msix_vec_count(struct mlx5_core_dev *dev, int function_id,
+ int msix_vec_count)
https://patchwork.kernel.org/project/netdevbpf/patch/20210114103140.866141-5-leon@kernel.org/
Thanks
From: Jakub Kicinski <kuba@kernel.org> Date: 2021-01-18 18:08:52
On Sun, 17 Jan 2021 09:24:41 +0200 Leon Romanovsky wrote:
On Sun, Jan 17, 2021 at 07:44:09AM +0200, Leon Romanovsky wrote:
quoted
On Thu, Jan 14, 2021 at 09:51:28AM -0800, Jakub Kicinski wrote:
quoted
On Thu, 14 Jan 2021 12:31:35 +0200 Leon Romanovsky wrote:
quoted
The number of MSI-X vectors is PCI property visible through lspci, that
field is read-only and configured by the device.
The static assignment of an amount of MSI-X vectors doesn't allow utilize
the newly created VF because it is not known to the device the future load
and configuration where that VF will be used.
The VFs are created on the hypervisor and forwarded to the VMs that have
different properties (for example number of CPUs).
To overcome the inefficiency in the spread of such MSI-X vectors, we
allow the kernel to instruct the device with the needed number of such
vectors, before VF is initialized and bounded to the driver.
Thanks Jakub,
I'll add kdocs to internal mlx5 functions.
IMHO, they are useless.
It's just scripts/kernel-doc, and it's checking if the kdoc is _valid_,
your call if you want to add kdoc, just a comment, or nothing at all.
At the end, it looks like CI false alarm.
drivers/net/ethernet/mellanox/mlx5/core/pci_irq.c:81: warning: Function parameter or member 'dev' not described in 'mlx5_set_msix_vec_count'
drivers/net/ethernet/mellanox/mlx5/core/pci_irq.c:81: warning: Function parameter or member 'function_id' not described in 'mlx5_set_msix_vec_count'
drivers/net/ethernet/mellanox/mlx5/core/pci_irq.c:81: warning: Function parameter or member 'msix_vec_count' not described in 'mlx5_set_msix_vec_count'
New warnings added
The function mlx5_set_msix_vec_count() is documented.
+/**
+ * mlx5_set_msix_vec_count() - Set dynamically allocated MSI-X to the VF
+ * @dev - PF to work on
+ * @function_id - internal PCI VF function id
+ * @msix_vec_count - Number of MSI-X to set
+ **/
+int mlx5_set_msix_vec_count(struct mlx5_core_dev *dev, int function_id,
+ int msix_vec_count)
https://patchwork.kernel.org/project/netdevbpf/patch/20210114103140.866141-5-leon@kernel.org/
AFAIU that's not valid kdoc, I _think_ you need to replace ' -' with ':'
for arguments (not my rules).
From: Leon Romanovsky <leon@kernel.org> Date: 2021-01-19 06:40:02
On Mon, Jan 18, 2021 at 10:07:32AM -0800, Jakub Kicinski wrote:
On Sun, 17 Jan 2021 09:24:41 +0200 Leon Romanovsky wrote:
quoted
On Sun, Jan 17, 2021 at 07:44:09AM +0200, Leon Romanovsky wrote:
quoted
On Thu, Jan 14, 2021 at 09:51:28AM -0800, Jakub Kicinski wrote:
quoted
On Thu, 14 Jan 2021 12:31:35 +0200 Leon Romanovsky wrote:
quoted
The number of MSI-X vectors is PCI property visible through lspci, that
field is read-only and configured by the device.
The static assignment of an amount of MSI-X vectors doesn't allow utilize
the newly created VF because it is not known to the device the future load
and configuration where that VF will be used.
The VFs are created on the hypervisor and forwarded to the VMs that have
different properties (for example number of CPUs).
To overcome the inefficiency in the spread of such MSI-X vectors, we
allow the kernel to instruct the device with the needed number of such
vectors, before VF is initialized and bounded to the driver.
Thanks Jakub,
I'll add kdocs to internal mlx5 functions.
IMHO, they are useless.
It's just scripts/kernel-doc, and it's checking if the kdoc is _valid_,
your call if you want to add kdoc, just a comment, or nothing at all.
I prefer clean CI, so will add.
quoted
At the end, it looks like CI false alarm.
drivers/net/ethernet/mellanox/mlx5/core/pci_irq.c:81: warning: Function parameter or member 'dev' not described in 'mlx5_set_msix_vec_count'
drivers/net/ethernet/mellanox/mlx5/core/pci_irq.c:81: warning: Function parameter or member 'function_id' not described in 'mlx5_set_msix_vec_count'
drivers/net/ethernet/mellanox/mlx5/core/pci_irq.c:81: warning: Function parameter or member 'msix_vec_count' not described in 'mlx5_set_msix_vec_count'
New warnings added
The function mlx5_set_msix_vec_count() is documented.
+/**
+ * mlx5_set_msix_vec_count() - Set dynamically allocated MSI-X to the VF
+ * @dev - PF to work on
+ * @function_id - internal PCI VF function id
+ * @msix_vec_count - Number of MSI-X to set
+ **/
+int mlx5_set_msix_vec_count(struct mlx5_core_dev *dev, int function_id,
+ int msix_vec_count)
https://patchwork.kernel.org/project/netdevbpf/patch/20210114103140.866141-5-leon@kernel.org/
AFAIU that's not valid kdoc, I _think_ you need to replace ' -' with ':'
for arguments (not my rules).