| Message ID | 20181218101650.23089-1-sebott@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [1/2] PCI/IOV: provide flag to skip VF scanning | expand |
On Tue, Dec 18, 2018 at 11:16:49AM +0100, Sebastian Ott wrote: > Provide a flag to skip scanning for new VFs after SRIOV enablement. > This can be set by implementations for which the VFs are already > reported by other means. > > Signed-off-by: Sebastian Ott <sebott@linux.ibm.com> Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de>
Hi Sebastian, On Tue, Dec 18, 2018 at 11:16:49AM +0100, Sebastian Ott wrote: > Provide a flag to skip scanning for new VFs after SRIOV enablement. > This can be set by implementations for which the VFs are already > reported by other means. > > Signed-off-by: Sebastian Ott <sebott@linux.ibm.com> > --- > drivers/pci/iov.c | 48 ++++++++++++++++++++++++++++++++++++------------ > include/linux/pci.h | 1 + > 2 files changed, 37 insertions(+), 12 deletions(-) > > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c > index 9616eca3182f..3aa115ed3a65 100644 > --- a/drivers/pci/iov.c > +++ b/drivers/pci/iov.c > @@ -252,6 +252,27 @@ int __weak pcibios_sriov_disable(struct pci_dev *pdev) > return 0; > } > > +static int sriov_add_vfs(struct pci_dev *dev, u16 num_vfs) > +{ > + unsigned int i; > + int rc; > + > + if (dev->no_vf_scan) > + return 0; > + > + for (i = 0; i < num_vfs; i++) { > + rc = pci_iov_add_virtfn(dev, i); > + if (rc) > + goto failed; > + } > + return 0; > +failed: > + while (i--) > + pci_iov_remove_virtfn(dev, i); > + > + return rc; > +} I think the strategy is fine, but can you restructure the patches like this: 1) Factor out sriov_add_vfs() and sriov_dev_vfs(). This makes no functional change at all. 2) Add dev->no_vf_scan, set it in the s390 pcibios_add_device(), and test it in sriov_add_vfs(), and sriov_del_vfs(). I think both pieces will be easier to review that way. > static int sriov_enable(struct pci_dev *dev, int nr_virtfn) > { > int rc; > @@ -337,21 +358,15 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn) > msleep(100); > pci_cfg_access_unlock(dev); > > - for (i = 0; i < initial; i++) { > - rc = pci_iov_add_virtfn(dev, i); > - if (rc) > - goto failed; > - } > + rc = sriov_add_vfs(dev, initial); > + if (rc) > + goto err_pcibios; > > kobject_uevent(&dev->dev.kobj, KOBJ_CHANGE); > iov->num_VFs = nr_virtfn; > > return 0; > > -failed: > - while (i--) > - pci_iov_remove_virtfn(dev, i); > - > err_pcibios: > iov->ctrl &= ~(PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE); > pci_cfg_access_lock(dev); > @@ -368,17 +383,26 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn) > return rc; > } > > -static void sriov_disable(struct pci_dev *dev) > +static void sriov_del_vfs(struct pci_dev *dev) > { > - int i; > struct pci_sriov *iov = dev->sriov; > + int i; > > - if (!iov->num_VFs) > + if (dev->no_vf_scan) > return; > > for (i = 0; i < iov->num_VFs; i++) > pci_iov_remove_virtfn(dev, i); > +} > + > +static void sriov_disable(struct pci_dev *dev) > +{ > + struct pci_sriov *iov = dev->sriov; > + > + if (!iov->num_VFs) > + return; > > + sriov_del_vfs(dev); > iov->ctrl &= ~(PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE); > pci_cfg_access_lock(dev); > pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl); > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 11c71c4ecf75..f70b9ccd3e86 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -405,6 +405,7 @@ struct pci_dev { > unsigned int non_compliant_bars:1; /* Broken BARs; ignore them */ > unsigned int is_probed:1; /* Device probing in progress */ > unsigned int link_active_reporting:1;/* Device capable of reporting link active */ > + unsigned int no_vf_scan:1; /* Don't scan for VF's after VF enablement */ > pci_dev_flags_t dev_flags; > atomic_t enable_cnt; /* pci_enable_device has been called */ > > -- > 2.13.4 >
Hello Bjorn, On Thu, 20 Dec 2018, Bjorn Helgaas wrote: > I think the strategy is fine, but can you restructure the patches > like this: > > 1) Factor out sriov_add_vfs() and sriov_dev_vfs(). This makes no > functional change at all. > > 2) Add dev->no_vf_scan, set it in the s390 pcibios_add_device(), and > test it in sriov_add_vfs(), and sriov_del_vfs(). > > I think both pieces will be easier to review that way. Done. I took the liberty to add Christoph's R-b to the first two patches since it's just a split of the patch he gave the R-b to. Thanks! Sebastian
On Fri, Dec 21, 2018 at 03:19:49PM +0100, Sebastian Ott wrote: > Hello Bjorn, > > On Thu, 20 Dec 2018, Bjorn Helgaas wrote: > > I think the strategy is fine, but can you restructure the patches > > like this: > > > > 1) Factor out sriov_add_vfs() and sriov_dev_vfs(). This makes no > > functional change at all. > > > > 2) Add dev->no_vf_scan, set it in the s390 pcibios_add_device(), and > > test it in sriov_add_vfs(), and sriov_del_vfs(). > > > > I think both pieces will be easier to review that way. > > Done. I took the liberty to add Christoph's R-b to the first two patches > since it's just a split of the patch he gave the R-b to. Thanks. It's really way too late to do this, but they're pretty trivial, and I've been out longer than expected for vacation and illness, so I applied these to pci/virtualization for v4.21. Bjorn
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c index 9616eca3182f..3aa115ed3a65 100644 --- a/drivers/pci/iov.c +++ b/drivers/pci/iov.c @@ -252,6 +252,27 @@ int __weak pcibios_sriov_disable(struct pci_dev *pdev) return 0; } +static int sriov_add_vfs(struct pci_dev *dev, u16 num_vfs) +{ + unsigned int i; + int rc; + + if (dev->no_vf_scan) + return 0; + + for (i = 0; i < num_vfs; i++) { + rc = pci_iov_add_virtfn(dev, i); + if (rc) + goto failed; + } + return 0; +failed: + while (i--) + pci_iov_remove_virtfn(dev, i); + + return rc; +} + static int sriov_enable(struct pci_dev *dev, int nr_virtfn) { int rc; @@ -337,21 +358,15 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn) msleep(100); pci_cfg_access_unlock(dev); - for (i = 0; i < initial; i++) { - rc = pci_iov_add_virtfn(dev, i); - if (rc) - goto failed; - } + rc = sriov_add_vfs(dev, initial); + if (rc) + goto err_pcibios; kobject_uevent(&dev->dev.kobj, KOBJ_CHANGE); iov->num_VFs = nr_virtfn; return 0; -failed: - while (i--) - pci_iov_remove_virtfn(dev, i); - err_pcibios: iov->ctrl &= ~(PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE); pci_cfg_access_lock(dev); @@ -368,17 +383,26 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn) return rc; } -static void sriov_disable(struct pci_dev *dev) +static void sriov_del_vfs(struct pci_dev *dev) { - int i; struct pci_sriov *iov = dev->sriov; + int i; - if (!iov->num_VFs) + if (dev->no_vf_scan) return; for (i = 0; i < iov->num_VFs; i++) pci_iov_remove_virtfn(dev, i); +} + +static void sriov_disable(struct pci_dev *dev) +{ + struct pci_sriov *iov = dev->sriov; + + if (!iov->num_VFs) + return; + sriov_del_vfs(dev); iov->ctrl &= ~(PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE); pci_cfg_access_lock(dev); pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl); diff --git a/include/linux/pci.h b/include/linux/pci.h index 11c71c4ecf75..f70b9ccd3e86 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -405,6 +405,7 @@ struct pci_dev { unsigned int non_compliant_bars:1; /* Broken BARs; ignore them */ unsigned int is_probed:1; /* Device probing in progress */ unsigned int link_active_reporting:1;/* Device capable of reporting link active */ + unsigned int no_vf_scan:1; /* Don't scan for VF's after VF enablement */ pci_dev_flags_t dev_flags; atomic_t enable_cnt; /* pci_enable_device has been called */
Provide a flag to skip scanning for new VFs after SRIOV enablement. This can be set by implementations for which the VFs are already reported by other means. Signed-off-by: Sebastian Ott <sebott@linux.ibm.com> --- drivers/pci/iov.c | 48 ++++++++++++++++++++++++++++++++++++------------ include/linux/pci.h | 1 + 2 files changed, 37 insertions(+), 12 deletions(-)