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

[Yi Liu <yi.l.liu@intel.com>]

On 2024/8/7 21:59, Jason Gunthorpe wrote:
> On Wed, Aug 07, 2024 at 03:00:50PM +0530, Vasant Hegde wrote:
>>
>> commit 312c2a99897937834f6a679b6f9956288e8c93de
>> Author: Vasant Hegde <vasant.hegde@amd.com>
>> Date:   Wed Aug 7 14:42:24 2024 +0530
>>
>>      vfio: User iommu_ops->domain_alloc_user() to allocate IOMMU domain
>>
>>      - Include uapi/linux/iommufd.h in vfio_iommu_type1.c so that we can use
>>        HWPT flags.
>>      - Move dev_iommu_ops() from iommu-priv.h to linux/iommu.h
>>
>>      Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> 
> Yeah, this seems like a good start
> 
>> @@ -2134,8 +2135,14 @@ static void vfio_iommu_iova_insert_copy(struct vfio_iommu
>> *iommu,
>>   static int vfio_iommu_domain_alloc(struct device *dev, void *data)
>>   {
>>   	struct iommu_domain **domain = data;
>> +	const struct iommu_ops *ops = dev_iommu_ops(dev);
>> +	u32 flags = 0;
>> +
>> +	if (ops->domain_alloc_user)
>> +		*domain = ops->domain_alloc_user(dev, flags, NULL, NULL);
>> +	else
>> +		*domain = iommu_paging_domain_alloc(dev);
> 
> I suggest to wrapper this in a EXPORT_SYMBOL rather than expose
> dev_iommu_ops ?
> 
>> @@ -359,11 +359,14 @@ struct iommu_vfio_ioas {
>>    *                                   enforced on device attachment
>>    * @IOMMU_HWPT_FAULT_ID_VALID: The fault_id field of hwpt allocation data is
>>    *                             valid.
>> + * @IOMMU_HWPT_ALLOC_PASID : If set, allocate a HWPT that support PASID
>> + *
>>    */
> 
> Needs more words, maybe even two flags?
> 
>   @IOMMU_HWPT_ALLOC_DEV_PASID: When the domain is used on a device,
>   with no PASID, the device will support later attaching a PASID as
>   well. Some HW requires a specific domain format on the device to
>   allow PASID to work.
> 
>   @IOMMU_HWPT_ALLOC_PASID: The domain can be used as a PASID. The
>   domain attached to the device must have also been allocated with
>   IOMMU_HWPT_ALLOC_DEV_PASID.
> 
> And then we'd want to enforce this rule in iommufd to force userspace
> to use the flags properly. Otherwise AMD will be permanently broken :\
> 
> I'm guessing that userspace will never want to share a PASID domain
> and a device domain, and current iommu HW will treat both flags as the
> same functionality (use the AMD v2 domain)
> 
> Yi will need to accomodate these flags in his series..

sure. This appears to be a part of the iommufd pasid series. :)


-- 
Regards,
Yi Liu