On 2022/8/18 21:36, Jason Gunthorpe wrote:
On Wed, Aug 17, 2022 at 09:20:18AM +0800, Lu Baolu wrote:+static int intel_svm_set_dev_pasid(struct iommu_domain *domain, + struct device *dev, ioasid_t pasid) +{ + struct device_domain_info *info = dev_iommu_priv_get(dev); + struct intel_iommu *iommu = info->iommu; + struct iommu_sva *sva; + int ret = 0; + + mutex_lock(&pasid_mutex); + /* + * Detach the domain if a blocking domain is set. Check the + * right domain type once the IOMMU driver supports a real + * blocking domain. + */ + if (!domain || domain->type == IOMMU_DOMAIN_UNMANAGED) { + intel_svm_unbind_mm(dev, pasid);See, I think this is exactly the wrong way to use the ops The blockin domain ops should have its own function that just unconditionally calls intel_svm_unbind_mm()+ } else { + struct mm_struct *mm = domain->mm; + + sva = intel_svm_bind_mm(iommu, dev, mm); + if (IS_ERR(sva)) + ret = PTR_ERR(sva);And similarly the SVA domain should have its own op that does this SVM call. Muxing the ops with tests on the domain is an anti-pattern. In fact I would say any time you see an op testing the domain->type it is very suspicious.
Both agreed. Will fix them in the next version. Best regards, baolu
