From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:20:48
Hi all,
First off, yes, this conflicts with just about everything else
currently in-flight. Sorry about that. If it stands up to initial review
then I'll start giving some thought to how to fit everything together
(particularly John's cleanup of strictness defaults, which I'd be
inclined to fold into a v2 of this series).
Anyway, this is my take on promoting the strict vs. non-strict DMA
domain choice to distinct domain types, so that it can fit logically
into the existing sysfs and Kconfig controls. The first 13 patches are
effectively preparatory cleanup to reduce churn in the later changes,
but could be merged in their own right even if the rest is too
contentious. I ended up splitting patches #2-#11 by driver for ease of
review, since some of them are more than just trivial deletions, but
they could readily be squashed (even as far as with #1 and #12 too).
I'm slightly surprised at how straightforward it's turned out, but it
has survived some very basic smoke testing for arm-smmu using dmatest
on my Arm Juno board. Branch here for convenience:
https://gitlab.arm.com/linux-arm/linux-rm/-/tree/iommu/fq
Please let me know what you think!
Robin.
Robin Murphy (23):
iommu: Pull IOVA cookie management into the core
iommu/amd: Drop IOVA cookie management
iommu/arm-smmu: Drop IOVA cookie management
iommu/vt-d: Drop IOVA cookie management
iommu/exynos: Drop IOVA cookie management
iommu/ipmmu-vmsa: Drop IOVA cookie management
iommu/mtk: Drop IOVA cookie management
iommu/rockchip: Drop IOVA cookie management
iommu/sprd: Drop IOVA cookie management
iommu/sun50i: Drop IOVA cookie management
iommu/virtio: Drop IOVA cookie management
iommu/dma: Unexport IOVA cookie management
iommu/dma: Remove redundant "!dev" checks
iommu: Introduce explicit type for non-strict DMA domains
iommu/amd: Prepare for multiple DMA domain types
iommu/arm-smmu: Prepare for multiple DMA domain types
iommu/vt-d: Prepare for multiple DMA domain types
iommu: Express DMA strictness via the domain type
iommu: Expose DMA domain strictness via sysfs
iommu: Allow choosing DMA strictness at build time
iommu/dma: Factor out flush queue init
iommu: Allow enabling non-strict mode dynamically
iommu/arm-smmu: Allow non-strict in pgtable_quirks interface
.../ABI/testing/sysfs-kernel-iommu_groups | 2 +
drivers/iommu/Kconfig | 48 +++++++++++++++----
drivers/iommu/amd/iommu.c | 21 +-------
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 25 ++++++----
drivers/iommu/arm/arm-smmu/arm-smmu.c | 24 ++++++----
drivers/iommu/arm/arm-smmu/qcom_iommu.c | 8 ----
drivers/iommu/dma-iommu.c | 44 +++++++++--------
drivers/iommu/exynos-iommu.c | 18 ++-----
drivers/iommu/intel/iommu.c | 23 +++------
drivers/iommu/iommu.c | 44 +++++++++++------
drivers/iommu/ipmmu-vmsa.c | 27 ++---------
drivers/iommu/mtk_iommu.c | 6 ---
drivers/iommu/rockchip-iommu.c | 11 +----
drivers/iommu/sprd-iommu.c | 6 ---
drivers/iommu/sun50i-iommu.c | 12 +----
drivers/iommu/virtio-iommu.c | 8 ----
include/linux/dma-iommu.h | 9 ++--
include/linux/iommu.h | 10 +++-
18 files changed, 160 insertions(+), 186 deletions(-)
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:20:51
Now that everyone has converged on iommu-dma for IOMMU_DOMAIN_DMA
support, we can abandon the notion of drivers being responsible for the
cookie type, and consolidate all the management into the core code.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/iommu.c | 7 +++++++
include/linux/iommu.h | 3 ++-
2 files changed, 9 insertions(+), 1 deletion(-)
@@ -1941,6 +1942,11 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,/* Assume all sizes by default; the driver may override this later */domain->pgsize_bitmap=bus->iommu_ops->pgsize_bitmap;+/* Temporarily ignore -EEXIST while drivers still get their own cookies */+if(type==IOMMU_DOMAIN_DMA&&iommu_get_dma_cookie(domain)==-ENOMEM){+iommu_domain_free(domain);+domain=NULL;+}returndomain;}
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:20:55
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/amd/iommu.c | 12 ------------
1 file changed, 12 deletions(-)
@@ -2021,7 +2015,6 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)structarm_smmu_domain*smmu_domain=to_smmu_domain(domain);structarm_smmu_device*smmu=smmu_domain->smmu;-iommu_put_dma_cookie(domain);free_io_pgtable_ops(smmu_domain->pgtbl_ops);/* Free the CD and ASID, if we allocated them */
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:20:59
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/intel/iommu.c | 8 --------
1 file changed, 8 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:21:04
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/exynos-iommu.c | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:21:07
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/ipmmu-vmsa.c | 27 ++++-----------------------
1 file changed, 4 insertions(+), 23 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:21:09
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/mtk_iommu.c | 6 ------
1 file changed, 6 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:21:11
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/rockchip-iommu.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:21:12
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/sprd-iommu.c | 6 ------
1 file changed, 6 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:21:15
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/sun50i-iommu.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:21:19
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/virtio-iommu.c | 8 --------
1 file changed, 8 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:21:31
IOVA cookies are now got and put by core code, so we no longer need to
export these to modular drivers. The export for getting MSI cookies
stays, since VFIO can still be a module, but it was already relying on
someone else putting them, so that aspect is unaffected.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/dma-iommu.c | 7 -------
drivers/iommu/iommu.c | 3 +--
2 files changed, 1 insertion(+), 9 deletions(-)
@@ -1942,8 +1942,7 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,/* Assume all sizes by default; the driver may override this later */domain->pgsize_bitmap=bus->iommu_ops->pgsize_bitmap;-/* Temporarily ignore -EEXIST while drivers still get their own cookies */-if(type==IOMMU_DOMAIN_DMA&&iommu_get_dma_cookie(domain)==-ENOMEM){+if(type==IOMMU_DOMAIN_DMA&&iommu_get_dma_cookie(domain)){iommu_domain_free(domain);domain=NULL;}
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:21:37
iommu_dma_init_domain() is now only called from iommu_setup_dma_ops(),
which has already assumed dev to be non-NULL.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/dma-iommu.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:21:39
Promote the difference between strict and non-strict DMA domains from an
internal detail to a distinct domain feature and type, to pave the road
for exposing it through the sysfs default domain interface.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/dma-iommu.c | 2 +-
drivers/iommu/iommu.c | 6 +++++-
include/linux/iommu.h | 6 ++++++
3 files changed, 12 insertions(+), 2 deletions(-)
@@ -1942,7 +1946,7 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,/* Assume all sizes by default; the driver may override this later */domain->pgsize_bitmap=bus->iommu_ops->pgsize_bitmap;-if(type==IOMMU_DOMAIN_DMA&&iommu_get_dma_cookie(domain)){+if((type&__IOMMU_DOMAIN_DMA_API)&&iommu_get_dma_cookie(domain)){iommu_domain_free(domain);domain=NULL;}
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:21:42
The DMA ops setup can simply be unconditional, since iommu-dma
already knows not to touch identity domains.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/amd/iommu.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
@@ -1707,14 +1707,9 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)staticvoidamd_iommu_probe_finalize(structdevice*dev){-structiommu_domain*domain;-/* Domains are initialized for this device - have a look what we ended up with */-domain=iommu_get_domain_for_dev(dev);-if(domain->type==IOMMU_DOMAIN_DMA)-iommu_setup_dma_ops(dev,0,U64_MAX);-else-set_dma_ops(dev,NULL);+set_dma_ops(dev,NULL);+iommu_setup_dma_ops(dev,0,U64_MAX);}staticvoidamd_iommu_release_device(structdevice*dev)
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:21:44
In preparation for the strict vs. non-strict decision for DMA domains to
be expressed in the domain type, make sure we expose our flush queue
awareness by accepting the new domain type, and test the specific
feature flag where we want to identify DMA domains in general.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 +
drivers/iommu/arm/arm-smmu/arm-smmu.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:21:49
In preparation for the strict vs. non-strict decision for DMA domains to
be expressed in the domain type, make sure we expose our flush queue
awareness by accepting the new domain type, and test the specific
feature flag where we want to identify DMA domains in general. The DMA
ops setup can simply be made unconditional, since iommu-dma already
knows not to touch identity domains.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/intel/iommu.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
@@ -601,7 +601,7 @@ struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)intiommu_id;/* si_domain and vm domain should not get here. */-if(WARN_ON(domain->domain.type!=IOMMU_DOMAIN_DMA))+if(WARN_ON(!(domain->domain.type&__IOMMU_DOMAIN_DMA)))returnNULL;for_each_domain_iommu(iommu_id,domain)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:21:50
Eliminate the iommu_get_dma_strict() indirection and pipe the
information through the domain type from the beginning. Besides
the flow simplification this also has several nice side-effects:
- Automatically implies strict mode for untrusted devices by
virtue of their IOMMU_DOMAIN_DMA override.
- Ensures that we only ends up using flush queues for drivers
which are aware of them and can actually benefit.
- Allows us to handle flush queue init failure by falling back
to strict mode instead of leaving it to possibly blow up later.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
drivers/iommu/arm/arm-smmu/arm-smmu.c | 2 +-
drivers/iommu/dma-iommu.c | 10 ++++++----
drivers/iommu/iommu.c | 14 ++++----------
include/linux/iommu.h | 1 -
5 files changed, 12 insertions(+), 17 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:21:53
The sysfs interface for default domain types exists primarily so users
can choose the performance/security tradeoff relevant to their own
workload. As such, the choice between the policies for DMA domains fits
perfectly as an additional point on that scale - downgrading a
particular device from a strict default to non-strict may be enough to
let it reach the desired level of performance, while still retaining
more peace of mind than with a wide-open identity domain. Now that we've
abstracted non-strict mode as a distinct type of DMA domain, allow it to
be chosen through the user interface as well.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
Documentation/ABI/testing/sysfs-kernel-iommu_groups | 2 ++
drivers/iommu/iommu.c | 2 ++
2 files changed, 4 insertions(+)
@@ -42,6 +42,8 @@ Description: /sys/kernel/iommu_groups/<grp_id>/type shows the type of default ======== ====================================================== DMA All the DMA transactions from the device in this group are translated by the iommu.+ DMA-FQ As above, but using batched invalidation to lazily+ remove translations after use. identity All the DMA transactions from the device in this group are not translated by the iommu. auto Change to the type the device was booted with.
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:21:54
To parallel the sysfs behaviour, extend the build-time configuration
for default domains to include the new type as well.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
This effectively replaces patch #3 of John's "iommu: Enhance IOMMU
default DMA mode build options" series.
---
drivers/iommu/Kconfig | 48 +++++++++++++++++++++++++++++++++++--------
drivers/iommu/iommu.c | 2 +-
2 files changed, 41 insertions(+), 9 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:21:59
Allocating and enabling a flush queue is in fact something we can
reasonably do while a DMA domain is active, without having to rebuild it
from scratch. Thus we can allow a strict -> non-strict transition from
sysfs without requiring to unbind the device's driver, which is of
particular interest to users who want to make selective relaxations to
critical devices like the one serving their root filesystem.
Disabling and draining a queue also seems technically possible to
achieve without rebuilding the whole domain, but would certainly be more
involved. Furthermore there's not such a clear use-case for tightening
up security *after* the device may already have done whatever it is that
you don't trust it not to do, so we only consider the relaxation case.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/iommu.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
@@ -3130,6 +3130,13 @@ static int iommu_change_dev_def_domain(struct iommu_group *group,gotoout;}+/* We can bring up a flush queue without tearing down the domain */+if(type==IOMMU_DOMAIN_DMA_FQ&&prev_dom->type==IOMMU_DOMAIN_DMA){+prev_dom->type=IOMMU_DOMAIN_DMA_FQ;+ret=iommu_dma_init_fq(prev_dom);+gotoout;+}+/* Sets group->default_domain to the newly allocated domain */ret=iommu_group_alloc_default_domain(dev->bus,group,type);if(ret)
@@ -3170,9 +3177,9 @@ static int iommu_change_dev_def_domain(struct iommu_group *group,}/*-*Changingthedefaultdomainthroughsysfsrequirestheuserstoubindthe-*driversfromthedevicesintheiommugroup.Returnfailureifthisdoesn't-*meet.+*Changingthedefaultdomainthroughsysfsrequirestheuserstounbindthe+*driversfromthedevicesintheiommugroup,exceptforaDMA->DMA-FQ+*transition.Returnfailureifthisisn'tmet.**Weneedtoconsidertheracebetweenthisandthedevicereleasepath.*device_lock(dev)isusedheretoguaranteethatthedevicereleasepath
@@ -3248,7 +3255,8 @@ static ssize_t iommu_group_store_type(struct iommu_group *group,/* Check if the device in the group still has a driver bound to it */device_lock(dev);-if(device_is_bound(dev)){+if(device_is_bound(dev)&&!(req_type==IOMMU_DOMAIN_DMA_FQ&&+group->default_domain->type==IOMMU_DOMAIN_DMA)){pr_err_ratelimited("Device is still bound to driver\n");ret=-EBUSY;gotoout;
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:22:01
Factor out flush queue setup from the initial domain init so that we
can potentially trigger it from sysfs later on in a domain's lifetime.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/dma-iommu.c | 31 ++++++++++++++++++++-----------
include/linux/dma-iommu.h | 9 ++++++---
2 files changed, 26 insertions(+), 14 deletions(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-21 18:22:03
To make io-pgtable aware of a flush queue being dynamically enabled,
allow IO_PGTABLE_QUIRK_NON_STRICT to be set even after a domain has been
attached to, and hook up the final piece of the puzzle in iommu-dma.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 15 +++++++++++++++
drivers/iommu/arm/arm-smmu/arm-smmu.c | 11 +++++++++++
drivers/iommu/dma-iommu.c | 3 +++
3 files changed, 29 insertions(+)
From: kernel test robot <hidden> Date: 2021-07-22 16:46:55
Hi Robin,
I love your patch! Yet something to improve:
[auto build test ERROR on iommu/next]
[also build test ERROR on rockchip/for-next linus/master v5.14-rc2 next-20210722]
[cannot apply to sunxi/sunxi/for-next]
[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/Robin-Murphy/iommu-Refactor-DMA-domain-strictness/20210722-022514
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 10.3.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/0day-ci/linux/commit/c05e0e1856b394eff1167c00f7bbd6ac7cc9dea6
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Robin-Murphy/iommu-Refactor-DMA-domain-strictness/20210722-022514
git checkout c05e0e1856b394eff1167c00f7bbd6ac7cc9dea6
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=ia64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>
All errors (new ones prefixed by >>):
In file included from arch/ia64/include/asm/bug.h:17,
from include/linux/bug.h:5,
from include/linux/thread_info.h:13,
from include/asm-generic/preempt.h:5,
from ./arch/ia64/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/wait.h:9,
from include/linux/wait_bit.h:8,
from include/linux/fs.h:6,
from include/linux/debugfs.h:15,
from drivers/iommu/intel/iommu.c:18:
drivers/iommu/intel/iommu.c: In function 'domain_get_iommu':
quoted
drivers/iommu/intel/iommu.c:604:38: error: '__IOMMU_DOMAIN_DMA' undeclared (first use in this function); did you mean 'IOMMU_DOMAIN_DMA'?
604 | if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
| ^~~~~~~~~~~~~~~~~~
include/asm-generic/bug.h:121:25: note: in definition of macro 'WARN_ON'
121 | int __ret_warn_on = !!(condition); \
| ^~~~~~~~~
drivers/iommu/intel/iommu.c:604:38: note: each undeclared identifier is reported only once for each function it appears in
604 | if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
| ^~~~~~~~~~~~~~~~~~
include/asm-generic/bug.h:121:25: note: in definition of macro 'WARN_ON'
121 | int __ret_warn_on = !!(condition); \
| ^~~~~~~~~
vim +604 drivers/iommu/intel/iommu.c
597
598 /* This functionin only returns single iommu in a domain */
599 struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)
600 {
601 int iommu_id;
602
603 /* si_domain and vm domain should not get here. */
> 604 if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
605 return NULL;
606
607 for_each_domain_iommu(iommu_id, domain)
608 break;
609
610 if (iommu_id < 0 || iommu_id >= g_num_of_iommus)
611 return NULL;
612
613 return g_iommus[iommu_id];
614 }
615
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-22 17:31:03
On 2021-07-22 17:44, kernel test robot wrote:
Hi Robin,
I love your patch! Yet something to improve:
[auto build test ERROR on iommu/next]
[also build test ERROR on rockchip/for-next linus/master v5.14-rc2 next-20210722]
[cannot apply to sunxi/sunxi/for-next]
[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/Robin-Murphy/iommu-Refactor-DMA-domain-strictness/20210722-022514
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 10.3.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/0day-ci/linux/commit/c05e0e1856b394eff1167c00f7bbd6ac7cc9dea6
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Robin-Murphy/iommu-Refactor-DMA-domain-strictness/20210722-022514
git checkout c05e0e1856b394eff1167c00f7bbd6ac7cc9dea6
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=ia64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>
All errors (new ones prefixed by >>):
In file included from arch/ia64/include/asm/bug.h:17,
from include/linux/bug.h:5,
from include/linux/thread_info.h:13,
from include/asm-generic/preempt.h:5,
from ./arch/ia64/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/wait.h:9,
from include/linux/wait_bit.h:8,
from include/linux/fs.h:6,
from include/linux/debugfs.h:15,
from drivers/iommu/intel/iommu.c:18:
drivers/iommu/intel/iommu.c: In function 'domain_get_iommu':
quoted
quoted
drivers/iommu/intel/iommu.c:604:38: error: '__IOMMU_DOMAIN_DMA' undeclared (first use in this function); did you mean 'IOMMU_DOMAIN_DMA'?
604 | if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
| ^~~~~~~~~~~~~~~~~~
include/asm-generic/bug.h:121:25: note: in definition of macro 'WARN_ON'
121 | int __ret_warn_on = !!(condition); \
| ^~~~~~~~~
drivers/iommu/intel/iommu.c:604:38: note: each undeclared identifier is reported only once for each function it appears in
604 | if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
| ^~~~~~~~~~~~~~~~~~
include/asm-generic/bug.h:121:25: note: in definition of macro 'WARN_ON'
121 | int __ret_warn_on = !!(condition); \
| ^~~~~~~~~
vim +604 drivers/iommu/intel/iommu.c
597
598 /* This functionin only returns single iommu in a domain */
599 struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)
600 {
601 int iommu_id;
602
603 /* si_domain and vm domain should not get here. */
> 604 if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
Bleh, of course that should be __IOMMU_DOMAIN_DMA_API like the other two
instances. I'll fix this locally ready for v2.
Thanks,
Robin.
From: Lu Baolu <baolu.lu@linux.intel.com> Date: 2021-07-24 05:23:40
Hi Robin,
On 2021/7/22 2:20, Robin Murphy wrote:
quoted hunk
In preparation for the strict vs. non-strict decision for DMA domains to
be expressed in the domain type, make sure we expose our flush queue
awareness by accepting the new domain type, and test the specific
feature flag where we want to identify DMA domains in general. The DMA
ops setup can simply be made unconditional, since iommu-dma already
knows not to touch identity domains.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/intel/iommu.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
@@ -601,7 +601,7 @@ struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)intiommu_id;/* si_domain and vm domain should not get here. */-if(WARN_ON(domain->domain.type!=IOMMU_DOMAIN_DMA))+if(WARN_ON(!(domain->domain.type&__IOMMU_DOMAIN_DMA)))returnNULL;for_each_domain_iommu(iommu_id,domain)
From: Lu Baolu <baolu.lu@linux.intel.com> Date: 2021-07-24 05:30:02
Hi Robin,
On 2021/7/22 2:20, Robin Murphy wrote:
Eliminate the iommu_get_dma_strict() indirection and pipe the
information through the domain type from the beginning. Besides
the flow simplification this also has several nice side-effects:
- Automatically implies strict mode for untrusted devices by
virtue of their IOMMU_DOMAIN_DMA override.
- Ensures that we only ends up using flush queues for drivers
which are aware of them and can actually benefit.
Is this expressed by vendor iommu driver has ops->flush_iotlb_all?
quoted hunk
- Allows us to handle flush queue init failure by falling back
to strict mode instead of leaving it to possibly blow up later.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
drivers/iommu/arm/arm-smmu/arm-smmu.c | 2 +-
drivers/iommu/dma-iommu.c | 10 ++++++----
drivers/iommu/iommu.c | 14 ++++----------
include/linux/iommu.h | 1 -
5 files changed, 12 insertions(+), 17 deletions(-)
From: John Garry <hidden> Date: 2021-07-26 08:13:40
On 21/07/2021 19:20, Robin Murphy wrote:
Hi all,
First off, yes, this conflicts with just about everything else
currently in-flight. Sorry about that. If it stands up to initial review
then I'll start giving some thought to how to fit everything together
(particularly John's cleanup of strictness defaults, which I'd be
inclined to fold into a v2 of this series).
It seems to me that patch #20 is the only real conflict, and that is
just a different form of mine in that passthrough, strict, and lazy are
under a single choice, as opposed to passthrough being a separate config
(for mine). And on that point, I did assume that we would have a
different sysfs file for strict vs lazy in this series, and not a new
domain type. But I assume that there is a good reason for that.
Anyway, I'd really like to see my series just merged now.
Thanks,
John
Anyway, this is my take on promoting the strict vs. non-strict DMA
domain choice to distinct domain types, so that it can fit logically
into the existing sysfs and Kconfig controls. The first 13 patches are
effectively preparatory cleanup to reduce churn in the later changes,
but could be merged in their own right even if the rest is too
contentious. I ended up splitting patches #2-#11 by driver for ease of
review, since some of them are more than just trivial deletions, but
they could readily be squashed (even as far as with #1 and #12 too).
I'm slightly surprised at how straightforward it's turned out, but it
has survived some very basic smoke testing for arm-smmu using dmatest
on my Arm Juno board. Branch here for convenience:
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-26 08:27:12
On 2021-07-24 06:29, Lu Baolu wrote:
Hi Robin,
On 2021/7/22 2:20, Robin Murphy wrote:
quoted
Eliminate the iommu_get_dma_strict() indirection and pipe the
information through the domain type from the beginning. Besides
the flow simplification this also has several nice side-effects:
- Automatically implies strict mode for untrusted devices by
virtue of their IOMMU_DOMAIN_DMA override.
- Ensures that we only ends up using flush queues for drivers
which are aware of them and can actually benefit.
Is this expressed by vendor iommu driver has ops->flush_iotlb_all?
No, it's literally whether ->domain_alloc accepts the DMA_DOMAIN_FQ type
or not.
quoted
- Allows us to handle flush queue init failure by falling back
to strict mode instead of leaving it to possibly blow up later.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
drivers/iommu/arm/arm-smmu/arm-smmu.c | 2 +-
drivers/iommu/dma-iommu.c | 10 ++++++----
drivers/iommu/iommu.c | 14 ++++----------
include/linux/iommu.h | 1 -
5 files changed, 12 insertions(+), 17 deletions(-)
iommu_create_device_direct_mappings(struct iommu_group *group,
unsigned long pg_size;
int ret = 0;
- if (!domain || domain->type != IOMMU_DOMAIN_DMA)
+ if (!domain || !(domain->type & __IOMMU_DOMAIN_DMA_API))
Nit: probably move above change to patch 14?
Indeed I'm not sure why this one ended up here, good catch!
Thanks,
Robin.
From: John Garry <hidden> Date: 2021-07-26 08:29:04
On 21/07/2021 19:20, Robin Murphy wrote:
iommu_dma_init_domain() is now only called from iommu_setup_dma_ops(),
which has already assumed dev to be non-NULL.
Signed-off-by: Robin Murphy<robin.murphy@arm.com>
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-26 08:30:21
On 2021-07-24 06:23, Lu Baolu wrote:
Hi Robin,
On 2021/7/22 2:20, Robin Murphy wrote:
quoted
In preparation for the strict vs. non-strict decision for DMA domains to
be expressed in the domain type, make sure we expose our flush queue
awareness by accepting the new domain type, and test the specific
feature flag where we want to identify DMA domains in general. The DMA
ops setup can simply be made unconditional, since iommu-dma already
knows not to touch identity domains.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/intel/iommu.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
dmar_domain *domain)
int iommu_id;
/* si_domain and vm domain should not get here. */
- if (WARN_ON(domain->domain.type != IOMMU_DOMAIN_DMA))
+ if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
return NULL;
for_each_domain_iommu(iommu_id, domain)
unsigned long iov_pfn,
if (domain_use_first_level(domain)) {
attr |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
- if (domain->domain.type == IOMMU_DOMAIN_DMA) {
+ if (domain->domain.type & __IOMMU_DOMAIN_DMA_API) {
attr |= DMA_FL_PTE_ACCESS;
if (prot & DMA_PTE_WRITE)
attr |= DMA_FL_PTE_DIRTY;
@@ -4528,6 +4528,7 @@ static struct iommu_domain
*intel_iommu_domain_alloc(unsigned type)
switch (type) {
case IOMMU_DOMAIN_DMA:
+ case IOMMU_DOMAIN_DMA_FQ:
case IOMMU_DOMAIN_UNMANAGED:
dmar_domain = alloc_domain(0);
if (!dmar_domain) {
From: Lu Baolu <baolu.lu@linux.intel.com> Date: 2021-07-26 11:31:39
On 2021/7/26 16:27, Robin Murphy wrote:
On 2021-07-24 06:29, Lu Baolu wrote:
quoted
Hi Robin,
On 2021/7/22 2:20, Robin Murphy wrote:
quoted
Eliminate the iommu_get_dma_strict() indirection and pipe the
information through the domain type from the beginning. Besides
the flow simplification this also has several nice side-effects:
- Automatically implies strict mode for untrusted devices by
virtue of their IOMMU_DOMAIN_DMA override.
- Ensures that we only ends up using flush queues for drivers
which are aware of them and can actually benefit.
Is this expressed by vendor iommu driver has ops->flush_iotlb_all?
No, it's literally whether ->domain_alloc accepts the DMA_DOMAIN_FQ type
or not.
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-26 12:06:51
On 2021-07-26 09:13, John Garry wrote:
On 21/07/2021 19:20, Robin Murphy wrote:
quoted
Hi all,
First off, yes, this conflicts with just about everything else
currently in-flight. Sorry about that. If it stands up to initial review
then I'll start giving some thought to how to fit everything together
(particularly John's cleanup of strictness defaults, which I'd be
inclined to fold into a v2 of this series).
It seems to me that patch #20 is the only real conflict, and that is
just a different form of mine in that passthrough, strict, and lazy are
under a single choice, as opposed to passthrough being a separate config
(for mine). And on that point, I did assume that we would have a
different sysfs file for strict vs lazy in this series, and not a new
domain type. But I assume that there is a good reason for that.
Yes, as mentioned by patch #18 it helps a surprising number of things
fall into place really neatly.
Anyway, I'd really like to see my series just merged now.
Sure, I was going to say I can happily rebase on top of your series
as-is if Joerg wants to apply it first, and now that's just happened :)
Cheers,
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Lu Baolu <baolu.lu@linux.intel.com> Date: 2021-07-26 12:30:05
On 2021/7/26 16:27, Robin Murphy wrote:
On 2021-07-24 06:29, Lu Baolu wrote:
quoted
Hi Robin,
On 2021/7/22 2:20, Robin Murphy wrote:
quoted
Eliminate the iommu_get_dma_strict() indirection and pipe the
information through the domain type from the beginning. Besides
the flow simplification this also has several nice side-effects:
- Automatically implies strict mode for untrusted devices by
virtue of their IOMMU_DOMAIN_DMA override.
- Ensures that we only ends up using flush queues for drivers
which are aware of them and can actually benefit.
Is this expressed by vendor iommu driver has ops->flush_iotlb_all?
No, it's literally whether ->domain_alloc accepts the DMA_DOMAIN_FQ type
or not.
quoted
quoted
- Allows us to handle flush queue init failure by falling back
to strict mode instead of leaving it to possibly blow up later.
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
drivers/iommu/arm/arm-smmu/arm-smmu.c | 2 +-
drivers/iommu/dma-iommu.c | 10 ++++++----
drivers/iommu/iommu.c | 14 ++++----------
include/linux/iommu.h | 1 -
5 files changed, 12 insertions(+), 17 deletions(-)
Perhaps we can remove the ops->flush_iotlb_all check with the
assumption that any vendor iommu driver with DMA_FQ domain support
should always provides this callback?
Best regards,
baolu
iommu_create_device_direct_mappings(struct iommu_group *group,
unsigned long pg_size;
int ret = 0;
- if (!domain || domain->type != IOMMU_DOMAIN_DMA)
+ if (!domain || !(domain->type & __IOMMU_DOMAIN_DMA_API))
Nit: probably move above change to patch 14?
Indeed I'm not sure why this one ended up here, good catch!
Thanks,
Robin.
Perhaps we can remove the ops->flush_iotlb_all check with the
assumption that any vendor iommu driver with DMA_FQ domain support
should always provides this callback?
Oh yes, indeed it wouldn't make sense for a driver to claim
IOMMU_DOMAIN_DMA_FQ support but not implement the one thing that that
needs the driver to provide. That's yet another neat little cleanup, thanks!
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, Jul 21, 2021 at 07:20:27PM +0100, Robin Murphy wrote:
- if (type == IOMMU_DOMAIN_DMA && using_legacy_binding)
+ if ((type & __IOMMU_DOMAIN_DMA_API) && using_legacy_binding)
Hmm, I wonder whether it is time to introduce helpers for these checks?
Something like iommu_domain_is_dma() is more readable.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Robin,
On Wed, Jul 21, 2021 at 07:20:11PM +0100, Robin Murphy wrote:
Robin Murphy (23):
iommu: Pull IOVA cookie management into the core
iommu/amd: Drop IOVA cookie management
iommu/arm-smmu: Drop IOVA cookie management
iommu/vt-d: Drop IOVA cookie management
iommu/exynos: Drop IOVA cookie management
iommu/ipmmu-vmsa: Drop IOVA cookie management
iommu/mtk: Drop IOVA cookie management
iommu/rockchip: Drop IOVA cookie management
iommu/sprd: Drop IOVA cookie management
iommu/sun50i: Drop IOVA cookie management
iommu/virtio: Drop IOVA cookie management
iommu/dma: Unexport IOVA cookie management
iommu/dma: Remove redundant "!dev" checks
iommu: Introduce explicit type for non-strict DMA domains
iommu/amd: Prepare for multiple DMA domain types
iommu/arm-smmu: Prepare for multiple DMA domain types
iommu/vt-d: Prepare for multiple DMA domain types
iommu: Express DMA strictness via the domain type
iommu: Expose DMA domain strictness via sysfs
iommu: Allow choosing DMA strictness at build time
iommu/dma: Factor out flush queue init
iommu: Allow enabling non-strict mode dynamically
iommu/arm-smmu: Allow non-strict in pgtable_quirks interface
I really like this patch-set. It is a nice cleanup and the
implementation is straightforward. Given no other major objections and
reviews I think this is material for 5.15.
Thanks,
Joerg
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-07-26 13:09:12
On 2021-07-26 13:46, Joerg Roedel wrote:
On Wed, Jul 21, 2021 at 07:20:27PM +0100, Robin Murphy wrote:
quoted
- if (type == IOMMU_DOMAIN_DMA && using_legacy_binding)
+ if ((type & __IOMMU_DOMAIN_DMA_API) && using_legacy_binding)
Hmm, I wonder whether it is time to introduce helpers for these checks?
Something like iommu_domain_is_dma() is more readable.
Ha, I had exactly that at one point, except I think in the order of
iommu_is_dma_domain() :)
The end result didn't seem to give enough extra clarity to justify the
header churn for me, but I'm happy to be wrong about that if you prefer.
Cheers,
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Jul 26, 2021 at 02:09:00PM +0100, Robin Murphy wrote:
Ha, I had exactly that at one point, except I think in the order of
iommu_is_dma_domain() :)
That name is fine too :)
The end result didn't seem to give enough extra clarity to justify the
header churn for me, but I'm happy to be wrong about that if you prefer.
Developers look more into the code than into headers, so I think the
header churn is worth it to improve code readability. But we can do that
on-top of these changes in an extra patch-set which also introduces
helpers for other domain types (if it is worth it).
Regards,
Jörg
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel