Re: [PATCH v4 3/7] iommu: Detaching pasid by attaching to the blocked_domain

[Baolu Lu <baolu.lu@linux.intel.com>]

On 11/10/24 12:15, Baolu Lu wrote:
> On 11/8/24 20:04, Yi Liu wrote:
>> The iommu drivers are on the way to detach pasid by attaching to the 
>> blocked
>> domain. However, this cannot be done in one shot. During the 
>> transition, iommu
>> core would select between the remove_dev_pasid op and the blocked domain.
>>
>> Suggested-by: Kevin Tian<kevin.tian@intel.com>
>> Suggested-by: Jason Gunthorpe<jgg@nvidia.com>
>> Reviewed-by: Kevin Tian<kevin.tian@intel.com>
>> Reviewed-by: Vasant Hegde<vasant.hegde@amd.com>
>> Reviewed-by: Jason Gunthorpe<jgg@nvidia.com>
>> Signed-off-by: Yi Liu<yi.l.liu@intel.com>
>> ---
>>   drivers/iommu/iommu.c | 16 ++++++++++++++--
>>   1 file changed, 14 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
> 
> with a minor comment below
> 
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 819c6e0188d5..6fd4b904f270 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -3302,8 +3302,18 @@ static void iommu_remove_dev_pasid(struct 
>> device *dev, ioasid_t pasid,
>>                      struct iommu_domain *domain)
>>   {
>>       const struct iommu_ops *ops = dev_iommu_ops(dev);
>> +    struct iommu_domain *blocked_domain = ops->blocked_domain;
>> +    int ret = 1;
>> -    ops->remove_dev_pasid(dev, pasid, domain);
>> +    if (blocked_domain && blocked_domain->ops->set_dev_pasid) {
>> +        ret = blocked_domain->ops->set_dev_pasid(blocked_domain,
>> +                             dev, pasid, domain);
> 
> How about removing "ret" and just add a WARN_ON around the return of
> setting blocking domain?
> 
>      /* Driver should never fail to set a blocking domain. */
>      WARN_ON(blocked_domain->ops->set_dev_pasid(...));

I saw this in patch 7/7. So never mind about this.

--
baolu