* [PATCH 1/6] of: create a wrapper for of_map_id()
2025-11-04 8:50 [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU Charan Teja Kalla
@ 2025-11-04 8:51 ` Charan Teja Kalla
2025-11-04 8:51 ` [PATCH 2/6] of: introduce wrapper function to query the cell count Charan Teja Kalla
` (6 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Charan Teja Kalla @ 2025-11-04 8:51 UTC (permalink / raw)
To: robin.murphy, will, joro, robh, dmitry.baryshkov, konrad.dybcio,
bjorn.andersson, bod, conor+dt, krzk+dt, saravanak, prakash.gupta,
vikash.garodia
Cc: iommu, linux-kernel, devicetree, Charan Teja Kalla
Make a new function, of_map_id_or_funcid(), which simply wraps
of_map_id(). The of_map_id_or_funcid() is improved in the subsequent
patches to parse the 'iommu-map' property based on the #iommu-cells.
No functional changes.
Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
---
drivers/of/base.c | 52 ++++++++++++++++++++++++++++-------------------
1 file changed, 31 insertions(+), 21 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 7043acd971a0..ac6b726cd5e3 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2045,28 +2045,12 @@ int of_find_last_cache_level(unsigned int cpu)
return cache_level;
}
-/**
- * of_map_id - Translate an ID through a downstream mapping.
- * @np: root complex device node.
- * @id: device ID to map.
- * @map_name: property name of the map to use.
- * @map_mask_name: optional property name of the mask to use.
- * @target: optional pointer to a target device node.
- * @id_out: optional pointer to receive the translated ID.
- *
- * Given a device ID, look up the appropriate implementation-defined
- * platform ID and/or the target device which receives transactions on that
- * ID, as per the "iommu-map" and "msi-map" bindings. Either of @target or
- * @id_out may be NULL if only the other is required. If @target points to
- * a non-NULL device node pointer, only entries targeting that node will be
- * matched; if it points to a NULL value, it will receive the device node of
- * the first matching target phandle, with a reference held.
- *
- * Return: 0 on success or a standard error code on failure.
+/*
+ * Look at the documentation of of_map_id.
*/
-int of_map_id(const struct device_node *np, u32 id,
- const char *map_name, const char *map_mask_name,
- struct device_node **target, u32 *id_out)
+static int of_map_id_or_funcid(const struct device_node *np, u32 id,
+ const char *map_name, const char *map_mask_name,
+ struct device_node **target, u32 *id_out)
{
u32 map_mask, masked_id;
int map_len;
@@ -2149,4 +2133,30 @@ int of_map_id(const struct device_node *np, u32 id,
*id_out = id;
return 0;
}
+
+/**
+ * of_map_id - Translate an ID through a downstream mapping.
+ * @np: root complex device node.
+ * @id: device ID to map.
+ * @map_name: property name of the map to use.
+ * @map_mask_name: optional property name of the mask to use.
+ * @target: optional pointer to a target device node.
+ * @id_out: optional pointer to receive the translated ID.
+ *
+ * Given a device ID, look up the appropriate implementation-defined
+ * platform ID and/or the target device which receives transactions on that
+ * ID, as per the "iommu-map" and "msi-map" bindings. Either of @target or
+ * @id_out may be NULL if only the other is required. If @target points to
+ * a non-NULL device node pointer, only entries targeting that node will be
+ * matched; if it points to a NULL value, it will receive the device node of
+ * the first matching target phandle, with a reference held.
+ *
+ * Return: 0 on success or a standard error code on failure.
+ */
+int of_map_id(const struct device_node *np, u32 id,
+ const char *map_name, const char *map_mask_name,
+ struct device_node **target, u32 *id_out)
+{
+ return of_map_id_or_funcid(np, id, map_name, map_mask_name, target, id_out);
+}
EXPORT_SYMBOL_GPL(of_map_id);
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 2/6] of: introduce wrapper function to query the cell count
2025-11-04 8:50 [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU Charan Teja Kalla
2025-11-04 8:51 ` [PATCH 1/6] of: create a wrapper for of_map_id() Charan Teja Kalla
@ 2025-11-04 8:51 ` Charan Teja Kalla
2025-11-05 17:35 ` Robin Murphy
2025-11-04 8:51 ` [PATCH 3/6] of: parse #<name>-cells property to get " Charan Teja Kalla
` (5 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Charan Teja Kalla @ 2025-11-04 8:51 UTC (permalink / raw)
To: robin.murphy, will, joro, robh, dmitry.baryshkov, konrad.dybcio,
bjorn.andersson, bod, conor+dt, krzk+dt, saravanak, prakash.gupta,
vikash.garodia
Cc: iommu, linux-kernel, devicetree, Charan Teja Kalla
Introduce the wrapper function, of_map_id_cell_count(), to query the
actual cell count that need to be considered by the of_map_id() when
used for translating the <property>-map entries. Accordingly make sure
of_map_id_or_funcid() operates on this returned cell count.
This wrapper function returns cell count as 1 thus no functional
changes.
The subsequent patches improve the logic in wrapper function to detect
the proper cell count.
Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
---
drivers/of/base.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index ac6b726cd5e3..5e76abcc7940 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2045,6 +2045,12 @@ int of_find_last_cache_level(unsigned int cpu)
return cache_level;
}
+static int of_map_id_cell_count(const __be32 *map, const char *map_name,
+ int map_len)
+{
+ return 1;
+}
+
/*
* Look at the documentation of of_map_id.
*/
@@ -2053,6 +2059,7 @@ static int of_map_id_or_funcid(const struct device_node *np, u32 id,
struct device_node **target, u32 *id_out)
{
u32 map_mask, masked_id;
+ u32 cell_count, total_cells;
int map_len;
const __be32 *map = NULL;
@@ -2068,7 +2075,13 @@ static int of_map_id_or_funcid(const struct device_node *np, u32 id,
return 0;
}
- if (!map_len || map_len % (4 * sizeof(*map))) {
+ cell_count = of_map_id_cell_count(map, map_name, map_len);
+ if (!cell_count)
+ return -EINVAL;
+
+ total_cells = 2 + cell_count + 1;
+
+ if (!map_len || map_len % (total_cells * sizeof(*map))) {
pr_err("%pOF: Error: Bad %s length: %d\n", np,
map_name, map_len);
return -EINVAL;
@@ -2085,12 +2098,12 @@ static int of_map_id_or_funcid(const struct device_node *np, u32 id,
of_property_read_u32(np, map_mask_name, &map_mask);
masked_id = map_mask & id;
- for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) {
+ for ( ; map_len > 0; map_len -= total_cells * sizeof(*map), map += total_cells) {
struct device_node *phandle_node;
u32 id_base = be32_to_cpup(map + 0);
u32 phandle = be32_to_cpup(map + 1);
u32 out_base = be32_to_cpup(map + 2);
- u32 id_len = be32_to_cpup(map + 3);
+ u32 id_len = be32_to_cpup(map + total_cells - 1);
if (id_base & ~map_mask) {
pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores id-base (0x%x)\n",
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 2/6] of: introduce wrapper function to query the cell count
2025-11-04 8:51 ` [PATCH 2/6] of: introduce wrapper function to query the cell count Charan Teja Kalla
@ 2025-11-05 17:35 ` Robin Murphy
0 siblings, 0 replies; 23+ messages in thread
From: Robin Murphy @ 2025-11-05 17:35 UTC (permalink / raw)
To: Charan Teja Kalla, will, joro, robh, dmitry.baryshkov,
konrad.dybcio, bjorn.andersson, bod, conor+dt, krzk+dt, saravanak,
prakash.gupta, vikash.garodia
Cc: iommu, linux-kernel, devicetree
On 2025-11-04 8:51 am, Charan Teja Kalla wrote:
> Introduce the wrapper function, of_map_id_cell_count(), to query the
> actual cell count that need to be considered by the of_map_id() when
> used for translating the <property>-map entries. Accordingly make sure
> of_map_id_or_funcid() operates on this returned cell count.
>
> This wrapper function returns cell count as 1 thus no functional
> changes.
>
> The subsequent patches improve the logic in wrapper function to detect
> the proper cell count.
>
> Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
> ---
> drivers/of/base.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index ac6b726cd5e3..5e76abcc7940 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2045,6 +2045,12 @@ int of_find_last_cache_level(unsigned int cpu)
> return cache_level;
> }
>
> +static int of_map_id_cell_count(const __be32 *map, const char *map_name,
> + int map_len)
> +{
> + return 1;
> +}
> +
> /*
> * Look at the documentation of of_map_id.
> */
> @@ -2053,6 +2059,7 @@ static int of_map_id_or_funcid(const struct device_node *np, u32 id,
> struct device_node **target, u32 *id_out)
> {
> u32 map_mask, masked_id;
> + u32 cell_count, total_cells;
> int map_len;
> const __be32 *map = NULL;
>
> @@ -2068,7 +2075,13 @@ static int of_map_id_or_funcid(const struct device_node *np, u32 id,
> return 0;
> }
>
> - if (!map_len || map_len % (4 * sizeof(*map))) {
> + cell_count = of_map_id_cell_count(map, map_name, map_len);
This doesn't work. The output cell count for any given entry depends on
whatever phandle *that* entry maps to - it can't be assumed to be
constant for the whole map.
Thanks,
Robin.
> + if (!cell_count)
> + return -EINVAL;
> +
> + total_cells = 2 + cell_count + 1;
> +
> + if (!map_len || map_len % (total_cells * sizeof(*map))) {
> pr_err("%pOF: Error: Bad %s length: %d\n", np,
> map_name, map_len);
> return -EINVAL;
> @@ -2085,12 +2098,12 @@ static int of_map_id_or_funcid(const struct device_node *np, u32 id,
> of_property_read_u32(np, map_mask_name, &map_mask);
>
> masked_id = map_mask & id;
> - for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) {
> + for ( ; map_len > 0; map_len -= total_cells * sizeof(*map), map += total_cells) {
> struct device_node *phandle_node;
> u32 id_base = be32_to_cpup(map + 0);
> u32 phandle = be32_to_cpup(map + 1);
> u32 out_base = be32_to_cpup(map + 2);
> - u32 id_len = be32_to_cpup(map + 3);
> + u32 id_len = be32_to_cpup(map + total_cells - 1);
>
> if (id_base & ~map_mask) {
> pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores id-base (0x%x)\n",
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 3/6] of: parse #<name>-cells property to get the cell count
2025-11-04 8:50 [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU Charan Teja Kalla
2025-11-04 8:51 ` [PATCH 1/6] of: create a wrapper for of_map_id() Charan Teja Kalla
2025-11-04 8:51 ` [PATCH 2/6] of: introduce wrapper function to query the cell count Charan Teja Kalla
@ 2025-11-04 8:51 ` Charan Teja Kalla
2025-11-04 8:51 ` [PATCH 4/6] of: detect and handle legacy iommu-map parsing Charan Teja Kalla
` (4 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Charan Teja Kalla @ 2025-11-04 8:51 UTC (permalink / raw)
To: robin.murphy, will, joro, robh, dmitry.baryshkov, konrad.dybcio,
bjorn.andersson, bod, conor+dt, krzk+dt, saravanak, prakash.gupta,
vikash.garodia
Cc: iommu, linux-kernel, devicetree, Charan Teja Kalla
Implement the generic function, of_map_id_cell_count(), that can
parse and return the number of cells represented by #<name>-cells,
eg: #msi-cells, #iommu-cells.
Since the second argument of a property that is parsed by the
of_map_id() is always phandle, that represents a controller, probe
it to get the #<name>-cells property.
Further patches properly consume cell count value returned by this
wrapper and this patch just returns 1 irrespective of cells value, thus
no functional changes.
Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
---
drivers/of/base.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 5e76abcc7940..e5ba8a318769 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2045,9 +2045,43 @@ int of_find_last_cache_level(unsigned int cpu)
return cache_level;
}
+/**
+ * of_map_id_cell_count - parse the cell count.
+ *
+ * @map: pointer to the property data that needs to be translated, eg: msi-map/iommu-map.
+ * @map_name: name to identify the details of @map.
+ * @map_len: length of @map in bytes.
+ *
+ * Return: number of cells that the caller should be considered while parsing
+ * the @map. It is > 0 for success, 0 for failure.
+ */
static int of_map_id_cell_count(const __be32 *map, const char *map_name,
int map_len)
{
+ const char *cells_prop_name __free(kfree) = NULL;
+ struct device_node *node __free(device_node) = NULL;
+ u32 cells;
+ int ret;
+
+ /* map + 1 is a controller. */
+ node = of_find_node_by_phandle(be32_to_cpup(map + 1));
+ if (!node) {
+ pr_err("Failed to find controller node from phandle\n");
+ return 0;
+ }
+
+ /* get #<name>-cells property from #<name>-map */
+ cells_prop_name = kasprintf(GFP_KERNEL, "#%.*scells",
+ (int)strlen(map_name) - 3, map_name);
+ if (!cells_prop_name)
+ return 0;
+
+ ret = of_property_read_u32(node, cells_prop_name, &cells);
+ if (ret) {
+ pr_err("%pOF: Failed to read %s property\n", node, cells_prop_name);
+ return 0;
+ }
+
return 1;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 4/6] of: detect and handle legacy iommu-map parsing
2025-11-04 8:50 [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU Charan Teja Kalla
` (2 preceding siblings ...)
2025-11-04 8:51 ` [PATCH 3/6] of: parse #<name>-cells property to get " Charan Teja Kalla
@ 2025-11-04 8:51 ` Charan Teja Kalla
2025-11-04 8:51 ` [PATCH 5/6] of: add infra to parse iommu-map per IOMMU cell count Charan Teja Kalla
` (3 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Charan Teja Kalla @ 2025-11-04 8:51 UTC (permalink / raw)
To: robin.murphy, will, joro, robh, dmitry.baryshkov, konrad.dybcio,
bjorn.andersson, bod, conor+dt, krzk+dt, saravanak, prakash.gupta,
vikash.garodia
Cc: iommu, linux-kernel, devicetree, Charan Teja Kalla
of_map_id currently ignores the value of #property-cells and assumes
it's always 1. iommu-map is one of the two currently known users (the
other being msi-map where such issues haven't showed up *so far*).
The arm-smmu-v2 bindings (as an example) allow 2 cells, leading to
the map containing incomplete data.
There have long existed DTs ignoring this loss, where the map only
contained the first cell, so care is taken to try and preserve
compatibility with these - the format for such occurences is as follows:
iommu-map = <FUNC_ID1 &iommu_provider cell1 length>,
<FUNC_ID2 &iommu_provider cell1 length>;
If this 'legacy' scheme is used, we can check whether there's exactly
4n entries in the array and if the second entry in each group of 4 is
(the same) phandle and length = 1[1]. The only outstanding case would be
if the iommu-map references two separate iommu providers, but that's
highly unlikely.
Implement the above logic to make passing more than one cell possible.
[1] https://lore.kernel.org/all/8d88cd9d-16e8-43f9-8eb3-89862da1d0c1@arm.com/
Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
---
drivers/of/base.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 58 insertions(+), 1 deletion(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index e5ba8a318769..997b2deb96f0 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2045,6 +2045,60 @@ int of_find_last_cache_level(unsigned int cpu)
return cache_level;
}
+/**
+ * of_iommu_map_id_legacy_cell_count - Determine the cell count for iommu-map.
+ *
+ * @map: pointer to the iommu-map property data that needs to be translated.
+ * @map_len: length of @map in bytes.
+ * @cell_count: value of #iommu-cells.
+ *
+ * It is the legacy case where the tuple representing iommu-map is always
+ * multiple of 4 and #iommu-cells > 1.
+ *
+ * Return: > 0 for success, 0 for failure.
+ */
+static u32 of_iommu_map_id_legacy_cell_count(const __be32 *map, int map_len)
+{
+ u32 phandle1, phandle2;
+
+ phandle1 = be32_to_cpup(map + 1);
+
+ for (map_len -= 4 * sizeof(*map), map += 4; map_len > 0;
+ map_len -= 4 * sizeof(*map), map += 4) {
+ u32 len;
+
+ phandle2 = be32_to_cpup(map + 1);
+ len = be32_to_cpup(map + 3);
+ if (phandle1 != phandle2 || len != 1)
+ break;
+ }
+
+ if (!map_len)
+ return 1;
+
+ return 0;
+}
+
+/**
+ * of_iommu_map_id_cell_count - Determine the cell count for iommu-map parsing.
+ *
+ * @map: pointer to the iommu-map property that needs to be translated.
+ * @map_len: length of @map in bytes.
+ *
+ * Use #iommu-cells property while parsing iommu-map. Detect and use legacy
+ * case where iommu-map is parsed as if cells = 1.
+ *
+ * Return: number of cells that the caller should be considered while parsing
+ * the @map. It is > 0 for success, 0 for failure.
+ */
+static int of_iommu_map_id_cell_count(const __be32 *map, int map_len)
+{
+ if (map_len % 4 != 0)
+ return 0;
+
+ return of_iommu_map_id_legacy_cell_count(map, map_len);
+}
+
/**
* of_map_id_cell_count - parse the cell count.
*
@@ -2082,7 +2136,10 @@ static int of_map_id_cell_count(const __be32 *map, const char *map_name,
return 0;
}
- return 1;
+ if (cells > 1 && !strcmp(map_name, "iommu-map"))
+ return of_iommu_map_id_cell_count(map, map_len);
+
+ return cells;
}
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 5/6] of: add infra to parse iommu-map per IOMMU cell count
2025-11-04 8:50 [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU Charan Teja Kalla
` (3 preceding siblings ...)
2025-11-04 8:51 ` [PATCH 4/6] of: detect and handle legacy iommu-map parsing Charan Teja Kalla
@ 2025-11-04 8:51 ` Charan Teja Kalla
2025-11-04 10:46 ` kernel test robot
` (2 more replies)
2025-11-04 8:51 ` [PATCH 6/6] of: use correct iommu-map parsing logic from of_iommu layer Charan Teja Kalla
` (2 subsequent siblings)
7 siblings, 3 replies; 23+ messages in thread
From: Charan Teja Kalla @ 2025-11-04 8:51 UTC (permalink / raw)
To: robin.murphy, will, joro, robh, dmitry.baryshkov, konrad.dybcio,
bjorn.andersson, bod, conor+dt, krzk+dt, saravanak, prakash.gupta,
vikash.garodia
Cc: iommu, linux-kernel, devicetree, Charan Teja Kalla
Add support to parse the iommu-map per the IOMMU specifer defined cell
length, defined in IOMMU controller.
When cell count is more than 1, it is generally complex to assume any
relation between input id and output specifier, thus it is invalid for
len > 1. And for such cases where linear relation is not possible, pass
the arguments without any translation to the iommu layer.
when IOMMU cell count is > 1, iommu-map is represented as:
<rid &iommu cell0 ... celln len>, where len == 1.
To add this infra, of_map_id_or_funcid() takes additional arguments,
callback function and arguments to it, and of_map_id() passes NULL
to these arguments thus callers of of_map_id() are not effected.
Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
---
drivers/of/base.c | 131 +++++++++++++++++++++++++++++++++++++++++----
include/linux/of.h | 4 ++
2 files changed, 124 insertions(+), 11 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 997b2deb96f0..cd221c003dd5 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2045,6 +2045,35 @@ int of_find_last_cache_level(unsigned int cpu)
return cache_level;
}
+/**
+ * of_iommu_map_id_actual_cell_count - determine the cell count for iommu-map.
+ *
+ * @map: pointer to the iommu-map property data that needs to be translated.
+ * @map_len: length of @map in bytes.
+ * @cell_count: value of #iommu-cells.
+ *
+ * The tuple represnting the iommu-map is in the format of
+ * 2 + cell_count(of #iommu-cells) + 1.
+ *
+ * Return: > 0 for success, 0 for failure.
+ */
+static u32 of_iommu_map_id_actual_cell_count(const __be32 *map, int map_len, u32 cell_count)
+{
+ u32 total_cell_count = 2 + cell_count + 1;
+
+ if (map_len % total_cell_count != 0)
+ return 0;
+
+ for (; map_len > 0; map_len -= total_cell_count * sizeof(*map), map += total_cell_count)
+ if (be32_to_cpup(map + total_cell_count - 1) != 1)
+ break;
+
+ if (map_len <= 0)
+ return cell_count;
+
+ return 0;
+}
+
/**
* of_iommu_map_id_legacy_cell_count - Determine the cell count for iommu-map.
*
@@ -2091,12 +2120,14 @@ static u32 of_iommu_map_id_legacy_cell_count(const __be32 *map, int map_len)
* Return: number of cells that the caller should be considered while parsing
* the @map. It is > 0 for success, 0 for failure.
*/
-static int of_iommu_map_id_cell_count(const __be32 *map, int map_len)
+static int of_iommu_map_id_cell_count(const __be32 *map, int map_len, u32 cells)
{
+ u32 legacy_iommu_cells;
+
if (map_len % 4 != 0)
- return 0;
+ legacy_iommu_cells = of_iommu_map_id_legacy_cell_count(map, map_len);
- return of_iommu_map_id_legacy_cell_count(map, map_len);
+ return legacy_iommu_cells ? : of_iommu_map_id_actual_cell_count(map, map_len, cells);
}
/**
@@ -2137,22 +2168,82 @@ static int of_map_id_cell_count(const __be32 *map, const char *map_name,
}
if (cells > 1 && !strcmp(map_name, "iommu-map"))
- return of_iommu_map_id_cell_count(map, map_len);
+ return of_iommu_map_id_cell_count(map, map_len, cells);
return cells;
}
-/*
- * Look at the documentation of of_map_id.
+/**
+ * of_map_id_untranslated - handle mapping of id that can't be translated.
+ *
+ * @map: pointer to the property data that needs to be translated, eg: msi-map/iommu-map.
+ * @cell_count: cell_count related to @map property.
+ * @id: input id that is given for translation.
+ * @arg: argument passed to the @fn.
+ * @pargs: filled by the contents of @map and pass to the @fn.
+ * @fn: Callback function that operated on @dev and @fn.
+ *
+ * The function populates the phandle args structure with the node and
+ * specifier cells, then invokes the callback function. The callback is
+ * responsible for releasing the node reference via of_node_put().
+ *
+ * Return: 0 on success, -EAGAIN, by fn() if ID doesn't match (continue searching),
+ * and standard error code on failure.
+ */
+static int of_map_id_untranslated(const __be32 *map, u32 cell_count, u32 id,
+ void *arg, struct of_phandle_args *pargs,
+ of_map_id_cb fn)
+{
+ struct device_node *phandle_node;
+ u32 id_base = be32_to_cpup(map + 0);
+ u32 phandle = be32_to_cpup(map + 1);
+ int ret, i;
+
+ if (!pargs || !fn)
+ return -EINVAL;
+
+ phandle_node = of_find_node_by_phandle(phandle);
+ if (!phandle_node)
+ return -ENODEV;
+
+ pargs->np = phandle_node;
+ pargs->args_count = cell_count;
+ for (i = 0; i < cell_count; ++i)
+ pargs->args[i] = be32_to_cpup(map + 2 + i);
+
+ /* fn() is responsible for calling of_node_put(pargs->np). */
+ ret = fn(id, id_base, arg, pargs);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+/**
+ * of_map_id_or_funcid - map an id through downstream mapping.
+ *
+ * @arg - optional argument and is passed onto @fn.
+ * @pargs - optional argument and is passed onto @fn.
+ * @fn - optional argument and is callback.
+ * Look at the documentation of of_map_id() for additional info.
+ *
+ * This is a wrapper function that includes iommu-map functionality,
+ * IOMMU specifier of which contains #iommu-cells > 1, i.e., tuple
+ * length that can be parsed by of_map_id() is > 4.
+ *
+ * Return: 0 on success or a standard error code on failure.
*/
static int of_map_id_or_funcid(const struct device_node *np, u32 id,
const char *map_name, const char *map_mask_name,
- struct device_node **target, u32 *id_out)
+ struct device_node **target, u32 *id_out,
+ void *arg, struct of_phandle_args *pargs,
+ of_map_id_cb fn)
{
u32 map_mask, masked_id;
u32 cell_count, total_cells;
int map_len;
const __be32 *map = NULL;
+ bool mapped_id_or_funcid = false;
if (!np || !map_name || (!target && !id_out))
return -EINVAL;
@@ -2163,7 +2254,7 @@ static int of_map_id_or_funcid(const struct device_node *np, u32 id,
return -ENODEV;
/* Otherwise, no map implies no translation */
*id_out = id;
- return 0;
+ goto bypass_or_callback;
}
cell_count = of_map_id_cell_count(map, map_name, map_len);
@@ -2195,6 +2286,15 @@ static int of_map_id_or_funcid(const struct device_node *np, u32 id,
u32 phandle = be32_to_cpup(map + 1);
u32 out_base = be32_to_cpup(map + 2);
u32 id_len = be32_to_cpup(map + total_cells - 1);
+ int ret;
+
+ if (cell_count > 1) {
+ ret = of_map_id_untranslated(map, cell_count, id, arg, pargs, fn);
+ if (ret && ret != -EAGAIN)
+ return ret;
+ mapped_id_or_funcid = true;
+ continue;
+ }
if (id_base & ~map_mask) {
pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores id-base (0x%x)\n",
@@ -2226,7 +2326,13 @@ static int of_map_id_or_funcid(const struct device_node *np, u32 id,
pr_debug("%pOF: %s, using mask %08x, id-base: %08x, out-base: %08x, length: %08x, id: %08x -> %08x\n",
np, map_name, map_mask, id_base, out_base,
id_len, id, masked_id - id_base + out_base);
- return 0;
+ goto bypass_or_callback;
+ }
+
+ if (cell_count > 1) {
+ if (mapped_id_or_funcid)
+ return 0;
+ return -EINVAL;
}
pr_info("%pOF: no %s translation for id 0x%x on %pOF\n", np, map_name,
@@ -2235,7 +2341,9 @@ static int of_map_id_or_funcid(const struct device_node *np, u32 id,
/* Bypasses translation */
if (id_out)
*id_out = id;
- return 0;
+
+bypass_or_callback:
+ return fn ? fn(id, id, arg, pargs) : 0;
}
/**
@@ -2261,6 +2369,7 @@ int of_map_id(const struct device_node *np, u32 id,
const char *map_name, const char *map_mask_name,
struct device_node **target, u32 *id_out)
{
- return of_map_id_or_funcid(np, id, map_name, map_mask_name, target, id_out);
+ return of_map_id_or_funcid(np, id, map_name, map_mask_name, target, id_out,
+ NULL, NULL, NULL);
}
EXPORT_SYMBOL_GPL(of_map_id);
diff --git a/include/linux/of.h b/include/linux/of.h
index a62154aeda1b..f8d976205ff4 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -22,8 +22,12 @@
#include <asm/byteorder.h>
+struct of_phandle_args;
+
typedef u32 phandle;
typedef u32 ihandle;
+typedef int (*of_map_id_cb)(u32 id, u32 id_base, void *arg,
+ struct of_phandle_args *pargs);
struct property {
char *name;
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 5/6] of: add infra to parse iommu-map per IOMMU cell count
2025-11-04 8:51 ` [PATCH 5/6] of: add infra to parse iommu-map per IOMMU cell count Charan Teja Kalla
@ 2025-11-04 10:46 ` kernel test robot
2025-11-04 10:57 ` Charan Teja Kalla
2025-11-04 13:02 ` kernel test robot
2025-11-06 15:03 ` Dan Carpenter
2 siblings, 1 reply; 23+ messages in thread
From: kernel test robot @ 2025-11-04 10:46 UTC (permalink / raw)
To: Charan Teja Kalla, robin.murphy, will, joro, robh,
dmitry.baryshkov, konrad.dybcio, bjorn.andersson, bod, conor+dt,
krzk+dt, saravanak, prakash.gupta, vikash.garodia
Cc: llvm, oe-kbuild-all, iommu, linux-kernel, devicetree,
Charan Teja Kalla
Hi Charan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v6.18-rc4 next-20251103]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Charan-Teja-Kalla/of-create-a-wrapper-for-of_map_id/20251104-170223
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/74d4ddf90c0fb485fda1feec5116dc38e5fd23cf.1762235099.git.charan.kalla%40oss.qualcomm.com
patch subject: [PATCH 5/6] of: add infra to parse iommu-map per IOMMU cell count
config: loongarch-allnoconfig (https://download.01.org/0day-ci/archive/20251104/202511041853.IxYgvWlc-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project d2625a438020ad35330cda29c3def102c1687b1b)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251104/202511041853.IxYgvWlc-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511041853.IxYgvWlc-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/of/base.c:2127:6: warning: variable 'legacy_iommu_cells' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
2127 | if (map_len % 4 != 0)
| ^~~~~~~~~~~~~~~~
drivers/of/base.c:2130:9: note: uninitialized use occurs here
2130 | return legacy_iommu_cells ? : of_iommu_map_id_actual_cell_count(map, map_len, cells);
| ^~~~~~~~~~~~~~~~~~
drivers/of/base.c:2127:2: note: remove the 'if' if its condition is always true
2127 | if (map_len % 4 != 0)
| ^~~~~~~~~~~~~~~~~~~~~
2128 | legacy_iommu_cells = of_iommu_map_id_legacy_cell_count(map, map_len);
drivers/of/base.c:2125:24: note: initialize the variable 'legacy_iommu_cells' to silence this warning
2125 | u32 legacy_iommu_cells;
| ^
| = 0
1 warning generated.
vim +2127 drivers/of/base.c
c74d6f4acf99ef Charan Teja Kalla 2025-11-04 2110
c74d6f4acf99ef Charan Teja Kalla 2025-11-04 2111 /**
c74d6f4acf99ef Charan Teja Kalla 2025-11-04 2112 * of_iommu_map_id_cell_count - Determine the cell count for iommu-map parsing.
c74d6f4acf99ef Charan Teja Kalla 2025-11-04 2113 *
c74d6f4acf99ef Charan Teja Kalla 2025-11-04 2114 * @map: pointer to the iommu-map property that needs to be translated.
c74d6f4acf99ef Charan Teja Kalla 2025-11-04 2115 * @map_len: length of @map in bytes.
c74d6f4acf99ef Charan Teja Kalla 2025-11-04 2116 *
c74d6f4acf99ef Charan Teja Kalla 2025-11-04 2117 * Use #iommu-cells property while parsing iommu-map. Detect and use legacy
c74d6f4acf99ef Charan Teja Kalla 2025-11-04 2118 * case where iommu-map is parsed as if cells = 1.
c74d6f4acf99ef Charan Teja Kalla 2025-11-04 2119 *
c74d6f4acf99ef Charan Teja Kalla 2025-11-04 2120 * Return: number of cells that the caller should be considered while parsing
c74d6f4acf99ef Charan Teja Kalla 2025-11-04 2121 * the @map. It is > 0 for success, 0 for failure.
c74d6f4acf99ef Charan Teja Kalla 2025-11-04 2122 */
5288ad2aeeeb41 Charan Teja Kalla 2025-11-04 2123 static int of_iommu_map_id_cell_count(const __be32 *map, int map_len, u32 cells)
c74d6f4acf99ef Charan Teja Kalla 2025-11-04 2124 {
5288ad2aeeeb41 Charan Teja Kalla 2025-11-04 2125 u32 legacy_iommu_cells;
5288ad2aeeeb41 Charan Teja Kalla 2025-11-04 2126
c74d6f4acf99ef Charan Teja Kalla 2025-11-04 @2127 if (map_len % 4 != 0)
5288ad2aeeeb41 Charan Teja Kalla 2025-11-04 2128 legacy_iommu_cells = of_iommu_map_id_legacy_cell_count(map, map_len);
c74d6f4acf99ef Charan Teja Kalla 2025-11-04 2129
5288ad2aeeeb41 Charan Teja Kalla 2025-11-04 2130 return legacy_iommu_cells ? : of_iommu_map_id_actual_cell_count(map, map_len, cells);
c74d6f4acf99ef Charan Teja Kalla 2025-11-04 2131 }
c74d6f4acf99ef Charan Teja Kalla 2025-11-04 2132
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 5/6] of: add infra to parse iommu-map per IOMMU cell count
2025-11-04 10:46 ` kernel test robot
@ 2025-11-04 10:57 ` Charan Teja Kalla
0 siblings, 0 replies; 23+ messages in thread
From: Charan Teja Kalla @ 2025-11-04 10:57 UTC (permalink / raw)
To: kernel test robot, robin.murphy, will, joro, robh,
dmitry.baryshkov, konrad.dybcio, bjorn.andersson, bod, conor+dt,
krzk+dt, saravanak, prakash.gupta, vikash.garodia
Cc: llvm, oe-kbuild-all, iommu, linux-kernel, devicetree
On 11/4/2025 4:16 PM, kernel test robot wrote:
>>> drivers/of/base.c:2127:6: warning: variable 'legacy_iommu_cells' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
> 2127 | if (map_len % 4 != 0)
> | ^~~~~~~~~~~~~~~~
> drivers/of/base.c:2130:9: note: uninitialized use occurs here
> 2130 | return legacy_iommu_cells ? : of_iommu_map_id_actual_cell_count(map, map_len, cells);
> | ^~~~~~~~~~~~~~~~~~
> drivers/of/base.c:2127:2: note: remove the 'if' if its condition is always true
> 2127 | if (map_len % 4 != 0)
> | ^~~~~~~~~~~~~~~~~~~~~
> 2128 | legacy_iommu_cells = of_iommu_map_id_legacy_cell_count(map, map_len);
> drivers/of/base.c:2125:24: note: initialize the variable 'legacy_iommu_cells' to silence this warning
> 2125 | u32 legacy_iommu_cells;
> | ^
> | = 0
> 1 warning generated.
Will wait for additional comments and take care of this in the next spin.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 5/6] of: add infra to parse iommu-map per IOMMU cell count
2025-11-04 8:51 ` [PATCH 5/6] of: add infra to parse iommu-map per IOMMU cell count Charan Teja Kalla
2025-11-04 10:46 ` kernel test robot
@ 2025-11-04 13:02 ` kernel test robot
2025-11-06 15:03 ` Dan Carpenter
2 siblings, 0 replies; 23+ messages in thread
From: kernel test robot @ 2025-11-04 13:02 UTC (permalink / raw)
To: Charan Teja Kalla, robin.murphy, will, joro, robh,
dmitry.baryshkov, konrad.dybcio, bjorn.andersson, bod, conor+dt,
krzk+dt, saravanak, prakash.gupta, vikash.garodia
Cc: oe-kbuild-all, iommu, linux-kernel, devicetree, Charan Teja Kalla
Hi Charan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v6.18-rc4 next-20251104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Charan-Teja-Kalla/of-create-a-wrapper-for-of_map_id/20251104-170223
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/74d4ddf90c0fb485fda1feec5116dc38e5fd23cf.1762235099.git.charan.kalla%40oss.qualcomm.com
patch subject: [PATCH 5/6] of: add infra to parse iommu-map per IOMMU cell count
config: microblaze-allnoconfig (https://download.01.org/0day-ci/archive/20251104/202511042005.MXfsNx3W-lkp@intel.com/config)
compiler: microblaze-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251104/202511042005.MXfsNx3W-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511042005.MXfsNx3W-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> Warning: drivers/of/base.c:2123 function parameter 'cells' not described in 'of_iommu_map_id_cell_count'
>> Warning: drivers/of/base.c:2240 function parameter 'np' not described in 'of_map_id_or_funcid'
>> Warning: drivers/of/base.c:2240 function parameter 'id' not described in 'of_map_id_or_funcid'
>> Warning: drivers/of/base.c:2240 function parameter 'map_name' not described in 'of_map_id_or_funcid'
>> Warning: drivers/of/base.c:2240 function parameter 'map_mask_name' not described in 'of_map_id_or_funcid'
>> Warning: drivers/of/base.c:2240 function parameter 'target' not described in 'of_map_id_or_funcid'
>> Warning: drivers/of/base.c:2240 function parameter 'id_out' not described in 'of_map_id_or_funcid'
>> Warning: drivers/of/base.c:2240 function parameter 'arg' not described in 'of_map_id_or_funcid'
>> Warning: drivers/of/base.c:2240 function parameter 'pargs' not described in 'of_map_id_or_funcid'
>> Warning: drivers/of/base.c:2240 function parameter 'fn' not described in 'of_map_id_or_funcid'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 5/6] of: add infra to parse iommu-map per IOMMU cell count
2025-11-04 8:51 ` [PATCH 5/6] of: add infra to parse iommu-map per IOMMU cell count Charan Teja Kalla
2025-11-04 10:46 ` kernel test robot
2025-11-04 13:02 ` kernel test robot
@ 2025-11-06 15:03 ` Dan Carpenter
2 siblings, 0 replies; 23+ messages in thread
From: Dan Carpenter @ 2025-11-06 15:03 UTC (permalink / raw)
To: oe-kbuild, Charan Teja Kalla, robin.murphy, will, joro, robh,
dmitry.baryshkov, konrad.dybcio, bjorn.andersson, bod, conor+dt,
krzk+dt, saravanak, prakash.gupta, vikash.garodia
Cc: lkp, oe-kbuild-all, iommu, linux-kernel, devicetree,
Charan Teja Kalla
Hi Charan,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Charan-Teja-Kalla/of-create-a-wrapper-for-of_map_id/20251104-170223
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/74d4ddf90c0fb485fda1feec5116dc38e5fd23cf.1762235099.git.charan.kalla%40oss.qualcomm.com
patch subject: [PATCH 5/6] of: add infra to parse iommu-map per IOMMU cell count
config: hexagon-randconfig-r072-20251105 (https://download.01.org/0day-ci/archive/20251105/202511051256.fSouTFuY-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project d2625a438020ad35330cda29c3def102c1687b1b)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202511051256.fSouTFuY-lkp@intel.com/
smatch warnings:
drivers/of/base.c:2130 of_iommu_map_id_cell_count() error: uninitialized symbol 'legacy_iommu_cells'.
vim +/legacy_iommu_cells +2130 drivers/of/base.c
5288ad2aeeeb419 Charan Teja Kalla 2025-11-04 2123 static int of_iommu_map_id_cell_count(const __be32 *map, int map_len, u32 cells)
c74d6f4acf99ef3 Charan Teja Kalla 2025-11-04 2124 {
5288ad2aeeeb419 Charan Teja Kalla 2025-11-04 2125 u32 legacy_iommu_cells;
5288ad2aeeeb419 Charan Teja Kalla 2025-11-04 2126
c74d6f4acf99ef3 Charan Teja Kalla 2025-11-04 2127 if (map_len % 4 != 0)
5288ad2aeeeb419 Charan Teja Kalla 2025-11-04 2128 legacy_iommu_cells = of_iommu_map_id_legacy_cell_count(map, map_len);
legacy_iommu_cells uninitialize on else path
c74d6f4acf99ef3 Charan Teja Kalla 2025-11-04 2129
5288ad2aeeeb419 Charan Teja Kalla 2025-11-04 @2130 return legacy_iommu_cells ? : of_iommu_map_id_actual_cell_count(map, map_len, cells);
c74d6f4acf99ef3 Charan Teja Kalla 2025-11-04 2131 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 6/6] of: use correct iommu-map parsing logic from of_iommu layer
2025-11-04 8:50 [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU Charan Teja Kalla
` (4 preceding siblings ...)
2025-11-04 8:51 ` [PATCH 5/6] of: add infra to parse iommu-map per IOMMU cell count Charan Teja Kalla
@ 2025-11-04 8:51 ` Charan Teja Kalla
2025-11-05 18:20 ` Robin Murphy
2025-11-05 17:28 ` [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU Robin Murphy
2025-11-07 8:07 ` Krzysztof Kozlowski
7 siblings, 1 reply; 23+ messages in thread
From: Charan Teja Kalla @ 2025-11-04 8:51 UTC (permalink / raw)
To: robin.murphy, will, joro, robh, dmitry.baryshkov, konrad.dybcio,
bjorn.andersson, bod, conor+dt, krzk+dt, saravanak, prakash.gupta,
vikash.garodia
Cc: iommu, linux-kernel, devicetree, Charan Teja Kalla
Use of_map_id_or_funcid() that takes #iommu-cells into account, while
keeping the compatibility with the existing DTs, which assume single
argument. A callback function is used to get the of_phandle arguments,
that contains iommu controller and its cells, and is called into the
vendor specific driver through of_iommu_xlate().
When 'args_count' passed in of_phandle_args is '1', it is expected that
of_map_id_or_funcid() fills the translated id into the of_phandle
arguments which then called into of_iommu_xlate().
When 'args_count' > 1, as it is complex to establish a defined relation
between the input id and output, of_iommu layer handles through the
below relation:
For platform devices, 'rid' defined in the iommu-map tuple indicates a
function, through a bit position, which is compared against passed input
'id' that represents a bitmap of functions represented by the device.
For others, 'rid' is compared against the input 'id' as an integer value.
When matched, of_iommu_xlate() is used to call into vendor specific
iommu driver.
Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
---
drivers/iommu/of_iommu.c | 59 +++++++++++++++++++++++++++++++++-------
drivers/of/base.c | 4 +--
include/linux/of.h | 15 ++++++++++
3 files changed, 66 insertions(+), 12 deletions(-)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 6b989a62def2..9575e37ad844 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -16,6 +16,7 @@
#include <linux/pci.h>
#include <linux/slab.h>
#include <linux/fsl/mc.h>
+#include <linux/platform_device.h>
#include "iommu-priv.h"
@@ -41,22 +42,60 @@ static int of_iommu_xlate(struct device *dev,
return ret;
}
+/**
+ * of_iommu_map_xlate - Translate IOMMU mapping and release node reference.
+ *
+ * @id: bitmap for function id parsing(platform devices) or base id.
+ * @id_base: checked against @id
+ * @arg: Device to configure.
+ * @iommu_spec: IOMMU specifier from device tree.
+ *
+ * Wrapper function that calls of_iommu_xlate() to configure the IOMMU
+ * mapping for a device, then releases the device tree node reference.
+ * This is used as a callback function for of_map_id_or_funcid().
+ *
+ * When the cell_count > 1, the relation between input id(@id) and the
+ * output specifier is complex to define, like linear case when cell_count = 1,
+ * Handle such cases where linear relation can't be established between
+ * the @id and the output with the below relation:
+ * a) For platform devices, @id_base represents the function id, which is
+ * compared against the input @id, all maintained in bitmap relation.
+ * b) For other devices, it performs exact ID matching.
+ *
+ * Return: 0 on success, -EAGAIN if ID doesn't match
+ * and standard negative error code on failure.
+ */
+static int of_iommu_map_xlate(u32 id, u32 id_base,
+ void *arg, struct of_phandle_args *iommu_spec)
+{
+ struct device *dev = arg;
+ int ret;
+
+ if (iommu_spec->args_count > 1) {
+ if (dev_is_platform(dev)) { /* case a. */
+ if (!(BIT(id_base - 1) & id))
+ return -EAGAIN;
+ } else if (id_base != id) { /* case b. */
+ return -EAGAIN;
+ }
+
+ }
+
+ ret = of_iommu_xlate(dev, iommu_spec);
+ of_node_put(iommu_spec->np);
+ return ret;
+}
+
static int of_iommu_configure_dev_id(struct device_node *master_np,
struct device *dev,
const u32 *id)
{
struct of_phandle_args iommu_spec = { .args_count = 1 };
- int err;
- err = of_map_id(master_np, *id, "iommu-map",
- "iommu-map-mask", &iommu_spec.np,
- iommu_spec.args);
- if (err)
- return err;
-
- err = of_iommu_xlate(dev, &iommu_spec);
- of_node_put(iommu_spec.np);
- return err;
+ return of_map_id_or_funcid(master_np, *id, "iommu-map",
+ "iommu-map-mask", &iommu_spec.np,
+ iommu_spec.args, dev, &iommu_spec,
+ of_iommu_map_xlate);
}
static int of_iommu_configure_dev(struct device_node *master_np,
diff --git a/drivers/of/base.c b/drivers/of/base.c
index cd221c003dd5..b5554c0b0bc5 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2181,7 +2181,7 @@ static int of_map_id_cell_count(const __be32 *map, const char *map_name,
* @id: input id that is given for translation.
* @arg: argument passed to the @fn.
* @pargs: filled by the contents of @map and pass to the @fn.
- * @fn: Callback function that operated on @dev and @fn.
+ * @fn: Callback function that operates on @arg.
*
* The function populates the phandle args structure with the node and
* specifier cells, then invokes the callback function. The callback is
@@ -2233,7 +2233,7 @@ static int of_map_id_untranslated(const __be32 *map, u32 cell_count, u32 id,
*
* Return: 0 on success or a standard error code on failure.
*/
-static int of_map_id_or_funcid(const struct device_node *np, u32 id,
+int of_map_id_or_funcid(const struct device_node *np, u32 id,
const char *map_name, const char *map_mask_name,
struct device_node **target, u32 *id_out,
void *arg, struct of_phandle_args *pargs,
diff --git a/include/linux/of.h b/include/linux/of.h
index f8d976205ff4..7935ff5ac09e 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -410,6 +410,12 @@ extern int of_phandle_iterator_args(struct of_phandle_iterator *it,
extern int of_alias_get_id(const struct device_node *np, const char *stem);
extern int of_alias_get_highest_id(const char *stem);
+extern int of_map_id_or_funcid(const struct device_node *np, u32 id,
+ const char *map_name, const char *map_mask_name,
+ struct device_node **target, u32 *id_out,
+ void *arg, struct of_phandle_args *pargs,
+ of_map_id_cb fn);
+
bool of_machine_compatible_match(const char *const *compats);
/**
@@ -909,6 +915,15 @@ static inline int of_map_id(const struct device_node *np, u32 id,
return -EINVAL;
}
+static inline int of_map_id_or_funcid(const struct device_node *np, u32 id,
+ const char *map_name, const char *map_mask_name,
+ struct device_node **target, u32 *id_out,
+ void *arg, struct of_phandle_args *pargs,
+ of_map_id_cb fn)
+{
+ return -EINVAL;
+}
+
static inline phys_addr_t of_dma_get_max_cpu_address(struct device_node *np)
{
return PHYS_ADDR_MAX;
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 6/6] of: use correct iommu-map parsing logic from of_iommu layer
2025-11-04 8:51 ` [PATCH 6/6] of: use correct iommu-map parsing logic from of_iommu layer Charan Teja Kalla
@ 2025-11-05 18:20 ` Robin Murphy
0 siblings, 0 replies; 23+ messages in thread
From: Robin Murphy @ 2025-11-05 18:20 UTC (permalink / raw)
To: Charan Teja Kalla, will, joro, robh, dmitry.baryshkov,
konrad.dybcio, bjorn.andersson, bod, conor+dt, krzk+dt, saravanak,
prakash.gupta, vikash.garodia
Cc: iommu, linux-kernel, devicetree
On 2025-11-04 8:51 am, Charan Teja Kalla wrote:
> Use of_map_id_or_funcid() that takes #iommu-cells into account, while
> keeping the compatibility with the existing DTs, which assume single
> argument. A callback function is used to get the of_phandle arguments,
> that contains iommu controller and its cells, and is called into the
> vendor specific driver through of_iommu_xlate().
>
> When 'args_count' passed in of_phandle_args is '1', it is expected that
> of_map_id_or_funcid() fills the translated id into the of_phandle
> arguments which then called into of_iommu_xlate().
>
> When 'args_count' > 1, as it is complex to establish a defined relation
> between the input id and output, of_iommu layer handles through the
> below relation:
> For platform devices, 'rid' defined in the iommu-map tuple indicates a
> function, through a bit position, which is compared against passed input
> 'id' that represents a bitmap of functions represented by the device.
>
> For others, 'rid' is compared against the input 'id' as an integer value.
Sorry, I struggle to comprehend what this is trying to achieve...
of_iommu doesn't have a clue what phandle arguments represent, only the
relevant IOMMU driver does. How are we asking the IOMMU driver to map an
input ID represented by an offset into a range, to an appropriate output
specifier, without passing all that information to a suitable driver
operation? Why would that have any dependency on bus types? And where
has this notion of Linux platform devices having some sort of inherently
standard bitmap suddenly sprung from?
Thanks,
Robin.
> When matched, of_iommu_xlate() is used to call into vendor specific
> iommu driver.
>
> Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
> ---
> drivers/iommu/of_iommu.c | 59 +++++++++++++++++++++++++++++++++-------
> drivers/of/base.c | 4 +--
> include/linux/of.h | 15 ++++++++++
> 3 files changed, 66 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 6b989a62def2..9575e37ad844 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -16,6 +16,7 @@
> #include <linux/pci.h>
> #include <linux/slab.h>
> #include <linux/fsl/mc.h>
> +#include <linux/platform_device.h>
>
> #include "iommu-priv.h"
>
> @@ -41,22 +42,60 @@ static int of_iommu_xlate(struct device *dev,
> return ret;
> }
>
> +/**
> + * of_iommu_map_xlate - Translate IOMMU mapping and release node reference.
> + *
> + * @id: bitmap for function id parsing(platform devices) or base id.
> + * @id_base: checked against @id
> + * @arg: Device to configure.
> + * @iommu_spec: IOMMU specifier from device tree.
> + *
> + * Wrapper function that calls of_iommu_xlate() to configure the IOMMU
> + * mapping for a device, then releases the device tree node reference.
> + * This is used as a callback function for of_map_id_or_funcid().
> + *
> + * When the cell_count > 1, the relation between input id(@id) and the
> + * output specifier is complex to define, like linear case when cell_count = 1,
> + * Handle such cases where linear relation can't be established between
> + * the @id and the output with the below relation:
> + * a) For platform devices, @id_base represents the function id, which is
> + * compared against the input @id, all maintained in bitmap relation.
> + * b) For other devices, it performs exact ID matching.
> + *
> + * Return: 0 on success, -EAGAIN if ID doesn't match
> + * and standard negative error code on failure.
> + */
> +static int of_iommu_map_xlate(u32 id, u32 id_base,
> + void *arg, struct of_phandle_args *iommu_spec)
> +{
> + struct device *dev = arg;
> + int ret;
> +
> + if (iommu_spec->args_count > 1) {
> + if (dev_is_platform(dev)) { /* case a. */
> + if (!(BIT(id_base - 1) & id))
> + return -EAGAIN;
> + } else if (id_base != id) { /* case b. */
> + return -EAGAIN;
> + }
> +
> + }
> +
> + ret = of_iommu_xlate(dev, iommu_spec);
> + of_node_put(iommu_spec->np);
> + return ret;
> +}
> +
> static int of_iommu_configure_dev_id(struct device_node *master_np,
> struct device *dev,
> const u32 *id)
> {
> struct of_phandle_args iommu_spec = { .args_count = 1 };
> - int err;
>
> - err = of_map_id(master_np, *id, "iommu-map",
> - "iommu-map-mask", &iommu_spec.np,
> - iommu_spec.args);
> - if (err)
> - return err;
> -
> - err = of_iommu_xlate(dev, &iommu_spec);
> - of_node_put(iommu_spec.np);
> - return err;
> + return of_map_id_or_funcid(master_np, *id, "iommu-map",
> + "iommu-map-mask", &iommu_spec.np,
> + iommu_spec.args, dev, &iommu_spec,
> + of_iommu_map_xlate);
> }
>
> static int of_iommu_configure_dev(struct device_node *master_np,
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index cd221c003dd5..b5554c0b0bc5 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2181,7 +2181,7 @@ static int of_map_id_cell_count(const __be32 *map, const char *map_name,
> * @id: input id that is given for translation.
> * @arg: argument passed to the @fn.
> * @pargs: filled by the contents of @map and pass to the @fn.
> - * @fn: Callback function that operated on @dev and @fn.
> + * @fn: Callback function that operates on @arg.
> *
> * The function populates the phandle args structure with the node and
> * specifier cells, then invokes the callback function. The callback is
> @@ -2233,7 +2233,7 @@ static int of_map_id_untranslated(const __be32 *map, u32 cell_count, u32 id,
> *
> * Return: 0 on success or a standard error code on failure.
> */
> -static int of_map_id_or_funcid(const struct device_node *np, u32 id,
> +int of_map_id_or_funcid(const struct device_node *np, u32 id,
> const char *map_name, const char *map_mask_name,
> struct device_node **target, u32 *id_out,
> void *arg, struct of_phandle_args *pargs,
> diff --git a/include/linux/of.h b/include/linux/of.h
> index f8d976205ff4..7935ff5ac09e 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -410,6 +410,12 @@ extern int of_phandle_iterator_args(struct of_phandle_iterator *it,
> extern int of_alias_get_id(const struct device_node *np, const char *stem);
> extern int of_alias_get_highest_id(const char *stem);
>
> +extern int of_map_id_or_funcid(const struct device_node *np, u32 id,
> + const char *map_name, const char *map_mask_name,
> + struct device_node **target, u32 *id_out,
> + void *arg, struct of_phandle_args *pargs,
> + of_map_id_cb fn);
> +
> bool of_machine_compatible_match(const char *const *compats);
>
> /**
> @@ -909,6 +915,15 @@ static inline int of_map_id(const struct device_node *np, u32 id,
> return -EINVAL;
> }
>
> +static inline int of_map_id_or_funcid(const struct device_node *np, u32 id,
> + const char *map_name, const char *map_mask_name,
> + struct device_node **target, u32 *id_out,
> + void *arg, struct of_phandle_args *pargs,
> + of_map_id_cb fn)
> +{
> + return -EINVAL;
> +}
> +
> static inline phys_addr_t of_dma_get_max_cpu_address(struct device_node *np)
> {
> return PHYS_ADDR_MAX;
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU
2025-11-04 8:50 [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU Charan Teja Kalla
` (5 preceding siblings ...)
2025-11-04 8:51 ` [PATCH 6/6] of: use correct iommu-map parsing logic from of_iommu layer Charan Teja Kalla
@ 2025-11-05 17:28 ` Robin Murphy
2025-11-11 18:27 ` Charan Teja Kalla
2025-11-07 8:07 ` Krzysztof Kozlowski
7 siblings, 1 reply; 23+ messages in thread
From: Robin Murphy @ 2025-11-05 17:28 UTC (permalink / raw)
To: Charan Teja Kalla, will, joro, robh, dmitry.baryshkov,
konrad.dybcio, bjorn.andersson, bod, conor+dt, krzk+dt, saravanak,
prakash.gupta, vikash.garodia
Cc: iommu, linux-kernel, devicetree
On 2025-11-04 8:50 am, Charan Teja Kalla wrote:
> The iommu-map property has been defined for the PCIe usecase and has
> been hardcoded to assume single cell for IOMMU specification, ignoring
> the #iommu-cells completely. Since the initial definition the iommu-maps
> property has been reused for other usecases and we can no longer assume
> that the single IOMMU cell properly describes the necessary IOMMU
> streams. Expand the iommu-map to take #iommu-cells into account, while
> keeping the compatibility with the existing DTs, which assume single
> argument.
>
> Unlike single iommu-cell, it is complex to establish a linear relation
> between input 'id' and output specifier for multi iommu-cells. To handle
> such cases, rely on arch-specific drivers called through
> of_iommu_xlate() from of_iommu layer, aswell it is expected the 'len'
> passed is always 1. In the of_iommu layer, the below relation is
> established before calling into vendor specific driver:
>
> a) For platform devices, 'rid' defined in the iommu-map tuple indicates
> a function, through a bit position, which is compared against passed
> input 'id' that represents a bitmap of functions represented by the
> device.
>
> b) For others, 'rid' is compared against the input 'id' as an integer
> value.
>
> Thus the final representation when #iommu-cells=n is going to be,
> iommu-map = <rid/functionid IOMMU_phandle cell0 .. celln len>;, where
> len = 1.
>
> The RFC for this patch set is found at [2].
>
> The other motivation for this patchset is the below usecase.
> USECASE [1]:
> ------------
> Video IP, 32bit, have 2 hardware sub blocks(or can be called as
> functions) called as pixel and nonpixel blocks, that does decode and
> encode of the video stream. These logical blocks are configured to
> generate different stream IDs.
>
> With the classical approach of representing all sids with iommus= end up
> in using a single translation context limited to the 4GB. There are
> video usecases which needs larger IOVA space, like higher concurrent
> video sessions(eg: 32 session and 192MB per session) where 4GB of IOVA
> is not sufficient.
>
> For this case, each functionality is represented in the firmware(device
> tree) by the 'rid' field of the iommu-map property and the video driver
> creates sub platform devices for each of this functionality and call
> into IOMMU configuration. Each rid(function id) in the dt property
> indicates the bit that can be associated by the driver passed input id.
>
> Example:
> iommu {
> #iommu-cells = 2;
> };
>
> video-codec@foobar {
> compatible = "qcom,video";
> iommus = <&apps_smmu 0x1234 0xca>;
> iommu-map= <0x1 &iommu 0x1940 0x0 0x1>,
> <0x1 &iommu 0x1941 0x0 0x1>,
> <0x2 &iommu 0x1942 0x0 0x1>,
> <0x4 &iommu 0x1943 0x0 0x1>,
> <0x4 &iommu 0x1944 0x0 0x1>;
> };
>
> video-driver:
> #define PIXEL_FUNC (1)
> #define NON_PIXEL_FUNC (2)
> #define SECURE_FUNC (4)
>
> case1: All these functionalities requires individual contexts.
> Create 3 subdevices for each of this function and call
> of_dma_configure_id(..,id), id = 0x1, 0x2, 0x4.
>
> Case2: Secure and non-secure functionalities require individual
> contexts. Create 2 subdevices and call of_dma_configure_id(..,id), id =
> 0x3(bitmap of pixel and non-pixel), 0x4 (secure).
>
> Credits: to Dmitry for thorough discussions on the RFC patch and major
> help in getting the consenus on this approach, to Konrad & Bjorn for
> offline discussions and reviews, to Robin for his inputs on IOMMU front,
> to Bod, Rob and Krzysztof for all valuable inputs.
>
> [1] https://lore.kernel.org/all/20250627-video_cb-v3-0-51e18c0ffbce@quicinc.com/
> [2] https://lore.kernel.org/all/20250928171718.436440-1-charan.kalla@oss.qualcomm.com/#r
>
> Charan Teja Kalla (6):
> of: create a wrapper for of_map_id()
> of: introduce wrapper function to query the cell count
> of: parse #<name>-cells property to get the cell count
> of: detect and handle legacy iommu-map parsing
> of: add infra to parse iommu-map per IOMMU cell count
> of: use correct iommu-map parsing logic from of_iommu layer
>
> drivers/iommu/of_iommu.c | 59 +++++++--
> drivers/of/base.c | 269 +++++++++++++++++++++++++++++++++++----
> include/linux/of.h | 19 +++
> 3 files changed, 314 insertions(+), 33 deletions(-)
Hmm, I did actually have a quick go at this the other week too, and
while I though it was a bit clunky, it was still significantly simpler
than this seems to be...
FWIW: https://gitlab.arm.com/linux-arm/linux-rm/-/commits/iommu-map - I
can give it some polish and testing to post properly if you like.
Thanks,
Robin.
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU
2025-11-05 17:28 ` [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU Robin Murphy
@ 2025-11-11 18:27 ` Charan Teja Kalla
2025-11-12 14:42 ` Charan Teja Kalla
0 siblings, 1 reply; 23+ messages in thread
From: Charan Teja Kalla @ 2025-11-11 18:27 UTC (permalink / raw)
To: Robin Murphy, will, joro, robh, dmitry.baryshkov, konrad.dybcio,
bjorn.andersson, bod, conor+dt, krzk+dt, saravanak, prakash.gupta,
vikash.garodia
Cc: iommu, linux-kernel, devicetree
On 11/5/2025 10:58 PM, Robin Murphy wrote:
>> The other motivation for this patchset is the below usecase.
>> USECASE [1]:
>> ------------
>> Video IP, 32bit, have 2 hardware sub blocks(or can be called as
>> functions) called as pixel and nonpixel blocks, that does decode and
>> encode of the video stream. These logical blocks are configured to
>> generate different stream IDs.
>>
>> With the classical approach of representing all sids with iommus= end up
>> in using a single translation context limited to the 4GB. There are
>> video usecases which needs larger IOVA space, like higher concurrent
>> video sessions(eg: 32 session and 192MB per session) where 4GB of IOVA
>> is not sufficient.
>>
>> For this case, each functionality is represented in the firmware(device
>> tree) by the 'rid' field of the iommu-map property and the video driver
>> creates sub platform devices for each of this functionality and call
>> into IOMMU configuration. Each rid(function id) in the dt property
>> indicates the bit that can be associated by the driver passed input id.
>>
>> Example:
>> iommu {
>> #iommu-cells = 2;
>> };
>>
>> video-codec@foobar {
>> compatible = "qcom,video";
>> iommus = <&apps_smmu 0x1234 0xca>;
>> iommu-map= <0x1 &iommu 0x1940 0x0 0x1>,
>> <0x1 &iommu 0x1941 0x0 0x1>,
>> <0x2 &iommu 0x1942 0x0 0x1>,
>> <0x4 &iommu 0x1943 0x0 0x1>,
>> <0x4 &iommu 0x1944 0x0 0x1>;
>> };
>>
>> video-driver:
>> #define PIXEL_FUNC (1)
>> #define NON_PIXEL_FUNC (2)
>> #define SECURE_FUNC (4)
>>
>> case1: All these functionalities requires individual contexts.
>> Create 3 subdevices for each of this function and call
>> of_dma_configure_id(..,id), id = 0x1, 0x2, 0x4.
>>
>> Case2: Secure and non-secure functionalities require individual
>> contexts. Create 2 subdevices and call of_dma_configure_id(..,id), id =
>> 0x3(bitmap of pixel and non-pixel), 0x4 (secure).
>>
>> Credits: to Dmitry for thorough discussions on the RFC patch and major
>> help in getting the consenus on this approach, to Konrad & Bjorn for
>> offline discussions and reviews, to Robin for his inputs on IOMMU front,
>> to Bod, Rob and Krzysztof for all valuable inputs.
>>
>> [1] https://lore.kernel.org/all/20250627-video_cb-
>> v3-0-51e18c0ffbce@quicinc.com/
>> [2] https://lore.kernel.org/all/20250928171718.436440-1-
>> charan.kalla@oss.qualcomm.com/#r
>>
>> Charan Teja Kalla (6):
>> of: create a wrapper for of_map_id()
>> of: introduce wrapper function to query the cell count
>> of: parse #<name>-cells property to get the cell count
>> of: detect and handle legacy iommu-map parsing
>> of: add infra to parse iommu-map per IOMMU cell count
>> of: use correct iommu-map parsing logic from of_iommu layer
>>
>> drivers/iommu/of_iommu.c | 59 +++++++--
>> drivers/of/base.c | 269 +++++++++++++++++++++++++++++++++++----
>> include/linux/of.h | 19 +++
>> 3 files changed, 314 insertions(+), 33 deletions(-)
>
> Hmm, I did actually have a quick go at this the other week too, and
> while I though it was a bit clunky, it was still significantly simpler
> than this seems to be...
>
> FWIW: https://gitlab.arm.com/linux-arm/linux-rm/-/commits/iommu-map - I
Thanks a lot Robin for taking a look and sorry for the delayed reply as
I was on vacation.
stripped code_snippet from your patch:
offset = 0;
out_base = map + offset + 2;
id_off = masked_id - id_base;
if (masked_id < id_base || id_off >= id_len)
continue;
for (int i = 0; id_out && i < cells; i++)
id_out[i] = id_off + be32_to_cpu(out_base[i]);
seems way cleaner than mine...
Actually, we also have a case of a device emitting 2 distinct
identifiers, eg: a device is emitting 0x1940, 0x1944 and 0x1A20 sids and
attached to a single context bank. If I use mask to cover all these sids
in a single iommu-map entry, it does overlap with other device SID.
I don't think that patch you shared can be used to cover the above, or
it is?
Hence I resorted to the approach where RID used as the bitmap of indices
to cover such cases for platform devices, which it seems you clearly
didn't like....otherwise, any otherway we can handle such cases?
> can give it some polish and testing to post properly if you like.
Sure, I am fine with it ...
or If you are busy please let me know so that i can take it forward as
_you are the only author of the patch._
[1]
https://gitlab.arm.com/linux-arm/linux-rm/-/commit/3e1bd87ce1a7b6f567ab17a0931e4294d8982ed2
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU
2025-11-11 18:27 ` Charan Teja Kalla
@ 2025-11-12 14:42 ` Charan Teja Kalla
2025-11-21 5:54 ` Charan Teja Kalla
2025-11-24 16:51 ` Robin Murphy
0 siblings, 2 replies; 23+ messages in thread
From: Charan Teja Kalla @ 2025-11-12 14:42 UTC (permalink / raw)
To: Robin Murphy, will, joro, robh, dmitry.baryshkov, konrad.dybcio,
bjorn.andersson, bod, conor+dt, krzk+dt, saravanak, prakash.gupta,
vikash.garodia
Cc: iommu, linux-kernel, devicetree
On 11/11/2025 11:57 PM, Charan Teja Kalla wrote:
>
> On 11/5/2025 10:58 PM, Robin Murphy wrote:
>>> The other motivation for this patchset is the below usecase.
>>> USECASE [1]:
>>> ------------
>>> Video IP, 32bit, have 2 hardware sub blocks(or can be called as
>>> functions) called as pixel and nonpixel blocks, that does decode and
>>> encode of the video stream. These logical blocks are configured to
>>> generate different stream IDs.
>>>
>>> With the classical approach of representing all sids with iommus= end up
>>> in using a single translation context limited to the 4GB. There are
>>> video usecases which needs larger IOVA space, like higher concurrent
>>> video sessions(eg: 32 session and 192MB per session) where 4GB of IOVA
>>> is not sufficient.
>>>
>>> For this case, each functionality is represented in the firmware(device
>>> tree) by the 'rid' field of the iommu-map property and the video driver
>>> creates sub platform devices for each of this functionality and call
>>> into IOMMU configuration. Each rid(function id) in the dt property
>>> indicates the bit that can be associated by the driver passed input id.
>>>
>>> Example:
>>> iommu {
>>> #iommu-cells = 2;
>>> };
>>>
>>> video-codec@foobar {
>>> compatible = "qcom,video";
>>> iommus = <&apps_smmu 0x1234 0xca>;
>>> iommu-map= <0x1 &iommu 0x1940 0x0 0x1>,
>>> <0x1 &iommu 0x1941 0x0 0x1>,
>>> <0x2 &iommu 0x1942 0x0 0x1>,
>>> <0x4 &iommu 0x1943 0x0 0x1>,
>>> <0x4 &iommu 0x1944 0x0 0x1>;
>>> };
>>>
>>> video-driver:
>>> #define PIXEL_FUNC (1)
>>> #define NON_PIXEL_FUNC (2)
>>> #define SECURE_FUNC (4)
>>>
>>> case1: All these functionalities requires individual contexts.
>>> Create 3 subdevices for each of this function and call
>>> of_dma_configure_id(..,id), id = 0x1, 0x2, 0x4.
>>>
>>> Case2: Secure and non-secure functionalities require individual
>>> contexts. Create 2 subdevices and call of_dma_configure_id(..,id), id =
>>> 0x3(bitmap of pixel and non-pixel), 0x4 (secure).
>>>
>>> Credits: to Dmitry for thorough discussions on the RFC patch and major
>>> help in getting the consenus on this approach, to Konrad & Bjorn for
>>> offline discussions and reviews, to Robin for his inputs on IOMMU front,
>>> to Bod, Rob and Krzysztof for all valuable inputs.
>>>
>>> [1] https://lore.kernel.org/all/20250627-video_cb-
>>> v3-0-51e18c0ffbce@quicinc.com/
>>> [2] https://lore.kernel.org/all/20250928171718.436440-1-
>>> charan.kalla@oss.qualcomm.com/#r
>>>
>>> Charan Teja Kalla (6):
>>> of: create a wrapper for of_map_id()
>>> of: introduce wrapper function to query the cell count
>>> of: parse #<name>-cells property to get the cell count
>>> of: detect and handle legacy iommu-map parsing
>>> of: add infra to parse iommu-map per IOMMU cell count
>>> of: use correct iommu-map parsing logic from of_iommu layer
>>>
>>> drivers/iommu/of_iommu.c | 59 +++++++--
>>> drivers/of/base.c | 269 +++++++++++++++++++++++++++++++++++----
>>> include/linux/of.h | 19 +++
>>> 3 files changed, 314 insertions(+), 33 deletions(-)
>> Hmm, I did actually have a quick go at this the other week too, and
>> while I though it was a bit clunky, it was still significantly simpler
>> than this seems to be...
>>
>> FWIW: https://gitlab.arm.com/linux-arm/linux-rm/-/commits/iommu-map - I
> Thanks a lot Robin for taking a look and sorry for the delayed reply as
> I was on vacation.
>
> stripped code_snippet from your patch:
> offset = 0;
> out_base = map + offset + 2;
> id_off = masked_id - id_base;
> if (masked_id < id_base || id_off >= id_len)
> continue;
> for (int i = 0; id_out && i < cells; i++)
> id_out[i] = id_off + be32_to_cpu(out_base[i]);
>
>
> seems way cleaner than mine...
>
> Actually, we also have a case of a device emitting 2 distinct
> identifiers, eg: a device is emitting 0x1940, 0x1944 and 0x1A20 sids and
> attached to a single context bank. If I use mask to cover all these sids
> in a single iommu-map entry, it does overlap with other device SID.
>
> I don't think that patch you shared can be used to cover the above, or
> it is?
>
> Hence I resorted to the approach where RID used as the bitmap of indices
> to cover such cases for platform devices, which it seems you clearly
> didn't like....otherwise, any otherway we can handle such cases?
Hi Robin,
Don't want to bother you with my ideas, but I can't think of other ways
to handle such cases of multi-map than the below. I just tried this code on
Qemu on top of your patches(with some nit compilation fixes) and just checked
if devices are added to the iommu groups.
----------------------8888---------------------------------------------
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index a511ecf21fcd..ac005e70de7d 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -16,6 +16,7 @@
#include <linux/pci.h>
#include <linux/slab.h>
#include <linux/fsl/mc.h>
+#include <linux/platform_device.h>
#include "iommu-priv.h"
@@ -41,6 +42,18 @@ static int of_iommu_xlate(struct device *dev,
return ret;
}
+static int of_iommu_configure_cb(void *arg, u32 *id_out)
+{
+ struct of_phandle_args *iommu_spec =
+ (struct of_phandle_args *)((void *)id_out - offsetof(struct of_phandle_args, args));
+ struct device *dev = arg;
+ int err;
+
+ err = of_iommu_xlate(dev, iommu_spec);
+ of_node_put(iommu_spec->np);
+ return err;
+}
+
static int of_iommu_configure_dev_id(struct device_node *master_np,
struct device *dev,
const u32 *id)
@@ -48,12 +61,10 @@ static int of_iommu_configure_dev_id(struct device_node *master_np,
struct of_phandle_args iommu_spec = { .args_count = 1 };
int err;
- err = of_map_iommu_id(master_np, *id, &iommu_spec.np, iommu_spec.args);
+ err = of_map_iommu_id(master_np, *id, &iommu_spec.np, iommu_spec.args,
+ dev_is_platform(dev) ? true : false, dev, of_iommu_configure_cb);
- if (err)
- return err;
-
- err = of_iommu_xlate(dev, &iommu_spec);
- of_node_put(iommu_spec.np);
return err;
}
diff --git a/drivers/of/base.c b/drivers/of/base.c
index a4fd4a4f720b..8abe36c17309 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2085,16 +2085,21 @@ static bool of_check_bad_map(const __be32 *map, int len)
*/
int of_map_id(const struct device_node *np, u32 id, const char *map_name,
const char *cells_name, const char *map_mask_name,
- struct device_node **target, u32 *id_out)
+ struct device_node **target, u32 *id_out, bool multi_map,
+ void *arg, of_map_id_cb cb)
{
u32 map_mask, masked_id;
int map_bytes, map_len, offset = 0;
bool bad_map = false;
const __be32 *map = NULL;
+ bool mapped_multi_id = false;
if (!np || !map_name || !cells_name || (!target && !id_out))
return -EINVAL;
+ if (multi_map && !cb)
+ return -EINVAL;
+
map = of_get_property(np, map_name, &map_bytes);
if (!map) {
if (target)
@@ -2189,16 +2194,29 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
pr_debug("%pOF: %s, using mask %08x, id-base: %08x, out-base: %08x, length: %08x, id: %08x -> %08x\n",
np, map_name, map_mask, id_base, be32_to_cpup(out_base),
id_len, id, id_off + be32_to_cpup(out_base));
- return 0;
+
+ if (multi_map) {
+ if (cb(arg, id_out))
+ return -EINVAL;
+
+ mapped_multi_id = true;
+ continue;
+ }
+
+ goto translated;
}
+ if (mapped_multi_id)
+ return 0;
+
pr_info("%pOF: no %s translation for id 0x%x on %pOF\n", np, map_name,
id, target && *target ? *target : NULL);
/* Bypasses translation */
if (id_out)
*id_out = id;
- return 0;
+translated:
+ return cb ? cb(arg, id_out) : 0;
err_map_len:
pr_err("%pOF: Error: Bad %s length: %d\n", np, map_name, map_bytes);
diff --git a/include/linux/of.h b/include/linux/of.h
index 183be897b088..84a24c2a1041 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -24,6 +24,7 @@
typedef u32 phandle;
typedef u32 ihandle;
+typedef int (*of_map_id_cb)(void *arg, u32 *id_out);
struct property {
char *name;
@@ -458,7 +459,8 @@ bool of_console_check(const struct device_node *dn, char *name, int index);
int of_map_id(const struct device_node *np, u32 id, const char *map_name,
const char *cells_name, const char *map_mask_name,
- struct device_node **target, u32 *id_out);
+ struct device_node **target, u32 *id_out,
+ bool multi_map, void *arg, of_map_id_cb cb);
phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);
@@ -1436,17 +1438,18 @@ static inline int of_property_read_s32(const struct device_node *np,
}
static inline int of_map_iommu_id(const struct device_node *np, u32 id,
- struct device_node **target, u32 *id_out)
+ struct device_node **target, u32 *id_out,
+ bool multi_map, void *arg, of_map_id_cb cb)
{
return of_map_id(np, id, "iommu-map", "#iommu-cells", "iommu-map-mask",
- target, id_out);
+ target, id_out, multi_map, arg, cb);
}
static inline int of_map_msi_id(const struct device_node *np, u32 id,
struct device_node **target, u32 *id_out)
{
return of_map_id(np, id, "msi-map", "#msi-cells", "msi-map-mask",
- target, id_out);
+ target, id_out, false, NULL, NULL);
}
#define of_for_each_phandle(it, err, np, ln, cn, cc)
-----------------------------------------------------------------------
full patch is at: https://github.com/charan-kalla-oss/linux-next/commits/refs/for/iommu_map
Thanks,
Charan
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU
2025-11-12 14:42 ` Charan Teja Kalla
@ 2025-11-21 5:54 ` Charan Teja Kalla
2025-11-24 16:42 ` Robin Murphy
2025-11-24 16:51 ` Robin Murphy
1 sibling, 1 reply; 23+ messages in thread
From: Charan Teja Kalla @ 2025-11-21 5:54 UTC (permalink / raw)
To: Robin Murphy, will, joro, robh, dmitry.baryshkov, konrad.dybcio,
bjorn.andersson, bod, conor+dt, krzk+dt, saravanak, prakash.gupta,
vikash.garodia
Cc: iommu, linux-kernel, devicetree
On 11/12/2025 8:12 PM, Charan Teja Kalla wrote:
> Hi Robin,
>
> Don't want to bother you with my ideas, but I can't think of other ways
> to handle such cases of multi-map than the below. I just tried this code on
> Qemu on top of your patches(with some nit compilation fixes) and just checked
> if devices are added to the iommu groups.
>
Hello Robin,
Not sure If this is early to ask for feedback, but waiting for your
valuable inputs here. Thanks in advance.
> ----------------------8888---------------------------------------------
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index a511ecf21fcd..ac005e70de7d 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -16,6 +16,7 @@
> #include <linux/pci.h>
> #include <linux/slab.h>
> #include <linux/fsl/mc.h>
> +#include <linux/platform_device.h>
>
> #include "iommu-priv.h"
>
> @@ -41,6 +42,18 @@ static int of_iommu_xlate(struct device *dev,
> return ret;
> }
>
> +static int of_iommu_configure_cb(void *arg, u32 *id_out)
> +{
> + struct of_phandle_args *iommu_spec =
> + (struct of_phandle_args *)((void *)id_out - offsetof(struct of_phandle_args, args));
> + struct device *dev = arg;
> + int err;
> +
> + err = of_iommu_xlate(dev, iommu_spec);
> + of_node_put(iommu_spec->np);
> + return err;
> +}
> +
> static int of_iommu_configure_dev_id(struct device_node *master_np,
> struct device *dev,
> const u32 *id)
> @@ -48,12 +61,10 @@ static int of_iommu_configure_dev_id(struct device_node *master_np,
> struct of_phandle_args iommu_spec = { .args_count = 1 };
> int err;
>
> - err = of_map_iommu_id(master_np, *id, &iommu_spec.np, iommu_spec.args);
> + err = of_map_iommu_id(master_np, *id, &iommu_spec.np, iommu_spec.args,
> + dev_is_platform(dev) ? true : false, dev, of_iommu_configure_cb);
> - if (err)
> - return err;
> -
> - err = of_iommu_xlate(dev, &iommu_spec);
> - of_node_put(iommu_spec.np);
> return err;
> }
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index a4fd4a4f720b..8abe36c17309 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2085,16 +2085,21 @@ static bool of_check_bad_map(const __be32 *map, int len)
> */
> int of_map_id(const struct device_node *np, u32 id, const char *map_name,
> const char *cells_name, const char *map_mask_name,
> - struct device_node **target, u32 *id_out)
> + struct device_node **target, u32 *id_out, bool multi_map,
> + void *arg, of_map_id_cb cb)
> {
> u32 map_mask, masked_id;
> int map_bytes, map_len, offset = 0;
> bool bad_map = false;
> const __be32 *map = NULL;
> + bool mapped_multi_id = false;
>
> if (!np || !map_name || !cells_name || (!target && !id_out))
> return -EINVAL;
>
> + if (multi_map && !cb)
> + return -EINVAL;
> +
> map = of_get_property(np, map_name, &map_bytes);
> if (!map) {
> if (target)
> @@ -2189,16 +2194,29 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
> pr_debug("%pOF: %s, using mask %08x, id-base: %08x, out-base: %08x, length: %08x, id: %08x -> %08x\n",
> np, map_name, map_mask, id_base, be32_to_cpup(out_base),
> id_len, id, id_off + be32_to_cpup(out_base));
> - return 0;
> +
> + if (multi_map) {
> + if (cb(arg, id_out))
> + return -EINVAL;
> +
> + mapped_multi_id = true;
> + continue;
> + }
> +
> + goto translated;
> }
>
> + if (mapped_multi_id)
> + return 0;
> +
> pr_info("%pOF: no %s translation for id 0x%x on %pOF\n", np, map_name,
> id, target && *target ? *target : NULL);
>
> /* Bypasses translation */
> if (id_out)
> *id_out = id;
> - return 0;
> +translated:
> + return cb ? cb(arg, id_out) : 0;
>
> err_map_len:
> pr_err("%pOF: Error: Bad %s length: %d\n", np, map_name, map_bytes);
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 183be897b088..84a24c2a1041 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -24,6 +24,7 @@
>
> typedef u32 phandle;
> typedef u32 ihandle;
> +typedef int (*of_map_id_cb)(void *arg, u32 *id_out);
>
> struct property {
> char *name;
> @@ -458,7 +459,8 @@ bool of_console_check(const struct device_node *dn, char *name, int index);
>
> int of_map_id(const struct device_node *np, u32 id, const char *map_name,
> const char *cells_name, const char *map_mask_name,
> - struct device_node **target, u32 *id_out);
> + struct device_node **target, u32 *id_out,
> + bool multi_map, void *arg, of_map_id_cb cb);
>
> phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);
>
> @@ -1436,17 +1438,18 @@ static inline int of_property_read_s32(const struct device_node *np,
> }
>
> static inline int of_map_iommu_id(const struct device_node *np, u32 id,
> - struct device_node **target, u32 *id_out)
> + struct device_node **target, u32 *id_out,
> + bool multi_map, void *arg, of_map_id_cb cb)
> {
> return of_map_id(np, id, "iommu-map", "#iommu-cells", "iommu-map-mask",
> - target, id_out);
> + target, id_out, multi_map, arg, cb);
> }
>
> static inline int of_map_msi_id(const struct device_node *np, u32 id,
> struct device_node **target, u32 *id_out)
> {
> return of_map_id(np, id, "msi-map", "#msi-cells", "msi-map-mask",
> - target, id_out);
> + target, id_out, false, NULL, NULL);
> }
>
> #define of_for_each_phandle(it, err, np, ln, cn, cc)
> -----------------------------------------------------------------------
>
> full patch is at: https://github.com/charan-kalla-oss/linux-next/commits/refs/for/iommu_map
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU
2025-11-21 5:54 ` Charan Teja Kalla
@ 2025-11-24 16:42 ` Robin Murphy
0 siblings, 0 replies; 23+ messages in thread
From: Robin Murphy @ 2025-11-24 16:42 UTC (permalink / raw)
To: Charan Teja Kalla, will, joro, robh, dmitry.baryshkov,
konrad.dybcio, bjorn.andersson, bod, conor+dt, krzk+dt, saravanak,
prakash.gupta, vikash.garodia
Cc: iommu, linux-kernel, devicetree
On 2025-11-21 5:54 am, Charan Teja Kalla wrote:
>
> On 11/12/2025 8:12 PM, Charan Teja Kalla wrote:
>> Hi Robin,
>>
>> Don't want to bother you with my ideas, but I can't think of other ways
>> to handle such cases of multi-map than the below. I just tried this code on
>> Qemu on top of your patches(with some nit compilation fixes) and just checked
>> if devices are added to the iommu groups.
>>
>
> Hello Robin,
> Not sure If this is early to ask for feedback, but waiting for your
> valuable inputs here. Thanks in advance.
Argh, sorry, I had written a whole reply, but apparently I never hit
send, and I've been swamped with other stuff lately. Let me see if I can
dig that out...
Robin.
>
>> ----------------------8888---------------------------------------------
>>
>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>> index a511ecf21fcd..ac005e70de7d 100644
>> --- a/drivers/iommu/of_iommu.c
>> +++ b/drivers/iommu/of_iommu.c
>> @@ -16,6 +16,7 @@
>> #include <linux/pci.h>
>> #include <linux/slab.h>
>> #include <linux/fsl/mc.h>
>> +#include <linux/platform_device.h>
>>
>> #include "iommu-priv.h"
>>
>> @@ -41,6 +42,18 @@ static int of_iommu_xlate(struct device *dev,
>> return ret;
>> }
>>
>> +static int of_iommu_configure_cb(void *arg, u32 *id_out)
>> +{
>> + struct of_phandle_args *iommu_spec =
>> + (struct of_phandle_args *)((void *)id_out - offsetof(struct of_phandle_args, args));
>> + struct device *dev = arg;
>> + int err;
>> +
>> + err = of_iommu_xlate(dev, iommu_spec);
>> + of_node_put(iommu_spec->np);
>> + return err;
>> +}
>> +
>> static int of_iommu_configure_dev_id(struct device_node *master_np,
>> struct device *dev,
>> const u32 *id)
>> @@ -48,12 +61,10 @@ static int of_iommu_configure_dev_id(struct device_node *master_np,
>> struct of_phandle_args iommu_spec = { .args_count = 1 };
>> int err;
>>
>> - err = of_map_iommu_id(master_np, *id, &iommu_spec.np, iommu_spec.args);
>> + err = of_map_iommu_id(master_np, *id, &iommu_spec.np, iommu_spec.args,
>> + dev_is_platform(dev) ? true : false, dev, of_iommu_configure_cb);
>> - if (err)
>> - return err;
>> -
>> - err = of_iommu_xlate(dev, &iommu_spec);
>> - of_node_put(iommu_spec.np);
>> return err;
>> }
>>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index a4fd4a4f720b..8abe36c17309 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -2085,16 +2085,21 @@ static bool of_check_bad_map(const __be32 *map, int len)
>> */
>> int of_map_id(const struct device_node *np, u32 id, const char *map_name,
>> const char *cells_name, const char *map_mask_name,
>> - struct device_node **target, u32 *id_out)
>> + struct device_node **target, u32 *id_out, bool multi_map,
>> + void *arg, of_map_id_cb cb)
>> {
>> u32 map_mask, masked_id;
>> int map_bytes, map_len, offset = 0;
>> bool bad_map = false;
>> const __be32 *map = NULL;
>> + bool mapped_multi_id = false;
>>
>> if (!np || !map_name || !cells_name || (!target && !id_out))
>> return -EINVAL;
>>
>> + if (multi_map && !cb)
>> + return -EINVAL;
>> +
>> map = of_get_property(np, map_name, &map_bytes);
>> if (!map) {
>> if (target)
>> @@ -2189,16 +2194,29 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
>> pr_debug("%pOF: %s, using mask %08x, id-base: %08x, out-base: %08x, length: %08x, id: %08x -> %08x\n",
>> np, map_name, map_mask, id_base, be32_to_cpup(out_base),
>> id_len, id, id_off + be32_to_cpup(out_base));
>> - return 0;
>> +
>> + if (multi_map) {
>> + if (cb(arg, id_out))
>> + return -EINVAL;
>> +
>> + mapped_multi_id = true;
>> + continue;
>> + }
>> +
>> + goto translated;
>> }
>>
>> + if (mapped_multi_id)
>> + return 0;
>> +
>> pr_info("%pOF: no %s translation for id 0x%x on %pOF\n", np, map_name,
>> id, target && *target ? *target : NULL);
>>
>> /* Bypasses translation */
>> if (id_out)
>> *id_out = id;
>> - return 0;
>> +translated:
>> + return cb ? cb(arg, id_out) : 0;
>>
>> err_map_len:
>> pr_err("%pOF: Error: Bad %s length: %d\n", np, map_name, map_bytes);
>> diff --git a/include/linux/of.h b/include/linux/of.h
>> index 183be897b088..84a24c2a1041 100644
>> --- a/include/linux/of.h
>> +++ b/include/linux/of.h
>> @@ -24,6 +24,7 @@
>>
>> typedef u32 phandle;
>> typedef u32 ihandle;
>> +typedef int (*of_map_id_cb)(void *arg, u32 *id_out);
>>
>> struct property {
>> char *name;
>> @@ -458,7 +459,8 @@ bool of_console_check(const struct device_node *dn, char *name, int index);
>>
>> int of_map_id(const struct device_node *np, u32 id, const char *map_name,
>> const char *cells_name, const char *map_mask_name,
>> - struct device_node **target, u32 *id_out);
>> + struct device_node **target, u32 *id_out,
>> + bool multi_map, void *arg, of_map_id_cb cb);
>>
>> phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);
>>
>> @@ -1436,17 +1438,18 @@ static inline int of_property_read_s32(const struct device_node *np,
>> }
>>
>> static inline int of_map_iommu_id(const struct device_node *np, u32 id,
>> - struct device_node **target, u32 *id_out)
>> + struct device_node **target, u32 *id_out,
>> + bool multi_map, void *arg, of_map_id_cb cb)
>> {
>> return of_map_id(np, id, "iommu-map", "#iommu-cells", "iommu-map-mask",
>> - target, id_out);
>> + target, id_out, multi_map, arg, cb);
>> }
>>
>> static inline int of_map_msi_id(const struct device_node *np, u32 id,
>> struct device_node **target, u32 *id_out)
>> {
>> return of_map_id(np, id, "msi-map", "#msi-cells", "msi-map-mask",
>> - target, id_out);
>> + target, id_out, false, NULL, NULL);
>> }
>>
>> #define of_for_each_phandle(it, err, np, ln, cn, cc)
>> -----------------------------------------------------------------------
>>
>> full patch is at: https://github.com/charan-kalla-oss/linux-next/commits/refs/for/iommu_map
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU
2025-11-12 14:42 ` Charan Teja Kalla
2025-11-21 5:54 ` Charan Teja Kalla
@ 2025-11-24 16:51 ` Robin Murphy
2025-11-26 17:30 ` Charan Teja Kalla
1 sibling, 1 reply; 23+ messages in thread
From: Robin Murphy @ 2025-11-24 16:51 UTC (permalink / raw)
To: Charan Teja Kalla, will, joro, robh, dmitry.baryshkov,
konrad.dybcio, bjorn.andersson, bod, conor+dt, krzk+dt, saravanak,
prakash.gupta, vikash.garodia
Cc: iommu, linux-kernel, devicetree
On 2025-11-12 2:42 pm, Charan Teja Kalla wrote:
>
>
> On 11/11/2025 11:57 PM, Charan Teja Kalla wrote:
>>
>> On 11/5/2025 10:58 PM, Robin Murphy wrote:
>>>> The other motivation for this patchset is the below usecase.
>>>> USECASE [1]:
>>>> ------------
>>>> Video IP, 32bit, have 2 hardware sub blocks(or can be called as
>>>> functions) called as pixel and nonpixel blocks, that does decode and
>>>> encode of the video stream. These logical blocks are configured to
>>>> generate different stream IDs.
>>>>
>>>> With the classical approach of representing all sids with iommus= end up
>>>> in using a single translation context limited to the 4GB. There are
>>>> video usecases which needs larger IOVA space, like higher concurrent
>>>> video sessions(eg: 32 session and 192MB per session) where 4GB of IOVA
>>>> is not sufficient.
>>>>
>>>> For this case, each functionality is represented in the firmware(device
>>>> tree) by the 'rid' field of the iommu-map property and the video driver
>>>> creates sub platform devices for each of this functionality and call
>>>> into IOMMU configuration. Each rid(function id) in the dt property
>>>> indicates the bit that can be associated by the driver passed input id.
>>>>
>>>> Example:
>>>> iommu {
>>>> #iommu-cells = 2;
>>>> };
>>>>
>>>> video-codec@foobar {
>>>> compatible = "qcom,video";
>>>> iommus = <&apps_smmu 0x1234 0xca>;
>>>> iommu-map= <0x1 &iommu 0x1940 0x0 0x1>,
>>>> <0x1 &iommu 0x1941 0x0 0x1>,
>>>> <0x2 &iommu 0x1942 0x0 0x1>,
>>>> <0x4 &iommu 0x1943 0x0 0x1>,
>>>> <0x4 &iommu 0x1944 0x0 0x1>;
>>>> };
>>>>
>>>> video-driver:
>>>> #define PIXEL_FUNC (1)
>>>> #define NON_PIXEL_FUNC (2)
>>>> #define SECURE_FUNC (4)
>>>>
>>>> case1: All these functionalities requires individual contexts.
>>>> Create 3 subdevices for each of this function and call
>>>> of_dma_configure_id(..,id), id = 0x1, 0x2, 0x4.
>>>>
>>>> Case2: Secure and non-secure functionalities require individual
>>>> contexts. Create 2 subdevices and call of_dma_configure_id(..,id), id =
>>>> 0x3(bitmap of pixel and non-pixel), 0x4 (secure).
>>>>
>>>> Credits: to Dmitry for thorough discussions on the RFC patch and major
>>>> help in getting the consenus on this approach, to Konrad & Bjorn for
>>>> offline discussions and reviews, to Robin for his inputs on IOMMU front,
>>>> to Bod, Rob and Krzysztof for all valuable inputs.
>>>>
>>>> [1] https://lore.kernel.org/all/20250627-video_cb-
>>>> v3-0-51e18c0ffbce@quicinc.com/
>>>> [2] https://lore.kernel.org/all/20250928171718.436440-1-
>>>> charan.kalla@oss.qualcomm.com/#r
>>>>
>>>> Charan Teja Kalla (6):
>>>> of: create a wrapper for of_map_id()
>>>> of: introduce wrapper function to query the cell count
>>>> of: parse #<name>-cells property to get the cell count
>>>> of: detect and handle legacy iommu-map parsing
>>>> of: add infra to parse iommu-map per IOMMU cell count
>>>> of: use correct iommu-map parsing logic from of_iommu layer
>>>>
>>>> drivers/iommu/of_iommu.c | 59 +++++++--
>>>> drivers/of/base.c | 269 +++++++++++++++++++++++++++++++++++----
>>>> include/linux/of.h | 19 +++
>>>> 3 files changed, 314 insertions(+), 33 deletions(-)
>>> Hmm, I did actually have a quick go at this the other week too, and
>>> while I though it was a bit clunky, it was still significantly simpler
>>> than this seems to be...
>>>
>>> FWIW: https://gitlab.arm.com/linux-arm/linux-rm/-/commits/iommu-map - I
>> Thanks a lot Robin for taking a look and sorry for the delayed reply as
>> I was on vacation.
>>
>> stripped code_snippet from your patch:
>> offset = 0;
>> out_base = map + offset + 2;
>> id_off = masked_id - id_base;
>> if (masked_id < id_base || id_off >= id_len)
>> continue;
>> for (int i = 0; id_out && i < cells; i++)
>> id_out[i] = id_off + be32_to_cpu(out_base[i]);
>>
>>
>> seems way cleaner than mine...
>>
>> Actually, we also have a case of a device emitting 2 distinct
>> identifiers, eg: a device is emitting 0x1940, 0x1944 and 0x1A20 sids and
>> attached to a single context bank. If I use mask to cover all these sids
>> in a single iommu-map entry, it does overlap with other device SID.
>>
>> I don't think that patch you shared can be used to cover the above, or
>> it is?
No, the point of my patch is just to correctly respect #cells and
hopefully discourage any further abuse of Linux's current behaviour. The
fact that Linux interprets multiple mappings for the same input ID as a
set of equivalent choices to pick one of, rather than a set that must
all be maintained in parallel, is an orthogonal concern. Again there's
not necessarily one right answer there.
>> Hence I resorted to the approach where RID used as the bitmap of indices
>> to cover such cases for platform devices, which it seems you clearly
>> didn't like....otherwise, any otherway we can handle such cases?
Eww, if the intent of that was to bake further abuse into DT ABI to
bodge around Linux behaviour then I double-NAK it even harder :)
> Hi Robin,
>
> Don't want to bother you with my ideas, but I can't think of other ways
> to handle such cases of multi-map than the below. I just tried this code on
> Qemu on top of your patches(with some nit compilation fixes) and just checked
> if devices are added to the iommu groups.
My initial thought would be to either add an index argument so that
callers can keep count and request the Nth match if they want to - like
we do in various resource management APIs, for instance - or go all the
way and convert the existing target and id_out complexity into
usage-specific callbacks too.
> ----------------------8888---------------------------------------------
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index a511ecf21fcd..ac005e70de7d 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -16,6 +16,7 @@
> #include <linux/pci.h>
> #include <linux/slab.h>
> #include <linux/fsl/mc.h>
> +#include <linux/platform_device.h>
>
> #include "iommu-priv.h"
>
> @@ -41,6 +42,18 @@ static int of_iommu_xlate(struct device *dev,
> return ret;
> }
>
> +static int of_iommu_configure_cb(void *arg, u32 *id_out)
> +{
> + struct of_phandle_args *iommu_spec =
> + (struct of_phandle_args *)((void *)id_out - offsetof(struct of_phandle_args, args));
Not sure whether to be impressed or disgusted... If we are to take a
callback approach then it should probably standardise on passing a full
of_phandle_args to encode the map output. Particularly given what I've
just noticed below...
> + struct device *dev = arg;
> + int err;
> +
> + err = of_iommu_xlate(dev, iommu_spec);
> + of_node_put(iommu_spec->np);
> + return err;
> +}
> +
> static int of_iommu_configure_dev_id(struct device_node *master_np,
> struct device *dev,
> const u32 *id)
> @@ -48,12 +61,10 @@ static int of_iommu_configure_dev_id(struct device_node *master_np,
> struct of_phandle_args iommu_spec = { .args_count = 1 };
Oh dear, I totally overlooked this, and off the top of my head I'm not
sure it's simple to fix :(
So it's still not actually working as intended. Oh well, I did say it
was untested...
Thanks,
Robin.
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU
2025-11-24 16:51 ` Robin Murphy
@ 2025-11-26 17:30 ` Charan Teja Kalla
0 siblings, 0 replies; 23+ messages in thread
From: Charan Teja Kalla @ 2025-11-26 17:30 UTC (permalink / raw)
To: Robin Murphy, will, joro, robh, dmitry.baryshkov, konrad.dybcio,
bjorn.andersson, bod, conor+dt, krzk+dt, saravanak, prakash.gupta,
vikash.garodia
Cc: iommu, linux-kernel, devicetree
Thanks a lot Robin!!
On 11/24/2025 10:21 PM, Robin Murphy wrote:
>> ----------------------8888---------------------------------------------
>>
>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>> index a511ecf21fcd..ac005e70de7d 100644
>> --- a/drivers/iommu/of_iommu.c
>> +++ b/drivers/iommu/of_iommu.c
>> @@ -16,6 +16,7 @@
>> #include <linux/pci.h>
>> #include <linux/slab.h>
>> #include <linux/fsl/mc.h>
>> +#include <linux/platform_device.h>
>> #include "iommu-priv.h"
>> @@ -41,6 +42,18 @@ static int of_iommu_xlate(struct device *dev,
>> return ret;
>> }
>> +static int of_iommu_configure_cb(void *arg, u32 *id_out)
>> +{
>> + struct of_phandle_args *iommu_spec =
>> + (struct of_phandle_args *)((void *)id_out - offsetof(struct
>> of_phandle_args, args));
>
> Not sure whether to be impressed or disgusted... If we are to take a
> callback approach then it should probably standardise on passing a full
> of_phandle_args to encode the map output. Particularly given what I've
> just noticed below...
>
Sure... will make patches based on this...
>> + struct device *dev = arg;
>> + int err;
>> +
>> + err = of_iommu_xlate(dev, iommu_spec);
>> + of_node_put(iommu_spec->np);
>> + return err;
>> +}
>> +
>> static int of_iommu_configure_dev_id(struct device_node *master_np,
>> struct device *dev,
>> const u32 *id)
>> @@ -48,12 +61,10 @@ static int of_iommu_configure_dev_id(struct
>> device_node *master_np,
>> struct of_phandle_args iommu_spec = { .args_count = 1 };
>
> Oh dear, I totally overlooked this, and off the top of my head I'm not
> sure it's simple to fix 🙁
>
> So it's still not actually working as intended. Oh well, I did say it
> was untested...
I assume you are talking about the .args_count here, since of_map_id()
filling id_out, should we also pass the &args_count and let of_map_id()
fill that(with cell_count) or as you mentioned, pass the complete
of_phandle_args ?
Thanks,
Charan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU
2025-11-04 8:50 [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU Charan Teja Kalla
` (6 preceding siblings ...)
2025-11-05 17:28 ` [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU Robin Murphy
@ 2025-11-07 8:07 ` Krzysztof Kozlowski
2025-11-11 18:45 ` Charan Teja Kalla
7 siblings, 1 reply; 23+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-07 8:07 UTC (permalink / raw)
To: Charan Teja Kalla
Cc: robin.murphy, will, joro, robh, dmitry.baryshkov, konrad.dybcio,
bjorn.andersson, bod, conor+dt, krzk+dt, saravanak, prakash.gupta,
vikash.garodia, iommu, linux-kernel, devicetree
On Tue, Nov 04, 2025 at 02:20:59PM +0530, Charan Teja Kalla wrote:
> The iommu-map property has been defined for the PCIe usecase and has
> been hardcoded to assume single cell for IOMMU specification, ignoring
> the #iommu-cells completely. Since the initial definition the iommu-maps
> property has been reused for other usecases and we can no longer assume
> that the single IOMMU cell properly describes the necessary IOMMU
> streams. Expand the iommu-map to take #iommu-cells into account, while
> keeping the compatibility with the existing DTs, which assume single
> argument.
>
> Unlike single iommu-cell, it is complex to establish a linear relation
> between input 'id' and output specifier for multi iommu-cells. To handle
> such cases, rely on arch-specific drivers called through
> of_iommu_xlate() from of_iommu layer, aswell it is expected the 'len'
> passed is always 1. In the of_iommu layer, the below relation is
> established before calling into vendor specific driver:
>
> a) For platform devices, 'rid' defined in the iommu-map tuple indicates
> a function, through a bit position, which is compared against passed
> input 'id' that represents a bitmap of functions represented by the
> device.
>
> b) For others, 'rid' is compared against the input 'id' as an integer
> value.
>
> Thus the final representation when #iommu-cells=n is going to be,
> iommu-map = <rid/functionid IOMMU_phandle cell0 .. celln len>;, where
> len = 1.
>
> The RFC for this patch set is found at [2].
So that's a v2 or v3? Then number your patchsets correctly.
Try yourself - b4 diff cover.1762235099.git.charan.kalla@oss.qualcomm.com
Works? No.
Where is the changelog?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU
2025-11-07 8:07 ` Krzysztof Kozlowski
@ 2025-11-11 18:45 ` Charan Teja Kalla
2025-11-11 20:39 ` Rob Herring
0 siblings, 1 reply; 23+ messages in thread
From: Charan Teja Kalla @ 2025-11-11 18:45 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: robin.murphy, will, joro, robh, dmitry.baryshkov, konrad.dybcio,
bjorn.andersson, bod, conor+dt, krzk+dt, saravanak, prakash.gupta,
vikash.garodia, iommu, linux-kernel, devicetree
On 11/7/2025 1:37 PM, Krzysztof Kozlowski wrote:
> On Tue, Nov 04, 2025 at 02:20:59PM +0530, Charan Teja Kalla wrote:
>> The iommu-map property has been defined for the PCIe usecase and has
>> been hardcoded to assume single cell for IOMMU specification, ignoring
>> the #iommu-cells completely. Since the initial definition the iommu-maps
>> property has been reused for other usecases and we can no longer assume
>> that the single IOMMU cell properly describes the necessary IOMMU
>> streams. Expand the iommu-map to take #iommu-cells into account, while
>> keeping the compatibility with the existing DTs, which assume single
>> argument.
>>
>> Unlike single iommu-cell, it is complex to establish a linear relation
>> between input 'id' and output specifier for multi iommu-cells. To handle
>> such cases, rely on arch-specific drivers called through
>> of_iommu_xlate() from of_iommu layer, aswell it is expected the 'len'
>> passed is always 1. In the of_iommu layer, the below relation is
>> established before calling into vendor specific driver:
>>
>> a) For platform devices, 'rid' defined in the iommu-map tuple indicates
>> a function, through a bit position, which is compared against passed
>> input 'id' that represents a bitmap of functions represented by the
>> device.
>>
>> b) For others, 'rid' is compared against the input 'id' as an integer
>> value.
>>
>> Thus the final representation when #iommu-cells=n is going to be,
>> iommu-map = <rid/functionid IOMMU_phandle cell0 .. celln len>;, where
>> len = 1.
>>
>> The RFC for this patch set is found at [2].
> So that's a v2 or v3? Then number your patchsets correctly.
Is there any kernel guidelines that patchset should start at V2 after an
RFC? I do see many patches are follwed by V1 after RFC. Eg: [1] is an
RFC followed by [2] as V1 -- Or it is the maintainers preference and
expectations?
>
> Try yourself - b4 diff cover.1762235099.git.charan.kalla@oss.qualcomm.com
>
> Works? No.
>
> Where is the changelog?My bad. Will update the changelog from RFC in my next patchset posting
as V2.
[1]
https://lore.kernel.org/all/20250815191031.3769540-1-Liam.Howlett@oracle.com/
[2]
https://lore.kernel.org/all/20250909190945.1030905-1-Liam.Howlett@oracle.com/#t
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU
2025-11-11 18:45 ` Charan Teja Kalla
@ 2025-11-11 20:39 ` Rob Herring
0 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2025-11-11 20:39 UTC (permalink / raw)
To: Charan Teja Kalla
Cc: Krzysztof Kozlowski, robin.murphy, will, joro, dmitry.baryshkov,
konrad.dybcio, bjorn.andersson, bod, conor+dt, krzk+dt, saravanak,
prakash.gupta, vikash.garodia, iommu, linux-kernel, devicetree
On Tue, Nov 11, 2025 at 12:45 PM Charan Teja Kalla
<charan.kalla@oss.qualcomm.com> wrote:
>
>
>
> On 11/7/2025 1:37 PM, Krzysztof Kozlowski wrote:
> > On Tue, Nov 04, 2025 at 02:20:59PM +0530, Charan Teja Kalla wrote:
> >> The iommu-map property has been defined for the PCIe usecase and has
> >> been hardcoded to assume single cell for IOMMU specification, ignoring
> >> the #iommu-cells completely. Since the initial definition the iommu-maps
> >> property has been reused for other usecases and we can no longer assume
> >> that the single IOMMU cell properly describes the necessary IOMMU
> >> streams. Expand the iommu-map to take #iommu-cells into account, while
> >> keeping the compatibility with the existing DTs, which assume single
> >> argument.
> >>
> >> Unlike single iommu-cell, it is complex to establish a linear relation
> >> between input 'id' and output specifier for multi iommu-cells. To handle
> >> such cases, rely on arch-specific drivers called through
> >> of_iommu_xlate() from of_iommu layer, aswell it is expected the 'len'
> >> passed is always 1. In the of_iommu layer, the below relation is
> >> established before calling into vendor specific driver:
> >>
> >> a) For platform devices, 'rid' defined in the iommu-map tuple indicates
> >> a function, through a bit position, which is compared against passed
> >> input 'id' that represents a bitmap of functions represented by the
> >> device.
> >>
> >> b) For others, 'rid' is compared against the input 'id' as an integer
> >> value.
> >>
> >> Thus the final representation when #iommu-cells=n is going to be,
> >> iommu-map = <rid/functionid IOMMU_phandle cell0 .. celln len>;, where
> >> len = 1.
> >>
> >> The RFC for this patch set is found at [2].
> > So that's a v2 or v3? Then number your patchsets correctly.
>
> Is there any kernel guidelines that patchset should start at V2 after an
> RFC? I do see many patches are follwed by V1 after RFC. Eg: [1] is an
> RFC followed by [2] as V1 -- Or it is the maintainers preference and
> expectations?
RFC and version number are orthogonal. RFC is a condition the patches
are in. You can have "RFC v10", though I'd hope not. RFC is like
'draft' in Gitlab or Github PRs.
Rob
^ permalink raw reply [flat|nested] 23+ messages in thread