Skip to contentSkip to conversation

Add support for unmanaged SR-IOV

PATCH Superseded Alexander Duyck 2 participants 12 messages Mar 13, 2018 – Mar 14, 2018

Subsystems (7)
Networking Network drivers KVM PCI +3 more subsystems

v6 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

v6 cover Add support for unmanaged SR-IOV

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 <[email protected]>
Cc Maximilian Heyne <[email protected]>
Cc Liang-Min Wang <[email protected]>
Cc David Woodhouse <[email protected]>

---

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

--

#1 lore raw

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

From: Alexander Duyck <[email protected]>

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.

---

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(+)
drivers/pci/iov.c +32 -0
index 677924ae0350..bd7021491fdb 100644
@@ -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);
include/linux/pci.h +3 -0
index 024a1beda008..f3099e940cda 100644
@@ -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) { }
maintainer Reviewed-by on v6 1/5
#2 lore raw

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

Quoted 28 lines · Alexander Duyck

From: Alexander Duyck <[email protected]>

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.

---

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(+)
drivers/pci/iov.c +2 -0
index 677924ae0350..bd7021491fdb 100644
@@ -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);
+
+/**

drivers/pci/iov.c int pci_sriov_get_totalvfs(struct pci_dev *dev) · line 821

Quoted 10 lines
drivers/pci/iov.c +10 -0
+ * 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..

drivers/pci/iov.c int pci_sriov_get_totalvfs(struct pci_dev *dev) · line 831

Quoted 10 lines
drivers/pci/iov.c +10 -0
+
+	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:

#3 lore raw

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

Quoted 33 lines · Christoph Hellwig
Quoted 32 lines · Alexander Duyck

From: Alexander Duyck <[email protected]>

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.

---

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(+)
drivers/pci/iov.c +6 -0
index 677924ae0350..bd7021491fdb 100644
@@ -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
+ *
Quoted 8 lines

drivers/pci/iov.c int pci_sriov_get_totalvfs(struct pci_dev *dev) · line 821

Quoted 6 lines
drivers/pci/iov.c +6 -0
+ * 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.

Quoted 13 lines

drivers/pci/iov.c int pci_sriov_get_totalvfs(struct pci_dev *dev) · line 831

Quoted 10 lines
drivers/pci/iov.c +10 -0
+
+     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:

Quoted 5 lines
        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.

Quoted 3 lines

Otherwise looks good:

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

#4 lore raw

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

In reply to Alexander Duyck

From: Alexander Duyck <[email protected]>

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.

---

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(+)
drivers/virtio/virtio_pci_common.c +1 -0
index 48d4d1cf1cb6..67a227fd7aa0 100644
@@ -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);
#6 lore raw

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

In reply to Alexander Duyck

From: Alexander Duyck <[email protected]>

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.

---

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(-)
drivers/net/ethernet/amazon/ena/ena_netdev.c +1 -27
index 6975150d144e..6054deb1e6aa 100644
@@ -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)
#7 lore raw

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

In reply to Alexander Duyck

From: Alexander Duyck <[email protected]>

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.

---

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(-)
drivers/nvme/host/pci.c +1 -19
index 5933a5c732e8..5e963058882a 100644
@@ -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,
 };
#9 lore raw

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

In reply to Alexander Duyck

From: Alexander Duyck <[email protected]>

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.

---

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
drivers/pci/Kconfig +12 -0
index 34b56a8f8480..cdef2a2a9bc5 100644
@@ -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
drivers/pci/Makefile +2 -0
index 941970936840..4e133d3df403 100644
@@ -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
drivers/pci/pci-pf-stub.c new file +60 -0

60 lines not on this page · raw message

include/linux/pci_ids.h +2 -0
index a6b30667a331..b10621896017 100644
@@ -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
maintainer Reviewed-by on v6 5/5
#10 lore raw

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

Quoted 5 lines
drivers/pci/pci-pf-stub.c new file +5 -0
+
+/**
+ * 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

drivers/pci/pci-pf-stub.c line 25

Quoted 10 lines
drivers/pci/pci-pf-stub.c new file +10 -0
+ * 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:

#11 lore raw

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

Quoted 9 lines · Christoph Hellwig
Quoted 9 lines
drivers/pci/pci-pf-stub.c new file +9 -0
+
+/**
+ * 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.
+ *
Quoted 8 lines
Quoted 6 lines

+ * 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.

Quoted 3 lines

Otherwise looks fine: