linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] pci-iov: Add support for unmanaged SR-IOV
@ 2018-03-02 23:44 Alexander Duyck
  2018-03-02 23:44 ` [PATCH 1/3] " Alexander Duyck
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Alexander Duyck @ 2018-03-02 23:44 UTC (permalink / raw)
  To: bhelgaas, alexander.h.duyck, linux-pci
  Cc: virtio-dev, kvm, netdev, dan.daly, linux-kernel, mheyne,
	liang-min.wang, mark.d.rustad, dwmw2, dwmw

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.

I have included the authors of the original 4 patches above in the Cc here.
My hope is to get feedback and/or review on if this works for their use
cases.

My hope is that for now the pci-stub and uio driver approaches can be 
addressed using the current patch that enables vfio-pci support. The only
limitation is that it is also setting the taint flag until we have a better 
solution.

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

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 (3):
      pci-iov: Add support for unmanaged SR-IOV
      vfio: Add support for unmanaged or userspace managed SR-IOV
      virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices


 Documentation/ABI/testing/sysfs-bus-pci |   17 +++++++++
 drivers/pci/iov.c                       |   37 ++++++++++++++++++++
 drivers/pci/pci-driver.c                |    2 +
 drivers/pci/pci-sysfs.c                 |   29 ++++++++++++++++
 drivers/pci/pci.h                       |    4 ++
 drivers/vfio/pci/vfio_pci.c             |   57 +++++++++++++++++++++++++++++++
 drivers/virtio/virtio_pci_common.c      |    4 ++
 include/linux/pci.h                     |    1 +
 8 files changed, 149 insertions(+), 2 deletions(-)

--

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] pci-iov: Add support for unmanaged SR-IOV
  2018-03-02 23:44 [PATCH 0/3] pci-iov: Add support for unmanaged SR-IOV Alexander Duyck
@ 2018-03-02 23:44 ` Alexander Duyck
  2018-03-02 23:59   ` Alex Williamson
  2018-03-02 23:44 ` [PATCH 2/3] vfio: Add support for unmanaged or userspace managed SR-IOV Alexander Duyck
  2018-03-02 23:45 ` [PATCH 3/3] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices Alexander Duyck
  2 siblings, 1 reply; 7+ messages in thread
From: Alexander Duyck @ 2018-03-02 23:44 UTC (permalink / raw)
  To: bhelgaas, alexander.h.duyck, linux-pci
  Cc: virtio-dev, kvm, netdev, dan.daly, linux-kernel, mheyne,
	liang-min.wang, mark.d.rustad, dwmw2, dwmw

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

This patch is meant to add some basic functionality to support for SR-IOV
on devices when the VFs are not managed by the kernel. The functions
provided here can be used by drivers such as vfio-pci and virtio to enable
SR-IOV on devices that are either managed by userspace, or by some sort of
firmware entity respectively.

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 userspace or 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 is
disabled. Attempts to update them when SR-IOV is in use will only update
the local value and will not update sriov->autoprobe.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-pci |   17 ++++++++++++++
 drivers/pci/iov.c                       |   37 +++++++++++++++++++++++++++++++
 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, 88 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
index 44d4b2be92fd..ff0b6c19cb1a 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -323,3 +323,20 @@ 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 <alexander.h.duyck@intel.com>
+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.
+
+		This overrides /sys/bus/pci/devices/.../sriov_drivers_autoprobe
+		when a PF driver is not present to manage a device, or the PF
+		driver does not provide functionality to support SR-IOV.
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 677924ae0350..3dcec1fa86bd 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -446,6 +446,7 @@ static int sriov_init(struct pci_dev *dev, int pos)
 	pci_read_config_word(dev, pos + PCI_SRIOV_VF_DID, &iov->vf_device);
 	iov->pgsz = pgsz;
 	iov->self = dev;
+	iov->autoprobe = true;
 	iov->drivers_autoprobe = true;
 	pci_read_config_dword(dev, pos + PCI_SRIOV_CAP, &iov->cap);
 	pci_read_config_byte(dev, pos + PCI_SRIOV_FUNC_LINK, &iov->link);
@@ -683,6 +684,9 @@ int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn)
 	if (!dev->is_physfn)
 		return -ENOSYS;
 
+	/* Update autoprobe setting to reflect managed device */
+	dev->sriov->autoprobe = dev->sriov->drivers_autoprobe;
+
 	return sriov_enable(dev, nr_virtfn);
 }
 EXPORT_SYMBOL_GPL(pci_enable_sriov);
@@ -807,3 +811,36 @@ int pci_sriov_get_totalvfs(struct pci_dev *dev)
 	return dev->sriov->total_VFs;
 }
 EXPORT_SYMBOL_GPL(pci_sriov_get_totalvfs);
+
+/**
+ * pci_sriov_configure_unmanaged - 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, or have no
+ * driver present.
+ */
+int pci_sriov_configure_unmanaged(struct pci_dev *dev, int nr_virtfn)
+{
+	int err;
+
+	might_sleep();
+
+	if (!dev->is_physfn)
+		return -ENODEV;
+
+	if (!nr_virtfn) {
+		sriov_disable(dev);
+
+		return 0;
+	}
+
+	/* Update autoprobe setting to reflect unmanaged device */
+	dev->sriov->autoprobe = dev->sriov->unmanaged_autoprobe;
+
+	err = sriov_enable(dev, nr_virtfn);
+
+	return err ? err : nr_virtfn;
+}
+EXPORT_SYMBOL_GPL(pci_sriov_configure_unmanaged);
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 3bed6beda051..2cc68dff6130 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -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)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index eb6bee8724cc..6f78fa73e317 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -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,
 };
 
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index fcd81911b127..b5f8b034f02d 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -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 */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 024a1beda008..553860a08131 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_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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] vfio: Add support for unmanaged or userspace managed SR-IOV
  2018-03-02 23:44 [PATCH 0/3] pci-iov: Add support for unmanaged SR-IOV Alexander Duyck
  2018-03-02 23:44 ` [PATCH 1/3] " Alexander Duyck
@ 2018-03-02 23:44 ` Alexander Duyck
  2018-03-06  4:49   ` kbuild test robot
  2018-03-02 23:45 ` [PATCH 3/3] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices Alexander Duyck
  2 siblings, 1 reply; 7+ messages in thread
From: Alexander Duyck @ 2018-03-02 23:44 UTC (permalink / raw)
  To: bhelgaas, alexander.h.duyck, linux-pci
  Cc: virtio-dev, kvm, netdev, dan.daly, linux-kernel, mheyne,
	liang-min.wang, mark.d.rustad, dwmw2, dwmw

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

This patch is meant to allow assignment of an SR-IOV enabled PF, as in VFs
have been generated, with vfio-pci. My understanding is the primary use
case for this is something like DPDK running the PF while the VFs are all
assigned to guests.

A secondary effect of this is that it provides an interface through which
it would be possible to enable SR-IOV on drivers that may not have a
physical function that actually manages the device.

Enabling SR-IOV should be pretty straight forward. As long as there are no
userspace processes currently controlling the interface the number of VFs
can be changed, and VFs will be generated without drivers being loaded on
the host. Once the userspace process begins controlling the interface the
number of VFs cannot be updated via the sysfs until the control is
released.

Note the VFs will have drivers load on them in the host if the
sriov_unmanaged_autoprobe is updated to a value of 1. However the behavior
of the VFs in such a setup cannot be guaranteed as the PF will not be
available until the userspace process starts and begins to manage the
device.

For now I am leaving the value as locked when the PF is being controlled
from userspace as a form of synchronization. Basically this way we cannot
have the number of VFs change out from under the process so it should not
require any notification framework, and the configuration can just be read
out via configuration space accesses.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 drivers/vfio/pci/vfio_pci.c |   57 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index b0f759476900..3023bda39aa9 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -1224,6 +1224,8 @@ static void vfio_pci_remove(struct pci_dev *pdev)
 				VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM);
 	}
 
+	pci_disable_sriov(pdev);
+
 	if (!disable_idle_d3)
 		pci_set_power_state(pdev, PCI_D0);
 }
@@ -1260,12 +1262,67 @@ static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev,
 	.error_detected = vfio_pci_aer_err_detected,
 };
 
+static int vfio_pci_sriov_configure(struct pci_dev *pdev, int nr_virtfn)
+{
+	struct vfio_pci_device *vdev;
+	struct vfio_device *device;
+	int err;
+
+	device = vfio_device_get_from_dev(&pdev->dev);
+	if (device == NULL)
+		return -ENODEV;
+
+	vdev = vfio_device_data(device);
+	if (vdev == NULL) {
+		vfio_device_put(device);
+		return -ENODEV;
+	}
+
+	/*
+	 * If a userspace process is already using this device just return
+	 * busy and don't allow for any changes.
+	 */
+	if (vdev->refcnt) {
+		pci_warn(pdev,
+			 "PF is currently in use, blocked until released by user\n");
+		return -EBUSY;
+	}
+
+	err = pci_sriov_configure_unmanaged(pdev, nr_virtfn);
+	if (err <= 0)
+		return err;
+
+	/*
+	 * We are now leaving VFs in the control of some unknown PF entity.
+	 *
+	 * Best case is a well behaved userspace PF is expected and any VMs
+	 * that the VFs will be assigned to are dependent on the userspace
+	 * entity anyway. An example being NFV where maybe the PF is acting
+	 * as an accelerated interface for a firewall or switch.
+	 *
+	 * Worst case is somebody really messed up and just enabled SR-IOV
+	 * on a device they were planning to assign to a VM somwhere.
+	 *
+	 * In either case it is probably best for us to set the taint flag
+	 * and warn the user since this could get really ugly really quick
+	 * if this wasn't what they were planning to do.
+	 */
+	add_taint(TAINT_USER, LOCKDEP_STILL_OK);
+	pci_warn(pdev,
+		 "Adding kernel taint for vfio-pci now managing SR-IOV PF device\n");
+
+	return nr_virtfn;
+}
+
 static struct pci_driver vfio_pci_driver = {
 	.name		= "vfio-pci",
 	.id_table	= NULL, /* only dynamic ids */
 	.probe		= vfio_pci_probe,
 	.remove		= vfio_pci_remove,
 	.err_handler	= &vfio_err_handlers,
+#ifdef CONFIG_PCI_IOV
+	.sriov_configure = vfio_pci_sriov_configure,
+#endif
 };
 
 struct vfio_devices {

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices
  2018-03-02 23:44 [PATCH 0/3] pci-iov: Add support for unmanaged SR-IOV Alexander Duyck
  2018-03-02 23:44 ` [PATCH 1/3] " Alexander Duyck
  2018-03-02 23:44 ` [PATCH 2/3] vfio: Add support for unmanaged or userspace managed SR-IOV Alexander Duyck
@ 2018-03-02 23:45 ` Alexander Duyck
  2 siblings, 0 replies; 7+ messages in thread
From: Alexander Duyck @ 2018-03-02 23:45 UTC (permalink / raw)
  To: bhelgaas, alexander.h.duyck, linux-pci
  Cc: virtio-dev, kvm, netdev, dan.daly, linux-kernel, mheyne,
	liang-min.wang, mark.d.rustad, dwmw2, dwmw

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>
---
 drivers/virtio/virtio_pci_common.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 48d4d1cf1cb6..ca1549393255 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -584,6 +584,7 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
 	else
 		virtio_pci_modern_remove(vp_dev);
 
+	pci_disable_sriov(pci_dev);
 	pci_disable_device(pci_dev);
 	put_device(dev);
 }
@@ -596,6 +597,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);

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] pci-iov: Add support for unmanaged SR-IOV
  2018-03-02 23:44 ` [PATCH 1/3] " Alexander Duyck
@ 2018-03-02 23:59   ` Alex Williamson
  2018-03-03  0:20     ` Alexander Duyck
  0 siblings, 1 reply; 7+ messages in thread
From: Alex Williamson @ 2018-03-02 23:59 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: bhelgaas, alexander.h.duyck, linux-pci, virtio-dev, kvm, netdev,
	dan.daly, linux-kernel, mheyne, liang-min.wang, mark.d.rustad,
	dwmw2, dwmw

On Fri, 02 Mar 2018 15:44:25 -0800
Alexander Duyck <alexander.duyck@gmail.com> wrote:

> From: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> This patch is meant to add some basic functionality to support for SR-IOV
> on devices when the VFs are not managed by the kernel. The functions
> provided here can be used by drivers such as vfio-pci and virtio to enable
> SR-IOV on devices that are either managed by userspace, or by some sort of
> firmware entity respectively.
> 
> 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 userspace or 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 is
> disabled. Attempts to update them when SR-IOV is in use will only update
> the local value and will not update sriov->autoprobe.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-pci |   17 ++++++++++++++
>  drivers/pci/iov.c                       |   37 +++++++++++++++++++++++++++++++
>  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, 88 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
> index 44d4b2be92fd..ff0b6c19cb1a 100644
> --- a/Documentation/ABI/testing/sysfs-bus-pci
> +++ b/Documentation/ABI/testing/sysfs-bus-pci
> @@ -323,3 +323,20 @@ 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 <alexander.h.duyck@intel.com>
> +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.
> +
> +		This overrides /sys/bus/pci/devices/.../sriov_drivers_autoprobe
> +		when a PF driver is not present to manage a device, or the PF
> +		driver does not provide functionality to support SR-IOV.


Given a pf, how does a user determine whether it is managed or unmanaged
and therefore which autoprobe attributes are in effect?  Thanks,

Alex

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] pci-iov: Add support for unmanaged SR-IOV
  2018-03-02 23:59   ` Alex Williamson
@ 2018-03-03  0:20     ` Alexander Duyck
  0 siblings, 0 replies; 7+ messages in thread
From: Alexander Duyck @ 2018-03-03  0:20 UTC (permalink / raw)
  To: Alex Williamson
  Cc: Bjorn Helgaas, Duyck, Alexander H, linux-pci, virtio-dev, kvm,
	Netdev, Daly, Dan, LKML, Maximilian Heyne, Wang, Liang-min,
	Rustad, Mark D, David Woodhouse, dwmw

On Fri, Mar 2, 2018 at 3:59 PM, Alex Williamson
<alex.williamson@redhat.com> wrote:
> On Fri, 02 Mar 2018 15:44:25 -0800
> Alexander Duyck <alexander.duyck@gmail.com> wrote:
>
>> From: Alexander Duyck <alexander.h.duyck@intel.com>
>>
>> This patch is meant to add some basic functionality to support for SR-IOV
>> on devices when the VFs are not managed by the kernel. The functions
>> provided here can be used by drivers such as vfio-pci and virtio to enable
>> SR-IOV on devices that are either managed by userspace, or by some sort of
>> firmware entity respectively.
>>
>> 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 userspace or 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 is
>> disabled. Attempts to update them when SR-IOV is in use will only update
>> the local value and will not update sriov->autoprobe.
>>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> ---
>>  Documentation/ABI/testing/sysfs-bus-pci |   17 ++++++++++++++
>>  drivers/pci/iov.c                       |   37 +++++++++++++++++++++++++++++++
>>  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, 88 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
>> index 44d4b2be92fd..ff0b6c19cb1a 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-pci
>> +++ b/Documentation/ABI/testing/sysfs-bus-pci
>> @@ -323,3 +323,20 @@ 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 <alexander.h.duyck@intel.com>
>> +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.
>> +
>> +             This overrides /sys/bus/pci/devices/.../sriov_drivers_autoprobe
>> +             when a PF driver is not present to manage a device, or the PF
>> +             driver does not provide functionality to support SR-IOV.
>
>
> Given a pf, how does a user determine whether it is managed or unmanaged
> and therefore which autoprobe attributes are in effect?  Thanks,
>
> Alex

Basically it comes down to what driver is loaded on it. For now
vfio-pci and virtio would be the only two using the "unmanaged"
version of things.

Really you don't know which autoprobe is in effect until SR-IOV is
enabled by whatever driver. As such you should really be setting both
the drivers_autoprobe and the unmanaged_autoprobe based on the
expected use case.

- Alex

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] vfio: Add support for unmanaged or userspace managed SR-IOV
  2018-03-02 23:44 ` [PATCH 2/3] vfio: Add support for unmanaged or userspace managed SR-IOV Alexander Duyck
@ 2018-03-06  4:49   ` kbuild test robot
  0 siblings, 0 replies; 7+ messages in thread
From: kbuild test robot @ 2018-03-06  4:49 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: kbuild-all, bhelgaas, alexander.h.duyck, linux-pci, virtio-dev,
	kvm, netdev, dan.daly, linux-kernel, mheyne, liang-min.wang,
	mark.d.rustad, dwmw2, dwmw

[-- Attachment #1: Type: text/plain, Size: 3594 bytes --]

Hi Alexander,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on pci/next]
[also build test ERROR on v4.16-rc4 next-20180305]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Alexander-Duyck/pci-iov-Add-support-for-unmanaged-SR-IOV/20180306-063954
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: s390-default_defconfig (attached as .config)
compiler: s390x-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=s390 

All errors (new ones prefixed by >>):

   drivers/vfio/pci/vfio_pci.c: In function 'vfio_pci_sriov_configure':
>> drivers/vfio/pci/vfio_pci.c:1291:8: error: implicit declaration of function 'pci_sriov_configure_unmanaged'; did you mean 'pci_write_config_dword'? [-Werror=implicit-function-declaration]
     err = pci_sriov_configure_unmanaged(pdev, nr_virtfn);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           pci_write_config_dword
   At top level:
   drivers/vfio/pci/vfio_pci.c:1265:12: warning: 'vfio_pci_sriov_configure' defined but not used [-Wunused-function]
    static int vfio_pci_sriov_configure(struct pci_dev *pdev, int nr_virtfn)
               ^~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +1291 drivers/vfio/pci/vfio_pci.c

  1264	
  1265	static int vfio_pci_sriov_configure(struct pci_dev *pdev, int nr_virtfn)
  1266	{
  1267		struct vfio_pci_device *vdev;
  1268		struct vfio_device *device;
  1269		int err;
  1270	
  1271		device = vfio_device_get_from_dev(&pdev->dev);
  1272		if (device == NULL)
  1273			return -ENODEV;
  1274	
  1275		vdev = vfio_device_data(device);
  1276		if (vdev == NULL) {
  1277			vfio_device_put(device);
  1278			return -ENODEV;
  1279		}
  1280	
  1281		/*
  1282		 * If a userspace process is already using this device just return
  1283		 * busy and don't allow for any changes.
  1284		 */
  1285		if (vdev->refcnt) {
  1286			pci_warn(pdev,
  1287				 "PF is currently in use, blocked until released by user\n");
  1288			return -EBUSY;
  1289		}
  1290	
> 1291		err = pci_sriov_configure_unmanaged(pdev, nr_virtfn);
  1292		if (err <= 0)
  1293			return err;
  1294	
  1295		/*
  1296		 * We are now leaving VFs in the control of some unknown PF entity.
  1297		 *
  1298		 * Best case is a well behaved userspace PF is expected and any VMs
  1299		 * that the VFs will be assigned to are dependent on the userspace
  1300		 * entity anyway. An example being NFV where maybe the PF is acting
  1301		 * as an accelerated interface for a firewall or switch.
  1302		 *
  1303		 * Worst case is somebody really messed up and just enabled SR-IOV
  1304		 * on a device they were planning to assign to a VM somwhere.
  1305		 *
  1306		 * In either case it is probably best for us to set the taint flag
  1307		 * and warn the user since this could get really ugly really quick
  1308		 * if this wasn't what they were planning to do.
  1309		 */
  1310		add_taint(TAINT_USER, LOCKDEP_STILL_OK);
  1311		pci_warn(pdev,
  1312			 "Adding kernel taint for vfio-pci now managing SR-IOV PF device\n");
  1313	
  1314		return nr_virtfn;
  1315	}
  1316	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 17664 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-03-06  4:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-02 23:44 [PATCH 0/3] pci-iov: Add support for unmanaged SR-IOV Alexander Duyck
2018-03-02 23:44 ` [PATCH 1/3] " Alexander Duyck
2018-03-02 23:59   ` Alex Williamson
2018-03-03  0:20     ` Alexander Duyck
2018-03-02 23:44 ` [PATCH 2/3] vfio: Add support for unmanaged or userspace managed SR-IOV Alexander Duyck
2018-03-06  4:49   ` kbuild test robot
2018-03-02 23:45 ` [PATCH 3/3] virtio_pci: Add support for unmanaged SR-IOV on virtio_pci devices Alexander Duyck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).