On 8/29/19 9:18 PM, Daniel Henrique Barboza wrote:
This patch adds mock of the /dev/vfio path, needed for proper
implementation of the support for multifunction/multiple devices
per iommu groups.
To do that, the existing bind and unbind operations were adapted
to operate with the mocked filesystem as well.
Signed-off-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>
---
tests/virpcimock.c | 159 ++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 143 insertions(+), 16 deletions(-)
diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index a5045ed97c..e9440e7910 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -47,6 +47,7 @@ char *fakerootdir;
# define SYSFS_PCI_PREFIX "/sys/bus/pci/"
+
# define STDERR(...) \
fprintf(stderr, "%s %zu: ", __FUNCTION__, (size_t) __LINE__); \
fprintf(stderr, __VA_ARGS__); \
@@ -105,6 +106,11 @@ struct pciDriver {
size_t len; /* @len is used for both @vendor and @device */
};
+struct pciIommuGroup {
+ int iommu;
+ size_t nDevicesBoundToVFIO; /* Indicates the devices in the group */
+};
+
struct pciDeviceAddress {
unsigned int domain;
unsigned int bus;
@@ -133,6 +139,9 @@ size_t nPCIDevices = 0;
struct pciDriver **pciDrivers = NULL;
size_t nPCIDrivers = 0;
+struct pciIommuGroup **pciIommuGroups = NULL;
+size_t npciIommuGroups = 0;
+
struct fdCallback *callbacks = NULL;
size_t nCallbacks = 0;
@@ -254,6 +263,15 @@ getrealpath(char **newpath,
errno = ENOMEM;
return -1;
}
+ } else if (STRPREFIX(path, "/sys/kernel/") ||
+ STRPREFIX(path, "/dev/vfio/")) {
+
+ if (virAsprintfQuiet(newpath, "%s/%s",
+ fakerootdir,
+ path) < 0) {
+ errno = ENOMEM;
+ return -1;
+ }
} else {
if (VIR_STRDUP_QUIET(*newpath, path) < 0)
return -1;
@@ -389,8 +407,10 @@ static void
pci_device_create_iommu(const struct pciDevice *dev,
const char *devid)
{
+ struct pciIommuGroup *iommuGroup;
VIR_AUTOFREE(char *) iommuPath = NULL;
char tmp[256];
+ size_t i;
if (virAsprintfQuiet(&iommuPath, "%s/sys/kernel/iommu_groups/%d/devices/",
fakerootdir, dev->iommuGroup) < 0)
@@ -406,6 +426,23 @@ pci_device_create_iommu(const struct pciDevice *dev,
}
make_symlink(iommuPath, devid, tmp);
+
+ /* pci_device_create_iommu can be called more than one for the
+ * same iommuGroup. Bail out here if the iommuGroup was already
+ * created beforehand. */
+ for (i = 0; i < npciIommuGroups; i++)
+ if (pciIommuGroups[i]->iommu == dev->iommuGroup)
+ return;
+
+ if (VIR_ALLOC_QUIET(iommuGroup) < 0)
+ ABORT_OOM();
+
+ iommuGroup->iommu = dev->iommuGroup;
+ iommuGroup->nDevicesBoundToVFIO = 0; /* No device bound to VFIO by default */
+
+ if (VIR_APPEND_ELEMENT_QUIET(pciIommuGroups, npciIommuGroups,
+ iommuGroup) < 0)
+ ABORT_OOM();
}
@@ -558,6 +595,77 @@ pci_device_autobind(struct pciDevice *dev)
return pci_driver_bind(driver, dev);
}
+static int
+pci_vfio_release_iommu(struct pciDevice *device)
+{
+ VIR_AUTOFREE(char *) vfiopath = NULL;
+ int ret = -1;
+ size_t i = 0;
+
+ for (i = 0; i < npciIommuGroups; i++) {
+ if (pciIommuGroups[i]->iommu == device->iommuGroup) {
I'd use "if (!cond) continue" so that you can save one level of indend.
+
+ if (pciIommuGroups[i]->nDevicesBoundToVFIO == 0) {
+ ret = 0;
+ goto cleanup;
+ }
+
+ pciIommuGroups[i]->nDevicesBoundToVFIO--;
+
+ if (!pciIommuGroups[i]->nDevicesBoundToVFIO) {
+
+ if (virAsprintfQuiet(&vfiopath, "%s/dev/vfio/%d",
+ fakerootdir,
+ device->iommuGroup) < 0) {
+ errno = ENOMEM;
+ goto cleanup;
+ }
+
+ if (unlink(vfiopath) < 0)
+ goto cleanup;
+ }
+ break;
+ }
+ }
+
+ ret = 0;
+
+ cleanup:
+ return ret;
This cleanup label looks needless to me.
+}
+
+static int
+pci_vfio_lock_iommu(struct pciDevice *device)
+{
+ VIR_AUTOFREE(char *) vfiopath = NULL;
+ int ret = -1;
+ size_t i = 0;
+ int fd = -1;
+
+ for (i = 0; i < npciIommuGroups; i++) {
+ if (pciIommuGroups[i]->iommu == device->iommuGroup) {
+ if (!pciIommuGroups[i]->nDevicesBoundToVFIO) {
+ if (virAsprintfQuiet(&vfiopath, "%s/dev/vfio/%d",
+ fakerootdir,
+ device->iommuGroup) < 0) {
+ errno = ENOMEM;
+ goto cleanup;
+ }
+ if ((fd = real_open(vfiopath, O_CREAT)) < 0)
+ goto cleanup;
+
+ pciIommuGroups[i]->nDevicesBoundToVFIO++;
No, This needs to go outside of this if() as it needs to be incremented
every time @device falls into iommu group we're processing here.
+ }
+ break;
+ }
+ }
+
+ ret = 0;
+
+ cleanup:
+ real_close(fd);
close() won't work with fd == -1. It will overwrite any errno we set
earlier.
+ return ret;
+}
/*
* PCI Driver functions
@@ -719,6 +827,10 @@ pci_driver_bind(struct pciDriver *driver,
if (symlink(devpath, driverpath) < 0)
return -1;
+ if (STREQ(driver->name, "vfio-pci"))
+ if (pci_vfio_lock_iommu(dev) < 0)
+ return -1;
+
dev->driver = driver;
return 0;
}
@@ -749,6 +861,10 @@ pci_driver_unbind(struct pciDriver *driver,
unlink(driverpath) < 0)
return -1;
+ if (STREQ(driver->name, "vfio-pci"))
+ if (pci_vfio_release_iommu(dev) < 0)
+ return -1;
These can be joined in one condition.
+
dev->driver = NULL;
return 0;
}
@@ -865,6 +981,15 @@ init_env(void)
make_dir(tmp, "drivers");
make_file(tmp, "drivers_probe", NULL, -1);
+ /* Create /dev/vfio/ dir and /dev/vfio/vfio file */
+ if (virAsprintfQuiet(&tmp, "%s/dev/vfio", fakerootdir) < 0)
+ ABORT_OOM();
At this point, before executing virAsprintf() the @tmp variable is
allocated, so it must be freed to avoid leaking it.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
