Re: [PATCH 4/4] dmaengine: idxd: Use DMA API for in-kernel DMA with PASID
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, &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 > > 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
> 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, > >> &wq->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
> 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, &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 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
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, &wq->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
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, > &wq->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
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, &wq->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
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
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
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(&idxd->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(&idxd->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(&idxd->pdev->dev); 533 if (pasid == INVALID_IOASID) { 534 dev_err(&idxd->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(&idxd->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
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
Re: [PATCH 4/4] dmaengine: idxd: Use DMA API for in-kernel DMA with PASID
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
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. 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
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? 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
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
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(&idxd->pdev->dev, NULL, &flags); - if (IS_ERR(sva)) { - dev_warn(&idxd->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(&idxd->pdev->dev); + if (pasid == INVALID_IOASID) { + dev_err(&idxd->pdev->dev, "No kernel DMA PASID\n"); return -ENODEV; } - - idxd->sva = sva; idxd->pasid = pasid; - dev_dbg(&idxd->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(&idxd->pdev->dev); + idxd->pasid = 0; } static int idxd_probe(struct idxd_device *idxd)
[PATCH 4/4] dmaengine: idxd: Use DMA API for in-kernel DMA with PASID
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(&idxd->pdev->dev, NULL, &flags); - if (IS_ERR(sva)) { - dev_warn(&idxd->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(&idxd->pdev->dev); + if (pasid == INVALID_IOASID) { + dev_err(&idxd->pdev->dev, "No kernel DMA PASID\n"); return -ENODEV; } - - idxd->sva = sva; idxd->pasid = pasid; - dev_dbg(&idxd->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(&idxd->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