[pci PATCH v6 0/5] Add support for unmanaged SR-IOV

[pci PATCH v6 0/5] Add support for unmanaged SR-IOV

[Alexander Duyck]

This series is meant to add support for SR-IOV on devices when the VFs are
not managed by the kernel. Examples of recent patches attempting to do this
include:
virto - https://patchwork.kernel.org/patch/10241225/
pci-stub - https://patchwork.kernel.org/patch/10109935/
vfio - https://patchwork.kernel.org/patch/10103353/
uio - https://patchwork.kernel.org/patch/9974031/

Since this is quickly blowing up into a multi-driver problem it is probably
best to implement this solution as generically as possible.

This series is an attempt to do that. What we do with this patch set is
provide a generic framework to enable SR-IOV in the case that the PF driver
doesn't support managing the VFs itself.

I based my patch set originally on the patch by Mark Rustad but there isn't
much left after going through and cleaning out the bits that were no longer
needed, and after incorporating the feedback from David Miller. At this point
the only items to be fully reused was his patch description which is now
present in patch 3 of the set.

This solution is limited in scope to just adding support for devices that
provide no functionality for SR-IOV other than allocating the VFs by
calling pci_enable_sriov. Previous sets had included patches for VFIO, but
for now I am dropping that as the scope of that work is larger then I
think I can take on at this time.

v2: Reduced scope back to just virtio_pci and vfio-pci
    Broke into 3 patch set from single patch
    Changed autoprobe behavior to always set when num_vfs is set non-zero
v3: Updated Documentation to clarify when sriov_unmanaged_autoprobe is used
    Wrapped vfio_pci_sriov_configure to fix build errors w/o SR-IOV in kernel
v4: Dropped vfio-pci patch
    Added ena and nvme to drivers now using pci_sriov_configure_unmanaged
    Dropped pci_disable_sriov call in virtio_pci to be consistent with ena
v5: Dropped sriov_unmanaged_autoprobe and pci_sriov_conifgure_unmanaged
    Added new patch that enables pci_sriov_configure_simple
    Updated drivers to use pci_sriov_configure_simple
v6: Defined pci_sriov_configure_simple as NULL when SR-IOV is not enabled
    Updated drivers to drop "#ifdef" checks for IOV
    Added pci-pf-stub as place for PF-only drivers to add support

Cc: Mark Rustad <mark.d.rustad@intel.com>
Cc: Maximilian Heyne <mheyne@amazon.de>
Cc: Liang-Min Wang <liang-min.wang@intel.com>
Cc: David Woodhouse <dwmw@amazon.co.uk>

---

Alexander Duyck (5):
      pci: Add pci_sriov_configure_simple for PFs that don't manage VF resources
      virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
      ena: Migrate over to unmanaged SR-IOV support
      nvme: Migrate over to unmanaged SR-IOV support
      pci-pf-stub: Add PF driver stub for PFs that function only to enable VFs


 drivers/net/ethernet/amazon/ena/ena_netdev.c |   28 ------------
 drivers/nvme/host/pci.c                      |   20 ---------
 drivers/pci/Kconfig                          |   12 +++++
 drivers/pci/Makefile                         |    2 +
 drivers/pci/iov.c                            |   32 ++++++++++++++
 drivers/pci/pci-pf-stub.c                    |   60 ++++++++++++++++++++++++++
 drivers/virtio/virtio_pci_common.c           |    1 
 include/linux/pci.h                          |    3 +
 include/linux/pci_ids.h                      |    2 +
 9 files changed, 114 insertions(+), 46 deletions(-)
 create mode 100644 drivers/pci/pci-pf-stub.c

--

[pci PATCH v6 1/5] pci: Add pci_sriov_configure_simple for PFs that don't manage VF resources

[Alexander Duyck]

From: Alexander Duyck <alexander.h.duyck@intel.com>

This patch adds a common configuration function called
pci_sriov_configure_simple that will allow for managing VFs on devices
where the PF is not capable of managing VF resources.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

v5: New patch replacing pci_sriov_configure_unmanaged with
      pci_sriov_configure_simple
    Dropped bits related to autoprobe changes
v6: Defined pci_sriov_configure_simple as NULL if IOV is disabled

 drivers/pci/iov.c   |   32 ++++++++++++++++++++++++++++++++
 include/linux/pci.h |    3 +++
 2 files changed, 35 insertions(+)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 677924ae0350..bd7021491fdb 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -807,3 +807,35 @@ int pci_sriov_get_totalvfs(struct pci_dev *dev)
 	return dev->sriov->total_VFs;
 }
 EXPORT_SYMBOL_GPL(pci_sriov_get_totalvfs);
+
+/**
+ * pci_sriov_configure_simple - helper to configure unmanaged SR-IOV
+ * @dev: the PCI device
+ * @nr_virtfn: number of virtual functions to enable, 0 to disable
+ *
+ * Used to provide generic enable/disable SR-IOV option for devices
+ * that do not manage the VFs generated by their driver
+ */
+int pci_sriov_configure_simple(struct pci_dev *dev, int nr_virtfn)
+{
+	int err = -EINVAL;
+
+	might_sleep();
+
+	if (!dev->is_physfn)
+		return -ENODEV;
+
+	if (pci_vfs_assigned(dev)) {
+		pci_warn(dev,
+			 "Cannot modify SR-IOV while VFs are assigned\n");
+		err = -EPERM;
+	} else if (!nr_virtfn) {
+		sriov_disable(dev);
+		err = 0;
+	} else if (!dev->sriov->num_VFs) {
+		err = sriov_enable(dev, nr_virtfn);
+	}
+
+	return err ? err : nr_virtfn;
+}
+EXPORT_SYMBOL_GPL(pci_sriov_configure_simple);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 024a1beda008..f3099e940cda 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1953,6 +1953,7 @@ static inline void pci_mmcfg_late_init(void) { }
 int pci_vfs_assigned(struct pci_dev *dev);
 int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs);
 int pci_sriov_get_totalvfs(struct pci_dev *dev);
+int pci_sriov_configure_simple(struct pci_dev *dev, int nr_virtfn);
 resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno);
 void pci_vf_drivers_autoprobe(struct pci_dev *dev, bool probe);
 #else
@@ -1980,6 +1981,8 @@ static inline int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs)
 { return 0; }
 static inline int pci_sriov_get_totalvfs(struct pci_dev *dev)
 { return 0; }
+/* since this expected to be used as a function pointer just define as NULL */
+#define pci_sriov_configure_simple NULL
 static inline resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno)
 { return 0; }
 static inline void pci_vf_drivers_autoprobe(struct pci_dev *dev, bool probe) { }

[pci PATCH v6 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices

[Alexander Duyck]

From: Alexander Duyck <alexander.h.duyck@intel.com>

Hardware-realized virtio_pci devices can implement SR-IOV, so this
patch enables its use. The device in question is an upcoming Intel
NIC that implements both a virtio_net PF and virtio_net VFs. These
are hardware realizations of what has been up to now been a software
interface.

The device in question has the following 4-part PCI IDs:

PF: vendor: 1af4 device: 1041 subvendor: 8086 subdevice: 15fe
VF: vendor: 1af4 device: 1041 subvendor: 8086 subdevice: 05fe

The patch currently needs no check for device ID, because the callback
will never be made for devices that do not assert the capability or
when run on a platform incapable of SR-IOV.

One reason for this patch is because the hardware requires the
vendor ID of a VF to be the same as the vendor ID of the PF that
created it. So it seemed logical to simply have a fully-functioning
virtio_net PF create the VFs. This patch makes that possible.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

v4: Dropped call to pci_disable_sriov in virtio_pci_remove function
v5: Replaced call to pci_sriov_configure_unmanaged with
        pci_sriov_configure_simple
v6: Dropped "#ifdef" checks for IOV wrapping sriov_configure definition

 drivers/virtio/virtio_pci_common.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 48d4d1cf1cb6..67a227fd7aa0 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -596,6 +596,7 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
 #ifdef CONFIG_PM_SLEEP
 	.driver.pm	= &virtio_pci_pm_ops,
 #endif
+	.sriov_configure = pci_sriov_configure_simple,
 };
 
 module_pci_driver(virtio_pci_driver);

