* [PATCH v5 1/5] iommu: Refactor iommu_group_store_type()
2022-02-14 17:29 [PATCH v5 0/5] iommu: Allow IOVA rcache range be configured John Garry
@ 2022-02-14 17:29 ` John Garry
2022-02-14 17:29 ` [PATCH v5 2/5] iova: Allow rcache range upper limit to be flexible John Garry
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: John Garry @ 2022-02-14 17:29 UTC (permalink / raw)
To: joro, will, robin.murphy
Cc: mst, jasowang, baolu.lu, iommu, linux-kernel, virtualization,
hch, chenxiang66, linuxarm, John Garry
Function iommu_group_store_type() supports changing the default domain
of an IOMMU group.
Many conditions need to be satisfied and steps taken for this action to be
successful.
Satisfying these conditions and steps will be required for setting other
IOMMU group attributes, so factor into a common part and a part specific
to update the IOMMU group attribute.
No functional change intended.
Some code comments are tidied up also.
Signed-off-by: John Garry <john.garry@huawei.com>
---
drivers/iommu/iommu.c | 96 ++++++++++++++++++++++++++++---------------
1 file changed, 62 insertions(+), 34 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 107dcf5938d6..5e7ed969b870 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3261,21 +3261,57 @@ static int iommu_change_dev_def_domain(struct iommu_group *group,
return ret;
}
+enum iommu_group_op {
+ CHANGE_GROUP_TYPE,
+};
+
+static int __iommu_group_store_type(const char *buf, struct iommu_group *group,
+ struct device *dev)
+{
+ int type;
+
+ if (sysfs_streq(buf, "identity"))
+ type = IOMMU_DOMAIN_IDENTITY;
+ else if (sysfs_streq(buf, "DMA"))
+ type = IOMMU_DOMAIN_DMA;
+ else if (sysfs_streq(buf, "DMA-FQ"))
+ type = IOMMU_DOMAIN_DMA_FQ;
+ else if (sysfs_streq(buf, "auto"))
+ type = 0;
+ else
+ return -EINVAL;
+
+ /*
+ * Check if the only device in the group still has a driver bound or
+ * we're transistioning from DMA -> DMA-FQ
+ */
+ if (device_is_bound(dev) && !(type == IOMMU_DOMAIN_DMA_FQ &&
+ group->default_domain->type == IOMMU_DOMAIN_DMA)) {
+ pr_err_ratelimited("Device is still bound to driver\n");
+ return -EINVAL;
+ }
+
+ return iommu_change_dev_def_domain(group, dev, type);
+}
+
/*
* 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.
+ * transition. Changing or any other IOMMU group attribute still requires the
+ * user to unbind the drivers from the devices in the iommu group. Return
+ * failure if these conditions are not 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
* will not be entered at the same time.
*/
-static ssize_t iommu_group_store_type(struct iommu_group *group,
- const char *buf, size_t count)
+static ssize_t iommu_group_store_common(struct iommu_group *group,
+ enum iommu_group_op op,
+ const char *buf, size_t count)
{
struct group_device *grp_dev;
struct device *dev;
- int ret, req_type;
+ int ret;
if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
return -EACCES;
@@ -3283,27 +3319,16 @@ static ssize_t iommu_group_store_type(struct iommu_group *group,
if (WARN_ON(!group))
return -EINVAL;
- if (sysfs_streq(buf, "identity"))
- 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
- return -EINVAL;
-
/*
* Lock/Unlock the group mutex here before device lock to
- * 1. Make sure that the iommu group has only one device (this is a
+ * 1. Make sure that the IOMMU group has only one device (this is a
* prerequisite for step 2)
* 2. Get struct *dev which is needed to lock device
*/
mutex_lock(&group->mutex);
if (iommu_group_device_count(group) != 1) {
mutex_unlock(&group->mutex);
- pr_err_ratelimited("Cannot change default domain: Group has more than one device\n");
+ pr_err_ratelimited("Cannot change IOMMU group default domain attribute: Group has more than one device\n");
return -EINVAL;
}
@@ -3315,16 +3340,16 @@ static ssize_t iommu_group_store_type(struct iommu_group *group,
/*
* Don't hold the group mutex because taking group mutex first and then
* the device lock could potentially cause a deadlock as below. Assume
- * two threads T1 and T2. T1 is trying to change default domain of an
- * iommu group and T2 is trying to hot unplug a device or release [1] VF
- * of a PCIe device which is in the same iommu group. T1 takes group
- * mutex and before it could take device lock assume T2 has taken device
- * lock and is yet to take group mutex. Now, both the threads will be
- * waiting for the other thread to release lock. Below, lock order was
- * suggested.
+ * two threads, T1 and T2. T1 is trying to change default domain
+ * attribute of an IOMMU group and T2 is trying to hot unplug a device
+ * or release [1] VF of a PCIe device which is in the same IOMMU group.
+ * T1 takes the group mutex and before it could take device lock T2 may
+ * have taken device lock and is yet to take group mutex. Now, both the
+ * threads will be waiting for the other thread to release lock. Below,
+ * lock order was suggested.
* device_lock(dev);
* mutex_lock(&group->mutex);
- * iommu_change_dev_def_domain();
+ * cb->iommu_change_dev_def_domain(); [example cb]
* mutex_unlock(&group->mutex);
* device_unlock(dev);
*
@@ -3338,21 +3363,24 @@ static ssize_t iommu_group_store_type(struct iommu_group *group,
*/
mutex_unlock(&group->mutex);
- /* Check if the device in the group still has a driver bound to it */
device_lock(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;
+ switch (op) {
+ case CHANGE_GROUP_TYPE:
+ ret = __iommu_group_store_type(buf, group, dev);
+ break;
+ default:
+ ret = -EINVAL;
}
-
- ret = iommu_change_dev_def_domain(group, dev, req_type);
ret = ret ?: count;
-out:
device_unlock(dev);
put_device(dev);
return ret;
}
+
+static ssize_t iommu_group_store_type(struct iommu_group *group,
+ const char *buf, size_t count)
+{
+ return iommu_group_store_common(group, CHANGE_GROUP_TYPE, buf, count);
+}
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v5 2/5] iova: Allow rcache range upper limit to be flexible
2022-02-14 17:29 [PATCH v5 0/5] iommu: Allow IOVA rcache range be configured John Garry
2022-02-14 17:29 ` [PATCH v5 1/5] iommu: Refactor iommu_group_store_type() John Garry
@ 2022-02-14 17:29 ` John Garry
2022-02-14 17:29 ` [PATCH v5 3/5] iommu: Allow iommu_change_dev_def_domain() realloc same default domain type John Garry
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: John Garry @ 2022-02-14 17:29 UTC (permalink / raw)
To: joro, will, robin.murphy
Cc: mst, jasowang, baolu.lu, iommu, linux-kernel, virtualization,
hch, chenxiang66, linuxarm, John Garry
Some low-level drivers may request DMA mappings whose IOVA length exceeds
that of the current rcache upper limit.
This means that allocations for those IOVAs will never be cached, and
always must be allocated and freed from the RB tree per DMA mapping cycle.
This has a significant effect on performance, more so since commit
4e89dce72521 ("iommu/iova: Retry from last rb tree node if iova search
fails"), as discussed at [0].
As a first step towards allowing the rcache range upper limit be
configured, hold this value in the IOVA rcache structure, and allocate
the rcaches separately.
Delete macro IOVA_RANGE_CACHE_MAX_SIZE in case it's reused by mistake.
[0] https://lore.kernel.org/linux-iommu/20210129092120.1482-1-thunder.leizhen@huawei.com/
Signed-off-by: John Garry <john.garry@huawei.com>
---
drivers/iommu/iova.c | 20 ++++++++++----------
include/linux/iova.h | 3 +++
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index 7e9c3a97c040..f6eb93c737cb 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -15,8 +15,6 @@
/* The anchor node sits above the top of the usable address space */
#define IOVA_ANCHOR ~0UL
-#define IOVA_RANGE_CACHE_MAX_SIZE 6 /* log of max cached IOVA range size (in pages) */
-
static bool iova_rcache_insert(struct iova_domain *iovad,
unsigned long pfn,
unsigned long size);
@@ -442,7 +440,7 @@ alloc_iova_fast(struct iova_domain *iovad, unsigned long size,
* rounding up anything cacheable to make sure that can't happen. The
* order of the unadjusted size will still match upon freeing.
*/
- if (size < (1 << (IOVA_RANGE_CACHE_MAX_SIZE - 1)))
+ if (size < (1 << (iovad->rcache_max_size - 1)))
size = roundup_pow_of_two(size);
iova_pfn = iova_rcache_get(iovad, size, limit_pfn + 1);
@@ -712,13 +710,15 @@ int iova_domain_init_rcaches(struct iova_domain *iovad)
unsigned int cpu;
int i, ret;
- iovad->rcaches = kcalloc(IOVA_RANGE_CACHE_MAX_SIZE,
+ iovad->rcache_max_size = 6; /* Arbitrarily high default */
+
+ iovad->rcaches = kcalloc(iovad->rcache_max_size,
sizeof(struct iova_rcache),
GFP_KERNEL);
if (!iovad->rcaches)
return -ENOMEM;
- for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; ++i) {
+ for (i = 0; i < iovad->rcache_max_size; ++i) {
struct iova_cpu_rcache *cpu_rcache;
struct iova_rcache *rcache;
@@ -815,7 +815,7 @@ static bool iova_rcache_insert(struct iova_domain *iovad, unsigned long pfn,
{
unsigned int log_size = order_base_2(size);
- if (log_size >= IOVA_RANGE_CACHE_MAX_SIZE)
+ if (log_size >= iovad->rcache_max_size)
return false;
return __iova_rcache_insert(iovad, &iovad->rcaches[log_size], pfn);
@@ -871,7 +871,7 @@ static unsigned long iova_rcache_get(struct iova_domain *iovad,
{
unsigned int log_size = order_base_2(size);
- if (log_size >= IOVA_RANGE_CACHE_MAX_SIZE || !iovad->rcaches)
+ if (log_size >= iovad->rcache_max_size || !iovad->rcaches)
return 0;
return __iova_rcache_get(&iovad->rcaches[log_size], limit_pfn - size);
@@ -887,7 +887,7 @@ static void free_iova_rcaches(struct iova_domain *iovad)
unsigned int cpu;
int i, j;
- for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; ++i) {
+ for (i = 0; i < iovad->rcache_max_size; ++i) {
rcache = &iovad->rcaches[i];
if (!rcache->cpu_rcaches)
break;
@@ -915,7 +915,7 @@ static void free_cpu_cached_iovas(unsigned int cpu, struct iova_domain *iovad)
unsigned long flags;
int i;
- for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; ++i) {
+ for (i = 0; i < iovad->rcache_max_size; ++i) {
rcache = &iovad->rcaches[i];
cpu_rcache = per_cpu_ptr(rcache->cpu_rcaches, cpu);
spin_lock_irqsave(&cpu_rcache->lock, flags);
@@ -934,7 +934,7 @@ static void free_global_cached_iovas(struct iova_domain *iovad)
unsigned long flags;
int i, j;
- for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; ++i) {
+ for (i = 0; i < iovad->rcache_max_size; ++i) {
rcache = &iovad->rcaches[i];
spin_lock_irqsave(&rcache->lock, flags);
for (j = 0; j < rcache->depot_size; ++j) {
diff --git a/include/linux/iova.h b/include/linux/iova.h
index 320a70e40233..02f7222fa85a 100644
--- a/include/linux/iova.h
+++ b/include/linux/iova.h
@@ -38,6 +38,9 @@ struct iova_domain {
struct iova_rcache *rcaches;
struct hlist_node cpuhp_dead;
+
+ /* log of max cached IOVA range size (in pages) */
+ unsigned long rcache_max_size;
};
static inline unsigned long iova_size(struct iova *iova)
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v5 3/5] iommu: Allow iommu_change_dev_def_domain() realloc same default domain type
2022-02-14 17:29 [PATCH v5 0/5] iommu: Allow IOVA rcache range be configured John Garry
2022-02-14 17:29 ` [PATCH v5 1/5] iommu: Refactor iommu_group_store_type() John Garry
2022-02-14 17:29 ` [PATCH v5 2/5] iova: Allow rcache range upper limit to be flexible John Garry
@ 2022-02-14 17:29 ` John Garry
2022-02-14 17:29 ` [PATCH v5 4/5] iommu: Allow max opt DMA len be set for a group via sysfs John Garry
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: John Garry @ 2022-02-14 17:29 UTC (permalink / raw)
To: joro, will, robin.murphy
Cc: mst, jasowang, baolu.lu, iommu, linux-kernel, virtualization,
hch, chenxiang66, linuxarm, John Garry
Allow iommu_change_dev_def_domain() to create a new default domain, keeping
the same as current.
Also remove comment about the function purpose, which will become stale.
Signed-off-by: John Garry <john.garry@huawei.com>
---
drivers/iommu/iommu.c | 49 ++++++++++++++++++++++---------------------
include/linux/iommu.h | 1 +
2 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 5e7ed969b870..df9ffd76c184 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3124,6 +3124,7 @@ u32 iommu_sva_get_pasid(struct iommu_sva *handle)
}
EXPORT_SYMBOL_GPL(iommu_sva_get_pasid);
+
/*
* Changes the default domain of an iommu group that has *only* one device
*
@@ -3134,10 +3135,6 @@ EXPORT_SYMBOL_GPL(iommu_sva_get_pasid);
*
* Returns 0 on success and error code on failure
*
- * Note:
- * 1. Presently, this function is called only when user requests to change the
- * group's default domain type through /sys/kernel/iommu_groups/<grp_id>/type
- * Please take a closer look if intended to use for other purposes.
*/
static int iommu_change_dev_def_domain(struct iommu_group *group,
struct device *prev_dev, int type)
@@ -3190,28 +3187,32 @@ static int iommu_change_dev_def_domain(struct iommu_group *group,
goto out;
}
- dev_def_dom = iommu_get_def_domain_type(dev);
- if (!type) {
+ if (type == __IOMMU_DOMAIN_SAME) {
+ type = prev_dom->type;
+ } else {
+ dev_def_dom = iommu_get_def_domain_type(dev);
+ if (!type) {
+ /*
+ * If the user hasn't requested any specific type of domain and
+ * if the device supports both the domains, then default to the
+ * domain the device was booted with
+ */
+ type = dev_def_dom ? : iommu_def_domain_type;
+ } else if (dev_def_dom && type != dev_def_dom) {
+ dev_err_ratelimited(prev_dev, "Device cannot be in %s domain\n",
+ iommu_domain_type_str(type));
+ ret = -EINVAL;
+ goto out;
+ }
+
/*
- * If the user hasn't requested any specific type of domain and
- * if the device supports both the domains, then default to the
- * domain the device was booted with
+ * Switch to a new domain only if the requested domain type is different
+ * from the existing default domain type
*/
- type = dev_def_dom ? : iommu_def_domain_type;
- } else if (dev_def_dom && type != dev_def_dom) {
- dev_err_ratelimited(prev_dev, "Device cannot be in %s domain\n",
- iommu_domain_type_str(type));
- ret = -EINVAL;
- goto out;
- }
-
- /*
- * Switch to a new domain only if the requested domain type is different
- * from the existing default domain type
- */
- if (prev_dom->type == type) {
- ret = 0;
- goto out;
+ if (prev_dom->type == type) {
+ ret = 0;
+ goto out;
+ }
}
/* We can bring up a flush queue without tearing down the domain */
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index de0c57a567c8..d242fccc7c2d 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -62,6 +62,7 @@ struct iommu_domain_geometry {
implementation */
#define __IOMMU_DOMAIN_PT (1U << 2) /* Domain is identity mapped */
#define __IOMMU_DOMAIN_DMA_FQ (1U << 3) /* DMA-API uses flush queue */
+#define __IOMMU_DOMAIN_SAME (1U << 4) /* Keep same type (internal) */
/*
* This are the possible domain-types
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v5 4/5] iommu: Allow max opt DMA len be set for a group via sysfs
2022-02-14 17:29 [PATCH v5 0/5] iommu: Allow IOVA rcache range be configured John Garry
` (2 preceding siblings ...)
2022-02-14 17:29 ` [PATCH v5 3/5] iommu: Allow iommu_change_dev_def_domain() realloc same default domain type John Garry
@ 2022-02-14 17:29 ` John Garry
2022-02-14 17:29 ` [PATCH v5 5/5] iova: Add iova_len argument to iova_domain_init_rcaches() John Garry
2022-02-28 9:44 ` [PATCH v5 0/5] iommu: Allow IOVA rcache range be configured John Garry
5 siblings, 0 replies; 7+ messages in thread
From: John Garry @ 2022-02-14 17:29 UTC (permalink / raw)
To: joro, will, robin.murphy
Cc: mst, jasowang, baolu.lu, iommu, linux-kernel, virtualization,
hch, chenxiang66, linuxarm, John Garry
Add support to allow the maximum optimised DMA len be set for an IOMMU
group via sysfs.
This is much the same with the method to change the default domain type
for a group.
Signed-off-by: John Garry <john.garry@huawei.com>
---
.../ABI/testing/sysfs-kernel-iommu_groups | 16 +++++
drivers/iommu/iommu.c | 59 ++++++++++++++++++-
include/linux/iommu.h | 6 ++
3 files changed, 79 insertions(+), 2 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-kernel-iommu_groups b/Documentation/ABI/testing/sysfs-kernel-iommu_groups
index b15af6a5bc08..ed6f72794f6c 100644
--- a/Documentation/ABI/testing/sysfs-kernel-iommu_groups
+++ b/Documentation/ABI/testing/sysfs-kernel-iommu_groups
@@ -63,3 +63,19 @@ Description: /sys/kernel/iommu_groups/<grp_id>/type shows the type of default
system could lead to catastrophic effects (the users might
need to reboot the machine to get it to normal state). So, it's
expected that the users understand what they're doing.
+
+What: /sys/kernel/iommu_groups/<grp_id>/max_opt_dma_size
+Date: Feb 2022
+KernelVersion: v5.18
+Contact: iommu@lists.linux-foundation.org
+Description: /sys/kernel/iommu_groups/<grp_id>/max_opt_dma_size shows the
+ max optimised DMA size for the default IOMMU domain associated
+ with the group.
+ Each IOMMU domain has an IOVA domain. The IOVA domain caches
+ IOVAs upto a certain size as a performance optimisation.
+ This sysfs file allows the range of the IOVA domain caching be
+ set, such that larger than default IOVAs may be cached.
+ A value of 0 means that the default caching range is chosen.
+ A privileged user could request the kernel the change the range
+ by writing to this file. For this to happen, the same rules
+ and procedure applies as in changing the default domain type.
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index df9ffd76c184..79f5cbea5c95 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -48,6 +48,7 @@ struct iommu_group {
struct iommu_domain *default_domain;
struct iommu_domain *domain;
struct list_head entry;
+ size_t max_opt_dma_size;
};
struct group_device {
@@ -89,6 +90,9 @@ static int iommu_create_device_direct_mappings(struct iommu_group *group,
static struct iommu_group *iommu_group_get_for_dev(struct device *dev);
static ssize_t iommu_group_store_type(struct iommu_group *group,
const char *buf, size_t count);
+static ssize_t iommu_group_store_max_opt_dma_size(struct iommu_group *group,
+ const char *buf,
+ size_t count);
#define IOMMU_GROUP_ATTR(_name, _mode, _show, _store) \
struct iommu_group_attribute iommu_group_attr_##_name = \
@@ -570,6 +574,12 @@ static ssize_t iommu_group_show_type(struct iommu_group *group,
return strlen(type);
}
+static ssize_t iommu_group_show_max_opt_dma_size(struct iommu_group *group,
+ char *buf)
+{
+ return sprintf(buf, "%zu\n", group->max_opt_dma_size);
+}
+
static IOMMU_GROUP_ATTR(name, S_IRUGO, iommu_group_show_name, NULL);
static IOMMU_GROUP_ATTR(reserved_regions, 0444,
@@ -578,6 +588,9 @@ static IOMMU_GROUP_ATTR(reserved_regions, 0444,
static IOMMU_GROUP_ATTR(type, 0644, iommu_group_show_type,
iommu_group_store_type);
+static IOMMU_GROUP_ATTR(max_opt_dma_size, 0644, iommu_group_show_max_opt_dma_size,
+ iommu_group_store_max_opt_dma_size);
+
static void iommu_group_release(struct kobject *kobj)
{
struct iommu_group *group = to_iommu_group(kobj);
@@ -664,6 +677,10 @@ struct iommu_group *iommu_group_alloc(void)
if (ret)
return ERR_PTR(ret);
+ ret = iommu_group_create_file(group, &iommu_group_attr_max_opt_dma_size);
+ if (ret)
+ return ERR_PTR(ret);
+
pr_debug("Allocated group %d\n", group->id);
return group;
@@ -2302,6 +2319,11 @@ struct iommu_domain *iommu_get_dma_domain(struct device *dev)
return dev->iommu_group->default_domain;
}
+size_t iommu_group_get_max_opt_dma_size(struct iommu_group *group)
+{
+ return group->max_opt_dma_size;
+}
+
/*
* IOMMU groups are really the natural working unit of the IOMMU, but
* the IOMMU API works on domains and devices. Bridge that gap by
@@ -3132,12 +3154,14 @@ EXPORT_SYMBOL_GPL(iommu_sva_get_pasid);
* @prev_dev: The device in the group (this is used to make sure that the device
* hasn't changed after the caller has called this function)
* @type: The type of the new default domain that gets associated with the group
+ * @max_opt_dma_size: Set the IOMMU group max_opt_dma_size if non-zero
*
* Returns 0 on success and error code on failure
*
*/
static int iommu_change_dev_def_domain(struct iommu_group *group,
- struct device *prev_dev, int type)
+ struct device *prev_dev, int type,
+ unsigned long max_opt_dma_size)
{
struct iommu_domain *prev_dom;
struct group_device *grp_dev;
@@ -3238,6 +3262,9 @@ static int iommu_change_dev_def_domain(struct iommu_group *group,
group->domain = group->default_domain;
+ if (max_opt_dma_size)
+ group->max_opt_dma_size = max_opt_dma_size;
+
/*
* Release the mutex here because ops->probe_finalize() call-back of
* some vendor IOMMU drivers calls arm_iommu_attach_device() which
@@ -3264,6 +3291,7 @@ static int iommu_change_dev_def_domain(struct iommu_group *group,
enum iommu_group_op {
CHANGE_GROUP_TYPE,
+ CHANGE_DMA_OPT_SIZE,
};
static int __iommu_group_store_type(const char *buf, struct iommu_group *group,
@@ -3292,7 +3320,24 @@ static int __iommu_group_store_type(const char *buf, struct iommu_group *group,
return -EINVAL;
}
- return iommu_change_dev_def_domain(group, dev, type);
+ return iommu_change_dev_def_domain(group, dev, type, 0);
+}
+
+static int __iommu_group_store_max_opt_dma_size(const char *buf,
+ struct iommu_group *group,
+ struct device *dev)
+{
+ unsigned long val;
+
+ if (kstrtoul(buf, 0, &val) || !val)
+ return -EINVAL;
+
+ if (device_is_bound(dev)) {
+ pr_err_ratelimited("Device is still bound to driver\n");
+ return -EINVAL;
+ }
+
+ return iommu_change_dev_def_domain(group, dev, __IOMMU_DOMAIN_SAME, val);
}
/*
@@ -3369,6 +3414,9 @@ static ssize_t iommu_group_store_common(struct iommu_group *group,
case CHANGE_GROUP_TYPE:
ret = __iommu_group_store_type(buf, group, dev);
break;
+ case CHANGE_DMA_OPT_SIZE:
+ ret = __iommu_group_store_max_opt_dma_size(buf, group, dev);
+ break;
default:
ret = -EINVAL;
}
@@ -3385,3 +3433,10 @@ static ssize_t iommu_group_store_type(struct iommu_group *group,
{
return iommu_group_store_common(group, CHANGE_GROUP_TYPE, buf, count);
}
+
+static ssize_t iommu_group_store_max_opt_dma_size(struct iommu_group *group,
+ const char *buf,
+ size_t count)
+{
+ return iommu_group_store_common(group, CHANGE_DMA_OPT_SIZE, buf, count);
+}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index d242fccc7c2d..f7f1799fb07a 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -434,6 +434,7 @@ extern int iommu_sva_unbind_gpasid(struct iommu_domain *domain,
struct device *dev, ioasid_t pasid);
extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
+extern size_t iommu_group_get_max_opt_dma_size(struct iommu_group *group);
extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
phys_addr_t paddr, size_t size, int prot);
extern int iommu_map_atomic(struct iommu_domain *domain, unsigned long iova,
@@ -732,6 +733,11 @@ static inline struct iommu_domain *iommu_get_domain_for_dev(struct device *dev)
return NULL;
}
+static inline size_t iommu_group_get_max_opt_dma_size(struct iommu_group *group)
+{
+ return 0;
+}
+
static inline int iommu_map(struct iommu_domain *domain, unsigned long iova,
phys_addr_t paddr, size_t size, int prot)
{
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v5 5/5] iova: Add iova_len argument to iova_domain_init_rcaches()
2022-02-14 17:29 [PATCH v5 0/5] iommu: Allow IOVA rcache range be configured John Garry
` (3 preceding siblings ...)
2022-02-14 17:29 ` [PATCH v5 4/5] iommu: Allow max opt DMA len be set for a group via sysfs John Garry
@ 2022-02-14 17:29 ` John Garry
2022-02-28 9:44 ` [PATCH v5 0/5] iommu: Allow IOVA rcache range be configured John Garry
5 siblings, 0 replies; 7+ messages in thread
From: John Garry @ 2022-02-14 17:29 UTC (permalink / raw)
To: joro, will, robin.murphy
Cc: mst, jasowang, baolu.lu, iommu, linux-kernel, virtualization,
hch, chenxiang66, linuxarm, John Garry
Add max opt argument to iova_domain_init_rcaches(), and use it to set the
rcaches range.
Also fix up all users to set this value (at 0, meaning use default),
including a wrapper for that, iova_domain_init_rcaches_default().
For dma-iommu.c we derive the iova_len argument from the IOMMU group
max opt DMA size.
Signed-off-by: John Garry <john.garry@huawei.com>
---
drivers/iommu/dma-iommu.c | 15 ++++++++++++++-
drivers/iommu/iova.c | 19 ++++++++++++++++---
drivers/vdpa/vdpa_user/iova_domain.c | 4 ++--
include/linux/iova.h | 3 ++-
4 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 908272f0153e..7afa2226c6bd 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -525,6 +525,8 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
struct iommu_dma_cookie *cookie = domain->iova_cookie;
unsigned long order, base_pfn;
struct iova_domain *iovad;
+ size_t max_opt_dma_size;
+ unsigned long iova_len = 0;
int ret;
if (!cookie || cookie->type != IOMMU_DMA_IOVA_COOKIE)
@@ -560,7 +562,18 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
}
init_iova_domain(iovad, 1UL << order, base_pfn);
- ret = iova_domain_init_rcaches(iovad);
+
+ max_opt_dma_size = iommu_group_get_max_opt_dma_size(dev->iommu_group);
+ if (max_opt_dma_size) {
+ unsigned long shift = __ffs(1UL << order);
+
+ iova_len = roundup_pow_of_two(max_opt_dma_size);
+ iova_len >>= shift;
+ if (!iova_len)
+ iova_len = 1;
+ }
+
+ ret = iova_domain_init_rcaches(iovad, iova_len);
if (ret)
return ret;
diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index f6eb93c737cb..f05aae044e25 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -705,12 +705,20 @@ static void iova_magazine_push(struct iova_magazine *mag, unsigned long pfn)
mag->pfns[mag->size++] = pfn;
}
-int iova_domain_init_rcaches(struct iova_domain *iovad)
+static unsigned long iova_len_to_rcache_max(unsigned long iova_len)
+{
+ return order_base_2(iova_len) + 1;
+}
+
+int iova_domain_init_rcaches(struct iova_domain *iovad, unsigned long iova_len)
{
unsigned int cpu;
int i, ret;
- iovad->rcache_max_size = 6; /* Arbitrarily high default */
+ if (iova_len)
+ iovad->rcache_max_size = iova_len_to_rcache_max(iova_len);
+ else
+ iovad->rcache_max_size = 6; /* Arbitrarily high default */
iovad->rcaches = kcalloc(iovad->rcache_max_size,
sizeof(struct iova_rcache),
@@ -754,7 +762,12 @@ int iova_domain_init_rcaches(struct iova_domain *iovad)
free_iova_rcaches(iovad);
return ret;
}
-EXPORT_SYMBOL_GPL(iova_domain_init_rcaches);
+
+int iova_domain_init_rcaches_default(struct iova_domain *iovad)
+{
+ return iova_domain_init_rcaches(iovad, 0);
+}
+EXPORT_SYMBOL_GPL(iova_domain_init_rcaches_default);
/*
* Try inserting IOVA range starting with 'iova_pfn' into 'rcache', and
diff --git a/drivers/vdpa/vdpa_user/iova_domain.c b/drivers/vdpa/vdpa_user/iova_domain.c
index 22f7d43f8a68..74471bc463ec 100644
--- a/drivers/vdpa/vdpa_user/iova_domain.c
+++ b/drivers/vdpa/vdpa_user/iova_domain.c
@@ -514,12 +514,12 @@ vduse_domain_create(unsigned long iova_limit, size_t bounce_size)
spin_lock_init(&domain->iotlb_lock);
init_iova_domain(&domain->stream_iovad,
PAGE_SIZE, IOVA_START_PFN);
- ret = iova_domain_init_rcaches(&domain->stream_iovad);
+ ret = iova_domain_init_rcaches_default(&domain->stream_iovad);
if (ret)
goto err_iovad_stream;
init_iova_domain(&domain->consistent_iovad,
PAGE_SIZE, bounce_pfns);
- ret = iova_domain_init_rcaches(&domain->consistent_iovad);
+ ret = iova_domain_init_rcaches_default(&domain->consistent_iovad);
if (ret)
goto err_iovad_consistent;
diff --git a/include/linux/iova.h b/include/linux/iova.h
index 02f7222fa85a..56281434ce0c 100644
--- a/include/linux/iova.h
+++ b/include/linux/iova.h
@@ -95,7 +95,8 @@ struct iova *reserve_iova(struct iova_domain *iovad, unsigned long pfn_lo,
unsigned long pfn_hi);
void init_iova_domain(struct iova_domain *iovad, unsigned long granule,
unsigned long start_pfn);
-int iova_domain_init_rcaches(struct iova_domain *iovad);
+int iova_domain_init_rcaches(struct iova_domain *iovad, unsigned long iova_len);
+int iova_domain_init_rcaches_default(struct iova_domain *iovad);
struct iova *find_iova(struct iova_domain *iovad, unsigned long pfn);
void put_iova_domain(struct iova_domain *iovad);
#else
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v5 0/5] iommu: Allow IOVA rcache range be configured
2022-02-14 17:29 [PATCH v5 0/5] iommu: Allow IOVA rcache range be configured John Garry
` (4 preceding siblings ...)
2022-02-14 17:29 ` [PATCH v5 5/5] iova: Add iova_len argument to iova_domain_init_rcaches() John Garry
@ 2022-02-28 9:44 ` John Garry
5 siblings, 0 replies; 7+ messages in thread
From: John Garry @ 2022-02-28 9:44 UTC (permalink / raw)
To: joro, will, robin.murphy
Cc: mst, jasowang, baolu.lu, iommu, linux-kernel, virtualization,
hch, chenxiang66, linuxarm
On 14/02/2022 17:29, John Garry wrote:
Hi guys,
And a friendly reminder on this series also.
Cheers,
john
> For streaming DMA mappings involving an IOMMU and whose IOVA len regularly
> exceeds the IOVA rcache upper limit (meaning that they are not cached),
> performance can be reduced.
>
> This may be much more pronounced from commit 4e89dce72521 ("iommu/iova:
> Retry from last rb tree node if iova search fails"), as discussed at [0].
>
> IOVAs which cannot be cached are highly involved in the IOVA ageing issue,
> as discussed at [1].
>
> This series allows the IOVA rcache range be configured, so that we may
> cache all IOVAs per domain, thus improving performance.
>
> A new IOMMU group sysfs file is added - max_opt_dma_size - which is used
> indirectly to configure the IOVA rcache range:
> /sys/kernel/iommu_groups/X/max_opt_dma_size
>
> This file is updated same as how the IOMMU group default domain type is
> updated, i.e. must unbind the only device in the group first.
>
> The inspiration here comes from block layer request queue sysfs
> "optimal_io_size" file, in /sys/block/sdX/queue/optimal_io_size
>
> Some old figures* for storage scenario (when increasing IOVA rcache range
> to cover all DMA mapping sizes from the LLD):
> v5.13-rc1 baseline: 1200K IOPS
> With series: 1800K IOPS
>
> All above are for IOMMU strict mode. Non-strict mode gives ~1800K IOPS in
> all scenarios.
>
> Based on v5.17-rc4 + [2]
> * I lost my high data throughout test setup
>
> Differences to v4:
> https://lore.kernel.org/linux-iommu/1626259003-201303-1-git-send-email-john.garry@huawei.com/
> - Major rebase
> - Change the "Refactor iommu_group_store_type()" to not use a callback
> and an op type enum instead
> - I didn't pick up Will's Ack as it has changed so much
> - Use a domain feature flag to keep same default group type
> - Add wrapper for default IOVA rcache range
> - Combine last 2x patches
>
> [0] https://lore.kernel.org/linux-iommu/20210129092120.1482-1-thunder.leizhen@huawei.com/
> [1] https://lore.kernel.org/linux-iommu/1607538189-237944-1-git-send-email-john.garry@huawei.com/
> [2] https://lore.kernel.org/linux-iommu/20220203063345-mutt-send-email-mst@kernel.org/T/#m5b2b59576d35cad544314470f32e5f40ac5d1fe9
>
> John Garry (5):
> iommu: Refactor iommu_group_store_type()
> iova: Allow rcache range upper limit to be flexible
> iommu: Allow iommu_change_dev_def_domain() realloc same default domain
> type
> iommu: Allow max opt DMA len be set for a group via sysfs
> iova: Add iova_len argument to iova_domain_init_rcaches()
>
> .../ABI/testing/sysfs-kernel-iommu_groups | 16 ++
> drivers/iommu/dma-iommu.c | 15 +-
> drivers/iommu/iommu.c | 202 +++++++++++++-----
> drivers/iommu/iova.c | 37 ++--
> drivers/vdpa/vdpa_user/iova_domain.c | 4 +-
> include/linux/iommu.h | 7 +
> include/linux/iova.h | 6 +-
> 7 files changed, 212 insertions(+), 75 deletions(-)
>
^ permalink raw reply [flat|nested] 7+ messages in thread