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
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <[email protected]>
---
drivers/iommu/exynos-iommu.c | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index d0fbf1d10e18..34085d069cda 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -735,20 +735,16 @@ static struct iommu_domain *exynos_iommu_domain_alloc(unsigned type)
/* Check if correct PTE offsets are initialized */
BUG_ON(PG_ENT_SHIFT < 0 || !dma_dev);
+ if (type != IOMMU_DOMAIN_DMA && type != IOMMU_DOMAIN_UNMANAGED)
+ return NULL;
+
domain = kzalloc(sizeof(*domain), GFP_KERNEL);
if (!domain)
return NULL;
- if (type == IOMMU_DOMAIN_DMA) {
- if (iommu_get_dma_cookie(&domain->domain) != 0)
- goto err_pgtable;
- } else if (type != IOMMU_DOMAIN_UNMANAGED) {
- goto err_pgtable;
- }
-
domain->pgtable = (sysmmu_pte_t *)__get_free_pages(GFP_KERNEL, 2);
if (!domain->pgtable)
- goto err_dma_cookie;
+ goto err_pgtable;
domain->lv2entcnt = (short *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 1);
if (!domain->lv2entcnt)
@@ -779,9 +775,6 @@ static struct iommu_domain *exynos_iommu_domain_alloc(unsigned type)
free_pages((unsigned long)domain->lv2entcnt, 1);
err_counter:
free_pages((unsigned long)domain->pgtable, 2);
-err_dma_cookie:
- if (type == IOMMU_DOMAIN_DMA)
- iommu_put_dma_cookie(&domain->domain);
err_pgtable:
kfree(domain);
return NULL;
@@ -809,9 +802,6 @@ static void exynos_iommu_domain_free(struct iommu_domain *iommu_domain)
spin_unlock_irqrestore(&domain->lock, flags);
- if (iommu_domain->type == IOMMU_DOMAIN_DMA)
- iommu_put_dma_cookie(iommu_domain);
-
dma_unmap_single(dma_dev, virt_to_phys(domain->pgtable), LV1TABLE_SIZE,
DMA_TO_DEVICE);
--
2.25.1
The DMA ops setup can simply be unconditional, since iommu-dma
already knows not to touch identity domains.
Signed-off-by: Robin Murphy <[email protected]>
---
drivers/iommu/amd/iommu.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 40ae7130fc80..1e992ef21f34 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1707,14 +1707,9 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
static void amd_iommu_probe_finalize(struct device *dev)
{
- struct iommu_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);
}
static void amd_iommu_release_device(struct device *dev)
--
2.25.1
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <[email protected]>
---
drivers/iommu/sun50i-iommu.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index 181bb1c3437c..c349a95ec7bd 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -610,14 +610,10 @@ static struct iommu_domain *sun50i_iommu_domain_alloc(unsigned type)
if (!sun50i_domain)
return NULL;
- if (type == IOMMU_DOMAIN_DMA &&
- iommu_get_dma_cookie(&sun50i_domain->domain))
- goto err_free_domain;
-
sun50i_domain->dt = (u32 *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
get_order(DT_SIZE));
if (!sun50i_domain->dt)
- goto err_put_cookie;
+ goto err_free_domain;
refcount_set(&sun50i_domain->refcnt, 1);
@@ -627,10 +623,6 @@ static struct iommu_domain *sun50i_iommu_domain_alloc(unsigned type)
return &sun50i_domain->domain;
-err_put_cookie:
- if (type == IOMMU_DOMAIN_DMA)
- iommu_put_dma_cookie(&sun50i_domain->domain);
-
err_free_domain:
kfree(sun50i_domain);
@@ -644,8 +636,6 @@ static void sun50i_iommu_domain_free(struct iommu_domain *domain)
free_pages((unsigned long)sun50i_domain->dt, get_order(DT_SIZE));
sun50i_domain->dt = NULL;
- iommu_put_dma_cookie(domain);
-
kfree(sun50i_domain);
}
--
2.25.1
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 <[email protected]>
---
drivers/iommu/dma-iommu.c | 31 ++++++++++++++++++++-----------
include/linux/dma-iommu.h | 9 ++++++---
2 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index a114a7ad88ec..64e9eefce00e 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -310,6 +310,25 @@ static bool dev_is_untrusted(struct device *dev)
return dev_is_pci(dev) && to_pci_dev(dev)->untrusted;
}
+int iommu_dma_init_fq(struct iommu_domain *domain)
+{
+ struct iommu_dma_cookie *cookie = domain->iova_cookie;
+
+ if (domain->type != IOMMU_DOMAIN_DMA_FQ || !domain->ops->flush_iotlb_all)
+ return -EINVAL;
+ if (cookie->fq_domain)
+ return 0;
+
+ if (init_iova_flush_queue(&cookie->iovad, iommu_dma_flush_iotlb_all,
+ iommu_dma_entry_dtor)) {
+ pr_warn("iova flush queue initialization failed\n");
+ domain->type = IOMMU_DOMAIN_DMA;
+ return -ENODEV;
+ }
+ cookie->fq_domain = domain;
+ return 0;
+}
+
/**
* iommu_dma_init_domain - Initialise a DMA mapping domain
* @domain: IOMMU domain previously prepared by iommu_get_dma_cookie()
@@ -362,17 +381,7 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
}
init_iova_domain(iovad, 1UL << order, base_pfn);
-
- if (domain->type == IOMMU_DOMAIN_DMA_FQ && !cookie->fq_domain &&
- domain->ops->flush_iotlb_all) {
- if (init_iova_flush_queue(iovad, iommu_dma_flush_iotlb_all,
- iommu_dma_entry_dtor)) {
- pr_warn("iova flush queue initialization failed\n");
- domain->type = IOMMU_DOMAIN_DMA;
- } else {
- cookie->fq_domain = domain;
- }
- }
+ iommu_dma_init_fq(domain);
return iova_reserve_iommu_regions(dev, domain);
}
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 758ca4694257..81ab647f1618 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -20,6 +20,7 @@ void iommu_put_dma_cookie(struct iommu_domain *domain);
/* Setup call for arch DMA mapping code */
void iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 dma_limit);
+int iommu_dma_init_fq(struct iommu_domain *domain);
/* The DMA API isn't _quite_ the whole story, though... */
/*
@@ -37,9 +38,6 @@ void iommu_dma_compose_msi_msg(struct msi_desc *desc,
void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list);
-void iommu_dma_free_cpu_cached_iovas(unsigned int cpu,
- struct iommu_domain *domain);
-
extern bool iommu_dma_forcedac;
#else /* CONFIG_IOMMU_DMA */
@@ -54,6 +52,11 @@ static inline void iommu_setup_dma_ops(struct device *dev, u64 dma_base,
{
}
+static inline int iommu_dma_init_fq(struct iommu_domain *domain)
+{
+ return -EINVAL;
+}
+
static inline int iommu_get_dma_cookie(struct iommu_domain *domain)
{
return -ENODEV;
--
2.25.1
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <[email protected]>
---
drivers/iommu/rockchip-iommu.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 9febfb7f3025..c24561f54f32 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1074,10 +1074,6 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
if (!rk_domain)
return NULL;
- if (type == IOMMU_DOMAIN_DMA &&
- iommu_get_dma_cookie(&rk_domain->domain))
- goto err_free_domain;
-
/*
* rk32xx iommus use a 2 level pagetable.
* Each level1 (dt) and level2 (pt) table has 1024 4-byte entries.
@@ -1085,7 +1081,7 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
*/
rk_domain->dt = (u32 *)get_zeroed_page(GFP_KERNEL | GFP_DMA32);
if (!rk_domain->dt)
- goto err_put_cookie;
+ goto err_free_domain;
rk_domain->dt_dma = dma_map_single(dma_dev, rk_domain->dt,
SPAGE_SIZE, DMA_TO_DEVICE);
@@ -1106,9 +1102,6 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
err_free_dt:
free_page((unsigned long)rk_domain->dt);
-err_put_cookie:
- if (type == IOMMU_DOMAIN_DMA)
- iommu_put_dma_cookie(&rk_domain->domain);
err_free_domain:
kfree(rk_domain);
@@ -1137,8 +1130,6 @@ static void rk_iommu_domain_free(struct iommu_domain *domain)
SPAGE_SIZE, DMA_TO_DEVICE);
free_page((unsigned long)rk_domain->dt);
- if (domain->type == IOMMU_DOMAIN_DMA)
- iommu_put_dma_cookie(&rk_domain->domain);
kfree(rk_domain);
}
--
2.25.1
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <[email protected]>
---
drivers/iommu/amd/iommu.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 811a49a95d04..40ae7130fc80 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1924,16 +1924,7 @@ static struct iommu_domain *amd_iommu_domain_alloc(unsigned type)
domain->domain.geometry.aperture_end = ~0ULL;
domain->domain.geometry.force_aperture = true;
- if (type == IOMMU_DOMAIN_DMA &&
- iommu_get_dma_cookie(&domain->domain) == -ENOMEM)
- goto free_domain;
-
return &domain->domain;
-
-free_domain:
- protection_domain_free(domain);
-
- return NULL;
}
static void amd_iommu_domain_free(struct iommu_domain *dom)
@@ -1950,9 +1941,6 @@ static void amd_iommu_domain_free(struct iommu_domain *dom)
if (!dom)
return;
- if (dom->type == IOMMU_DOMAIN_DMA)
- iommu_put_dma_cookie(&domain->domain);
-
if (domain->flags & PD_IOMMUV2_MASK)
free_gcr3_table(domain);
--
2.25.1
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 <[email protected]>
---
drivers/iommu/dma-iommu.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 10067fbc4309..e28396cea6eb 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -363,7 +363,7 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
init_iova_domain(iovad, 1UL << order, base_pfn);
- if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) &&
+ if (!cookie->fq_domain && !dev_is_untrusted(dev) &&
domain->ops->flush_iotlb_all && !iommu_get_dma_strict(domain)) {
if (init_iova_flush_queue(iovad, iommu_dma_flush_iotlb_all,
iommu_dma_entry_dtor))
@@ -372,9 +372,6 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
cookie->fq_domain = domain;
}
- if (!dev)
- return 0;
-
return iova_reserve_iommu_regions(dev, domain);
}
--
2.25.1
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <[email protected]>
---
drivers/iommu/sprd-iommu.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/iommu/sprd-iommu.c b/drivers/iommu/sprd-iommu.c
index 73dfd9946312..2bc1de6e823d 100644
--- a/drivers/iommu/sprd-iommu.c
+++ b/drivers/iommu/sprd-iommu.c
@@ -144,11 +144,6 @@ static struct iommu_domain *sprd_iommu_domain_alloc(unsigned int domain_type)
if (!dom)
return NULL;
- if (iommu_get_dma_cookie(&dom->domain)) {
- kfree(dom);
- return NULL;
- }
-
spin_lock_init(&dom->pgtlock);
dom->domain.geometry.aperture_start = 0;
@@ -161,7 +156,6 @@ static void sprd_iommu_domain_free(struct iommu_domain *domain)
{
struct sprd_iommu_domain *dom = to_sprd_domain(domain);
- iommu_put_dma_cookie(domain);
kfree(dom);
}
--
2.25.1
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 <[email protected]>
---
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(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index fa41026d272e..260b560d0075 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2175,7 +2175,7 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain,
.iommu_dev = smmu->dev,
};
- if (!iommu_get_dma_strict(domain))
+ if (domain->type == IOMMU_DOMAIN_DMA_FQ)
pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
pgtbl_ops = alloc_io_pgtable_ops(fmt, &pgtbl_cfg, smmu_domain);
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index dbc14c265b15..2c717f3be056 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -765,7 +765,7 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
.iommu_dev = smmu->dev,
};
- if (!iommu_get_dma_strict(domain))
+ if (domain->type == IOMMU_DOMAIN_DMA_FQ)
pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
if (smmu->impl && smmu->impl->init_context) {
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index b1af1ff324c5..a114a7ad88ec 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -363,13 +363,15 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
init_iova_domain(iovad, 1UL << order, base_pfn);
- if (!cookie->fq_domain && !dev_is_untrusted(dev) &&
- domain->ops->flush_iotlb_all && !iommu_get_dma_strict(domain)) {
+ if (domain->type == IOMMU_DOMAIN_DMA_FQ && !cookie->fq_domain &&
+ domain->ops->flush_iotlb_all) {
if (init_iova_flush_queue(iovad, iommu_dma_flush_iotlb_all,
- iommu_dma_entry_dtor))
+ iommu_dma_entry_dtor)) {
pr_warn("iova flush queue initialization failed\n");
- else
+ domain->type = IOMMU_DOMAIN_DMA;
+ } else {
cookie->fq_domain = domain;
+ }
}
return iova_reserve_iommu_regions(dev, domain);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 8333c334891e..d7eaacae0944 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -135,6 +135,9 @@ static int __init iommu_subsys_init(void)
}
}
+ if (!iommu_default_passthrough() && !iommu_dma_strict)
+ iommu_def_domain_type = IOMMU_DOMAIN_DMA_FQ;
+
pr_info("Default domain type: %s %s\n",
iommu_domain_type_str(iommu_def_domain_type),
(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?
@@ -352,15 +355,6 @@ void iommu_set_dma_strict(bool strict)
iommu_dma_strict = strict;
}
-bool iommu_get_dma_strict(struct iommu_domain *domain)
-{
- /* only allow lazy flushing for DMA domains */
- if (domain->type == IOMMU_DOMAIN_DMA)
- return iommu_dma_strict;
- return true;
-}
-EXPORT_SYMBOL_GPL(iommu_get_dma_strict);
-
static ssize_t iommu_group_attr_show(struct kobject *kobj,
struct attribute *__attr, char *buf)
{
@@ -764,7 +758,7 @@ static int 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))
return 0;
BUG_ON(!domain->pgsize_bitmap);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 56519110d43f..557c4c12e2cf 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -484,7 +484,6 @@ int iommu_set_pgtable_quirks(struct iommu_domain *domain,
unsigned long quirks);
void iommu_set_dma_strict(bool val);
-bool iommu_get_dma_strict(struct iommu_domain *domain);
extern int report_iommu_fault(struct iommu_domain *domain, struct device *dev,
unsigned long iova, int flags);
--
2.25.1
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 <[email protected]>
---
drivers/iommu/intel/iommu.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index e2add5a0caef..77d322272743 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -601,7 +601,7 @@ struct intel_iommu *domain_get_iommu(struct 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)
@@ -1035,7 +1035,7 @@ static struct dma_pte *pfn_to_dma_pte(struct dmar_domain *domain,
pteval = ((uint64_t)virt_to_dma_pfn(tmp_page) << VTD_PAGE_SHIFT) | DMA_PTE_READ | DMA_PTE_WRITE;
if (domain_use_first_level(domain)) {
pteval |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
- if (domain->domain.type == IOMMU_DOMAIN_DMA)
+ if (domain->domain.type & __IOMMU_DOMAIN_DMA_API)
pteval |= DMA_FL_PTE_ACCESS;
}
if (cmpxchg64(&pte->val, 0ULL, pteval))
@@ -2346,7 +2346,7 @@ __domain_mapping(struct dmar_domain *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) {
@@ -5164,12 +5165,8 @@ static void intel_iommu_release_device(struct device *dev)
static void intel_iommu_probe_finalize(struct device *dev)
{
- struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
-
- if (domain && 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);
}
static void intel_iommu_get_resv_regions(struct device *device,
--
2.25.1
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <[email protected]>
---
drivers/iommu/mtk_iommu.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 6f7c69688ce2..e39a6d1da28d 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -441,17 +441,11 @@ static struct iommu_domain *mtk_iommu_domain_alloc(unsigned type)
if (!dom)
return NULL;
- if (iommu_get_dma_cookie(&dom->domain)) {
- kfree(dom);
- return NULL;
- }
-
return &dom->domain;
}
static void mtk_iommu_domain_free(struct iommu_domain *domain)
{
- iommu_put_dma_cookie(domain);
kfree(to_mtk_domain(domain));
}
--
2.25.1
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <[email protected]>
---
drivers/iommu/ipmmu-vmsa.c | 27 ++++-----------------------
1 file changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 51ea6f00db2f..31252268f0d0 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -564,10 +564,13 @@ static irqreturn_t ipmmu_irq(int irq, void *dev)
* IOMMU Operations
*/
-static struct iommu_domain *__ipmmu_domain_alloc(unsigned type)
+static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
{
struct ipmmu_vmsa_domain *domain;
+ if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
+ return NULL;
+
domain = kzalloc(sizeof(*domain), GFP_KERNEL);
if (!domain)
return NULL;
@@ -577,27 +580,6 @@ static struct iommu_domain *__ipmmu_domain_alloc(unsigned type)
return &domain->io_domain;
}
-static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
-{
- struct iommu_domain *io_domain = NULL;
-
- switch (type) {
- case IOMMU_DOMAIN_UNMANAGED:
- io_domain = __ipmmu_domain_alloc(type);
- break;
-
- case IOMMU_DOMAIN_DMA:
- io_domain = __ipmmu_domain_alloc(type);
- if (io_domain && iommu_get_dma_cookie(io_domain)) {
- kfree(io_domain);
- io_domain = NULL;
- }
- break;
- }
-
- return io_domain;
-}
-
static void ipmmu_domain_free(struct iommu_domain *io_domain)
{
struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
@@ -606,7 +588,6 @@ static void ipmmu_domain_free(struct iommu_domain *io_domain)
* Free the domain resources. We assume that all devices have already
* been detached.
*/
- iommu_put_dma_cookie(io_domain);
ipmmu_domain_destroy_context(domain);
free_io_pgtable_ops(domain->iop);
kfree(domain);
--
2.25.1
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <[email protected]>
---
drivers/iommu/virtio-iommu.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 6abdcab7273b..80930ce04a16 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -598,12 +598,6 @@ static struct iommu_domain *viommu_domain_alloc(unsigned type)
spin_lock_init(&vdomain->mappings_lock);
vdomain->mappings = RB_ROOT_CACHED;
- if (type == IOMMU_DOMAIN_DMA &&
- iommu_get_dma_cookie(&vdomain->domain)) {
- kfree(vdomain);
- return NULL;
- }
-
return &vdomain->domain;
}
@@ -643,8 +637,6 @@ static void viommu_domain_free(struct iommu_domain *domain)
{
struct viommu_domain *vdomain = to_viommu_domain(domain);
- iommu_put_dma_cookie(domain);
-
/* Free all remaining mappings (size 2^64) */
viommu_del_mappings(vdomain, 0, 0);
--
2.25.1
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 <[email protected]>
---
drivers/iommu/dma-iommu.c | 2 +-
drivers/iommu/iommu.c | 6 +++++-
include/linux/iommu.h | 6 ++++++
3 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index e28396cea6eb..b1af1ff324c5 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -1311,7 +1311,7 @@ void iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 dma_limit)
* The IOMMU core code allocates the default DMA domain, which the
* underlying IOMMU driver needs to support via the dma-iommu layer.
*/
- if (domain->type == IOMMU_DOMAIN_DMA) {
+ if (domain->type & __IOMMU_DOMAIN_DMA_API) {
if (iommu_dma_init_domain(domain, dma_base, dma_limit, dev))
goto out_err;
dev->dma_ops = &iommu_dma_ops;
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index e23d0c9be9db..8333c334891e 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -114,6 +114,7 @@ static const char *iommu_domain_type_str(unsigned int t)
case IOMMU_DOMAIN_UNMANAGED:
return "Unmanaged";
case IOMMU_DOMAIN_DMA:
+ case IOMMU_DOMAIN_DMA_FQ:
return "Translated";
default:
return "Unknown";
@@ -547,6 +548,9 @@ static ssize_t iommu_group_show_type(struct iommu_group *group,
case IOMMU_DOMAIN_DMA:
type = "DMA\n";
break;
+ case IOMMU_DOMAIN_DMA_FQ:
+ type = "DMA-FQ\n";
+ break;
}
}
mutex_unlock(&group->mutex);
@@ -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;
}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 007662b65474..56519110d43f 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -61,6 +61,7 @@ struct iommu_domain_geometry {
#define __IOMMU_DOMAIN_DMA_API (1U << 1) /* Domain for use in DMA-API
implementation */
#define __IOMMU_DOMAIN_PT (1U << 2) /* Domain is identity mapped */
+#define __IOMMU_DOMAIN_DMA_FQ (1U << 3) /* DMA-API uses flush queue */
/*
* This are the possible domain-types
@@ -73,12 +74,17 @@ struct iommu_domain_geometry {
* IOMMU_DOMAIN_DMA - Internally used for DMA-API implementations.
* This flag allows IOMMU drivers to implement
* certain optimizations for these domains
+ * IOMMU_DOMAIN_DMA_FQ - As above, but definitely using batched TLB
+ * invalidation.
*/
#define IOMMU_DOMAIN_BLOCKED (0U)
#define IOMMU_DOMAIN_IDENTITY (__IOMMU_DOMAIN_PT)
#define IOMMU_DOMAIN_UNMANAGED (__IOMMU_DOMAIN_PAGING)
#define IOMMU_DOMAIN_DMA (__IOMMU_DOMAIN_PAGING | \
__IOMMU_DOMAIN_DMA_API)
+#define IOMMU_DOMAIN_DMA_FQ (__IOMMU_DOMAIN_PAGING | \
+ __IOMMU_DOMAIN_DMA_API | \
+ __IOMMU_DOMAIN_DMA_FQ)
struct iommu_domain {
unsigned type;
--
2.25.1
The core code bakes its own cookies now.
Signed-off-by: Robin Murphy <[email protected]>
---
drivers/iommu/intel/iommu.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index dd22fc7d5176..e2add5a0caef 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1980,10 +1980,6 @@ static void domain_exit(struct dmar_domain *domain)
/* Remove associated devices and clear attached or cached domains */
domain_remove_dev_info(domain);
- /* destroy iovas */
- if (domain->domain.type == IOMMU_DOMAIN_DMA)
- iommu_put_dma_cookie(&domain->domain);
-
if (domain->pgd) {
struct page *freelist;
@@ -4544,10 +4540,6 @@ static struct iommu_domain *intel_iommu_domain_alloc(unsigned type)
return NULL;
}
- if (type == IOMMU_DOMAIN_DMA &&
- iommu_get_dma_cookie(&dmar_domain->domain))
- return NULL;
-
domain = &dmar_domain->domain;
domain->geometry.aperture_start = 0;
domain->geometry.aperture_end =
--
2.25.1
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 <[email protected]>
---
drivers/iommu/dma-iommu.c | 7 -------
drivers/iommu/iommu.c | 3 +--
2 files changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 98ba927aee1a..10067fbc4309 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -98,9 +98,6 @@ static struct iommu_dma_cookie *cookie_alloc(enum iommu_dma_cookie_type type)
/**
* iommu_get_dma_cookie - Acquire DMA-API resources for a domain
* @domain: IOMMU domain to prepare for DMA-API usage
- *
- * IOMMU drivers should normally call this from their domain_alloc
- * callback when domain->type == IOMMU_DOMAIN_DMA.
*/
int iommu_get_dma_cookie(struct iommu_domain *domain)
{
@@ -113,7 +110,6 @@ int iommu_get_dma_cookie(struct iommu_domain *domain)
return 0;
}
-EXPORT_SYMBOL(iommu_get_dma_cookie);
/**
* iommu_get_msi_cookie - Acquire just MSI remapping resources
@@ -151,8 +147,6 @@ EXPORT_SYMBOL(iommu_get_msi_cookie);
* iommu_put_dma_cookie - Release a domain's DMA mapping resources
* @domain: IOMMU domain previously prepared by iommu_get_dma_cookie() or
* iommu_get_msi_cookie()
- *
- * IOMMU drivers should normally call this from their domain_free callback.
*/
void iommu_put_dma_cookie(struct iommu_domain *domain)
{
@@ -172,7 +166,6 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
kfree(cookie);
domain->iova_cookie = NULL;
}
-EXPORT_SYMBOL(iommu_put_dma_cookie);
/**
* iommu_dma_get_resv_regions - Reserved region driver helper
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0952de57c466..e23d0c9be9db 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -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
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 <[email protected]>
---
drivers/iommu/iommu.c | 7 +++++++
include/linux/iommu.h | 3 ++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 5419c4b9f27a..0952de57c466 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -7,6 +7,7 @@
#define pr_fmt(fmt) "iommu: " fmt
#include <linux/device.h>
+#include <linux/dma-iommu.h>
#include <linux/kernel.h>
#include <linux/bug.h>
#include <linux/types.h>
@@ -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;
+ }
return domain;
}
@@ -1952,6 +1958,7 @@ EXPORT_SYMBOL_GPL(iommu_domain_alloc);
void iommu_domain_free(struct iommu_domain *domain)
{
+ iommu_put_dma_cookie(domain);
domain->ops->domain_free(domain);
}
EXPORT_SYMBOL_GPL(iommu_domain_free);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 32d448050bf7..007662b65474 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -40,6 +40,7 @@ struct iommu_domain;
struct notifier_block;
struct iommu_sva;
struct iommu_fault_event;
+struct iommu_dma_cookie;
/* iommu fault flags */
#define IOMMU_FAULT_READ 0x0
@@ -86,7 +87,7 @@ struct iommu_domain {
iommu_fault_handler_t handler;
void *handler_token;
struct iommu_domain_geometry geometry;
- void *iova_cookie;
+ struct iommu_dma_cookie *iova_cookie;
};
enum iommu_cap {
--
2.25.1
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 <[email protected]>
---
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(-)
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 07b7c25cbed8..e3f7990046ae 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -79,16 +79,48 @@ config IOMMU_DEBUGFS
debug/iommu directory, and then populate a subdirectory with
entries as required.
-config IOMMU_DEFAULT_PASSTHROUGH
- bool "IOMMU passthrough by default"
+choice
+ prompt "Default IOMMU domain type"
depends on IOMMU_API
- help
- Enable passthrough by default, removing the need to pass in
- iommu.passthrough=on or iommu=pt through command line. If this
- is enabled, you can still disable with iommu.passthrough=off
- or iommu=nopt depending on the architecture.
+ default IOMMU_DEFAULT_DMA_LAZY if INTEL_IOMMU || AMD_IOMMU
+ default IOMMU_DEFAULT_DMA_STRICT
- If unsure, say N here.
+config IOMMU_DEFAULT_DMA_STRICT
+ bool "Translated - Strict"
+ help
+ Trusted devices use translation to restrict their access to only
+ DMA-mapped pages, with strict TLB invalidation on unmap. Equivalent
+ to passing "iommu.passthrough=0 iommu.strict=1" on the command line.
+
+ Untrusted devices always use this mode, with an additional layer of
+ bounce-buffering such that they cannot gain access to any unrelated
+ data within a mapped page.
+
+config IOMMU_DEFAULT_DMA_LAZY
+ bool "Translated - Lazy"
+ help
+ Trusted devices use translation to restrict their access to only
+ DMA-mapped pages, but with "lazy" batched TLB invalidation. This
+ mode allows higher performance with some IOMMUs due to reduced TLB
+ flushing, but at the cost of reduced isolation since devices may be
+ able to access memory for some time after it has been unmapped.
+ Equivalent to passing "iommu.passthrough=0 iommu.strict=0" on the
+ command line.
+
+ If this mode is not supported by the IOMMU driver, the effective
+ runtime default will fall back to IOMMU_DEFAULT_DMA_STRICT.
+
+config IOMMU_DEFAULT_PASSTHROUGH
+ bool "Passthrough"
+ help
+ Trusted devices are identity-mapped, giving them unrestricted access
+ to memory with minimal performance overhead. Equivalent to passing
+ "iommu.passthrough=1" (historically "iommu=pt") on the command line.
+
+ If this mode is not supported by the IOMMU driver, the effective
+ runtime default will fall back to IOMMU_DEFAULT_DMA_STRICT.
+
+endchoice
config OF_IOMMU
def_bool y
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d3b562a33ac4..4fad6d427d9d 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -30,7 +30,7 @@ static struct kset *iommu_group_kset;
static DEFINE_IDA(iommu_group_ida);
static unsigned int iommu_def_domain_type __read_mostly;
-static bool iommu_dma_strict __read_mostly = true;
+static bool iommu_dma_strict __read_mostly = IS_ENABLED(CONFIG_IOMMU_DEFAULT_DMA_STRICT);
static u32 iommu_cmd_line __read_mostly;
struct iommu_group {
--
2.25.1
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 <[email protected]>
---
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(+)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 260b560d0075..ca19e4551468 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2714,6 +2714,20 @@ static int arm_smmu_enable_nesting(struct iommu_domain *domain)
return ret;
}
+static int arm_smmu_set_pgtable_quirks(struct iommu_domain *domain,
+ unsigned long quirks)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+
+ if (quirks == IO_PGTABLE_QUIRK_NON_STRICT && smmu_domain->pgtbl_ops) {
+ struct io_pgtable *iop = io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops);
+
+ iop->cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
+ return 0;
+ }
+ return -EINVAL;
+}
+
static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
{
return iommu_fwspec_add_ids(dev, args->args, 1);
@@ -2828,6 +2842,7 @@ static struct iommu_ops arm_smmu_ops = {
.release_device = arm_smmu_release_device,
.device_group = arm_smmu_device_group,
.enable_nesting = arm_smmu_enable_nesting,
+ .set_pgtable_quirks = arm_smmu_set_pgtable_quirks,
.of_xlate = arm_smmu_of_xlate,
.get_resv_regions = arm_smmu_get_resv_regions,
.put_resv_regions = generic_iommu_put_resv_regions,
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 2c717f3be056..0f181f76c31b 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1522,6 +1522,17 @@ static int arm_smmu_set_pgtable_quirks(struct iommu_domain *domain,
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
int ret = 0;
+ if (quirks == IO_PGTABLE_QUIRK_NON_STRICT) {
+ struct io_pgtable *iop;
+
+ if (!smmu_domain->pgtbl_ops)
+ return -EINVAL;
+
+ iop = io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops);
+ iop->cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
+ return 0;
+ }
+
mutex_lock(&smmu_domain->init_mutex);
if (smmu_domain->smmu)
ret = -EPERM;
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 64e9eefce00e..6b51e45e2358 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -16,6 +16,7 @@
#include <linux/huge_mm.h>
#include <linux/iommu.h>
#include <linux/iova.h>
+#include <linux/io-pgtable.h>
#include <linux/irq.h>
#include <linux/mm.h>
#include <linux/mutex.h>
@@ -326,6 +327,8 @@ int iommu_dma_init_fq(struct iommu_domain *domain)
return -ENODEV;
}
cookie->fq_domain = domain;
+ if (domain->ops->set_pgtable_quirks)
+ domain->ops->set_pgtable_quirks(domain, IO_PGTABLE_QUIRK_NON_STRICT);
return 0;
}
--
2.25.1
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 <[email protected]>
---
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(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 039f371d2f8b..fa41026d272e 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1972,6 +1972,7 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
if (type != IOMMU_DOMAIN_UNMANAGED &&
type != IOMMU_DOMAIN_DMA &&
+ type != IOMMU_DOMAIN_DMA_FQ &&
type != IOMMU_DOMAIN_IDENTITY)
return NULL;
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 354be8f4c0ef..dbc14c265b15 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -870,10 +870,11 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
if (type != IOMMU_DOMAIN_UNMANAGED &&
type != IOMMU_DOMAIN_DMA &&
+ type != IOMMU_DOMAIN_DMA_FQ &&
type != IOMMU_DOMAIN_IDENTITY)
return NULL;
- if (type == IOMMU_DOMAIN_DMA && using_legacy_binding)
+ if ((type & __IOMMU_DOMAIN_DMA_API) && using_legacy_binding)
return NULL;
/*
* Allocate the domain and initialise some of its data structures.
--
2.25.1
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 <[email protected]>
---
drivers/iommu/iommu.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 4fad6d427d9d..43a2041d9629 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3130,6 +3130,13 @@ static int iommu_change_dev_def_domain(struct iommu_group *group,
goto out;
}
+ /* 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);
+ goto out;
+ }
+
/* 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,
}
/*
- * Changing the default domain through sysfs requires the users to ubind the
- * drivers from the devices in the iommu group. Return failure if this doesn't
- * meet.
+ * Changing the default domain through sysfs requires the users to unbind the
+ * drivers from the devices in the iommu group, except for a DMA -> DMA-FQ
+ * transition. Return failure if this isn't met.
*
* We need to consider the race between this and the device release path.
* device_lock(dev) is used here to guarantee that the device release path
@@ -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;
goto out;
--
2.25.1
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 <[email protected]>
---
Documentation/ABI/testing/sysfs-kernel-iommu_groups | 2 ++
drivers/iommu/iommu.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-kernel-iommu_groups b/Documentation/ABI/testing/sysfs-kernel-iommu_groups
index eae2f1c1e11e..43ba764ba5b7 100644
--- a/Documentation/ABI/testing/sysfs-kernel-iommu_groups
+++ b/Documentation/ABI/testing/sysfs-kernel-iommu_groups
@@ -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.
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d7eaacae0944..d3b562a33ac4 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3195,6 +3195,8 @@ static ssize_t iommu_group_store_type(struct iommu_group *group,
req_type = IOMMU_DOMAIN_IDENTITY;
else if (sysfs_streq(buf, "DMA"))
req_type = IOMMU_DOMAIN_DMA;
+ else if (sysfs_streq(buf, "DMA-FQ"))
+ req_type = IOMMU_DOMAIN_DMA_FQ;
else if (sysfs_streq(buf, "auto"))
req_type = 0;
else
--
2.25.1
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 <[email protected]>
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':
>> 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/[email protected]
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 <[email protected]>
>
> 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':
>>> 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.
> 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/[email protected]
>
>
> _______________________________________________
> iommu mailing list
> [email protected]
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>
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: x86_64-randconfig-a015-20210722 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 9625ca5b602616b2f5584e8a49ba93c52c141e40)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/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=clang make.cross O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> drivers/iommu/intel/iommu.c:604:38: error: use of undeclared identifier '__IOMMU_DOMAIN_DMA'
if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
^
1 error generated.
vim +/__IOMMU_DOMAIN_DMA +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/[email protected]
Hi Robin,
On 2021/7/22 2:20, Robin Murphy wrote:
> 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 <[email protected]>
> ---
> drivers/iommu/intel/iommu.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index e2add5a0caef..77d322272743 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -601,7 +601,7 @@ struct intel_iommu *domain_get_iommu(struct 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)
> @@ -1035,7 +1035,7 @@ static struct dma_pte *pfn_to_dma_pte(struct dmar_domain *domain,
> pteval = ((uint64_t)virt_to_dma_pfn(tmp_page) << VTD_PAGE_SHIFT) | DMA_PTE_READ | DMA_PTE_WRITE;
> if (domain_use_first_level(domain)) {
> pteval |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
> - if (domain->domain.type == IOMMU_DOMAIN_DMA)
> + if (domain->domain.type & __IOMMU_DOMAIN_DMA_API)
> pteval |= DMA_FL_PTE_ACCESS;
> }
> if (cmpxchg64(&pte->val, 0ULL, pteval))
> @@ -2346,7 +2346,7 @@ __domain_mapping(struct dmar_domain *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) {
> @@ -5164,12 +5165,8 @@ static void intel_iommu_release_device(struct device *dev)
>
> static void intel_iommu_probe_finalize(struct device *dev)
> {
> - struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
> -
> - if (domain && domain->type == IOMMU_DOMAIN_DMA)
> - iommu_setup_dma_ops(dev, 0, U64_MAX);
> - else
> - set_dma_ops(dev, NULL);
> + set_dma_ops(dev, NULL);
Is it reasonable to remove above line? The idea is that vendor iommu
driver should not override the dma_ops if device doesn't have a DMA
domain.
> + iommu_setup_dma_ops(dev, 0, U64_MAX);
> }
>
> static void intel_iommu_get_resv_regions(struct device *device,
>
Best regards,
baolu
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?
> - 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 <[email protected]>
> ---
> 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(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index fa41026d272e..260b560d0075 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2175,7 +2175,7 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain,
> .iommu_dev = smmu->dev,
> };
>
> - if (!iommu_get_dma_strict(domain))
> + if (domain->type == IOMMU_DOMAIN_DMA_FQ)
> pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
>
> pgtbl_ops = alloc_io_pgtable_ops(fmt, &pgtbl_cfg, smmu_domain);
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index dbc14c265b15..2c717f3be056 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -765,7 +765,7 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
> .iommu_dev = smmu->dev,
> };
>
> - if (!iommu_get_dma_strict(domain))
> + if (domain->type == IOMMU_DOMAIN_DMA_FQ)
> pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
>
> if (smmu->impl && smmu->impl->init_context) {
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index b1af1ff324c5..a114a7ad88ec 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -363,13 +363,15 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>
> init_iova_domain(iovad, 1UL << order, base_pfn);
>
> - if (!cookie->fq_domain && !dev_is_untrusted(dev) &&
> - domain->ops->flush_iotlb_all && !iommu_get_dma_strict(domain)) {
> + if (domain->type == IOMMU_DOMAIN_DMA_FQ && !cookie->fq_domain &&
> + domain->ops->flush_iotlb_all) {
> if (init_iova_flush_queue(iovad, iommu_dma_flush_iotlb_all,
> - iommu_dma_entry_dtor))
> + iommu_dma_entry_dtor)) {
> pr_warn("iova flush queue initialization failed\n");
> - else
> + domain->type = IOMMU_DOMAIN_DMA;
> + } else {
> cookie->fq_domain = domain;
> + }
> }
>
> return iova_reserve_iommu_regions(dev, domain);
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 8333c334891e..d7eaacae0944 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -135,6 +135,9 @@ static int __init iommu_subsys_init(void)
> }
> }
>
> + if (!iommu_default_passthrough() && !iommu_dma_strict)
> + iommu_def_domain_type = IOMMU_DOMAIN_DMA_FQ;
> +
> pr_info("Default domain type: %s %s\n",
> iommu_domain_type_str(iommu_def_domain_type),
> (iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?
> @@ -352,15 +355,6 @@ void iommu_set_dma_strict(bool strict)
> iommu_dma_strict = strict;
> }
>
> -bool iommu_get_dma_strict(struct iommu_domain *domain)
> -{
> - /* only allow lazy flushing for DMA domains */
> - if (domain->type == IOMMU_DOMAIN_DMA)
> - return iommu_dma_strict;
> - return true;
> -}
> -EXPORT_SYMBOL_GPL(iommu_get_dma_strict);
> -
> static ssize_t iommu_group_attr_show(struct kobject *kobj,
> struct attribute *__attr, char *buf)
> {
> @@ -764,7 +758,7 @@ static int 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?
> return 0;
>
> BUG_ON(!domain->pgsize_bitmap);
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 56519110d43f..557c4c12e2cf 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -484,7 +484,6 @@ int iommu_set_pgtable_quirks(struct iommu_domain *domain,
> unsigned long quirks);
>
> void iommu_set_dma_strict(bool val);
> -bool iommu_get_dma_strict(struct iommu_domain *domain);
>
> extern int report_iommu_fault(struct iommu_domain *domain, struct device *dev,
> unsigned long iova, int flags);
>
Best regards,
baolu
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:
On 2021-07-24 06:29, Lu Baolu wrote:
> 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?
No, it's literally whether ->domain_alloc accepts the DMA_DOMAIN_FQ type
or not.
>> - 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 <[email protected]>
>> ---
>> 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(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> index fa41026d272e..260b560d0075 100644
>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>> @@ -2175,7 +2175,7 @@ static int arm_smmu_domain_finalise(struct
>> iommu_domain *domain,
>> .iommu_dev = smmu->dev,
>> };
>> - if (!iommu_get_dma_strict(domain))
>> + if (domain->type == IOMMU_DOMAIN_DMA_FQ)
>> pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
>> pgtbl_ops = alloc_io_pgtable_ops(fmt, &pgtbl_cfg, smmu_domain);
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> index dbc14c265b15..2c717f3be056 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> @@ -765,7 +765,7 @@ static int arm_smmu_init_domain_context(struct
>> iommu_domain *domain,
>> .iommu_dev = smmu->dev,
>> };
>> - if (!iommu_get_dma_strict(domain))
>> + if (domain->type == IOMMU_DOMAIN_DMA_FQ)
>> pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
>> if (smmu->impl && smmu->impl->init_context) {
>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>> index b1af1ff324c5..a114a7ad88ec 100644
>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c
>> @@ -363,13 +363,15 @@ static int iommu_dma_init_domain(struct
>> iommu_domain *domain, dma_addr_t base,
>> init_iova_domain(iovad, 1UL << order, base_pfn);
>> - if (!cookie->fq_domain && !dev_is_untrusted(dev) &&
>> - domain->ops->flush_iotlb_all && !iommu_get_dma_strict(domain)) {
>> + if (domain->type == IOMMU_DOMAIN_DMA_FQ && !cookie->fq_domain &&
>> + domain->ops->flush_iotlb_all) {
>> if (init_iova_flush_queue(iovad, iommu_dma_flush_iotlb_all,
>> - iommu_dma_entry_dtor))
>> + iommu_dma_entry_dtor)) {
>> pr_warn("iova flush queue initialization failed\n");
>> - else
>> + domain->type = IOMMU_DOMAIN_DMA;
>> + } else {
>> cookie->fq_domain = domain;
>> + }
>> }
>> return iova_reserve_iommu_regions(dev, domain);
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 8333c334891e..d7eaacae0944 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -135,6 +135,9 @@ static int __init iommu_subsys_init(void)
>> }
>> }
>> + if (!iommu_default_passthrough() && !iommu_dma_strict)
>> + iommu_def_domain_type = IOMMU_DOMAIN_DMA_FQ;
>> +
>> pr_info("Default domain type: %s %s\n",
>> iommu_domain_type_str(iommu_def_domain_type),
>> (iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?
>> @@ -352,15 +355,6 @@ void iommu_set_dma_strict(bool strict)
>> iommu_dma_strict = strict;
>> }
>> -bool iommu_get_dma_strict(struct iommu_domain *domain)
>> -{
>> - /* only allow lazy flushing for DMA domains */
>> - if (domain->type == IOMMU_DOMAIN_DMA)
>> - return iommu_dma_strict;
>> - return true;
>> -}
>> -EXPORT_SYMBOL_GPL(iommu_get_dma_strict);
>> -
>> static ssize_t iommu_group_attr_show(struct kobject *kobj,
>> struct attribute *__attr, char *buf)
>> {
>> @@ -764,7 +758,7 @@ static int
>> 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.
>> return 0;
>> BUG_ON(!domain->pgsize_bitmap);
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index 56519110d43f..557c4c12e2cf 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -484,7 +484,6 @@ int iommu_set_pgtable_quirks(struct iommu_domain
>> *domain,
>> unsigned long quirks);
>> void iommu_set_dma_strict(bool val);
>> -bool iommu_get_dma_strict(struct iommu_domain *domain);
>> extern int report_iommu_fault(struct iommu_domain *domain, struct
>> device *dev,
>> unsigned long iova, int flags);
>>
>
> Best regards,
> baolu
On 2021-07-24 06:23, Lu Baolu wrote:
> Hi Robin,
>
> On 2021/7/22 2:20, Robin Murphy wrote:
>> 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 <[email protected]>
>> ---
>> drivers/iommu/intel/iommu.c | 15 ++++++---------
>> 1 file changed, 6 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>> index e2add5a0caef..77d322272743 100644
>> --- a/drivers/iommu/intel/iommu.c
>> +++ b/drivers/iommu/intel/iommu.c
>> @@ -601,7 +601,7 @@ struct intel_iommu *domain_get_iommu(struct
>> 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)
>> @@ -1035,7 +1035,7 @@ static struct dma_pte *pfn_to_dma_pte(struct
>> dmar_domain *domain,
>> pteval = ((uint64_t)virt_to_dma_pfn(tmp_page) <<
>> VTD_PAGE_SHIFT) | DMA_PTE_READ | DMA_PTE_WRITE;
>> if (domain_use_first_level(domain)) {
>> pteval |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
>> - if (domain->domain.type == IOMMU_DOMAIN_DMA)
>> + if (domain->domain.type & __IOMMU_DOMAIN_DMA_API)
>> pteval |= DMA_FL_PTE_ACCESS;
>> }
>> if (cmpxchg64(&pte->val, 0ULL, pteval))
>> @@ -2346,7 +2346,7 @@ __domain_mapping(struct dmar_domain *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) {
>> @@ -5164,12 +5165,8 @@ static void intel_iommu_release_device(struct
>> device *dev)
>> static void intel_iommu_probe_finalize(struct device *dev)
>> {
>> - struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
>> -
>> - if (domain && domain->type == IOMMU_DOMAIN_DMA)
>> - iommu_setup_dma_ops(dev, 0, U64_MAX);
>> - else
>> - set_dma_ops(dev, NULL);
>> + set_dma_ops(dev, NULL);
>
> Is it reasonable to remove above line? The idea is that vendor iommu
> driver should not override the dma_ops if device doesn't have a DMA
> domain.
As the commit message implies, that's exactly how iommu_setup_dma_ops()
has always behaved anyway. There should be no functional change here.
Robin.
>> + iommu_setup_dma_ops(dev, 0, U64_MAX);
>> }
>> static void intel_iommu_get_resv_regions(struct device *device,
>>
>
> Best regards,
> baolu
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<[email protected]>
FWIW,
Reviewed-by: John Garry <[email protected]>
On 2021/7/26 16:27, Robin Murphy wrote:
> On 2021-07-24 06:29, Lu Baolu wrote:
>> 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?
>
> No, it's literally whether ->domain_alloc accepts the DMA_DOMAIN_FQ type
> or not.
Get it. Thank you!
Best regards,
baolu
On 2021-07-26 09:13, John Garry wrote:
> 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.
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.
On 2021/7/26 16:27, Robin Murphy wrote:
> On 2021-07-24 06:29, Lu Baolu wrote:
>> 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?
>
> No, it's literally whether ->domain_alloc accepts the DMA_DOMAIN_FQ type
> or not.
>
>>> - 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 <[email protected]>
>>> ---
>>> 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(-)
>>>
>>> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>> index fa41026d272e..260b560d0075 100644
>>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
>>> @@ -2175,7 +2175,7 @@ static int arm_smmu_domain_finalise(struct
>>> iommu_domain *domain,
>>> .iommu_dev = smmu->dev,
>>> };
>>> - if (!iommu_get_dma_strict(domain))
>>> + if (domain->type == IOMMU_DOMAIN_DMA_FQ)
>>> pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
>>> pgtbl_ops = alloc_io_pgtable_ops(fmt, &pgtbl_cfg, smmu_domain);
>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
>>> b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>>> index dbc14c265b15..2c717f3be056 100644
>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>>> @@ -765,7 +765,7 @@ static int arm_smmu_init_domain_context(struct
>>> iommu_domain *domain,
>>> .iommu_dev = smmu->dev,
>>> };
>>> - if (!iommu_get_dma_strict(domain))
>>> + if (domain->type == IOMMU_DOMAIN_DMA_FQ)
>>> pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
>>> if (smmu->impl && smmu->impl->init_context) {
>>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>>> index b1af1ff324c5..a114a7ad88ec 100644
>>> --- a/drivers/iommu/dma-iommu.c
>>> +++ b/drivers/iommu/dma-iommu.c
>>> @@ -363,13 +363,15 @@ static int iommu_dma_init_domain(struct
>>> iommu_domain *domain, dma_addr_t base,
>>> init_iova_domain(iovad, 1UL << order, base_pfn);
>>> - if (!cookie->fq_domain && !dev_is_untrusted(dev) &&
>>> - domain->ops->flush_iotlb_all &&
>>> !iommu_get_dma_strict(domain)) {
>>> + if (domain->type == IOMMU_DOMAIN_DMA_FQ && !cookie->fq_domain &&
>>> + domain->ops->flush_iotlb_all) {
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
>>> if (init_iova_flush_queue(iovad, iommu_dma_flush_iotlb_all,
>>> - iommu_dma_entry_dtor))
>>> + iommu_dma_entry_dtor)) {
>>> pr_warn("iova flush queue initialization failed\n");
>>> - else
>>> + domain->type = IOMMU_DOMAIN_DMA;
>>> + } else {
>>> cookie->fq_domain = domain;
>>> + }
>>> }
>>> return iova_reserve_iommu_regions(dev, domain);
>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>>> index 8333c334891e..d7eaacae0944 100644
>>> --- a/drivers/iommu/iommu.c
>>> +++ b/drivers/iommu/iommu.c
>>> @@ -135,6 +135,9 @@ static int __init iommu_subsys_init(void)
>>> }
>>> }
>>> + if (!iommu_default_passthrough() && !iommu_dma_strict)
>>> + iommu_def_domain_type = IOMMU_DOMAIN_DMA_FQ;
>>> +
>>> pr_info("Default domain type: %s %s\n",
>>> iommu_domain_type_str(iommu_def_domain_type),
>>> (iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?
>>> @@ -352,15 +355,6 @@ void iommu_set_dma_strict(bool strict)
>>> iommu_dma_strict = strict;
>>> }
>>> -bool iommu_get_dma_strict(struct iommu_domain *domain)
>>> -{
>>> - /* only allow lazy flushing for DMA domains */
>>> - if (domain->type == IOMMU_DOMAIN_DMA)
>>> - return iommu_dma_strict;
>>> - return true;
>>> -}
>>> -EXPORT_SYMBOL_GPL(iommu_get_dma_strict);
>>> -
>>> static ssize_t iommu_group_attr_show(struct kobject *kobj,
>>> struct attribute *__attr, char *buf)
>>> {
>>> @@ -764,7 +758,7 @@ static int
>>> 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.
>
>>> return 0;
>>> BUG_ON(!domain->pgsize_bitmap);
>>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>>> index 56519110d43f..557c4c12e2cf 100644
>>> --- a/include/linux/iommu.h
>>> +++ b/include/linux/iommu.h
>>> @@ -484,7 +484,6 @@ int iommu_set_pgtable_quirks(struct iommu_domain
>>> *domain,
>>> unsigned long quirks);
>>> void iommu_set_dma_strict(bool val);
>>> -bool iommu_get_dma_strict(struct iommu_domain *domain);
>>> extern int report_iommu_fault(struct iommu_domain *domain, struct
>>> device *dev,
>>> unsigned long iova, int flags);
>>>
>>
>> Best regards,
>> baolu
On 2021-07-26 13:29, Lu Baolu wrote:
[...]
>>>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>>>> index b1af1ff324c5..a114a7ad88ec 100644
>>>> --- a/drivers/iommu/dma-iommu.c
>>>> +++ b/drivers/iommu/dma-iommu.c
>>>> @@ -363,13 +363,15 @@ static int iommu_dma_init_domain(struct
>>>> iommu_domain *domain, dma_addr_t base,
>>>> init_iova_domain(iovad, 1UL << order, base_pfn);
>>>> - if (!cookie->fq_domain && !dev_is_untrusted(dev) &&
>>>> - domain->ops->flush_iotlb_all &&
>>>> !iommu_get_dma_strict(domain)) {
>>>> + if (domain->type == IOMMU_DOMAIN_DMA_FQ && !cookie->fq_domain &&
>>>> + domain->ops->flush_iotlb_all) {
>
> 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.
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.
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
On 2021-07-26 13:46, Joerg Roedel wrote:
> 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.
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.
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