Re: [PATCH 1/2] iommu/sva: Tighten SVA bind API with explicit flags
Hi Jacob, I love your patch! Yet something to improve: [auto build test ERROR on vkoul-dmaengine/next] [also build test ERROR on char-misc/char-misc-testing v5.12-rc7] [cannot apply to iommu/next next-20210413] [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/iommu-sva-Tighten-SVA-bind-API-with-explicit-flags/20210409-094521 base: https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git next config: x86_64-randconfig-a016-20210413 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 9829f5e6b1bca9b61efc629770d28bb9014dec45) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # https://github.com/0day-ci/linux/commit/3b009446e2f451c401cff7a6d4766424acbcc890 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Jacob-Pan/iommu-sva-Tighten-SVA-bind-API-with-explicit-flags/20210409-094521 git checkout 3b009446e2f451c401cff7a6d4766424acbcc890 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 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:307:10: error: use of undeclared identifier >> 'IOMMU_SVA_BIND_SUPERVISOR' flags = IOMMU_SVA_BIND_SUPERVISOR; ^ drivers/dma/idxd/init.c:422:30: warning: shift count >= width of type [-Wshift-count-overflow] rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(64)); ^~~~ include/linux/dma-mapping.h:76:54: note: expanded from macro 'DMA_BIT_MASK' #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1)) ^ ~~~ drivers/dma/idxd/init.c:428:41: warning: shift count >= width of type [-Wshift-count-overflow] rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64)); ^~~~ include/linux/dma-mapping.h:76:54: note: expanded from macro 'DMA_BIT_MASK' #define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1)) ^ ~~~ 2 warnings and 1 error generated. vim +/IOMMU_SVA_BIND_SUPERVISOR +307 drivers/dma/idxd/init.c 300 301 static int idxd_enable_system_pasid(struct idxd_device *idxd) 302 { 303 unsigned int flags; 304 unsigned int pasid; 305 struct iommu_sva *sva; 306 > 307 flags = IOMMU_SVA_BIND_SUPERVISOR; 308 309 sva = iommu_sva_bind_device(&idxd->pdev->dev, NULL, flags); 310 if (IS_ERR(sva)) { 311 dev_warn(&idxd->pdev->dev, 312 "iommu sva bind failed: %ld\n", PTR_ERR(sva)); 313 return PTR_ERR(sva); 314 } 315 316 pasid = iommu_sva_get_pasid(sva); 317 if (pasid == IOMMU_PASID_INVALID) { 318 iommu_sva_unbind_device(sva); 319 return -ENODEV; 320 } 321 322 idxd->sva = sva; 323 idxd->pasid = pasid; 324 dev_dbg(&idxd->pdev->dev, "system pasid: %u\n", pasid); 325 return 0; 326 } 327 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH 1/2] iommu/sva: Tighten SVA bind API with explicit flags
Hi Baolu, Thanks for the view. On Fri, 9 Apr 2021 20:24:22 +0800, Lu Baolu wrote: > Hi Jacob, > > On 2021/4/9 1:08, Jacob Pan wrote: > > The void* drvdata parameter isn't really used in iommu_sva_bind_device() > > API, the current IDXD code "borrows" the drvdata for a VT-d private flag > > for supervisor SVA usage. > > > > Supervisor/Privileged mode request is a generic feature. It should be > > promoted from the VT-d vendor driver to the generic code. > > > > This patch replaces void* drvdata with a unsigned int flags parameter > > and adjusts callers accordingly. > > > > Link: https://lore.kernel.org/linux-iommu/YFhiMLR35WWMW%2FHu@myrica/ > > Suggested-by: Jean-Philippe Brucker > > Signed-off-by: Jacob Pan > > --- > > drivers/dma/idxd/cdev.c | 2 +- > > drivers/dma/idxd/init.c | 6 +++--- > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 2 +- > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 4 ++-- > > drivers/iommu/intel/Kconfig | 1 + > > drivers/iommu/intel/svm.c | 18 > > ++ drivers/iommu/iommu.c | 9 > > ++--- drivers/misc/uacce/uacce.c | 2 +- > > include/linux/intel-iommu.h | 2 +- > > include/linux/intel-svm.h | 17 ++--- > > include/linux/iommu.h | 19 > > --- 11 files changed, 40 insertions(+), 42 deletions(-) > > > > diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c > > index 0db9b82..21ec82b 100644 > > --- a/drivers/dma/idxd/cdev.c > > +++ b/drivers/dma/idxd/cdev.c > > @@ -103,7 +103,7 @@ static int idxd_cdev_open(struct inode *inode, > > struct file *filp) filp->private_data = ctx; > > > > if (device_pasid_enabled(idxd)) { > > - sva = iommu_sva_bind_device(dev, current->mm, NULL); > > + sva = iommu_sva_bind_device(dev, current->mm, 0); > > if (IS_ERR(sva)) { > > rc = PTR_ERR(sva); > > dev_err(dev, "pasid allocation failed: %d\n", > > rc); diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c > > index 085a0c3..cdc85f1 100644 > > --- a/drivers/dma/idxd/init.c > > +++ b/drivers/dma/idxd/init.c > > @@ -300,13 +300,13 @@ static struct idxd_device *idxd_alloc(struct > > pci_dev *pdev) > > static int idxd_enable_system_pasid(struct idxd_device *idxd) > > { > > - int flags; > > + unsigned int flags; > > unsigned int pasid; > > struct iommu_sva *sva; > > > > - flags = SVM_FLAG_SUPERVISOR_MODE; > > + flags = IOMMU_SVA_BIND_SUPERVISOR; > > With SVM_FLAG_SUPERVISOR_MODE removed, I guess > > #include > > in this file could be removed as well. yes, good point. > > > > > - sva = iommu_sva_bind_device(&idxd->pdev->dev, NULL, &flags); > > + 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)); diff --git > > a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c index > > bb251ca..23e287e 100644 --- > > a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c +++ > > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c @@ -354,7 +354,7 @@ > > __arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm) } > > struct iommu_sva * > > -arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm, void > > *drvdata) +arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm, > > unsigned int flags) { > > struct iommu_sva *handle; > > struct iommu_domain *domain = iommu_get_domain_for_dev(dev); > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h index f985817..b971d4d > > 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > > @@ -711,7 +711,7 @@ bool arm_smmu_master_sva_enabled(struct > > arm_smmu_master *master); int arm_smmu_master_enable_sva(struct > > arm_smmu_master *master); int arm_smmu_master_disable_sva(struct > > arm_smmu_master *master); struct iommu_sva *arm_smmu_sva_bind(struct > > device *dev, struct mm_struct *mm, > > - void *drvdata); > > + unsigned int flags); > > void arm_smmu_sva_unbind(struct iommu_sva *handle); > > u32 arm_smmu_sva_get_pasid(struct iommu_sva *handle); > > void arm_smmu_sva_notifier_synchronize(void); > > @@ -742,7 +742,7 @@ static inline int > > arm_smmu_master_disable_sva(struct arm_smmu_master *master) } > > > > static inline struct iommu_sva * > > -arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm, void > > *drvdata) +arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm, > > unsigned int flags) { > > return ERR_PTR(-ENODEV); > > } > > diff --git a/
Re: [PATCH 1/2] iommu/sva: Tighten SVA bind API with explicit flags
Hi Jacob, I love your patch! Yet something to improve: [auto build test ERROR on vkoul-dmaengine/next] [also build test ERROR on char-misc/char-misc-testing v5.12-rc7] [cannot apply to iommu/next next-20210413] [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/iommu-sva-Tighten-SVA-bind-API-with-explicit-flags/20210409-094521 base: https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git next config: x86_64-randconfig-a016-20200531 (attached as .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/3b009446e2f451c401cff7a6d4766424acbcc890 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Jacob-Pan/iommu-sva-Tighten-SVA-bind-API-with-explicit-flags/20210409-094521 git checkout 3b009446e2f451c401cff7a6d4766424acbcc890 # save the attached .config to linux build tree make W=1 ARCH=x86_64 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:307:10: error: 'IOMMU_SVA_BIND_SUPERVISOR' >> undeclared (first use in this function) 307 | flags = IOMMU_SVA_BIND_SUPERVISOR; | ^ drivers/dma/idxd/init.c:307:10: note: each undeclared identifier is reported only once for each function it appears in vim +/IOMMU_SVA_BIND_SUPERVISOR +307 drivers/dma/idxd/init.c 300 301 static int idxd_enable_system_pasid(struct idxd_device *idxd) 302 { 303 unsigned int flags; 304 unsigned int pasid; 305 struct iommu_sva *sva; 306 > 307 flags = IOMMU_SVA_BIND_SUPERVISOR; 308 309 sva = iommu_sva_bind_device(&idxd->pdev->dev, NULL, flags); 310 if (IS_ERR(sva)) { 311 dev_warn(&idxd->pdev->dev, 312 "iommu sva bind failed: %ld\n", PTR_ERR(sva)); 313 return PTR_ERR(sva); 314 } 315 316 pasid = iommu_sva_get_pasid(sva); 317 if (pasid == IOMMU_PASID_INVALID) { 318 iommu_sva_unbind_device(sva); 319 return -ENODEV; 320 } 321 322 idxd->sva = sva; 323 idxd->pasid = pasid; 324 dev_dbg(&idxd->pdev->dev, "system pasid: %u\n", pasid); 325 return 0; 326 } 327 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH 1/2] iommu/sva: Tighten SVA bind API with explicit flags
Hi Jean-Philippe, On Fri, 9 Apr 2021 12:22:21 +0200, Jean-Philippe Brucker wrote: > On Thu, Apr 08, 2021 at 10:08:55AM -0700, Jacob Pan wrote: > > The void* drvdata parameter isn't really used in iommu_sva_bind_device() > > API, > > Right, it used to be a cookie passed to the device driver in the exit_mm() > callback, but that went away with edcc40d2ab5f ("iommu: Remove > iommu_sva_ops::mm_exit()") > > > the current IDXD code "borrows" the drvdata for a VT-d private flag > > for supervisor SVA usage. > > > > Supervisor/Privileged mode request is a generic feature. It should be > > promoted from the VT-d vendor driver to the generic code. > > > > This patch replaces void* drvdata with a unsigned int flags parameter > > and adjusts callers accordingly. > > Thanks for cleaning this up. Making flags unsigned long seems more common > (I suggested int without thinking). But it doesn't matter much, we won't > get to 32 flags. > I was just thinking unsigned int is 32 bit for both 32 and 64 bit machine. > > > > Link: https://lore.kernel.org/linux-iommu/YFhiMLR35WWMW%2FHu@myrica/ > > Suggested-by: Jean-Philippe Brucker > > Signed-off-by: Jacob Pan > > --- > > drivers/dma/idxd/cdev.c | 2 +- > > drivers/dma/idxd/init.c | 6 +++--- > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 2 +- > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 4 ++-- > > drivers/iommu/intel/Kconfig | 1 + > > drivers/iommu/intel/svm.c | 18 ++ > > drivers/iommu/iommu.c | 9 ++--- > > drivers/misc/uacce/uacce.c | 2 +- > > include/linux/intel-iommu.h | 2 +- > > include/linux/intel-svm.h | 17 ++--- > > include/linux/iommu.h | 19 > > --- 11 files changed, 40 insertions(+), 42 deletions(-) > > > > diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c > > index 0db9b82..21ec82b 100644 > > --- a/drivers/dma/idxd/cdev.c > > +++ b/drivers/dma/idxd/cdev.c > > @@ -103,7 +103,7 @@ static int idxd_cdev_open(struct inode *inode, > > struct file *filp) filp->private_data = ctx; > > > > if (device_pasid_enabled(idxd)) { > > - sva = iommu_sva_bind_device(dev, current->mm, NULL); > > + sva = iommu_sva_bind_device(dev, current->mm, 0); > > if (IS_ERR(sva)) { > > rc = PTR_ERR(sva); > > dev_err(dev, "pasid allocation failed: %d\n", > > rc); diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c > > index 085a0c3..cdc85f1 100644 > > --- a/drivers/dma/idxd/init.c > > +++ b/drivers/dma/idxd/init.c > > @@ -300,13 +300,13 @@ static struct idxd_device *idxd_alloc(struct > > pci_dev *pdev) > > static int idxd_enable_system_pasid(struct idxd_device *idxd) > > { > > - int flags; > > + unsigned int flags; > > unsigned int pasid; > > struct iommu_sva *sva; > > > > - flags = SVM_FLAG_SUPERVISOR_MODE; > > + flags = IOMMU_SVA_BIND_SUPERVISOR; > > > > - sva = iommu_sva_bind_device(&idxd->pdev->dev, NULL, &flags); > > + 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)); > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c index > > bb251ca..23e287e 100644 --- > > a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c +++ > > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c @@ -354,7 +354,7 @@ > > __arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm) } > > > > struct iommu_sva * > > -arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm, void > > *drvdata) +arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm, > > unsigned int flags) > > Could you add a check on flags: > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c index > bb251cab61f3..145ceb2fc5da 100644 --- > a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c +++ > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c @@ -354,12 +354,15 @@ > __arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm) } > > struct iommu_sva * > -arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm, void > *drvdata) +arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm, > unsigned int flags) { > struct iommu_sva *handle; > struct iommu_domain *domain = iommu_get_domain_for_dev(dev); > struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); > > + if (flags) > + return ERR_PTR(-EINVAL); > + yes, will do. > if (smmu_domain->stage != ARM_SMMU_DOMAIN_S1) > return ERR_PTR(-EINVAL); > > > > > { > > struct iommu_sva *handle; > > struct iommu_domain *domain = io
Re: [PATCH 1/2] iommu/sva: Tighten SVA bind API with explicit flags
Hi Jacob, On 2021/4/9 1:08, Jacob Pan wrote: The void* drvdata parameter isn't really used in iommu_sva_bind_device() API, the current IDXD code "borrows" the drvdata for a VT-d private flag for supervisor SVA usage. Supervisor/Privileged mode request is a generic feature. It should be promoted from the VT-d vendor driver to the generic code. This patch replaces void* drvdata with a unsigned int flags parameter and adjusts callers accordingly. Link: https://lore.kernel.org/linux-iommu/YFhiMLR35WWMW%2FHu@myrica/ Suggested-by: Jean-Philippe Brucker Signed-off-by: Jacob Pan --- drivers/dma/idxd/cdev.c | 2 +- drivers/dma/idxd/init.c | 6 +++--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 2 +- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 4 ++-- drivers/iommu/intel/Kconfig | 1 + drivers/iommu/intel/svm.c | 18 ++ drivers/iommu/iommu.c | 9 ++--- drivers/misc/uacce/uacce.c | 2 +- include/linux/intel-iommu.h | 2 +- include/linux/intel-svm.h | 17 ++--- include/linux/iommu.h | 19 --- 11 files changed, 40 insertions(+), 42 deletions(-) diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c index 0db9b82..21ec82b 100644 --- a/drivers/dma/idxd/cdev.c +++ b/drivers/dma/idxd/cdev.c @@ -103,7 +103,7 @@ static int idxd_cdev_open(struct inode *inode, struct file *filp) filp->private_data = ctx; if (device_pasid_enabled(idxd)) { - sva = iommu_sva_bind_device(dev, current->mm, NULL); + sva = iommu_sva_bind_device(dev, current->mm, 0); if (IS_ERR(sva)) { rc = PTR_ERR(sva); dev_err(dev, "pasid allocation failed: %d\n", rc); diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c index 085a0c3..cdc85f1 100644 --- a/drivers/dma/idxd/init.c +++ b/drivers/dma/idxd/init.c @@ -300,13 +300,13 @@ static struct idxd_device *idxd_alloc(struct pci_dev *pdev) static int idxd_enable_system_pasid(struct idxd_device *idxd) { - int flags; + unsigned int flags; unsigned int pasid; struct iommu_sva *sva; - flags = SVM_FLAG_SUPERVISOR_MODE; + flags = IOMMU_SVA_BIND_SUPERVISOR; With SVM_FLAG_SUPERVISOR_MODE removed, I guess #include in this file could be removed as well. - sva = iommu_sva_bind_device(&idxd->pdev->dev, NULL, &flags); + 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)); diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c index bb251ca..23e287e 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c @@ -354,7 +354,7 @@ __arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm) } struct iommu_sva * -arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm, void *drvdata) +arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm, unsigned int flags) { struct iommu_sva *handle; struct iommu_domain *domain = iommu_get_domain_for_dev(dev); diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h index f985817..b971d4d 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h @@ -711,7 +711,7 @@ bool arm_smmu_master_sva_enabled(struct arm_smmu_master *master); int arm_smmu_master_enable_sva(struct arm_smmu_master *master); int arm_smmu_master_disable_sva(struct arm_smmu_master *master); struct iommu_sva *arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm, - void *drvdata); + unsigned int flags); void arm_smmu_sva_unbind(struct iommu_sva *handle); u32 arm_smmu_sva_get_pasid(struct iommu_sva *handle); void arm_smmu_sva_notifier_synchronize(void); @@ -742,7 +742,7 @@ static inline int arm_smmu_master_disable_sva(struct arm_smmu_master *master) } static inline struct iommu_sva * -arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm, void *drvdata) +arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm, unsigned int flags) { return ERR_PTR(-ENODEV); } diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig index 28a3d15..5415052 100644 --- a/drivers/iommu/intel/Kconfig +++ b/drivers/iommu/intel/Kconfig @@ -41,6 +41,7 @@ config INTEL_IOMMU_SVM select PCI_PRI select MMU_NOTIFIER select IOASID + select IOMMU_SVA_LIB Why? help Shared Virtual Memory (SVM) provides a facility for devices
Re: [PATCH 1/2] iommu/sva: Tighten SVA bind API with explicit flags
On Thu, Apr 08, 2021 at 10:08:55AM -0700, Jacob Pan wrote: > The void* drvdata parameter isn't really used in iommu_sva_bind_device() > API, Right, it used to be a cookie passed to the device driver in the exit_mm() callback, but that went away with edcc40d2ab5f ("iommu: Remove iommu_sva_ops::mm_exit()") > the current IDXD code "borrows" the drvdata for a VT-d private flag > for supervisor SVA usage. > > Supervisor/Privileged mode request is a generic feature. It should be > promoted from the VT-d vendor driver to the generic code. > > This patch replaces void* drvdata with a unsigned int flags parameter > and adjusts callers accordingly. Thanks for cleaning this up. Making flags unsigned long seems more common (I suggested int without thinking). But it doesn't matter much, we won't get to 32 flags. > > Link: https://lore.kernel.org/linux-iommu/YFhiMLR35WWMW%2FHu@myrica/ > Suggested-by: Jean-Philippe Brucker > Signed-off-by: Jacob Pan > --- > drivers/dma/idxd/cdev.c | 2 +- > drivers/dma/idxd/init.c | 6 +++--- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 2 +- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 4 ++-- > drivers/iommu/intel/Kconfig | 1 + > drivers/iommu/intel/svm.c | 18 ++ > drivers/iommu/iommu.c | 9 ++--- > drivers/misc/uacce/uacce.c | 2 +- > include/linux/intel-iommu.h | 2 +- > include/linux/intel-svm.h | 17 ++--- > include/linux/iommu.h | 19 --- > 11 files changed, 40 insertions(+), 42 deletions(-) > > diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c > index 0db9b82..21ec82b 100644 > --- a/drivers/dma/idxd/cdev.c > +++ b/drivers/dma/idxd/cdev.c > @@ -103,7 +103,7 @@ static int idxd_cdev_open(struct inode *inode, struct > file *filp) > filp->private_data = ctx; > > if (device_pasid_enabled(idxd)) { > - sva = iommu_sva_bind_device(dev, current->mm, NULL); > + sva = iommu_sva_bind_device(dev, current->mm, 0); > if (IS_ERR(sva)) { > rc = PTR_ERR(sva); > dev_err(dev, "pasid allocation failed: %d\n", rc); > diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c > index 085a0c3..cdc85f1 100644 > --- a/drivers/dma/idxd/init.c > +++ b/drivers/dma/idxd/init.c > @@ -300,13 +300,13 @@ static struct idxd_device *idxd_alloc(struct pci_dev > *pdev) > > static int idxd_enable_system_pasid(struct idxd_device *idxd) > { > - int flags; > + unsigned int flags; > unsigned int pasid; > struct iommu_sva *sva; > > - flags = SVM_FLAG_SUPERVISOR_MODE; > + flags = IOMMU_SVA_BIND_SUPERVISOR; > > - sva = iommu_sva_bind_device(&idxd->pdev->dev, NULL, &flags); > + 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)); > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > index bb251ca..23e287e 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > @@ -354,7 +354,7 @@ __arm_smmu_sva_bind(struct device *dev, struct mm_struct > *mm) > } > > struct iommu_sva * > -arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm, void *drvdata) > +arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm, unsigned int > flags) Could you add a check on flags: diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c index bb251cab61f3..145ceb2fc5da 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c @@ -354,12 +354,15 @@ __arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm) } struct iommu_sva * -arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm, void *drvdata) +arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm, unsigned int flags) { struct iommu_sva *handle; struct iommu_domain *domain = iommu_get_domain_for_dev(dev); struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain); + if (flags) + return ERR_PTR(-EINVAL); + if (smmu_domain->stage != ARM_SMMU_DOMAIN_S1) return ERR_PTR(-EINVAL); > { > struct iommu_sva *handle; > struct iommu_domain *domain = iommu_get_domain_for_dev(dev); > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > index f985817..b971d4d 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > @@ -711,7 +711,7 @@ bool arm_smmu_master_sva_enabled(st