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