Hi Kevin, On Fri, 18 Mar 2022 06:10:40 +0000, "Tian, Kevin" <kevin.t...@intel.com> wrote:
> > From: Jacob Pan <jacob.jun....@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.gp1002...@nvidia.com/ > > Signed-off-by: Jacob Pan <jacob.jun....@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. > agreed, will do. > > { > > - 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. > This should be in a separate patch. I consulted with Dave, sva module param is not needed anymore. Thanks for pointing it out. > > } > > - > > 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 > Thanks, Jacob _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu