| Message ID | 20210114103140.866141-1-leon@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Dynamically assign MSI-X vectors count | expand |
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. Hi Leon! Looks like you got some missing kdoc here, check out the test in patchwork so we don't need to worry about this later: https://patchwork.kernel.org/project/netdevbpf/list/?series=414497
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: > > 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. > > > Hi Leon! > > Looks like you got some missing kdoc here, check out the test in > patchwork so we don't need to worry about this later: > > https://patchwork.kernel.org/project/netdevbpf/list/?series=414497 Thanks Jakub, I'll add kdocs to internal mlx5 functions. IMHO, they are useless. Thanks
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: > > 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. > > > > > > Hi Leon! > > > > Looks like you got some missing kdoc here, check out the test in > > patchwork so we don't need to worry about this later: > > > > https://patchwork.kernel.org/project/netdevbpf/list/?series=414497 > > 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 > > Thanks
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: > > 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: > > > > 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. > > > > > > > > > Hi Leon! > > > > > > Looks like you got some missing kdoc here, check out the test in > > > patchwork so we don't need to worry about this later: > > > > > > https://patchwork.kernel.org/project/netdevbpf/list/?series=414497 > > > > 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).
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: > > 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: > > > > 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. > > > > > > > > > > > > Hi Leon! > > > > > > > > Looks like you got some missing kdoc here, check out the test in > > > > patchwork so we don't need to worry about this later: > > > > > > > > https://patchwork.kernel.org/project/netdevbpf/list/?series=414497 > > > > > > 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. > > > 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). Right, I figured it when submitted v3. Thanks
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