RE: [PATCH v4 09/13] iommu/vt-d: Consolidate the dev_pasid code in intel_svm_set_dev_pasid()

Tian, Kevin
2024-11-06T07:58:31Z

> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Monday, November 4, 2024 9:19 PM
>
> Use the domain_add_dev_pasid() and domain_remove_dev_pasid().
>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>

this could move to follow patch5 as it's an immediate user on the
two helpers before talking about replacement.

> @@ -201,43 +201,29 @@ static int intel_svm_set_dev_pasid(struct
> iommu_domain *domain,
> struct iommu_domain *old)
> {
> struct device_domain_info *info = dev_iommu_priv_get(dev);
> - struct dmar_domain *dmar_domain = to_dmar_domain(domain);
> struct intel_iommu *iommu = info->iommu;
> struct mm_struct *mm = domain->mm;
> struct dev_pasid_info *dev_pasid;
> unsigned long sflags;
> - unsigned long flags;
> int ret = 0;
>
> - dev_pasid = kzalloc(sizeof(*dev_pasid), GFP_KERNEL);
> - if (!dev_pasid)
> - return -ENOMEM;
> -
> - dev_pasid->dev = dev;
> - dev_pasid->pasid = pasid;
> -
> - ret = cache_tag_assign_domain(to_dmar_domain(domain), dev,
> pasid);
> - if (ret)
> - goto free_dev_pasid;
> + dev_pasid = domain_add_dev_pasid(domain, dev, pasid);
> + if (IS_ERR(dev_pasid))
> + return PTR_ERR(dev_pasid);
>
> /* Setup the pasid table: */
> sflags = cpu_feature_enabled(X86_FEATURE_LA57) ?
> PASID_FLAG_FL5LP : 0;
> ret = intel_pasid_setup_first_level(iommu, dev, mm->pgd, pasid,
> FLPT_DEFAULT_DID, sflags);
> if (ret)
> - goto unassign_tag;
> + goto out_remove_dev_pasid;
>
> - spin_lock_irqsave(&dmar_domain->lock, flags);
> - list_add(&dev_pasid->link_domain, &dmar_domain->dev_pasids);
> - spin_unlock_irqrestore(&dmar_domain->lock, flags);
> + domain_remove_dev_pasid(old, dev, pasid);
>

this also changes the order between pasid entry setup and the list
update. and intel_mm_release() walks the dev_pasids list and
call intel_pasid_tear_down_entry() which WARN_ON if an entry
is not valid.

but looks it's still OK here as intel_mm_release() is triggered in
the last step of iommu_domain_free() which cannot happen
when an attach is still in progress.

Just raise this point in case something is overlooked. otherwise,

Reviewed-by: Kevin Tian <kevin.tian@intel.com>