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

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

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(...));

> +	} else {
> +		ops->remove_dev_pasid(dev, pasid, domain);
> +		ret = 0;
> +	}
> +
> +	WARN_ON(ret);
>   }

--
baolu