diff mbox

[2/3] vfio: Add support for unmanaged or userspace managed SR-IOV

Message ID 20180302234432.3337.96123.stgit@localhost.localdomain (mailing list archive)
State New, archived
Headers show

Commit Message

Alexander Duyck March 2, 2018, 11:44 p.m. UTC 
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(+)

Comments

kernel test robot March 6, 2018, 4:49 a.m. UTC | #1 
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
diff mbox

Patch

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 {