Re: [PATCH v9 10/11] iommu: Per-domain I/O page fault handling - Jean-Philippe Brucker

From: Jean-Philippe Brucker <jean-philippe@linaro.org>
To: "Tian, Kevin" <kevin.tian@intel.com>
Cc: "Jiang, Dave" <dave.jiang@intel.com>,
	"Raj, Ashok" <ashok.raj@intel.com>,
	"iommu@lists.linux.dev" <iommu@lists.linux.dev>,
	Robin Murphy <robin.murphy@arm.com>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Christoph Hellwig <hch@infradead.org>,
	Jean-Philippe Brucker <jean-philippe@linaro.com>,
	Vinod Koul <vkoul@kernel.org>,
	"Pan, Jacob jun" <jacob.jun.pan@intel.com>,
	Jason Gunthorpe <jgg@nvidia.com>, Will Deacon <will@kernel.org>
Subject: Re: [PATCH v9 10/11] iommu: Per-domain I/O page fault handling
Date: Tue, 28 Jun 2022 10:44:10 +0100	[thread overview]
Message-ID: <YrrNarTWvMydIDwQ@myrica> (raw)
In-Reply-To: <BN9PR11MB52768181B0EB29DBCB11AEA68CB89@BN9PR11MB5276.namprd11.prod.outlook.com>

On Tue, Jun 28, 2022 at 08:39:36AM +0000, Tian, Kevin wrote:
> > From: Lu Baolu <baolu.lu@linux.intel.com>
> > Sent: Tuesday, June 21, 2022 10:44 PM
> > 
> > Tweak the I/O page fault handling framework to route the page faults to
> > the domain and call the page fault handler retrieved from the domain.
> > This makes the I/O page fault handling framework possible to serve more
> > usage scenarios as long as they have an IOMMU domain and install a page
> > fault handler in it. Some unused functions are also removed to avoid
> > dead code.
> > 
> > The iommu_get_domain_for_dev_pasid() which retrieves attached domain
> > for a {device, PASID} pair is used. It will be used by the page fault
> > handling framework which knows {device, PASID} reported from the iommu
> > driver. We have a guarantee that the SVA domain doesn't go away during
> > IOPF handling, because unbind() waits for pending faults with
> > iopf_queue_flush_dev() before freeing the domain. Hence, there's no need
> > to synchronize life cycle of the iommu domains between the unbind() and
> > the interrupt threads.
> 
> I found iopf_queue_flush_dev() is only called in intel-iommu driver. Did
> I overlook anything?

The SMMU driver will need it as well when we upstream PRI support.
Currently it only supports stall, and that requires the device driver to
flush all DMA including stalled transactions *before* calling unbind(), so
ne need for iopf_queue_flush_dev() in this case.

Thanks,
Jean

> 
> >  static void iopf_handle_group(struct work_struct *work)
> >  {
> >  	struct iopf_group *group;
> > +	struct iommu_domain *domain;
> >  	struct iopf_fault *iopf, *next;
> >  	enum iommu_page_response_code status =
> > IOMMU_PAGE_RESP_SUCCESS;
> > 
> >  	group = container_of(work, struct iopf_group, work);
> > +	domain = iommu_get_domain_for_dev_pasid(group->dev,
> > +				group->last_fault.fault.prm.pasid);
> > +	if (!domain || !domain->iopf_handler)
> > +		status = IOMMU_PAGE_RESP_INVALID;
> 
> Miss a comment on why no refcnt is required on domain as explained
> in the commit msg.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu