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

2021-12-09 Thread Jacob Pan
Hi Kevin,

On Thu, 9 Dec 2021 01:48:09 +, "Tian, Kevin" 
wrote:

> > From: Jason Gunthorpe 
> > Sent: Thursday, December 9, 2021 1:51 AM
> >   
> > > > > + /*
> > > > > +  * 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, >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  
> 
> Two reasons:
> 
> On native the shared WQ is useful when the kernel wants to offload
> some memory operations (e.g. page-zeroing) to DSA. When #CPUs are
> more than #WQs, this allows per-cpu lock-less submissions using
> ENQCMD(PASID, payload) instruction.
> 
> In guest the virtual DSA HW may only contain a WQ in shared mode
> (unchangeable by the guest) when the host admin wants to share
> the limited WQ resource among many VMs. Then there is no choice
> in guest regardless whether it's for user or kernel controlled DMA.
I will add these to the next cover letter.


Thanks,

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


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

2021-12-08 Thread Tian, Kevin
> From: Jiang, Dave 
> Sent: Thursday, December 9, 2021 8:12 AM
> >>>
> >> Do you mean wq completion record address? It is already using DMA API.
> >>wq->compls = dma_alloc_coherent(dev, wq->compls_size,
> >> >compls_addr, GFP_KERNEL);
> >>desc->compl_dma = wq->compls_addr + idxd->data->compl_size * i;
> > I would have expected something on the queue submission side too?
> 
> DSA is different than typical DMA devices in the past. Instead of a
> software descriptor ring where the device DMA to fetch the descriptors
> after the software ringing a doorbell or writing a head index, the
> descriptors are submitted directly to the device via a CPU instruction
> (i.e. MOVDIR64B or ENQCMD(S)). The CPU takes the KVA of the 64B
> descriptor and writes to the device atomically. No DMA mapping is
> necessary in this case.
> 

To be accurate, the cpu reads the 64B descriptor from KVA and
then write the descriptor to the device atomically. 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


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

2021-12-08 Thread Tian, Kevin
> From: Jason Gunthorpe 
> Sent: Thursday, December 9, 2021 1:51 AM
> 
> > > > +   /*
> > > > +* 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, >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

Two reasons:

On native the shared WQ is useful when the kernel wants to offload
some memory operations (e.g. page-zeroing) to DSA. When #CPUs are
more than #WQs, this allows per-cpu lock-less submissions using
ENQCMD(PASID, payload) instruction.

In guest the virtual DSA HW may only contain a WQ in shared mode
(unchangeable by the guest) when the host admin wants to share
the limited WQ resource among many VMs. Then there is no choice
in guest regardless whether it's for user or kernel controlled DMA.

Thanks
Kevin
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


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

2021-12-08 Thread Dave Jiang



On 12/8/2021 4:39 PM, Jason Gunthorpe wrote:

On Wed, Dec 08, 2021 at 01:59:45PM -0800, Jacob Pan wrote:

Hi Jason,

On Wed, 8 Dec 2021 16:30:22 -0400, Jason Gunthorpe  wrote:


On Wed, Dec 08, 2021 at 11:55:16AM -0800, Jacob Pan wrote:

Hi Jason,

On Wed, 8 Dec 2021 09:13:58 -0400, Jason Gunthorpe 
wrote:

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..

The IDXD driver is not aware of addressing mode, it is up to the user of
dmaengine API to prepare the buffer mappings. Here we only set up the
PASID such that it can be picked up during DMA work submission. I
tested with /drivers/dma/dmatest.c which does dma_map_page(),
map_single etc. also tested with other pieces under development.

Ignoring the work, doesn't IDXD prepare the DMA queues itself, don't
those need the DMA API?


Do you mean wq completion record address? It is already using DMA API.
wq->compls = dma_alloc_coherent(dev, wq->compls_size,
>compls_addr, GFP_KERNEL);
desc->compl_dma = wq->compls_addr + idxd->data->compl_size * i;

I would have expected something on the queue submission side too?


DSA is different than typical DMA devices in the past. Instead of a 
software descriptor ring where the device DMA to fetch the descriptors 
after the software ringing a doorbell or writing a head index, the 
descriptors are submitted directly to the device via a CPU instruction 
(i.e. MOVDIR64B or ENQCMD(S)). The CPU takes the KVA of the 64B 
descriptor and writes to the device atomically. No DMA mapping is 
necessary in this case.






the same thing, they do not use the same IOVA's. Did you test this
with bypass mode off?

Yes with dmatest. IOVA is the default, I separated out the SATC patch which
will put internal accelerators in bypass mode. It can also be verified by
iommu debugfs dump of DMA PASID (2) and PASID 0 (RIDPASID) are pointing to
he same default domain. e.g

Well, OK then..

Jason

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


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

2021-12-08 Thread Jason Gunthorpe via iommu
On Wed, Dec 08, 2021 at 01:59:45PM -0800, Jacob Pan wrote:
> Hi Jason,
> 
> On Wed, 8 Dec 2021 16:30:22 -0400, Jason Gunthorpe  wrote:
> 
> > On Wed, Dec 08, 2021 at 11:55:16AM -0800, Jacob Pan wrote:
> > > Hi Jason,
> > > 
> > > On Wed, 8 Dec 2021 09:13:58 -0400, Jason Gunthorpe 
> > > wrote: 
> > > > > 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..  
> > > The IDXD driver is not aware of addressing mode, it is up to the user of
> > > dmaengine API to prepare the buffer mappings. Here we only set up the
> > > PASID such that it can be picked up during DMA work submission. I
> > > tested with /drivers/dma/dmatest.c which does dma_map_page(),
> > > map_single etc. also tested with other pieces under development.  
> > 
> > Ignoring the work, doesn't IDXD prepare the DMA queues itself, don't
> > those need the DMA API?
> > 
> Do you mean wq completion record address? It is already using DMA API.
>   wq->compls = dma_alloc_coherent(dev, wq->compls_size,
> >compls_addr, GFP_KERNEL);
>   desc->compl_dma = wq->compls_addr + idxd->data->compl_size * i;

I would have expected something on the queue submission side too?

> > the same thing, they do not use the same IOVA's. Did you test this
> > with bypass mode off?
> Yes with dmatest. IOVA is the default, I separated out the SATC patch which
> will put internal accelerators in bypass mode. It can also be verified by
> iommu debugfs dump of DMA PASID (2) and PASID 0 (RIDPASID) are pointing to
> he same default domain. e.g

Well, OK then..

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


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

2021-12-08 Thread Jacob Pan
Hi Jason,

On Wed, 8 Dec 2021 16:30:22 -0400, Jason Gunthorpe  wrote:

> On Wed, Dec 08, 2021 at 11:55:16AM -0800, Jacob Pan wrote:
> > Hi Jason,
> > 
> > On Wed, 8 Dec 2021 09:13:58 -0400, Jason Gunthorpe 
> > wrote: 
> > > > 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..  
> > The IDXD driver is not aware of addressing mode, it is up to the user of
> > dmaengine API to prepare the buffer mappings. Here we only set up the
> > PASID such that it can be picked up during DMA work submission. I
> > tested with /drivers/dma/dmatest.c which does dma_map_page(),
> > map_single etc. also tested with other pieces under development.  
> 
> Ignoring the work, doesn't IDXD prepare the DMA queues itself, don't
> those need the DMA API?
> 
Do you mean wq completion record address? It is already using DMA API.
wq->compls = dma_alloc_coherent(dev, wq->compls_size,
>compls_addr, GFP_KERNEL);
desc->compl_dma = wq->compls_addr + idxd->data->compl_size * i;

> I'm still very confused how this can radically change from using kSVA
> to DMA API and NOT introduce some more changes than this. They are not
I am guessing the confusion comes from that fact the user of kSVA is not
merged. We were in the process of upstreaming then abandon it. Perhaps that
is why you don't see removing kSVA code?

> the same thing, they do not use the same IOVA's. Did you test this
> with bypass mode off?
Yes with dmatest. IOVA is the default, I separated out the SATC patch which
will put internal accelerators in bypass mode. It can also be verified by
iommu debugfs dump of DMA PASID (2) and PASID 0 (RIDPASID) are pointing to
he same default domain. e.g
PASID   PASID_table_entry
0   0x000119ed7004:0x0082:0x004d
1   0x0001:0x0081:0x010d
2   0x000119ed7004:0x0082:0x004d


> 
> Jason


Thanks,

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


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

2021-12-08 Thread Jason Gunthorpe via iommu
On Wed, Dec 08, 2021 at 11:55:16AM -0800, Jacob Pan wrote:
> Hi Jason,
> 
> On Wed, 8 Dec 2021 09:13:58 -0400, Jason Gunthorpe  wrote:
> 
> > > 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..
> The IDXD driver is not aware of addressing mode, it is up to the user of
> dmaengine API to prepare the buffer mappings. Here we only set up the PASID
> such that it can be picked up during DMA work submission. I tested with
> /drivers/dma/dmatest.c which does dma_map_page(), map_single etc. also
> tested with other pieces under development.

Ignoring the work, doesn't IDXD prepare the DMA queues itself, don't
those need the DMA API?

I'm still very confused how this can radically change from using kSVA
to DMA API and NOT introduce some more changes than this. They are not
the same thing, they do not use the same IOVA's. Did you test this
with bypass mode off?

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


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

2021-12-08 Thread Jacob Pan
Hi Jason,

On Wed, 8 Dec 2021 09:13:58 -0400, Jason Gunthorpe  wrote:

> > 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..
The IDXD driver is not aware of addressing mode, it is up to the user of
dmaengine API to prepare the buffer mappings. Here we only set up the PASID
such that it can be picked up during DMA work submission. I tested with
/drivers/dma/dmatest.c which does dma_map_page(), map_single etc. also
tested with other pieces under development.

Thanks,

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


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

2021-12-08 Thread kernel test robot
Hi Jacob,

I love your patch! Yet something to improve:

[auto build test ERROR on joro-iommu/next]
[also build test ERROR on vkoul-dmaengine/next linux/master linus/master 
v5.16-rc4 next-20211208]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Jacob-Pan/Enable-PASID-for-DMA-API-users/20211208-063637
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: x86_64-randconfig-a013-20211208 
(https://download.01.org/0day-ci/archive/20211209/202112090231.mek0hpuw-...@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# 
https://github.com/0day-ci/linux/commit/018108bcd08fc145526791870b4b58edeecfca1e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Jacob-Pan/Enable-PASID-for-DMA-API-users/20211208-063637
git checkout 018108bcd08fc145526791870b4b58edeecfca1e
# save the config file to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/dma/idxd/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

   drivers/dma/idxd/init.c: In function 'idxd_enable_system_pasid':
>> drivers/dma/idxd/init.c:532:10: error: implicit declaration of function 
>> 'iommu_enable_pasid_dma'; did you mean 'iommu_enable_nesting'? 
>> [-Werror=implicit-function-declaration]
 532 |  pasid = iommu_enable_pasid_dma(>pdev->dev);
 |  ^~
 |  iommu_enable_nesting
   drivers/dma/idxd/init.c: In function 'idxd_disable_system_pasid':
>> drivers/dma/idxd/init.c:544:2: error: implicit declaration of function 
>> 'iommu_disable_pasid_dma' [-Werror=implicit-function-declaration]
 544 |  iommu_disable_pasid_dma(>pdev->dev);
 |  ^~~
   cc1: some warnings being treated as errors


vim +532 drivers/dma/idxd/init.c

   527  
   528  static int idxd_enable_system_pasid(struct idxd_device *idxd)
   529  {
   530  u32 pasid;
   531  
 > 532  pasid = iommu_enable_pasid_dma(>pdev->dev);
   533  if (pasid == INVALID_IOASID) {
   534  dev_err(>pdev->dev, "No kernel DMA PASID\n");
   535  return -ENODEV;
   536  }
   537  idxd->pasid = pasid;
   538  
   539  return 0;
   540  }
   541  
   542  static void idxd_disable_system_pasid(struct idxd_device *idxd)
   543  {
 > 544  iommu_disable_pasid_dma(>pdev->dev);
   545  idxd->pasid = 0;
   546  }
   547  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


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

2021-12-08 Thread Jason Gunthorpe via iommu
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, >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


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

2021-12-08 Thread Jacob Pan
Hi Vinod,

On Wed, 8 Dec 2021 10:26:22 +0530, Vinod Koul  wrote:

> Pls resend collecting acks. I dont have this in my queue
Will do. Sorry I missed the dmaengine list.

Thanks,

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


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

2021-12-08 Thread Dave Jiang



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, >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.





Jason

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


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

2021-12-08 Thread Jason Gunthorpe via iommu
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, >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?

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


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

2021-12-07 Thread Vinod Koul
On 07-12-21, 16:27, Dave Jiang wrote:
> 
> On 12/7/2021 6:47 AM, 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.
> > 
> > Link: 
> > https://lore.kernel.org/linux-iommu/20210511194726.gp1002...@nvidia.com/
> > Signed-off-by: Jacob Pan 
> 
> Acked-by: Dave Jiang 
> 
> Also cc Vinod and dmaengine@vger

Pls resend collecting acks. I dont have this in my queue

-- 
~Vinod
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


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

2021-12-07 Thread Dave Jiang



On 12/7/2021 6:47 AM, 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.

Link: https://lore.kernel.org/linux-iommu/20210511194726.gp1002...@nvidia.com/
Signed-off-by: Jacob Pan 


Acked-by: Dave Jiang 

Also cc Vinod and dmaengine@vger



---
  .../admin-guide/kernel-parameters.txt |  6 --
  drivers/dma/Kconfig   | 10 
  drivers/dma/idxd/idxd.h   |  1 -
  drivers/dma/idxd/init.c   | 59 ++-
  drivers/dma/idxd/sysfs.c  |  7 ---
  5 files changed, 19 insertions(+), 64 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index 9725c546a0d4..fe73d02c62f3 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1751,12 +1751,6 @@
In such case C2/C3 won't be used again.
idle=nomwait: Disable mwait for CPU C-states
  
-	idxd.sva=	[HW]

-   Format: 
-   Allow force disabling of Shared Virtual Memory (SVA)
-   support for the idxd driver. By default it is set to
-   true (1).
-
idxd.tc_override= [HW]
Format: 
Allow override of default traffic class configuration
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 6bcdb4e6a0d1..3b28bd720e7d 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -313,16 +313,6 @@ config INTEL_IDXD_COMPAT
  
  	  If unsure, say N.
  
-# Config symbol that collects all the dependencies that's necessary to

-# support shared virtual memory for the devices supported by idxd.
-config INTEL_IDXD_SVM
-   bool "Accelerator Shared Virtual Memory Support"
-   depends on INTEL_IDXD
-   depends on INTEL_IOMMU_SVM
-   depends on PCI_PRI
-   depends on PCI_PASID
-   depends on PCI_IOV
-
  config INTEL_IDXD_PERFMON
bool "Intel Data Accelerators performance monitor support"
depends on INTEL_IDXD
diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
index 0cf8d3145870..3155e3a2d3ae 100644
--- a/drivers/dma/idxd/idxd.h
+++ b/drivers/dma/idxd/idxd.h
@@ -262,7 +262,6 @@ struct idxd_device {
struct idxd_wq **wqs;
struct idxd_engine **engines;
  
-	struct iommu_sva *sva;

unsigned int pasid;
  
  	int num_groups;

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 7bf03f371ce1..44633f8113e2 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -16,6 +16,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include "../dmaengine.h"
@@ -28,10 +29,6 @@ MODULE_LICENSE("GPL v2");
  MODULE_AUTHOR("Intel Corporation");
  MODULE_IMPORT_NS(IDXD);
  
-static bool sva = true;

-module_param(sva, bool, 0644);
-MODULE_PARM_DESC(sva, "Toggle SVA support on/off");
-
  bool tc_override;
  module_param(tc_override, bool, 0644);
  MODULE_PARM_DESC(tc_override, "Override traffic class defaults");
@@ -530,36 +527,22 @@ static struct idxd_device *idxd_alloc(struct pci_dev 
*pdev, struct idxd_driver_d
  
  static int idxd_enable_system_pasid(struct idxd_device *idxd)

  {
-   int flags;
-   unsigned int pasid;
-   struct iommu_sva *sva;
-
-   flags = SVM_FLAG_SUPERVISOR_MODE;
-
-   sva = iommu_sva_bind_device(>pdev->dev, NULL, );
-   if (IS_ERR(sva)) {
-   dev_warn(>pdev->dev,
-"iommu sva bind failed: %ld\n", PTR_ERR(sva));
-   return PTR_ERR(sva);
-   }
+   u32 pasid;
  
-	pasid = iommu_sva_get_pasid(sva);

-   if (pasid == IOMMU_PASID_INVALID) {
-   iommu_sva_unbind_device(sva);
+   pasid = iommu_enable_pasid_dma(>pdev->dev);
+   if (pasid == INVALID_IOASID) {
+   dev_err(>pdev->dev, "No kernel DMA PASID\n");
return -ENODEV;
}
-
-   idxd->sva = sva;
idxd->pasid = pasid;
-   dev_dbg(>pdev->dev, "system pasid: %u\n", pasid);
+
return 0;
  }
  
  static void idxd_disable_system_pasid(struct idxd_device *idxd)

  {
-
-   iommu_sva_unbind_device(idxd->sva);
-   idxd->sva = NULL;
+   iommu_disable_pasid_dma(>pdev->dev);
+   idxd->pasid = 0;
  }
  
  static int idxd_probe(struct idxd_device *idxd)

@@ -575,21 +558,17 @@ static int 

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

2021-12-07 Thread Jacob Pan
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.

Link: https://lore.kernel.org/linux-iommu/20210511194726.gp1002...@nvidia.com/
Signed-off-by: Jacob Pan 
---
 .../admin-guide/kernel-parameters.txt |  6 --
 drivers/dma/Kconfig   | 10 
 drivers/dma/idxd/idxd.h   |  1 -
 drivers/dma/idxd/init.c   | 59 ++-
 drivers/dma/idxd/sysfs.c  |  7 ---
 5 files changed, 19 insertions(+), 64 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index 9725c546a0d4..fe73d02c62f3 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1751,12 +1751,6 @@
In such case C2/C3 won't be used again.
idle=nomwait: Disable mwait for CPU C-states
 
-   idxd.sva=   [HW]
-   Format: 
-   Allow force disabling of Shared Virtual Memory (SVA)
-   support for the idxd driver. By default it is set to
-   true (1).
-
idxd.tc_override= [HW]
Format: 
Allow override of default traffic class configuration
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 6bcdb4e6a0d1..3b28bd720e7d 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -313,16 +313,6 @@ config INTEL_IDXD_COMPAT
 
  If unsure, say N.
 
-# Config symbol that collects all the dependencies that's necessary to
-# support shared virtual memory for the devices supported by idxd.
-config INTEL_IDXD_SVM
-   bool "Accelerator Shared Virtual Memory Support"
-   depends on INTEL_IDXD
-   depends on INTEL_IOMMU_SVM
-   depends on PCI_PRI
-   depends on PCI_PASID
-   depends on PCI_IOV
-
 config INTEL_IDXD_PERFMON
bool "Intel Data Accelerators performance monitor support"
depends on INTEL_IDXD
diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
index 0cf8d3145870..3155e3a2d3ae 100644
--- a/drivers/dma/idxd/idxd.h
+++ b/drivers/dma/idxd/idxd.h
@@ -262,7 +262,6 @@ struct idxd_device {
struct idxd_wq **wqs;
struct idxd_engine **engines;
 
-   struct iommu_sva *sva;
unsigned int pasid;
 
int num_groups;
diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 7bf03f371ce1..44633f8113e2 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include "../dmaengine.h"
@@ -28,10 +29,6 @@ MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Intel Corporation");
 MODULE_IMPORT_NS(IDXD);
 
-static bool sva = true;
-module_param(sva, bool, 0644);
-MODULE_PARM_DESC(sva, "Toggle SVA support on/off");
-
 bool tc_override;
 module_param(tc_override, bool, 0644);
 MODULE_PARM_DESC(tc_override, "Override traffic class defaults");
@@ -530,36 +527,22 @@ static struct idxd_device *idxd_alloc(struct pci_dev 
*pdev, struct idxd_driver_d
 
 static int idxd_enable_system_pasid(struct idxd_device *idxd)
 {
-   int flags;
-   unsigned int pasid;
-   struct iommu_sva *sva;
-
-   flags = SVM_FLAG_SUPERVISOR_MODE;
-
-   sva = iommu_sva_bind_device(>pdev->dev, NULL, );
-   if (IS_ERR(sva)) {
-   dev_warn(>pdev->dev,
-"iommu sva bind failed: %ld\n", PTR_ERR(sva));
-   return PTR_ERR(sva);
-   }
+   u32 pasid;
 
-   pasid = iommu_sva_get_pasid(sva);
-   if (pasid == IOMMU_PASID_INVALID) {
-   iommu_sva_unbind_device(sva);
+   pasid = iommu_enable_pasid_dma(>pdev->dev);
+   if (pasid == INVALID_IOASID) {
+   dev_err(>pdev->dev, "No kernel DMA PASID\n");
return -ENODEV;
}
-
-   idxd->sva = sva;
idxd->pasid = pasid;
-   dev_dbg(>pdev->dev, "system pasid: %u\n", pasid);
+
return 0;
 }
 
 static void idxd_disable_system_pasid(struct idxd_device *idxd)
 {
-
-   iommu_sva_unbind_device(idxd->sva);
-   idxd->sva = NULL;
+   iommu_disable_pasid_dma(>pdev->dev);
+   idxd->pasid = 0;
 }
 
 static int idxd_probe(struct idxd_device *idxd)
@@ -575,21 +558,17 @@ static int idxd_probe(struct idxd_device *idxd)
 
dev_dbg(dev, "IDXD reset complete\n");
 
-   if