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

[David Matlack <dmatlack@google.com>]

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