Messages in this thread | | | From | "Tian, Kevin" <> | Subject | RE: [PATCH v2 6/8] dmaengine: idxd: Use DMA API for in-kernel DMA with PASID | Date | Fri, 18 Mar 2022 06:10:40 +0000 |
| |
> From: Jacob Pan <jacob.jun.pan@linux.intel.com> > Sent: Tuesday, March 15, 2022 1:07 PM > > The current in-kernel supervisor PASID support is based on the SVM/SVA > machinery in SVA lib. The binding between a kernel PASID and kernel > mapping has many flaws. See discussions in the link below. > > This patch enables in-kernel DMA by switching from SVA lib to the > standard DMA mapping APIs. Since both DMA requests with and without > PASIDs are mapped identically, there is no change to how DMA APIs are > used after the kernel PASID is enabled. > > Link: https://lore.kernel.org/linux- > iommu/20210511194726.GP1002214@nvidia.com/ > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com> > --- > drivers/dma/idxd/idxd.h | 1 - > drivers/dma/idxd/init.c | 34 +++++++++------------------------- > drivers/dma/idxd/sysfs.c | 7 ------- > 3 files changed, 9 insertions(+), 33 deletions(-) > > diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h > index da72eb15f610..a09ab4a6e1c1 100644 > --- a/drivers/dma/idxd/idxd.h > +++ b/drivers/dma/idxd/idxd.h > @@ -276,7 +276,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 08a5f4310188..5d1f8dd4abf6 100644 > --- a/drivers/dma/idxd/init.c > +++ b/drivers/dma/idxd/init.c > @@ -16,6 +16,7 @@ > #include <linux/idr.h> > #include <linux/intel-svm.h> > #include <linux/iommu.h> > +#include <linux/dma-iommu.h> > #include <uapi/linux/idxd.h> > #include <linux/dmaengine.h> > #include "../dmaengine.h" > @@ -466,36 +467,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)
idxd_enable_pasid_dma() since system pasid is a confusing term now? Or just remove the idxd specific wrappers and have the caller to call iommu_enable_pasid_dma() directly given the simple logic here.
> { > - int flags; > - unsigned int pasid; > - struct iommu_sva *sva; > + u32 pasid; > + int ret; > > - 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); > - } > - > - pasid = iommu_sva_get_pasid(sva); > - if (pasid == IOMMU_PASID_INVALID) { > - iommu_sva_unbind_device(sva); > - return -ENODEV; > + ret = iommu_enable_pasid_dma(&idxd->pdev->dev, &pasid); > + if (ret) { > + dev_err(&idxd->pdev->dev, "No DMA PASID %d\n", ret); > + return ret; > } > - > - 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); > } > > static int idxd_probe(struct idxd_device *idxd) > @@ -524,10 +511,7 @@ static int idxd_probe(struct idxd_device *idxd) > } else { > dev_warn(dev, "Unable to turn on SVA feature.\n"); > } > - } else if (!sva) { > - dev_warn(dev, "User forced SVA off via module param.\n");
why removing above 2 lines? they are related to a module param thus not affected by the logic in this series.
> } > - > idxd_read_caps(idxd); > idxd_read_table_offsets(idxd); > > diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c > index 7e19ab92b61a..fde6656695ba 100644 > --- a/drivers/dma/idxd/sysfs.c > +++ b/drivers/dma/idxd/sysfs.c > @@ -839,13 +839,6 @@ static ssize_t wq_name_store(struct device *dev, > if (strlen(buf) > WQ_NAME_SIZE || strlen(buf) == 0) > return -EINVAL; > > - /* > - * This is temporarily placed here until we have SVM support for > - * dmaengine. > - */ > - if (wq->type == IDXD_WQT_KERNEL && device_pasid_enabled(wq- > >idxd)) > - return -EOPNOTSUPP; > - > memset(wq->name, 0, WQ_NAME_SIZE + 1); > strncpy(wq->name, buf, WQ_NAME_SIZE); > strreplace(wq->name, '\n', '\0'); > -- > 2.25.1
| |