[PATCH 1/2] vfio: selftests: Add support of creating multiple iommus from iommufd

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 <redacted>
---
 .../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

On 2026-05-05 10:14 PM, Samiullah Khawaja wrote:

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.

Use MODE_IOMMUFD instead of string literal.

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);

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.

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);

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?

Please introduce vfio_pci_device_attach_iommu() in its own patch.

On Fri, May 08, 2026 at 06:17:19PM +0000, David Matlack wrote:

On 2026-05-05 10:14 PM, Samiullah Khawaja wrote:

quoted

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 <redacted>
---
 .../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]

quoted

 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.

quoted

 	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]

quoted

+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.

quoted

+
+	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.

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?

quoted

+
+	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.

quoted

+
+	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]

quoted

+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

quoted

+
+	VFIO_ASSERT_NE(pt_id, 0);

Why is this check needed?

I think it is redundant. I will remove it.

quoted

+	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.

quoted

+
 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

On 2026-05-11 08:21 PM, Samiullah Khawaja wrote:

On Fri, May 08, 2026 at 06:17:19PM +0000, David Matlack wrote:

quoted

On 2026-05-05 10:14 PM, Samiullah Khawaja wrote:

quoted

@@ -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);

quoted

+
+	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;
}

On Mon, May 11, 2026 at 08:59:53PM +0000, David Matlack wrote:

On 2026-05-11 08:21 PM, Samiullah Khawaja wrote:

quoted

On Fri, May 08, 2026 at 06:17:19PM +0000, David Matlack wrote:

quoted

On 2026-05-05 10:14 PM, Samiullah Khawaja wrote:

quoted

quoted

@@ -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);

quoted

+
+	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.