Re: [PATCH 4/4] dmaengine: idxd: Use DMA API for in-kernel DMA with PASID

From: Jason Gunthorpe via iommu <iommu@lists.linux-foundation.org>
To: Dave Jiang <dave.jiang@intel.com>
Cc: "Tian, Kevin" <kevin.tian@intel.com>,
	Raj Ashok <ashok.raj@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Kumar, Sanjay K" <sanjay.k.kumar@intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Christoph Hellwig <hch@infradead.org>,
	iommu@lists.linux-foundation.org, Tony Luck <tony.luck@intel.com>,
	Jacob Pan <jacob.jun.pan@intel.com>,
	Barry Song <21cnbao@gmail.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Jean-Philippe Brucker <jean-philippe@linaro.com>,
	"Zanussi, Tom" <tom.zanussi@intel.com>
Subject: Re: [PATCH 4/4] dmaengine: idxd: Use DMA API for in-kernel DMA with PASID
Date: Wed, 8 Dec 2021 13:51:16 -0400	[thread overview]
Message-ID: <20211208175116.GC6385@nvidia.com> (raw)
In-Reply-To: <1ffc1366-2711-3026-fb09-8f60a260f618@intel.com>

On Wed, Dec 08, 2021 at 08:35:49AM -0700, Dave Jiang wrote:
> 
> On 12/8/2021 6:13 AM, Jason Gunthorpe wrote:
> > On Tue, Dec 07, 2021 at 05:47:14AM -0800, Jacob Pan wrote:
> > > In-kernel DMA should be managed by DMA mapping API. The existing kernel
> > > PASID support is based on the SVA machinery in SVA lib that is intended
> > > for user process SVA. The binding between a kernel PASID and kernel
> > > mapping has many flaws. See discussions in the link below.
> > > 
> > > This patch utilizes iommu_enable_pasid_dma() to enable DSA to perform DMA
> > > requests with PASID under the same mapping managed by DMA mapping API.
> > > In addition, SVA-related bits for kernel DMA are removed. As a result,
> > > DSA users shall use DMA mapping API to obtain DMA handles instead of
> > > using kernel virtual addresses.
> > Er, shouldn't this be adding dma_map/etc type calls?
> > 
> > You can't really say a driver is using the DMA API without actually
> > calling the DMA API..
> > 
> > > +	/*
> > > +	 * Try to enable both in-kernel and user DMA request with PASID.
> > > +	 * PASID is supported unless both user and kernel PASID are
> > > +	 * supported. Do not fail probe here in that idxd can still be
> > > +	 * used w/o PASID or IOMMU.
> > > +	 */
> > > +	if (iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA) ||
> > > +		idxd_enable_system_pasid(idxd)) {
> > > +		dev_warn(dev, "Failed to enable PASID\n");
> > > +	} else {
> > > +		set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
> > >   	}
> > Huh? How can the driver keep going if PASID isn't supported? I thought
> > the whole point of this was because the device cannot do DMA without
> > PASID at all?
> 
> There are 2 types of WQ supported with the DSA devices. A dedicated WQ type
> and a shared WQ type. The dedicated WQ type can support DMA with and without
> PASID. The shared wq type must have a PASID to operate. The driver can
> support dedicated WQ only without PASID usage when there is no PASID
> support.

Can you add to the cover letter why does the kernel require to use the
shared WQ?

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