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(>pdev->dev, NULL, flags);
310 if (IS_ERR(sva)) {
311 dev_warn(>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(>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
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
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(>pdev->dev, NULL, );
> > + sva = iommu_sva_bind_device(>pdev->dev, NULL, flags);
> > if (IS_ERR(sva)) {
> > dev_warn(>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
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(>pdev->dev, NULL, flags);
310 if (IS_ERR(sva)) {
311 dev_warn(>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(>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
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
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(>pdev->dev, NULL, );
> > + sva = iommu_sva_bind_device(>pdev->dev, NULL, flags);
> > if (IS_ERR(sva)) {
> > dev_warn(>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 =
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(>pdev->dev, NULL, );
+ sva = iommu_sva_bind_device(>pdev->dev, NULL, flags);
if (IS_ERR(sva)) {
dev_warn(>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
to access DMA
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(>pdev->dev, NULL, );
> + sva = iommu_sva_bind_device(>pdev->dev, NULL, flags);
> if (IS_ERR(sva)) {
> dev_warn(>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(struct arm_smmu_master
>
[PATCH 1/2] iommu/sva: Tighten SVA bind API with explicit flags
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;
- sva = iommu_sva_bind_device(>pdev->dev, NULL, );
+ sva = iommu_sva_bind_device(>pdev->dev, NULL, flags);
if (IS_ERR(sva)) {
dev_warn(>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
help
Shared Virtual Memory (SVM) provides a facility for devices
to access DMA resources through process address space by
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 574a7e6..4b5f8b0 100644
--- a/drivers/iommu/intel/svm.c
+++
