Re: [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops

From: Jason Gunthorpe via iommu <iommu@lists.linux-foundation.org>
To: Baolu Lu <baolu.lu@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>,
	Ashok Raj <ashok.raj@intel.com>,
	Robin Murphy <robin.murphy@arm.com>,
	linux-kernel@vger.kernel.org,
	Christoph Hellwig <hch@infradead.org>,
	Jean-Philippe Brucker <jean-philippe@linaro.com>,
	iommu@lists.linux-foundation.org,
	Jacob jun Pan <jacob.jun.pan@intel.com>,
	Will Deacon <will@kernel.org>
Subject: Re: [PATCH 2/5] iommu: Add blocking_domain_ops field in iommu_ops
Date: Tue, 17 May 2022 10:08:03 -0300	[thread overview]
Message-ID: <20220517130803.GT1343366@nvidia.com> (raw)
In-Reply-To: <c8492b29-bc27-ae12-d5c4-9fbbc797e310@linux.intel.com>

On Tue, May 17, 2022 at 10:37:55AM +0800, Baolu Lu wrote:
> Hi Jason,
> 
> On 2022/5/16 21:57, Jason Gunthorpe wrote:
> > On Mon, May 16, 2022 at 12:22:08PM +0100, Robin Murphy wrote:
> > > On 2022-05-16 02:57, Lu Baolu wrote:
> > > > Each IOMMU driver must provide a blocking domain ops. If the hardware
> > > > supports detaching domain from device, setting blocking domain equals
> > > > detaching the existing domain from the deivce. Otherwise, an UNMANAGED
> > > > domain without any mapping will be used instead.
> > > Unfortunately that's backwards - most of the implementations of .detach_dev
> > > are disabling translation entirely, meaning the device ends up effectively
> > > in passthrough rather than blocked.
> > Ideally we'd convert the detach_dev of every driver into either
> > a blocking or identity domain. The trick is knowing which is which..
> 
> I am still a bit puzzled about how the blocking_domain should be used when
> it is extended to support ->set_dev_pasid.
> 
> If it's a blocking domain, the IOMMU driver knows that setting the
> blocking domain to device pasid means detaching the existing one.

> But if it's an identity domain, how could the IOMMU driver choose
> between:

The identity domain would never be used for detaching a PASID. The
reason it is in this explanation is because every detach_dev op should
be deleted - and the code that it was doing can remain in the driver
as either a blocking or identity domain.

> > So, the approach here should be to go driver by driver and convert
> > detach_dev to either identity, blocking or just delete it entirely,
> > excluding the above 7 that don't support default domains. And get acks
> > from the driver owners.

> Agreed. There seems to be a long way to go. I am wondering if we could
> decouple this refactoring from my new SVA API work? We can easily switch
> .detach_dev_pasid to using blocking domain later.

Well, PASID has nothing to do with this. PASID enabled drivers would
just need:
 - Must be using default domains and must have a NULL detach_dev op
 - Must provide a blocking_domain
 - Must provide set_dev_pasid for the unmanaged and blocking domains
   it creates

That is it.

Realigning the existing drivers for their base RID support is another
task.

The appeal of this is that it matches how base RID support should look
and doesn't continue the confusing appearance that there is a strictly
paired attach/detach operation.

Any domain can be set ony and RID/PASID at any time.

Drivers intended to be used with VFIO really want a blocking_domain
anyhow for efficiency.

Jason
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu