* [PATCH] iommu: Add Kconfig help text for IOMMU_SVA
@ 2023-05-06 13:31 Jacob Pan
2023-05-06 15:19 ` kernel test robot
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Jacob Pan @ 2023-05-06 13:31 UTC (permalink / raw)
To: LKML, iommu, Jason Gunthorpe, Lu Baolu, Joerg Roedel,
Jean-Philippe Brucker, Robin Murphy
Cc: Will Deacon, Raj Ashok, Tian, Kevin, Yi Liu, Yu, Fenghua,
torvalds, Jacob Pan
Shared Virtual Addressing (SVA) is not immediately intuitive to people
who work outside IOMMU subsystem, add some explanation to make it less
opaque.
Link: https://lore.kernel.org/lkml/ZE70doFi8X3KgfrV@ziepe.ca/t/#meaa938c91857665b376db59e16b532ba9d4ea013
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
drivers/iommu/Kconfig | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index db98c3f86e8c..0f7181007dc1 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -1,4 +1,4 @@
-# SPDX-License-Identifier: GPL-2.0-only
+# PDX-License-Identifier: GPL-2.0-only
# The IOVA library may also be used by non-IOMMU_API users
config IOMMU_IOVA
tristate
@@ -155,7 +155,14 @@ config IOMMU_DMA
# Shared Virtual Addressing
config IOMMU_SVA
- bool
+ bool "Shared Virtual Addressing"
+ help
+ IOMMU and CPU share page tables for user processes thus DMA can use
+ CPU virtual address. Under intel architecture, it is also called
+ Shared Virtual Memory (SVM).
+ With SVA, DMA requests are tagged with PCIe Process Address Space ID
+ (PASID) or Substream ID (ARM SMMU term) with which page tables are
+ associated.
config FSL_PAMU
bool "Freescale IOMMU support"
--
2.25.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH] iommu: Add Kconfig help text for IOMMU_SVA 2023-05-06 13:31 [PATCH] iommu: Add Kconfig help text for IOMMU_SVA Jacob Pan @ 2023-05-06 15:19 ` kernel test robot 2023-05-06 15:39 ` kernel test robot ` (2 subsequent siblings) 3 siblings, 0 replies; 16+ messages in thread From: kernel test robot @ 2023-05-06 15:19 UTC (permalink / raw) To: Jacob Pan, LKML, iommu, Jason Gunthorpe, Lu Baolu, Joerg Roedel, Jean-Philippe Brucker, Robin Murphy Cc: oe-kbuild-all, Will Deacon, Raj Ashok, Tian, Kevin, Yi Liu, Yu, Fenghua, torvalds, Jacob Pan Hi Jacob, kernel test robot noticed the following build errors: [auto build test ERROR on next-20230505] [also build test ERROR on linus/master] [cannot apply to joro-iommu/next v6.3 v6.3-rc7 v6.3-rc6 v6.3] [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#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Jacob-Pan/iommu-Add-Kconfig-help-text-for-IOMMU_SVA/20230506-212836 base: next-20230505 patch link: https://lore.kernel.org/r/20230506133134.1492395-1-jacob.jun.pan%40linux.intel.com patch subject: [PATCH] iommu: Add Kconfig help text for IOMMU_SVA config: alpha-defconfig (https://download.01.org/0day-ci/archive/20230506/202305062305.U5i9By2z-lkp@intel.com/config) compiler: alpha-linux-gcc (GCC) 12.1.0 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 # https://github.com/intel-lab-lkp/linux/commit/7a9fdfc3792c64278f1950f3880278b989749944 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Jacob-Pan/iommu-Add-Kconfig-help-text-for-IOMMU_SVA/20230506-212836 git checkout 7a9fdfc3792c64278f1950f3880278b989749944 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=alpha olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=alpha SHELL=/bin/bash drivers/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> | Link: https://lore.kernel.org/oe-kbuild-all/202305062305.U5i9By2z-lkp@intel.com/ All errors (new ones prefixed by >>): drivers/iommu/iommu-sva.c: In function 'iommu_sva_bind_device': >> drivers/iommu/iommu-sva.c:69:32: error: invalid use of undefined type 'struct dev_iommu' 69 | max_pasids = dev->iommu->max_pasids; | ^~ >> drivers/iommu/iommu-sva.c:78:32: error: invalid application of 'sizeof' to incomplete type 'struct iommu_sva' 78 | handle = kzalloc(sizeof(*handle), GFP_KERNEL); | ^ >> drivers/iommu/iommu-sva.c:109:15: error: invalid use of undefined type 'struct iommu_sva' 109 | handle->dev = dev; | ^~ drivers/iommu/iommu-sva.c:110:15: error: invalid use of undefined type 'struct iommu_sva' 110 | handle->domain = domain; | ^~ drivers/iommu/iommu-sva.c: In function 'iommu_sva_unbind_device': drivers/iommu/iommu-sva.c:134:45: error: invalid use of undefined type 'struct iommu_sva' 134 | struct iommu_domain *domain = handle->domain; | ^~ drivers/iommu/iommu-sva.c:136:36: error: invalid use of undefined type 'struct iommu_sva' 136 | struct device *dev = handle->dev; | ^~ drivers/iommu/iommu-sva.c: In function 'iommu_sva_get_pasid': drivers/iommu/iommu-sva.c:150:45: error: invalid use of undefined type 'struct iommu_sva' 150 | struct iommu_domain *domain = handle->domain; | ^~ -- In file included from include/linux/spinlock.h:63, from include/linux/mmzone.h:8, from include/linux/gfp.h:7, from include/linux/mm.h:7, from include/linux/scatterlist.h:8, from include/linux/iommu.h:10, from drivers/iommu/io-pgfault.c:8: drivers/iommu/io-pgfault.c: In function 'iommu_queue_iopf': >> drivers/iommu/io-pgfault.c:153:35: error: invalid use of undefined type 'struct dev_iommu' 153 | lockdep_assert_held(¶m->lock); | ^~ include/linux/lockdep.h:430:61: note: in definition of macro 'lockdep_assert_held' 430 | #define lockdep_assert_held(l) do { (void)(l); } while (0) | ^ drivers/iommu/io-pgfault.c:163:27: error: invalid use of undefined type 'struct dev_iommu' 163 | iopf_param = param->iopf_param; | ^~ drivers/iommu/io-pgfault.c: In function 'iopf_queue_flush_dev': drivers/iommu/io-pgfault.c:239:26: error: invalid use of undefined type 'struct dev_iommu' 239 | mutex_lock(¶m->lock); | ^~ drivers/iommu/io-pgfault.c:240:27: error: invalid use of undefined type 'struct dev_iommu' 240 | iopf_param = param->iopf_param; | ^~ drivers/iommu/io-pgfault.c:245:28: error: invalid use of undefined type 'struct dev_iommu' 245 | mutex_unlock(¶m->lock); | ^~ drivers/iommu/io-pgfault.c: In function 'iopf_queue_add_device': drivers/iommu/io-pgfault.c:307:26: error: invalid use of undefined type 'struct dev_iommu' 307 | mutex_lock(¶m->lock); | ^~ drivers/iommu/io-pgfault.c:308:19: error: invalid use of undefined type 'struct dev_iommu' 308 | if (!param->iopf_param) { | ^~ drivers/iommu/io-pgfault.c:310:22: error: invalid use of undefined type 'struct dev_iommu' 310 | param->iopf_param = iopf_param; | ^~ drivers/iommu/io-pgfault.c:313:28: error: invalid use of undefined type 'struct dev_iommu' 313 | mutex_unlock(¶m->lock); | ^~ drivers/iommu/io-pgfault.c: In function 'iopf_queue_remove_device': drivers/iommu/io-pgfault.c:343:26: error: invalid use of undefined type 'struct dev_iommu' 343 | mutex_lock(¶m->lock); | ^~ drivers/iommu/io-pgfault.c:344:27: error: invalid use of undefined type 'struct dev_iommu' 344 | iopf_param = param->iopf_param; | ^~ drivers/iommu/io-pgfault.c:347:22: error: invalid use of undefined type 'struct dev_iommu' 347 | param->iopf_param = NULL; | ^~ drivers/iommu/io-pgfault.c:350:28: error: invalid use of undefined type 'struct dev_iommu' 350 | mutex_unlock(¶m->lock); | ^~ vim +69 drivers/iommu/iommu-sva.c be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 45 be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 46 /** be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 47 * iommu_sva_bind_device() - Bind a process address space to a device be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 48 * @dev: the device be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 49 * @mm: the mm to bind, caller must hold a reference to mm_users be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 50 * be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 51 * Create a bond between device and address space, allowing the device to be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 52 * access the mm using the PASID returned by iommu_sva_get_pasid(). If a be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 53 * bond already exists between @device and @mm, an additional internal be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 54 * reference is taken. Caller must call iommu_sva_unbind_device() be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 55 * to release each reference. be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 56 * be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 57 * iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA) must be called first, to be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 58 * initialize the required SVA features. be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 59 * be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 60 * On error, returns an ERR_PTR value. be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 61 */ be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 62 struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct *mm) be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 63 { be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 64 struct iommu_domain *domain; be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 65 struct iommu_sva *handle; be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 66 ioasid_t max_pasids; be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 67 int ret; be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 68 be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 @69 max_pasids = dev->iommu->max_pasids; be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 70 if (!max_pasids) be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 71 return ERR_PTR(-EOPNOTSUPP); be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 72 be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 73 /* Allocate mm->pasid if necessary. */ be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 74 ret = iommu_sva_alloc_pasid(mm, 1, max_pasids - 1); be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 75 if (ret) be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 76 return ERR_PTR(ret); be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 77 be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 @78 handle = kzalloc(sizeof(*handle), GFP_KERNEL); be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 79 if (!handle) be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 80 return ERR_PTR(-ENOMEM); be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 81 be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 82 mutex_lock(&iommu_sva_lock); be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 83 /* Search for an existing domain. */ be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 84 domain = iommu_get_domain_for_dev_pasid(dev, mm->pasid, be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 85 IOMMU_DOMAIN_SVA); be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 86 if (IS_ERR(domain)) { be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 87 ret = PTR_ERR(domain); be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 88 goto out_unlock; be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 89 } be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 90 be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 91 if (domain) { be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 92 domain->users++; be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 93 goto out; be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 94 } be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 95 be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 96 /* Allocate a new domain and set it on device pasid. */ be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 97 domain = iommu_sva_domain_alloc(dev, mm); be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 98 if (!domain) { be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 99 ret = -ENOMEM; be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 100 goto out_unlock; be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 101 } be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 102 be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 103 ret = iommu_attach_device_pasid(domain, dev, mm->pasid); be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 104 if (ret) be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 105 goto out_free_domain; be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 106 domain->users = 1; be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 107 out: be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 108 mutex_unlock(&iommu_sva_lock); be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 @109 handle->dev = dev; be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 110 handle->domain = domain; be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 111 be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 112 return handle; be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 113 be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 114 out_free_domain: be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 115 iommu_domain_free(domain); be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 116 out_unlock: be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 117 mutex_unlock(&iommu_sva_lock); be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 118 kfree(handle); be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 119 be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 120 return ERR_PTR(ret); be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 121 } be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 122 EXPORT_SYMBOL_GPL(iommu_sva_bind_device); be51b1d6bbff48 drivers/iommu/iommu-sva-lib.c Lu Baolu 2022-10-31 123 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iommu: Add Kconfig help text for IOMMU_SVA 2023-05-06 13:31 [PATCH] iommu: Add Kconfig help text for IOMMU_SVA Jacob Pan 2023-05-06 15:19 ` kernel test robot @ 2023-05-06 15:39 ` kernel test robot 2023-05-06 15:39 ` kernel test robot 2023-05-06 15:43 ` Linus Torvalds 3 siblings, 0 replies; 16+ messages in thread From: kernel test robot @ 2023-05-06 15:39 UTC (permalink / raw) To: Jacob Pan, LKML, iommu, Jason Gunthorpe, Lu Baolu, Joerg Roedel, Jean-Philippe Brucker, Robin Murphy Cc: oe-kbuild-all, Will Deacon, Raj Ashok, Tian, Kevin, Yi Liu, Yu, Fenghua, torvalds, Jacob Pan Hi Jacob, kernel test robot noticed the following build warnings: [auto build test WARNING on next-20230505] [also build test WARNING on linus/master] [cannot apply to joro-iommu/next v6.3 v6.3-rc7 v6.3-rc6 v6.3] [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#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Jacob-Pan/iommu-Add-Kconfig-help-text-for-IOMMU_SVA/20230506-212836 base: next-20230505 patch link: https://lore.kernel.org/r/20230506133134.1492395-1-jacob.jun.pan%40linux.intel.com patch subject: [PATCH] iommu: Add Kconfig help text for IOMMU_SVA config: csky-defconfig (https://download.01.org/0day-ci/archive/20230506/202305062355.8cUG2sjd-lkp@intel.com/config) compiler: csky-linux-gcc (GCC) 12.1.0 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 # https://github.com/intel-lab-lkp/linux/commit/7a9fdfc3792c64278f1950f3880278b989749944 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Jacob-Pan/iommu-Add-Kconfig-help-text-for-IOMMU_SVA/20230506-212836 git checkout 7a9fdfc3792c64278f1950f3880278b989749944 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=csky olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=csky SHELL=/bin/bash drivers/iommu/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> | Link: https://lore.kernel.org/oe-kbuild-all/202305062355.8cUG2sjd-lkp@intel.com/ All warnings (new ones prefixed by >>): In file included from arch/csky/include/asm/mmu_context.h:6, from include/linux/mmu_context.h:5, from drivers/iommu/iommu-sva.c:5: >> include/asm-generic/mm_hooks.h:10:40: warning: 'struct mm_struct' declared inside parameter list will not be visible outside of this definition or declaration 10 | static inline int arch_dup_mmap(struct mm_struct *oldmm, | ^~~~~~~~~ include/asm-generic/mm_hooks.h:16:42: warning: 'struct mm_struct' declared inside parameter list will not be visible outside of this definition or declaration 16 | static inline void arch_exit_mmap(struct mm_struct *mm) | ^~~~~~~~~ include/asm-generic/mm_hooks.h:20:38: warning: 'struct mm_struct' declared inside parameter list will not be visible outside of this definition or declaration 20 | static inline void arch_unmap(struct mm_struct *mm, | ^~~~~~~~~ include/asm-generic/mm_hooks.h:25:15: error: unknown type name 'bool' 25 | static inline bool arch_vma_access_permitted(struct vm_area_struct *vma, | ^~~~ include/asm-generic/mm_hooks.h:26:17: error: unknown type name 'bool' 26 | bool write, bool execute, bool foreign) | ^~~~ include/asm-generic/mm_hooks.h:1:1: note: 'bool' is defined in header '<stdbool.h>'; did you forget to '#include <stdbool.h>'? +++ |+#include <stdbool.h> 1 | /* SPDX-License-Identifier: GPL-2.0 */ include/asm-generic/mm_hooks.h:26:29: error: unknown type name 'bool' 26 | bool write, bool execute, bool foreign) | ^~~~ include/asm-generic/mm_hooks.h:26:29: note: 'bool' is defined in header '<stdbool.h>'; did you forget to '#include <stdbool.h>'? include/asm-generic/mm_hooks.h:26:43: error: unknown type name 'bool' 26 | bool write, bool execute, bool foreign) | ^~~~ include/asm-generic/mm_hooks.h:26:43: note: 'bool' is defined in header '<stdbool.h>'; did you forget to '#include <stdbool.h>'? drivers/iommu/iommu-sva.c: In function 'iommu_sva_bind_device': drivers/iommu/iommu-sva.c:69:32: error: invalid use of undefined type 'struct dev_iommu' 69 | max_pasids = dev->iommu->max_pasids; | ^~ drivers/iommu/iommu-sva.c:78:32: error: invalid application of 'sizeof' to incomplete type 'struct iommu_sva' 78 | handle = kzalloc(sizeof(*handle), GFP_KERNEL); | ^ drivers/iommu/iommu-sva.c:109:15: error: invalid use of undefined type 'struct iommu_sva' 109 | handle->dev = dev; | ^~ drivers/iommu/iommu-sva.c:110:15: error: invalid use of undefined type 'struct iommu_sva' 110 | handle->domain = domain; | ^~ drivers/iommu/iommu-sva.c: In function 'iommu_sva_unbind_device': drivers/iommu/iommu-sva.c:134:45: error: invalid use of undefined type 'struct iommu_sva' 134 | struct iommu_domain *domain = handle->domain; | ^~ drivers/iommu/iommu-sva.c:136:36: error: invalid use of undefined type 'struct iommu_sva' 136 | struct device *dev = handle->dev; | ^~ drivers/iommu/iommu-sva.c: In function 'iommu_sva_get_pasid': drivers/iommu/iommu-sva.c:150:45: error: invalid use of undefined type 'struct iommu_sva' 150 | struct iommu_domain *domain = handle->domain; | ^~ vim +10 include/asm-generic/mm_hooks.h d6dd61c831226f Jeremy Fitzhardinge 2007-05-02 9 c10e83f598d080 Thomas Gleixner 2017-12-14 @10 static inline int arch_dup_mmap(struct mm_struct *oldmm, d6dd61c831226f Jeremy Fitzhardinge 2007-05-02 11 struct mm_struct *mm) d6dd61c831226f Jeremy Fitzhardinge 2007-05-02 12 { c10e83f598d080 Thomas Gleixner 2017-12-14 13 return 0; d6dd61c831226f Jeremy Fitzhardinge 2007-05-02 14 } d6dd61c831226f Jeremy Fitzhardinge 2007-05-02 15 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iommu: Add Kconfig help text for IOMMU_SVA 2023-05-06 13:31 [PATCH] iommu: Add Kconfig help text for IOMMU_SVA Jacob Pan 2023-05-06 15:19 ` kernel test robot 2023-05-06 15:39 ` kernel test robot @ 2023-05-06 15:39 ` kernel test robot 2023-05-06 15:43 ` Linus Torvalds 3 siblings, 0 replies; 16+ messages in thread From: kernel test robot @ 2023-05-06 15:39 UTC (permalink / raw) To: Jacob Pan, LKML, iommu, Jason Gunthorpe, Lu Baolu, Joerg Roedel, Jean-Philippe Brucker, Robin Murphy Cc: oe-kbuild-all, Will Deacon, Raj Ashok, Tian, Kevin, Yi Liu, Yu, Fenghua, torvalds, Jacob Pan Hi Jacob, kernel test robot noticed the following build warnings: [auto build test WARNING on next-20230505] [also build test WARNING on linus/master] [cannot apply to joro-iommu/next v6.3 v6.3-rc7 v6.3-rc6 v6.3] [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#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Jacob-Pan/iommu-Add-Kconfig-help-text-for-IOMMU_SVA/20230506-212836 base: next-20230505 patch link: https://lore.kernel.org/r/20230506133134.1492395-1-jacob.jun.pan%40linux.intel.com patch subject: [PATCH] iommu: Add Kconfig help text for IOMMU_SVA config: sparc-defconfig (https://download.01.org/0day-ci/archive/20230506/202305062339.d6Rnj38E-lkp@intel.com/config) compiler: sparc-linux-gcc (GCC) 12.1.0 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 # https://github.com/intel-lab-lkp/linux/commit/7a9fdfc3792c64278f1950f3880278b989749944 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Jacob-Pan/iommu-Add-Kconfig-help-text-for-IOMMU_SVA/20230506-212836 git checkout 7a9fdfc3792c64278f1950f3880278b989749944 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc SHELL=/bin/bash drivers/iommu/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> | Link: https://lore.kernel.org/oe-kbuild-all/202305062339.d6Rnj38E-lkp@intel.com/ All warnings (new ones prefixed by >>): In file included from arch/sparc/include/asm/mmu_context_32.h:7, from arch/sparc/include/asm/mmu_context.h:7, from include/linux/mmu_context.h:5, from drivers/iommu/iommu-sva.c:5: include/asm-generic/mm_hooks.h:10:40: warning: 'struct mm_struct' declared inside parameter list will not be visible outside of this definition or declaration 10 | static inline int arch_dup_mmap(struct mm_struct *oldmm, | ^~~~~~~~~ include/asm-generic/mm_hooks.h:16:42: warning: 'struct mm_struct' declared inside parameter list will not be visible outside of this definition or declaration 16 | static inline void arch_exit_mmap(struct mm_struct *mm) | ^~~~~~~~~ include/asm-generic/mm_hooks.h:20:38: warning: 'struct mm_struct' declared inside parameter list will not be visible outside of this definition or declaration 20 | static inline void arch_unmap(struct mm_struct *mm, | ^~~~~~~~~ include/asm-generic/mm_hooks.h:25:15: error: unknown type name 'bool' 25 | static inline bool arch_vma_access_permitted(struct vm_area_struct *vma, | ^~~~ include/asm-generic/mm_hooks.h:26:17: error: unknown type name 'bool' 26 | bool write, bool execute, bool foreign) | ^~~~ include/asm-generic/mm_hooks.h:1:1: note: 'bool' is defined in header '<stdbool.h>'; did you forget to '#include <stdbool.h>'? +++ |+#include <stdbool.h> 1 | /* SPDX-License-Identifier: GPL-2.0 */ include/asm-generic/mm_hooks.h:26:29: error: unknown type name 'bool' 26 | bool write, bool execute, bool foreign) | ^~~~ include/asm-generic/mm_hooks.h:26:29: note: 'bool' is defined in header '<stdbool.h>'; did you forget to '#include <stdbool.h>'? include/asm-generic/mm_hooks.h:26:43: error: unknown type name 'bool' 26 | bool write, bool execute, bool foreign) | ^~~~ include/asm-generic/mm_hooks.h:26:43: note: 'bool' is defined in header '<stdbool.h>'; did you forget to '#include <stdbool.h>'? >> arch/sparc/include/asm/mmu_context_32.h:13:54: warning: 'struct mm_struct' declared inside parameter list will not be visible outside of this definition or declaration 13 | int init_new_context(struct task_struct *tsk, struct mm_struct *mm); | ^~~~~~~~~ >> arch/sparc/include/asm/mmu_context_32.h:13:29: warning: 'struct task_struct' declared inside parameter list will not be visible outside of this definition or declaration 13 | int init_new_context(struct task_struct *tsk, struct mm_struct *mm); | ^~~~~~~~~~~ arch/sparc/include/asm/mmu_context_32.h:21:29: warning: 'struct mm_struct' declared inside parameter list will not be visible outside of this definition or declaration 21 | void destroy_context(struct mm_struct *mm); | ^~~~~~~~~ arch/sparc/include/asm/mmu_context_32.h:25:23: warning: 'struct task_struct' declared inside parameter list will not be visible outside of this definition or declaration 25 | struct task_struct *tsk); | ^~~~~~~~~~~ arch/sparc/include/asm/mmu_context_32.h:24:23: warning: 'struct mm_struct' declared inside parameter list will not be visible outside of this definition or declaration 24 | void switch_mm(struct mm_struct *old_mm, struct mm_struct *mm, | ^~~~~~~~~ In file included from arch/sparc/include/asm/mmu.h:7, from include/linux/mmu_context.h:6: arch/sparc/include/asm/mmu_32.h:9:8: error: unknown type name 'ctxd_t' 9 | extern ctxd_t *srmmu_ctx_table_phys; | ^~~~~~ include/linux/mmu_context.h:39:15: error: unknown type name 'bool' 39 | static inline bool arch_pgtable_dma_compat(struct mm_struct *mm) | ^~~~ include/linux/mmu_context.h: In function 'arch_pgtable_dma_compat': include/linux/mmu_context.h:41:16: error: 'true' undeclared (first use in this function) 41 | return true; | ^~~~ include/linux/mmu_context.h:7:1: note: 'true' is defined in header '<stdbool.h>'; did you forget to '#include <stdbool.h>'? 6 | #include <asm/mmu.h> +++ |+#include <stdbool.h> 7 | include/linux/mmu_context.h:41:16: note: each undeclared identifier is reported only once for each function it appears in 41 | return true; | ^~~~ drivers/iommu/iommu-sva.c: In function 'iommu_sva_bind_device': drivers/iommu/iommu-sva.c:69:32: error: invalid use of undefined type 'struct dev_iommu' 69 | max_pasids = dev->iommu->max_pasids; | ^~ drivers/iommu/iommu-sva.c:78:32: error: invalid application of 'sizeof' to incomplete type 'struct iommu_sva' 78 | handle = kzalloc(sizeof(*handle), GFP_KERNEL); | ^ drivers/iommu/iommu-sva.c:109:15: error: invalid use of undefined type 'struct iommu_sva' 109 | handle->dev = dev; | ^~ drivers/iommu/iommu-sva.c:110:15: error: invalid use of undefined type 'struct iommu_sva' 110 | handle->domain = domain; | ^~ drivers/iommu/iommu-sva.c: In function 'iommu_sva_unbind_device': drivers/iommu/iommu-sva.c:134:45: error: invalid use of undefined type 'struct iommu_sva' 134 | struct iommu_domain *domain = handle->domain; | ^~ drivers/iommu/iommu-sva.c:136:36: error: invalid use of undefined type 'struct iommu_sva' 136 | struct device *dev = handle->dev; | ^~ drivers/iommu/iommu-sva.c: In function 'iommu_sva_get_pasid': drivers/iommu/iommu-sva.c:150:45: error: invalid use of undefined type 'struct iommu_sva' 150 | struct iommu_domain *domain = handle->domain; | ^~ include/linux/mmu_context.h: In function 'arch_pgtable_dma_compat': include/linux/mmu_context.h:42:1: error: control reaches end of non-void function [-Werror=return-type] 42 | } | ^ cc1: some warnings being treated as errors vim +13 arch/sparc/include/asm/mmu_context_32.h f5e706ad886b6a include/asm-sparc/mmu_context_32.h Sam Ravnborg 2008-07-17 8 b585e8551b352c arch/sparc/include/asm/mmu_context_32.h Sam Ravnborg 2012-07-26 9 /* Initialize a new mmu context. This is invoked when a new f5e706ad886b6a include/asm-sparc/mmu_context_32.h Sam Ravnborg 2008-07-17 10 * address space instance (unique or shared) is instantiated. f5e706ad886b6a include/asm-sparc/mmu_context_32.h Sam Ravnborg 2008-07-17 11 */ ca0f34b575ade0 arch/sparc/include/asm/mmu_context_32.h Nicholas Piggin 2020-09-02 12 #define init_new_context init_new_context b585e8551b352c arch/sparc/include/asm/mmu_context_32.h Sam Ravnborg 2012-07-26 @13 int init_new_context(struct task_struct *tsk, struct mm_struct *mm); f5e706ad886b6a include/asm-sparc/mmu_context_32.h Sam Ravnborg 2008-07-17 14 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iommu: Add Kconfig help text for IOMMU_SVA 2023-05-06 13:31 [PATCH] iommu: Add Kconfig help text for IOMMU_SVA Jacob Pan ` (2 preceding siblings ...) 2023-05-06 15:39 ` kernel test robot @ 2023-05-06 15:43 ` Linus Torvalds 2023-05-06 22:07 ` Jacob Pan 3 siblings, 1 reply; 16+ messages in thread From: Linus Torvalds @ 2023-05-06 15:43 UTC (permalink / raw) To: Jacob Pan Cc: LKML, iommu, Jason Gunthorpe, Lu Baolu, Joerg Roedel, Jean-Philippe Brucker, Robin Murphy, Will Deacon, Raj Ashok, Tian, Kevin, Yi Liu, Yu, Fenghua On Sat, May 6, 2023 at 6:27 AM Jacob Pan <jacob.jun.pan@linux.intel.com> wrote: > > Shared Virtual Addressing (SVA) is not immediately intuitive to people > who work outside IOMMU subsystem, add some explanation to make it less > opaque. So the patch has at least two problems. The first is that you corrupted the SPDX line. But the second is that the change from - bool + bool "Shared Virtual Addressing" means that now Kconfig *asks* about this thing, which was never the intent. The other Kconfig options that need it all do a select IOMMU_SVA to get it picked. I suspect that's why it then causes errors - because now the test robot can enable the option even in situations where it makes no sense and doesn't work. So no, it's not that the option needs a help entry that can be queried at Kconfig time. It's that the option needs a name that makes sense and isn't some random jumble of letters that is only understandable to people who already know exactly what it is. Linus ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iommu: Add Kconfig help text for IOMMU_SVA 2023-05-06 15:43 ` Linus Torvalds @ 2023-05-06 22:07 ` Jacob Pan 2023-05-07 18:52 ` Linus Torvalds 0 siblings, 1 reply; 16+ messages in thread From: Jacob Pan @ 2023-05-06 22:07 UTC (permalink / raw) To: Linus Torvalds Cc: LKML, iommu, Jason Gunthorpe, Lu Baolu, Joerg Roedel, Jean-Philippe Brucker, Robin Murphy, Will Deacon, Raj Ashok, Tian, Kevin, Yi Liu, Yu, Fenghua, jacob.jun.pan Hi Linus, On Sat, 6 May 2023 08:43:08 -0700, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Sat, May 6, 2023 at 6:27 AM Jacob Pan <jacob.jun.pan@linux.intel.com> > wrote: > > > > Shared Virtual Addressing (SVA) is not immediately intuitive to people > > who work outside IOMMU subsystem, add some explanation to make it less > > opaque. > > So the patch has at least two problems. > > The first is that you corrupted the SPDX line. > Sorry, my mistake. Will not try sending patches before fully waking up. > But the second is that the change from > > - bool > + bool "Shared Virtual Addressing" > > means that now Kconfig *asks* about this thing, which was never the > intent. The other Kconfig options that need it all do a > > select IOMMU_SVA > > to get it picked. > > I suspect that's why it then causes errors - because now the test > robot can enable the option even in situations where it makes no sense > and doesn't work. > > So no, it's not that the option needs a help entry that can be queried > at Kconfig time. It's that the option needs a name that makes sense > and isn't some random jumble of letters that is only understandable to > people who already know exactly what it is. Right, how about IOMMU_SHARING_CPU_PGTABLE? Since this is more of a one-sided relationship where IOMMU walks CPU page tables. Thanks, Jacob ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iommu: Add Kconfig help text for IOMMU_SVA 2023-05-06 22:07 ` Jacob Pan @ 2023-05-07 18:52 ` Linus Torvalds 2023-05-08 16:40 ` Jacob Pan 0 siblings, 1 reply; 16+ messages in thread From: Linus Torvalds @ 2023-05-07 18:52 UTC (permalink / raw) To: Jacob Pan Cc: LKML, iommu, Jason Gunthorpe, Lu Baolu, Joerg Roedel, Jean-Philippe Brucker, Robin Murphy, Will Deacon, Raj Ashok, Tian, Kevin, Yi Liu, Yu, Fenghua On Sat, May 6, 2023 at 3:03 PM Jacob Pan <jacob.jun.pan@linux.intel.com> wrote: > > Right, how about IOMMU_SHARING_CPU_PGTABLE? I think from a VM / process angle, I'd actually prefer calling the "pasid" part just that: IOMMU_PASID. The VM code certainly understands about address space IDs, even if people have called them different things: normal people called them ASID's long ago, then Intel at some pointed decided that "PCID" made sense as a name (narrator: "no it didn't"), and then you got that combined "PASID" thing. Now, it may be that this then goes hand-in-hand with other IOMMU code that isn't *about* PASID itself, but that depends on PASID's being present, and so I'd just expect IOMMU_PASID to be one of those options that are selected by other options. So maybe there is some part of IOMMU_SVA that is not about PASID itself, but I really think that the PASID code itself should just have that CONFIG_PASID around it. End result: from a legibility standpoint, I think it could be as simple as having that config IOMMU_SVA option have a "select IOMMU_PASID". Then make the VM/process PASID code depend on that. Maybe the "struct device *" stuff makes more sense under CONFIG_IOMMU_SVA, ie things like iopf_queue_add_device() and friends. How does that sound? Maybe those two options then always end up going together, but even if that is the case, I think from a VM/process standpoint it makes a lot more sense to simply have a "PASID enabled" option. It's much more understandable in that context, while something like "IOMMU_SVA" really is just a random jumble of letters to a VM person. And while the individual words in IOMMU_SHARING_CPU_PGTABLE all make sense, it's not clear what the combination means, and why it should have anything to do with then having an address space identifier for it. Linus ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iommu: Add Kconfig help text for IOMMU_SVA 2023-05-07 18:52 ` Linus Torvalds @ 2023-05-08 16:40 ` Jacob Pan 2023-05-08 16:42 ` Linus Torvalds 0 siblings, 1 reply; 16+ messages in thread From: Jacob Pan @ 2023-05-08 16:40 UTC (permalink / raw) To: Linus Torvalds Cc: LKML, iommu, Jason Gunthorpe, Lu Baolu, Joerg Roedel, Jean-Philippe Brucker, Robin Murphy, Will Deacon, Raj Ashok, Tian, Kevin, Yi Liu, Yu, Fenghua, jacob.jun.pan, Hansen, Dave Hi Linus, On Sun, 7 May 2023 11:52:18 -0700, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Sat, May 6, 2023 at 3:03 PM Jacob Pan <jacob.jun.pan@linux.intel.com> > wrote: > > > > Right, how about IOMMU_SHARING_CPU_PGTABLE? > > I think from a VM / process angle, I'd actually prefer calling the > "pasid" part just that: IOMMU_PASID. > > The VM code certainly understands about address space IDs, even if > people have called them different things: normal people called them > ASID's long ago, then Intel at some pointed decided that "PCID" made > sense as a name (narrator: "no it didn't"), and then you got that > combined "PASID" thing. > > Now, it may be that this then goes hand-in-hand with other IOMMU code > that isn't *about* PASID itself, but that depends on PASID's being > present, and so I'd just expect IOMMU_PASID to be one of those options > that are selected by other options. > > So maybe there is some part of IOMMU_SVA that is not about PASID > itself, but I really think that the PASID code itself should just have > that CONFIG_PASID around it. Conversely, we could also have some part of PASID that is not about SVA. e.g. Today, on PASID enabled IOMMUs, DMA request w/o PASID (legacy) uses a special PASID 0. This has nothing to do with mm->pasid. > End result: from a legibility standpoint, I think it could be as > simple as having that > > config IOMMU_SVA > > option have a "select IOMMU_PASID". > Then make the VM/process PASID code depend on that. Maybe the "struct > device *" stuff makes more sense under CONFIG_IOMMU_SVA, ie things > like iopf_queue_add_device() and friends. right, we don;t support non-PASID IOPF. > How does that sound? Maybe those two options then always end up going > together, but even if that is the case, I think from a VM/process > standpoint it makes a lot more sense to simply have a "PASID enabled" > option. It's much more understandable in that context, while something > like "IOMMU_SVA" really is just a random jumble of letters to a VM > person. My only concern is the case above where DMA API uses a PASID for legacy DMA requests w/o PASID. I am also trying to add non-zero PASID for Intel's ENQCMDS. https://lore.kernel.org/linux-iommu/20230427174937.471668-1-jacob.jun.pan@linux.intel.com/ The PASIDs used in this case uses IOVA page tables, not shared with any mm_struct. From this use case, we need to select IOMMU_PASID, but not necessarily for mm->pasid which IMHO is only meaningful when IOMMU shares page tables with the CPU. > And while the individual words in IOMMU_SHARING_CPU_PGTABLE all make > sense, it's not clear what the combination means, and why it should > have anything to do with then having an address space identifier for > it. > > Linus > Thanks, Jacob ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iommu: Add Kconfig help text for IOMMU_SVA 2023-05-08 16:40 ` Jacob Pan @ 2023-05-08 16:42 ` Linus Torvalds 2023-05-08 16:55 ` Jason Gunthorpe 0 siblings, 1 reply; 16+ messages in thread From: Linus Torvalds @ 2023-05-08 16:42 UTC (permalink / raw) To: Jacob Pan Cc: LKML, iommu, Jason Gunthorpe, Lu Baolu, Joerg Roedel, Jean-Philippe Brucker, Robin Murphy, Will Deacon, Raj Ashok, Tian, Kevin, Yi Liu, Yu, Fenghua, Hansen, Dave On Mon, May 8, 2023 at 9:35 AM Jacob Pan <jacob.jun.pan@linux.intel.com> wrote: > > Conversely, we could also have some part of PASID that is not about SVA. If that's the case, then we should *definitely* have that IOMMU_PASID kind of config option. Then IOMMU_SVA - that needs PASID - can "select" it, but any other use - that doesn't bother about SVA but might want PASID for its own nefarious purposes - can also select it. This is why we have config options. Not just for legibility, but also for "there are different scenarios". Linus ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iommu: Add Kconfig help text for IOMMU_SVA 2023-05-08 16:42 ` Linus Torvalds @ 2023-05-08 16:55 ` Jason Gunthorpe 2023-05-08 17:17 ` Linus Torvalds 0 siblings, 1 reply; 16+ messages in thread From: Jason Gunthorpe @ 2023-05-08 16:55 UTC (permalink / raw) To: Linus Torvalds Cc: Jacob Pan, LKML, iommu, Lu Baolu, Joerg Roedel, Jean-Philippe Brucker, Robin Murphy, Will Deacon, Raj Ashok, Tian, Kevin, Yi Liu, Yu, Fenghua, Hansen, Dave On Mon, May 08, 2023 at 09:42:18AM -0700, Linus Torvalds wrote: > On Mon, May 8, 2023 at 9:35 AM Jacob Pan <jacob.jun.pan@linux.intel.com> wrote: > > > > Conversely, we could also have some part of PASID that is not about SVA. > > If that's the case, then we should *definitely* have that IOMMU_PASID > kind of config option. > > Then IOMMU_SVA - that needs PASID - can "select" it, but any other use > - that doesn't bother about SVA but might want PASID for its own > nefarious purposes - can also select it. Yes, this has been unwinding slowly, but this mm part is about SVA, and specifically ENQCMD, and not about PASID, other than SVA and ENCQCMD are both using PASID. CONFIG_IOMMU_MM_PASID perhaps? Says what it does without a clue about why it does it. x86 arch code could select it ideally? Jason ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iommu: Add Kconfig help text for IOMMU_SVA 2023-05-08 16:55 ` Jason Gunthorpe @ 2023-05-08 17:17 ` Linus Torvalds 2023-05-08 20:21 ` Jacob Pan ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Linus Torvalds @ 2023-05-08 17:17 UTC (permalink / raw) To: Jason Gunthorpe Cc: Jacob Pan, LKML, iommu, Lu Baolu, Joerg Roedel, Jean-Philippe Brucker, Robin Murphy, Will Deacon, Raj Ashok, Tian, Kevin, Yi Liu, Yu, Fenghua, Hansen, Dave On Mon, May 8, 2023 at 9:55 AM Jason Gunthorpe <jgg@nvidia.com> wrote: > > CONFIG_IOMMU_MM_PASID perhaps? Says what it does without a clue about > why it does it. x86 arch code could select it ideally? Maybe we don't even need the "IOMMU" part. It's a core x86 architecture feature. Maybe it usually (always?) gets used within the framework of some IOMMU work, but I guess ENCQCMD could be used in some hardwired way even without that (ie is it possible to have just some "basic PASID set up by VMM environment" thing?) I don't actually know who uses it and how, so I'm probably not the right person to name it, but just looking at the x86 code that sets it, the PASID code technically has no connection to any iommu code, it's literally a core CPU feature with an MSR and some magic faulting thing, and seems to be possibly usable as-is. That existing #ifdef CONFIG_IOMMU_SVA in the x86 traps.c code just looks odd, and making it be CONFIG_IOMMU_MM_PASID sounds better to me. I'm just not sure about the "IOMMU" part either. Just "MM_PASID"? That said, the arm-smmu-v3-sva.c code clearly *also* uses pasid, except it seems to really want to call it "ssid". So "having a per-mm ASID for IO" is clearly a common feature. But naming seems hard, with x86 calling it PASID, arm64 seemingly calling it "SSID". Right now the only user *does* seem to be through the IOMMU SVA code, but that may or may not be fundamental. Now, "SSID" is a completely horrible name, as I immediately realized when I tried to google for it. So arm64 is just wrong, and we're most definitely continuing to call it PASID. I'd lean towards just "CONFIG_MM_PASID" or something, but at some point this is bikeshedding, and I don't know about any possible non-iommu direct uses? Linus ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iommu: Add Kconfig help text for IOMMU_SVA 2023-05-08 17:17 ` Linus Torvalds @ 2023-05-08 20:21 ` Jacob Pan 2023-05-09 0:13 ` Tian, Kevin 2023-05-09 1:55 ` Baolu Lu 2023-05-09 0:10 ` Tian, Kevin 2023-05-09 23:06 ` Jason Gunthorpe 2 siblings, 2 replies; 16+ messages in thread From: Jacob Pan @ 2023-05-08 20:21 UTC (permalink / raw) To: Linus Torvalds Cc: Jason Gunthorpe, LKML, iommu, Lu Baolu, Joerg Roedel, Jean-Philippe Brucker, Robin Murphy, Will Deacon, Raj Ashok, Tian, Kevin, Yi Liu, Yu, Fenghua, Hansen, Dave, jacob.jun.pan, Jean-Philippe Brucker Hi Linus, On Mon, 8 May 2023 10:17:53 -0700, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Mon, May 8, 2023 at 9:55 AM Jason Gunthorpe <jgg@nvidia.com> wrote: > > > > CONFIG_IOMMU_MM_PASID perhaps? Says what it does without a clue about > > why it does it. x86 arch code could select it ideally? > > Maybe we don't even need the "IOMMU" part. It's a core x86 > architecture feature. Maybe it usually (always?) gets used within the > framework of some IOMMU work, but I guess ENCQCMD could be used in > some hardwired way even without that (ie is it possible to have just > some "basic PASID set up by VMM environment" thing?) > > I don't actually know who uses it and how, so I'm probably not the > right person to name it, but just looking at the x86 code that sets > it, the PASID code technically has no connection to any iommu code, > it's literally a core CPU feature with an MSR and some magic faulting > thing, and seems to be possibly usable as-is. > > That existing > > #ifdef CONFIG_IOMMU_SVA > > in the x86 traps.c code just looks odd, and making it be > CONFIG_IOMMU_MM_PASID sounds better to me. I'm just not sure about the > "IOMMU" part either. Just "MM_PASID"? > > That said, the arm-smmu-v3-sva.c code clearly *also* uses pasid, > except it seems to really want to call it "ssid". > > So "having a per-mm ASID for IO" is clearly a common feature. But > naming seems hard, with x86 calling it PASID, arm64 seemingly calling > it "SSID". > > Right now the only user *does* seem to be through the IOMMU SVA code, > but that may or may not be fundamental. > > Now, "SSID" is a completely horrible name, as I immediately realized > when I tried to google for it. So arm64 is just wrong, and we're most > definitely continuing to call it PASID. > > I'd lean towards just "CONFIG_MM_PASID" or something, but at some > point this is bikeshedding, and I don't know about any possible > non-iommu direct uses? +Jean who has mentioned potential use of PASID without IOMMU. But I don't think there is anything in the current kernel. Leave the name MM_PASID aside, I am trying to capture the discussion with a patch below. I am struggling to get a clean solution since SVA code is common as you said "having a per-mm ASID for IO". Having x86 Kconfig select MM_PASID would be redundant if it is already selected by IOMMU_SVA. Perhaps I am totally missing the point. diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c index 58b1f208eff5..d69acc69bbbb 100644 --- a/arch/x86/kernel/traps.c +++ b/arch/x86/kernel/traps.c @@ -652,7 +652,7 @@ static bool fixup_iopl_exception(struct pt_regs *regs) */ static bool try_fixup_enqcmd_gp(void) { -#ifdef CONFIG_IOMMU_SVA +#ifdef CONFIG_IOMMU_MM_PASID u32 pasid; /* diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index db98c3f86e8c..7106f3af74ee 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -153,9 +153,13 @@ config IOMMU_DMA select IRQ_MSI_IOMMU select NEED_SG_DMA_LENGTH +config IOMMU_MM_PASID + bool + # Shared Virtual Addressing config IOMMU_SVA bool + select IOMMU_MM_PASID config FSL_PAMU bool "Freescale IOMMU support" diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig index 2e56bd79f589..b4d7bd68a911 100644 --- a/drivers/iommu/intel/Kconfig +++ b/drivers/iommu/intel/Kconfig @@ -50,6 +50,7 @@ config INTEL_IOMMU_SVM depends on X86_64 select MMU_NOTIFIER select IOMMU_SVA + select IOMMU_MM_PASID help Shared Virtual Memory (SVM) provides a facility for devices to access DMA resources through process address space by diff --git a/include/linux/iommu.h b/include/linux/iommu.h index e8c9a7da1060..bdd7f4c1b9ad 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -1166,22 +1166,12 @@ static inline bool tegra_dev_iommu_get_stream_id(struct device *dev, u32 *stream } #ifdef CONFIG_IOMMU_SVA -static inline void mm_pasid_init(struct mm_struct *mm) -{ - mm->pasid = IOMMU_PASID_INVALID; -} -static inline bool mm_valid_pasid(struct mm_struct *mm) -{ - return mm->pasid != IOMMU_PASID_INVALID; -} -void mm_pasid_drop(struct mm_struct *mm); struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct *mm); void iommu_sva_unbind_device(struct iommu_sva *handle); u32 iommu_sva_get_pasid(struct iommu_sva *handle); #else -static inline struct iommu_sva * -iommu_sva_bind_device(struct device *dev, struct mm_struct *mm) +static inline struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct *mm) { return NULL; } @@ -1194,9 +1184,22 @@ static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle) { return IOMMU_PASID_INVALID; } +#endif /* CONFIG_IOMMU_SVA */ + +#ifdef CONFIG_IOMMU_MM_PASID +static inline void mm_pasid_init(struct mm_struct *mm) +{ + mm->pasid = IOMMU_PASID_INVALID; +} +static inline bool mm_valid_pasid(struct mm_struct *mm) +{ + return mm->pasid != IOMMU_PASID_INVALID; +} +void mm_pasid_drop(struct mm_struct *mm); +#else static inline void mm_pasid_init(struct mm_struct *mm) {} static inline bool mm_valid_pasid(struct mm_struct *mm) { return false; } static inline void mm_pasid_drop(struct mm_struct *mm) {} -#endif /* CONFIG_IOMMU_SVA */ +#endif /* CONFIG_IOMMU_MM_PASID */ #endif /* __LINUX_IOMMU_H */ diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h index 306a3d1a0fa6..70740a4ab58a 100644 --- a/include/linux/mm_types.h +++ b/include/linux/mm_types.h @@ -771,7 +771,7 @@ struct mm_struct { #endif struct work_struct async_put_work; -#ifdef CONFIG_IOMMU_SVA +#ifdef CONFIG_IOMMU_MM_PASID u32 pasid; #endif #ifdef CONFIG_KSM diff --git a/include/linux/sched.h b/include/linux/sched.h index eed5d65b8d1f..0b6498eafb0c 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -945,7 +945,7 @@ struct task_struct { /* Recursion prevention for eventfd_signal() */ unsigned in_eventfd:1; #endif -#ifdef CONFIG_IOMMU_SVA +#ifdef CONFIG_IOMMU_MM_PASID unsigned pasid_activated:1; #endif #ifdef CONFIG_CPU_SUP_INTEL diff --git a/kernel/fork.c b/kernel/fork.c index ed4e01daccaa..cb02ddadd6fb 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1177,7 +1177,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node) tsk->use_memdelay = 0; #endif -#ifdef CONFIG_IOMMU_SVA +#ifdef CONFIG_IOMMU_MM_PASID tsk->pasid_activated = 0; #endif diff --git a/mm/init-mm.c b/mm/init-mm.c index efa97b57acfd..b97414c2b2f7 100644 --- a/mm/init-mm.c +++ b/mm/init-mm.c @@ -42,7 +42,7 @@ struct mm_struct init_mm = { #endif .user_ns = &init_user_ns, .cpu_bitmap = CPU_BITS_NONE, -#ifdef CONFIG_IOMMU_SVA +#ifdef CONFIG_IOMMU_MM_PASID .pasid = IOMMU_PASID_INVALID, #endif INIT_MM_CONTEXT(init_mm) Thanks, Jacob ^ permalink raw reply related [flat|nested] 16+ messages in thread
* RE: [PATCH] iommu: Add Kconfig help text for IOMMU_SVA 2023-05-08 20:21 ` Jacob Pan @ 2023-05-09 0:13 ` Tian, Kevin 2023-05-09 1:55 ` Baolu Lu 1 sibling, 0 replies; 16+ messages in thread From: Tian, Kevin @ 2023-05-09 0:13 UTC (permalink / raw) To: Jacob Pan, Torvalds, Linus Cc: Jason Gunthorpe, LKML, iommu@lists.linux.dev, Lu Baolu, Joerg Roedel, Jean-Philippe Brucker, Robin Murphy, Will Deacon, Raj, Ashok, Liu, Yi L, Yu, Fenghua, Hansen, Dave, Jean-Philippe Brucker > From: Jacob Pan <jacob.jun.pan@linux.intel.com> > Sent: Tuesday, May 9, 2023 4:22 AM > > Leave the name MM_PASID aside, I am trying to capture the discussion with > a > patch below. I am struggling to get a clean solution since SVA code is > common as you said "having a per-mm ASID for IO". Having x86 Kconfig > select > MM_PASID would be redundant if it is already selected by IOMMU_SVA. Currently SVA relies on mm->pasid, w/ or w/o ENQCMD. From this angle having IOMMU_SVA select MM_PASID looks sufficient to me. If one day there is a reason to constrain mm->pasid only for ENQCMD, then we can move to let arch config select MM_PASID. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iommu: Add Kconfig help text for IOMMU_SVA 2023-05-08 20:21 ` Jacob Pan 2023-05-09 0:13 ` Tian, Kevin @ 2023-05-09 1:55 ` Baolu Lu 1 sibling, 0 replies; 16+ messages in thread From: Baolu Lu @ 2023-05-09 1:55 UTC (permalink / raw) To: Jacob Pan, Linus Torvalds Cc: baolu.lu, Jason Gunthorpe, LKML, iommu, Joerg Roedel, Jean-Philippe Brucker, Robin Murphy, Will Deacon, Raj Ashok, Tian, Kevin, Yi Liu, Yu, Fenghua, Hansen, Dave, Jean-Philippe Brucker On 5/9/23 4:21 AM, Jacob Pan wrote: > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > index db98c3f86e8c..7106f3af74ee 100644 > --- a/drivers/iommu/Kconfig > +++ b/drivers/iommu/Kconfig > @@ -153,9 +153,13 @@ config IOMMU_DMA > select IRQ_MSI_IOMMU > select NEED_SG_DMA_LENGTH > > +config IOMMU_MM_PASID > + bool > + > # Shared Virtual Addressing > config IOMMU_SVA > bool > + select IOMMU_MM_PASID > > config FSL_PAMU > bool "Freescale IOMMU support" > diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig > index 2e56bd79f589..b4d7bd68a911 100644 > --- a/drivers/iommu/intel/Kconfig > +++ b/drivers/iommu/intel/Kconfig > @@ -50,6 +50,7 @@ config INTEL_IOMMU_SVM > depends on X86_64 > select MMU_NOTIFIER > select IOMMU_SVA > + select IOMMU_MM_PASID IOMMU_SVA has already selected IOMMU_MM_PASID, so there's no need to select it again here? IOMMU_SVA selects IOMMU_MM_PASID, the vt-d driver itself has no need to know about this. > help > Shared Virtual Memory (SVM) provides a facility for devices > to access DMA resources through process address space by Best regards, baolu ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH] iommu: Add Kconfig help text for IOMMU_SVA 2023-05-08 17:17 ` Linus Torvalds 2023-05-08 20:21 ` Jacob Pan @ 2023-05-09 0:10 ` Tian, Kevin 2023-05-09 23:06 ` Jason Gunthorpe 2 siblings, 0 replies; 16+ messages in thread From: Tian, Kevin @ 2023-05-09 0:10 UTC (permalink / raw) To: Torvalds, Linus, Jason Gunthorpe Cc: Jacob Pan, LKML, iommu@lists.linux.dev, Lu Baolu, Joerg Roedel, Jean-Philippe Brucker, Robin Murphy, Will Deacon, Raj, Ashok, Liu, Yi L, Yu, Fenghua, Hansen, Dave > From: Linus Torvalds <torvalds@linux-foundation.org> > Sent: Tuesday, May 9, 2023 1:18 AM > > On Mon, May 8, 2023 at 9:55 AM Jason Gunthorpe <jgg@nvidia.com> wrote: > > > > CONFIG_IOMMU_MM_PASID perhaps? Says what it does without a clue > about > > why it does it. x86 arch code could select it ideally? > > Maybe we don't even need the "IOMMU" part. It's a core x86 > architecture feature. Maybe it usually (always?) gets used within the > framework of some IOMMU work, but I guess ENCQCMD could be used in > some hardwired way even without that (ie is it possible to have just > some "basic PASID set up by VMM environment" thing?) This makes sense to me. At least that is one possible way in concept, though we don't have such example now. > > I don't actually know who uses it and how, so I'm probably not the > right person to name it, but just looking at the x86 code that sets > it, the PASID code technically has no connection to any iommu code, > it's literally a core CPU feature with an MSR and some magic faulting > thing, and seems to be possibly usable as-is. > > That existing > > #ifdef CONFIG_IOMMU_SVA > > in the x86 traps.c code just looks odd, and making it be > CONFIG_IOMMU_MM_PASID sounds better to me. I'm just not sure about > the > "IOMMU" part either. Just "MM_PASID"? > > That said, the arm-smmu-v3-sva.c code clearly *also* uses pasid, > except it seems to really want to call it "ssid". > > So "having a per-mm ASID for IO" is clearly a common feature. But Yes. Currently PASID for mm is globally allocated so each mm is associated with a single unique pasid. > naming seems hard, with x86 calling it PASID, arm64 seemingly calling > it "SSID". > > Right now the only user *does* seem to be through the IOMMU SVA code, > but that may or may not be fundamental. > > Now, "SSID" is a completely horrible name, as I immediately realized > when I tried to google for it. So arm64 is just wrong, and we're most > definitely continuing to call it PASID. We have undergone this naming game when enabling SVA before. At that point IOASID was considered as a generic term to cover both PCI PASID and ARM specific SSID. But in the end PASID/IOASID are still mixed in many common code. IMHO sticking with PASID should be fine. > > I'd lean towards just "CONFIG_MM_PASID" or something, but at some > point this is bikeshedding, and I don't know about any possible > non-iommu direct uses? > I agree that CONFIG_MM_PASID is more accurate in definition. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iommu: Add Kconfig help text for IOMMU_SVA 2023-05-08 17:17 ` Linus Torvalds 2023-05-08 20:21 ` Jacob Pan 2023-05-09 0:10 ` Tian, Kevin @ 2023-05-09 23:06 ` Jason Gunthorpe 2 siblings, 0 replies; 16+ messages in thread From: Jason Gunthorpe @ 2023-05-09 23:06 UTC (permalink / raw) To: Linus Torvalds Cc: Jacob Pan, LKML, iommu, Lu Baolu, Joerg Roedel, Jean-Philippe Brucker, Robin Murphy, Will Deacon, Raj Ashok, Tian, Kevin, Yi Liu, Yu, Fenghua, Hansen, Dave On Mon, May 08, 2023 at 10:17:53AM -0700, Linus Torvalds wrote: > That said, the arm-smmu-v3-sva.c code clearly *also* uses pasid, > except it seems to really want to call it "ssid". This code is incorrect.. It is using the mm->pasid to try to deliver an invalidation, but our new design does not restrict the userspace to a single PASID, thus it will fail to properly synchronize the ATC in some degenerate cases. In general the driver needs to be able to go over a list of PASIDs that the domains are linked to and replicate invalidations (of any sort) to the whole list. It is still a WIP to make this driver implement the driver contract for PASID logic properly. > Now, "SSID" is a completely horrible name, as I immediately realized > when I tried to google for it. So arm64 is just wrong, and we're most > definitely continuing to call it PASID. Yes, PASID comes from PCIe so we are using that. > I'd lean towards just "CONFIG_MM_PASID" or something, but at some > point this is bikeshedding, and I don't know about any possible > non-iommu direct uses? I don't think there is any other use, IOMMU should be the only consumer of PASID, so it is a fine name as well. Thanks, Jason ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-05-09 23:06 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-05-06 13:31 [PATCH] iommu: Add Kconfig help text for IOMMU_SVA Jacob Pan 2023-05-06 15:19 ` kernel test robot 2023-05-06 15:39 ` kernel test robot 2023-05-06 15:39 ` kernel test robot 2023-05-06 15:43 ` Linus Torvalds 2023-05-06 22:07 ` Jacob Pan 2023-05-07 18:52 ` Linus Torvalds 2023-05-08 16:40 ` Jacob Pan 2023-05-08 16:42 ` Linus Torvalds 2023-05-08 16:55 ` Jason Gunthorpe 2023-05-08 17:17 ` Linus Torvalds 2023-05-08 20:21 ` Jacob Pan 2023-05-09 0:13 ` Tian, Kevin 2023-05-09 1:55 ` Baolu Lu 2023-05-09 0:10 ` Tian, Kevin 2023-05-09 23:06 ` Jason Gunthorpe
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox