On Tue, Jul 15, 2025 at 11:29:41AM +0100, Jonathan Cameron wrote:
> > + if (!iommufd_backend_alloc_viommu(idev->iommufd, idev->devid,
> > + IOMMU_VIOMMU_TYPE_ARM_SMMUV3,
> > + s2_hwpt_id, &viommu_id, errp)) {
> > + return false;
> > + }
[...]
> > +free_abort_hwpt:
> > + iommufd_backend_free_id(idev->iommufd, viommu->abort_hwpt_id);
> > +free_viommu:
> > + iommufd_backend_free_id(idev->iommufd, viommu->core.viommu_id);
> > + g_free(viommu);
>
> No unwinding of iommufd_backened_alloc_viommu?
> Looks like we just leak it until destruction of the fd.
>
> Maybe add a comment for those like me who aren't all that familiar with
> this stuff and see an alloc with no matching free.
Those iommufd_backend_free_id calls are the reverts. An iommufd
object is free-ed using its object id, i.e. the "viommu_id" and
"abort_hwpt_id" in the lines.
Adding comments to every single iommufd_backened_free_id() call
isn't optimal, IMHO, as that function would be invoked across
different vIOMMU files and even the vfio/iommufd core files.
Perhaps QEMU should wrap it up with a helper, E.g.
static inline void iommufd_backend_free(int iommufd, int obj_id)
{
iommufd_backend_free_id(iommufd, obj_id);
}
if it helps readability?
Nicolin
