Support tests that want to add multiple devices to the same
container/iommufd by decoupling struct vfio_pci_device from
struct iommu.

Multi-devices tests can now put multiple devices in the same
container/iommufd like so:

  iommu = iommu_init(iommu_mode);

  device1 = vfio_pci_device_init(bdf1, iommu);
  device2 = vfio_pci_device_init(bdf2, iommu);
  device3 = vfio_pci_device_init(bdf3, iommu);

  ...

  vfio_pci_device_cleanup(device3);
  vfio_pci_device_cleanup(device2);
  vfio_pci_device_cleanup(device1);

  iommu_cleanup(iommu);

To account for the new separation of vfio_pci_device and iommu, update
existing tests to initialize and cleanup a struct iommu.

Signed-off-by: David Matlack <[email protected]>
---
 .../selftests/vfio/lib/include/vfio_util.h    |   6 +-
 .../selftests/vfio/lib/vfio_pci_device.c      | 102 +++++++++++-------
 .../selftests/vfio/vfio_dma_mapping_test.c    |  10 +-
 .../selftests/vfio/vfio_pci_device_test.c     |  10 +-
 .../selftests/vfio/vfio_pci_driver_test.c     |  26 ++++-
 5 files changed, 105 insertions(+), 49 deletions(-)

diff --git a/tools/testing/selftests/vfio/lib/include/vfio_util.h 
b/tools/testing/selftests/vfio/lib/include/vfio_util.h
index 3160f2d1ea6d..379942dc5357 100644
--- a/tools/testing/selftests/vfio/lib/include/vfio_util.h
+++ b/tools/testing/selftests/vfio/lib/include/vfio_util.h
@@ -216,8 +216,12 @@ const char *vfio_pci_get_cdev_path(const char *bdf);
 
 extern const char *default_iommu_mode;
 
-struct vfio_pci_device *vfio_pci_device_init(const char *bdf, const char 
*iommu_mode);
+struct iommu *iommu_init(const char *iommu_mode);
+void iommu_cleanup(struct iommu *iommu);
+
+struct vfio_pci_device *vfio_pci_device_init(const char *bdf, struct iommu 
*iommu);
 void vfio_pci_device_cleanup(struct vfio_pci_device *device);
+
 void vfio_pci_device_reset(struct vfio_pci_device *device);
 
 struct iommu_iova_range *vfio_pci_iova_ranges(struct vfio_pci_device *device,
diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c 
b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
index e47f3ccf6d49..57bdd22573d4 100644
--- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
+++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
@@ -86,13 +86,13 @@ static struct vfio_iommu_type1_info 
*vfio_iommu_get_info(struct vfio_pci_device
                .argsz = sizeof(*info),
        };
 
-       ioctl_assert(device->container_fd, VFIO_IOMMU_GET_INFO, info);
+       ioctl_assert(device->iommu->container_fd, VFIO_IOMMU_GET_INFO, info);
        VFIO_ASSERT_GE(info->argsz, sizeof(*info));
 
        info = realloc(info, info->argsz);
        VFIO_ASSERT_NOT_NULL(info);
 
-       ioctl_assert(device->container_fd, VFIO_IOMMU_GET_INFO, info);
+       ioctl_assert(device->iommu->container_fd, VFIO_IOMMU_GET_INFO, info);
        VFIO_ASSERT_GE(info->argsz, sizeof(*info));
 
        return info;
@@ -142,10 +142,10 @@ static struct iommu_iova_range 
*iommufd_iova_ranges(struct vfio_pci_device *devi
 
        struct iommu_ioas_iova_ranges query = {
                .size = sizeof(query),
-               .ioas_id = device->ioas_id,
+               .ioas_id = device->iommu->ioas_id,
        };
 
-       ret = ioctl(device->iommufd, IOMMU_IOAS_IOVA_RANGES, &query);
+       ret = ioctl(device->iommu->iommufd, IOMMU_IOAS_IOVA_RANGES, &query);
        VFIO_ASSERT_EQ(ret, -1);
        VFIO_ASSERT_EQ(errno, EMSGSIZE);
        VFIO_ASSERT_GT(query.num_iovas, 0);
@@ -155,7 +155,7 @@ static struct iommu_iova_range *iommufd_iova_ranges(struct 
vfio_pci_device *devi
 
        query.allowed_iovas = (uintptr_t)ranges;
 
-       ioctl_assert(device->iommufd, IOMMU_IOAS_IOVA_RANGES, &query);
+       ioctl_assert(device->iommu->iommufd, IOMMU_IOAS_IOVA_RANGES, &query);
        *nranges = query.num_iovas;
 
        return ranges;
@@ -180,7 +180,7 @@ struct iommu_iova_range *vfio_pci_iova_ranges(struct 
vfio_pci_device *device,
 {
        struct iommu_iova_range *ranges;
 
-       if (device->iommufd)
+       if (device->iommu->iommufd)
                ranges = iommufd_iova_ranges(device, nranges);
        else
                ranges = vfio_iommu_iova_ranges(device, nranges);
@@ -633,23 +633,21 @@ static void vfio_pci_group_setup(struct vfio_pci_device 
*device, const char *bdf
 
 static void vfio_pci_container_setup(struct vfio_pci_device *device, const 
char *bdf)
 {
-       unsigned long iommu_type = device->iommu->mode->iommu_type;
-       const char *path = device->iommu->mode->container_path;
-       int version;
+       struct iommu *iommu = device->iommu;
+       unsigned long iommu_type = iommu->mode->iommu_type;
        int ret;
 
-       device->iommu->container_fd = open(path, O_RDWR);
-       VFIO_ASSERT_GE(device->iommu->container_fd, 0, "open(%s) failed\n", 
path);
-
-       version = ioctl(device->iommu->container_fd, VFIO_GET_API_VERSION);
-       VFIO_ASSERT_EQ(version, VFIO_API_VERSION, "Unsupported version: %d\n", 
version);
-
        vfio_pci_group_setup(device, bdf);
 
-       ret = ioctl(device->iommu->container_fd, VFIO_CHECK_EXTENSION, 
iommu_type);
+       ret = ioctl(iommu->container_fd, VFIO_CHECK_EXTENSION, iommu_type);
        VFIO_ASSERT_GT(ret, 0, "VFIO IOMMU type %lu not supported\n", 
iommu_type);
 
-       ioctl_assert(device->iommu->container_fd, VFIO_SET_IOMMU, (void 
*)iommu_type);
+       /*
+        * Allow multiple threads to race to set the IOMMU type on the
+        * container. The first will succeed and the rest should fail
+        * because the IOMMU type is already set.
+        */
+       (void)ioctl(iommu->container_fd, VFIO_SET_IOMMU, (void *)iommu_type);
 
        device->fd = ioctl(device->group_fd, VFIO_GROUP_GET_DEVICE_FD, bdf);
        VFIO_ASSERT_GE(device->fd, 0);
@@ -797,32 +795,53 @@ static void vfio_pci_iommufd_setup(struct vfio_pci_device 
*device, const char *b
        VFIO_ASSERT_GE(device->fd, 0);
        free((void *)cdev_path);
 
-       /*
-        * Require device->iommufd to be >0 so that a simple non-0 check can be
-        * used to check if iommufd is enabled. In practice open() will never
-        * return 0 unless stdin is closed.
-        */
-       device->iommu->iommufd = open("/dev/iommu", O_RDWR);
-       VFIO_ASSERT_GT(device->iommu->iommufd, 0);
-
        vfio_device_bind_iommufd(device->fd, device->iommu->iommufd);
-       device->iommu->ioas_id = iommufd_ioas_alloc(device->iommu->iommufd);
        vfio_device_attach_iommufd_pt(device->fd, device->iommu->ioas_id);
 }
 
-struct vfio_pci_device *vfio_pci_device_init(const char *bdf, const char 
*iommu_mode)
+struct iommu *iommu_init(const char *iommu_mode)
+{
+       const char *container_path;
+       struct iommu *iommu;
+       int version;
+
+       iommu = calloc(1, sizeof(*iommu));
+       VFIO_ASSERT_NOT_NULL(iommu);
+
+       INIT_LIST_HEAD(&iommu->dma_regions);
+
+       iommu->mode = lookup_iommu_mode(iommu_mode);
+
+       container_path = iommu->mode->container_path;
+       if (container_path) {
+               iommu->container_fd = open(container_path, O_RDWR);
+               VFIO_ASSERT_GE(iommu->container_fd, 0, "open(%s) failed\n", 
container_path);
+
+               version = ioctl(iommu->container_fd, VFIO_GET_API_VERSION);
+               VFIO_ASSERT_EQ(version, VFIO_API_VERSION, "Unsupported version: 
%d\n", version);
+       } else {
+               /*
+                * Require device->iommufd to be >0 so that a simple non-0 
check can be
+                * used to check if iommufd is enabled. In practice open() will 
never
+                * return 0 unless stdin is closed.
+                */
+               iommu->iommufd = open("/dev/iommu", O_RDWR);
+               VFIO_ASSERT_GT(iommu->iommufd, 0);
+
+               iommu->ioas_id = iommufd_ioas_alloc(iommu->iommufd);
+       }
+
+       return iommu;
+}
+
+struct vfio_pci_device *vfio_pci_device_init(const char *bdf, struct iommu 
*iommu)
 {
        struct vfio_pci_device *device;
 
        device = calloc(1, sizeof(*device));
        VFIO_ASSERT_NOT_NULL(device);
 
-       device->iommu = calloc(1, sizeof(*device->iommu));
-       VFIO_ASSERT_NOT_NULL(device->iommu);
-
-       INIT_LIST_HEAD(&device->iommu->dma_regions);
-
-       device->iommu->mode = lookup_iommu_mode(iommu_mode);
+       device->iommu = iommu;
 
        if (device->iommu->mode->container_path)
                vfio_pci_container_setup(device, bdf);
@@ -853,17 +872,22 @@ void vfio_pci_device_cleanup(struct vfio_pci_device 
*device)
                VFIO_ASSERT_EQ(close(device->msi_eventfds[i]), 0);
        }
 
-       if (device->iommu->iommufd) {
-               VFIO_ASSERT_EQ(close(device->iommu->iommufd), 0);
-       } else {
+       if (device->group_fd)
                VFIO_ASSERT_EQ(close(device->group_fd), 0);
-               VFIO_ASSERT_EQ(close(device->iommu->container_fd), 0);
-       }
 
-       free(device->iommu);
        free(device);
 }
 
+void iommu_cleanup(struct iommu *iommu)
+{
+       if (iommu->iommufd)
+               VFIO_ASSERT_EQ(close(iommu->iommufd), 0);
+       else
+               VFIO_ASSERT_EQ(close(iommu->container_fd), 0);
+
+       free(iommu);
+}
+
 static bool is_bdf(const char *str)
 {
        unsigned int s, b, d, f;
diff --git a/tools/testing/selftests/vfio/vfio_dma_mapping_test.c 
b/tools/testing/selftests/vfio/vfio_dma_mapping_test.c
index 102603d4407d..4727feb214c8 100644
--- a/tools/testing/selftests/vfio/vfio_dma_mapping_test.c
+++ b/tools/testing/selftests/vfio/vfio_dma_mapping_test.c
@@ -94,6 +94,7 @@ static int iommu_mapping_get(const char *bdf, u64 iova,
 }
 
 FIXTURE(vfio_dma_mapping_test) {
+       struct iommu *iommu;
        struct vfio_pci_device *device;
        struct iova_allocator *iova_allocator;
 };
@@ -119,7 +120,8 @@ FIXTURE_VARIANT_ADD_ALL_IOMMU_MODES(anonymous_hugetlb_1gb, 
SZ_1G, MAP_HUGETLB |
 
 FIXTURE_SETUP(vfio_dma_mapping_test)
 {
-       self->device = vfio_pci_device_init(device_bdf, variant->iommu_mode);
+       self->iommu = iommu_init(variant->iommu_mode);
+       self->device = vfio_pci_device_init(device_bdf, self->iommu);
        self->iova_allocator = iova_allocator_init(self->device);
 }
 
@@ -127,6 +129,7 @@ FIXTURE_TEARDOWN(vfio_dma_mapping_test)
 {
        iova_allocator_cleanup(self->iova_allocator);
        vfio_pci_device_cleanup(self->device);
+       iommu_cleanup(self->iommu);
 }
 
 TEST_F(vfio_dma_mapping_test, dma_map_unmap)
@@ -203,6 +206,7 @@ TEST_F(vfio_dma_mapping_test, dma_map_unmap)
 }
 
 FIXTURE(vfio_dma_map_limit_test) {
+       struct iommu *iommu;
        struct vfio_pci_device *device;
        struct vfio_dma_region region;
        size_t mmap_size;
@@ -235,7 +239,8 @@ FIXTURE_SETUP(vfio_dma_map_limit_test)
         */
        self->mmap_size = 2 * region_size;
 
-       self->device = vfio_pci_device_init(device_bdf, variant->iommu_mode);
+       self->iommu = iommu_init(variant->iommu_mode);
+       self->device = vfio_pci_device_init(device_bdf, self->iommu);
        region->vaddr = mmap(NULL, self->mmap_size, PROT_READ | PROT_WRITE,
                             MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
        ASSERT_NE(region->vaddr, MAP_FAILED);
@@ -253,6 +258,7 @@ FIXTURE_SETUP(vfio_dma_map_limit_test)
 FIXTURE_TEARDOWN(vfio_dma_map_limit_test)
 {
        vfio_pci_device_cleanup(self->device);
+       iommu_cleanup(self->iommu);
        ASSERT_EQ(munmap(self->region.vaddr, self->mmap_size), 0);
 }
 
diff --git a/tools/testing/selftests/vfio/vfio_pci_device_test.c 
b/tools/testing/selftests/vfio/vfio_pci_device_test.c
index 7a270698e4d2..e95217933c6b 100644
--- a/tools/testing/selftests/vfio/vfio_pci_device_test.c
+++ b/tools/testing/selftests/vfio/vfio_pci_device_test.c
@@ -23,17 +23,20 @@ static const char *device_bdf;
 #define MAX_TEST_MSI 16U
 
 FIXTURE(vfio_pci_device_test) {
+       struct iommu *iommu;
        struct vfio_pci_device *device;
 };
 
 FIXTURE_SETUP(vfio_pci_device_test)
 {
-       self->device = vfio_pci_device_init(device_bdf, default_iommu_mode);
+       self->iommu = iommu_init(default_iommu_mode);
+       self->device = vfio_pci_device_init(device_bdf, self->iommu);
 }
 
 FIXTURE_TEARDOWN(vfio_pci_device_test)
 {
        vfio_pci_device_cleanup(self->device);
+       iommu_cleanup(self->iommu);
 }
 
 #define read_pci_id_from_sysfs(_file) ({                                       
                \
@@ -99,6 +102,7 @@ TEST_F(vfio_pci_device_test, validate_bars)
 }
 
 FIXTURE(vfio_pci_irq_test) {
+       struct iommu *iommu;
        struct vfio_pci_device *device;
 };
 
@@ -116,12 +120,14 @@ FIXTURE_VARIANT_ADD(vfio_pci_irq_test, msix) {
 
 FIXTURE_SETUP(vfio_pci_irq_test)
 {
-       self->device = vfio_pci_device_init(device_bdf, default_iommu_mode);
+       self->iommu = iommu_init(default_iommu_mode);
+       self->device = vfio_pci_device_init(device_bdf, self->iommu);
 }
 
 FIXTURE_TEARDOWN(vfio_pci_irq_test)
 {
        vfio_pci_device_cleanup(self->device);
+       iommu_cleanup(self->iommu);
 }
 
 TEST_F(vfio_pci_irq_test, enable_trigger_disable)
diff --git a/tools/testing/selftests/vfio/vfio_pci_driver_test.c 
b/tools/testing/selftests/vfio/vfio_pci_driver_test.c
index f69eec8b928d..b0c7d812de1f 100644
--- a/tools/testing/selftests/vfio/vfio_pci_driver_test.c
+++ b/tools/testing/selftests/vfio/vfio_pci_driver_test.c
@@ -44,6 +44,7 @@ static void region_teardown(struct vfio_pci_device *device,
 }
 
 FIXTURE(vfio_pci_driver_test) {
+       struct iommu *iommu;
        struct vfio_pci_device *device;
        struct iova_allocator *iova_allocator;
        struct vfio_dma_region memcpy_region;
@@ -73,7 +74,8 @@ FIXTURE_SETUP(vfio_pci_driver_test)
 {
        struct vfio_pci_driver *driver;
 
-       self->device = vfio_pci_device_init(device_bdf, variant->iommu_mode);
+       self->iommu = iommu_init(variant->iommu_mode);
+       self->device = vfio_pci_device_init(device_bdf, self->iommu);
        self->iova_allocator = iova_allocator_init(self->device);
 
        driver = &self->device->driver;
@@ -113,6 +115,7 @@ FIXTURE_TEARDOWN(vfio_pci_driver_test)
 
        iova_allocator_cleanup(self->iova_allocator);
        vfio_pci_device_cleanup(self->device);
+       iommu_cleanup(self->iommu);
 }
 
 TEST_F(vfio_pci_driver_test, init_remove)
@@ -231,18 +234,31 @@ TEST_F_TIMEOUT(vfio_pci_driver_test, memcpy_storm, 60)
        ASSERT_NO_MSI(self->msi_fd);
 }
 
-int main(int argc, char *argv[])
+static bool device_has_selftests_driver(const char *bdf)
 {
        struct vfio_pci_device *device;
+       struct iommu *iommu;
+       bool has_driver;
+
+       iommu = iommu_init(default_iommu_mode);
+       device = vfio_pci_device_init(device_bdf, iommu);
+
+       has_driver = !!device->driver.ops;
+
+       vfio_pci_device_cleanup(device);
+       iommu_cleanup(iommu);
 
+       return has_driver;
+}
+
+int main(int argc, char *argv[])
+{
        device_bdf = vfio_selftests_get_bdf(&argc, argv);
 
-       device = vfio_pci_device_init(device_bdf, default_iommu_mode);
-       if (!device->driver.ops) {
+       if (!device_has_selftests_driver(device_bdf)) {
                fprintf(stderr, "No driver found for device %s\n", device_bdf);
                return KSFT_SKIP;
        }
-       vfio_pci_device_cleanup(device);
 
        return test_harness_run(argc, argv);
 }
-- 
2.52.0.rc1.455.g30608eb744-goog


Reply via email to