Re: [PATCH mlx5-next 1/7] PCI/IOV: Provide internal VF index
From: Leon Romanovsky <leon@kernel.org>
Date: 2021-09-23 06:35:39
Also in:
kvm, linux-pci, linux-rdma, lkml
Subsystem:
mellanox mlx5 core vpi driver, networking drivers, the rest · Maintainers:
Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Mark Bloch, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
On Wed, Sep 22, 2021 at 04:59:30PM -0500, Bjorn Helgaas wrote:
On Wed, Sep 22, 2021 at 01:38:50PM +0300, Leon Romanovsky wrote:quoted
From: Jason Gunthorpe <jgg@nvidia.com> The PCI core uses the VF index internally, often called the vf_id, during the setup of the VF, eg pci_iov_add_virtfn(). This index is needed for device drivers that implement live migration for their internal operations that configure/control their VFs. Specifically, mlx5_vfio_pci driver that is introduced in coming patches from this series needs it and not the bus/device/function which is exposed today. Add pci_iov_vf_id() which computes the vf_id by reversing the math that was used to create the bus/device/function. Signed-off-by: Yishai Hadas <yishaih@nvidia.com> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>Acked-by: Bjorn Helgaas <bhelgaas@google.com> mlx5_core_sriov_set_msix_vec_count() looks like it does basically the same thing as pci_iov_vf_id() by iterating through VFs until it finds one with a matching devfn (although it *doesn't* check for a matching bus number, which seems like a bug). Maybe that should use pci_iov_vf_id()?
Yes, I gave same comment internally and we decided to simply reduce the amount of changes in mlx5_core to have less distractions and submit as a followup. Most likely will add this hunk in v1.
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sriov.c b/drivers/net/ethernet/mellanox/mlx5/core/sriov.c
index e8185b69ac6c..b66be0b4244a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/sriov.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/sriov.c@@ -209,15 +209,8 @@ int mlx5_core_sriov_set_msix_vec_count(struct pci_dev *vf, int msix_vec_count) /* Reversed translation of PCI VF function number to the internal * function_id, which exists in the name of virtfn symlink. */ - 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) + id = pci_iov_vf_id(vf); + if (id < 0 || !sriov->vfs_ctx[id].enabled) return -EINVAL; return mlx5_set_msix_vec_count(dev, id + 1, msix_vec_count);
Thanks
quoted
--- drivers/pci/iov.c | 14 ++++++++++++++ include/linux/pci.h | 7 ++++++- 2 files changed, 20 insertions(+), 1 deletion(-)diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c index dafdc652fcd0..e7751fa3fe0b 100644 --- a/drivers/pci/iov.c +++ b/drivers/pci/iov.c@@ -33,6 +33,20 @@ int pci_iov_virtfn_devfn(struct pci_dev *dev, int vf_id) } EXPORT_SYMBOL_GPL(pci_iov_virtfn_devfn); +int pci_iov_vf_id(struct pci_dev *dev) +{ + struct pci_dev *pf; + + if (!dev->is_virtfn) + return -EINVAL; + + pf = pci_physfn(dev); + return (((dev->bus->number << 8) + dev->devfn) - + ((pf->bus->number << 8) + pf->devfn + pf->sriov->offset)) / + pf->sriov->stride; +} +EXPORT_SYMBOL_GPL(pci_iov_vf_id); + /* * Per SR-IOV spec sec 3.3.10 and 3.3.11, First VF Offset and VF Stride may * change when NumVFs changes.diff --git a/include/linux/pci.h b/include/linux/pci.h index cd8aa6fce204..4d6c73506e18 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h@@ -2153,7 +2153,7 @@ void __iomem *pci_ioremap_wc_bar(struct pci_dev *pdev, int bar); #ifdef CONFIG_PCI_IOV int pci_iov_virtfn_bus(struct pci_dev *dev, int id); int pci_iov_virtfn_devfn(struct pci_dev *dev, int id); - +int pci_iov_vf_id(struct pci_dev *dev); int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn); void pci_disable_sriov(struct pci_dev *dev);@@ -2181,6 +2181,11 @@ static inline int pci_iov_virtfn_devfn(struct pci_dev *dev, int id) { return -ENOSYS; } +static inline int pci_iov_vf_id(struct pci_dev *dev) +{ + return -ENOSYS; +} +Drop the blank line to match the surrounding stubs.
Sure, thanks
quoted
static inline int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn) { return -ENODEV; }