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

From: Jason Gunthorpe <jgg@nvidia.com>
To: Lu Baolu <baolu.lu@linux.intel.com>
Cc: Joerg Roedel <joro@8bytes.org>,
	Christoph Hellwig <hch@infradead.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Kevin Tian <kevin.tian@intel.com>,
	Ashok Raj <ashok.raj@intel.com>, Will Deacon <will@kernel.org>,
	Robin Murphy <robin.murphy@arm.com>,
	Jean-Philippe Brucker <jean-philippe@linaro.com>,
	Dave Jiang <dave.jiang@intel.com>,
	Fenghua Yu <fenghua.yu@intel.com>, Vinod Koul <vkoul@kernel.org>,
	Eric Auger <eric.auger@redhat.com>, Liu Yi L <yi.l.liu@intel.com>,
	Jacob jun Pan <jacob.jun.pan@intel.com>,
	Zhangfei Gao <zhangfei.gao@linaro.org>,
	Zhu Tony <tony.zhu@intel.com>,
	iommu@lists.linux.dev, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v13 07/13] iommu/vt-d: Add SVA domain support
Date: Thu, 22 Sep 2022 12:49:28 -0300	[thread overview]
Message-ID: <YyyECCbmfsaDpDgJ@nvidia.com> (raw)
In-Reply-To: <20220906124458.46461-8-baolu.lu@linux.intel.com>

On Tue, Sep 06, 2022 at 08:44:52PM +0800, Lu Baolu wrote:
> +static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
> +{
> +	struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
> +	struct iommu_domain *domain;
> +
> +	/* Domain type specific cleanup: */
> +	domain = iommu_get_domain_for_dev_pasid(dev, pasid, 0);
> +	if (domain) {
> +		switch (domain->type) {
> +		case IOMMU_DOMAIN_SVA:
> +			intel_svm_remove_dev_pasid(dev, pasid);
> +			break;
> +		default:
> +			/* should never reach here */
> +			WARN_ON(1);
> +			break;

This is eventually going to need a lot more cleaning up to split out
the PASID from the SVM stuff.

SVA should *only* be a set of predefined handlers (in the core code!)
for the generic PRI mechanism, it shouldn't be entangled deeply into
PASID or the drivers like this.

When we get done with this, the flow should have the core code attach
a SVA domain to a PASID with PRI enabled and the core code should
supply a generic PRI implementation that does the mmu_notifier
stuff.

Also, stuff like this:

				/* We mandate that no page faults may be outstanding
				 * for the PASID when intel_svm_unbind_mm() is called.
				 * If that is not obeyed, subtle errors will happen.
				 * Let's make them less subtle... */

Are going to be problematic for VFIO as well. In a VFIO world the
entire RID and its entire PASID table has to be owned by VFIO and
never shared - so these sequencing issues should be solvable.

But this is all for further series..

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason