Re: [PATCH v2 3/8] iommu: Add new flag to explictly request PASID capable domain

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

Jason,


On 10/3/2024 12:53 AM, Jason Gunthorpe wrote:
> On Wed, Sep 11, 2024 at 10:19:06AM +0000, Vasant Hegde wrote:
>> +static struct iommu_domain *__iommu_paging_domain_alloc_flags(struct device *dev,
>> +						       unsigned int type,
>> +						       unsigned int flags)
>>  {
>>  	const struct iommu_ops *ops;
>>  	struct iommu_domain *domain;
>> @@ -2059,9 +2067,24 @@ struct iommu_domain *iommu_paging_domain_alloc_flags(struct device *dev,
>>  	if (!domain)
>>  		return ERR_PTR(-ENOMEM);
>>  
>> -	iommu_domain_init(domain, IOMMU_DOMAIN_UNMANAGED, ops);
>> +	iommu_domain_init(domain, type, ops);
>>  	return domain;
>>  }
>> +
>> +/**
>> + * iommu_paging_domain_alloc_flags() - Allocate a paging domain
>> + * @dev: device for which the domain is allocated
>> + * @flags: Bitmap of iommufd_hwpt_alloc_flags
>> + *
>> + * Allocate a paging domain which will be managed by a kernel driver. Return
>> + * allocated domain if successful, or a ERR pointer for failure.
>> + */
>> +struct iommu_domain *iommu_paging_domain_alloc_flags(struct device *dev,
>> +						     unsigned int flags)
>> +{
>> +	return __iommu_paging_domain_alloc_flags(dev,
>> +					 IOMMU_DOMAIN_UNMANAGED, flags);
> 
> Can we get rid of the type parameter here? Like this:

This will create original problem for AMD driver. i. e. for VFIO we want to
allocate V1 page table (at least for now).


> 
>        if (req_type & __IOMMU_DOMAIN_PAGING) {
>                dom =  __iommu_paging_domain_alloc_flags(dev,
>                                dev->iommu->max_pasids ?
> 			       IOMMU_HWPT_ALLOC_PASID : 0);
> 		if (IS_ERR(dom))
> 		   return dom;
> 		dom->type = req_type;
>        }
> 
> Maybe at the end of the series, I guess you want AMD to not have a
> bisection issue?
> 
> I think this looks OK otherwise
> 
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Thanks

-Vasant