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

Baolu Lu
2024-11-10T04:16:29Z

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