Re: [PATCH 1/2] PCI/IOV: provide flag to skip VF scanning
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
Re: [PATCH 1/2] PCI/IOV: provide flag to skip VF scanning
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
Re: [PATCH 1/2] PCI/IOV: provide flag to skip VF scanning
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
> ---
> 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.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 intnon_compliant_bars:1; /* Broken BARs; ignore them */
> unsigned intis_probed:1;/* Device probing in progress */
> unsigned intlink_active_reporting:1;/* Device capable of reporting
> link active */
> + unsigned intno_vf_scan:1; /* Don't scan for VF's after VF
> enablement */
> pci_dev_flags_t dev_flags;
> atomic_tenable_cnt; /* pci_enable_device has been called */
>
> --
> 2.13.4
>
Re: [PATCH 1/2] PCI/IOV: provide flag to skip VF scanning
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 Looks good, Reviewed-by: Christoph Hellwig
[PATCH 1/2] PCI/IOV: provide flag to skip VF scanning
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
---
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;
+}
+
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.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 intnon_compliant_bars:1; /* Broken BARs; ignore them */
unsigned intis_probed:1;/* Device probing in progress */
unsigned intlink_active_reporting:1;/* Device capable of reporting
link active */
+ unsigned intno_vf_scan:1; /* Don't scan for VF's after VF
enablement */
pci_dev_flags_t dev_flags;
atomic_tenable_cnt; /* pci_enable_device has been called */
--
2.13.4
