Re: [PATCH 2/2] iommu/vt-d: Replace intel SVM APIs with generic SVA APIs

From: "Raj, Ashok" <ashok.raj@intel.com>
To: Jean-Philippe Brucker <jean-philippe@linaro.org>
Cc: "Tian, Kevin" <kevin.tian@intel.com>,
	Dave Jiang <dave.jiang@intel.com>,
	Ashok Raj <ashok.raj@intel.com>,
	Jean-Philippe Brucker <jean-philippe@linaro.com>,
	"Lu, Baolu" <baolu.lu@intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	iommu@lists.linux-foundation.org,
	David Woodhouse <dwmw2@infradead.org>
Subject: Re: [PATCH 2/2] iommu/vt-d: Replace intel SVM APIs with generic SVA APIs
Date: Mon, 23 Mar 2020 16:01:13 -0700	[thread overview]
Message-ID: <20200323230113.GA84386@otc-nc-03> (raw)
In-Reply-To: <20200320092955.GA1702630@myrica>

Hi Jean

On Fri, Mar 20, 2020 at 10:29:55AM +0100, Jean-Philippe Brucker wrote:
> > +#define to_intel_svm_dev(handle) container_of(handle, struct intel_svm_dev, sva)
> > +struct iommu_sva *
> > +intel_svm_bind(struct device *dev, struct mm_struct *mm, void *drvdata)
> > +{
> > +	struct iommu_sva *sva = ERR_PTR(-EINVAL);
> > +	struct intel_svm_dev *sdev = NULL;
> > +	int flags = 0;
> > +	int ret;
> > +
> > +	/*
> > +	 * TODO: Consolidate with generic iommu-sva bind after it is merged.
> > +	 * It will require shared SVM data structures, i.e. combine io_mm
> > +	 * and intel_svm etc.
> > +	 */
> > +	if (drvdata)
> > +		flags = *(int *)drvdata;
> 
> drvdata is more for storing device driver contexts that can be passed to
> iommu_sva_ops, but I get that this is temporary.
> 
> As usual I'm dreading supervisor mode making it into the common API. What
> are your plans regarding SUPERVISOR_MODE and PRIVATE_PASID flags?  The
> previous discussion on the subject [1] had me hoping that you could
> replace supervisor mode with normal mappings (auxiliary domains?)
> I'm less worried about PRIVATE_PASID, it would just add complexity into

We don't seem to have an immediate need for PRIVATE_PASID. There are some talks
about potential usages, but nothing concrete. I think it might be good to
get rid of it now and add when we really need.

For SUPERVISOR_MODE, the idea is to have aux domain. Baolu is working on
something to replace. Certainly the entire kernel address is opening up 
the whole kimono.. so we are looking at dynamically creating mappings on demand. 
It might take some of the benefits of SVA in general with no need to create
mappings, but for security somebody has to pay the price :-)

Cheers,
Ashok

> the API and iommu-sva implementation, but doesn't really have security
> implications.
> 
> [1] https://lore.kernel.org/linux-iommu/20190228220449.GA12682@araj-mobl1.jf.intel.com/
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu