* [PATCH 0/2] vfio: selftests: Add iommufd multi iommu test
@ 2026-05-05 22:14 Samiullah Khawaja
2026-05-05 22:14 ` [PATCH 1/2] vfio: selftests: Add support of creating multiple iommus from iommufd Samiullah Khawaja
2026-05-05 22:14 ` [PATCH 2/2] vfio: selftests: Add iommufd multi iommu test Samiullah Khawaja
0 siblings, 2 replies; 11+ messages in thread
From: Samiullah Khawaja @ 2026-05-05 22:14 UTC (permalink / raw)
To: David Matlack, Aex Williamson, Shuah Khan; +Cc: Samiullah Khawaja, linux-kernel
IOMMUFD allows creating IOAS and the underlying HWPTs using already open
iommufd. Currently the VFIO selftest framework only allows user to
create one iommu against an iommufd.
This series adds new functionality to allow,
- Creating an IOMMU against an already open iommufd.
- The new IOMMU basically uses a new iommufd IOAS.
- The user has option to create a new HWPT when creating the new IOMMU
instead of using automatically created HWPT internally.
- Attaching the new iommufd IOMMU with a device that was setup using a
different IOMMU.
This series also adds a new test that creates multiple iommus and
switches between them and does DMA to verify that everything works
properly.
This was tested in Qemu with a virtio-pci device and also on an Intel
machine with Intel DSA device.
TAP version 13
1..9
# Starting 9 tests from 9 test cases.
# RUN vfio_iommufd_multi_iommu_test.DEFAULT_to_DEFAULT.memcpy ...
# OK vfio_iommufd_multi_iommu_test.DEFAULT_to_DEFAULT.memcpy
ok 1 vfio_iommufd_multi_iommu_test.DEFAULT_to_DEFAULT.memcpy
# RUN vfio_iommufd_multi_iommu_test.DEFAULT_to_WITH_PT.memcpy ...
# OK vfio_iommufd_multi_iommu_test.DEFAULT_to_WITH_PT.memcpy
ok 2 vfio_iommufd_multi_iommu_test.DEFAULT_to_WITH_PT.memcpy
# RUN vfio_iommufd_multi_iommu_test.DEFAULT_to_WITHOUT_PT.memcpy ...
# OK vfio_iommufd_multi_iommu_test.DEFAULT_to_WITHOUT_PT.memcpy
ok 3 vfio_iommufd_multi_iommu_test.DEFAULT_to_WITHOUT_PT.memcpy
# RUN vfio_iommufd_multi_iommu_test.WITH_PT_to_DEFAULT.memcpy ...
# OK vfio_iommufd_multi_iommu_test.WITH_PT_to_DEFAULT.memcpy
ok 4 vfio_iommufd_multi_iommu_test.WITH_PT_to_DEFAULT.memcpy
# RUN vfio_iommufd_multi_iommu_test.WITH_PT_to_WITH_PT.memcpy ...
# OK vfio_iommufd_multi_iommu_test.WITH_PT_to_WITH_PT.memcpy
ok 5 vfio_iommufd_multi_iommu_test.WITH_PT_to_WITH_PT.memcpy
# RUN vfio_iommufd_multi_iommu_test.WITH_PT_to_WITHOUT_PT.memcpy ...
# OK vfio_iommufd_multi_iommu_test.WITH_PT_to_WITHOUT_PT.memcpy
ok 6 vfio_iommufd_multi_iommu_test.WITH_PT_to_WITHOUT_PT.memcpy
# RUN vfio_iommufd_multi_iommu_test.WITHOUT_PT_to_DEFAULT.memcpy ...
# OK vfio_iommufd_multi_iommu_test.WITHOUT_PT_to_DEFAULT.memcpy
ok 7 vfio_iommufd_multi_iommu_test.WITHOUT_PT_to_DEFAULT.memcpy
# RUN vfio_iommufd_multi_iommu_test.WITHOUT_PT_to_WITH_PT.memcpy ...
# OK vfio_iommufd_multi_iommu_test.WITHOUT_PT_to_WITH_PT.memcpy
ok 8 vfio_iommufd_multi_iommu_test.WITHOUT_PT_to_WITH_PT.memcpy
# RUN vfio_iommufd_multi_iommu_test.WITHOUT_PT_to_WITHOUT_PT.memcpy ...
# OK vfio_iommufd_multi_iommu_test.WITHOUT_PT_to_WITHOUT_PT.memcpy
ok 9 vfio_iommufd_multi_iommu_test.WITHOUT_PT_to_WITHOUT_PT.memcpy
# PASSED: 9 / 9 tests passed.
# Totals: pass:9 fail:0 xfail:0 xpass:0 skip:0 error:0
The vfio framework patch was earlier sent with a different series, but I
am sending it separately in this series as this can go in independently:
https://lore.kernel.org/all/20260107201800.2486137-3-skhawaja@google.com/
Samiullah Khawaja (2):
vfio: selftests: Add support of creating multiple iommus from iommufd
vfio: selftests: Add iommufd multi iommu test
tools/testing/selftests/vfio/Makefile | 1 +
.../vfio/lib/include/libvfio/iommu.h | 5 +
.../lib/include/libvfio/vfio_pci_device.h | 2 +
tools/testing/selftests/vfio/lib/iommu.c | 62 +++++-
.../selftests/vfio/lib/vfio_pci_device.c | 22 +-
.../vfio/vfio_iommufd_multi_iommu_test.c | 203 ++++++++++++++++++
6 files changed, 288 insertions(+), 7 deletions(-)
create mode 100644 tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c
base-commit: 9974969c14031a097d6b45bcb7a06bb4aa525c40
--
2.54.0.545.g6539524ca2-goog
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] vfio: selftests: Add support of creating multiple iommus from iommufd
2026-05-05 22:14 [PATCH 0/2] vfio: selftests: Add iommufd multi iommu test Samiullah Khawaja
@ 2026-05-05 22:14 ` Samiullah Khawaja
2026-05-08 18:17 ` David Matlack
2026-05-05 22:14 ` [PATCH 2/2] vfio: selftests: Add iommufd multi iommu test Samiullah Khawaja
1 sibling, 1 reply; 11+ messages in thread
From: Samiullah Khawaja @ 2026-05-05 22:14 UTC (permalink / raw)
To: David Matlack, Aex Williamson, Shuah Khan
Cc: Samiullah Khawaja, Alex Mastro, Raghavendra Rao Ananta, kvm,
linux-kselftest, linux-kernel
IOMMUFD allows creating multiple IOAS and HWPTs under one iommufd, Add
API to init a struct iommu using an already opened iommufd. The API
internally creates a new IOAS and also a new HWPT as an option based on
the flags passed to the function.
Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
---
.../vfio/lib/include/libvfio/iommu.h | 5 ++
.../lib/include/libvfio/vfio_pci_device.h | 2 +
tools/testing/selftests/vfio/lib/iommu.c | 62 +++++++++++++++++--
.../selftests/vfio/lib/vfio_pci_device.c | 22 ++++++-
4 files changed, 84 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h b/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h
index e9a3386a4719..89249a294920 100644
--- a/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h
+++ b/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h
@@ -9,6 +9,9 @@
typedef u64 iova_t;
+/* Create IOMMU with page tables */
+#define IOMMUFD_IOMMU_INIT_CREATE_PT 1
+
struct iommu_mode {
const char *name;
const char *container_path;
@@ -29,10 +32,12 @@ struct iommu {
int container_fd;
int iommufd;
u32 ioas_id;
+ u32 hwpt_id;
struct list_head dma_regions;
};
struct iommu *iommu_init(const char *iommu_mode);
+struct iommu *iommufd_iommu_init(int iommufd, u32 dev_id, u32 flags);
void iommu_cleanup(struct iommu *iommu);
int __iommu_map(struct iommu *iommu, struct dma_region *region);
diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
index 2858885a89bb..1143ceb6a9b8 100644
--- a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
+++ b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
@@ -19,6 +19,7 @@ struct vfio_pci_device {
const char *bdf;
int fd;
int group_fd;
+ u32 dev_id;
struct iommu *iommu;
@@ -65,6 +66,7 @@ void vfio_pci_config_access(struct vfio_pci_device *device, bool write,
#define vfio_pci_config_writew(_d, _o, _v) vfio_pci_config_write(_d, _o, _v, u16)
#define vfio_pci_config_writel(_d, _o, _v) vfio_pci_config_write(_d, _o, _v, u32)
+void vfio_pci_device_attach_iommu(struct vfio_pci_device *device, struct iommu *iommu);
void vfio_pci_irq_enable(struct vfio_pci_device *device, u32 index,
u32 vector, int count);
void vfio_pci_irq_disable(struct vfio_pci_device *device, u32 index);
diff --git a/tools/testing/selftests/vfio/lib/iommu.c b/tools/testing/selftests/vfio/lib/iommu.c
index 035dac069d60..644c049cf9f0 100644
--- a/tools/testing/selftests/vfio/lib/iommu.c
+++ b/tools/testing/selftests/vfio/lib/iommu.c
@@ -408,6 +408,18 @@ struct iommu_iova_range *iommu_iova_ranges(struct iommu *iommu, u32 *nranges)
return ranges;
}
+static u32 iommufd_hwpt_alloc(struct iommu *iommu, u32 dev_id)
+{
+ struct iommu_hwpt_alloc args = {
+ .size = sizeof(args),
+ .pt_id = iommu->ioas_id,
+ .dev_id = dev_id,
+ };
+
+ ioctl_assert(iommu->iommufd, IOMMU_HWPT_ALLOC, &args);
+ return args.out_hwpt_id;
+}
+
static u32 iommufd_ioas_alloc(int iommufd)
{
struct iommu_ioas_alloc args = {
@@ -418,11 +430,9 @@ static u32 iommufd_ioas_alloc(int iommufd)
return args.out_ioas_id;
}
-struct iommu *iommu_init(const char *iommu_mode)
+static struct iommu *iommu_alloc(const char *iommu_mode)
{
- const char *container_path;
struct iommu *iommu;
- int version;
iommu = calloc(1, sizeof(*iommu));
VFIO_ASSERT_NOT_NULL(iommu);
@@ -430,6 +440,16 @@ struct iommu *iommu_init(const char *iommu_mode)
INIT_LIST_HEAD(&iommu->dma_regions);
iommu->mode = lookup_iommu_mode(iommu_mode);
+ return iommu;
+}
+
+struct iommu *iommu_init(const char *iommu_mode)
+{
+ const char *container_path;
+ struct iommu *iommu;
+ int version;
+
+ iommu = iommu_alloc(iommu_mode);
container_path = iommu->mode->container_path;
if (container_path) {
@@ -453,10 +473,44 @@ struct iommu *iommu_init(const char *iommu_mode)
return iommu;
}
+struct iommu *iommufd_iommu_init(int iommufd, u32 dev_id, u32 flags)
+{
+ struct iommu *iommu;
+
+ iommu = iommu_alloc("iommufd");
+
+ iommu->iommufd = dup(iommufd);
+ VFIO_ASSERT_GT(iommu->iommufd, 0);
+
+ iommu->ioas_id = iommufd_ioas_alloc(iommu->iommufd);
+
+ if (flags & IOMMUFD_IOMMU_INIT_CREATE_PT)
+ iommu->hwpt_id = iommufd_hwpt_alloc(iommu, dev_id);
+
+ return iommu;
+}
+
+static void iommufd_cleanup(struct iommu *iommu)
+{
+ struct iommu_destroy args = {
+ .size = sizeof(args),
+ };
+
+ if (iommu->hwpt_id) {
+ args.id = iommu->hwpt_id;
+ ioctl_assert(iommu->iommufd, IOMMU_DESTROY, &args);
+ }
+
+ args.id = iommu->ioas_id;
+ ioctl_assert(iommu->iommufd, IOMMU_DESTROY, &args);
+
+ VFIO_ASSERT_EQ(close(iommu->iommufd), 0);
+}
+
void iommu_cleanup(struct iommu *iommu)
{
if (iommu->iommufd)
- VFIO_ASSERT_EQ(close(iommu->iommufd), 0);
+ iommufd_cleanup(iommu);
else
VFIO_ASSERT_EQ(close(iommu->container_fd), 0);
diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
index fc75e04ef010..e2b71fe63cae 100644
--- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
+++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
@@ -322,7 +322,7 @@ const char *vfio_pci_get_cdev_path(const char *bdf)
return cdev_path;
}
-static void vfio_device_bind_iommufd(int device_fd, int iommufd)
+static int vfio_device_bind_iommufd(int device_fd, int iommufd)
{
struct vfio_device_bind_iommufd args = {
.argsz = sizeof(args),
@@ -330,6 +330,7 @@ static void vfio_device_bind_iommufd(int device_fd, int iommufd)
};
ioctl_assert(device_fd, VFIO_DEVICE_BIND_IOMMUFD, &args);
+ return args.out_devid;
}
static void vfio_device_attach_iommufd_pt(int device_fd, u32 pt_id)
@@ -342,6 +343,21 @@ static void vfio_device_attach_iommufd_pt(int device_fd, u32 pt_id)
ioctl_assert(device_fd, VFIO_DEVICE_ATTACH_IOMMUFD_PT, &args);
}
+void vfio_pci_device_attach_iommu(struct vfio_pci_device *device, struct iommu *iommu)
+{
+ u32 pt_id = iommu->ioas_id;
+
+ /* Only iommufd supports changing struct iommu attachments */
+ VFIO_ASSERT_TRUE(iommu->iommufd);
+
+ if (iommu->hwpt_id)
+ pt_id = iommu->hwpt_id;
+
+ VFIO_ASSERT_NE(pt_id, 0);
+ vfio_device_attach_iommufd_pt(device->fd, pt_id);
+ device->iommu = iommu;
+}
+
static void vfio_pci_iommufd_setup(struct vfio_pci_device *device, const char *bdf)
{
const char *cdev_path = vfio_pci_get_cdev_path(bdf);
@@ -350,8 +366,8 @@ static void vfio_pci_iommufd_setup(struct vfio_pci_device *device, const char *b
VFIO_ASSERT_GE(device->fd, 0);
free((void *)cdev_path);
- vfio_device_bind_iommufd(device->fd, device->iommu->iommufd);
- vfio_device_attach_iommufd_pt(device->fd, device->iommu->ioas_id);
+ device->dev_id = vfio_device_bind_iommufd(device->fd, device->iommu->iommufd);
+ vfio_pci_device_attach_iommu(device, device->iommu);
}
struct vfio_pci_device *vfio_pci_device_init(const char *bdf, struct iommu *iommu)
--
2.54.0.545.g6539524ca2-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] vfio: selftests: Add iommufd multi iommu test
2026-05-05 22:14 [PATCH 0/2] vfio: selftests: Add iommufd multi iommu test Samiullah Khawaja
2026-05-05 22:14 ` [PATCH 1/2] vfio: selftests: Add support of creating multiple iommus from iommufd Samiullah Khawaja
@ 2026-05-05 22:14 ` Samiullah Khawaja
2026-05-08 20:14 ` David Matlack
1 sibling, 1 reply; 11+ messages in thread
From: Samiullah Khawaja @ 2026-05-05 22:14 UTC (permalink / raw)
To: David Matlack, Aex Williamson, Shuah Khan
Cc: Samiullah Khawaja, linux-kernel, kvm, linux-kselftest
Add a test for iommufd to verify creation of multiple iommus using an
already setup iommufd and switch between them.
Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
---
tools/testing/selftests/vfio/Makefile | 1 +
.../vfio/vfio_iommufd_multi_iommu_test.c | 203 ++++++++++++++++++
2 files changed, 204 insertions(+)
create mode 100644 tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c
diff --git a/tools/testing/selftests/vfio/Makefile b/tools/testing/selftests/vfio/Makefile
index 0684932d91bf..24bdeaad590f 100644
--- a/tools/testing/selftests/vfio/Makefile
+++ b/tools/testing/selftests/vfio/Makefile
@@ -8,6 +8,7 @@ else
CFLAGS = $(KHDR_INCLUDES)
TEST_GEN_PROGS += vfio_dma_mapping_test
TEST_GEN_PROGS += vfio_dma_mapping_mmio_test
+TEST_GEN_PROGS += vfio_iommufd_multi_iommu_test
TEST_GEN_PROGS += vfio_iommufd_setup_test
TEST_GEN_PROGS += vfio_pci_device_test
TEST_GEN_PROGS += vfio_pci_device_init_perf_test
diff --git a/tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c b/tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c
new file mode 100644
index 000000000000..7199c662637c
--- /dev/null
+++ b/tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c
@@ -0,0 +1,203 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+
+#include <linux/sizes.h>
+#include <linux/vfio.h>
+
+#include <libvfio.h>
+
+#include "kselftest_harness.h"
+
+static const char *device_bdf;
+
+#define IOMMU_DEFAULT 0
+#define IOMMU_WITH_PT 1
+#define IOMMU_WITHOUT_PT 2
+
+static void region_setup(struct iommu *iommu,
+ struct iova_allocator *iova_allocator,
+ struct dma_region *region, u64 size)
+{
+ const int flags = MAP_SHARED | MAP_ANONYMOUS;
+ const int prot = PROT_READ | PROT_WRITE;
+ void *vaddr;
+
+ vaddr = mmap(NULL, size, prot, flags, -1, 0);
+ VFIO_ASSERT_NE(vaddr, MAP_FAILED);
+
+ region->vaddr = vaddr;
+ region->iova = iova_allocator_alloc(iova_allocator, size);
+ region->size = size;
+
+ iommu_map(iommu, region);
+}
+
+static void region_teardown(struct iommu *iommu, struct dma_region *region)
+{
+ iommu_unmap(iommu, region);
+ VFIO_ASSERT_EQ(munmap(region->vaddr, region->size), 0);
+}
+
+FIXTURE(vfio_iommufd_multi_iommu_test) {
+ struct iommu *iommu;
+ struct vfio_pci_device *device;
+ struct iova_allocator *iova_allocator;
+ struct dma_region memcpy_region;
+ void *vaddr;
+
+ u64 size;
+ void *src;
+ void *dst;
+ iova_t src_iova;
+ iova_t dst_iova;
+};
+
+FIXTURE_SETUP(vfio_iommufd_multi_iommu_test)
+{
+ struct vfio_pci_driver *driver;
+
+ self->iommu = iommu_init("iommufd");
+ self->device = vfio_pci_device_init(device_bdf, self->iommu);
+ self->iova_allocator = iova_allocator_init(self->iommu);
+
+ driver = &self->device->driver;
+
+ region_setup(self->iommu, self->iova_allocator, &self->memcpy_region, SZ_1G);
+ region_setup(self->iommu, self->iova_allocator, &driver->region, SZ_2M);
+
+ if (driver->ops)
+ vfio_pci_driver_init(self->device);
+
+ self->size = self->memcpy_region.size / 2;
+ self->src = self->memcpy_region.vaddr;
+ self->dst = self->src + self->size;
+
+ self->src_iova = to_iova(self->device, self->src);
+ self->dst_iova = to_iova(self->device, self->dst);
+}
+
+FIXTURE_TEARDOWN(vfio_iommufd_multi_iommu_test)
+{
+ struct vfio_pci_driver *driver = &self->device->driver;
+
+ if (driver->ops)
+ vfio_pci_driver_remove(self->device);
+
+ region_teardown(self->iommu, &self->memcpy_region);
+ region_teardown(self->iommu, &driver->region);
+
+ iova_allocator_cleanup(self->iova_allocator);
+ vfio_pci_device_cleanup(self->device);
+ iommu_cleanup(self->iommu);
+}
+
+FIXTURE_VARIANT(vfio_iommufd_multi_iommu_test) {
+ u32 start_iommu;
+ u32 end_iommu;
+};
+
+#define ADD_IOMMU_VARIANT(S, E) \
+ FIXTURE_VARIANT_ADD(vfio_iommufd_multi_iommu_test, S##_to_##E) { \
+ .start_iommu = IOMMU_##S, \
+ .end_iommu = IOMMU_##E \
+ };
+
+#define ADD_ALL_VARIANTS(S) \
+ ADD_IOMMU_VARIANT(S, DEFAULT) \
+ ADD_IOMMU_VARIANT(S, WITH_PT) \
+ ADD_IOMMU_VARIANT(S, WITHOUT_PT)
+
+ADD_ALL_VARIANTS(DEFAULT)
+ADD_ALL_VARIANTS(WITH_PT)
+ADD_ALL_VARIANTS(WITHOUT_PT)
+
+static struct iommu *setup_variant_iommu(struct iommu *fixture_iommu,
+ u32 dev_id, u32 type)
+{
+ switch (type) {
+ case IOMMU_WITH_PT:
+ return iommufd_iommu_init(fixture_iommu->iommufd, dev_id,
+ IOMMUFD_IOMMU_INIT_CREATE_PT);
+ case IOMMU_WITHOUT_PT:
+ return iommufd_iommu_init(fixture_iommu->iommufd, dev_id, 0);
+ default:
+ return fixture_iommu;
+ }
+}
+
+static void test_memcpy(struct vfio_pci_device *device, void *src, void *dst,
+ iova_t src_iova, iova_t dst_iova, u64 size, char val)
+{
+ if (!device->driver.ops)
+ return;
+
+ memset(src, val, size);
+ memset(dst, 0, size);
+
+ vfio_pci_driver_memcpy_start(device, src_iova, dst_iova, size, 100);
+ VFIO_ASSERT_EQ(vfio_pci_driver_memcpy_wait(device), 0);
+ VFIO_ASSERT_EQ(memcmp(src, dst, size), 0);
+}
+
+TEST_F(vfio_iommufd_multi_iommu_test, memcpy)
+{
+ struct dma_region memcpy_region1, driver_region1;
+ struct dma_region memcpy_region2, driver_region2;
+ struct iommu *iommu1;
+ struct iommu *iommu2;
+
+ iommu1 = setup_variant_iommu(self->iommu, self->device->dev_id,
+ variant->start_iommu);
+ if (iommu1 != self->iommu) {
+ memcpy_region1 = self->memcpy_region;
+ driver_region1 = self->device->driver.region;
+
+ iommu_map(iommu1, &memcpy_region1);
+ iommu_map(iommu1, &driver_region1);
+
+ vfio_pci_device_attach_iommu(self->device, iommu1);
+ }
+
+ test_memcpy(self->device, self->src, self->dst, self->src_iova,
+ self->dst_iova, self->size, 'x');
+
+ iommu2 = setup_variant_iommu(self->iommu, self->device->dev_id,
+ variant->end_iommu);
+ if (iommu2 != self->iommu) {
+ memcpy_region2 = self->memcpy_region;
+ driver_region2 = self->device->driver.region;
+
+ iommu_map(iommu2, &memcpy_region2);
+ iommu_map(iommu2, &driver_region2);
+ }
+
+ vfio_pci_device_attach_iommu(self->device, iommu2);
+ test_memcpy(self->device, self->src, self->dst, self->src_iova,
+ self->dst_iova, self->size, 'a');
+
+ vfio_pci_device_attach_iommu(self->device, iommu1);
+ test_memcpy(self->device, self->src, self->dst, self->src_iova,
+ self->dst_iova, self->size, '1');
+
+ if (iommu1 != self->iommu) {
+ /* attach back to default iommu for cleanup */
+ vfio_pci_device_attach_iommu(self->device, self->iommu);
+ iommu_unmap(iommu1, &memcpy_region1);
+ iommu_unmap(iommu1, &driver_region1);
+ iommu_cleanup(iommu1);
+ }
+
+ if (iommu2 != self->iommu) {
+ iommu_unmap(iommu2, &memcpy_region2);
+ iommu_unmap(iommu2, &driver_region2);
+ iommu_cleanup(iommu2);
+ }
+}
+
+int main(int argc, char *argv[])
+{
+ device_bdf = vfio_selftests_get_bdf(&argc, argv);
+
+ return test_harness_run(argc, argv);
+}
--
2.54.0.545.g6539524ca2-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] vfio: selftests: Add support of creating multiple iommus from iommufd
2026-05-05 22:14 ` [PATCH 1/2] vfio: selftests: Add support of creating multiple iommus from iommufd Samiullah Khawaja
@ 2026-05-08 18:17 ` David Matlack
2026-05-11 20:21 ` Samiullah Khawaja
0 siblings, 1 reply; 11+ messages in thread
From: David Matlack @ 2026-05-08 18:17 UTC (permalink / raw)
To: Samiullah Khawaja
Cc: Aex Williamson, Shuah Khan, Alex Mastro, Raghavendra Rao Ananta,
kvm, linux-kselftest, linux-kernel
On 2026-05-05 10:14 PM, Samiullah Khawaja wrote:
> IOMMUFD allows creating multiple IOAS and HWPTs under one iommufd, Add
> API to init a struct iommu using an already opened iommufd. The API
> internally creates a new IOAS and also a new HWPT as an option based on
> the flags passed to the function.
>
> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
> ---
> .../vfio/lib/include/libvfio/iommu.h | 5 ++
> .../lib/include/libvfio/vfio_pci_device.h | 2 +
> tools/testing/selftests/vfio/lib/iommu.c | 62 +++++++++++++++++--
> .../selftests/vfio/lib/vfio_pci_device.c | 22 ++++++-
> 4 files changed, 84 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h b/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h
> index e9a3386a4719..89249a294920 100644
> --- a/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h
> +++ b/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h
> @@ -9,6 +9,9 @@
>
> typedef u64 iova_t;
>
> +/* Create IOMMU with page tables */
> +#define IOMMUFD_IOMMU_INIT_CREATE_PT 1
> +
> struct iommu_mode {
> const char *name;
> const char *container_path;
> @@ -29,10 +32,12 @@ struct iommu {
> int container_fd;
> int iommufd;
> u32 ioas_id;
> + u32 hwpt_id;
> struct list_head dma_regions;
> };
>
> struct iommu *iommu_init(const char *iommu_mode);
> +struct iommu *iommufd_iommu_init(int iommufd, u32 dev_id, u32 flags);
> void iommu_cleanup(struct iommu *iommu);
>
> int __iommu_map(struct iommu *iommu, struct dma_region *region);
> diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
> index 2858885a89bb..1143ceb6a9b8 100644
> --- a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
> +++ b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
> @@ -19,6 +19,7 @@ struct vfio_pci_device {
> const char *bdf;
> int fd;
> int group_fd;
> + u32 dev_id;
Please introduce an initialize this field in its own patch and, in the
commit message, explain why it is being added and how it will be used.
>
> struct iommu *iommu;
>
> @@ -65,6 +66,7 @@ void vfio_pci_config_access(struct vfio_pci_device *device, bool write,
> #define vfio_pci_config_writew(_d, _o, _v) vfio_pci_config_write(_d, _o, _v, u16)
> #define vfio_pci_config_writel(_d, _o, _v) vfio_pci_config_write(_d, _o, _v, u32)
>
> +void vfio_pci_device_attach_iommu(struct vfio_pci_device *device, struct iommu *iommu);
> void vfio_pci_irq_enable(struct vfio_pci_device *device, u32 index,
> u32 vector, int count);
> void vfio_pci_irq_disable(struct vfio_pci_device *device, u32 index);
> diff --git a/tools/testing/selftests/vfio/lib/iommu.c b/tools/testing/selftests/vfio/lib/iommu.c
> index 035dac069d60..644c049cf9f0 100644
> --- a/tools/testing/selftests/vfio/lib/iommu.c
> +++ b/tools/testing/selftests/vfio/lib/iommu.c
> @@ -408,6 +408,18 @@ struct iommu_iova_range *iommu_iova_ranges(struct iommu *iommu, u32 *nranges)
> return ranges;
> }
>
> +static u32 iommufd_hwpt_alloc(struct iommu *iommu, u32 dev_id)
> +{
> + struct iommu_hwpt_alloc args = {
> + .size = sizeof(args),
> + .pt_id = iommu->ioas_id,
> + .dev_id = dev_id,
> + };
> +
> + ioctl_assert(iommu->iommufd, IOMMU_HWPT_ALLOC, &args);
> + return args.out_hwpt_id;
> +}
> +
> static u32 iommufd_ioas_alloc(int iommufd)
> {
> struct iommu_ioas_alloc args = {
> @@ -418,11 +430,9 @@ static u32 iommufd_ioas_alloc(int iommufd)
> return args.out_ioas_id;
> }
>
> -struct iommu *iommu_init(const char *iommu_mode)
> +static struct iommu *iommu_alloc(const char *iommu_mode)
> {
> - const char *container_path;
> struct iommu *iommu;
> - int version;
>
> iommu = calloc(1, sizeof(*iommu));
> VFIO_ASSERT_NOT_NULL(iommu);
> @@ -430,6 +440,16 @@ struct iommu *iommu_init(const char *iommu_mode)
> INIT_LIST_HEAD(&iommu->dma_regions);
>
> iommu->mode = lookup_iommu_mode(iommu_mode);
> + return iommu;
> +}
> +
> +struct iommu *iommu_init(const char *iommu_mode)
> +{
> + const char *container_path;
> + struct iommu *iommu;
> + int version;
> +
> + iommu = iommu_alloc(iommu_mode);
>
> container_path = iommu->mode->container_path;
> if (container_path) {
> @@ -453,10 +473,44 @@ struct iommu *iommu_init(const char *iommu_mode)
> return iommu;
> }
>
> +struct iommu *iommufd_iommu_init(int iommufd, u32 dev_id, u32 flags)
> +{
> + struct iommu *iommu;
> +
> + iommu = iommu_alloc("iommufd");
Use MODE_IOMMUFD instead of string literal.
> +
> + iommu->iommufd = dup(iommufd);
> + VFIO_ASSERT_GT(iommu->iommufd, 0);
> +
> + iommu->ioas_id = iommufd_ioas_alloc(iommu->iommufd);
Please avoid duplicating this code. e.g. Something like this:
diff --git a/tools/testing/selftests/vfio/lib/iommu.c b/tools/testing/selftests/vfio/lib/iommu.c
index 644c049cf9f0..4909cda5c840 100644
--- a/tools/testing/selftests/vfio/lib/iommu.c
+++ b/tools/testing/selftests/vfio/lib/iommu.c
@@ -443,6 +443,19 @@ static struct iommu *iommu_alloc(const char *iommu_mode)
return iommu;
}
+static void iommufd_init(struct iommu *iommu, int iommufd)
+{
+ /*
+ * 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.
+ */
+ VFIO_ASSERT_GT(iommufd, 0);
+
+ iommu->iommufd = iommufd;
+ iommu->ioas_id = iommufd_ioas_alloc(iommu->iommufd);
+}
+
struct iommu *iommu_init(const char *iommu_mode)
{
const char *container_path;
@@ -459,15 +472,7 @@ struct iommu *iommu_init(const char *iommu_mode)
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);
+ iommufd_init(iommu, open("/dev/iommu", O_RDWR));
}
return iommu;
@@ -478,11 +483,7 @@ struct iommu *iommufd_iommu_init(int iommufd, u32 dev_id, u32 flags)
struct iommu *iommu;
iommu = iommu_alloc("iommufd");
-
- iommu->iommufd = dup(iommufd);
- VFIO_ASSERT_GT(iommu->iommufd, 0);
-
- iommu->ioas_id = iommufd_ioas_alloc(iommu->iommufd);
+ iommufd_init(iommu, dup(iommufd));
if (flags & IOMMUFD_IOMMU_INIT_CREATE_PT)
iommu->hwpt_id = iommufd_hwpt_alloc(iommu, dev_id);
> +
> + if (flags & IOMMUFD_IOMMU_INIT_CREATE_PT)
> + iommu->hwpt_id = iommufd_hwpt_alloc(iommu, dev_id);
Does this need to be part of iommufd_iommu_init()? Maybe it would be
better to expose a separate helper to allocate a HWPT for a given struct
iommu *.
This would enable use of HWPTs in iommus constructed by iommu_init() as
well, which someone may want to do in the future.
If you go this route, please introduce this in its own commit.
> +
> + return iommu;
> +}
> +
> +static void iommufd_cleanup(struct iommu *iommu)
> +{
> + struct iommu_destroy args = {
> + .size = sizeof(args),
> + };
> +
> + if (iommu->hwpt_id) {
> + args.id = iommu->hwpt_id;
> + ioctl_assert(iommu->iommufd, IOMMU_DESTROY, &args);
> + }
> +
> + args.id = iommu->ioas_id;
> + ioctl_assert(iommu->iommufd, IOMMU_DESTROY, &args);
Please create a helper function to do IOMMU_DESTROY ioctl. Then here you
can just do:
if (iommu->hwpt_id)
iommufd_iommu_destroy(iommu, iommu->hwpt_id);
iommfd_iommu_destroy(iommu, iommu->ioas_id);
> +
> + VFIO_ASSERT_EQ(close(iommu->iommufd), 0);
> +}
> +
> void iommu_cleanup(struct iommu *iommu)
> {
> if (iommu->iommufd)
> - VFIO_ASSERT_EQ(close(iommu->iommufd), 0);
> + iommufd_cleanup(iommu);
> else
> VFIO_ASSERT_EQ(close(iommu->container_fd), 0);
>
> diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> index fc75e04ef010..e2b71fe63cae 100644
> --- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> +++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> @@ -322,7 +322,7 @@ const char *vfio_pci_get_cdev_path(const char *bdf)
> return cdev_path;
> }
>
> -static void vfio_device_bind_iommufd(int device_fd, int iommufd)
> +static int vfio_device_bind_iommufd(int device_fd, int iommufd)
> {
> struct vfio_device_bind_iommufd args = {
> .argsz = sizeof(args),
> @@ -330,6 +330,7 @@ static void vfio_device_bind_iommufd(int device_fd, int iommufd)
> };
>
> ioctl_assert(device_fd, VFIO_DEVICE_BIND_IOMMUFD, &args);
> + return args.out_devid;
> }
>
> static void vfio_device_attach_iommufd_pt(int device_fd, u32 pt_id)
> @@ -342,6 +343,21 @@ static void vfio_device_attach_iommufd_pt(int device_fd, u32 pt_id)
> ioctl_assert(device_fd, VFIO_DEVICE_ATTACH_IOMMUFD_PT, &args);
> }
>
> +void vfio_pci_device_attach_iommu(struct vfio_pci_device *device, struct iommu *iommu)
> +{
> + u32 pt_id = iommu->ioas_id;
> +
> + /* Only iommufd supports changing struct iommu attachments */
> + VFIO_ASSERT_TRUE(iommu->iommufd);
> +
> + if (iommu->hwpt_id)
> + pt_id = iommu->hwpt_id;
nit: This can be folded into the variable declaration.
const u32 pt_id = iommu->hwpt_id ?: iommu->ioas_id;
> +
> + VFIO_ASSERT_NE(pt_id, 0);
Why is this check needed?
> + vfio_device_attach_iommufd_pt(device->fd, pt_id);
> + device->iommu = iommu;
> +}
Please introduce vfio_pci_device_attach_iommu() in its own patch.
> +
> static void vfio_pci_iommufd_setup(struct vfio_pci_device *device, const char *bdf)
> {
> const char *cdev_path = vfio_pci_get_cdev_path(bdf);
> @@ -350,8 +366,8 @@ static void vfio_pci_iommufd_setup(struct vfio_pci_device *device, const char *b
> VFIO_ASSERT_GE(device->fd, 0);
> free((void *)cdev_path);
>
> - vfio_device_bind_iommufd(device->fd, device->iommu->iommufd);
> - vfio_device_attach_iommufd_pt(device->fd, device->iommu->ioas_id);
> + device->dev_id = vfio_device_bind_iommufd(device->fd, device->iommu->iommufd);
> + vfio_pci_device_attach_iommu(device, device->iommu);
> }
>
> struct vfio_pci_device *vfio_pci_device_init(const char *bdf, struct iommu *iommu)
> --
> 2.54.0.545.g6539524ca2-goog
>
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] vfio: selftests: Add iommufd multi iommu test
2026-05-05 22:14 ` [PATCH 2/2] vfio: selftests: Add iommufd multi iommu test Samiullah Khawaja
@ 2026-05-08 20:14 ` David Matlack
2026-05-11 20:53 ` Samiullah Khawaja
0 siblings, 1 reply; 11+ messages in thread
From: David Matlack @ 2026-05-08 20:14 UTC (permalink / raw)
To: Samiullah Khawaja
Cc: Aex Williamson, Shuah Khan, linux-kernel, kvm, linux-kselftest
On 2026-05-05 10:14 PM, Samiullah Khawaja wrote:
> Add a test for iommufd to verify creation of multiple iommus using an
> already setup iommufd and switch between them.
>
> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
> ---
> tools/testing/selftests/vfio/Makefile | 1 +
> .../vfio/vfio_iommufd_multi_iommu_test.c | 203 ++++++++++++++++++
> 2 files changed, 204 insertions(+)
> create mode 100644 tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c
>
> diff --git a/tools/testing/selftests/vfio/Makefile b/tools/testing/selftests/vfio/Makefile
> index 0684932d91bf..24bdeaad590f 100644
> --- a/tools/testing/selftests/vfio/Makefile
> +++ b/tools/testing/selftests/vfio/Makefile
> @@ -8,6 +8,7 @@ else
> CFLAGS = $(KHDR_INCLUDES)
> TEST_GEN_PROGS += vfio_dma_mapping_test
> TEST_GEN_PROGS += vfio_dma_mapping_mmio_test
> +TEST_GEN_PROGS += vfio_iommufd_multi_iommu_test
> TEST_GEN_PROGS += vfio_iommufd_setup_test
> TEST_GEN_PROGS += vfio_pci_device_test
> TEST_GEN_PROGS += vfio_pci_device_init_perf_test
> diff --git a/tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c b/tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c
> new file mode 100644
> index 000000000000..7199c662637c
> --- /dev/null
> +++ b/tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c
> @@ -0,0 +1,203 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <sys/ioctl.h>
> +#include <sys/mman.h>
> +
> +#include <linux/sizes.h>
> +#include <linux/vfio.h>
> +
> +#include <libvfio.h>
> +
> +#include "kselftest_harness.h"
> +
> +static const char *device_bdf;
> +
> +#define IOMMU_DEFAULT 0
> +#define IOMMU_WITH_PT 1
> +#define IOMMU_WITHOUT_PT 2
> +
> +static void region_setup(struct iommu *iommu,
> + struct iova_allocator *iova_allocator,
> + struct dma_region *region, u64 size)
> +{
> + const int flags = MAP_SHARED | MAP_ANONYMOUS;
> + const int prot = PROT_READ | PROT_WRITE;
> + void *vaddr;
> +
> + vaddr = mmap(NULL, size, prot, flags, -1, 0);
> + VFIO_ASSERT_NE(vaddr, MAP_FAILED);
> +
> + region->vaddr = vaddr;
> + region->iova = iova_allocator_alloc(iova_allocator, size);
> + region->size = size;
> +
> + iommu_map(iommu, region);
> +}
> +
> +static void region_teardown(struct iommu *iommu, struct dma_region *region)
> +{
> + iommu_unmap(iommu, region);
> + VFIO_ASSERT_EQ(munmap(region->vaddr, region->size), 0);
> +}
Can you create library helpers in the library to share with
vfio_pci_driver_test.c?
dma_region_setup()
dma_region_teardown()
> +
> +FIXTURE(vfio_iommufd_multi_iommu_test) {
> + struct iommu *iommu;
> + struct vfio_pci_device *device;
> + struct iova_allocator *iova_allocator;
> + struct dma_region memcpy_region;
> + void *vaddr;
> +
> + u64 size;
> + void *src;
> + void *dst;
> + iova_t src_iova;
> + iova_t dst_iova;
> +};
> +
> +FIXTURE_SETUP(vfio_iommufd_multi_iommu_test)
> +{
> + struct vfio_pci_driver *driver;
> +
> + self->iommu = iommu_init("iommufd");
MODE_IOMMUFD
> + self->device = vfio_pci_device_init(device_bdf, self->iommu);
> + self->iova_allocator = iova_allocator_init(self->iommu);
> +
> + driver = &self->device->driver;
> +
> + region_setup(self->iommu, self->iova_allocator, &self->memcpy_region, SZ_1G);
> + region_setup(self->iommu, self->iova_allocator, &driver->region, SZ_2M);
> +
> + if (driver->ops)
> + vfio_pci_driver_init(self->device);
> +
> + self->size = self->memcpy_region.size / 2;
> + self->src = self->memcpy_region.vaddr;
> + self->dst = self->src + self->size;
> +
> + self->src_iova = to_iova(self->device, self->src);
> + self->dst_iova = to_iova(self->device, self->dst);
> +}
> +
> +FIXTURE_TEARDOWN(vfio_iommufd_multi_iommu_test)
> +{
> + struct vfio_pci_driver *driver = &self->device->driver;
> +
> + if (driver->ops)
> + vfio_pci_driver_remove(self->device);
> +
> + region_teardown(self->iommu, &self->memcpy_region);
> + region_teardown(self->iommu, &driver->region);
> +
> + iova_allocator_cleanup(self->iova_allocator);
> + vfio_pci_device_cleanup(self->device);
> + iommu_cleanup(self->iommu);
> +}
> +
> +FIXTURE_VARIANT(vfio_iommufd_multi_iommu_test) {
> + u32 start_iommu;
> + u32 end_iommu;
> +};
> +
> +#define ADD_IOMMU_VARIANT(S, E) \
> + FIXTURE_VARIANT_ADD(vfio_iommufd_multi_iommu_test, S##_to_##E) { \
> + .start_iommu = IOMMU_##S, \
> + .end_iommu = IOMMU_##E \
> + };
> +
> +#define ADD_ALL_VARIANTS(S) \
> + ADD_IOMMU_VARIANT(S, DEFAULT) \
> + ADD_IOMMU_VARIANT(S, WITH_PT) \
> + ADD_IOMMU_VARIANT(S, WITHOUT_PT)
> +
> +ADD_ALL_VARIANTS(DEFAULT)
> +ADD_ALL_VARIANTS(WITH_PT)
> +ADD_ALL_VARIANTS(WITHOUT_PT)
> +
> +static struct iommu *setup_variant_iommu(struct iommu *fixture_iommu,
> + u32 dev_id, u32 type)
> +{
> + switch (type) {
> + case IOMMU_WITH_PT:
> + return iommufd_iommu_init(fixture_iommu->iommufd, dev_id,
> + IOMMUFD_IOMMU_INIT_CREATE_PT);
> + case IOMMU_WITHOUT_PT:
> + return iommufd_iommu_init(fixture_iommu->iommufd, dev_id, 0);
> + default:
> + return fixture_iommu;
> + }
> +}
> +
> +static void test_memcpy(struct vfio_pci_device *device, void *src, void *dst,
> + iova_t src_iova, iova_t dst_iova, u64 size, char val)
> +{
> + if (!device->driver.ops)
> + return;
> +
> + memset(src, val, size);
> + memset(dst, 0, size);
> +
> + vfio_pci_driver_memcpy_start(device, src_iova, dst_iova, size, 100);
Why do 100 copies?
> + VFIO_ASSERT_EQ(vfio_pci_driver_memcpy_wait(device), 0);
> + VFIO_ASSERT_EQ(memcmp(src, dst, size), 0);
> +}
> +
> +TEST_F(vfio_iommufd_multi_iommu_test, memcpy)
> +{
> + struct dma_region memcpy_region1, driver_region1;
> + struct dma_region memcpy_region2, driver_region2;
> + struct iommu *iommu1;
> + struct iommu *iommu2;
> +
> + iommu1 = setup_variant_iommu(self->iommu, self->device->dev_id,
> + variant->start_iommu);
> + if (iommu1 != self->iommu) {
> + memcpy_region1 = self->memcpy_region;
> + driver_region1 = self->device->driver.region;
> +
> + iommu_map(iommu1, &memcpy_region1);
Can you make a helper to copy dma_region into a new iommu? It should set
up the new struct dma_region based on the old one, re-initialize the
link field, and then call iommu_map().
> + iommu_map(iommu1, &driver_region1);
> +
> + vfio_pci_device_attach_iommu(self->device, iommu1);
> + }
> +
> + test_memcpy(self->device, self->src, self->dst, self->src_iova,
> + self->dst_iova, self->size, 'x');
nit: You can pass in self to avoid unpacking it in every call to
test_memcpy(). The type would be:
struct _test_data_vfio_iommufd_multi_iommu_test *self
> +
> + iommu2 = setup_variant_iommu(self->iommu, self->device->dev_id,
> + variant->end_iommu);
> + if (iommu2 != self->iommu) {
> + memcpy_region2 = self->memcpy_region;
> + driver_region2 = self->device->driver.region;
> +
> + iommu_map(iommu2, &memcpy_region2);
> + iommu_map(iommu2, &driver_region2);
> + }
> +
> + vfio_pci_device_attach_iommu(self->device, iommu2);
> + test_memcpy(self->device, self->src, self->dst, self->src_iova,
> + self->dst_iova, self->size, 'a');
test_memcpy() zeroes self->dst every time so do you need to use a
different character for each call?
> +
> + vfio_pci_device_attach_iommu(self->device, iommu1);
> + test_memcpy(self->device, self->src, self->dst, self->src_iova,
> + self->dst_iova, self->size, '1');
> +
> + if (iommu1 != self->iommu) {
> + /* attach back to default iommu for cleanup */
> + vfio_pci_device_attach_iommu(self->device, self->iommu);
> + iommu_unmap(iommu1, &memcpy_region1);
> + iommu_unmap(iommu1, &driver_region1);
> + iommu_cleanup(iommu1);
> + }
> +
> + if (iommu2 != self->iommu) {
> + iommu_unmap(iommu2, &memcpy_region2);
> + iommu_unmap(iommu2, &driver_region2);
nit: You don't need to unmap before cleanup. Is this just to exercise
that unmap works on the variant iommus?
> + iommu_cleanup(iommu2);
> + }
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + device_bdf = vfio_selftests_get_bdf(&argc, argv);
> +
> + return test_harness_run(argc, argv);
> +}
> --
> 2.54.0.545.g6539524ca2-goog
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] vfio: selftests: Add support of creating multiple iommus from iommufd
2026-05-08 18:17 ` David Matlack
@ 2026-05-11 20:21 ` Samiullah Khawaja
2026-05-11 20:59 ` David Matlack
0 siblings, 1 reply; 11+ messages in thread
From: Samiullah Khawaja @ 2026-05-11 20:21 UTC (permalink / raw)
To: David Matlack
Cc: Aex Williamson, Shuah Khan, Alex Mastro, Raghavendra Rao Ananta,
kvm, linux-kselftest, linux-kernel
On Fri, May 08, 2026 at 06:17:19PM +0000, David Matlack wrote:
>On 2026-05-05 10:14 PM, Samiullah Khawaja wrote:
>> IOMMUFD allows creating multiple IOAS and HWPTs under one iommufd, Add
>> API to init a struct iommu using an already opened iommufd. The API
>> internally creates a new IOAS and also a new HWPT as an option based on
>> the flags passed to the function.
>>
>> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
>> ---
>> .../vfio/lib/include/libvfio/iommu.h | 5 ++
>> .../lib/include/libvfio/vfio_pci_device.h | 2 +
>> tools/testing/selftests/vfio/lib/iommu.c | 62 +++++++++++++++++--
>> .../selftests/vfio/lib/vfio_pci_device.c | 22 ++++++-
>> 4 files changed, 84 insertions(+), 7 deletions(-)
>>
[snip]
>> int __iommu_map(struct iommu *iommu, struct dma_region *region);
>> diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
>> index 2858885a89bb..1143ceb6a9b8 100644
>> --- a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
>> +++ b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
>> @@ -19,6 +19,7 @@ struct vfio_pci_device {
>> const char *bdf;
>> int fd;
>> int group_fd;
>> + u32 dev_id;
>
>Please introduce an initialize this field in its own patch and, in the
>commit message, explain why it is being added and how it will be used.
Agreed. Will do in next revision.
>
>>
>> struct iommu *iommu;
>>
>> @@ -65,6 +66,7 @@ void vfio_pci_config_access(struct vfio_pci_device *device, bool write,
>> #define vfio_pci_config_writew(_d, _o, _v) vfio_pci_config_write(_d, _o, _v, u16)
>> #define vfio_pci_config_writel(_d, _o, _v) vfio_pci_config_write(_d, _o, _v, u32)
>>
>> +void vfio_pci_device_attach_iommu(struct vfio_pci_device *device, struct iommu *iommu);
>> void vfio_pci_irq_enable(struct vfio_pci_device *device, u32 index,
>> u32 vector, int count);
>> void vfio_pci_irq_disable(struct vfio_pci_device *device, u32 index);
>> diff --git a/tools/testing/selftests/vfio/lib/iommu.c b/tools/testing/selftests/vfio/lib/iommu.c
>> index 035dac069d60..644c049cf9f0 100644
>> --- a/tools/testing/selftests/vfio/lib/iommu.c
>> +++ b/tools/testing/selftests/vfio/lib/iommu.c
>> @@ -408,6 +408,18 @@ struct iommu_iova_range *iommu_iova_ranges(struct iommu *iommu, u32 *nranges)
>> return ranges;
>> }
>>
[snip]
>> +struct iommu *iommufd_iommu_init(int iommufd, u32 dev_id, u32 flags)
>> +{
>> + struct iommu *iommu;
>> +
>> + iommu = iommu_alloc("iommufd");
>
>Use MODE_IOMMUFD instead of string literal.
Agreed. Will update this.
>
>> +
>> + iommu->iommufd = dup(iommufd);
>> + VFIO_ASSERT_GT(iommu->iommufd, 0);
>> +
>> + iommu->ioas_id = iommufd_ioas_alloc(iommu->iommufd);
>
>Please avoid duplicating this code. e.g. Something like this:
Agreed. Will update this.
>
>diff --git a/tools/testing/selftests/vfio/lib/iommu.c b/tools/testing/selftests/vfio/lib/iommu.c
>index 644c049cf9f0..4909cda5c840 100644
>--- a/tools/testing/selftests/vfio/lib/iommu.c
>+++ b/tools/testing/selftests/vfio/lib/iommu.c
>@@ -443,6 +443,19 @@ static struct iommu *iommu_alloc(const char *iommu_mode)
> return iommu;
> }
>
>+static void iommufd_init(struct iommu *iommu, int iommufd)
>+{
>+ /*
>+ * 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.
>+ */
>+ VFIO_ASSERT_GT(iommufd, 0);
>+
>+ iommu->iommufd = iommufd;
>+ iommu->ioas_id = iommufd_ioas_alloc(iommu->iommufd);
>+}
>+
> struct iommu *iommu_init(const char *iommu_mode)
> {
> const char *container_path;
>@@ -459,15 +472,7 @@ struct iommu *iommu_init(const char *iommu_mode)
> 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);
>+ iommufd_init(iommu, open("/dev/iommu", O_RDWR));
> }
>
> return iommu;
>@@ -478,11 +483,7 @@ struct iommu *iommufd_iommu_init(int iommufd, u32 dev_id, u32 flags)
> struct iommu *iommu;
>
> iommu = iommu_alloc("iommufd");
>-
>- iommu->iommufd = dup(iommufd);
>- VFIO_ASSERT_GT(iommu->iommufd, 0);
>-
>- iommu->ioas_id = iommufd_ioas_alloc(iommu->iommufd);
>+ iommufd_init(iommu, dup(iommufd));
>
> if (flags & IOMMUFD_IOMMU_INIT_CREATE_PT)
> iommu->hwpt_id = iommufd_hwpt_alloc(iommu, dev_id);
>
>> +
>> + if (flags & IOMMUFD_IOMMU_INIT_CREATE_PT)
>> + iommu->hwpt_id = iommufd_hwpt_alloc(iommu, dev_id);
>
>Does this need to be part of iommufd_iommu_init()? Maybe it would be
>better to expose a separate helper to allocate a HWPT for a given struct
>iommu *.
Hmm.. that is interesting.. I think we can have following immediate
possibilities when creating a struct iommu (there will be more down the
road, but can be handled in similar way):
- create using struct iommu with a new IOAS.
- create using struct iommu with existing IOAS but new HWPT.
- create using struct iommu with new IOAS and new HWPT.
I think I should probably add following flags:
IOMMUFD_IOMMU_INIT_CREATE_IOPT (IO Page Table) or _PT
IOMMUFD_IOMMU_INIT_CREATE_IOAS (IO address Space) or _AS
At least one of those should be required when creating new struct iommu
from an existing one. WDYT?
>
>This would enable use of HWPTs in iommus constructed by iommu_init() as
>well, which someone may want to do in the future.
>
>If you go this route, please introduce this in its own commit.
Both flags can be introduced in same commit or should be separate?
>
>> +
>> + return iommu;
>> +}
>> +
>> +static void iommufd_cleanup(struct iommu *iommu)
>> +{
>> + struct iommu_destroy args = {
>> + .size = sizeof(args),
>> + };
>> +
>> + if (iommu->hwpt_id) {
>> + args.id = iommu->hwpt_id;
>> + ioctl_assert(iommu->iommufd, IOMMU_DESTROY, &args);
>> + }
>> +
>> + args.id = iommu->ioas_id;
>> + ioctl_assert(iommu->iommufd, IOMMU_DESTROY, &args);
>
>
>Please create a helper function to do IOMMU_DESTROY ioctl. Then here you
>can just do:
>
> if (iommu->hwpt_id)
> iommufd_iommu_destroy(iommu, iommu->hwpt_id);
>
> iommfd_iommu_destroy(iommu, iommu->ioas_id);
Looks great. Will do.
>
>> +
>> + VFIO_ASSERT_EQ(close(iommu->iommufd), 0);
>> +}
>> +
>> void iommu_cleanup(struct iommu *iommu)
>> {
>> if (iommu->iommufd)
>> - VFIO_ASSERT_EQ(close(iommu->iommufd), 0);
>> + iommufd_cleanup(iommu);
>> else
>> VFIO_ASSERT_EQ(close(iommu->container_fd), 0);
>>
[snip]
>> +void vfio_pci_device_attach_iommu(struct vfio_pci_device *device, struct iommu *iommu)
>> +{
>> + u32 pt_id = iommu->ioas_id;
>> +
>> + /* Only iommufd supports changing struct iommu attachments */
>> + VFIO_ASSERT_TRUE(iommu->iommufd);
>> +
>> + if (iommu->hwpt_id)
>> + pt_id = iommu->hwpt_id;
>
>nit: This can be folded into the variable declaration.
>
> const u32 pt_id = iommu->hwpt_id ?: iommu->ioas_id;
Agreed
>
>> +
>> + VFIO_ASSERT_NE(pt_id, 0);
>
>Why is this check needed?
I think it is redundant. I will remove it.
>
>> + vfio_device_attach_iommufd_pt(device->fd, pt_id);
>> + device->iommu = iommu;
>> +}
>
>Please introduce vfio_pci_device_attach_iommu() in its own patch.
Agreed. Will do.
>
>> +
>> static void vfio_pci_iommufd_setup(struct vfio_pci_device *device, const char *bdf)
>> {
>> const char *cdev_path = vfio_pci_get_cdev_path(bdf);
>> @@ -350,8 +366,8 @@ static void vfio_pci_iommufd_setup(struct vfio_pci_device *device, const char *b
>> VFIO_ASSERT_GE(device->fd, 0);
>> free((void *)cdev_path);
>>
>> - vfio_device_bind_iommufd(device->fd, device->iommu->iommufd);
>> - vfio_device_attach_iommufd_pt(device->fd, device->iommu->ioas_id);
>> + device->dev_id = vfio_device_bind_iommufd(device->fd, device->iommu->iommufd);
>> + vfio_pci_device_attach_iommu(device, device->iommu);
>> }
>>
>> struct vfio_pci_device *vfio_pci_device_init(const char *bdf, struct iommu *iommu)
>> --
>> 2.54.0.545.g6539524ca2-goog
>>
Thanks,
Sami
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] vfio: selftests: Add iommufd multi iommu test
2026-05-08 20:14 ` David Matlack
@ 2026-05-11 20:53 ` Samiullah Khawaja
2026-05-11 21:10 ` David Matlack
0 siblings, 1 reply; 11+ messages in thread
From: Samiullah Khawaja @ 2026-05-11 20:53 UTC (permalink / raw)
To: David Matlack
Cc: Aex Williamson, Shuah Khan, linux-kernel, kvm, linux-kselftest
On Fri, May 08, 2026 at 08:14:08PM +0000, David Matlack wrote:
>On 2026-05-05 10:14 PM, Samiullah Khawaja wrote:
>> Add a test for iommufd to verify creation of multiple iommus using an
>> already setup iommufd and switch between them.
>>
>> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
>> ---
>> tools/testing/selftests/vfio/Makefile | 1 +
>> .../vfio/vfio_iommufd_multi_iommu_test.c | 203 ++++++++++++++++++
>> 2 files changed, 204 insertions(+)
>> create mode 100644 tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c
>>
>> diff --git a/tools/testing/selftests/vfio/Makefile b/tools/testing/selftests/vfio/Makefile
>> index 0684932d91bf..24bdeaad590f 100644
>> --- a/tools/testing/selftests/vfio/Makefile
>> +++ b/tools/testing/selftests/vfio/Makefile
>> @@ -8,6 +8,7 @@ else
>> CFLAGS = $(KHDR_INCLUDES)
>> TEST_GEN_PROGS += vfio_dma_mapping_test
>> TEST_GEN_PROGS += vfio_dma_mapping_mmio_test
>> +TEST_GEN_PROGS += vfio_iommufd_multi_iommu_test
>> TEST_GEN_PROGS += vfio_iommufd_setup_test
>> TEST_GEN_PROGS += vfio_pci_device_test
>> TEST_GEN_PROGS += vfio_pci_device_init_perf_test
>> diff --git a/tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c b/tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c
>> new file mode 100644
>> index 000000000000..7199c662637c
>> --- /dev/null
>> +++ b/tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c
>> @@ -0,0 +1,203 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +#include <sys/ioctl.h>
>> +#include <sys/mman.h>
>> +
>> +#include <linux/sizes.h>
>> +#include <linux/vfio.h>
>> +
>> +#include <libvfio.h>
>> +
>> +#include "kselftest_harness.h"
>> +
>> +static const char *device_bdf;
>> +
>> +#define IOMMU_DEFAULT 0
>> +#define IOMMU_WITH_PT 1
>> +#define IOMMU_WITHOUT_PT 2
>> +
>> +static void region_setup(struct iommu *iommu,
>> + struct iova_allocator *iova_allocator,
>> + struct dma_region *region, u64 size)
>> +{
>> + const int flags = MAP_SHARED | MAP_ANONYMOUS;
>> + const int prot = PROT_READ | PROT_WRITE;
>> + void *vaddr;
>> +
>> + vaddr = mmap(NULL, size, prot, flags, -1, 0);
>> + VFIO_ASSERT_NE(vaddr, MAP_FAILED);
>> +
>> + region->vaddr = vaddr;
>> + region->iova = iova_allocator_alloc(iova_allocator, size);
>> + region->size = size;
>> +
>> + iommu_map(iommu, region);
>> +}
>> +
>> +static void region_teardown(struct iommu *iommu, struct dma_region *region)
>> +{
>> + iommu_unmap(iommu, region);
>> + VFIO_ASSERT_EQ(munmap(region->vaddr, region->size), 0);
>> +}
>
>Can you create library helpers in the library to share with
>vfio_pci_driver_test.c?
>
> dma_region_setup()
> dma_region_teardown()
I thought it is ok to duplicate this as each test (in future) might have
different mapping requirements? we might have memfd, guest_memfd,
dma_buf with various mapping strategies for different usecases?
Or you are thinking of a basic helper that can be used by tests if they
are not doing anything fancy?
>
>> +
>> +FIXTURE(vfio_iommufd_multi_iommu_test) {
>> + struct iommu *iommu;
>> + struct vfio_pci_device *device;
>> + struct iova_allocator *iova_allocator;
>> + struct dma_region memcpy_region;
>> + void *vaddr;
>> +
>> + u64 size;
>> + void *src;
>> + void *dst;
>> + iova_t src_iova;
>> + iova_t dst_iova;
>> +};
>> +
>> +FIXTURE_SETUP(vfio_iommufd_multi_iommu_test)
>> +{
>> + struct vfio_pci_driver *driver;
>> +
>> + self->iommu = iommu_init("iommufd");
>
>MODE_IOMMUFD
Will update.
>
>> + self->device = vfio_pci_device_init(device_bdf, self->iommu);
>> + self->iova_allocator = iova_allocator_init(self->iommu);
>> +
>> + driver = &self->device->driver;
>> +
>> + region_setup(self->iommu, self->iova_allocator, &self->memcpy_region, SZ_1G);
>> + region_setup(self->iommu, self->iova_allocator, &driver->region, SZ_2M);
>> +
>> + if (driver->ops)
>> + vfio_pci_driver_init(self->device);
>> +
>> + self->size = self->memcpy_region.size / 2;
>> + self->src = self->memcpy_region.vaddr;
>> + self->dst = self->src + self->size;
>> +
>> + self->src_iova = to_iova(self->device, self->src);
>> + self->dst_iova = to_iova(self->device, self->dst);
>> +}
>> +
[snip]
>> +static void test_memcpy(struct vfio_pci_device *device, void *src, void *dst,
>> + iova_t src_iova, iova_t dst_iova, u64 size, char val)
>> +{
>> + if (!device->driver.ops)
>> + return;
>> +
>> + memset(src, val, size);
>> + memset(dst, 0, size);
>> +
>> + vfio_pci_driver_memcpy_start(device, src_iova, dst_iova, size, 100);
>
>Why do 100 copies?
Will remove in next revision.
>
>> + VFIO_ASSERT_EQ(vfio_pci_driver_memcpy_wait(device), 0);
>> + VFIO_ASSERT_EQ(memcmp(src, dst, size), 0);
>> +}
>> +
>> +TEST_F(vfio_iommufd_multi_iommu_test, memcpy)
>> +{
>> + struct dma_region memcpy_region1, driver_region1;
>> + struct dma_region memcpy_region2, driver_region2;
>> + struct iommu *iommu1;
>> + struct iommu *iommu2;
>> +
>> + iommu1 = setup_variant_iommu(self->iommu, self->device->dev_id,
>> + variant->start_iommu);
>> + if (iommu1 != self->iommu) {
>> + memcpy_region1 = self->memcpy_region;
>> + driver_region1 = self->device->driver.region;
>> +
>> + iommu_map(iommu1, &memcpy_region1);
>
>Can you make a helper to copy dma_region into a new iommu? It should set
>up the new struct dma_region based on the old one, re-initialize the
>link field, and then call iommu_map().
Can you please clarify, I didn't get it completely. so basically
something that can be called like following?
setup_copy_dma_regions(self, iommu1, &memcpy_region1, &driver_region1);
I think I get that you are concerned about link init. Maybe I can do?
int clone_dma_region(struct dma_region *source_region,
struct dma_region *dest_region);
We can maybe add it to the library?
>
>> + iommu_map(iommu1, &driver_region1);
>> +
>> + vfio_pci_device_attach_iommu(self->device, iommu1);
>> + }
>> +
>> + test_memcpy(self->device, self->src, self->dst, self->src_iova,
>> + self->dst_iova, self->size, 'x');
>
>nit: You can pass in self to avoid unpacking it in every call to
>test_memcpy(). The type would be:
>
> struct _test_data_vfio_iommufd_multi_iommu_test *self
Agreed. Will do.
>
>> +
>> + iommu2 = setup_variant_iommu(self->iommu, self->device->dev_id,
>> + variant->end_iommu);
>> + if (iommu2 != self->iommu) {
>> + memcpy_region2 = self->memcpy_region;
>> + driver_region2 = self->device->driver.region;
>> +
>> + iommu_map(iommu2, &memcpy_region2);
>> + iommu_map(iommu2, &driver_region2);
>> + }
>> +
>> + vfio_pci_device_attach_iommu(self->device, iommu2);
>> + test_memcpy(self->device, self->src, self->dst, self->src_iova,
>> + self->dst_iova, self->size, 'a');
>
>test_memcpy() zeroes self->dst every time so do you need to use a
>different character for each call?
I will update this.
>
>> +
>> + vfio_pci_device_attach_iommu(self->device, iommu1);
>> + test_memcpy(self->device, self->src, self->dst, self->src_iova,
>> + self->dst_iova, self->size, '1');
>> +
>> + if (iommu1 != self->iommu) {
>> + /* attach back to default iommu for cleanup */
>> + vfio_pci_device_attach_iommu(self->device, self->iommu);
>> + iommu_unmap(iommu1, &memcpy_region1);
>> + iommu_unmap(iommu1, &driver_region1);
>> + iommu_cleanup(iommu1);
>> + }
>> +
>> + if (iommu2 != self->iommu) {
>> + iommu_unmap(iommu2, &memcpy_region2);
>> + iommu_unmap(iommu2, &driver_region2);
>
>nit: You don't need to unmap before cleanup. Is this just to exercise
>that unmap works on the variant iommus?
Not really. I thought it is needed. But I think I will keep it to
exercise that. Will add a comment to clarify.
>
>> + iommu_cleanup(iommu2);
>> + }
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> + device_bdf = vfio_selftests_get_bdf(&argc, argv);
>> +
>> + return test_harness_run(argc, argv);
>> +}
>> --
>> 2.54.0.545.g6539524ca2-goog
>>
Thanks,
Sami
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] vfio: selftests: Add support of creating multiple iommus from iommufd
2026-05-11 20:21 ` Samiullah Khawaja
@ 2026-05-11 20:59 ` David Matlack
2026-05-11 21:41 ` Samiullah Khawaja
0 siblings, 1 reply; 11+ messages in thread
From: David Matlack @ 2026-05-11 20:59 UTC (permalink / raw)
To: Samiullah Khawaja
Cc: Aex Williamson, Shuah Khan, Alex Mastro, Raghavendra Rao Ananta,
kvm, linux-kselftest, linux-kernel
On 2026-05-11 08:21 PM, Samiullah Khawaja wrote:
> On Fri, May 08, 2026 at 06:17:19PM +0000, David Matlack wrote:
> > On 2026-05-05 10:14 PM, Samiullah Khawaja wrote:
> > @@ -478,11 +483,7 @@ struct iommu *iommufd_iommu_init(int iommufd, u32 dev_id, u32 flags)
> > struct iommu *iommu;
> >
> > iommu = iommu_alloc("iommufd");
> > -
> > - iommu->iommufd = dup(iommufd);
> > - VFIO_ASSERT_GT(iommu->iommufd, 0);
> > -
> > - iommu->ioas_id = iommufd_ioas_alloc(iommu->iommufd);
> > + iommufd_init(iommu, dup(iommufd));
> >
> > if (flags & IOMMUFD_IOMMU_INIT_CREATE_PT)
> > iommu->hwpt_id = iommufd_hwpt_alloc(iommu, dev_id);
> >
> > > +
> > > + if (flags & IOMMUFD_IOMMU_INIT_CREATE_PT)
> > > + iommu->hwpt_id = iommufd_hwpt_alloc(iommu, dev_id);
> >
> > Does this need to be part of iommufd_iommu_init()? Maybe it would be
> > better to expose a separate helper to allocate a HWPT for a given struct
> > iommu *.
>
> Hmm.. that is interesting.. I think we can have following immediate
> possibilities when creating a struct iommu (there will be more down the
> road, but can be handled in similar way):
>
> - create using struct iommu with a new IOAS.
> - create using struct iommu with existing IOAS but new HWPT.
> - create using struct iommu with new IOAS and new HWPT.
>
> I think I should probably add following flags:
>
> IOMMUFD_IOMMU_INIT_CREATE_IOPT (IO Page Table) or _PT
> IOMMUFD_IOMMU_INIT_CREATE_IOAS (IO address Space) or _AS
>
> At least one of those should be required when creating new struct iommu
> from an existing one. WDYT?
I don't love the idea of passing in flags to control the logic,
especially since not every combination of flags is valid. And also tests
would be required to pass in dev_id even if it's not needed (first
possibility in your list).
Maybe add explicit helpers for each case?
/*** static (private) helpers ***/
static u32 iommufd_hwpt_alloc(struct iommu *iommu, u32 dev_id)
{
struct iommu_hwpt_alloc args = {
.size = sizeof(args),
.pt_id = iommu->ioas_id,
.dev_id = dev_id,
};
VFIO_ASSERT_EQ(iommu->hwpt_id, 0);
ioctl_assert(iommu->iommufd, IOMMU_HWPT_ALLOC, &args);
iommu->hwpt_id = args.out_hwpt_id;
}
static struct iommu *iommufd_new(int iommufd, u32 ioas_id)
{
struct iommu *new;
new = iommu_alloc("iommufd");
new->iommufd = dup(iommufd);
VFIO_ASSERT_GT(new->iommufd, 0);
new->ioas_id = ioas_id;
return new;
}
/*** Public API for tests ***/
struct iommu *iommufd_new_ioas(struct iommu *cur)
{
return iommufd_new(cur->iommufd, iommufd_ioas_alloc(cur->iommufd));
}
struct iommu *iommufd_new_hwpt(struct iommu *cur, u32 dev_id)
{
struct iommu *new = iommufd_new(cur->iommufd, cur->ioas_id);
iommufd_hwpt_alloc(new, dev_id);
return new;
}
struct iommu *iommufd_new_ioas_hwpt(struct iommu *cur, u32 dev_id)
{
struct iommu *new = iommufd_new_ioas(cur);
iommufd_hwpt_alloc(new, dev_id);
return new;
}
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] vfio: selftests: Add iommufd multi iommu test
2026-05-11 20:53 ` Samiullah Khawaja
@ 2026-05-11 21:10 ` David Matlack
2026-05-11 21:27 ` Samiullah Khawaja
0 siblings, 1 reply; 11+ messages in thread
From: David Matlack @ 2026-05-11 21:10 UTC (permalink / raw)
To: Samiullah Khawaja
Cc: Aex Williamson, Shuah Khan, linux-kernel, kvm, linux-kselftest
On 2026-05-11 08:53 PM, Samiullah Khawaja wrote:
> On Fri, May 08, 2026 at 08:14:08PM +0000, David Matlack wrote:
> > On 2026-05-05 10:14 PM, Samiullah Khawaja wrote:
> > > Add a test for iommufd to verify creation of multiple iommus using an
> > > already setup iommufd and switch between them.
> > >
> > > Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
> > > ---
> > > tools/testing/selftests/vfio/Makefile | 1 +
> > > .../vfio/vfio_iommufd_multi_iommu_test.c | 203 ++++++++++++++++++
> > > 2 files changed, 204 insertions(+)
> > > create mode 100644 tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c
> > >
> > > diff --git a/tools/testing/selftests/vfio/Makefile b/tools/testing/selftests/vfio/Makefile
> > > index 0684932d91bf..24bdeaad590f 100644
> > > --- a/tools/testing/selftests/vfio/Makefile
> > > +++ b/tools/testing/selftests/vfio/Makefile
> > > @@ -8,6 +8,7 @@ else
> > > CFLAGS = $(KHDR_INCLUDES)
> > > TEST_GEN_PROGS += vfio_dma_mapping_test
> > > TEST_GEN_PROGS += vfio_dma_mapping_mmio_test
> > > +TEST_GEN_PROGS += vfio_iommufd_multi_iommu_test
> > > TEST_GEN_PROGS += vfio_iommufd_setup_test
> > > TEST_GEN_PROGS += vfio_pci_device_test
> > > TEST_GEN_PROGS += vfio_pci_device_init_perf_test
> > > diff --git a/tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c b/tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c
> > > new file mode 100644
> > > index 000000000000..7199c662637c
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c
> > > @@ -0,0 +1,203 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +#include <sys/ioctl.h>
> > > +#include <sys/mman.h>
> > > +
> > > +#include <linux/sizes.h>
> > > +#include <linux/vfio.h>
> > > +
> > > +#include <libvfio.h>
> > > +
> > > +#include "kselftest_harness.h"
> > > +
> > > +static const char *device_bdf;
> > > +
> > > +#define IOMMU_DEFAULT 0
> > > +#define IOMMU_WITH_PT 1
> > > +#define IOMMU_WITHOUT_PT 2
> > > +
> > > +static void region_setup(struct iommu *iommu,
> > > + struct iova_allocator *iova_allocator,
> > > + struct dma_region *region, u64 size)
> > > +{
> > > + const int flags = MAP_SHARED | MAP_ANONYMOUS;
> > > + const int prot = PROT_READ | PROT_WRITE;
> > > + void *vaddr;
> > > +
> > > + vaddr = mmap(NULL, size, prot, flags, -1, 0);
> > > + VFIO_ASSERT_NE(vaddr, MAP_FAILED);
> > > +
> > > + region->vaddr = vaddr;
> > > + region->iova = iova_allocator_alloc(iova_allocator, size);
> > > + region->size = size;
> > > +
> > > + iommu_map(iommu, region);
> > > +}
> > > +
> > > +static void region_teardown(struct iommu *iommu, struct dma_region *region)
> > > +{
> > > + iommu_unmap(iommu, region);
> > > + VFIO_ASSERT_EQ(munmap(region->vaddr, region->size), 0);
> > > +}
> >
> > Can you create library helpers in the library to share with
> > vfio_pci_driver_test.c?
> >
> > dma_region_setup()
> > dma_region_teardown()
>
> I thought it is ok to duplicate this as each test (in future) might have
> different mapping requirements? we might have memfd, guest_memfd,
> dma_buf with various mapping strategies for different usecases?
>
> Or you are thinking of a basic helper that can be used by tests if they
> are not doing anything fancy?
We have 2 tests now that need the same code to setup their regions so we
might as well share it. As new and more complex use-cases arise we can
figure out how to support them (per-test logic or fancier library
support).
> > > +TEST_F(vfio_iommufd_multi_iommu_test, memcpy)
> > > +{
> > > + struct dma_region memcpy_region1, driver_region1;
> > > + struct dma_region memcpy_region2, driver_region2;
> > > + struct iommu *iommu1;
> > > + struct iommu *iommu2;
> > > +
> > > + iommu1 = setup_variant_iommu(self->iommu, self->device->dev_id,
> > > + variant->start_iommu);
> > > + if (iommu1 != self->iommu) {
> > > + memcpy_region1 = self->memcpy_region;
> > > + driver_region1 = self->device->driver.region;
> > > +
> > > + iommu_map(iommu1, &memcpy_region1);
> >
> > Can you make a helper to copy dma_region into a new iommu? It should set
> > up the new struct dma_region based on the old one, re-initialize the
> > link field, and then call iommu_map().
>
> Can you please clarify, I didn't get it completely. so basically
> something that can be called like following?
>
> setup_copy_dma_regions(self, iommu1, &memcpy_region1, &driver_region1);
I was thinking something like this exposed by lib/iommu.c:
void iommu_copy_mapping(struct iommu *iommu,
struct dma_region *new_region,
struct dma_region *existing_region)
{
*new_region = *existing_region;
INIT_LIST_HEAD(&new_region->link);
iommu_map(iommu, new_region);
}
And then in the test you just do:
if (iommu1 != self->iommu)
iommu_copy_mapping(iommu1, memcpy_region1, self->memcpy_region);
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] vfio: selftests: Add iommufd multi iommu test
2026-05-11 21:10 ` David Matlack
@ 2026-05-11 21:27 ` Samiullah Khawaja
0 siblings, 0 replies; 11+ messages in thread
From: Samiullah Khawaja @ 2026-05-11 21:27 UTC (permalink / raw)
To: David Matlack
Cc: Aex Williamson, Shuah Khan, linux-kernel, kvm, linux-kselftest
On Mon, May 11, 2026 at 09:10:42PM +0000, David Matlack wrote:
>On 2026-05-11 08:53 PM, Samiullah Khawaja wrote:
>> On Fri, May 08, 2026 at 08:14:08PM +0000, David Matlack wrote:
>> > On 2026-05-05 10:14 PM, Samiullah Khawaja wrote:
>> > > Add a test for iommufd to verify creation of multiple iommus using an
>> > > already setup iommufd and switch between them.
>> > >
>> > > Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
>> > > ---
>> > > tools/testing/selftests/vfio/Makefile | 1 +
>> > > .../vfio/vfio_iommufd_multi_iommu_test.c | 203 ++++++++++++++++++
>> > > 2 files changed, 204 insertions(+)
>> > > create mode 100644 tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c
>> > >
>> > > diff --git a/tools/testing/selftests/vfio/Makefile b/tools/testing/selftests/vfio/Makefile
>> > > index 0684932d91bf..24bdeaad590f 100644
>> > > --- a/tools/testing/selftests/vfio/Makefile
>> > > +++ b/tools/testing/selftests/vfio/Makefile
>> > > @@ -8,6 +8,7 @@ else
>> > > CFLAGS = $(KHDR_INCLUDES)
>> > > TEST_GEN_PROGS += vfio_dma_mapping_test
>> > > TEST_GEN_PROGS += vfio_dma_mapping_mmio_test
>> > > +TEST_GEN_PROGS += vfio_iommufd_multi_iommu_test
>> > > TEST_GEN_PROGS += vfio_iommufd_setup_test
>> > > TEST_GEN_PROGS += vfio_pci_device_test
>> > > TEST_GEN_PROGS += vfio_pci_device_init_perf_test
>> > > diff --git a/tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c b/tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c
>> > > new file mode 100644
>> > > index 000000000000..7199c662637c
>> > > --- /dev/null
>> > > +++ b/tools/testing/selftests/vfio/vfio_iommufd_multi_iommu_test.c
>> > > @@ -0,0 +1,203 @@
>> > > +// SPDX-License-Identifier: GPL-2.0-only
>> > > +#include <sys/ioctl.h>
>> > > +#include <sys/mman.h>
>> > > +
>> > > +#include <linux/sizes.h>
>> > > +#include <linux/vfio.h>
>> > > +
>> > > +#include <libvfio.h>
>> > > +
>> > > +#include "kselftest_harness.h"
>> > > +
>> > > +static const char *device_bdf;
>> > > +
>> > > +#define IOMMU_DEFAULT 0
>> > > +#define IOMMU_WITH_PT 1
>> > > +#define IOMMU_WITHOUT_PT 2
>> > > +
>> > > +static void region_setup(struct iommu *iommu,
>> > > + struct iova_allocator *iova_allocator,
>> > > + struct dma_region *region, u64 size)
>> > > +{
>> > > + const int flags = MAP_SHARED | MAP_ANONYMOUS;
>> > > + const int prot = PROT_READ | PROT_WRITE;
>> > > + void *vaddr;
>> > > +
>> > > + vaddr = mmap(NULL, size, prot, flags, -1, 0);
>> > > + VFIO_ASSERT_NE(vaddr, MAP_FAILED);
>> > > +
>> > > + region->vaddr = vaddr;
>> > > + region->iova = iova_allocator_alloc(iova_allocator, size);
>> > > + region->size = size;
>> > > +
>> > > + iommu_map(iommu, region);
>> > > +}
>> > > +
>> > > +static void region_teardown(struct iommu *iommu, struct dma_region *region)
>> > > +{
>> > > + iommu_unmap(iommu, region);
>> > > + VFIO_ASSERT_EQ(munmap(region->vaddr, region->size), 0);
>> > > +}
>> >
>> > Can you create library helpers in the library to share with
>> > vfio_pci_driver_test.c?
>> >
>> > dma_region_setup()
>> > dma_region_teardown()
>>
>> I thought it is ok to duplicate this as each test (in future) might have
>> different mapping requirements? we might have memfd, guest_memfd,
>> dma_buf with various mapping strategies for different usecases?
>>
>> Or you are thinking of a basic helper that can be used by tests if they
>> are not doing anything fancy?
>
>We have 2 tests now that need the same code to setup their regions so we
>might as well share it. As new and more complex use-cases arise we can
>figure out how to support them (per-test logic or fancier library
>support).
Ok I will add this as a separate patch.
>
>> > > +TEST_F(vfio_iommufd_multi_iommu_test, memcpy)
>> > > +{
>> > > + struct dma_region memcpy_region1, driver_region1;
>> > > + struct dma_region memcpy_region2, driver_region2;
>> > > + struct iommu *iommu1;
>> > > + struct iommu *iommu2;
>> > > +
>> > > + iommu1 = setup_variant_iommu(self->iommu, self->device->dev_id,
>> > > + variant->start_iommu);
>> > > + if (iommu1 != self->iommu) {
>> > > + memcpy_region1 = self->memcpy_region;
>> > > + driver_region1 = self->device->driver.region;
>> > > +
>> > > + iommu_map(iommu1, &memcpy_region1);
>> >
>> > Can you make a helper to copy dma_region into a new iommu? It should set
>> > up the new struct dma_region based on the old one, re-initialize the
>> > link field, and then call iommu_map().
>>
>> Can you please clarify, I didn't get it completely. so basically
>> something that can be called like following?
>>
>> setup_copy_dma_regions(self, iommu1, &memcpy_region1, &driver_region1);
>
>I was thinking something like this exposed by lib/iommu.c:
>
>void iommu_copy_mapping(struct iommu *iommu,
> struct dma_region *new_region,
> struct dma_region *existing_region)
>{
> *new_region = *existing_region;
> INIT_LIST_HEAD(&new_region->link);
> iommu_map(iommu, new_region);
>}
>
>And then in the test you just do:
>
>if (iommu1 != self->iommu)
> iommu_copy_mapping(iommu1, memcpy_region1, self->memcpy_region);
Yeah that makes sense. I will update this in next revision.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] vfio: selftests: Add support of creating multiple iommus from iommufd
2026-05-11 20:59 ` David Matlack
@ 2026-05-11 21:41 ` Samiullah Khawaja
0 siblings, 0 replies; 11+ messages in thread
From: Samiullah Khawaja @ 2026-05-11 21:41 UTC (permalink / raw)
To: David Matlack
Cc: Aex Williamson, Shuah Khan, Alex Mastro, Raghavendra Rao Ananta,
kvm, linux-kselftest, linux-kernel
On Mon, May 11, 2026 at 08:59:53PM +0000, David Matlack wrote:
>On 2026-05-11 08:21 PM, Samiullah Khawaja wrote:
>> On Fri, May 08, 2026 at 06:17:19PM +0000, David Matlack wrote:
>> > On 2026-05-05 10:14 PM, Samiullah Khawaja wrote:
>
>> > @@ -478,11 +483,7 @@ struct iommu *iommufd_iommu_init(int iommufd, u32 dev_id, u32 flags)
>> > struct iommu *iommu;
>> >
>> > iommu = iommu_alloc("iommufd");
>> > -
>> > - iommu->iommufd = dup(iommufd);
>> > - VFIO_ASSERT_GT(iommu->iommufd, 0);
>> > -
>> > - iommu->ioas_id = iommufd_ioas_alloc(iommu->iommufd);
>> > + iommufd_init(iommu, dup(iommufd));
>> >
>> > if (flags & IOMMUFD_IOMMU_INIT_CREATE_PT)
>> > iommu->hwpt_id = iommufd_hwpt_alloc(iommu, dev_id);
>> >
>> > > +
>> > > + if (flags & IOMMUFD_IOMMU_INIT_CREATE_PT)
>> > > + iommu->hwpt_id = iommufd_hwpt_alloc(iommu, dev_id);
>> >
>> > Does this need to be part of iommufd_iommu_init()? Maybe it would be
>> > better to expose a separate helper to allocate a HWPT for a given struct
>> > iommu *.
>>
>> Hmm.. that is interesting.. I think we can have following immediate
>> possibilities when creating a struct iommu (there will be more down the
>> road, but can be handled in similar way):
>>
>> - create using struct iommu with a new IOAS.
>> - create using struct iommu with existing IOAS but new HWPT.
>> - create using struct iommu with new IOAS and new HWPT.
>>
>> I think I should probably add following flags:
>>
>> IOMMUFD_IOMMU_INIT_CREATE_IOPT (IO Page Table) or _PT
>> IOMMUFD_IOMMU_INIT_CREATE_IOAS (IO address Space) or _AS
>>
>> At least one of those should be required when creating new struct iommu
>> from an existing one. WDYT?
>
>I don't love the idea of passing in flags to control the logic,
>especially since not every combination of flags is valid. And also tests
>would be required to pass in dev_id even if it's not needed (first
>possibility in your list).
Agreed.
>
>Maybe add explicit helpers for each case?
>
>/*** static (private) helpers ***/
>
>static u32 iommufd_hwpt_alloc(struct iommu *iommu, u32 dev_id)
>{
> struct iommu_hwpt_alloc args = {
> .size = sizeof(args),
> .pt_id = iommu->ioas_id,
> .dev_id = dev_id,
> };
>
> VFIO_ASSERT_EQ(iommu->hwpt_id, 0);
> ioctl_assert(iommu->iommufd, IOMMU_HWPT_ALLOC, &args);
>
> iommu->hwpt_id = args.out_hwpt_id;
>}
>
>static struct iommu *iommufd_new(int iommufd, u32 ioas_id)
>{
> struct iommu *new;
>
> new = iommu_alloc("iommufd");
>
> new->iommufd = dup(iommufd);
> VFIO_ASSERT_GT(new->iommufd, 0);
>
> new->ioas_id = ioas_id;
>
> return new;
>}
>
>/*** Public API for tests ***/
>
>struct iommu *iommufd_new_ioas(struct iommu *cur)
>{
> return iommufd_new(cur->iommufd, iommufd_ioas_alloc(cur->iommufd));
>}
>
>struct iommu *iommufd_new_hwpt(struct iommu *cur, u32 dev_id)
>{
> struct iommu *new = iommufd_new(cur->iommufd, cur->ioas_id);
>
> iommufd_hwpt_alloc(new, dev_id);
> return new;
>}
>
>struct iommu *iommufd_new_ioas_hwpt(struct iommu *cur, u32 dev_id)
>{
> struct iommu *new = iommufd_new_ioas(cur);
>
> iommufd_hwpt_alloc(new, dev_id);
> return new;
>}
This looks great. I will update in next revision.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-05-11 21:41 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-05 22:14 [PATCH 0/2] vfio: selftests: Add iommufd multi iommu test Samiullah Khawaja
2026-05-05 22:14 ` [PATCH 1/2] vfio: selftests: Add support of creating multiple iommus from iommufd Samiullah Khawaja
2026-05-08 18:17 ` David Matlack
2026-05-11 20:21 ` Samiullah Khawaja
2026-05-11 20:59 ` David Matlack
2026-05-11 21:41 ` Samiullah Khawaja
2026-05-05 22:14 ` [PATCH 2/2] vfio: selftests: Add iommufd multi iommu test Samiullah Khawaja
2026-05-08 20:14 ` David Matlack
2026-05-11 20:53 ` Samiullah Khawaja
2026-05-11 21:10 ` David Matlack
2026-05-11 21:27 ` Samiullah Khawaja
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox