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

[kbuild test robot]

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(>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 infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

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

[Alexander Duyck]

From: Alexander Duyck 

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 
---
 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(>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= _err_handlers,
+#ifdef CONFIG_PCI_IOV
+   .sriov_configure = vfio_pci_sriov_configure,
+#endif
 };
 
 struct vfio_devices {