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

---

Alexander Duyck (4):
pci-iov: Add support for unmanaged SR-IOV
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

 Documentation/ABI/testing/sysfs-bus-pci      |   25 +++++++++++++++
 drivers/net/ethernet/amazon/ena/ena_netdev.c |   30 ++-----------------
 drivers/nvme/host/pci.c                      |   22 ++------------
 drivers/pci/iov.c                            |   42 ++++++++++++++++++++++++++
 drivers/pci/pci-driver.c                     |    2 +
 drivers/pci/pci-sysfs.c                      |   29 ++++++++++++++++++
 drivers/pci/pci.h                            |    4 ++
 drivers/virtio/virtio_pci_common.c           |    3 ++
 include/linux/pci.h                          |    1 +
 9 files changed, 110 insertions(+), 48 deletions(-)

--

From: Alexander Duyck <[email protected]>

This patch is meant to add some basic functionality to support for SR-IOV on devices when the VFs are not managed by some other entity in the device other than the kernel.

A new sysfs value called sriov_unmanaged_autoprobe has been added. This value is used as the drivers_autoprobe setting of the VFs when they are being managed by an external entity such as device firmware instead of being managed by the kernel.

One side effect of this change is that the sriov_drivers_autoprobe and sriov_unmanaged_autoprobe will only apply their updates when SR-IOV VFs are allocated. Attempts to update them when SR-IOV is in use will only update the local value and will not update sriov->autoprobe.

---

v3: Updated documentation to better explain sriov_unmanaged_autoprobe use
v4: Further documentation updates
Added bits so that sriov->autoprobe is not updated while VFs are active Added code to check for assigned VFs in pci_sriov_configure_unamanged

 Documentation/ABI/testing/sysfs-bus-pci |   25 ++++++++++++++++++
 drivers/pci/iov.c                       |   42 +++++++++++++++++++++++++++++++
 drivers/pci/pci-driver.c                |    2 +
 drivers/pci/pci-sysfs.c                 |   29 +++++++++++++++++++++
 drivers/pci/pci.h                       |    4 ++-
 include/linux/pci.h                     |    1 +
 6 files changed, 101 insertions(+), 2 deletions(-)

index 44d4b2be92fd..82c9520e365f 100644
@@ -323,3 +323,28 @@ Description:
 
 		This is similar to /sys/bus/pci/drivers_autoprobe, but
 		affects only the VFs associated with a specific PF.
+
+What:		/sys/bus/pci/devices/.../sriov_unmanaged_autoprobe
+Date:		March 2018
+Contact:	Alexander Duyck <[email protected]>
+Description:
+		This file is associated with the PF of a device that
+		supports SR-IOV.  It determines whether newly-enabled VFs
+		are immediately bound to a driver when the PF driver does
+		not manage the VFs itself.  It initially contains 0, which
+		means the kernel will not automatically bind VFs to a driver.
+		If an application writes 1 to the file before enabling VFs,
+		the kernel will bind VFs to a compatible driver immediately
+		after they are enabled.
+
+		Currently the use of this setting is limited to drivers that
+		make use of pci_sriov_configure_unmanaged. Examples might
+		include drivers such as virtio_net which would be exposed as
+		a PCIe PF due to the fact that a given device exposes all of
+		the VFs and PFs as the same general device type but can add
+		the SR-IOV configuration bits to the configuration space of
+		the PF.
+
+		This overrides /sys/bus/pci/devices/.../sriov_drivers_autoprobe
+		when a PF driver does not provide functionality to manage the
+		VFs when SR-IOV is enabled.

index 3bed6beda051..2cc68dff6130 100644
@@ -398,7 +398,7 @@ void __weak pcibios_free_irq(struct pci_dev *dev)
 #ifdef CONFIG_PCI_IOV
 static inline bool pci_device_can_probe(struct pci_dev *pdev)
 {
-	return (!pdev->is_virtfn || pdev->physfn->sriov->drivers_autoprobe);
+	return (!pdev->is_virtfn || pdev->physfn->sriov->autoprobe);
 }
 #else
 static inline bool pci_device_can_probe(struct pci_dev *pdev)

index eb6bee8724cc..6f78fa73e317 100644
@@ -710,6 +710,30 @@ static ssize_t sriov_drivers_autoprobe_store(struct device *dev,
 	return count;
 }
 
+static ssize_t sriov_unmanaged_autoprobe_show(struct device *dev,
+					      struct device_attribute *attr,
+					      char *buf)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+
+	return sprintf(buf, "%u\n", pdev->sriov->unmanaged_autoprobe);
+}
+
+static ssize_t sriov_unmanaged_autoprobe_store(struct device *dev,
+					       struct device_attribute *attr,
+					       const char *buf, size_t count)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	bool unmanaged_autoprobe;
+
+	if (kstrtobool(buf, &unmanaged_autoprobe) < 0)
+		return -EINVAL;
+
+	pdev->sriov->unmanaged_autoprobe = unmanaged_autoprobe;
+
+	return count;
+}
+
 static struct device_attribute sriov_totalvfs_attr = __ATTR_RO(sriov_totalvfs);
 static struct device_attribute sriov_numvfs_attr =
 		__ATTR(sriov_numvfs, (S_IRUGO|S_IWUSR|S_IWGRP),

@@ -720,6 +744,10 @@ static ssize_t sriov_drivers_autoprobe_store(struct device *dev,
 static struct device_attribute sriov_drivers_autoprobe_attr =
 		__ATTR(sriov_drivers_autoprobe, (S_IRUGO|S_IWUSR|S_IWGRP),
 		       sriov_drivers_autoprobe_show, sriov_drivers_autoprobe_store);
+static struct device_attribute sriov_unmanaged_autoprobe_attr =
+		__ATTR(sriov_unmanaged_autoprobe, (S_IRUGO|S_IWUSR|S_IWGRP),
+		       sriov_unmanaged_autoprobe_show,
+		       sriov_unmanaged_autoprobe_store);
 #endif /* CONFIG_PCI_IOV */
 
 static ssize_t driver_override_store(struct device *dev,

@@ -1789,6 +1817,7 @@ static umode_t pcie_dev_attrs_are_visible(struct kobject *kobj,
 	&sriov_stride_attr.attr,
 	&sriov_vf_device_attr.attr,
 	&sriov_drivers_autoprobe_attr.attr,
+	&sriov_unmanaged_autoprobe_attr.attr,
 	NULL,
 };
 

index fcd81911b127..b5f8b034f02d 100644
@@ -272,7 +272,9 @@ struct pci_sriov {
 	struct pci_dev	*dev;		/* Lowest numbered PF */
 	struct pci_dev	*self;		/* This PF */
 	resource_size_t	barsz[PCI_SRIOV_NUM_BARS];	/* VF BAR size */
-	bool		drivers_autoprobe; /* Auto probing of VFs by driver */
+	bool		autoprobe;	/* Auto probing of VFs by VF driver */
+	bool		drivers_autoprobe;	/* "" managed by PF driver */
+	bool		unmanaged_autoprobe;	/* "" unmanaged by kernel */
 };
 
 /* pci_dev priv_flags */

index 024a1beda008..553860a08131 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_unmanaged(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

I still struggle to understand why we need this "unmanaged" complication and how a user of the sysfs API is expected to have any idea whether a PF is managed or unmanaged and why they should care. Can't we just have a pci_simple_sriov_configure() helper and ignore this unmanaged business? Thanks,

Alex

Just a pci_simple_sriov_configure is exactly what I envisioned originally.

I can drop the "unmanaged" bits if that is what is wanted, but based on previous conversations I thought there was some concern about the kernel loading VFs when there was some foreign entity managing the VFs other than the kernel.

Thanks.

- Alex

My concern has always been whether the PF driver is trusted and by dropping the vfio bits, the remaining drivers here are native, trusted, host drivers, so I don't see that we have any reason to consider the VFs as anything other than trusted as well. It's VFs where the PF driver is untrusted, such as a userspace driver, which needs some kind of quarantine, imo. Thanks,

Alex

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

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

index 48d4d1cf1cb6..27ab090163b4 100644
@@ -596,6 +596,9 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
 #ifdef CONFIG_PM_SLEEP
 	.driver.pm	= &virtio_pci_pm_ops,
 #endif
+#ifdef CONFIG_PCI_IOV
+	.sriov_configure = pci_sriov_configure_unmanaged,
+#endif
 };
 
 module_pci_driver(virtio_pci_driver);

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_unmanaged function.

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

index 6975150d144e..fa708dd86c96 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,9 @@ static int ena_resume(struct pci_dev *pdev)
 	.suspend    = ena_suspend,
 	.resume     = ena_resume,
 #endif
-	.sriov_configure = ena_sriov_configure,
+#ifdef CONFIG_PCI_IOV
+	.sriov_configure = pci_sriov_configure_unmanaged,
+#endif
 };
 
 static int __init ena_init(void)

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_unmanaged function.

---
 drivers/nvme/host/pci.c |   22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

index 6fe7af00a1f4..2c753ccc1771 100644
@@ -2570,24 +2570,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)
 {

@@ -2706,7 +2688,9 @@ static void nvme_error_resume(struct pci_dev *pdev)
 	.driver		= {
 		.pm	= &nvme_dev_pm_ops,
 	},
-	.sriov_configure = nvme_pci_sriov_configure,
+#ifdef CONFIG_PCI_IOV
+	.sriov_configure = pci_sriov_configure_unmanaged,
+#endif
 	.err_handler	= &nvme_err_handler,
 };