* [PATCH 1/2] vfio: selftests: Add support of creating multiple iommus from iommufd
[not found] <20260505221518.619123-1-skhawaja@google.com>
@ 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; 10+ 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] 10+ messages in thread
* [PATCH 2/2] vfio: selftests: Add iommufd multi iommu test
[not found] <20260505221518.619123-1-skhawaja@google.com>
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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ messages in thread
end of thread, other threads:[~2026-05-11 21:41 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260505221518.619123-1-skhawaja@google.com>
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