Messages in this thread | | | Date | Tue, 23 Aug 2022 15:33:04 +0800 | Subject | Re: [PATCH v11 07/13] iommu/vt-d: Add SVA domain support | From | Baolu Lu <> |
| |
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
| |