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

[Vasant Hegde <vasant.hegde@amd.com>]

Jason,



On 8/19/2024 11:22 PM, Jason Gunthorpe wrote:
> 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:

Sure. I think we are in good shape to get the proper patch.
Will post the patch soon.

-Vasant


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