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.
