Re: [PATCH RFCv2] iommu: Add domain type and flag to domain_alloc_paging()

[Jason Gunthorpe <jgg@ziepe.ca>]

On Mon, Aug 19, 2024 at 01:57:11PM +0530, Vasant Hegde wrote:
> Jason,
> 
> 
> On 8/17/2024 12:07 AM, Jason Gunthorpe wrote:
> > On Fri, Aug 16, 2024 at 01:47:52PM +0530, Vasant Hegde wrote:
> >> Jason,
> >>
> >>
> >> On 8/15/2024 7:17 PM, Jason Gunthorpe wrote:
> >>> On Thu, Aug 15, 2024 at 12:59:13PM +0800, Baolu Lu wrote:
> >>>> I suppose that this also includes converting paging domain to user
> >>>> domain allocation in VFIO (ideally vhost-vdpa as well), right?
> >>>
> >>> Another thought is to add a flag to iommu_paging_domain_alloc()..
> >>>
> >>> Maybe like the below. The small duplication with
> >>> __iommu_domain_alloc() would go away when iommu_domain_alloc() finally
> >>> goes, then we'd be able to directly call iommu_paging_domain_alloc().
> >>
> >> This will work. But we have to define NO_PASID flag and pass it from vfio.
> > 
> > flags = 0 means NO_PASID
> > 
> > I didn't show it but the dma api domain's allocation path would have
> > to specify IOMMU_HWPT_ALLOC_PASID to make it work.
> 
> 
> That's fine. Then in your patch we have to prioritize domain_alloc_user() over
> domain_alloc_paging. Change will look something like below (paste the code path
> that's changing).
> 
> 
> 2037 struct iommu_domain *iommu_paging_domain_alloc_flags(struct device *dev,
> 2038                                                      unsigned int flags)
> 2039 {
> 2040         const struct iommu_ops *ops = dev_iommu_ops(dev);
> 2041         struct iommu_domain *domain;
> 2042
> 2043         if (!dev_has_iommu(dev))
> 2044                 return ERR_PTR(-ENODEV);
> 2045
> 2046         if (ops->domain_alloc_user)
> 2047                 domain = ops->domain_alloc_user(dev, flags, NULL, NULL);

I mean to make it explicit that the DMA API is requesting PASID:

--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1589,8 +1589,18 @@ EXPORT_SYMBOL_GPL(fsl_mc_device_group);
 static struct iommu_domain *
 __iommu_group_alloc_default_domain(struct iommu_group *group, int req_type)
 {
+       struct device *dev = iommu_group_first_dev(group);
+
        if (group->default_domain && group->default_domain->type == req_type)
                return group->default_domain;
+
+       /*
+        * When allocating the DMA API domain assume that the driver is going to
+        * use PASID and make sure the RID's domain is PASID compatible.
+        */
+       if (req_type & __IOMMU_DOMAIN_PAGING)
+               return iommu_paging_domain_alloc_flags(
+                       dev, dev->iommu->max_pasids ? IOMMUFD_HWPT_PASID : 0);
        return __iommu_group_domain_alloc(group, req_type);
 }

Jason