[pci PATCH v6 3/5] ena: Migrate over to unmanaged SR-IOV support

[Alexander Duyck]

From: Alexander Duyck <alexander.h.duyck@intel.com>

Instead of implementing our own version of a SR-IOV configuration stub in
the ena driver we can just reuse the existing
pci_sriov_configure_simple function.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

v5: Replaced call to pci_sriov_configure_unmanaged with
        pci_sriov_configure_simple
v6: Dropped "#ifdef" checks for IOV wrapping sriov_configure definition

 drivers/net/ethernet/amazon/ena/ena_netdev.c |   28 +-------------------------
 1 file changed, 1 insertion(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 6975150d144e..6054deb1e6aa 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -3385,32 +3385,6 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 }
 
 /*****************************************************************************/
-static int ena_sriov_configure(struct pci_dev *dev, int numvfs)
-{
-	int rc;
-
-	if (numvfs > 0) {
-		rc = pci_enable_sriov(dev, numvfs);
-		if (rc != 0) {
-			dev_err(&dev->dev,
-				"pci_enable_sriov failed to enable: %d vfs with the error: %d\n",
-				numvfs, rc);
-			return rc;
-		}
-
-		return numvfs;
-	}
-
-	if (numvfs == 0) {
-		pci_disable_sriov(dev);
-		return 0;
-	}
-
-	return -EINVAL;
-}
-
-/*****************************************************************************/
-/*****************************************************************************/
 
 /* ena_remove - Device Removal Routine
  * @pdev: PCI device information struct
@@ -3525,7 +3499,7 @@ static int ena_resume(struct pci_dev *pdev)
 	.suspend    = ena_suspend,
 	.resume     = ena_resume,
 #endif
-	.sriov_configure = ena_sriov_configure,
+	.sriov_configure = pci_sriov_configure_simple,
 };
 
 static int __init ena_init(void)

[pci PATCH v6 4/5] nvme: Migrate over to unmanaged SR-IOV support

[Alexander Duyck]

From: Alexander Duyck <alexander.h.duyck@intel.com>

Instead of implementing our own version of a SR-IOV configuration stub in
the nvme driver we can just reuse the existing
pci_sriov_configure_simple function.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

v5: Replaced call to pci_sriov_configure_unmanaged with
        pci_sriov_configure_simple
v6: Dropped "#ifdef" checks for IOV wrapping sriov_configure definition

 drivers/nvme/host/pci.c |   20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 5933a5c732e8..5e963058882a 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2580,24 +2580,6 @@ static void nvme_remove(struct pci_dev *pdev)
 	nvme_put_ctrl(&dev->ctrl);
 }
 
-static int nvme_pci_sriov_configure(struct pci_dev *pdev, int numvfs)
-{
-	int ret = 0;
-
-	if (numvfs == 0) {
-		if (pci_vfs_assigned(pdev)) {
-			dev_warn(&pdev->dev,
-				"Cannot disable SR-IOV VFs while assigned\n");
-			return -EPERM;
-		}
-		pci_disable_sriov(pdev);
-		return 0;
-	}
-
-	ret = pci_enable_sriov(pdev, numvfs);
-	return ret ? ret : numvfs;
-}
-
 #ifdef CONFIG_PM_SLEEP
 static int nvme_suspend(struct device *dev)
 {
@@ -2716,7 +2698,7 @@ static void nvme_error_resume(struct pci_dev *pdev)
 	.driver		= {
 		.pm	= &nvme_dev_pm_ops,
 	},
-	.sriov_configure = nvme_pci_sriov_configure,
+	.sriov_configure = pci_sriov_configure_simple,
 	.err_handler	= &nvme_err_handler,
 };
 

[pci PATCH v6 5/5] pci-pf-stub: Add PF driver stub for PFs that function only to enable VFs

[Alexander Duyck]

From: Alexander Duyck <alexander.h.duyck@intel.com>

Add a new driver called "pci-pf-stub" to act as a "white-list" for PF
devices that provide no other functionality other then acting as a means of
allocating a set of VFs. For now I only have one example ID provided by
Amazon in terms of devices that require this functionality. The general
idea is that in the future we will see other devices added as vendors come
up with devices where the PF is more or less just a lightweight shim used
to allocate VFs.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

v6: New driver to address concerns about Amazon devices left unsupported

 drivers/pci/Kconfig       |   12 +++++++++
 drivers/pci/Makefile      |    2 ++
 drivers/pci/pci-pf-stub.c |   60 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci_ids.h   |    2 ++
 4 files changed, 76 insertions(+)
 create mode 100644 drivers/pci/pci-pf-stub.c

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 34b56a8f8480..cdef2a2a9bc5 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -71,6 +71,18 @@ config PCI_STUB
 
 	  When in doubt, say N.
 
+config PCI_PF_STUB
+	tristate "PCI PF Stub driver"
+	depends on PCI
+	depends on PCI_IOV
+	help
+	  Say Y or M here if you want to enable support for devices that
+	  require SR-IOV support, while at the same time the PF itself is
+	  not providing any actual services on the host itself such as
+	  storage or networking.
+
+	  When in doubt, say N.
+
 config XEN_PCIDEV_FRONTEND
         tristate "Xen PCI Frontend"
         depends on PCI && X86 && XEN
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index 941970936840..4e133d3df403 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -43,6 +43,8 @@ obj-$(CONFIG_PCI_SYSCALL) += syscall.o
 
 obj-$(CONFIG_PCI_STUB) += pci-stub.o
 
+obj-$(CONFIG_PCI_PF_STUB) += pci-pf-stub.o
+
 obj-$(CONFIG_PCI_ECAM) += ecam.o
 
 obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o
diff --git a/drivers/pci/pci-pf-stub.c b/drivers/pci/pci-pf-stub.c
new file mode 100644
index 000000000000..d218924d9bdb
--- /dev/null
+++ b/drivers/pci/pci-pf-stub.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+/* pci-pf-stub - simple stub driver for PCI SR-IOV PF device
+ *
+ * This driver is meant to act as a "white-list" for devices that provde
+ * SR-IOV functionality while at the same time not actually needing a
+ * driver of their own.
+ */
+
+#include <linux/module.h>
+#include <linux/pci.h>
+
+/**
+ * pci_pf_stub_white_list - White list of devices to bind pci-pf-stub onto
+ *
+ * This table provides the list of IDs this driver is supposed to bind
+ * onto. You could think of this as a list of "quirked" devices where we
+ * are adding support for SR-IOV here since there are no other drivers
+ * that they would be running under.
+ *
+ * Layout of the table below is as follows:
+ * { Vendor ID, Device ID,
+ *   SubVendor ID, SubDevice ID,
+ *   Class, Class Mask,
+ *   private data (not used) }
+ */
+static const struct pci_device_id pci_pf_stub_white_list[] = {
+	{ PCI_VDEVICE(AMAZON, 0x0053) },
+	/* required last entry */
+	{ 0 }
+};
+MODULE_DEVICE_TABLE(pci, pci_pf_stub_white_list);
+
+static int pci_pf_stub_probe(struct pci_dev *dev,
+			     const struct pci_device_id *id)
+{
+	pci_info(dev, "claimed by pci-pf-stub\n");
+	return 0;
+}
+
+static struct pci_driver pf_stub_driver = {
+	.name			= "pci-pf-stub",
+	.id_table		= pci_pf_stub_white_list,
+	.probe			= pci_pf_stub_probe,
+	.sriov_configure	= pci_sriov_configure_simple,
+};
+
+static int __init pci_pf_stub_init(void)
+{
+	return pci_register_driver(&pf_stub_driver);
+}
+
+static void __exit pci_pf_stub_exit(void)
+{
+	pci_unregister_driver(&pf_stub_driver);
+}
+
+module_init(pci_pf_stub_init);
+module_exit(pci_pf_stub_exit);
+
+MODULE_LICENSE("GPL");
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index a6b30667a331..b10621896017 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2548,6 +2548,8 @@
 #define PCI_VENDOR_ID_CIRCUITCO		0x1cc8
 #define PCI_SUBSYSTEM_ID_CIRCUITCO_MINNOWBOARD	0x0001
 
+#define PCI_VENDOR_ID_AMAZON		0x1d0f
+
 #define PCI_VENDOR_ID_TEKRAM		0x1de1
 #define PCI_DEVICE_ID_TEKRAM_DC290	0xdc29
 

Re: [pci PATCH v6 1/5] pci: Add pci_sriov_configure_simple for PFs that don't manage VF resources

[Christoph Hellwig]

On Tue, Mar 13, 2018 at 02:28:49PM -0700, Alexander Duyck wrote:
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> This patch adds a common configuration function called
> pci_sriov_configure_simple that will allow for managing VFs on devices
> where the PF is not capable of managing VF resources.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
> 
> v5: New patch replacing pci_sriov_configure_unmanaged with
>       pci_sriov_configure_simple
>     Dropped bits related to autoprobe changes
> v6: Defined pci_sriov_configure_simple as NULL if IOV is disabled
> 
>  drivers/pci/iov.c   |   32 ++++++++++++++++++++++++++++++++
>  include/linux/pci.h |    3 +++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 677924ae0350..bd7021491fdb 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -807,3 +807,35 @@ int pci_sriov_get_totalvfs(struct pci_dev *dev)
>  	return dev->sriov->total_VFs;
>  }
>  EXPORT_SYMBOL_GPL(pci_sriov_get_totalvfs);
> +
> +/**
> + * pci_sriov_configure_simple - helper to configure unmanaged SR-IOV
> + * @dev: the PCI device
> + * @nr_virtfn: number of virtual functions to enable, 0 to disable
> + *
> + * Used to provide generic enable/disable SR-IOV option for devices
> + * that do not manage the VFs generated by their driver
> + */
> +int pci_sriov_configure_simple(struct pci_dev *dev, int nr_virtfn)
> +{
> +	int err = -EINVAL;

This assignment seems like it is never used..

> +
> +	might_sleep();
> +
> +	if (!dev->is_physfn)
> +		return -ENODEV;
> +
> +	if (pci_vfs_assigned(dev)) {
> +		pci_warn(dev,
> +			 "Cannot modify SR-IOV while VFs are assigned\n");
> +		err = -EPERM;

Why not:

	if (pci_vfs_assigned(dev)) {
		pci_warn(dev,
			 "Cannot modify SR-IOV while VFs are assigned\n");
		return -EPERM;
	}

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [pci PATCH v6 2/5] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [pci PATCH v6 4/5] nvme: Migrate over to unmanaged SR-IOV support

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [pci PATCH v6 5/5] pci-pf-stub: Add PF driver stub for PFs that function only to enable VFs

[Christoph Hellwig]

> +
> +/**
> + * pci_pf_stub_white_list - White list of devices to bind pci-pf-stub onto
> + *
> + * This table provides the list of IDs this driver is supposed to bind
> + * onto. You could think of this as a list of "quirked" devices where we
> + * are adding support for SR-IOV here since there are no other drivers
> + * that they would be running under.
> + *
> + * Layout of the table below is as follows:
> + * { Vendor ID, Device ID,
> + *   SubVendor ID, SubDevice ID,
> + *   Class, Class Mask,
> + *   private data (not used) }
> + */

No need to document the PCI device table format in a random driver.

Otherwise looks fine:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [pci PATCH v6 1/5] pci: Add pci_sriov_configure_simple for PFs that don't manage VF resources

[Alexander Duyck]

On Wed, Mar 14, 2018 at 1:54 AM, Christoph Hellwig <hch@lst.de> wrote:
> On Tue, Mar 13, 2018 at 02:28:49PM -0700, Alexander Duyck wrote:
>> From: Alexander Duyck <alexander.h.duyck@intel.com>
>>
>> This patch adds a common configuration function called
>> pci_sriov_configure_simple that will allow for managing VFs on devices
>> where the PF is not capable of managing VF resources.
>>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> ---
>>
>> v5: New patch replacing pci_sriov_configure_unmanaged with
>>       pci_sriov_configure_simple
>>     Dropped bits related to autoprobe changes
>> v6: Defined pci_sriov_configure_simple as NULL if IOV is disabled
>>
>>  drivers/pci/iov.c   |   32 ++++++++++++++++++++++++++++++++
>>  include/linux/pci.h |    3 +++
>>  2 files changed, 35 insertions(+)
>>
>> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
>> index 677924ae0350..bd7021491fdb 100644
>> --- a/drivers/pci/iov.c
>> +++ b/drivers/pci/iov.c
>> @@ -807,3 +807,35 @@ int pci_sriov_get_totalvfs(struct pci_dev *dev)
>>       return dev->sriov->total_VFs;
>>  }
>>  EXPORT_SYMBOL_GPL(pci_sriov_get_totalvfs);
>> +
>> +/**
>> + * pci_sriov_configure_simple - helper to configure unmanaged SR-IOV
>> + * @dev: the PCI device
>> + * @nr_virtfn: number of virtual functions to enable, 0 to disable
>> + *
>> + * Used to provide generic enable/disable SR-IOV option for devices
>> + * that do not manage the VFs generated by their driver
>> + */
>> +int pci_sriov_configure_simple(struct pci_dev *dev, int nr_virtfn)
>> +{
>> +     int err = -EINVAL;
>
> This assignment seems like it is never used..

It applies in the case where we are setting a number of VFs when VFs
are already allocated. It was needed since the last statement ended in
an if.

>> +
>> +     might_sleep();
>> +
>> +     if (!dev->is_physfn)
>> +             return -ENODEV;
>> +
>> +     if (pci_vfs_assigned(dev)) {
>> +             pci_warn(dev,
>> +                      "Cannot modify SR-IOV while VFs are assigned\n");
>> +             err = -EPERM;
>
> Why not:
>
>         if (pci_vfs_assigned(dev)) {
>                 pci_warn(dev,
>                          "Cannot modify SR-IOV while VFs are assigned\n");
>                 return -EPERM;
>         }

Symmetry. I had this as one of 3 blocks and just did it this way to be
consistent with the next two statements. I could probably just return
from each if block if that is preferred.

> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

For v7 I can probably drop the err parameter and just return the
necessary error values directly.

Re: [pci PATCH v6 5/5] pci-pf-stub: Add PF driver stub for PFs that function only to enable VFs

[Alexander Duyck]

On Wed, Mar 14, 2018 at 1:56 AM, Christoph Hellwig <hch@lst.de> wrote:
>> +
>> +/**
>> + * pci_pf_stub_white_list - White list of devices to bind pci-pf-stub onto
>> + *
>> + * This table provides the list of IDs this driver is supposed to bind
>> + * onto. You could think of this as a list of "quirked" devices where we
>> + * are adding support for SR-IOV here since there are no other drivers
>> + * that they would be running under.
>> + *
>> + * Layout of the table below is as follows:
>> + * { Vendor ID, Device ID,
>> + *   SubVendor ID, SubDevice ID,
>> + *   Class, Class Mask,
>> + *   private data (not used) }
>> + */
>
> No need to document the PCI device table format in a random driver.

I realize this probably isn't necessary, but I wanted to document it
to make it clear in terms of layout so that it is't confused with the
quirk structure layout since this has been referred to as a "quirk"
driver a few times.

> Otherwise looks fine:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>