Re: [PATCH v11 07/13] iommu/vt-d: Add SVA domain support

[Jason Gunthorpe]

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.

Jason