11:02 p.m.
Before a PCI device can be assigned to a guest with VFIO, that device
must be bound to the vfio-pci driver rather than to the device's
normal driver. The vfio-pci driver provides APIs that permit QEMU to
perform all the necessary operations to make the device accessible to
the guest.
There has been kernel work recently to support vendor/device-specific
VFIO drivers that provide the basic vfio-pci driver functionality
while adding support for device-specific operations (for example these
device-specific drivers are planned to support live migration of
certain devices). All that will be needed to make this functionality
available will be to bind the new vendor-specific driver to the device
(rather than the generic vfio-pci driver, which will continue to work
just without the extra functionality).
But until now libvirt has required that all PCI devices being assigned
to a guest with VFIO specifically have the "vfio-pci" driver bound to
the device. So even if the user manually binds a shiny new
vendor-specific driver to the device (and puts "managed='no'" in the
config to prevent libvirt from changing that), libvirt will just fail
during startup of the guest (or during hotplug) because the driver
bound to the device isn't named exactly "vfio-pci".
Fortunately these new vendor/device-specific drivers can be easily
identified as being "vfio-pci + extra stuff" - all that's needed is to
look at the output of the "modinfo $driver_name" command to see if
"vfio_pci" is in the alias list for the driver.
That's what this patch does. When libvirt checks the driver bound to a
device (either to decide if it needs to bind to a different driver or
perform some other operation, or if the current driver is acceptable
as-is), if the driver isn't specifically "vfio-pci", then it will look
at the output of modinfo for the driver that *is* bound to the device;
if modinfo shows vfio_pci as an alias for that device, then we'll
behave as if the driver was exactly "vfio-pci".
The effect of this patch is that users will now be able to pre-setup a
device to be bound to a vendor-specific driver, then put
"managed='no'" in the config and libvirt will allow that driver.
What this patch does *not* do is handle automatically determining the
proper/best vendor-specific driver and binding to it in the case of
"managed='yes'". This will be implemented later when there is a widely
available driver / device combo we can use for testing. This initial
simple patch is just something simple that will permit initial testing
of the new drivers' functionality.
(I personally had to add an extra patch playing with driver names to
my build just to test that everything was working as expected; that's
okay for a patch as simple as this, but wouldn't be acceptable testing
for anything more complex.)
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
meson.build | 1 +
src/hypervisor/virhostdev.c | 26 ++++-------
src/util/virpci.c | 90 ++++++++++++++++++++++++++++++++++---
src/util/virpci.h | 3 ++
4 files changed, 97 insertions(+), 23 deletions(-)
diff --git a/meson.build b/meson.build
index de59b1be9c..9d96eb3ee3 100644
--- a/meson.build
+++ b/meson.build
@@ -822,6 +822,7 @@ optional_programs = [
'iscsiadm',
'mdevctl',
'mm-ctl',
+ 'modinfo',
'modprobe',
'ovs-vsctl',
'pdwtags',
diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c
index c0ce867596..15b35fa75e 100644
--- a/src/hypervisor/virhostdev.c
+++ b/src/hypervisor/virhostdev.c
@@ -747,9 +747,8 @@ virHostdevPreparePCIDevicesImpl(virHostdevManager *mgr,
mgr->inactivePCIHostdevs) < 0)
goto reattachdevs;
} else {
- g_autofree char *driverPath = NULL;
- g_autofree char *driverName = NULL;
- int stub;
+ g_autofree char *drvName = NULL;
+ virPCIStubDriver drvType;
/* Unmanaged devices should already have been marked as
* inactive: if that's the case, we can simply move on */
@@ -769,18 +768,14 @@ virHostdevPreparePCIDevicesImpl(virHostdevManager *mgr,
* information about active / inactive device across
* daemon restarts has been implemented */
- if (virPCIDeviceGetDriverPathAndName(pci,
- &driverPath, &driverName) <
0)
+ if (virPCIDeviceGetDriverNameAndType(pci, &drvName, &drvType) <
0)
goto reattachdevs;
- stub = virPCIStubDriverTypeFromString(driverName);
-
- if (stub > VIR_PCI_STUB_DRIVER_NONE &&
- stub < VIR_PCI_STUB_DRIVER_LAST) {
+ if (drvType > VIR_PCI_STUB_DRIVER_NONE) {
/* The device is bound to a known stub driver: store this
* information and add a copy to the inactive list */
- virPCIDeviceSetStubDriver(pci, stub);
+ virPCIDeviceSetStubDriver(pci, drvType);
VIR_DEBUG("Adding PCI device %s to inactive list",
virPCIDeviceGetName(pci));
@@ -2292,18 +2287,13 @@ virHostdevPrepareOneNVMeDevice(virHostdevManager *hostdev_mgr,
/* Let's check if all PCI devices are NVMe disks. */
for (i = 0; i < virPCIDeviceListCount(pciDevices); i++) {
virPCIDevice *pci = virPCIDeviceListGet(pciDevices, i);
- g_autofree char *drvPath = NULL;
g_autofree char *drvName = NULL;
- int stub = VIR_PCI_STUB_DRIVER_NONE;
+ virPCIStubDriver drvType;
- if (virPCIDeviceGetDriverPathAndName(pci, &drvPath, &drvName) < 0)
+ if (virPCIDeviceGetDriverNameAndType(pci, &drvName, &drvType) < 0)
goto cleanup;
- if (drvName)
- stub = virPCIStubDriverTypeFromString(drvName);
-
- if (stub == VIR_PCI_STUB_DRIVER_VFIO ||
- STREQ_NULLABLE(drvName, "nvme"))
+ if (drvType == VIR_PCI_STUB_DRIVER_VFIO || STREQ_NULLABLE(drvName,
"nvme"))
continue;
VIR_WARN("Suspicious NVMe disk assignment. PCI device "
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 7800966963..8b714d3ddc 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -37,6 +37,7 @@
#include "virstring.h"
#include "viralloc.h"
#include "virpcivpd.h"
+#include "vircommand.h"
VIR_LOG_INIT("util.pci");
@@ -277,6 +278,84 @@ virPCIDeviceGetDriverPathAndName(virPCIDevice *dev, char **path, char
**name)
}
+/**
+ * virPCIDeviceGetDriverNameAndType:
+ * @dev: virPCIDevice object to examine
+ * @drvName: returns name of driver bound to this device (if any)
+ * @drvType: returns type of driver if it is a known stub driver type
+ *
+ * Find the name of the driver bound to @dev (if any) and the type of
+ * the driver if it is a known/recognized "stub" driver (based on the
+ * name). If the name doesn't match one of the virPCIStubDrivers
+ * exactly, check the output of "modinfo vfio-pci" to see if
+ * "vfio_pci" is included in the driver's list of aliases; if so, then
+ * this driver has all the functionality of the basic vfio_pci driver,
+ * so it should be considered of the type VIR_PCI_STUB_DRIVER_VFIO.
+ *
+ * Return 0 on success, -1 on failure. If -1 is returned, then an error
+ * message has been logged.
+ */
+int
+virPCIDeviceGetDriverNameAndType(virPCIDevice *dev,
+ char **drvName,
+ virPCIStubDriver *drvType)
+{
+ g_autofree char *drvPath = NULL;
+ g_autoptr(virCommand) cmd = NULL;
+ g_autofree char *output = NULL;
+ g_autoptr(GRegex) regex = NULL;
+ g_autoptr(GError) err = NULL;
+ g_autoptr(GMatchInfo) info = NULL;
+ int exit;
+ int tmpType;
+
+ if (virPCIDeviceGetDriverPathAndName(dev, &drvPath, drvName) < 0)
+ return -1;
+
+ if (!*drvName) {
+ *drvType = VIR_PCI_STUB_DRIVER_NONE;
+ return 0;
+ }
+
+ tmpType = virPCIStubDriverTypeFromString(*drvName);
+
+ if (tmpType > VIR_PCI_STUB_DRIVER_NONE) {
+ *drvType = tmpType;
+ return 0; /* exact match of a known driver name (or no name) */
+ }
+
+ /* Check the output of "modinfo $drvName" to see if it has
+ * "vfio_pci" as an alias. If it does, then this driver should
+ * also be considered as a vfio-pci driver, because it implements
+ * all the functionality of the basic vfio-pci (plus additional
+ * device-specific stuff).
+ */
+
+ cmd = virCommandNewArgList(MODINFO, *drvName, NULL);
+ virCommandSetOutputBuffer(cmd, &output);
+ if (virCommandRun(cmd, &exit) < 0)
+ return -1;
+
+ regex = g_regex_new("^alias: +vfio_pci:", G_REGEX_MULTILINE, 0, &err);
+ if (!regex) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to compile regex %s"), err->message);
+ return -1;
+ }
+
+ g_regex_match(regex, output, 0, &info);
+ if (g_match_info_matches(info)) {
+ VIR_DEBUG("Driver %s is a vfio_pci driver", *drvName);
+ *drvType = VIR_PCI_STUB_DRIVER_VFIO;
+ } else {
+ VIR_DEBUG("Driver %s is NOT a vfio_pci driver", *drvName);
+ *drvType = VIR_PCI_STUB_DRIVER_NONE;
+ }
+
+ return 0;
+}
+
+
static int
virPCIDeviceConfigOpenInternal(virPCIDevice *dev, bool readonly, bool fatal)
{
@@ -1004,8 +1083,8 @@ virPCIDeviceReset(virPCIDevice *dev,
virPCIDeviceList *activeDevs,
virPCIDeviceList *inactiveDevs)
{
- g_autofree char *drvPath = NULL;
g_autofree char *drvName = NULL;
+ virPCIStubDriver drvType;
int ret = -1;
int fd = -1;
int hdrType = -1;
@@ -1032,15 +1111,16 @@ virPCIDeviceReset(virPCIDevice *dev,
* reset it whenever appropriate, so doing it ourselves would just
* be redundant.
*/
- if (virPCIDeviceGetDriverPathAndName(dev, &drvPath, &drvName) < 0)
+ if (virPCIDeviceGetDriverNameAndType(dev, &drvName, &drvType) < 0)
goto cleanup;
- if (virPCIStubDriverTypeFromString(drvName) == VIR_PCI_STUB_DRIVER_VFIO) {
- VIR_DEBUG("Device %s is bound to vfio-pci - skip reset",
- dev->name);
+ if (drvType == VIR_PCI_STUB_DRIVER_VFIO) {
+
+ VIR_DEBUG("Device %s is bound to %s - skip reset", dev->name,
drvName);
ret = 0;
goto cleanup;
}
+
VIR_DEBUG("Resetting device %s", dev->name);
if ((fd = virPCIDeviceConfigOpenWrite(dev)) < 0)
diff --git a/src/util/virpci.h b/src/util/virpci.h
index 4d9193f24e..0532b90f90 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -280,6 +280,9 @@ int virPCIDeviceRebind(virPCIDevice *dev);
int virPCIDeviceGetDriverPathAndName(virPCIDevice *dev,
char **path,
char **name);
+int virPCIDeviceGetDriverNameAndType(virPCIDevice *dev,
+ char **drvName,
+ virPCIStubDriver *drvType);
int virPCIDeviceIsPCIExpress(virPCIDevice *dev);
int virPCIDeviceHasPCIExpressLink(virPCIDevice *dev);
--
2.35.3