Re: [PATCH v4 09/12] iommu/amd: Enhance amd_iommu_domain_alloc_user()

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

Jason,


On 10/23/2024 6:16 PM, Jason Gunthorpe wrote:
> On Wed, Oct 23, 2024 at 10:42:04AM +0000, Vasant Hegde wrote:
>> Previous patch enhanced core layer to check device PASID capability and
>> pass right flags to ops->domain_alloc_user().
>>
>> Enhance amd_iommu_domain_alloc_user() to allocate domain with
>> appropriate page table based on flags parameter.
>>   - If flags is empty then allocate domain with default page table type.
>>     This will eventually replace ops->domain_alloc().
>>     For UNMANAGED domain, core will call this interface with flags=0. So
>>     AMD driver will continue to allocate V1 page table.
>>
>>   - If IOMMU_HWPT_ALLOC_PASID flags is passed then allocate domain with v2
>>     page table.
>>
>> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
>> ---
>>  drivers/iommu/amd/iommu.c | 18 +++++++++++++++++-
>>  1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
>> index cb6a72564c23..cdc93c794eff 100644
>> --- a/drivers/iommu/amd/iommu.c
>> +++ b/drivers/iommu/amd/iommu.c
>> @@ -2403,10 +2403,26 @@ amd_iommu_domain_alloc_user(struct device *dev, u32 flags,
>>  
>>  {
>>  	unsigned int type = IOMMU_DOMAIN_UNMANAGED;
>> +	const u32 supported_flags = IOMMU_HWPT_ALLOC_DIRTY_TRACKING |
>> +						IOMMU_HWPT_ALLOC_PASID;
>>  
>> -	if ((flags & ~IOMMU_HWPT_ALLOC_DIRTY_TRACKING) || parent || user_data)
>> +	if ((flags & ~supported_flags) || parent || user_data)
>>  		return ERR_PTR(-EOPNOTSUPP);
>>  
>> +	/* Allocate domain with default page table */
>> +	if (!flags) {
>> +		return do_iommu_domain_alloc(IOMMU_DOMAIN_DMA,
>> +					     dev, 0, amd_iommu_pgtable);
>> +	}
>> +
>> +	/* Allocate domain with v2 page table if IOMMU supports PASID. */
>> +	if (flags & IOMMU_HWPT_ALLOC_PASID) {
>> +		if (!amd_iommu_pasid_supported())
>> +			return ERR_PTR(-EOPNOTSUPP);
>> +
>> +		return do_iommu_domain_alloc(type, dev, flags, AMD_IOMMU_V2);
>> +	}
>> +
>>  	return do_iommu_domain_alloc(type, dev, flags, AMD_IOMMU_V1);
> 
> This should be more specific about what the dirty_tracking
> configuration needs:

Sure. Fixed it.

-Vasant

> 
> 	if (flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING) {
> 		if (!amd_iommu_hd_support(iommu) ||
> 		    (flags & IOMMU_HWPT_ALLOC_PASID))
> 			return ERR_PTR(-EOPNOTSUPP);
> 		return do_iommu_domain_alloc(type, dev, flags, AMD_IOMMU_V1);
> 	}
> 
> 	if (flags & IOMMU_HWPT_ALLOC_PASID) {
> 		if (!amd_iommu_pasid_supported())
> 			return ERR_PTR(-EOPNOTSUPP);
> 		return do_iommu_domain_alloc(type, dev, flags, AMD_IOMMU_V2);
> 	}
> 
> 	/* If nothing specific is required use the kernel commandline default */
> 	return do_iommu_domain_alloc(IOMMU_DOMAIN_DMA, dev, 0,
> 				     amd_iommu_pgtable);
> 
> Jason