* [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
@ 2025-09-28 17:17 Charan Teja Kalla
2025-09-28 17:17 ` [RFC PATCH 1/3] dtbindings: add binding for iommu-map-masked property Charan Teja Kalla
` (4 more replies)
0 siblings, 5 replies; 36+ messages in thread
From: Charan Teja Kalla @ 2025-09-28 17:17 UTC (permalink / raw)
To: joro, will, robin.murphy, saravanak, conor+dt, robh, mchehab, bod,
krzk+dt, abhinav.kumar, vikash.garodia, dikshita.agarwal
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel, iommu,
Charan Teja Kalla
This series introduces a new iommu property called iommu-map-masked(may
be there is a better name), which is used to represent the IOMMU
specifier pairs for each function of a __multi-functional platform
device__, where each function can emit unique master id(s) that can be
associated with individual translation context.
Currently, the iommu configuration - at least for arm architecture-
requires all the functions of a platform device will be represented
under single dt node thus endup in using only a single translation
context.
A simple solution to associate individual translation context for each
function of a device can be through creating per function child nodes in
the device tree, but dt is only to just represent the soc layout to
linux kernel.
Supporting such cases requires a new iommu property called,
iommu-map-masked(taking cue from iommu-map for pci devices) and syntax
is:
iommu-map-masked = <FUNCTION_ID1 &iommu ID1 MASK1>,
<FUNCTION_ID2 &iommu ID2 MASK2>;
NOTE: As an RFC, it is considered that this property always expects 4
cells.
During the probe phase of the driver for a multi-functional device
behind an IOMMU, a child device is instantiated for each FUNCTION_ID.
The call to of_dma_configure_id() on each child sets up the IOMMU
configuration, ensuring that each function of the device is associated
with a distinct translation context.
This property can also be used in association with 'iommus=' when dt
bindings requires the presence of 'iommus=', example[2]. For these
cases, representation will be(on arm64):
iommus = <&iommu sid mask>; //for default function.
iommu-map-masked = <FUNCTION_ID &iommu sid mask>;//additional
function.
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 sub 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, it can be considered as iommus= property can be
associated with pixel functionality and iommu-map-masked= is with
non-pixel or viceversa.
[1] https://lore.kernel.org/all/20250627-video_cb-v3-0-51e18c0ffbce@quicinc.com/
Charan Teja Kalla (3):
dtbindings: add binding for iommu-map-masked property
of: create a wrapper for of_map_id()
of: implment the 'iommu-map-masked' to represent multi-functional
devices
.../bindings/media/qcom,sm8550-iris.yaml | 31 ++++++++++-
drivers/iommu/of_iommu.c | 44 +++++++++++++++
drivers/of/base.c | 55 ++++++++++++++++---
include/linux/of.h | 15 +++++
4 files changed, 133 insertions(+), 12 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [RFC PATCH 1/3] dtbindings: add binding for iommu-map-masked property
2025-09-28 17:17 [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices Charan Teja Kalla
@ 2025-09-28 17:17 ` Charan Teja Kalla
2025-09-28 17:17 ` [RFC PATCH 2/3] of: create a wrapper for of_map_id() Charan Teja Kalla
` (3 subsequent siblings)
4 siblings, 0 replies; 36+ messages in thread
From: Charan Teja Kalla @ 2025-09-28 17:17 UTC (permalink / raw)
To: joro, will, robin.murphy, saravanak, conor+dt, robh, mchehab, bod,
krzk+dt, abhinav.kumar, vikash.garodia, dikshita.agarwal
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel, iommu,
Charan Teja Kalla
Add bindings for iommu-map-masked property, which is used to represent
the IOMMU specifier pairs for each function of a multi-functional
platform device, where each function can emit unique master id that can
be associated with individual translation context. It contains syntax as
below:
iommu-map-masked = <FUNCTION_ID1 &iommu ID1 MASK1>,
<FUNCTION_ID2 &iommu ID2 MASK2>;
The right place for this binding to go is [1], but posting it here as
this is RFC. Please ignore the other concerns like ABI breakage, running
dt schema e.t.c.
[1] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/iommu/iommu.yaml
Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
---
.../bindings/media/qcom,sm8550-iris.yaml | 31 +++++++++++++++++--
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
index 9c4b760508b5..9940a3d7b8f9 100644
--- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
@@ -75,7 +75,7 @@ properties:
- const: core
iommus:
- maxItems: 2
+ maxItems: 1
dma-coherent: true
@@ -87,6 +87,20 @@ properties:
opp-table:
type: object
+ iommu-map-masked:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description:
+ Maps function IDs to IOMMU specifiers for multi-functional devices,
+ and each function requires its own address space/translation context.
+
+ Each entry contains:-
+ a) Function - ID Identifier for the device function.
+ b) IOMMU phandle - Identifier for iommu.
+ c) Arguments - arguments representing iommu handle.
+
+ items:
+ maxItems: 4
+
required:
- compatible
- power-domain-names
@@ -96,6 +110,7 @@ required:
- reset-names
- iommus
- dma-coherent
+ - iommu-map-masked
allOf:
- if:
@@ -116,6 +131,16 @@ allOf:
reset-names:
maxItems: 1
+ - if:
+ properties:
+ iommu-map-masked: true
+ then:
+ not:
+ anyOf:
+ - required: [iommu-map]
+ description:
+ iommu-map-masked is mutually exclusive with iommu-map property.
+
unevaluatedProperties: false
examples:
@@ -156,8 +181,8 @@ examples:
resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
reset-names = "bus";
- iommus = <&apps_smmu 0x1940 0x0000>,
- <&apps_smmu 0x1947 0x0000>;
+ iommus = <&apps_smmu 0x1940 0x0000>;
+ iommu-map-masked = <0x0 &apps_smmu 0x1947 0x0000>;
dma-coherent;
operating-points-v2 = <&iris_opp_table>;
--
2.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [RFC PATCH 2/3] of: create a wrapper for of_map_id()
2025-09-28 17:17 [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices Charan Teja Kalla
2025-09-28 17:17 ` [RFC PATCH 1/3] dtbindings: add binding for iommu-map-masked property Charan Teja Kalla
@ 2025-09-28 17:17 ` Charan Teja Kalla
2025-09-28 17:17 ` [RFC PATCH 3/3] of: implment the 'iommu-map-masked' to represent multi-functional devices Charan Teja Kalla
` (2 subsequent siblings)
4 siblings, 0 replies; 36+ messages in thread
From: Charan Teja Kalla @ 2025-09-28 17:17 UTC (permalink / raw)
To: joro, will, robin.murphy, saravanak, conor+dt, robh, mchehab, bod,
krzk+dt, abhinav.kumar, vikash.garodia, dikshita.agarwal
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel, iommu,
Charan Teja Kalla
Create a wrapper function for of_map_id(). This wrapper takes additional
params in the subsequent patches. No functional changes.
Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
---
drivers/of/base.c | 36 ++++++++++++++++++++++++++++--------
include/linux/of.h | 11 +++++++++++
2 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 7043acd971a0..ed2a924d1fab 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2054,17 +2054,11 @@ int of_find_last_cache_level(unsigned int cpu)
* @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.
+ * Look at the documentation of of_map_id().
*
* Return: 0 on success or a standard error code on failure.
*/
-int of_map_id(const struct device_node *np, u32 id,
+int of_map_id_and_mask(const struct device_node *np, u32 id,
const char *map_name, const char *map_mask_name,
struct device_node **target, u32 *id_out)
{
@@ -2149,4 +2143,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_and_mask(np, id, map_name, map_mask_name,
+ target, id_out);
+}
EXPORT_SYMBOL_GPL(of_map_id);
diff --git a/include/linux/of.h b/include/linux/of.h
index a62154aeda1b..6fcc46e8b3da 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -460,6 +460,10 @@ 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);
+int of_map_id_and_mask(const struct device_node *np, u32 id,
+ const char *map_name, const char *map_mask_name,
+ struct device_node **target, u32 *id_out);
+
phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);
struct kimage;
@@ -905,6 +909,13 @@ static inline int of_map_id(const struct device_node *np, u32 id,
return -EINVAL;
}
+static inline int of_map_id_and_mask(const struct device_node *np, u32 id,
+ const char *map_name, const char *map_mask_name,
+ struct device_node **target, u32 *id_out)
+{
+ 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] 36+ messages in thread
* [RFC PATCH 3/3] of: implment the 'iommu-map-masked' to represent multi-functional devices
2025-09-28 17:17 [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices Charan Teja Kalla
2025-09-28 17:17 ` [RFC PATCH 1/3] dtbindings: add binding for iommu-map-masked property Charan Teja Kalla
2025-09-28 17:17 ` [RFC PATCH 2/3] of: create a wrapper for of_map_id() Charan Teja Kalla
@ 2025-09-28 17:17 ` Charan Teja Kalla
2025-09-28 20:23 ` [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices Rob Herring
2025-09-29 10:20 ` Robin Murphy
4 siblings, 0 replies; 36+ messages in thread
From: Charan Teja Kalla @ 2025-09-28 17:17 UTC (permalink / raw)
To: joro, will, robin.murphy, saravanak, conor+dt, robh, mchehab, bod,
krzk+dt, abhinav.kumar, vikash.garodia, dikshita.agarwal
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel, iommu,
Charan Teja Kalla
The classical representation used by the platform device to represent
the IOMMU specifier pairs is through 'iommus ='. All the IOMMU specifier
pairs represented will be marked to use the single translation context,
at least is the case for ARM IOMMU.
When each functionality of a multi-functional device required to use
individual address spaces, for arm architecture, it need to be mentioned
as multiple sub-device nodes in the device tree. But since the device
tree is used to just pass on the soc layout to the linux kernel, it
doesn't truly fit.
Introduce the iommu-map-masked property(taking cue from iommu-map
for pci devices) where each functionality is represented(in arm64
architecture language) as:
iommu-map-masked = <FUNCTION_ID1 &iommu SID1 MASK1>,
<FUNCTION_ID2 &iommu SID2 MASK2>;
Iommu client drivers can dynamically create the child devices for each
of these function ID's and call of_dma_configure_id() on these child
devices which sets up the IOMMU configuration.
Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
---
drivers/iommu/of_iommu.c | 44 ++++++++++++++++++++++++++++++++++++++++
drivers/of/base.c | 21 +++++++++++++++++--
include/linux/of.h | 8 ++++++--
3 files changed, 69 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 6b989a62def2..2363de8f2fd6 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -41,12 +41,56 @@ static int of_iommu_xlate(struct device *dev,
return ret;
}
+static int of_iommu_map_id(const __be32 *map, u32 id,
+ struct device *dev, void *data)
+{
+ struct device_node *phandle_node;
+ struct of_phandle_args *iommu_spec = data;
+ u32 id_base = be32_to_cpup(map + 0);
+ u32 phandle = be32_to_cpup(map + 1);
+ u32 master_id0 = be32_to_cpup(map + 2);
+ u32 master_id1 = be32_to_cpup(map + 3);
+ int err;
+
+ phandle_node = of_find_node_by_phandle(phandle);
+ if (!phandle_node)
+ return -ENODEV;
+
+ if (id != id_base)
+ return -EAGAIN;
+
+ iommu_spec->np = phandle_node;
+ iommu_spec->args[0] = master_id0;
+ iommu_spec->args[1] = master_id1;
+
+ err = of_iommu_xlate(dev, iommu_spec);
+ of_node_put(iommu_spec->np);
+
+ return err;
+}
+
+static int of_iommu_configure_map_id_and_mask(struct device_node *master_np,
+ struct device *dev,
+ const u32 *id)
+{
+ struct of_phandle_args iommu_spec = { .args_count = 2 };
+
+ return of_map_id_and_mask(master_np, *id,
+ "iommu-map-masked", NULL,
+ &iommu_spec.np, NULL,
+ dev, (void *)&iommu_spec, of_iommu_map_id);
+}
+
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;
+ bool iommu_map_masked = !!of_find_property(master_np, "iommu-map-masked", NULL);
+
+ if (iommu_map_masked)
+ return of_iommu_configure_map_id_and_mask(master_np, dev, id);
err = of_map_id(master_np, *id, "iommu-map",
"iommu-map-mask", &iommu_spec.np,
diff --git a/drivers/of/base.c b/drivers/of/base.c
index ed2a924d1fab..bb11125e9624 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2053,6 +2053,9 @@ int of_find_last_cache_level(unsigned int cpu)
* @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.
+ * @dev: TODO
+ * @data: optional param that to be passed to fn.
+ * @fn: custom function to get implementation defined platform/device id.
*
* Look at the documentation of of_map_id().
*
@@ -2060,10 +2063,13 @@ int of_find_last_cache_level(unsigned int cpu)
*/
int of_map_id_and_mask(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,
+ struct device *dev, void *data,
+ int (*fn)(const __be32 *map, u32 id, struct device *dev, void *data))
{
u32 map_mask, masked_id;
int map_len;
+ int ret;
const __be32 *map = NULL;
if (!np || !map_name || (!target && !id_out))
@@ -2109,6 +2115,13 @@ int of_map_id_and_mask(const struct device_node *np, u32 id,
return -EFAULT;
}
+ if (fn) {
+ ret = fn(map, id, dev, data);
+ if (ret != -EAGAIN)
+ break;
+ continue;
+ }
+
if (masked_id < id_base || masked_id >= id_base + id_len)
continue;
@@ -2135,12 +2148,16 @@ int of_map_id_and_mask(const struct device_node *np, u32 id,
return 0;
}
+ if (fn)
+ return ret;
+
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;
}
@@ -2167,6 +2184,6 @@ int of_map_id(const struct device_node *np, u32 id,
struct device_node **target, u32 *id_out)
{
return of_map_id_and_mask(np, id, map_name, map_mask_name,
- target, id_out);
+ 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 6fcc46e8b3da..7f3890ab26d5 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -462,7 +462,9 @@ int of_map_id(const struct device_node *np, u32 id,
int of_map_id_and_mask(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,
+ struct device *dev, void *data,
+ int (*fn)(const __be32 *map, u32 id, struct device *dev, void *data));
phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);
@@ -911,7 +913,9 @@ static inline int of_map_id(const struct device_node *np, u32 id,
static inline int of_map_id_and_mask(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,
+ struct device *dev, void *data,
+ int (*fn)(const __be32 *map, u32 id, void *data))
{
return -EINVAL;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-09-28 17:17 [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices Charan Teja Kalla
` (2 preceding siblings ...)
2025-09-28 17:17 ` [RFC PATCH 3/3] of: implment the 'iommu-map-masked' to represent multi-functional devices Charan Teja Kalla
@ 2025-09-28 20:23 ` Rob Herring
2025-10-09 0:26 ` Krzysztof Kozlowski
2025-10-09 3:05 ` Vikash Garodia
2025-09-29 10:20 ` Robin Murphy
4 siblings, 2 replies; 36+ messages in thread
From: Rob Herring @ 2025-09-28 20:23 UTC (permalink / raw)
To: Charan Teja Kalla
Cc: joro, will, robin.murphy, saravanak, conor+dt, mchehab, bod,
krzk+dt, abhinav.kumar, vikash.garodia, dikshita.agarwal,
linux-media, linux-arm-msm, devicetree, linux-kernel, iommu
On Sun, Sep 28, 2025 at 12:17 PM Charan Teja Kalla
<charan.kalla@oss.qualcomm.com> wrote:
>
> This series introduces a new iommu property called iommu-map-masked(may
> be there is a better name), which is used to represent the IOMMU
> specifier pairs for each function of a __multi-functional platform
> device__, where each function can emit unique master id(s) that can be
> associated with individual translation context.
>
> Currently, the iommu configuration - at least for arm architecture-
> requires all the functions of a platform device will be represented
> under single dt node thus endup in using only a single translation
> context.
>
> A simple solution to associate individual translation context for each
> function of a device can be through creating per function child nodes in
> the device tree, but dt is only to just represent the soc layout to
> linux kernel.
>
> Supporting such cases requires a new iommu property called,
> iommu-map-masked(taking cue from iommu-map for pci devices) and syntax
> is:
> iommu-map-masked = <FUNCTION_ID1 &iommu ID1 MASK1>,
> <FUNCTION_ID2 &iommu ID2 MASK2>;
> NOTE: As an RFC, it is considered that this property always expects 4
> cells.
>
> During the probe phase of the driver for a multi-functional device
> behind an IOMMU, a child device is instantiated for each FUNCTION_ID.
> The call to of_dma_configure_id() on each child sets up the IOMMU
> configuration, ensuring that each function of the device is associated
> with a distinct translation context.
>
> This property can also be used in association with 'iommus=' when dt
> bindings requires the presence of 'iommus=', example[2]. For these
> cases, representation will be(on arm64):
> iommus = <&iommu sid mask>; //for default function.
> iommu-map-masked = <FUNCTION_ID &iommu sid mask>;//additional
> function.
Where does the FUNCTION_ID value come from?
Why can't you just have multiple "iommus" entries where the index
defines the default and any FUNCTION_ID entries? What's in each index
is specific to the device.
Rob
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-09-28 17:17 [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices Charan Teja Kalla
` (3 preceding siblings ...)
2025-09-28 20:23 ` [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices Rob Herring
@ 2025-09-29 10:20 ` Robin Murphy
2025-10-08 19:10 ` Charan Teja Kalla
4 siblings, 1 reply; 36+ messages in thread
From: Robin Murphy @ 2025-09-29 10:20 UTC (permalink / raw)
To: Charan Teja Kalla, joro, will, saravanak, conor+dt, robh, mchehab,
bod, krzk+dt, abhinav.kumar, vikash.garodia, dikshita.agarwal
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel, iommu
On 2025-09-28 6:17 pm, Charan Teja Kalla wrote:
> This series introduces a new iommu property called iommu-map-masked(may
> be there is a better name), which is used to represent the IOMMU
> specifier pairs for each function of a __multi-functional platform
> device__, where each function can emit unique master id(s) that can be
> associated with individual translation context.
>
> Currently, the iommu configuration - at least for arm architecture-
> requires all the functions of a platform device will be represented
> under single dt node thus endup in using only a single translation
> context.
>
> A simple solution to associate individual translation context for each
> function of a device can be through creating per function child nodes in
> the device tree, but dt is only to just represent the soc layout to
> linux kernel.
>
> Supporting such cases requires a new iommu property called,
> iommu-map-masked(taking cue from iommu-map for pci devices) and syntax
> is:
> iommu-map-masked = <FUNCTION_ID1 &iommu ID1 MASK1>,
> <FUNCTION_ID2 &iommu ID2 MASK2>;
> NOTE: As an RFC, it is considered that this property always expects 4
> cells.
>
> During the probe phase of the driver for a multi-functional device
> behind an IOMMU, a child device is instantiated for each FUNCTION_ID.
> The call to of_dma_configure_id() on each child sets up the IOMMU
> configuration, ensuring that each function of the device is associated
> with a distinct translation context.
>
> This property can also be used in association with 'iommus=' when dt
> bindings requires the presence of 'iommus=', example[2]. For these
> cases, representation will be(on arm64):
> iommus = <&iommu sid mask>; //for default function.
> iommu-map-masked = <FUNCTION_ID &iommu sid mask>;//additional
> function.
>
> 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 sub blocks are __configured__ to
> generate different stream IDs.
So please clarify why you can't:
a) Describe the sub-blocks as individual child nodes each with their own
distinct "iommus" property
or:
b) Use standard "iommu-map" which already supports mapping a masked
input ID to an arbitrary IOMMU specifier
Thanks,
Robin.
> 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, it can be considered as iommus= property can be
> associated with pixel functionality and iommu-map-masked= is with
> non-pixel or viceversa.
>
> [1] https://lore.kernel.org/all/20250627-video_cb-v3-0-51e18c0ffbce@quicinc.com/
>
> Charan Teja Kalla (3):
> dtbindings: add binding for iommu-map-masked property
> of: create a wrapper for of_map_id()
> of: implment the 'iommu-map-masked' to represent multi-functional
> devices
>
> .../bindings/media/qcom,sm8550-iris.yaml | 31 ++++++++++-
> drivers/iommu/of_iommu.c | 44 +++++++++++++++
> drivers/of/base.c | 55 ++++++++++++++++---
> include/linux/of.h | 15 +++++
> 4 files changed, 133 insertions(+), 12 deletions(-)
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-09-29 10:20 ` Robin Murphy
@ 2025-10-08 19:10 ` Charan Teja Kalla
2025-10-09 10:46 ` Robin Murphy
0 siblings, 1 reply; 36+ messages in thread
From: Charan Teja Kalla @ 2025-10-08 19:10 UTC (permalink / raw)
To: Robin Murphy, joro, will, saravanak, conor+dt, robh, mchehab, bod,
krzk+dt, abhinav.kumar, vikash.garodia, dikshita.agarwal,
Konrad Dybcio, bjorn.andersson, Dmitry Baryshkov
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel, iommu
On 9/29/2025 3:50 PM, Robin Murphy wrote:
>> 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 sub blocks are __configured__ to
>> generate different stream IDs.
>
> So please clarify why you can't:
>
> a) Describe the sub-blocks as individual child nodes each with their own
> distinct "iommus" property
>
Thanks Robin for your time. Sorry for late reply as I really didn't have
concrete answer for this question.
First let me clarify the word "sub blocks" -- This is just the logical
separation with no separate address space to really able to define them
as sub devices. Think of it like a single video IP with 2 dma
engines(used for pixel and non-pixel purpose).
I should agree that the child-nodes in the device tree is the easy one
and infact, it is how being used in downstream.
For upstream -- Since there is no real address space to interact with
these sub-blocks(or logical blocks), does it really qualify to define as
child nodes in the device tree? I see there is some push back[1].
> or:
>
> b) Use standard "iommu-map" which already supports mapping a masked
> input ID to an arbitrary IOMMU specifier
>
I think clients is also required to program non-zero smr mask, where as
iommu-map just maps the id to an IOMMU specifier(sid). Please LMK if I
am unable to catch your thought here.
Do you suggest to extend the iommu-map to give the non-zero SMR mask
value[2]?
[1]
https://lore.kernel.org/all/ogslvjglnz56lz6nria7py6i4ccp6zvcd4ujpiusrq6xutydsm@xb72os5wk67r/#t
[2]
https://lore.kernel.org/all/bffc8478-4de9-4a9b-9248-96a936f20096@oss.qualcomm.com/>
Thanks,
> Robin.
>
>> 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.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-09-28 20:23 ` [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices Rob Herring
@ 2025-10-09 0:26 ` Krzysztof Kozlowski
2025-10-09 12:16 ` Robin Murphy
2025-10-09 13:14 ` Dmitry Baryshkov
2025-10-09 3:05 ` Vikash Garodia
1 sibling, 2 replies; 36+ messages in thread
From: Krzysztof Kozlowski @ 2025-10-09 0:26 UTC (permalink / raw)
To: Rob Herring, Charan Teja Kalla
Cc: joro, will, robin.murphy, saravanak, conor+dt, mchehab, bod,
krzk+dt, abhinav.kumar, vikash.garodia, dikshita.agarwal,
linux-media, linux-arm-msm, devicetree, linux-kernel, iommu
On 29/09/2025 05:23, Rob Herring wrote:
> On Sun, Sep 28, 2025 at 12:17 PM Charan Teja Kalla
> <charan.kalla@oss.qualcomm.com> wrote:
>>
>> This series introduces a new iommu property called iommu-map-masked(may
>> be there is a better name), which is used to represent the IOMMU
>> specifier pairs for each function of a __multi-functional platform
>> device__, where each function can emit unique master id(s) that can be
>> associated with individual translation context.
>>
>> Currently, the iommu configuration - at least for arm architecture-
>> requires all the functions of a platform device will be represented
>> under single dt node thus endup in using only a single translation
>> context.
>>
>> A simple solution to associate individual translation context for each
>> function of a device can be through creating per function child nodes in
>> the device tree, but dt is only to just represent the soc layout to
>> linux kernel.
>>
>> Supporting such cases requires a new iommu property called,
>> iommu-map-masked(taking cue from iommu-map for pci devices) and syntax
>> is:
>> iommu-map-masked = <FUNCTION_ID1 &iommu ID1 MASK1>,
>> <FUNCTION_ID2 &iommu ID2 MASK2>;
>> NOTE: As an RFC, it is considered that this property always expects 4
>> cells.
>>
>> During the probe phase of the driver for a multi-functional device
>> behind an IOMMU, a child device is instantiated for each FUNCTION_ID.
>> The call to of_dma_configure_id() on each child sets up the IOMMU
>> configuration, ensuring that each function of the device is associated
>> with a distinct translation context.
>>
>> This property can also be used in association with 'iommus=' when dt
>> bindings requires the presence of 'iommus=', example[2]. For these
>> cases, representation will be(on arm64):
>> iommus = <&iommu sid mask>; //for default function.
>> iommu-map-masked = <FUNCTION_ID &iommu sid mask>;//additional
>> function.
>
> Where does the FUNCTION_ID value come from?
>
> Why can't you just have multiple "iommus" entries where the index
> defines the default and any FUNCTION_ID entries? What's in each index
> is specific to the device.
We discussed the problem earlier and that is what I asked them to do.
Apparently I was just ignored so now two maintainers say the same. We
can get ignored still and the third maintainer will have to tell this.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-09-28 20:23 ` [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices Rob Herring
2025-10-09 0:26 ` Krzysztof Kozlowski
@ 2025-10-09 3:05 ` Vikash Garodia
1 sibling, 0 replies; 36+ messages in thread
From: Vikash Garodia @ 2025-10-09 3:05 UTC (permalink / raw)
To: Rob Herring, Charan Teja Kalla
Cc: joro, will, robin.murphy, saravanak, conor+dt, mchehab, bod,
krzk+dt, abhinav.kumar, dikshita.agarwal, linux-media,
linux-arm-msm, devicetree, linux-kernel, iommu,
Bryan O'Donoghue
On 9/29/2025 1:53 AM, Rob Herring wrote:
> On Sun, Sep 28, 2025 at 12:17 PM Charan Teja Kalla
> <charan.kalla@oss.qualcomm.com> wrote:
>>
>> This series introduces a new iommu property called iommu-map-masked(may
>> be there is a better name), which is used to represent the IOMMU
>> specifier pairs for each function of a __multi-functional platform
>> device__, where each function can emit unique master id(s) that can be
>> associated with individual translation context.
>>
>> Currently, the iommu configuration - at least for arm architecture-
>> requires all the functions of a platform device will be represented
>> under single dt node thus endup in using only a single translation
>> context.
>>
>> A simple solution to associate individual translation context for each
>> function of a device can be through creating per function child nodes in
>> the device tree, but dt is only to just represent the soc layout to
>> linux kernel.
>>
>> Supporting such cases requires a new iommu property called,
>> iommu-map-masked(taking cue from iommu-map for pci devices) and syntax
>> is:
>> iommu-map-masked = <FUNCTION_ID1 &iommu ID1 MASK1>,
>> <FUNCTION_ID2 &iommu ID2 MASK2>;
>> NOTE: As an RFC, it is considered that this property always expects 4
>> cells.
>>
>> During the probe phase of the driver for a multi-functional device
>> behind an IOMMU, a child device is instantiated for each FUNCTION_ID.
>> The call to of_dma_configure_id() on each child sets up the IOMMU
>> configuration, ensuring that each function of the device is associated
>> with a distinct translation context.
>>
>> This property can also be used in association with 'iommus=' when dt
>> bindings requires the presence of 'iommus=', example[2]. For these
>> cases, representation will be(on arm64):
>> iommus = <&iommu sid mask>; //for default function.
>> iommu-map-masked = <FUNCTION_ID &iommu sid mask>;//additional
>> function.
>
> Where does the FUNCTION_ID value come from?
>
> Why can't you just have multiple "iommus" entries where the index
> defines the default and any FUNCTION_ID entries? What's in each index
> is specific to the device.
Are you trying to suggest something like this [1] ? I am not sure, if extending
the iommus would get us "unique" devices where those SIDs (from different
function_id) can be associated with respective device. AFAIU, existing iommus
entries associates all of them in same device.
[1]
https://lore.kernel.org/linux-media/9bae595a-597e-46e6-8eb2-44424fe21db6@linaro.org/
Regards,
Vikash
>
> Rob
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-08 19:10 ` Charan Teja Kalla
@ 2025-10-09 10:46 ` Robin Murphy
2025-10-09 13:19 ` Dmitry Baryshkov
0 siblings, 1 reply; 36+ messages in thread
From: Robin Murphy @ 2025-10-09 10:46 UTC (permalink / raw)
To: Charan Teja Kalla, joro, will, saravanak, conor+dt, robh, mchehab,
bod, krzk+dt, abhinav.kumar, vikash.garodia, dikshita.agarwal,
Konrad Dybcio, bjorn.andersson, Dmitry Baryshkov
Cc: linux-media, linux-arm-msm, devicetree, linux-kernel, iommu
On 2025-10-08 8:10 pm, Charan Teja Kalla wrote:
>
> On 9/29/2025 3:50 PM, Robin Murphy wrote:
>>> 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 sub blocks are __configured__ to
>>> generate different stream IDs.
>>
>> So please clarify why you can't:
>>
>> a) Describe the sub-blocks as individual child nodes each with their own
>> distinct "iommus" property
>>
>
> Thanks Robin for your time. Sorry for late reply as I really didn't have
> concrete answer for this question.
>
> First let me clarify the word "sub blocks" -- This is just the logical
> separation with no separate address space to really able to define them
> as sub devices. Think of it like a single video IP with 2 dma
> engines(used for pixel and non-pixel purpose).
>
> I should agree that the child-nodes in the device tree is the easy one
> and infact, it is how being used in downstream.
>
> For upstream -- Since there is no real address space to interact with
> these sub-blocks(or logical blocks), does it really qualify to define as
> child nodes in the device tree? I see there is some push back[1].
Who says you need an address space? Child nodes without "reg"
properties, referenced by name, compatible or phandle, exist all over
the place for all manner of reasons. If there are distinct logical
functions with their own distinct hardware properties, then I would say
having child nodes to describe and associate those properties with their
respective functions is entirely natural and appropriate. The first
example that comes to mind of where this is a well-established practice
is PMICs - to pick one at random:
Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml
For bonus irony, you can't take the other approaches without inherently
*introducing* a notional address space in the form of your logical
function IDs anyway.
> > or:
>>
>> b) Use standard "iommu-map" which already supports mapping a masked
>> input ID to an arbitrary IOMMU specifier
>>
>
> I think clients is also required to program non-zero smr mask, where as
> iommu-map just maps the id to an IOMMU specifier(sid). Please LMK if I
> am unable to catch your thought here.
An IOMMU specifier is whatever the target IOMMU node's #iommu-cells says
it is. The fact that Linux's parsing code only works properly for
#iommu-cells = 1 is not really a DT binding problem (other than it
stemming from a loose assumption stated in the PCI binding's use of the
property).
However, I still lean toward the first approach, as this definitely
seems like it wants to be one overall device with a level of internal
hierarchy, rather than a full-blown bus abstraction.
Thanks,
Robin.
>
> Do you suggest to extend the iommu-map to give the non-zero SMR mask
> value[2]?
>
>
> [1]
> https://lore.kernel.org/all/ogslvjglnz56lz6nria7py6i4ccp6zvcd4ujpiusrq6xutydsm@xb72os5wk67r/#t
>
> [2]
> https://lore.kernel.org/all/bffc8478-4de9-4a9b-9248-96a936f20096@oss.qualcomm.com/>
> Thanks,
>> Robin.
>>
>>> 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.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-09 0:26 ` Krzysztof Kozlowski
@ 2025-10-09 12:16 ` Robin Murphy
2025-10-09 13:16 ` Dmitry Baryshkov
2025-10-09 13:14 ` Dmitry Baryshkov
1 sibling, 1 reply; 36+ messages in thread
From: Robin Murphy @ 2025-10-09 12:16 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rob Herring, Charan Teja Kalla
Cc: joro, will, saravanak, conor+dt, mchehab, bod, krzk+dt,
abhinav.kumar, vikash.garodia, dikshita.agarwal, linux-media,
linux-arm-msm, devicetree, linux-kernel, iommu
On 2025-10-09 1:26 am, Krzysztof Kozlowski wrote:
> On 29/09/2025 05:23, Rob Herring wrote:
>> On Sun, Sep 28, 2025 at 12:17 PM Charan Teja Kalla
>> <charan.kalla@oss.qualcomm.com> wrote:
>>>
>>> This series introduces a new iommu property called iommu-map-masked(may
>>> be there is a better name), which is used to represent the IOMMU
>>> specifier pairs for each function of a __multi-functional platform
>>> device__, where each function can emit unique master id(s) that can be
>>> associated with individual translation context.
>>>
>>> Currently, the iommu configuration - at least for arm architecture-
>>> requires all the functions of a platform device will be represented
>>> under single dt node thus endup in using only a single translation
>>> context.
>>>
>>> A simple solution to associate individual translation context for each
>>> function of a device can be through creating per function child nodes in
>>> the device tree, but dt is only to just represent the soc layout to
>>> linux kernel.
>>>
>>> Supporting such cases requires a new iommu property called,
>>> iommu-map-masked(taking cue from iommu-map for pci devices) and syntax
>>> is:
>>> iommu-map-masked = <FUNCTION_ID1 &iommu ID1 MASK1>,
>>> <FUNCTION_ID2 &iommu ID2 MASK2>;
>>> NOTE: As an RFC, it is considered that this property always expects 4
>>> cells.
>>>
>>> During the probe phase of the driver for a multi-functional device
>>> behind an IOMMU, a child device is instantiated for each FUNCTION_ID.
>>> The call to of_dma_configure_id() on each child sets up the IOMMU
>>> configuration, ensuring that each function of the device is associated
>>> with a distinct translation context.
>>>
>>> This property can also be used in association with 'iommus=' when dt
>>> bindings requires the presence of 'iommus=', example[2]. For these
>>> cases, representation will be(on arm64):
>>> iommus = <&iommu sid mask>; //for default function.
>>> iommu-map-masked = <FUNCTION_ID &iommu sid mask>;//additional
>>> function.
>>
>> Where does the FUNCTION_ID value come from?
>>
>> Why can't you just have multiple "iommus" entries where the index
>> defines the default and any FUNCTION_ID entries? What's in each index
>> is specific to the device.
>
>
> We discussed the problem earlier and that is what I asked them to do.
> Apparently I was just ignored so now two maintainers say the same. We
> can get ignored still and the third maintainer will have to tell this.
The reason why that isn't really viable is that the "iommus" property
needs to be consumed directly by the relevant IOMMU driver(s) without a
dependency on any driver for the client device represented by the node
itself. For security/isolation purposes the IOMMU has to know about all
potential DMA sources and be able to be configured for them *before*
anyone else gets a chance to start initiating DMA from them. If the DT
consumer is, say, a bare-metal hypervisor, it may have no understanding
whatsoever of what most of the client devices are nor how they work.
This is part of the reason why we went for a separate "iommu-map"
property for buses, so that an IOMMU consumer *can* easily tell when
multiple specifiers do not represent an indivisible set tied to the
given device, and thus it can expect help from a bus driver to subdivide
them later (but in the meantime can still safely isolate the entire bus
based on the output of the map without having to understand the inputs.)
Now, I suppose in some cases it might be technically possible for a
client device driver to collude with an IOMMU driver to divide a
monolithic DT device into logical functions after the fact, but for
Linux I don't see an acceptable way of doing that without some major
changes to the driver model abstraction and IOMMU API...
Thanks,
Robin.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-09 0:26 ` Krzysztof Kozlowski
2025-10-09 12:16 ` Robin Murphy
@ 2025-10-09 13:14 ` Dmitry Baryshkov
1 sibling, 0 replies; 36+ messages in thread
From: Dmitry Baryshkov @ 2025-10-09 13:14 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Charan Teja Kalla, joro, will, robin.murphy,
saravanak, conor+dt, mchehab, bod, krzk+dt, abhinav.kumar,
vikash.garodia, dikshita.agarwal, linux-media, linux-arm-msm,
devicetree, linux-kernel, iommu
On Thu, Oct 09, 2025 at 09:26:43AM +0900, Krzysztof Kozlowski wrote:
> On 29/09/2025 05:23, Rob Herring wrote:
> > On Sun, Sep 28, 2025 at 12:17 PM Charan Teja Kalla
> > <charan.kalla@oss.qualcomm.com> wrote:
> >>
> >> This series introduces a new iommu property called iommu-map-masked(may
> >> be there is a better name), which is used to represent the IOMMU
> >> specifier pairs for each function of a __multi-functional platform
> >> device__, where each function can emit unique master id(s) that can be
> >> associated with individual translation context.
> >>
> >> Currently, the iommu configuration - at least for arm architecture-
> >> requires all the functions of a platform device will be represented
> >> under single dt node thus endup in using only a single translation
> >> context.
> >>
> >> A simple solution to associate individual translation context for each
> >> function of a device can be through creating per function child nodes in
> >> the device tree, but dt is only to just represent the soc layout to
> >> linux kernel.
> >>
> >> Supporting such cases requires a new iommu property called,
> >> iommu-map-masked(taking cue from iommu-map for pci devices) and syntax
> >> is:
> >> iommu-map-masked = <FUNCTION_ID1 &iommu ID1 MASK1>,
> >> <FUNCTION_ID2 &iommu ID2 MASK2>;
> >> NOTE: As an RFC, it is considered that this property always expects 4
> >> cells.
> >>
> >> During the probe phase of the driver for a multi-functional device
> >> behind an IOMMU, a child device is instantiated for each FUNCTION_ID.
> >> The call to of_dma_configure_id() on each child sets up the IOMMU
> >> configuration, ensuring that each function of the device is associated
> >> with a distinct translation context.
> >>
> >> This property can also be used in association with 'iommus=' when dt
> >> bindings requires the presence of 'iommus=', example[2]. For these
> >> cases, representation will be(on arm64):
> >> iommus = <&iommu sid mask>; //for default function.
> >> iommu-map-masked = <FUNCTION_ID &iommu sid mask>;//additional
> >> function.
> >
> > Where does the FUNCTION_ID value come from?
> >
> > Why can't you just have multiple "iommus" entries where the index
> > defines the default and any FUNCTION_ID entries? What's in each index
> > is specific to the device.
>
>
> We discussed the problem earlier and that is what I asked them to do.
> Apparently I was just ignored so now two maintainers say the same. We
> can get ignored still and the third maintainer will have to tell this.
The main problem (which comes from IOMMU definition) is that currently
the iommus property is not defined nor used as an ordered list or
anything like that. Other devices depend on it being a set with no
additional structure. We can change that, but it might potentially
affect others.
The iommu-maps is e.g. used by Tegra display device to map multiple
contexts separately, but it doesn't fit all the needs because it doesn't
allow us to specify the mask.
Also, the video-codec is not unique, we have other similar usecases, the
display, camera and GPU, which also need to map some of the contexts
manually.
Last, but not least, there are e.g. fastrpc devices which have
subdevices just to declare the IOMMU entry for the context stream. I
would very much prefer to be able to drop the subnodes in a longer term.
Speaking from the drivers point of view, we also don't have any control
on how the IOMMUs are attached, while we need to control it for these
kind of contexts.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-09 12:16 ` Robin Murphy
@ 2025-10-09 13:16 ` Dmitry Baryshkov
0 siblings, 0 replies; 36+ messages in thread
From: Dmitry Baryshkov @ 2025-10-09 13:16 UTC (permalink / raw)
To: Robin Murphy
Cc: Krzysztof Kozlowski, Rob Herring, Charan Teja Kalla, joro, will,
saravanak, conor+dt, mchehab, bod, krzk+dt, abhinav.kumar,
vikash.garodia, dikshita.agarwal, linux-media, linux-arm-msm,
devicetree, linux-kernel, iommu
On Thu, Oct 09, 2025 at 01:16:10PM +0100, Robin Murphy wrote:
> On 2025-10-09 1:26 am, Krzysztof Kozlowski wrote:
> > On 29/09/2025 05:23, Rob Herring wrote:
> > > On Sun, Sep 28, 2025 at 12:17 PM Charan Teja Kalla
> > > <charan.kalla@oss.qualcomm.com> wrote:
> > > >
> > > > This series introduces a new iommu property called iommu-map-masked(may
> > > > be there is a better name), which is used to represent the IOMMU
> > > > specifier pairs for each function of a __multi-functional platform
> > > > device__, where each function can emit unique master id(s) that can be
> > > > associated with individual translation context.
> > > >
> > > > Currently, the iommu configuration - at least for arm architecture-
> > > > requires all the functions of a platform device will be represented
> > > > under single dt node thus endup in using only a single translation
> > > > context.
> > > >
> > > > A simple solution to associate individual translation context for each
> > > > function of a device can be through creating per function child nodes in
> > > > the device tree, but dt is only to just represent the soc layout to
> > > > linux kernel.
> > > >
> > > > Supporting such cases requires a new iommu property called,
> > > > iommu-map-masked(taking cue from iommu-map for pci devices) and syntax
> > > > is:
> > > > iommu-map-masked = <FUNCTION_ID1 &iommu ID1 MASK1>,
> > > > <FUNCTION_ID2 &iommu ID2 MASK2>;
> > > > NOTE: As an RFC, it is considered that this property always expects 4
> > > > cells.
> > > >
> > > > During the probe phase of the driver for a multi-functional device
> > > > behind an IOMMU, a child device is instantiated for each FUNCTION_ID.
> > > > The call to of_dma_configure_id() on each child sets up the IOMMU
> > > > configuration, ensuring that each function of the device is associated
> > > > with a distinct translation context.
> > > >
> > > > This property can also be used in association with 'iommus=' when dt
> > > > bindings requires the presence of 'iommus=', example[2]. For these
> > > > cases, representation will be(on arm64):
> > > > iommus = <&iommu sid mask>; //for default function.
> > > > iommu-map-masked = <FUNCTION_ID &iommu sid mask>;//additional
> > > > function.
> > >
> > > Where does the FUNCTION_ID value come from?
> > >
> > > Why can't you just have multiple "iommus" entries where the index
> > > defines the default and any FUNCTION_ID entries? What's in each index
> > > is specific to the device.
> >
> >
> > We discussed the problem earlier and that is what I asked them to do.
> > Apparently I was just ignored so now two maintainers say the same. We
> > can get ignored still and the third maintainer will have to tell this.
>
> The reason why that isn't really viable is that the "iommus" property needs
> to be consumed directly by the relevant IOMMU driver(s) without a dependency
> on any driver for the client device represented by the node itself. For
> security/isolation purposes the IOMMU has to know about all potential DMA
> sources and be able to be configured for them *before* anyone else gets a
> chance to start initiating DMA from them. If the DT consumer is, say, a
> bare-metal hypervisor, it may have no understanding whatsoever of what most
> of the client devices are nor how they work.
>
> This is part of the reason why we went for a separate "iommu-map" property
> for buses, so that an IOMMU consumer *can* easily tell when multiple
> specifiers do not represent an indivisible set tied to the given device, and
> thus it can expect help from a bus driver to subdivide them later (but in
> the meantime can still safely isolate the entire bus based on the output of
> the map without having to understand the inputs.)
>
> Now, I suppose in some cases it might be technically possible for a client
> device driver to collude with an IOMMU driver to divide a monolithic DT
> device into logical functions after the fact, but for Linux I don't see an
> acceptable way of doing that without some major changes to the driver model
> abstraction and IOMMU API...
Tegra host1x handled this via iommu-map, but that's only because they
don't need to specify SMR masks. That was the reason why we modelled
iommu-map-masked in this way - to follow the approach established by the
Tegra display drivers but in a way useable on a platforms where we need
to specify the mask rather than the number of SIDs.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-09 10:46 ` Robin Murphy
@ 2025-10-09 13:19 ` Dmitry Baryshkov
2025-10-09 17:03 ` Robin Murphy
0 siblings, 1 reply; 36+ messages in thread
From: Dmitry Baryshkov @ 2025-10-09 13:19 UTC (permalink / raw)
To: Robin Murphy
Cc: Charan Teja Kalla, joro, will, saravanak, conor+dt, robh, mchehab,
bod, krzk+dt, abhinav.kumar, vikash.garodia, dikshita.agarwal,
Konrad Dybcio, bjorn.andersson, linux-media, linux-arm-msm,
devicetree, linux-kernel, iommu
On Thu, Oct 09, 2025 at 11:46:55AM +0100, Robin Murphy wrote:
> On 2025-10-08 8:10 pm, Charan Teja Kalla wrote:
> >
> > On 9/29/2025 3:50 PM, Robin Murphy wrote:
> > > > 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 sub blocks are __configured__ to
> > > > generate different stream IDs.
> > >
> > > So please clarify why you can't:
> > >
> > > a) Describe the sub-blocks as individual child nodes each with their own
> > > distinct "iommus" property
> > >
> >
> > Thanks Robin for your time. Sorry for late reply as I really didn't have
> > concrete answer for this question.
> >
> > First let me clarify the word "sub blocks" -- This is just the logical
> > separation with no separate address space to really able to define them
> > as sub devices. Think of it like a single video IP with 2 dma
> > engines(used for pixel and non-pixel purpose).
> >
> > I should agree that the child-nodes in the device tree is the easy one
> > and infact, it is how being used in downstream.
> >
> > For upstream -- Since there is no real address space to interact with
> > these sub-blocks(or logical blocks), does it really qualify to define as
> > child nodes in the device tree? I see there is some push back[1].
>
> Who says you need an address space? Child nodes without "reg" properties,
> referenced by name, compatible or phandle, exist all over the place for all
> manner of reasons. If there are distinct logical functions with their own
> distinct hardware properties, then I would say having child nodes to
> describe and associate those properties with their respective functions is
> entirely natural and appropriate. The first example that comes to mind of
> where this is a well-established practice is PMICs - to pick one at random:
> Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml
Logical function, that's correct. And also note, for PMICs that practice
has bitten us back. For PM8008 we switched back to a non-subdevice
representation.
> For bonus irony, you can't take the other approaches without inherently
> *introducing* a notional address space in the form of your logical function
> IDs anyway.
>
> > > or:
> > >
> > > b) Use standard "iommu-map" which already supports mapping a masked
> > > input ID to an arbitrary IOMMU specifier
> > >
> >
> > I think clients is also required to program non-zero smr mask, where as
> > iommu-map just maps the id to an IOMMU specifier(sid). Please LMK if I
> > am unable to catch your thought here.
> An IOMMU specifier is whatever the target IOMMU node's #iommu-cells says it
> is. The fact that Linux's parsing code only works properly for #iommu-cells
> = 1 is not really a DT binding problem (other than it stemming from a loose
> assumption stated in the PCI binding's use of the property).
I really don't like the idea of extending the #iommu-cells. The ARM SMMU
has only one cell, which is correct even for our platforms. The fact
that we need to identify different IOMMU SIDs (and handle them in a
differnt ways) is internal to the video device (and several other
devices). There is nothing to be handled on the ARM SMMU side.
>
> However, I still lean toward the first approach, as this definitely seems
> like it wants to be one overall device with a level of internal hierarchy,
> rather than a full-blown bus abstraction.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-09 13:19 ` Dmitry Baryshkov
@ 2025-10-09 17:03 ` Robin Murphy
2025-10-09 18:25 ` Dmitry Baryshkov
0 siblings, 1 reply; 36+ messages in thread
From: Robin Murphy @ 2025-10-09 17:03 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Charan Teja Kalla, joro, will, saravanak, conor+dt, robh, mchehab,
bod, krzk+dt, abhinav.kumar, vikash.garodia, dikshita.agarwal,
Konrad Dybcio, bjorn.andersson, linux-media, linux-arm-msm,
devicetree, linux-kernel, iommu
On 2025-10-09 2:19 pm, Dmitry Baryshkov wrote:
> On Thu, Oct 09, 2025 at 11:46:55AM +0100, Robin Murphy wrote:
>> On 2025-10-08 8:10 pm, Charan Teja Kalla wrote:
>>>
>>> On 9/29/2025 3:50 PM, Robin Murphy wrote:
>>>>> 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 sub blocks are __configured__ to
>>>>> generate different stream IDs.
>>>>
>>>> So please clarify why you can't:
>>>>
>>>> a) Describe the sub-blocks as individual child nodes each with their own
>>>> distinct "iommus" property
>>>>
>>>
>>> Thanks Robin for your time. Sorry for late reply as I really didn't have
>>> concrete answer for this question.
>>>
>>> First let me clarify the word "sub blocks" -- This is just the logical
>>> separation with no separate address space to really able to define them
>>> as sub devices. Think of it like a single video IP with 2 dma
>>> engines(used for pixel and non-pixel purpose).
>>>
>>> I should agree that the child-nodes in the device tree is the easy one
>>> and infact, it is how being used in downstream.
>>>
>>> For upstream -- Since there is no real address space to interact with
>>> these sub-blocks(or logical blocks), does it really qualify to define as
>>> child nodes in the device tree? I see there is some push back[1].
>>
>> Who says you need an address space? Child nodes without "reg" properties,
>> referenced by name, compatible or phandle, exist all over the place for all
>> manner of reasons. If there are distinct logical functions with their own
>> distinct hardware properties, then I would say having child nodes to
>> describe and associate those properties with their respective functions is
>> entirely natural and appropriate. The first example that comes to mind of
>> where this is a well-established practice is PMICs - to pick one at random:
>> Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml
>
> Logical function, that's correct. And also note, for PMICs that practice
> has bitten us back. For PM8008 we switched back to a non-subdevice
> representation.
>
>> For bonus irony, you can't take the other approaches without inherently
>> *introducing* a notional address space in the form of your logical function
>> IDs anyway.
>>
>>> > or:
>>>>
>>>> b) Use standard "iommu-map" which already supports mapping a masked
>>>> input ID to an arbitrary IOMMU specifier
>>>>
>>>
>>> I think clients is also required to program non-zero smr mask, where as
>>> iommu-map just maps the id to an IOMMU specifier(sid). Please LMK if I
>>> am unable to catch your thought here.
>> An IOMMU specifier is whatever the target IOMMU node's #iommu-cells says it
>> is. The fact that Linux's parsing code only works properly for #iommu-cells
>> = 1 is not really a DT binding problem (other than it stemming from a loose
>> assumption stated in the PCI binding's use of the property).
>
> I really don't like the idea of extending the #iommu-cells. The ARM SMMU
> has only one cell, which is correct even for our platforms. The fact
> that we need to identify different IOMMU SIDs (and handle them in a
> differnt ways) is internal to the video device (and several other
> devices). There is nothing to be handled on the ARM SMMU side.
Huh? So if you prefer not to change anything, are you suggesting this
series doesn't need to exist at all? Now I'm thoroughly confused...
If you want to use SMR masks, then you absolutely need #iommu-cells = 2,
because that is the SMMU binding for using SMR masks. It would
definitely not be OK to have some magic property trying to smuggle
IOMMU-driver-specific data contrary to what the IOMMU node itself says.
As for iommu-map, I don't see what would be objectionable about
improving the parsing to respect a real #iommu-cells value rather than
hard-coding an assumption. Yes, we'd probably need to forbid entries
with length > 1 targeting IOMMUs with #iommu-cells > 1, since the notion
of a linear relationship between the input ID and the output specifier
falls apart when the specifier is complex, but that seems simple enough
to implement and document (even if it's too fiddly to describe in the
schema itself), and still certainly no worse than having another
property that *is* just iommu-map with implicit length = 1.
And if you want individual StreamIDs for logical functions to be
attachable to distinct contexts then those functions absolutely must be
visible to the IOMMU layer and the SMMU driver as independent devices
with their own unique properties, which means either they come that way
from the DT as of_platform devices in the first place, or you implement
a full bus_type abstraction which will have to be hooked up to the IOMMU
layer. You cannot make IOMMU configuration "internal" to the actual
client driver which is only allowed to bind *after* said IOMMU
configuration has already been made.
Thanks,
Robin.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-09 17:03 ` Robin Murphy
@ 2025-10-09 18:25 ` Dmitry Baryshkov
2025-10-10 19:53 ` Charan Teja Kalla
2025-10-13 11:20 ` Robin Murphy
0 siblings, 2 replies; 36+ messages in thread
From: Dmitry Baryshkov @ 2025-10-09 18:25 UTC (permalink / raw)
To: Robin Murphy
Cc: Charan Teja Kalla, joro, will, saravanak, conor+dt, robh, mchehab,
bod, krzk+dt, abhinav.kumar, vikash.garodia, dikshita.agarwal,
Konrad Dybcio, bjorn.andersson, linux-media, linux-arm-msm,
devicetree, linux-kernel, iommu
On Thu, Oct 09, 2025 at 06:03:29PM +0100, Robin Murphy wrote:
> On 2025-10-09 2:19 pm, Dmitry Baryshkov wrote:
> > On Thu, Oct 09, 2025 at 11:46:55AM +0100, Robin Murphy wrote:
> > > On 2025-10-08 8:10 pm, Charan Teja Kalla wrote:
> > > >
> > > > On 9/29/2025 3:50 PM, Robin Murphy wrote:
> > > > > > 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 sub blocks are __configured__ to
> > > > > > generate different stream IDs.
> > > > >
> > > > > So please clarify why you can't:
> > > > >
> > > > > a) Describe the sub-blocks as individual child nodes each with their own
> > > > > distinct "iommus" property
> > > > >
> > > >
> > > > Thanks Robin for your time. Sorry for late reply as I really didn't have
> > > > concrete answer for this question.
> > > >
> > > > First let me clarify the word "sub blocks" -- This is just the logical
> > > > separation with no separate address space to really able to define them
> > > > as sub devices. Think of it like a single video IP with 2 dma
> > > > engines(used for pixel and non-pixel purpose).
> > > >
> > > > I should agree that the child-nodes in the device tree is the easy one
> > > > and infact, it is how being used in downstream.
> > > >
> > > > For upstream -- Since there is no real address space to interact with
> > > > these sub-blocks(or logical blocks), does it really qualify to define as
> > > > child nodes in the device tree? I see there is some push back[1].
> > >
> > > Who says you need an address space? Child nodes without "reg" properties,
> > > referenced by name, compatible or phandle, exist all over the place for all
> > > manner of reasons. If there are distinct logical functions with their own
> > > distinct hardware properties, then I would say having child nodes to
> > > describe and associate those properties with their respective functions is
> > > entirely natural and appropriate. The first example that comes to mind of
> > > where this is a well-established practice is PMICs - to pick one at random:
> > > Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml
> >
> > Logical function, that's correct. And also note, for PMICs that practice
> > has bitten us back. For PM8008 we switched back to a non-subdevice
> > representation.
> >
> > > For bonus irony, you can't take the other approaches without inherently
> > > *introducing* a notional address space in the form of your logical function
> > > IDs anyway.
> > >
> > > > > or:
> > > > >
> > > > > b) Use standard "iommu-map" which already supports mapping a masked
> > > > > input ID to an arbitrary IOMMU specifier
> > > > >
> > > >
> > > > I think clients is also required to program non-zero smr mask, where as
> > > > iommu-map just maps the id to an IOMMU specifier(sid). Please LMK if I
> > > > am unable to catch your thought here.
> > > An IOMMU specifier is whatever the target IOMMU node's #iommu-cells says it
> > > is. The fact that Linux's parsing code only works properly for #iommu-cells
> > > = 1 is not really a DT binding problem (other than it stemming from a loose
> > > assumption stated in the PCI binding's use of the property).
> >
> > I really don't like the idea of extending the #iommu-cells. The ARM SMMU
> > has only one cell, which is correct even for our platforms. The fact
> > that we need to identify different IOMMU SIDs (and handle them in a
> > differnt ways) is internal to the video device (and several other
> > devices). There is nothing to be handled on the ARM SMMU side.
>
> Huh? So if you prefer not to change anything, are you suggesting this series
> doesn't need to exist at all? Now I'm thoroughly confused...
Hmm. We need changes, but I don't feel like adding the FUNCTION_ID to
#iommu-cells is the best idea.
> If you want to use SMR masks, then you absolutely need #iommu-cells = 2,
> because that is the SMMU binding for using SMR masks. It would definitely
I'm sorry. Yes, we have #iommu-cells = <2>.
> not be OK to have some magic property trying to smuggle
> IOMMU-driver-specific data contrary to what the IOMMU node itself says. As
> for iommu-map, I don't see what would be objectionable about improving the
> parsing to respect a real #iommu-cells value rather than hard-coding an
> assumption. Yes, we'd probably need to forbid entries with length > 1
> targeting IOMMUs with #iommu-cells > 1, since the notion of a linear
This will break e.g. PCIe on Qualcomm platforms:
iommu-map = <0x0 &apps_smmu 0x1400 0x1>,
<0x100 &apps_smmu 0x1401 0x1>;
But this seems unlogical anyway wrt. apps_smmu having #iommu-cells =
<2>. It depends on ARM SMMU ignoring the second cell when it's not
present.
> relationship between the input ID and the output specifier falls apart when
> the specifier is complex, but that seems simple enough to implement and
> document (even if it's too fiddly to describe in the schema itself), and
> still certainly no worse than having another property that *is* just
> iommu-map with implicit length = 1.
>
> And if you want individual StreamIDs for logical functions to be attachable
> to distinct contexts then those functions absolutely must be visible to the
> IOMMU layer and the SMMU driver as independent devices with their own unique
> properties, which means either they come that way from the DT as of_platform
> devices in the first place, or you implement a full bus_type abstraction
Not necessarily. Tegra display driver creates a device for each context
on its own. In fact, using OF to create context devices is _less_
robust, because now the driver needs to sync, checking that there is a
subdevice, that it has probed, etc. Using manually created devices seems
better from my POV.
> which will have to be hooked up to the IOMMU layer. You cannot make IOMMU
> configuration "internal" to the actual client driver which is only allowed
> to bind *after* said IOMMU configuration has already been made.
I'm not sure I follow this, I'm sorry.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-09 18:25 ` Dmitry Baryshkov
@ 2025-10-10 19:53 ` Charan Teja Kalla
2025-10-10 22:30 ` Rob Herring
2025-10-12 20:44 ` Bryan O'Donoghue
2025-10-13 11:20 ` Robin Murphy
1 sibling, 2 replies; 36+ messages in thread
From: Charan Teja Kalla @ 2025-10-10 19:53 UTC (permalink / raw)
To: Dmitry Baryshkov, Robin Murphy
Cc: joro, will, saravanak, conor+dt, robh, mchehab, bod, krzk+dt,
abhinav.kumar, vikash.garodia, dikshita.agarwal, Konrad Dybcio,
bjorn.andersson, linux-media, linux-arm-msm, devicetree,
linux-kernel, iommu
On 10/9/2025 11:55 PM, Dmitry Baryshkov wrote:
>>> I really don't like the idea of extending the #iommu-cells. The ARM SMMU
>>> has only one cell, which is correct even for our platforms. The fact
>>> that we need to identify different IOMMU SIDs (and handle them in a
>>> differnt ways) is internal to the video device (and several other
>>> devices). There is nothing to be handled on the ARM SMMU side.
>> Huh? So if you prefer not to change anything, are you suggesting this series
>> doesn't need to exist at all? Now I'm thoroughly confused...
> Hmm. We need changes, but I don't feel like adding the FUNCTION_ID to
> #iommu-cells is the best idea.
>
>> If you want to use SMR masks, then you absolutely need #iommu-cells = 2,
>> because that is the SMMU binding for using SMR masks. It would definitely
> I'm sorry. Yes, we have #iommu-cells = <2>.
>
>> not be OK to have some magic property trying to smuggle
>> IOMMU-driver-specific data contrary to what the IOMMU node itself says. As
>> for iommu-map, I don't see what would be objectionable about improving the
>> parsing to respect a real #iommu-cells value rather than hard-coding an
>> assumption. Yes, we'd probably need to forbid entries with length > 1
>> targeting IOMMUs with #iommu-cells > 1, since the notion of a linear
> This will break e.g. PCIe on Qualcomm platforms:
>
> iommu-map = <0x0 &apps_smmu 0x1400 0x1>,
> <0x100 &apps_smmu 0x1401 0x1>;
>
>
> But this seems unlogical anyway wrt. apps_smmu having #iommu-cells =
> <2>. It depends on ARM SMMU ignoring the second cell when it's not
> present.
>
>> relationship between the input ID and the output specifier falls apart when
>> the specifier is complex, but that seems simple enough to implement and
>> document (even if it's too fiddly to describe in the schema itself), and
>> still certainly no worse than having another property that *is* just
>> iommu-map with implicit length = 1.
>>
>> And if you want individual StreamIDs for logical functions to be attachable
>> to distinct contexts then those functions absolutely must be visible to the
>> IOMMU layer and the SMMU driver as independent devices with their own unique
>> properties, which means either they come that way from the DT as of_platform
>> devices in the first place, or you implement a full bus_type abstraction
I don't want to dilute what Dmitry is saying here, but the below is what
i can make out of Robin comments, please CMIW:
iommu {
#iommu-cells = <2>;
}
video {
iommu = <iommu sid1 mask1>, <iommu sid2 mask2>;
#iommu-map-cells = 2; /* does it look weird to define here, even if
it is SMMU property? */
iommu-map = <0 smmu sid3 mask3>,
<0 smmu sid4 mask4>;
};
video driver calls of_dma_configure_id(, id) for each of the logical
functionality, id is the index(which can be treated as SMMU identifier
for logical function). Multiple smmu identifiers for a logical function
can be represented as bitmap indices in 'id'...
The sample code below, based on #iommu-map-cells defined in video{}
node, on top of this RFC patch.
-------------------------------8888-------------------------------------
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 2363de8f2fd6..ed5568278e2a 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -41,27 +41,24 @@ static int of_iommu_xlate(struct device *dev,
return ret;
}
-static int of_iommu_map_id(const __be32 *map, u32 id,
+static int of_iommu_map_id(const __be32 *map, u32 idx, u32 id, u32
cell_count,
struct device *dev, void *data)
{
struct device_node *phandle_node;
struct of_phandle_args *iommu_spec = data;
- u32 id_base = be32_to_cpup(map + 0);
u32 phandle = be32_to_cpup(map + 1);
- u32 master_id0 = be32_to_cpup(map + 2);
- u32 master_id1 = be32_to_cpup(map + 3);
- int err;
+ int err, i;
phandle_node = of_find_node_by_phandle(phandle);
if (!phandle_node)
return -ENODEV;
- if (id != id_base)
+ if (!test_bit(idx, (unsigned long*)&id))
return -EAGAIN;
iommu_spec->np = phandle_node;
- iommu_spec->args[0] = master_id0;
- iommu_spec->args[1] = master_id1;
+ for (i = 0; i <= cell_count; ++i)
+ iommu_spec->args[i] = be32_to_cpup(map + 2 + i);
err = of_iommu_xlate(dev, iommu_spec);
of_node_put(iommu_spec->np);
@@ -76,7 +73,7 @@ static int of_iommu_configure_map_id_and_mask(struct
device_node *master_np,
struct of_phandle_args iommu_spec = { .args_count = 2 };
return of_map_id_and_mask(master_np, *id,
- "iommu-map-masked", NULL,
+ "iommu-map", "iommu-map-mask",
&iommu_spec.np, NULL,
dev, (void *)&iommu_spec, of_iommu_map_id);
}
@@ -86,10 +83,14 @@ static int of_iommu_configure_dev_id(struct
device_node *master_np,
const u32 *id)
{
struct of_phandle_args iommu_spec = { .args_count = 1 };
+ u32 cell_count;
int err;
- bool iommu_map_masked = !!of_find_property(master_np,
"iommu-map-masked", NULL);
- if (iommu_map_masked)
+ err = of_property_read_u32(master_np, "iommu-map-cells", &cell_count);
+ if (err)
+ return err;
+
+ if (cell_count > 1)
return of_iommu_configure_map_id_and_mask(master_np, dev, id);
err = of_map_id(master_np, *id, "iommu-map",
diff --git a/drivers/of/base.c b/drivers/of/base.c
index bb11125e9624..c42d77beed52 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2065,12 +2065,15 @@ int of_map_id_and_mask(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 *dev, void *data,
- int (*fn)(const __be32 *map, u32 id, struct device *dev, void
*data))
+ int (*fn)(const __be32 *map, u32 idx, u32 id,
+ u32 cell_count, struct device *dev, void *data))
{
+ const char *cells_prop_name __free(kfree);
u32 map_mask, masked_id;
+ u32 cell_count;
int map_len;
- int ret;
const __be32 *map = NULL;
+ int ret, list_count, idx;
if (!np || !map_name || (!target && !id_out))
return -EINVAL;
@@ -2084,7 +2087,16 @@ int of_map_id_and_mask(const struct device_node
*np, u32 id,
return 0;
}
- if (!map_len || map_len % (4 * sizeof(*map))) {
+ cells_prop_name = kasprintf(GFP_KERNEL, "#%s-cells", map_name);
+ ret = of_property_read_u32(np, cells_prop_name, &cell_count);
+ if (ret == -EINVAL) // property doesn't defined
+ cell_count = 1;
+ else
+ return ret;
+
+ /* syntax: [base iommu cell0 <cell1 ...celln> len] */
+ list_count = 2 + cell_count + 1;
+ if (!map_len || map_len % (list_count * sizeof(*map))) {
pr_err("%pOF: Error: Bad %s length: %d\n", np,
map_name, map_len);
return -EINVAL;
@@ -2101,12 +2113,12 @@ int of_map_id_and_mask(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 (idx = 0; map_len > 0; map_len -= list_count * sizeof(*map), map
+= list_count, idx++) {
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 + cell_count - 1);
if (id_base & ~map_mask) {
pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores
id-base (0x%x)\n",
@@ -2115,8 +2127,14 @@ int of_map_id_and_mask(const struct device_node
*np, u32 id,
return -EFAULT;
}
- if (fn) {
- ret = fn(map, id, dev, data);
+ /* for >1 cell count, avoid the linear translation. It is
+ * expected that custom mapping managed in cb fn().
+ */
+ if (cell_count > 2) {
+ if (!fn)
+ return -EINVAL;
+ /* list of required indices is mentioned in id */
+ ret = fn(map, idx, id, cell_count, dev, data);
if (ret != -EAGAIN)
break;
continue;
diff --git a/include/linux/of.h b/include/linux/of.h
index 7f3890ab26d5..b7f38a43ffc7 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -464,7 +464,8 @@ int of_map_id_and_mask(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 *dev, void *data,
- int (*fn)(const __be32 *map, u32 id, struct device *dev, void *data));
+ int (*fn)(const __be32 *map, u32 id, u32 idx, u32 cell_count,
+ struct device *dev, void *data));
phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);
------------------------------------------------------------------------
Thanks,
Charan
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-10 19:53 ` Charan Teja Kalla
@ 2025-10-10 22:30 ` Rob Herring
2025-10-11 0:54 ` Dmitry Baryshkov
2025-10-12 20:44 ` Bryan O'Donoghue
1 sibling, 1 reply; 36+ messages in thread
From: Rob Herring @ 2025-10-10 22:30 UTC (permalink / raw)
To: Charan Teja Kalla
Cc: Dmitry Baryshkov, Robin Murphy, joro, will, saravanak, conor+dt,
mchehab, bod, krzk+dt, abhinav.kumar, vikash.garodia,
dikshita.agarwal, Konrad Dybcio, bjorn.andersson, linux-media,
linux-arm-msm, devicetree, linux-kernel, iommu
On Fri, Oct 10, 2025 at 2:53 PM Charan Teja Kalla
<charan.kalla@oss.qualcomm.com> wrote:
>
>
>
> On 10/9/2025 11:55 PM, Dmitry Baryshkov wrote:
> >>> I really don't like the idea of extending the #iommu-cells. The ARM SMMU
> >>> has only one cell, which is correct even for our platforms. The fact
> >>> that we need to identify different IOMMU SIDs (and handle them in a
> >>> differnt ways) is internal to the video device (and several other
> >>> devices). There is nothing to be handled on the ARM SMMU side.
> >> Huh? So if you prefer not to change anything, are you suggesting this series
> >> doesn't need to exist at all? Now I'm thoroughly confused...
> > Hmm. We need changes, but I don't feel like adding the FUNCTION_ID to
> > #iommu-cells is the best idea.
> >
> >> If you want to use SMR masks, then you absolutely need #iommu-cells = 2,
> >> because that is the SMMU binding for using SMR masks. It would definitely
> > I'm sorry. Yes, we have #iommu-cells = <2>.
> >
> >> not be OK to have some magic property trying to smuggle
> >> IOMMU-driver-specific data contrary to what the IOMMU node itself says. As
> >> for iommu-map, I don't see what would be objectionable about improving the
> >> parsing to respect a real #iommu-cells value rather than hard-coding an
> >> assumption. Yes, we'd probably need to forbid entries with length > 1
> >> targeting IOMMUs with #iommu-cells > 1, since the notion of a linear
> > This will break e.g. PCIe on Qualcomm platforms:
> >
> > iommu-map = <0x0 &apps_smmu 0x1400 0x1>,
> > <0x100 &apps_smmu 0x1401 0x1>;
> >
> >
> > But this seems unlogical anyway wrt. apps_smmu having #iommu-cells =
> > <2>. It depends on ARM SMMU ignoring the second cell when it's not
> > present.
> >
> >> relationship between the input ID and the output specifier falls apart when
> >> the specifier is complex, but that seems simple enough to implement and
> >> document (even if it's too fiddly to describe in the schema itself), and
> >> still certainly no worse than having another property that *is* just
> >> iommu-map with implicit length = 1.
> >>
> >> And if you want individual StreamIDs for logical functions to be attachable
> >> to distinct contexts then those functions absolutely must be visible to the
> >> IOMMU layer and the SMMU driver as independent devices with their own unique
> >> properties, which means either they come that way from the DT as of_platform
> >> devices in the first place, or you implement a full bus_type abstraction
>
> I don't want to dilute what Dmitry is saying here, but the below is what
> i can make out of Robin comments, please CMIW:
>
> iommu {
> #iommu-cells = <2>;
> }
>
> video {
> iommu = <iommu sid1 mask1>, <iommu sid2 mask2>;
> #iommu-map-cells = 2; /* does it look weird to define here, even if
> it is SMMU property? */
No, not weird. interrupt-map similarly requires #interrupt-cells. So
it would be just #iommu-cells here.
> iommu-map = <0 smmu sid3 mask3>,
> <0 smmu sid4 mask4>;
But you only have 1 cell, not 2 here. The #iommu-cells in this node
would define the number of cells before 'smmu'. The #iommu-cells in
the &smmu node is the number of cells after the &smmu phandle.
Rob
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-10 22:30 ` Rob Herring
@ 2025-10-11 0:54 ` Dmitry Baryshkov
0 siblings, 0 replies; 36+ messages in thread
From: Dmitry Baryshkov @ 2025-10-11 0:54 UTC (permalink / raw)
To: Rob Herring
Cc: Charan Teja Kalla, Robin Murphy, joro, will, saravanak, conor+dt,
mchehab, bod, krzk+dt, abhinav.kumar, vikash.garodia,
dikshita.agarwal, Konrad Dybcio, bjorn.andersson, linux-media,
linux-arm-msm, devicetree, linux-kernel, iommu
On Fri, Oct 10, 2025 at 05:30:11PM -0500, Rob Herring wrote:
> On Fri, Oct 10, 2025 at 2:53 PM Charan Teja Kalla
> <charan.kalla@oss.qualcomm.com> wrote:
> > On 10/9/2025 11:55 PM, Dmitry Baryshkov wrote:
> > >>> I really don't like the idea of extending the #iommu-cells. The ARM SMMU
> > >>> has only one cell, which is correct even for our platforms. The fact
> > >>> that we need to identify different IOMMU SIDs (and handle them in a
> > >>> differnt ways) is internal to the video device (and several other
> > >>> devices). There is nothing to be handled on the ARM SMMU side.
> > >> Huh? So if you prefer not to change anything, are you suggesting this series
> > >> doesn't need to exist at all? Now I'm thoroughly confused...
> > > Hmm. We need changes, but I don't feel like adding the FUNCTION_ID to
> > > #iommu-cells is the best idea.
> > >
> > >> If you want to use SMR masks, then you absolutely need #iommu-cells = 2,
> > >> because that is the SMMU binding for using SMR masks. It would definitely
> > > I'm sorry. Yes, we have #iommu-cells = <2>.
> > >
> > >> not be OK to have some magic property trying to smuggle
> > >> IOMMU-driver-specific data contrary to what the IOMMU node itself says. As
> > >> for iommu-map, I don't see what would be objectionable about improving the
> > >> parsing to respect a real #iommu-cells value rather than hard-coding an
> > >> assumption. Yes, we'd probably need to forbid entries with length > 1
> > >> targeting IOMMUs with #iommu-cells > 1, since the notion of a linear
> > > This will break e.g. PCIe on Qualcomm platforms:
> > >
> > > iommu-map = <0x0 &apps_smmu 0x1400 0x1>,
> > > <0x100 &apps_smmu 0x1401 0x1>;
> > >
> > >
> > > But this seems unlogical anyway wrt. apps_smmu having #iommu-cells =
> > > <2>. It depends on ARM SMMU ignoring the second cell when it's not
> > > present.
> > >
> > >> relationship between the input ID and the output specifier falls apart when
> > >> the specifier is complex, but that seems simple enough to implement and
> > >> document (even if it's too fiddly to describe in the schema itself), and
> > >> still certainly no worse than having another property that *is* just
> > >> iommu-map with implicit length = 1.
> > >>
> > >> And if you want individual StreamIDs for logical functions to be attachable
> > >> to distinct contexts then those functions absolutely must be visible to the
> > >> IOMMU layer and the SMMU driver as independent devices with their own unique
> > >> properties, which means either they come that way from the DT as of_platform
> > >> devices in the first place, or you implement a full bus_type abstraction
> >
> > I don't want to dilute what Dmitry is saying here, but the below is what
> > i can make out of Robin comments, please CMIW:
> >
> > iommu {
> > #iommu-cells = <2>;
> > }
> >
> > video {
> > iommu = <iommu sid1 mask1>, <iommu sid2 mask2>;
> > #iommu-map-cells = 2; /* does it look weird to define here, even if
> > it is SMMU property? */
>
> No, not weird. interrupt-map similarly requires #interrupt-cells. So
> it would be just #iommu-cells here.
The major problem is that our DTs already use the currently-defined
single-cell iommu-maps. I'm not sure if it is possible to support
old and new semantics. So #iommu-map-cells (but placed into the IOMMU
device) might be a good way: by default iommu-map parsing code will try
parsing just a SID, but with #iommu-map-cells it will use specified
number of arguments. The only question is what if #iommu-map-cells !=
#iommu-cells.
>
> > iommu-map = <0 smmu sid3 mask3>,
> > <0 smmu sid4 mask4>;
>
> But you only have 1 cell, not 2 here. The #iommu-cells in this node
> would define the number of cells before 'smmu'. The #iommu-cells in
> the &smmu node is the number of cells after the &smmu phandle.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-10 19:53 ` Charan Teja Kalla
2025-10-10 22:30 ` Rob Herring
@ 2025-10-12 20:44 ` Bryan O'Donoghue
2025-10-12 21:57 ` Bryan O'Donoghue
2025-10-12 22:47 ` Dmitry Baryshkov
1 sibling, 2 replies; 36+ messages in thread
From: Bryan O'Donoghue @ 2025-10-12 20:44 UTC (permalink / raw)
To: Charan Teja Kalla, Dmitry Baryshkov, Robin Murphy
Cc: joro, will, saravanak, conor+dt, robh, mchehab, krzk+dt,
abhinav.kumar, vikash.garodia, dikshita.agarwal, Konrad Dybcio,
bjorn.andersson, linux-media, linux-arm-msm, devicetree,
linux-kernel, iommu
On 10/10/2025 20:53, Charan Teja Kalla wrote:
> I don't want to dilute what Dmitry is saying here, but the below is what
> i can make out of Robin comments, please CMIW:
>
> iommu {
> #iommu-cells = <2>;
> }
>
> video {
> iommu = <iommu sid1 mask1>, <iommu sid2 mask2>;
> #iommu-map-cells = 2; /* does it look weird to define here, even if
> it is SMMU property? */
> iommu-map = <0 smmu sid3 mask3>,
> <0 smmu sid4 mask4>;
> };
This whole iommu-map thing is a wrong direction, its a workaround.
It stems from here:
1. Vikash posted a series adding a platform device
https://lore.kernel.org/linux-media/20250627-video_cb-v3-0-51e18c0ffbce@quicinc.com/
The two objectives of this are
a. Allow Linux, the APPS as qcom calls it,@ EL1 or EL2
to setup iommu entries for function_ids that are
not the APPS @ EL1/EL2.
For example the APPS running in TEE or one of the
various co-processors - like say the Compute DSP cDSP.
b. Allowing for each device to have a full IOVA range.
2. Krzysztof queried about changing _existing_ entries e.g.
https://lore.kernel.org/linux-media/6fd3fa34-69e1-484f-ad6f-8caa852f1a6c@kernel.org/
The point about ABI breakage.
3. This proposal to introduce iommu-map as a workaround
Gets the FUNCTION_ID APPS v cDSP v TZ into the DT
So it solves 1/a I'm not sure it solves 1/b
However if you were designing from scratch you wouldn't
have a motivation to assign this additional property.
The motivation is to not break the ABI I think.
4. Robin said
"And if you want individual StreamIDs for logical functions to be
attachable to distinct contexts then those functions absolutely
must be visible to the IOMMU layer and the SMMU driver as
independent devices"
5. If you think about this, its actually the right long term solution
- Individual devices means something like:
video-codec@aa00000 {
/* Any SID mapping to S1_VIDEO_HLOS belongs here */
compatible = "qcom,sm8550-iris";
iommus = <&apps_smmu 0x1947 0x0000>;
};
video-codec-non-pixel {
/* Any SID mapping to S1_VIDEO_HLOS_P belongs here */
compatible = "qcom,sm8550-iris-non-pixel";
iommus = <&apps_smmu 0x1940 0x0000>;
};
- Or do something like that above again in platform code.
6. We should on introduction of a new SoC
- Fix the iommus = <> for "qcom,newsoc-iris" to contain
only what is pertinent to S1_VIDEO_HLOS
- Make new devices in the DT for each FUNCTION_ID
- Then look at how - if - that fix can be brought back to Lemans
My problem with introducing the iommu-map is that it bakes into the
video codec definitions a fixup which then gets carried forward.
But the right thing to do is individual devices so, let's do that and
worry about how to back-port that fix to older SoCs once done.
---
bod
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-12 20:44 ` Bryan O'Donoghue
@ 2025-10-12 21:57 ` Bryan O'Donoghue
2025-10-12 22:47 ` Dmitry Baryshkov
1 sibling, 0 replies; 36+ messages in thread
From: Bryan O'Donoghue @ 2025-10-12 21:57 UTC (permalink / raw)
To: Charan Teja Kalla, Dmitry Baryshkov, Robin Murphy
Cc: joro, will, saravanak, conor+dt, robh, mchehab, krzk+dt,
abhinav.kumar, vikash.garodia, dikshita.agarwal, Konrad Dybcio,
bjorn.andersson, linux-media, linux-arm-msm, devicetree,
linux-kernel, iommu
> 6. We should on introduction of a new SoC
>
> - Fix the iommus = <> for "qcom,newsoc-iris" to contain
> only what is pertinent to S1_VIDEO_HLOS
>
> - Make new devices in the DT for each FUNCTION_ID
>
> - Then look at how - if - that fix can be brought back to Lemans
[sic] Lanai/sm8550
---
bod
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-12 20:44 ` Bryan O'Donoghue
2025-10-12 21:57 ` Bryan O'Donoghue
@ 2025-10-12 22:47 ` Dmitry Baryshkov
2025-10-13 10:18 ` Bryan O'Donoghue
1 sibling, 1 reply; 36+ messages in thread
From: Dmitry Baryshkov @ 2025-10-12 22:47 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: Charan Teja Kalla, Robin Murphy, joro, will, saravanak, conor+dt,
robh, mchehab, krzk+dt, abhinav.kumar, vikash.garodia,
dikshita.agarwal, Konrad Dybcio, bjorn.andersson, linux-media,
linux-arm-msm, devicetree, linux-kernel, iommu
On Sun, Oct 12, 2025 at 09:44:43PM +0100, Bryan O'Donoghue wrote:
> On 10/10/2025 20:53, Charan Teja Kalla wrote:
> > I don't want to dilute what Dmitry is saying here, but the below is what
> > i can make out of Robin comments, please CMIW:
> >
> > iommu {
> > #iommu-cells = <2>;
> > }
> >
> > video {
> > iommu = <iommu sid1 mask1>, <iommu sid2 mask2>;
> > #iommu-map-cells = 2; /* does it look weird to define here, even if
> > it is SMMU property? */
> > iommu-map = <0 smmu sid3 mask3>,
> > <0 smmu sid4 mask4>;
> > };
>
>
> This whole iommu-map thing is a wrong direction, its a workaround.
>
> It stems from here:
>
> 1. Vikash posted a series adding a platform device
> https://lore.kernel.org/linux-media/20250627-video_cb-v3-0-51e18c0ffbce@quicinc.com/
>
> The two objectives of this are
>
> a. Allow Linux, the APPS as qcom calls it,@ EL1 or EL2
> to setup iommu entries for function_ids that are
> not the APPS @ EL1/EL2.
No.
Up to now we were talking only about the non-pixel bitstreams and secure
en-/decoding data. None of that is related to anything except Linux
running in EL1/EL2. Only Linux consumes / provides normal non-pixel
data. Only Linux handles decoded secure buffers. Only Linux sets up the
video decoding of secure data and then blending of that data inside DPU.
> For example the APPS running in TEE or one of the
> various co-processors - like say the Compute DSP cDSP.
How did CDSP or TEE get into the picture?
>
> b. Allowing for each device to have a full IOVA range.
>
> 2. Krzysztof queried about changing _existing_ entries e.g.
> https://lore.kernel.org/linux-media/6fd3fa34-69e1-484f-ad6f-8caa852f1a6c@kernel.org/
>
> The point about ABI breakage.
>
> 3. This proposal to introduce iommu-map as a workaround
> Gets the FUNCTION_ID APPS v cDSP v TZ into the DT
It's neither CDSP nor TZ. The source or the consumer of the data might
be crypto core or just Linux process. For non-secured non-pixel data it
_is_ Linux process.
>
> So it solves 1/a I'm not sure it solves 1/b
>
> However if you were designing from scratch you wouldn't
> have a motivation to assign this additional property.
>
> The motivation is to not break the ABI I think.
>
> 4. Robin said
>
> "And if you want individual StreamIDs for logical functions to be
> attachable to distinct contexts then those functions absolutely
> must be visible to the IOMMU layer and the SMMU driver as
> independent devices"
Correct. But it doesn't require separate OF device nodes. See
host1x_memory_context_list_init().
>
> 5. If you think about this, its actually the right long term solution
>
> - Individual devices means something like:
>
> video-codec@aa00000 {
> /* Any SID mapping to S1_VIDEO_HLOS belongs here */
> compatible = "qcom,sm8550-iris";
> iommus = <&apps_smmu 0x1947 0x0000>;
> };
>
> video-codec-non-pixel {
> /* Any SID mapping to S1_VIDEO_HLOS_P belongs here */
> compatible = "qcom,sm8550-iris-non-pixel";
> iommus = <&apps_smmu 0x1940 0x0000>;
> };
Which piece of hardware is described by this node? Why is it separate
from the main video-codec? The IOMMU stream doesn't have any specifics,
it's just a part of the video codec core.
>
> - Or do something like that above again in platform code.
>
> 6. We should on introduction of a new SoC
>
> - Fix the iommus = <> for "qcom,newsoc-iris" to contain
> only what is pertinent to S1_VIDEO_HLOS
>
> - Make new devices in the DT for each FUNCTION_ID
>
> - Then look at how - if - that fix can be brought back to Lemans
>
> My problem with introducing the iommu-map is that it bakes into the video
> codec definitions a fixup which then gets carried forward.
>
> But the right thing to do is individual devices so, let's do that and worry
> about how to back-port that fix to older SoCs once done.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-12 22:47 ` Dmitry Baryshkov
@ 2025-10-13 10:18 ` Bryan O'Donoghue
0 siblings, 0 replies; 36+ messages in thread
From: Bryan O'Donoghue @ 2025-10-13 10:18 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Charan Teja Kalla, Robin Murphy, joro, will, saravanak, conor+dt,
robh, mchehab, krzk+dt, abhinav.kumar, vikash.garodia,
dikshita.agarwal, Konrad Dybcio, bjorn.andersson, linux-media,
linux-arm-msm, devicetree, linux-kernel, iommu
On 12/10/2025 23:47, Dmitry Baryshkov wrote:
> On Sun, Oct 12, 2025 at 09:44:43PM +0100, Bryan O'Donoghue wrote:
>> On 10/10/2025 20:53, Charan Teja Kalla wrote:
>>> I don't want to dilute what Dmitry is saying here, but the below is what
>>> i can make out of Robin comments, please CMIW:
>>>
>>> iommu {
>>> #iommu-cells = <2>;
>>> }
>>>
>>> video {
>>> iommu = <iommu sid1 mask1>, <iommu sid2 mask2>;
>>> #iommu-map-cells = 2; /* does it look weird to define here, even if
>>> it is SMMU property? */
>>> iommu-map = <0 smmu sid3 mask3>,
>>> <0 smmu sid4 mask4>;
>>> };
>>
>>
>> This whole iommu-map thing is a wrong direction, its a workaround.
>>
>> It stems from here:
>>
>> 1. Vikash posted a series adding a platform device
>> https://lore.kernel.org/linux-media/20250627-video_cb-v3-0-51e18c0ffbce@quicinc.com/
>>
>> The two objectives of this are
>>
>> a. Allow Linux, the APPS as qcom calls it,@ EL1 or EL2
>> to setup iommu entries for function_ids that are
>> not the APPS @ EL1/EL2.
>
> No.
>
> Up to now we were talking only about the non-pixel bitstreams and secure
> en-/decoding data. None of that is related to anything except Linux
> running in EL1/EL2. Only Linux consumes / provides normal non-pixel
> data. Only Linux handles decoded secure buffers. Only Linux sets up the
> video decoding of secure data and then blending of that data inside DPU.
As I understand some of these >> For example the APPS running in
TEE or one of the
>> various co-processors - like say the Compute DSP cDSP.
>
> How did CDSP or TEE get into the picture?
Hypothetical examples of the non-HLOS VMID. Call these AC_VM_CP_BITSREAM
or AC_VM_CP_NON_PIXEL to use values from the documentation.
>>
>> b. Allowing for each device to have a full IOVA range.
>>
>> 2. Krzysztof queried about changing _existing_ entries e.g.
>> https://lore.kernel.org/linux-media/6fd3fa34-69e1-484f-ad6f-8caa852f1a6c@kernel.org/
>>
>> The point about ABI breakage.
>>
>> 3. This proposal to introduce iommu-map as a workaround
>> Gets the FUNCTION_ID APPS v cDSP v TZ into the DT
>
> It's neither CDSP nor TZ. The source or the consumer of the data might
> be crypto core or just Linux process. For non-secured non-pixel data it
> _is_ Linux process.
>
>>
>> So it solves 1/a I'm not sure it solves 1/b
>>
>> However if you were designing from scratch you wouldn't
>> have a motivation to assign this additional property.
>>
>> The motivation is to not break the ABI I think.
>>
>> 4. Robin said
>>
>> "And if you want individual StreamIDs for logical functions to be
>> attachable to distinct contexts then those functions absolutely
>> must be visible to the IOMMU layer and the SMMU driver as
>> independent devices"
>
> Correct. But it doesn't require separate OF device nodes. See
> host1x_memory_context_list_init().
Fine could be platform code too.
>>
>> 5. If you think about this, its actually the right long term solution
>>
>> - Individual devices means something like:
>>
>> video-codec@aa00000 {
>> /* Any SID mapping to S1_VIDEO_HLOS belongs here */
>> compatible = "qcom,sm8550-iris";
>> iommus = <&apps_smmu 0x1947 0x0000>;
>> };
>>
>> video-codec-non-pixel {
>> /* Any SID mapping to S1_VIDEO_HLOS_P belongs here */
>> compatible = "qcom,sm8550-iris-non-pixel";
>> iommus = <&apps_smmu 0x1940 0x0000>;
>> };
>
> Which piece of hardware is described by this node? Why is it separate
> from the main video-codec? The IOMMU stream doesn't have any specifics,
> it's just a part of the video codec core.
You could conceivably start associating /dev/video entries with a device
that maps to AC_VM_CP_PIXEL - the protected video stream.
There may be data other than SID/FUNCTION_ID that we would want to
associate with those devices, I'll stipulate to further discussion there.
>>
>> - Or do something like that above again in platform code.
>>
>> 6. We should on introduction of a new SoC
>>
>> - Fix the iommus = <> for "qcom,newsoc-iris" to contain
>> only what is pertinent to S1_VIDEO_HLOS
>>
>> - Make new devices in the DT for each FUNCTION_ID
>>
>> - Then look at how - if - that fix can be brought back to Lemans
>>
>> My problem with introducing the iommu-map is that it bakes into the video
>> codec definitions a fixup which then gets carried forward.
>>
>> But the right thing to do is individual devices so, let's do that and worry
>> about how to back-port that fix to older SoCs once done.
So really whether we end up representing these devices in DT or platform
code, separate devices are the answer - both for the FUNCTION_ID mapping
and the IOVA range.
You just need to carefully think about what ends up being a device if
the IOVA range is a concern.
Its unfortunate that sm8550 has an addtional iommu entry that wants to
live in a different device - but, that's a problem for sm8550.
Perhaps something we can backport to Lanai, Lemans and friends once we
get the new submissions right..
---
bod
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-09 18:25 ` Dmitry Baryshkov
2025-10-10 19:53 ` Charan Teja Kalla
@ 2025-10-13 11:20 ` Robin Murphy
2025-10-13 12:31 ` Dmitry Baryshkov
1 sibling, 1 reply; 36+ messages in thread
From: Robin Murphy @ 2025-10-13 11:20 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Charan Teja Kalla, joro, will, saravanak, conor+dt, robh, mchehab,
bod, krzk+dt, abhinav.kumar, vikash.garodia, dikshita.agarwal,
Konrad Dybcio, bjorn.andersson, linux-media, linux-arm-msm,
devicetree, linux-kernel, iommu
On 2025-10-09 7:25 pm, Dmitry Baryshkov wrote:
> On Thu, Oct 09, 2025 at 06:03:29PM +0100, Robin Murphy wrote:
>> On 2025-10-09 2:19 pm, Dmitry Baryshkov wrote:
>>> On Thu, Oct 09, 2025 at 11:46:55AM +0100, Robin Murphy wrote:
>>>> On 2025-10-08 8:10 pm, Charan Teja Kalla wrote:
>>>>>
>>>>> On 9/29/2025 3:50 PM, Robin Murphy wrote:
>>>>>>> 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 sub blocks are __configured__ to
>>>>>>> generate different stream IDs.
>>>>>>
>>>>>> So please clarify why you can't:
>>>>>>
>>>>>> a) Describe the sub-blocks as individual child nodes each with their own
>>>>>> distinct "iommus" property
>>>>>>
>>>>>
>>>>> Thanks Robin for your time. Sorry for late reply as I really didn't have
>>>>> concrete answer for this question.
>>>>>
>>>>> First let me clarify the word "sub blocks" -- This is just the logical
>>>>> separation with no separate address space to really able to define them
>>>>> as sub devices. Think of it like a single video IP with 2 dma
>>>>> engines(used for pixel and non-pixel purpose).
>>>>>
>>>>> I should agree that the child-nodes in the device tree is the easy one
>>>>> and infact, it is how being used in downstream.
>>>>>
>>>>> For upstream -- Since there is no real address space to interact with
>>>>> these sub-blocks(or logical blocks), does it really qualify to define as
>>>>> child nodes in the device tree? I see there is some push back[1].
>>>>
>>>> Who says you need an address space? Child nodes without "reg" properties,
>>>> referenced by name, compatible or phandle, exist all over the place for all
>>>> manner of reasons. If there are distinct logical functions with their own
>>>> distinct hardware properties, then I would say having child nodes to
>>>> describe and associate those properties with their respective functions is
>>>> entirely natural and appropriate. The first example that comes to mind of
>>>> where this is a well-established practice is PMICs - to pick one at random:
>>>> Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml
>>>
>>> Logical function, that's correct. And also note, for PMICs that practice
>>> has bitten us back. For PM8008 we switched back to a non-subdevice
>>> representation.
>>>
>>>> For bonus irony, you can't take the other approaches without inherently
>>>> *introducing* a notional address space in the form of your logical function
>>>> IDs anyway.
>>>>
>>>>> > or:
>>>>>>
>>>>>> b) Use standard "iommu-map" which already supports mapping a masked
>>>>>> input ID to an arbitrary IOMMU specifier
>>>>>>
>>>>>
>>>>> I think clients is also required to program non-zero smr mask, where as
>>>>> iommu-map just maps the id to an IOMMU specifier(sid). Please LMK if I
>>>>> am unable to catch your thought here.
>>>> An IOMMU specifier is whatever the target IOMMU node's #iommu-cells says it
>>>> is. The fact that Linux's parsing code only works properly for #iommu-cells
>>>> = 1 is not really a DT binding problem (other than it stemming from a loose
>>>> assumption stated in the PCI binding's use of the property).
>>>
>>> I really don't like the idea of extending the #iommu-cells. The ARM SMMU
>>> has only one cell, which is correct even for our platforms. The fact
>>> that we need to identify different IOMMU SIDs (and handle them in a
>>> differnt ways) is internal to the video device (and several other
>>> devices). There is nothing to be handled on the ARM SMMU side.
>>
>> Huh? So if you prefer not to change anything, are you suggesting this series
>> doesn't need to exist at all? Now I'm thoroughly confused...
>
> Hmm. We need changes, but I don't feel like adding the FUNCTION_ID to
> #iommu-cells is the best idea.
What? No, any function ID would be an *input* to a map, not part of the
output specifier; indeed it should never go anywhere near the IOMMU, I
don't think anyone suggested that.
>> If you want to use SMR masks, then you absolutely need #iommu-cells = 2,
>> because that is the SMMU binding for using SMR masks. It would definitely
>
> I'm sorry. Yes, we have #iommu-cells = <2>.
>
>> not be OK to have some magic property trying to smuggle
>> IOMMU-driver-specific data contrary to what the IOMMU node itself says. As
>> for iommu-map, I don't see what would be objectionable about improving the
>> parsing to respect a real #iommu-cells value rather than hard-coding an
>> assumption. Yes, we'd probably need to forbid entries with length > 1
>> targeting IOMMUs with #iommu-cells > 1, since the notion of a linear
>
> This will break e.g. PCIe on Qualcomm platforms:
>
> iommu-map = <0x0 &apps_smmu 0x1400 0x1>,
> <0x100 &apps_smmu 0x1401 0x1>;
>
>
> But this seems unlogical anyway wrt. apps_smmu having #iommu-cells =
> <2>. It depends on ARM SMMU ignoring the second cell when it's not
> present.
Urgh, yes, that's just broken already :(
At least they all seem to be a sufficiently consistent pattern that a
targeted workaround to detect old DTBs looks feasible (I'm thinking, if
iommu-map size % 4 == 0 and cells n*4 + 3 are all 1 and cells n*4 + 1
are all the same phandle to an IOMMU with #iommu-cells == 2, then parse
as if #iommu-cells == 1)
>> relationship between the input ID and the output specifier falls apart when
>> the specifier is complex, but that seems simple enough to implement and
>> document (even if it's too fiddly to describe in the schema itself), and
>> still certainly no worse than having another property that *is* just
>> iommu-map with implicit length = 1.
>>
>> And if you want individual StreamIDs for logical functions to be attachable
>> to distinct contexts then those functions absolutely must be visible to the
>> IOMMU layer and the SMMU driver as independent devices with their own unique
>> properties, which means either they come that way from the DT as of_platform
>> devices in the first place, or you implement a full bus_type abstraction
>
> Not necessarily. Tegra display driver creates a device for each context
> on its own.
No, the *display* driver does not; the host1x bus driver does, which is
the point I was making - that has a proper bus abstraction tied into the
IOMMU layer, such that the devices are correctly configured long before
the actual DRM driver(s) get anywhere near them.
> In fact, using OF to create context devices is _less_
> robust, because now the driver needs to sync, checking that there is a
> subdevice, that it has probed, etc. Using manually created devices seems
> better from my POV.
Huh? A simple call to of_platform_populate() is somehow less robust than
open-coding much of the same logic that of_platform_populate() does plus
a bunch of hackery to try to fake up an of_node to make the new device
appear to own the appropriate properties?
Having entire sub-*drivers* for child devices or not is an orthogonal
issue regardless of whichever way they are created.
>> which will have to be hooked up to the IOMMU layer. You cannot make IOMMU
>> configuration "internal" to the actual client driver which is only allowed
>> to bind *after* said IOMMU configuration has already been made.
>
> I'm not sure I follow this, I'm sorry.
I mean IOMMU configuration is designed to happen at device_add() time,
and client drivers must not assume otherwise (the mechanisms for
handling IOMMU drivers registering "late" from modules are internal
details that can and will change). If you're under the impression that a
straightforward platform driver for the video codec itself would be able
to invoke IOMMU configuration for the video codec platform device
(without unacceptable levels of hackery) then you are mistaken, sorry.
Again, to be able to assign StreamIDs to different contexts, those
StreamIDs must uniquely belong to different struct devices. Thus in
terms of how you get to those struct devices from a DT representation,
either they come from distinct DT nodes with standard "iommus"
properties that the generic of_platform code can create and configure
accordingly, or you're doing a non-trivial amount of work to implement
your own bus layer like host1x_context_bus to manage your own type of
sub-device. There is no valid middle ground of trying to stuff
driver-specific knowledge of arbitrarily made-up function IDs into the
generic platform bus code.
Thanks,
Robin.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-13 11:20 ` Robin Murphy
@ 2025-10-13 12:31 ` Dmitry Baryshkov
2025-10-14 14:07 ` Robin Murphy
2025-10-14 15:07 ` Bryan O'Donoghue
0 siblings, 2 replies; 36+ messages in thread
From: Dmitry Baryshkov @ 2025-10-13 12:31 UTC (permalink / raw)
To: Robin Murphy
Cc: Charan Teja Kalla, joro, will, saravanak, conor+dt, robh, mchehab,
bod, krzk+dt, abhinav.kumar, vikash.garodia, dikshita.agarwal,
Konrad Dybcio, bjorn.andersson, linux-media, linux-arm-msm,
devicetree, linux-kernel, iommu
On Mon, Oct 13, 2025 at 12:20:54PM +0100, Robin Murphy wrote:
> On 2025-10-09 7:25 pm, Dmitry Baryshkov wrote:
> > On Thu, Oct 09, 2025 at 06:03:29PM +0100, Robin Murphy wrote:
> > > On 2025-10-09 2:19 pm, Dmitry Baryshkov wrote:
> > > > On Thu, Oct 09, 2025 at 11:46:55AM +0100, Robin Murphy wrote:
> > > > > On 2025-10-08 8:10 pm, Charan Teja Kalla wrote:
> > > > > >
> > > > > > On 9/29/2025 3:50 PM, Robin Murphy wrote:
> > > > > > > > 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 sub blocks are __configured__ to
> > > > > > > > generate different stream IDs.
> > > > > > >
> > > > > > > So please clarify why you can't:
> > > > > > >
> > > > > > > a) Describe the sub-blocks as individual child nodes each with their own
> > > > > > > distinct "iommus" property
> > > > > > >
> > > > > >
> > > > > > Thanks Robin for your time. Sorry for late reply as I really didn't have
> > > > > > concrete answer for this question.
> > > > > >
> > > > > > First let me clarify the word "sub blocks" -- This is just the logical
> > > > > > separation with no separate address space to really able to define them
> > > > > > as sub devices. Think of it like a single video IP with 2 dma
> > > > > > engines(used for pixel and non-pixel purpose).
> > > > > >
> > > > > > I should agree that the child-nodes in the device tree is the easy one
> > > > > > and infact, it is how being used in downstream.
> > > > > >
> > > > > > For upstream -- Since there is no real address space to interact with
> > > > > > these sub-blocks(or logical blocks), does it really qualify to define as
> > > > > > child nodes in the device tree? I see there is some push back[1].
> > > > >
> > > > > Who says you need an address space? Child nodes without "reg" properties,
> > > > > referenced by name, compatible or phandle, exist all over the place for all
> > > > > manner of reasons. If there are distinct logical functions with their own
> > > > > distinct hardware properties, then I would say having child nodes to
> > > > > describe and associate those properties with their respective functions is
> > > > > entirely natural and appropriate. The first example that comes to mind of
> > > > > where this is a well-established practice is PMICs - to pick one at random:
> > > > > Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml
> > > >
> > > > Logical function, that's correct. And also note, for PMICs that practice
> > > > has bitten us back. For PM8008 we switched back to a non-subdevice
> > > > representation.
> > > >
> > > > > For bonus irony, you can't take the other approaches without inherently
> > > > > *introducing* a notional address space in the form of your logical function
> > > > > IDs anyway.
> > > > >
> > > > > > > or:
> > > > > > >
> > > > > > > b) Use standard "iommu-map" which already supports mapping a masked
> > > > > > > input ID to an arbitrary IOMMU specifier
> > > > > > >
> > > > > >
> > > > > > I think clients is also required to program non-zero smr mask, where as
> > > > > > iommu-map just maps the id to an IOMMU specifier(sid). Please LMK if I
> > > > > > am unable to catch your thought here.
> > > > > An IOMMU specifier is whatever the target IOMMU node's #iommu-cells says it
> > > > > is. The fact that Linux's parsing code only works properly for #iommu-cells
> > > > > = 1 is not really a DT binding problem (other than it stemming from a loose
> > > > > assumption stated in the PCI binding's use of the property).
> > > >
> > > > I really don't like the idea of extending the #iommu-cells. The ARM SMMU
> > > > has only one cell, which is correct even for our platforms. The fact
> > > > that we need to identify different IOMMU SIDs (and handle them in a
> > > > differnt ways) is internal to the video device (and several other
> > > > devices). There is nothing to be handled on the ARM SMMU side.
> > >
> > > Huh? So if you prefer not to change anything, are you suggesting this series
> > > doesn't need to exist at all? Now I'm thoroughly confused...
> >
> > Hmm. We need changes, but I don't feel like adding the FUNCTION_ID to
> > #iommu-cells is the best idea.
>
> What? No, any function ID would be an *input* to a map, not part of the
> output specifier; indeed it should never go anywhere near the IOMMU, I don't
> think anyone suggested that.
It was Bryan, https://lore.kernel.org/linux-arm-msm/9bae595a-597e-46e6-8eb2-44424fe21db6@linaro.org
>
> > > If you want to use SMR masks, then you absolutely need #iommu-cells = 2,
> > > because that is the SMMU binding for using SMR masks. It would definitely
> >
> > I'm sorry. Yes, we have #iommu-cells = <2>.
> >
> > > not be OK to have some magic property trying to smuggle
> > > IOMMU-driver-specific data contrary to what the IOMMU node itself says. As
> > > for iommu-map, I don't see what would be objectionable about improving the
> > > parsing to respect a real #iommu-cells value rather than hard-coding an
> > > assumption. Yes, we'd probably need to forbid entries with length > 1
> > > targeting IOMMUs with #iommu-cells > 1, since the notion of a linear
> >
> > This will break e.g. PCIe on Qualcomm platforms:
> >
> > iommu-map = <0x0 &apps_smmu 0x1400 0x1>,
> > <0x100 &apps_smmu 0x1401 0x1>;
> >
> >
> > But this seems unlogical anyway wrt. apps_smmu having #iommu-cells =
> > <2>. It depends on ARM SMMU ignoring the second cell when it's not
> > present.
>
> Urgh, yes, that's just broken already :(
>
> At least they all seem to be a sufficiently consistent pattern that a
> targeted workaround to detect old DTBs looks feasible (I'm thinking, if
> iommu-map size % 4 == 0 and cells n*4 + 3 are all 1 and cells n*4 + 1 are
> all the same phandle to an IOMMU with #iommu-cells == 2, then parse as if
> #iommu-cells == 1)
How do we handle the case of #iommu-cells = <2>? I.e. what should be the
"fixed" representation of the map above? Should we have usual cells and
one extra "length" just for the sake of it?
iommu-map = <0x0 &apps_smmu 0x1400 0x0 0x1>,
<0x100 &apps_smmu 0x1401 0x0 0x1>;
I really like the idea of fixing iommu-map as that would remove the need
for other properties, but
>
> > > relationship between the input ID and the output specifier falls apart when
> > > the specifier is complex, but that seems simple enough to implement and
> > > document (even if it's too fiddly to describe in the schema itself), and
> > > still certainly no worse than having another property that *is* just
> > > iommu-map with implicit length = 1.
> > >
> > > And if you want individual StreamIDs for logical functions to be attachable
> > > to distinct contexts then those functions absolutely must be visible to the
> > > IOMMU layer and the SMMU driver as independent devices with their own unique
> > > properties, which means either they come that way from the DT as of_platform
> > > devices in the first place, or you implement a full bus_type abstraction
> >
> > Not necessarily. Tegra display driver creates a device for each context
> > on its own.
> No, the *display* driver does not; the host1x bus driver does, which is the
> point I was making - that has a proper bus abstraction tied into the IOMMU
> layer, such that the devices are correctly configured long before the actual
> DRM driver(s) get anywhere near them.
Ack. I agree. it's drivers/gpu/host1x/context, not drivers/gpu/drm/
>
> > In fact, using OF to create context devices is _less_
> > robust, because now the driver needs to sync, checking that there is a
> > subdevice, that it has probed, etc. Using manually created devices seems
> > better from my POV.
>
> Huh? A simple call to of_platform_populate() is somehow less robust than
> open-coding much of the same logic that of_platform_populate() does plus a
> bunch of hackery to try to fake up an of_node to make the new device appear
> to own the appropriate properties?
>
> Having entire sub-*drivers* for child devices or not is an orthogonal issue
> regardless of whichever way they are created.
I was (again) looking at host1x. It doesn't fake of_node (nor does it
have actual OF nodes). Instead it just mapps IOMMUs directly to the
context devices. Compare this to misc/fastrpc.c, which has subdevices
and drivers to map contexts. The latter one looks less robust.
And from DT perspective compare:
fastrpc {
compatible = "qcom,fastrpc";
#address-cells = <1>;
#size-cells = <0>;
compute-cb@3 {
compatible = "qcom,fastrpc-compute-cb";
reg = <3>;
iommus = <&apps_smmu 0x1803 0x0>;
};
compute-cb@4 {
compatible = "qcom,fastrpc-compute-cb";
reg = <4>;
iommus = <&apps_smmu 0x1804 0x0>;
};
compute-cb@5 {
compatible = "qcom,fastrpc-compute-cb";
reg = <5>;
iommus = <&apps_smmu 0x1805 0x0>;
};
};
VS (note, it doesn't have 'length', it can be added back with no issues):
fastrpc {
compatible = "qcom,fastrpc";
#address-cells = <1>;
#size-cells = <0>;
iommu-map = <3 &apps_smmu 0x1803 0x0>,
<4 &apps_smmu 0x1804 0x0>,
<5 &apps_smmu 0x1805 0x0>;
};
I think the latter is more compact, and more robust.
Note, to make a complete example, it should be probably something like
(sc7280, cdsp, note duplicate IDs in the map, again, I omitted length):
fastrpc {
compatible = "qcom,fastrpc";
iommu-map = <1 &apps_smmu 0x11a1 0x0420>,
<1 &apps_smmu 0x1181 0x0420>,
<2 &apps_smmu 0x11a2 0x0420>,
<2 &apps_smmu 0x1182 0x0420>,
<3 &apps_smmu 0x11a3 0x0420>,
<3 &apps_smmu 0x1183 0x0420>;
dma-coherent;
};
> > > which will have to be hooked up to the IOMMU layer. You cannot make IOMMU
> > > configuration "internal" to the actual client driver which is only allowed
> > > to bind *after* said IOMMU configuration has already been made.
> >
> > I'm not sure I follow this, I'm sorry.
> I mean IOMMU configuration is designed to happen at device_add() time, and
> client drivers must not assume otherwise (the mechanisms for handling IOMMU
> drivers registering "late" from modules are internal details that can and
> will change). If you're under the impression that a straightforward platform
> driver for the video codec itself would be able to invoke IOMMU
> configuration for the video codec platform device (without unacceptable
> levels of hackery) then you are mistaken, sorry.
>
> Again, to be able to assign StreamIDs to different contexts, those StreamIDs
> must uniquely belong to different struct devices. Thus in terms of how you
> get to those struct devices from a DT representation, either they come from
> distinct DT nodes with standard "iommus" properties that the generic
> of_platform code can create and configure accordingly, or you're doing a
> non-trivial amount of work to implement your own bus layer like
> host1x_context_bus to manage your own type of sub-device. There is no valid
> middle ground of trying to stuff driver-specific knowledge of arbitrarily
> made-up function IDs into the generic platform bus code.
I'd totally prefer something like:
video-codec@foobar {
compatible = "qcom,video";
iommus = <&apps_smmu 0x1234 0xca>;
iommu-maps = <PIXEL &apps_smmu 0xabcdef 0xac>,
<SECURE_PIXEL &apps_smmu 0x898989 0xac>,
<SECURE_BITSTREAM &apps_smmu 0x898998 0xac>;
};
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-13 12:31 ` Dmitry Baryshkov
@ 2025-10-14 14:07 ` Robin Murphy
2025-10-14 18:33 ` Dmitry Baryshkov
2025-10-15 8:32 ` Charan Teja Kalla
2025-10-14 15:07 ` Bryan O'Donoghue
1 sibling, 2 replies; 36+ messages in thread
From: Robin Murphy @ 2025-10-14 14:07 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Charan Teja Kalla, joro, will, saravanak, conor+dt, robh, mchehab,
bod, krzk+dt, abhinav.kumar, vikash.garodia, dikshita.agarwal,
Konrad Dybcio, bjorn.andersson, linux-media, linux-arm-msm,
devicetree, linux-kernel, iommu
On 2025-10-13 1:31 pm, Dmitry Baryshkov wrote:
> On Mon, Oct 13, 2025 at 12:20:54PM +0100, Robin Murphy wrote:
>> On 2025-10-09 7:25 pm, Dmitry Baryshkov wrote:
>>> On Thu, Oct 09, 2025 at 06:03:29PM +0100, Robin Murphy wrote:
>>>> On 2025-10-09 2:19 pm, Dmitry Baryshkov wrote:
>>>>> On Thu, Oct 09, 2025 at 11:46:55AM +0100, Robin Murphy wrote:
>>>>>> On 2025-10-08 8:10 pm, Charan Teja Kalla wrote:
>>>>>>>
>>>>>>> On 9/29/2025 3:50 PM, Robin Murphy wrote:
>>>>>>>>> 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 sub blocks are __configured__ to
>>>>>>>>> generate different stream IDs.
>>>>>>>>
>>>>>>>> So please clarify why you can't:
>>>>>>>>
>>>>>>>> a) Describe the sub-blocks as individual child nodes each with their own
>>>>>>>> distinct "iommus" property
>>>>>>>>
>>>>>>>
>>>>>>> Thanks Robin for your time. Sorry for late reply as I really didn't have
>>>>>>> concrete answer for this question.
>>>>>>>
>>>>>>> First let me clarify the word "sub blocks" -- This is just the logical
>>>>>>> separation with no separate address space to really able to define them
>>>>>>> as sub devices. Think of it like a single video IP with 2 dma
>>>>>>> engines(used for pixel and non-pixel purpose).
>>>>>>>
>>>>>>> I should agree that the child-nodes in the device tree is the easy one
>>>>>>> and infact, it is how being used in downstream.
>>>>>>>
>>>>>>> For upstream -- Since there is no real address space to interact with
>>>>>>> these sub-blocks(or logical blocks), does it really qualify to define as
>>>>>>> child nodes in the device tree? I see there is some push back[1].
>>>>>>
>>>>>> Who says you need an address space? Child nodes without "reg" properties,
>>>>>> referenced by name, compatible or phandle, exist all over the place for all
>>>>>> manner of reasons. If there are distinct logical functions with their own
>>>>>> distinct hardware properties, then I would say having child nodes to
>>>>>> describe and associate those properties with their respective functions is
>>>>>> entirely natural and appropriate. The first example that comes to mind of
>>>>>> where this is a well-established practice is PMICs - to pick one at random:
>>>>>> Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml
>>>>>
>>>>> Logical function, that's correct. And also note, for PMICs that practice
>>>>> has bitten us back. For PM8008 we switched back to a non-subdevice
>>>>> representation.
>>>>>
>>>>>> For bonus irony, you can't take the other approaches without inherently
>>>>>> *introducing* a notional address space in the form of your logical function
>>>>>> IDs anyway.
>>>>>>
>>>>>>> > or:
>>>>>>>>
>>>>>>>> b) Use standard "iommu-map" which already supports mapping a masked
>>>>>>>> input ID to an arbitrary IOMMU specifier
>>>>>>>>
>>>>>>>
>>>>>>> I think clients is also required to program non-zero smr mask, where as
>>>>>>> iommu-map just maps the id to an IOMMU specifier(sid). Please LMK if I
>>>>>>> am unable to catch your thought here.
>>>>>> An IOMMU specifier is whatever the target IOMMU node's #iommu-cells says it
>>>>>> is. The fact that Linux's parsing code only works properly for #iommu-cells
>>>>>> = 1 is not really a DT binding problem (other than it stemming from a loose
>>>>>> assumption stated in the PCI binding's use of the property).
>>>>>
>>>>> I really don't like the idea of extending the #iommu-cells. The ARM SMMU
>>>>> has only one cell, which is correct even for our platforms. The fact
>>>>> that we need to identify different IOMMU SIDs (and handle them in a
>>>>> differnt ways) is internal to the video device (and several other
>>>>> devices). There is nothing to be handled on the ARM SMMU side.
>>>>
>>>> Huh? So if you prefer not to change anything, are you suggesting this series
>>>> doesn't need to exist at all? Now I'm thoroughly confused...
>>>
>>> Hmm. We need changes, but I don't feel like adding the FUNCTION_ID to
>>> #iommu-cells is the best idea.
>>
>> What? No, any function ID would be an *input* to a map, not part of the
>> output specifier; indeed it should never go anywhere near the IOMMU, I don't
>> think anyone suggested that.
>
> It was Bryan, https://lore.kernel.org/linux-arm-msm/9bae595a-597e-46e6-8eb2-44424fe21db6@linaro.org
Ah, I wasn't on that thread. But indeed, as I hopefully explained
before, that whole idea is a non-starter anyway due to who the consumers
of "iommus" actually are.
>>>> If you want to use SMR masks, then you absolutely need #iommu-cells = 2,
>>>> because that is the SMMU binding for using SMR masks. It would definitely
>>>
>>> I'm sorry. Yes, we have #iommu-cells = <2>.
>>>
>>>> not be OK to have some magic property trying to smuggle
>>>> IOMMU-driver-specific data contrary to what the IOMMU node itself says. As
>>>> for iommu-map, I don't see what would be objectionable about improving the
>>>> parsing to respect a real #iommu-cells value rather than hard-coding an
>>>> assumption. Yes, we'd probably need to forbid entries with length > 1
>>>> targeting IOMMUs with #iommu-cells > 1, since the notion of a linear
>>>
>>> This will break e.g. PCIe on Qualcomm platforms:
>>>
>>> iommu-map = <0x0 &apps_smmu 0x1400 0x1>,
>>> <0x100 &apps_smmu 0x1401 0x1>;
>>>
>>>
>>> But this seems unlogical anyway wrt. apps_smmu having #iommu-cells =
>>> <2>. It depends on ARM SMMU ignoring the second cell when it's not
>>> present.
>>
>> Urgh, yes, that's just broken already :(
>>
>> At least they all seem to be a sufficiently consistent pattern that a
>> targeted workaround to detect old DTBs looks feasible (I'm thinking, if
>> iommu-map size % 4 == 0 and cells n*4 + 3 are all 1 and cells n*4 + 1 are
>> all the same phandle to an IOMMU with #iommu-cells == 2, then parse as if
>> #iommu-cells == 1)
>
> How do we handle the case of #iommu-cells = <2>? I.e. what should be the
> "fixed" representation of the map above? Should we have usual cells and
> one extra "length" just for the sake of it?
It's not really "for the sake of it", it is the defined format of the
"iommu-map" binding - IMO it would be far more horrible if each entry
did or didn't include a length cell depending on the size of the
preceding IOMMU specifier. It's also far from infeasible to have *some*
well-defined relationship between a non-singular input ID range and a
multi-cell base IOMMU specifier, it just needs more IOMMU-specific
interpretation in the consumer than Linux cares to bother with. Thus it
is appropriate for the binding to be able to describe that even though
Linux as a consumer continues to refuse to support it. The binding does
not describe Linux, or the property would be named "linux,iommu-map".
> iommu-map = <0x0 &apps_smmu 0x1400 0x0 0x1>,
> <0x100 &apps_smmu 0x1401 0x0 0x1>;
>
>
> I really like the idea of fixing iommu-map as that would remove the need
> for other properties, but
>
>>
>>>> relationship between the input ID and the output specifier falls apart when
>>>> the specifier is complex, but that seems simple enough to implement and
>>>> document (even if it's too fiddly to describe in the schema itself), and
>>>> still certainly no worse than having another property that *is* just
>>>> iommu-map with implicit length = 1.
>>>>
>>>> And if you want individual StreamIDs for logical functions to be attachable
>>>> to distinct contexts then those functions absolutely must be visible to the
>>>> IOMMU layer and the SMMU driver as independent devices with their own unique
>>>> properties, which means either they come that way from the DT as of_platform
>>>> devices in the first place, or you implement a full bus_type abstraction
>>>
>>> Not necessarily. Tegra display driver creates a device for each context
>>> on its own.
>> No, the *display* driver does not; the host1x bus driver does, which is the
>> point I was making - that has a proper bus abstraction tied into the IOMMU
>> layer, such that the devices are correctly configured long before the actual
>> DRM driver(s) get anywhere near them.
>
> Ack. I agree. it's drivers/gpu/host1x/context, not drivers/gpu/drm/
>
>>
>>> In fact, using OF to create context devices is _less_
>>> robust, because now the driver needs to sync, checking that there is a
>>> subdevice, that it has probed, etc. Using manually created devices seems
>>> better from my POV.
>>
>> Huh? A simple call to of_platform_populate() is somehow less robust than
>> open-coding much of the same logic that of_platform_populate() does plus a
>> bunch of hackery to try to fake up an of_node to make the new device appear
>> to own the appropriate properties?
>>
>> Having entire sub-*drivers* for child devices or not is an orthogonal issue
>> regardless of whichever way they are created.
>
> I was (again) looking at host1x. It doesn't fake of_node (nor does it
> have actual OF nodes). Instead it just mapps IOMMUs directly to the
> context devices. Compare this to misc/fastrpc.c, which has subdevices
> and drivers to map contexts. The latter one looks less robust.
>
> And from DT perspective compare:
>
> fastrpc {
> compatible = "qcom,fastrpc";
> #address-cells = <1>;
> #size-cells = <0>;
>
> compute-cb@3 {
> compatible = "qcom,fastrpc-compute-cb";
> reg = <3>;
> iommus = <&apps_smmu 0x1803 0x0>;
> };
>
> compute-cb@4 {
> compatible = "qcom,fastrpc-compute-cb";
> reg = <4>;
> iommus = <&apps_smmu 0x1804 0x0>;
> };
>
> compute-cb@5 {
> compatible = "qcom,fastrpc-compute-cb";
> reg = <5>;
> iommus = <&apps_smmu 0x1805 0x0>;
> };
> };
>
> VS (note, it doesn't have 'length', it can be added back with no issues):
>
> fastrpc {
> compatible = "qcom,fastrpc";
> #address-cells = <1>;
> #size-cells = <0>;
>
> iommu-map = <3 &apps_smmu 0x1803 0x0>,
> <4 &apps_smmu 0x1804 0x0>,
> <5 &apps_smmu 0x1805 0x0>;
> };
>
>
> I think the latter is more compact, and more robust.
For that particular case I concur that iommu-map might fit just as well,
since it appears similar to the Tegra one - essentially just a pool of
identical hardware contexts with no special individual properties, whose
purpose is defined by the software using them (be that the driver
itself, or the firmware on the other end). IOW, the DT really isn't
describing anything more than a mapping between a context ID and an
IOMMU specifier either way.
That said I also see nothing immediately wrong with the fastrpc driver
as-is either; if anything it looks like a pretty ideal example of the
"self-contained" non-bus approach I was alluding to. The "fake of_node"
notion only applies to the idea of trying to keep that same driver
structure but just replace of_platform_populate() with conjuring
platform_devices out of thin air.
> Note, to make a complete example, it should be probably something like
> (sc7280, cdsp, note duplicate IDs in the map, again, I omitted length):
>
> fastrpc {
> compatible = "qcom,fastrpc";
>
> iommu-map = <1 &apps_smmu 0x11a1 0x0420>,
> <1 &apps_smmu 0x1181 0x0420>,
> <2 &apps_smmu 0x11a2 0x0420>,
> <2 &apps_smmu 0x1182 0x0420>,
> <3 &apps_smmu 0x11a3 0x0420>,
> <3 &apps_smmu 0x1183 0x0420>;
Note that as another orthogonal issue, Linux also doesn't support 1:many
maps like that - we'll only parse the first matching entry. However this
specific example (and the current DTs) doesn't make sense anyway, since
each pair of SMRs encodes the same set of matches (0x118x, 0x11ax,
0x158x, 0x15ax), so at best it's redundant while at worst it's a stream
match conflict fault waiting to happen?
> dma-coherent;
> };
>
>
>>>> which will have to be hooked up to the IOMMU layer. You cannot make IOMMU
>>>> configuration "internal" to the actual client driver which is only allowed
>>>> to bind *after* said IOMMU configuration has already been made.
>>>
>>> I'm not sure I follow this, I'm sorry.
>> I mean IOMMU configuration is designed to happen at device_add() time, and
>> client drivers must not assume otherwise (the mechanisms for handling IOMMU
>> drivers registering "late" from modules are internal details that can and
>> will change). If you're under the impression that a straightforward platform
>> driver for the video codec itself would be able to invoke IOMMU
>> configuration for the video codec platform device (without unacceptable
>> levels of hackery) then you are mistaken, sorry.
>>
>> Again, to be able to assign StreamIDs to different contexts, those StreamIDs
>> must uniquely belong to different struct devices. Thus in terms of how you
>> get to those struct devices from a DT representation, either they come from
>> distinct DT nodes with standard "iommus" properties that the generic
>> of_platform code can create and configure accordingly, or you're doing a
>> non-trivial amount of work to implement your own bus layer like
>> host1x_context_bus to manage your own type of sub-device. There is no valid
>> middle ground of trying to stuff driver-specific knowledge of arbitrarily
>> made-up function IDs into the generic platform bus code.
>
>
> I'd totally prefer something like:
>
> video-codec@foobar {
> compatible = "qcom,video";
>
> iommus = <&apps_smmu 0x1234 0xca>;
> iommu-maps = <PIXEL &apps_smmu 0xabcdef 0xac>,
> <SECURE_PIXEL &apps_smmu 0x898989 0xac>,
> <SECURE_BITSTREAM &apps_smmu 0x898998 0xac>;
> };
This is where I maintain a differing opinion - if it's *not* a "pool of
identical contexts" case, but a single nominal hardware block with a
small number of distinct DMA streams for fundamentally different
purposes defined by the hardware design, then I would usually consider
it more natural, honest and useful to make those differences explicit by
name/compatible with child nodes, rather than hide them behind an opaque
arbitrary integer. If by nature of being functionally different they
also might require individual properties - such as memory-regions - then
child nodes are the only option anyway.
However, if there is actually some meaningful hardware notion of
"function ID", the design/usage model is such that it would generally be
logical for a consumer driver to be structured as managing a set of
fixed-function sub-devices on an internal bus, and you're absolutely
definite that those sub-devices will never ever need any DT properties
of their own in future revisions/integrations, then maybe an
"iommu-map"-based binding is OK. All I can say for sure is that
describing complex hardware well is very nuanced and there is no one
universal right answer.
Thanks,
Robin.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-13 12:31 ` Dmitry Baryshkov
2025-10-14 14:07 ` Robin Murphy
@ 2025-10-14 15:07 ` Bryan O'Donoghue
2025-10-14 18:35 ` Dmitry Baryshkov
1 sibling, 1 reply; 36+ messages in thread
From: Bryan O'Donoghue @ 2025-10-14 15:07 UTC (permalink / raw)
To: Dmitry Baryshkov, Robin Murphy
Cc: Charan Teja Kalla, joro, will, saravanak, conor+dt, robh, mchehab,
krzk+dt, abhinav.kumar, vikash.garodia, dikshita.agarwal,
Konrad Dybcio, bjorn.andersson, linux-media, linux-arm-msm,
devicetree, linux-kernel, iommu
On 13/10/2025 13:31, Dmitry Baryshkov wrote:
> On Mon, Oct 13, 2025 at 12:20:54PM +0100, Robin Murphy wrote:
>> On 2025-10-09 7:25 pm, Dmitry Baryshkov wrote:
>>> On Thu, Oct 09, 2025 at 06:03:29PM +0100, Robin Murphy wrote:
>>>> On 2025-10-09 2:19 pm, Dmitry Baryshkov wrote:
>>>>> On Thu, Oct 09, 2025 at 11:46:55AM +0100, Robin Murphy wrote:
>>>>>> On 2025-10-08 8:10 pm, Charan Teja Kalla wrote:
>>>>>>>
>>>>>>> On 9/29/2025 3:50 PM, Robin Murphy wrote:
>>>>>>>>> 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 sub blocks are __configured__ to
>>>>>>>>> generate different stream IDs.
>>>>>>>>
>>>>>>>> So please clarify why you can't:
>>>>>>>>
>>>>>>>> a) Describe the sub-blocks as individual child nodes each with their own
>>>>>>>> distinct "iommus" property
>>>>>>>>
>>>>>>>
>>>>>>> Thanks Robin for your time. Sorry for late reply as I really didn't have
>>>>>>> concrete answer for this question.
>>>>>>>
>>>>>>> First let me clarify the word "sub blocks" -- This is just the logical
>>>>>>> separation with no separate address space to really able to define them
>>>>>>> as sub devices. Think of it like a single video IP with 2 dma
>>>>>>> engines(used for pixel and non-pixel purpose).
>>>>>>>
>>>>>>> I should agree that the child-nodes in the device tree is the easy one
>>>>>>> and infact, it is how being used in downstream.
>>>>>>>
>>>>>>> For upstream -- Since there is no real address space to interact with
>>>>>>> these sub-blocks(or logical blocks), does it really qualify to define as
>>>>>>> child nodes in the device tree? I see there is some push back[1].
>>>>>>
>>>>>> Who says you need an address space? Child nodes without "reg" properties,
>>>>>> referenced by name, compatible or phandle, exist all over the place for all
>>>>>> manner of reasons. If there are distinct logical functions with their own
>>>>>> distinct hardware properties, then I would say having child nodes to
>>>>>> describe and associate those properties with their respective functions is
>>>>>> entirely natural and appropriate. The first example that comes to mind of
>>>>>> where this is a well-established practice is PMICs - to pick one at random:
>>>>>> Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml
>>>>>
>>>>> Logical function, that's correct. And also note, for PMICs that practice
>>>>> has bitten us back. For PM8008 we switched back to a non-subdevice
>>>>> representation.
>>>>>
>>>>>> For bonus irony, you can't take the other approaches without inherently
>>>>>> *introducing* a notional address space in the form of your logical function
>>>>>> IDs anyway.
>>>>>>
>>>>>>> > or:
>>>>>>>>
>>>>>>>> b) Use standard "iommu-map" which already supports mapping a masked
>>>>>>>> input ID to an arbitrary IOMMU specifier
>>>>>>>>
>>>>>>>
>>>>>>> I think clients is also required to program non-zero smr mask, where as
>>>>>>> iommu-map just maps the id to an IOMMU specifier(sid). Please LMK if I
>>>>>>> am unable to catch your thought here.
>>>>>> An IOMMU specifier is whatever the target IOMMU node's #iommu-cells says it
>>>>>> is. The fact that Linux's parsing code only works properly for #iommu-cells
>>>>>> = 1 is not really a DT binding problem (other than it stemming from a loose
>>>>>> assumption stated in the PCI binding's use of the property).
>>>>>
>>>>> I really don't like the idea of extending the #iommu-cells. The ARM SMMU
>>>>> has only one cell, which is correct even for our platforms. The fact
>>>>> that we need to identify different IOMMU SIDs (and handle them in a
>>>>> differnt ways) is internal to the video device (and several other
>>>>> devices). There is nothing to be handled on the ARM SMMU side.
>>>>
>>>> Huh? So if you prefer not to change anything, are you suggesting this series
>>>> doesn't need to exist at all? Now I'm thoroughly confused...
>>>
>>> Hmm. We need changes, but I don't feel like adding the FUNCTION_ID to
>>> #iommu-cells is the best idea.
>>
>> What? No, any function ID would be an *input* to a map, not part of the
>> output specifier; indeed it should never go anywhere near the IOMMU, I don't
>> think anyone suggested that.
>
> It was Bryan, https://lore.kernel.org/linux-arm-msm/9bae595a-597e-46e6-8eb2-44424fe21db6@linaro.org
>
>>
>>>> If you want to use SMR masks, then you absolutely need #iommu-cells = 2,
>>>> because that is the SMMU binding for using SMR masks. It would definitely
>>>
>>> I'm sorry. Yes, we have #iommu-cells = <2>.
>>>
>>>> not be OK to have some magic property trying to smuggle
>>>> IOMMU-driver-specific data contrary to what the IOMMU node itself says. As
>>>> for iommu-map, I don't see what would be objectionable about improving the
>>>> parsing to respect a real #iommu-cells value rather than hard-coding an
>>>> assumption. Yes, we'd probably need to forbid entries with length > 1
>>>> targeting IOMMUs with #iommu-cells > 1, since the notion of a linear
>>>
>>> This will break e.g. PCIe on Qualcomm platforms:
>>>
>>> iommu-map = <0x0 &apps_smmu 0x1400 0x1>,
>>> <0x100 &apps_smmu 0x1401 0x1>;
>>>
>>>
>>> But this seems unlogical anyway wrt. apps_smmu having #iommu-cells =
>>> <2>. It depends on ARM SMMU ignoring the second cell when it's not
>>> present.
>>
>> Urgh, yes, that's just broken already :(
>>
>> At least they all seem to be a sufficiently consistent pattern that a
>> targeted workaround to detect old DTBs looks feasible (I'm thinking, if
>> iommu-map size % 4 == 0 and cells n*4 + 3 are all 1 and cells n*4 + 1 are
>> all the same phandle to an IOMMU with #iommu-cells == 2, then parse as if
>> #iommu-cells == 1)
>
> How do we handle the case of #iommu-cells = <2>? I.e. what should be the
> "fixed" representation of the map above? Should we have usual cells and
> one extra "length" just for the sake of it?
>
> iommu-map = <0x0 &apps_smmu 0x1400 0x0 0x1>,
> <0x100 &apps_smmu 0x1401 0x0 0x1>;
>
>
> I really like the idea of fixing iommu-map as that would remove the need
> for other properties, but
>
>>
>>>> relationship between the input ID and the output specifier falls apart when
>>>> the specifier is complex, but that seems simple enough to implement and
>>>> document (even if it's too fiddly to describe in the schema itself), and
>>>> still certainly no worse than having another property that *is* just
>>>> iommu-map with implicit length = 1.
>>>>
>>>> And if you want individual StreamIDs for logical functions to be attachable
>>>> to distinct contexts then those functions absolutely must be visible to the
>>>> IOMMU layer and the SMMU driver as independent devices with their own unique
>>>> properties, which means either they come that way from the DT as of_platform
>>>> devices in the first place, or you implement a full bus_type abstraction
>>>
>>> Not necessarily. Tegra display driver creates a device for each context
>>> on its own.
>> No, the *display* driver does not; the host1x bus driver does, which is the
>> point I was making - that has a proper bus abstraction tied into the IOMMU
>> layer, such that the devices are correctly configured long before the actual
>> DRM driver(s) get anywhere near them.
>
> Ack. I agree. it's drivers/gpu/host1x/context, not drivers/gpu/drm/
>
>>
>>> In fact, using OF to create context devices is _less_
>>> robust, because now the driver needs to sync, checking that there is a
>>> subdevice, that it has probed, etc. Using manually created devices seems
>>> better from my POV.
>>
>> Huh? A simple call to of_platform_populate() is somehow less robust than
>> open-coding much of the same logic that of_platform_populate() does plus a
>> bunch of hackery to try to fake up an of_node to make the new device appear
>> to own the appropriate properties?
>>
>> Having entire sub-*drivers* for child devices or not is an orthogonal issue
>> regardless of whichever way they are created.
>
> I was (again) looking at host1x. It doesn't fake of_node (nor does it
> have actual OF nodes). Instead it just mapps IOMMUs directly to the
> context devices. Compare this to misc/fastrpc.c, which has subdevices
> and drivers to map contexts. The latter one looks less robust.
>
> And from DT perspective compare:
>
> fastrpc {
> compatible = "qcom,fastrpc";
> #address-cells = <1>;
> #size-cells = <0>;
>
> compute-cb@3 {
> compatible = "qcom,fastrpc-compute-cb";
> reg = <3>;
> iommus = <&apps_smmu 0x1803 0x0>;
> };
>
> compute-cb@4 {
> compatible = "qcom,fastrpc-compute-cb";
> reg = <4>;
> iommus = <&apps_smmu 0x1804 0x0>;
> };
>
> compute-cb@5 {
> compatible = "qcom,fastrpc-compute-cb";
> reg = <5>;
> iommus = <&apps_smmu 0x1805 0x0>;
> };
> };
Sorry this is perfect.
Each function id can be associated with a device and a compat string
associated with it.
There's no weirdness with iommu-map, you get a struct device for your
SID and you associate the SID with the FUNCTION_ID you want.
In fact the FUNCTION_ID could conceivably be the reg. It could be stored
in platform code.
---
bod
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-14 14:07 ` Robin Murphy
@ 2025-10-14 18:33 ` Dmitry Baryshkov
2025-10-15 8:32 ` Charan Teja Kalla
1 sibling, 0 replies; 36+ messages in thread
From: Dmitry Baryshkov @ 2025-10-14 18:33 UTC (permalink / raw)
To: Robin Murphy
Cc: Charan Teja Kalla, joro, will, saravanak, conor+dt, robh, mchehab,
bod, krzk+dt, abhinav.kumar, vikash.garodia, dikshita.agarwal,
Konrad Dybcio, bjorn.andersson, linux-media, linux-arm-msm,
devicetree, linux-kernel, iommu
On Tue, Oct 14, 2025 at 03:07:34PM +0100, Robin Murphy wrote:
> On 2025-10-13 1:31 pm, Dmitry Baryshkov wrote:
> > On Mon, Oct 13, 2025 at 12:20:54PM +0100, Robin Murphy wrote:
> > > On 2025-10-09 7:25 pm, Dmitry Baryshkov wrote:
> > > > On Thu, Oct 09, 2025 at 06:03:29PM +0100, Robin Murphy wrote:
> > > > > On 2025-10-09 2:19 pm, Dmitry Baryshkov wrote:
> > > > > > On Thu, Oct 09, 2025 at 11:46:55AM +0100, Robin Murphy wrote:
> > > > > > > On 2025-10-08 8:10 pm, Charan Teja Kalla wrote:
> > > > > > > >
> > > > > > > > On 9/29/2025 3:50 PM, Robin Murphy wrote:
> > > > > > > > > > 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 sub blocks are __configured__ to
> > > > > > > > > > generate different stream IDs.
> > > > > > > > >
> > > > > > > > > So please clarify why you can't:
> > > > > > > > >
> > > > > > > > > a) Describe the sub-blocks as individual child nodes each with their own
> > > > > > > > > distinct "iommus" property
> > > > > > > > >
> > > > > > > >
> > > > > > > > Thanks Robin for your time. Sorry for late reply as I really didn't have
> > > > > > > > concrete answer for this question.
> > > > > > > >
> > > > > > > > First let me clarify the word "sub blocks" -- This is just the logical
> > > > > > > > separation with no separate address space to really able to define them
> > > > > > > > as sub devices. Think of it like a single video IP with 2 dma
> > > > > > > > engines(used for pixel and non-pixel purpose).
> > > > > > > >
> > > > > > > > I should agree that the child-nodes in the device tree is the easy one
> > > > > > > > and infact, it is how being used in downstream.
> > > > > > > >
> > > > > > > > For upstream -- Since there is no real address space to interact with
> > > > > > > > these sub-blocks(or logical blocks), does it really qualify to define as
> > > > > > > > child nodes in the device tree? I see there is some push back[1].
> > > > > > >
> > > > > > > Who says you need an address space? Child nodes without "reg" properties,
> > > > > > > referenced by name, compatible or phandle, exist all over the place for all
> > > > > > > manner of reasons. If there are distinct logical functions with their own
> > > > > > > distinct hardware properties, then I would say having child nodes to
> > > > > > > describe and associate those properties with their respective functions is
> > > > > > > entirely natural and appropriate. The first example that comes to mind of
> > > > > > > where this is a well-established practice is PMICs - to pick one at random:
> > > > > > > Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml
> > > > > >
> > > > > > Logical function, that's correct. And also note, for PMICs that practice
> > > > > > has bitten us back. For PM8008 we switched back to a non-subdevice
> > > > > > representation.
> > > > > >
> > > > > > > For bonus irony, you can't take the other approaches without inherently
> > > > > > > *introducing* a notional address space in the form of your logical function
> > > > > > > IDs anyway.
> > > > > > >
> > > > > > > > > or:
> > > > > > > > >
> > > > > > > > > b) Use standard "iommu-map" which already supports mapping a masked
> > > > > > > > > input ID to an arbitrary IOMMU specifier
> > > > > > > > >
> > > > > > > >
> > > > > > > > I think clients is also required to program non-zero smr mask, where as
> > > > > > > > iommu-map just maps the id to an IOMMU specifier(sid). Please LMK if I
> > > > > > > > am unable to catch your thought here.
> > > > > > > An IOMMU specifier is whatever the target IOMMU node's #iommu-cells says it
> > > > > > > is. The fact that Linux's parsing code only works properly for #iommu-cells
> > > > > > > = 1 is not really a DT binding problem (other than it stemming from a loose
> > > > > > > assumption stated in the PCI binding's use of the property).
> > > > > >
> > > > > > I really don't like the idea of extending the #iommu-cells. The ARM SMMU
> > > > > > has only one cell, which is correct even for our platforms. The fact
> > > > > > that we need to identify different IOMMU SIDs (and handle them in a
> > > > > > differnt ways) is internal to the video device (and several other
> > > > > > devices). There is nothing to be handled on the ARM SMMU side.
> > > > >
> > > > > Huh? So if you prefer not to change anything, are you suggesting this series
> > > > > doesn't need to exist at all? Now I'm thoroughly confused...
> > > >
> > > > Hmm. We need changes, but I don't feel like adding the FUNCTION_ID to
> > > > #iommu-cells is the best idea.
> > >
> > > What? No, any function ID would be an *input* to a map, not part of the
> > > output specifier; indeed it should never go anywhere near the IOMMU, I don't
> > > think anyone suggested that.
> >
> > It was Bryan, https://lore.kernel.org/linux-arm-msm/9bae595a-597e-46e6-8eb2-44424fe21db6@linaro.org
>
> Ah, I wasn't on that thread. But indeed, as I hopefully explained before,
> that whole idea is a non-starter anyway due to who the consumers of "iommus"
> actually are.
>
> > > > > If you want to use SMR masks, then you absolutely need #iommu-cells = 2,
> > > > > because that is the SMMU binding for using SMR masks. It would definitely
> > > >
> > > > I'm sorry. Yes, we have #iommu-cells = <2>.
> > > >
> > > > > not be OK to have some magic property trying to smuggle
> > > > > IOMMU-driver-specific data contrary to what the IOMMU node itself says. As
> > > > > for iommu-map, I don't see what would be objectionable about improving the
> > > > > parsing to respect a real #iommu-cells value rather than hard-coding an
> > > > > assumption. Yes, we'd probably need to forbid entries with length > 1
> > > > > targeting IOMMUs with #iommu-cells > 1, since the notion of a linear
> > > >
> > > > This will break e.g. PCIe on Qualcomm platforms:
> > > >
> > > > iommu-map = <0x0 &apps_smmu 0x1400 0x1>,
> > > > <0x100 &apps_smmu 0x1401 0x1>;
> > > >
> > > >
> > > > But this seems unlogical anyway wrt. apps_smmu having #iommu-cells =
> > > > <2>. It depends on ARM SMMU ignoring the second cell when it's not
> > > > present.
> > >
> > > Urgh, yes, that's just broken already :(
> > >
> > > At least they all seem to be a sufficiently consistent pattern that a
> > > targeted workaround to detect old DTBs looks feasible (I'm thinking, if
> > > iommu-map size % 4 == 0 and cells n*4 + 3 are all 1 and cells n*4 + 1 are
> > > all the same phandle to an IOMMU with #iommu-cells == 2, then parse as if
> > > #iommu-cells == 1)
> >
> > How do we handle the case of #iommu-cells = <2>? I.e. what should be the
> > "fixed" representation of the map above? Should we have usual cells and
> > one extra "length" just for the sake of it?
>
> It's not really "for the sake of it", it is the defined format of the
> "iommu-map" binding - IMO it would be far more horrible if each entry did or
> didn't include a length cell depending on the size of the preceding IOMMU
> specifier. It's also far from infeasible to have *some* well-defined
> relationship between a non-singular input ID range and a multi-cell base
> IOMMU specifier, it just needs more IOMMU-specific interpretation in the
> consumer than Linux cares to bother with. Thus it is appropriate for the
> binding to be able to describe that even though Linux as a consumer
> continues to refuse to support it. The binding does not describe Linux, or
> the property would be named "linux,iommu-map".
Ack.
>
> > iommu-map = <0x0 &apps_smmu 0x1400 0x0 0x1>,
> > <0x100 &apps_smmu 0x1401 0x0 0x1>;
> >
> >
> > I really like the idea of fixing iommu-map as that would remove the need
> > for other properties, but
> >
> > >
> > > > > relationship between the input ID and the output specifier falls apart when
> > > > > the specifier is complex, but that seems simple enough to implement and
> > > > > document (even if it's too fiddly to describe in the schema itself), and
> > > > > still certainly no worse than having another property that *is* just
> > > > > iommu-map with implicit length = 1.
> > > > >
> > > > > And if you want individual StreamIDs for logical functions to be attachable
> > > > > to distinct contexts then those functions absolutely must be visible to the
> > > > > IOMMU layer and the SMMU driver as independent devices with their own unique
> > > > > properties, which means either they come that way from the DT as of_platform
> > > > > devices in the first place, or you implement a full bus_type abstraction
> > > >
> > > > Not necessarily. Tegra display driver creates a device for each context
> > > > on its own.
> > > No, the *display* driver does not; the host1x bus driver does, which is the
> > > point I was making - that has a proper bus abstraction tied into the IOMMU
> > > layer, such that the devices are correctly configured long before the actual
> > > DRM driver(s) get anywhere near them.
> >
> > Ack. I agree. it's drivers/gpu/host1x/context, not drivers/gpu/drm/
> >
> > >
> > > > In fact, using OF to create context devices is _less_
> > > > robust, because now the driver needs to sync, checking that there is a
> > > > subdevice, that it has probed, etc. Using manually created devices seems
> > > > better from my POV.
> > >
> > > Huh? A simple call to of_platform_populate() is somehow less robust than
> > > open-coding much of the same logic that of_platform_populate() does plus a
> > > bunch of hackery to try to fake up an of_node to make the new device appear
> > > to own the appropriate properties?
> > >
> > > Having entire sub-*drivers* for child devices or not is an orthogonal issue
> > > regardless of whichever way they are created.
> >
> > I was (again) looking at host1x. It doesn't fake of_node (nor does it
> > have actual OF nodes). Instead it just mapps IOMMUs directly to the
> > context devices. Compare this to misc/fastrpc.c, which has subdevices
> > and drivers to map contexts. The latter one looks less robust.
> >
> > And from DT perspective compare:
> >
> > fastrpc {
> > compatible = "qcom,fastrpc";
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > compute-cb@3 {
> > compatible = "qcom,fastrpc-compute-cb";
> > reg = <3>;
> > iommus = <&apps_smmu 0x1803 0x0>;
> > };
> >
> > compute-cb@4 {
> > compatible = "qcom,fastrpc-compute-cb";
> > reg = <4>;
> > iommus = <&apps_smmu 0x1804 0x0>;
> > };
> >
> > compute-cb@5 {
> > compatible = "qcom,fastrpc-compute-cb";
> > reg = <5>;
> > iommus = <&apps_smmu 0x1805 0x0>;
> > };
> > };
> >
> > VS (note, it doesn't have 'length', it can be added back with no issues):
> >
> > fastrpc {
> > compatible = "qcom,fastrpc";
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > iommu-map = <3 &apps_smmu 0x1803 0x0>,
> > <4 &apps_smmu 0x1804 0x0>,
> > <5 &apps_smmu 0x1805 0x0>;
> > };
> >
> >
> > I think the latter is more compact, and more robust.
>
> For that particular case I concur that iommu-map might fit just as well,
> since it appears similar to the Tegra one - essentially just a pool of
> identical hardware contexts with no special individual properties, whose
> purpose is defined by the software using them (be that the driver itself, or
> the firmware on the other end). IOW, the DT really isn't describing anything
> more than a mapping between a context ID and an IOMMU specifier either way.
Yep. Subdevices look like an overkill to me.
>
> That said I also see nothing immediately wrong with the fastrpc driver as-is
> either; if anything it looks like a pretty ideal example of the
> "self-contained" non-bus approach I was alluding to. The "fake of_node"
> notion only applies to the idea of trying to keep that same driver structure
> but just replace of_platform_populate() with conjuring platform_devices out
> of thin air.
This creates a bit of race conditions: there might be a time when the
main device has probed (and thus is available in /dev for the userspace
to ping), but the context driver haven't yet probed (or not probed
enough of them), causing weird heisenbugs to userspace.
> > Note, to make a complete example, it should be probably something like
> > (sc7280, cdsp, note duplicate IDs in the map, again, I omitted length):
> >
> > fastrpc {
> > compatible = "qcom,fastrpc";
> >
> > iommu-map = <1 &apps_smmu 0x11a1 0x0420>,
> > <1 &apps_smmu 0x1181 0x0420>,
> > <2 &apps_smmu 0x11a2 0x0420>,
> > <2 &apps_smmu 0x1182 0x0420>,
> > <3 &apps_smmu 0x11a3 0x0420>,
> > <3 &apps_smmu 0x1183 0x0420>;
>
> Note that as another orthogonal issue, Linux also doesn't support 1:many
> maps like that - we'll only parse the first matching entry. However this
> specific example (and the current DTs) doesn't make sense anyway, since each
> pair of SMRs encodes the same set of matches (0x118x, 0x11ax, 0x158x,
> 0x15ax), so at best it's redundant while at worst it's a stream match
> conflict fault waiting to happen?
In this case they seem superflux (although that remodelled from the
existing platform). I wanted to point out 1:many, as you guessed. But
anyway, that can be handled in software.
>
> > dma-coherent;
> > };
> >
> >
> > > > > which will have to be hooked up to the IOMMU layer. You cannot make IOMMU
> > > > > configuration "internal" to the actual client driver which is only allowed
> > > > > to bind *after* said IOMMU configuration has already been made.
> > > >
> > > > I'm not sure I follow this, I'm sorry.
> > > I mean IOMMU configuration is designed to happen at device_add() time, and
> > > client drivers must not assume otherwise (the mechanisms for handling IOMMU
> > > drivers registering "late" from modules are internal details that can and
> > > will change). If you're under the impression that a straightforward platform
> > > driver for the video codec itself would be able to invoke IOMMU
> > > configuration for the video codec platform device (without unacceptable
> > > levels of hackery) then you are mistaken, sorry.
> > >
> > > Again, to be able to assign StreamIDs to different contexts, those StreamIDs
> > > must uniquely belong to different struct devices. Thus in terms of how you
> > > get to those struct devices from a DT representation, either they come from
> > > distinct DT nodes with standard "iommus" properties that the generic
> > > of_platform code can create and configure accordingly, or you're doing a
> > > non-trivial amount of work to implement your own bus layer like
> > > host1x_context_bus to manage your own type of sub-device. There is no valid
> > > middle ground of trying to stuff driver-specific knowledge of arbitrarily
> > > made-up function IDs into the generic platform bus code.
> >
> >
> > I'd totally prefer something like:
> >
> > video-codec@foobar {
> > compatible = "qcom,video";
> >
> > iommus = <&apps_smmu 0x1234 0xca>;
> > iommu-maps = <PIXEL &apps_smmu 0xabcdef 0xac>,
> > <SECURE_PIXEL &apps_smmu 0x898989 0xac>,
> > <SECURE_BITSTREAM &apps_smmu 0x898998 0xac>;
> > };
> This is where I maintain a differing opinion - if it's *not* a "pool of
> identical contexts" case, but a single nominal hardware block with a small
> number of distinct DMA streams for fundamentally different purposes defined
> by the hardware design, then I would usually consider it more natural,
> honest and useful to make those differences explicit by name/compatible with
> child nodes, rather than hide them behind an opaque arbitrary integer. If by
> nature of being functionally different they also might require individual
> properties - such as memory-regions - then child nodes are the only option
> anyway.
>
> However, if there is actually some meaningful hardware notion of "function
> ID", the design/usage model is such that it would generally be logical for a
> consumer driver to be structured as managing a set of fixed-function
> sub-devices on an internal bus, and you're absolutely definite that those
> sub-devices will never ever need any DT properties of their own in future
> revisions/integrations, then maybe an "iommu-map"-based binding is OK. All I
> can say for sure is that describing complex hardware well is very nuanced
> and there is no one universal right answer.
Vikash has tried this initially ([1]) and got a rejection from Krzysztof
([2]).
[1] https://lore.kernel.org/linux-arm-msm/20250627-video_cb-v3-1-51e18c0ffbce@quicinc.com/
[2] https://lore.kernel.org/linux-arm-msm/24dd3e64-958a-41d5-8032-1da91063eaeb@kernel.org/
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-14 15:07 ` Bryan O'Donoghue
@ 2025-10-14 18:35 ` Dmitry Baryshkov
2025-10-14 20:49 ` Bryan O'Donoghue
0 siblings, 1 reply; 36+ messages in thread
From: Dmitry Baryshkov @ 2025-10-14 18:35 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: Robin Murphy, Charan Teja Kalla, joro, will, saravanak, conor+dt,
robh, mchehab, krzk+dt, abhinav.kumar, vikash.garodia,
dikshita.agarwal, Konrad Dybcio, bjorn.andersson, linux-media,
linux-arm-msm, devicetree, linux-kernel, iommu
On Tue, Oct 14, 2025 at 04:07:37PM +0100, Bryan O'Donoghue wrote:
> On 13/10/2025 13:31, Dmitry Baryshkov wrote:
> >
> > And from DT perspective compare:
> >
> > fastrpc {
> > compatible = "qcom,fastrpc";
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > compute-cb@3 {
> > compatible = "qcom,fastrpc-compute-cb";
> > reg = <3>;
> > iommus = <&apps_smmu 0x1803 0x0>;
> > };
> >
> > compute-cb@4 {
> > compatible = "qcom,fastrpc-compute-cb";
> > reg = <4>;
> > iommus = <&apps_smmu 0x1804 0x0>;
> > };
> >
> > compute-cb@5 {
> > compatible = "qcom,fastrpc-compute-cb";
> > reg = <5>;
> > iommus = <&apps_smmu 0x1805 0x0>;
> > };
> > };
>
> Sorry this is perfect.
>
> Each function id can be associated with a device and a compat string
> associated with it.
So, which part of the hardware is described by the -cb device? What does
it mean _here_?
> There's no weirdness with iommu-map, you get a struct device for your SID
> and you associate the SID with the FUNCTION_ID you want.
>
> In fact the FUNCTION_ID could conceivably be the reg. It could be stored in
> platform code.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-14 18:35 ` Dmitry Baryshkov
@ 2025-10-14 20:49 ` Bryan O'Donoghue
2025-10-14 22:18 ` Dmitry Baryshkov
2025-10-14 22:38 ` Dmitry Baryshkov
0 siblings, 2 replies; 36+ messages in thread
From: Bryan O'Donoghue @ 2025-10-14 20:49 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Robin Murphy, Charan Teja Kalla, joro, will, saravanak, conor+dt,
robh, mchehab, krzk+dt, abhinav.kumar, vikash.garodia,
dikshita.agarwal, Konrad Dybcio, bjorn.andersson, linux-media,
linux-arm-msm, devicetree, linux-kernel, iommu
On 14/10/2025 19:35, Dmitry Baryshkov wrote:
>> Each function id can be associated with a device and a compat string
>> associated with it.
> So, which part of the hardware is described by the -cb device? What does
> it mean_here_?
The non-pixel path video encoder, the tz video encoder...
What's not clear about that ?
---
bod
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-14 20:49 ` Bryan O'Donoghue
@ 2025-10-14 22:18 ` Dmitry Baryshkov
2025-10-15 8:53 ` Bryan O'Donoghue
2025-10-14 22:38 ` Dmitry Baryshkov
1 sibling, 1 reply; 36+ messages in thread
From: Dmitry Baryshkov @ 2025-10-14 22:18 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: Robin Murphy, Charan Teja Kalla, joro, will, saravanak, conor+dt,
robh, mchehab, krzk+dt, abhinav.kumar, vikash.garodia,
dikshita.agarwal, Konrad Dybcio, bjorn.andersson, linux-media,
linux-arm-msm, devicetree, linux-kernel, iommu
On Tue, Oct 14, 2025 at 09:49:17PM +0100, Bryan O'Donoghue wrote:
> On 14/10/2025 19:35, Dmitry Baryshkov wrote:
> > > Each function id can be associated with a device and a compat string
> > > associated with it.
> > So, which part of the hardware is described by the -cb device? What does
> > it mean_here_?
>
> The non-pixel path video encoder, the tz video encoder...
>
> What's not clear about that ?
Where do you have pixel encoders in the fastrpc device node?
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-14 20:49 ` Bryan O'Donoghue
2025-10-14 22:18 ` Dmitry Baryshkov
@ 2025-10-14 22:38 ` Dmitry Baryshkov
1 sibling, 0 replies; 36+ messages in thread
From: Dmitry Baryshkov @ 2025-10-14 22:38 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: Robin Murphy, Charan Teja Kalla, joro, will, saravanak, conor+dt,
robh, mchehab, krzk+dt, abhinav.kumar, vikash.garodia,
dikshita.agarwal, Konrad Dybcio, bjorn.andersson, linux-media,
linux-arm-msm, devicetree, linux-kernel, iommu
On Tue, Oct 14, 2025 at 09:49:17PM +0100, Bryan O'Donoghue wrote:
> On 14/10/2025 19:35, Dmitry Baryshkov wrote:
> > > Each function id can be associated with a device and a compat string
> > > associated with it.
> > So, which part of the hardware is described by the -cb device? What does
> > it mean_here_?
>
> The non-pixel path video encoder, the tz video encoder...
>
> What's not clear about that ?
And anyway. The "non-pixel path" doesn't sound like a piece of hardware
to me (nor did it to Krzysztof, [1]). It is just a port through which
the data is being transferred. The encoder acquires pixel data through a
pixel port, processes it and returns it back through a non-pixel port
and vice versa for the decoder.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-14 14:07 ` Robin Murphy
2025-10-14 18:33 ` Dmitry Baryshkov
@ 2025-10-15 8:32 ` Charan Teja Kalla
2025-10-19 12:13 ` Dmitry Baryshkov
1 sibling, 1 reply; 36+ messages in thread
From: Charan Teja Kalla @ 2025-10-15 8:32 UTC (permalink / raw)
To: Robin Murphy, Dmitry Baryshkov
Cc: joro, will, saravanak, conor+dt, robh, mchehab, bod, krzk+dt,
abhinav.kumar, vikash.garodia, dikshita.agarwal, Konrad Dybcio,
bjorn.andersson, linux-media, linux-arm-msm, devicetree,
linux-kernel, iommu
On 10/14/2025 7:37 PM, Robin Murphy wrote:
> On 2025-10-13 1:31 pm, Dmitry Baryshkov wrote:
>> On Mon, Oct 13, 2025 at 12:20:54PM +0100, Robin Murphy wrote:
>>> On 2025-10-09 7:25 pm, Dmitry Baryshkov wrote:
>>>> On Thu, Oct 09, 2025 at 06:03:29PM +0100, Robin Murphy wrote:
>>>>> On 2025-10-09 2:19 pm, Dmitry Baryshkov wrote:
>>>>>> On Thu, Oct 09, 2025 at 11:46:55AM +0100, Robin Murphy wrote:
>>>>>>> On 2025-10-08 8:10 pm, Charan Teja Kalla wrote:
>>>>>>>>
>>>>>>>> On 9/29/2025 3:50 PM, Robin Murphy wrote:
>>>>>>>>>> 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 sub blocks are
>>>>>>>>>> __configured__ to
>>>>>>>>>> generate different stream IDs.
>>>>>>>>>
>>>>>>>>> So please clarify why you can't:
>>>>>>>>>
>>>>>>>>> a) Describe the sub-blocks as individual child nodes each with
>>>>>>>>> their own
>>>>>>>>> distinct "iommus" property
>>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks Robin for your time. Sorry for late reply as I really
>>>>>>>> didn't have
>>>>>>>> concrete answer for this question.
>>>>>>>>
>>>>>>>> First let me clarify the word "sub blocks" -- This is just the
>>>>>>>> logical
>>>>>>>> separation with no separate address space to really able to
>>>>>>>> define them
>>>>>>>> as sub devices. Think of it like a single video IP with 2 dma
>>>>>>>> engines(used for pixel and non-pixel purpose).
>>>>>>>>
>>>>>>>> I should agree that the child-nodes in the device tree is the
>>>>>>>> easy one
>>>>>>>> and infact, it is how being used in downstream.
>>>>>>>>
>>>>>>>> For upstream -- Since there is no real address space to interact
>>>>>>>> with
>>>>>>>> these sub-blocks(or logical blocks), does it really qualify to
>>>>>>>> define as
>>>>>>>> child nodes in the device tree? I see there is some push back[1].
>>>>>>>
>>>>>>> Who says you need an address space? Child nodes without "reg"
>>>>>>> properties,
>>>>>>> referenced by name, compatible or phandle, exist all over the
>>>>>>> place for all
>>>>>>> manner of reasons. If there are distinct logical functions with
>>>>>>> their own
>>>>>>> distinct hardware properties, then I would say having child nodes to
>>>>>>> describe and associate those properties with their respective
>>>>>>> functions is
>>>>>>> entirely natural and appropriate. The first example that comes to
>>>>>>> mind of
>>>>>>> where this is a well-established practice is PMICs - to pick one
>>>>>>> at random:
>>>>>>> Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml
>>>>>>
>>>>>> Logical function, that's correct. And also note, for PMICs that
>>>>>> practice
>>>>>> has bitten us back. For PM8008 we switched back to a non-subdevice
>>>>>> representation.
>>>>>>
>>>>>>> For bonus irony, you can't take the other approaches without
>>>>>>> inherently
>>>>>>> *introducing* a notional address space in the form of your
>>>>>>> logical function
>>>>>>> IDs anyway.
>>>>>>>
>>>>>>>> > or:
>>>>>>>>>
>>>>>>>>> b) Use standard "iommu-map" which already supports mapping a
>>>>>>>>> masked
>>>>>>>>> input ID to an arbitrary IOMMU specifier
>>>>>>>>>
>>>>>>>>
>>>>>>>> I think clients is also required to program non-zero smr mask,
>>>>>>>> where as
>>>>>>>> iommu-map just maps the id to an IOMMU specifier(sid). Please
>>>>>>>> LMK if I
>>>>>>>> am unable to catch your thought here.
>>>>>>> An IOMMU specifier is whatever the target IOMMU node's #iommu-
>>>>>>> cells says it
>>>>>>> is. The fact that Linux's parsing code only works properly for
>>>>>>> #iommu-cells
>>>>>>> = 1 is not really a DT binding problem (other than it stemming
>>>>>>> from a loose
>>>>>>> assumption stated in the PCI binding's use of the property).
>>>>>>
>>>>>> I really don't like the idea of extending the #iommu-cells. The
>>>>>> ARM SMMU
>>>>>> has only one cell, which is correct even for our platforms. The fact
>>>>>> that we need to identify different IOMMU SIDs (and handle them in a
>>>>>> differnt ways) is internal to the video device (and several other
>>>>>> devices). There is nothing to be handled on the ARM SMMU side.
>>>>>
>>>>> Huh? So if you prefer not to change anything, are you suggesting
>>>>> this series
>>>>> doesn't need to exist at all? Now I'm thoroughly confused...
>>>>
>>>> Hmm. We need changes, but I don't feel like adding the FUNCTION_ID to
>>>> #iommu-cells is the best idea.
>>>
>>> What? No, any function ID would be an *input* to a map, not part of the
>>> output specifier; indeed it should never go anywhere near the IOMMU,
>>> I don't
>>> think anyone suggested that.
>>
>> It was Bryan, https://lore.kernel.org/linux-arm-
>> msm/9bae595a-597e-46e6-8eb2-44424fe21db6@linaro.org
>
> Ah, I wasn't on that thread. But indeed, as I hopefully explained
> before, that whole idea is a non-starter anyway due to who the consumers
> of "iommus" actually are.
>
>>>>> If you want to use SMR masks, then you absolutely need #iommu-cells
>>>>> = 2,
>>>>> because that is the SMMU binding for using SMR masks. It would
>>>>> definitely
>>>>
>>>> I'm sorry. Yes, we have #iommu-cells = <2>.
>>>>
>>>>> not be OK to have some magic property trying to smuggle
>>>>> IOMMU-driver-specific data contrary to what the IOMMU node itself
>>>>> says. As
>>>>> for iommu-map, I don't see what would be objectionable about
>>>>> improving the
>>>>> parsing to respect a real #iommu-cells value rather than hard-
>>>>> coding an
>>>>> assumption. Yes, we'd probably need to forbid entries with length > 1
>>>>> targeting IOMMUs with #iommu-cells > 1, since the notion of a linear
>>>>
>>>> This will break e.g. PCIe on Qualcomm platforms:
>>>>
>>>> iommu-map = <0x0 &apps_smmu 0x1400 0x1>,
>>>> <0x100 &apps_smmu 0x1401 0x1>;
>>>>
>>>>
>>>> But this seems unlogical anyway wrt. apps_smmu having #iommu-cells =
>>>> <2>. It depends on ARM SMMU ignoring the second cell when it's not
>>>> present.
>>>
>>> Urgh, yes, that's just broken already 🙁
>>>
>>> At least they all seem to be a sufficiently consistent pattern that a
>>> targeted workaround to detect old DTBs looks feasible (I'm thinking, if
>>> iommu-map size % 4 == 0 and cells n*4 + 3 are all 1 and cells n*4 + 1
>>> are
>>> all the same phandle to an IOMMU with #iommu-cells == 2, then parse
>>> as if
>>> #iommu-cells == 1)
>>
>> How do we handle the case of #iommu-cells = <2>? I.e. what should be the
>> "fixed" representation of the map above? Should we have usual cells and
>> one extra "length" just for the sake of it?
>
> It's not really "for the sake of it", it is the defined format of the
> "iommu-map" binding - IMO it would be far more horrible if each entry
> did or didn't include a length cell depending on the size of the
> preceding IOMMU specifier. It's also far from infeasible to have *some*
> well-defined relationship between a non-singular input ID range and a
> multi-cell base IOMMU specifier, it just needs more IOMMU-specific
> interpretation in the consumer than Linux cares to bother with. Thus it
> is appropriate for the binding to be able to describe that even though
> Linux as a consumer continues to refuse to support it. The binding does
> not describe Linux, or the property would be named "linux,iommu-map".
>
>> iommu-map = <0x0 &apps_smmu 0x1400 0x0 0x1>,
>> <0x100 &apps_smmu 0x1401 0x0 0x1>;
>>
Dmitry, Wanted to understand if you did consider to include additional
#iommu-map-cells in the above representation? or it is just based on the
#iommu-cells?
And this is the same representation for PCI devices as well or it is
still parsed as if #iommu-cells = 1 just for old dtbs based on the
workaround by Robin above?
so, it will be like:
iommu-map = <rid/func-id phandler sid_base <mask> len>; and if mask is
not defined, treat this as if #iommu-cell = 1.
>>
>> I really like the idea of fixing iommu-map as that would remove the need
>> for other properties, but
>>
>>>
>>>>> relationship between the input ID and the output specifier falls
>>>>> apart when
>>>>> the specifier is complex, but that seems simple enough to implement
>>>>> and
>>>>> document (even if it's too fiddly to describe in the schema
>>>>> itself), and
>>>>> still certainly no worse than having another property that *is* just
>>>>> iommu-map with implicit length = 1.
>>>>>
>>>>> And if you want individual StreamIDs for logical functions to be
>>>>> attachable
>>>>> to distinct contexts then those functions absolutely must be
>>>>> visible to the
>>>>> IOMMU layer and the SMMU driver as independent devices with their
>>>>> own unique
>>>>> properties, which means either they come that way from the DT as
>>>>> of_platform
>>>>> devices in the first place, or you implement a full bus_type
>>>>> abstraction
>>>>
>>>> Not necessarily. Tegra display driver creates a device for each context
>>>> on its own.
>>> No, the *display* driver does not; the host1x bus driver does, which
>>> is the
>>> point I was making - that has a proper bus abstraction tied into the
>>> IOMMU
>>> layer, such that the devices are correctly configured long before the
>>> actual
>>> DRM driver(s) get anywhere near them.
>>
>> Ack. I agree. it's drivers/gpu/host1x/context, not drivers/gpu/drm/
>>
>>>
>>>> In fact, using OF to create context devices is _less_
>>>> robust, because now the driver needs to sync, checking that there is a
>>>> subdevice, that it has probed, etc. Using manually created devices
>>>> seems
>>>> better from my POV.
>>>
>>> Huh? A simple call to of_platform_populate() is somehow less robust than
>>> open-coding much of the same logic that of_platform_populate() does
>>> plus a
>>> bunch of hackery to try to fake up an of_node to make the new device
>>> appear
>>> to own the appropriate properties?
>>>
>>> Having entire sub-*drivers* for child devices or not is an orthogonal
>>> issue
>>> regardless of whichever way they are created.
>>
>> I was (again) looking at host1x. It doesn't fake of_node (nor does it
>> have actual OF nodes). Instead it just mapps IOMMUs directly to the
>> context devices. Compare this to misc/fastrpc.c, which has subdevices
>> and drivers to map contexts. The latter one looks less robust.
>>
>> And from DT perspective compare:
>>
>> fastrpc {
>> compatible = "qcom,fastrpc";
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> compute-cb@3 {
>> compatible = "qcom,fastrpc-compute-cb";
>> reg = <3>;
>> iommus = <&apps_smmu 0x1803 0x0>;
>> };
>>
>> compute-cb@4 {
>> compatible = "qcom,fastrpc-compute-cb";
>> reg = <4>;
>> iommus = <&apps_smmu 0x1804 0x0>;
>> };
>>
>> compute-cb@5 {
>> compatible = "qcom,fastrpc-compute-cb";
>> reg = <5>;
>> iommus = <&apps_smmu 0x1805 0x0>;
>> };
>> };
>>
>> VS (note, it doesn't have 'length', it can be added back with no issues):
>>
>> fastrpc {
>> compatible = "qcom,fastrpc";
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> iommu-map = <3 &apps_smmu 0x1803 0x0>,
>> <4 &apps_smmu 0x1804 0x0>,
>> <5 &apps_smmu 0x1805 0x0>;
>> };
>>
>>
>> I think the latter is more compact, and more robust.
>
> For that particular case I concur that iommu-map might fit just as well,
> since it appears similar to the Tegra one - essentially just a pool of
> identical hardware contexts with no special individual properties, whose
> purpose is defined by the software using them (be that the driver
> itself, or the firmware on the other end). IOW, the DT really isn't
> describing anything more than a mapping between a context ID and an
> IOMMU specifier either way.
>
> That said I also see nothing immediately wrong with the fastrpc driver
> as-is either; if anything it looks like a pretty ideal example of the
> "self-contained" non-bus approach I was alluding to. The "fake of_node"
> notion only applies to the idea of trying to keep that same driver
> structure but just replace of_platform_populate() with conjuring
> platform_devices out of thin air.
>> Note, to make a complete example, it should be probably something like
>> (sc7280, cdsp, note duplicate IDs in the map, again, I omitted length):
>>
>> fastrpc {
>> compatible = "qcom,fastrpc";
>>
>> iommu-map = <1 &apps_smmu 0x11a1 0x0420>,
>> <1 &apps_smmu 0x1181 0x0420>,
>> <2 &apps_smmu 0x11a2 0x0420>,
>> <2 &apps_smmu 0x1182 0x0420>,
>> <3 &apps_smmu 0x11a3 0x0420>,
>> <3 &apps_smmu 0x1183 0x0420>;
>
> Note that as another orthogonal issue, Linux also doesn't support 1:many
> maps like that - we'll only parse the first matching entry. However this
> specific example (and the current DTs) doesn't make sense anyway, since
> each pair of SMRs encodes the same set of matches (0x118x, 0x11ax,
> 0x158x, 0x15ax), so at best it's redundant while at worst it's a stream
> match conflict fault waiting to happen?
>
>> dma-coherent;
>> };
>>
>>
>>>>> which will have to be hooked up to the IOMMU layer. You cannot make
>>>>> IOMMU
>>>>> configuration "internal" to the actual client driver which is only
>>>>> allowed
>>>>> to bind *after* said IOMMU configuration has already been made.
>>>>
>>>> I'm not sure I follow this, I'm sorry.
>>> I mean IOMMU configuration is designed to happen at device_add()
>>> time, and
>>> client drivers must not assume otherwise (the mechanisms for handling
>>> IOMMU
>>> drivers registering "late" from modules are internal details that can
>>> and
>>> will change). If you're under the impression that a straightforward
>>> platform
>>> driver for the video codec itself would be able to invoke IOMMU
>>> configuration for the video codec platform device (without unacceptable
>>> levels of hackery) then you are mistaken, sorry.
>>>
>>> Again, to be able to assign StreamIDs to different contexts, those
>>> StreamIDs
>>> must uniquely belong to different struct devices. Thus in terms of
>>> how you
>>> get to those struct devices from a DT representation, either they
>>> come from
>>> distinct DT nodes with standard "iommus" properties that the generic
>>> of_platform code can create and configure accordingly, or you're doing a
>>> non-trivial amount of work to implement your own bus layer like
>>> host1x_context_bus to manage your own type of sub-device. There is no
>>> valid
>>> middle ground of trying to stuff driver-specific knowledge of
>>> arbitrarily
>>> made-up function IDs into the generic platform bus code.
>>
>>
>> I'd totally prefer something like:
>>
>> video-codec@foobar {
>> compatible = "qcom,video";
>>
>> iommus = <&apps_smmu 0x1234 0xca>;
>> iommu-maps = <PIXEL &apps_smmu 0xabcdef 0xac>,
>> <SECURE_PIXEL &apps_smmu 0x898989 0xac>,
>> <SECURE_BITSTREAM &apps_smmu 0x898998 0xac>;
>> };
> This is where I maintain a differing opinion - if it's *not* a "pool of
> identical contexts" case, but a single nominal hardware block with a
> small number of distinct DMA streams for fundamentally different
> purposes defined by the hardware design, then I would usually consider
> it more natural, honest and useful to make those differences explicit by
> name/compatible with child nodes, rather than hide them behind an opaque
> arbitrary integer. If by nature of being functionally different they
> also might require individual properties - such as memory-regions - then
> child nodes are the only option anyway.
>
> However, if there is actually some meaningful hardware notion of
> "function ID", the design/usage model is such that it would generally be
> logical for a consumer driver to be structured as managing a set of
> fixed-function sub-devices on an internal bus, and you're absolutely
> definite that those sub-devices will never ever need any DT properties
> of their own in future revisions/integrations, then maybe an "iommu-
> map"-based binding is OK. All I can say for sure is that describing
> complex hardware well is very nuanced and there is no one universal
> right answer.
>
> Thanks,
> Robin.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-14 22:18 ` Dmitry Baryshkov
@ 2025-10-15 8:53 ` Bryan O'Donoghue
2025-10-15 21:55 ` Rob Herring
0 siblings, 1 reply; 36+ messages in thread
From: Bryan O'Donoghue @ 2025-10-15 8:53 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Robin Murphy, Charan Teja Kalla, joro, will, saravanak, conor+dt,
robh, mchehab, krzk+dt, abhinav.kumar, vikash.garodia,
dikshita.agarwal, Konrad Dybcio, bjorn.andersson, linux-media,
linux-arm-msm, devicetree, linux-kernel, iommu
On 14/10/2025 23:18, Dmitry Baryshkov wrote:
> On Tue, Oct 14, 2025 at 09:49:17PM +0100, Bryan O'Donoghue wrote:
>> On 14/10/2025 19:35, Dmitry Baryshkov wrote:
>>>> Each function id can be associated with a device and a compat string
>>>> associated with it.
>>> So, which part of the hardware is described by the -cb device? What does
>>> it mean_here_?
>>
>> The non-pixel path video encoder, the tz video encoder...
>>
>> What's not clear about that ?
>
> Where do you have pixel encoders in the fastrpc device node?
>
> --
> With best wishes
> Dmitry
Haha, no sorry I didn't mean to suggest that at all.
I mean do something _like_ that, for these FUNCION_IDs.
We could replicate that for a new iris add for say Glymur or Kanaapali.
Sub-nodes of the main iris device. They have a real purpose in that the
'device' requirement is full range IOVA for the SID and implicit
identification of the FUNCTION_ID with the compat string
iris-video@0xdeadbeef {
video@0 {
reg = <0>; /* FUNCTION_ID HLOS could also go here */
compat = "qcom,glymur-iris";
iommus = <&apps_smmu 0x1940 0x0000>;
};
video@1 {
reg = <1>;
compat = "qcom,glymur-iris-non-pixel";
iommus = <&apps_smmu 0x1947 0x0000>;
};
};
The reg property could also be the function_id
video@FUNC_ID_HLOS {
reg = <FUNC_ID_HLOS>;
...
};
There's no need for a new iommu specific property to help us fixup
sm8550 iommu definition.
As I say if that error wasn't already in sm8550, we wouldn't be trying
to solve the problem this way.
So lets solve the problem for Glymur and Kanaapali and then backport
upstream if we can or downstream if we can't.
What we need are new devices what we will do with the data in
iommu-map-masked is make new devices. We are mapping data - iommu SID to
device and implicit FUNCTION_ID to a device.
So we should be declaring devices, instead of burying the data in a new
property that is not obvious what it does or why it exists.
Robin has suggested devices, we have two real bits of hardware data to
go with those devices...
---
bod
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-15 8:53 ` Bryan O'Donoghue
@ 2025-10-15 21:55 ` Rob Herring
0 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2025-10-15 21:55 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: Dmitry Baryshkov, Robin Murphy, Charan Teja Kalla, joro, will,
saravanak, conor+dt, mchehab, krzk+dt, abhinav.kumar,
vikash.garodia, dikshita.agarwal, Konrad Dybcio, bjorn.andersson,
linux-media, linux-arm-msm, devicetree, linux-kernel, iommu
On Wed, Oct 15, 2025 at 3:53 AM Bryan O'Donoghue <bod@kernel.org> wrote:
>
> On 14/10/2025 23:18, Dmitry Baryshkov wrote:
> > On Tue, Oct 14, 2025 at 09:49:17PM +0100, Bryan O'Donoghue wrote:
> >> On 14/10/2025 19:35, Dmitry Baryshkov wrote:
> >>>> Each function id can be associated with a device and a compat string
> >>>> associated with it.
> >>> So, which part of the hardware is described by the -cb device? What does
> >>> it mean_here_?
> >>
> >> The non-pixel path video encoder, the tz video encoder...
> >>
> >> What's not clear about that ?
> >
> > Where do you have pixel encoders in the fastrpc device node?
> >
> > --
> > With best wishes
> > Dmitry
>
> Haha, no sorry I didn't mean to suggest that at all.
>
> I mean do something _like_ that, for these FUNCION_IDs.
>
> We could replicate that for a new iris add for say Glymur or Kanaapali.
>
> Sub-nodes of the main iris device. They have a real purpose in that the
> 'device' requirement is full range IOVA for the SID and implicit
> identification of the FUNCTION_ID with the compat string
>
> iris-video@0xdeadbeef {
> video@0 {
> reg = <0>; /* FUNCTION_ID HLOS could also go here */
> compat = "qcom,glymur-iris";
>
> iommus = <&apps_smmu 0x1940 0x0000>;
> };
>
> video@1 {
> reg = <1>;
> compat = "qcom,glymur-iris-non-pixel";
> iommus = <&apps_smmu 0x1947 0x0000>;
> };
> };
>
> The reg property could also be the function_id
>
> video@FUNC_ID_HLOS {
> reg = <FUNC_ID_HLOS>;
> ...
> };
>
> There's no need for a new iommu specific property to help us fixup
> sm8550 iommu definition.
>
> As I say if that error wasn't already in sm8550, we wouldn't be trying
> to solve the problem this way.
>
> So lets solve the problem for Glymur and Kanaapali and then backport
> upstream if we can or downstream if we can't.
>
> What we need are new devices what we will do with the data in
> iommu-map-masked is make new devices. We are mapping data - iommu SID to
> device and implicit FUNCTION_ID to a device.
>
> So we should be declaring devices, instead of burying the data in a new
> property that is not obvious what it does or why it exists.
No, these aren't separate devices. Please stop going down this route.
Rob
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices
2025-10-15 8:32 ` Charan Teja Kalla
@ 2025-10-19 12:13 ` Dmitry Baryshkov
0 siblings, 0 replies; 36+ messages in thread
From: Dmitry Baryshkov @ 2025-10-19 12:13 UTC (permalink / raw)
To: Charan Teja Kalla
Cc: Robin Murphy, joro, will, saravanak, conor+dt, robh, mchehab, bod,
krzk+dt, abhinav.kumar, vikash.garodia, dikshita.agarwal,
Konrad Dybcio, bjorn.andersson, linux-media, linux-arm-msm,
devicetree, linux-kernel, iommu
On Wed, Oct 15, 2025 at 02:02:10PM +0530, Charan Teja Kalla wrote:
>
>
> On 10/14/2025 7:37 PM, Robin Murphy wrote:
> > On 2025-10-13 1:31 pm, Dmitry Baryshkov wrote:
> >> On Mon, Oct 13, 2025 at 12:20:54PM +0100, Robin Murphy wrote:
> >>> On 2025-10-09 7:25 pm, Dmitry Baryshkov wrote:
> >>>> On Thu, Oct 09, 2025 at 06:03:29PM +0100, Robin Murphy wrote:
> >>>>> On 2025-10-09 2:19 pm, Dmitry Baryshkov wrote:
> >>>>>> On Thu, Oct 09, 2025 at 11:46:55AM +0100, Robin Murphy wrote:
> >>>>>>> On 2025-10-08 8:10 pm, Charan Teja Kalla wrote:
> >>>>>>>>
> >>>>>>>> On 9/29/2025 3:50 PM, Robin Murphy wrote:
> >>>>>>>>>> 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 sub blocks are
> >>>>>>>>>> __configured__ to
> >>>>>>>>>> generate different stream IDs.
> >>>>>>>>>
> >>>>>>>>> So please clarify why you can't:
> >>>>>>>>>
> >>>>>>>>> a) Describe the sub-blocks as individual child nodes each with
> >>>>>>>>> their own
> >>>>>>>>> distinct "iommus" property
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> Thanks Robin for your time. Sorry for late reply as I really
> >>>>>>>> didn't have
> >>>>>>>> concrete answer for this question.
> >>>>>>>>
> >>>>>>>> First let me clarify the word "sub blocks" -- This is just the
> >>>>>>>> logical
> >>>>>>>> separation with no separate address space to really able to
> >>>>>>>> define them
> >>>>>>>> as sub devices. Think of it like a single video IP with 2 dma
> >>>>>>>> engines(used for pixel and non-pixel purpose).
> >>>>>>>>
> >>>>>>>> I should agree that the child-nodes in the device tree is the
> >>>>>>>> easy one
> >>>>>>>> and infact, it is how being used in downstream.
> >>>>>>>>
> >>>>>>>> For upstream -- Since there is no real address space to interact
> >>>>>>>> with
> >>>>>>>> these sub-blocks(or logical blocks), does it really qualify to
> >>>>>>>> define as
> >>>>>>>> child nodes in the device tree? I see there is some push back[1].
> >>>>>>>
> >>>>>>> Who says you need an address space? Child nodes without "reg"
> >>>>>>> properties,
> >>>>>>> referenced by name, compatible or phandle, exist all over the
> >>>>>>> place for all
> >>>>>>> manner of reasons. If there are distinct logical functions with
> >>>>>>> their own
> >>>>>>> distinct hardware properties, then I would say having child nodes to
> >>>>>>> describe and associate those properties with their respective
> >>>>>>> functions is
> >>>>>>> entirely natural and appropriate. The first example that comes to
> >>>>>>> mind of
> >>>>>>> where this is a well-established practice is PMICs - to pick one
> >>>>>>> at random:
> >>>>>>> Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml
> >>>>>>
> >>>>>> Logical function, that's correct. And also note, for PMICs that
> >>>>>> practice
> >>>>>> has bitten us back. For PM8008 we switched back to a non-subdevice
> >>>>>> representation.
> >>>>>>
> >>>>>>> For bonus irony, you can't take the other approaches without
> >>>>>>> inherently
> >>>>>>> *introducing* a notional address space in the form of your
> >>>>>>> logical function
> >>>>>>> IDs anyway.
> >>>>>>>
> >>>>>>>> > or:
> >>>>>>>>>
> >>>>>>>>> b) Use standard "iommu-map" which already supports mapping a
> >>>>>>>>> masked
> >>>>>>>>> input ID to an arbitrary IOMMU specifier
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> I think clients is also required to program non-zero smr mask,
> >>>>>>>> where as
> >>>>>>>> iommu-map just maps the id to an IOMMU specifier(sid). Please
> >>>>>>>> LMK if I
> >>>>>>>> am unable to catch your thought here.
> >>>>>>> An IOMMU specifier is whatever the target IOMMU node's #iommu-
> >>>>>>> cells says it
> >>>>>>> is. The fact that Linux's parsing code only works properly for
> >>>>>>> #iommu-cells
> >>>>>>> = 1 is not really a DT binding problem (other than it stemming
> >>>>>>> from a loose
> >>>>>>> assumption stated in the PCI binding's use of the property).
> >>>>>>
> >>>>>> I really don't like the idea of extending the #iommu-cells. The
> >>>>>> ARM SMMU
> >>>>>> has only one cell, which is correct even for our platforms. The fact
> >>>>>> that we need to identify different IOMMU SIDs (and handle them in a
> >>>>>> differnt ways) is internal to the video device (and several other
> >>>>>> devices). There is nothing to be handled on the ARM SMMU side.
> >>>>>
> >>>>> Huh? So if you prefer not to change anything, are you suggesting
> >>>>> this series
> >>>>> doesn't need to exist at all? Now I'm thoroughly confused...
> >>>>
> >>>> Hmm. We need changes, but I don't feel like adding the FUNCTION_ID to
> >>>> #iommu-cells is the best idea.
> >>>
> >>> What? No, any function ID would be an *input* to a map, not part of the
> >>> output specifier; indeed it should never go anywhere near the IOMMU,
> >>> I don't
> >>> think anyone suggested that.
> >>
> >> It was Bryan, https://lore.kernel.org/linux-arm-
> >> msm/9bae595a-597e-46e6-8eb2-44424fe21db6@linaro.org
> >
> > Ah, I wasn't on that thread. But indeed, as I hopefully explained
> > before, that whole idea is a non-starter anyway due to who the consumers
> > of "iommus" actually are.
> >
> >>>>> If you want to use SMR masks, then you absolutely need #iommu-cells
> >>>>> = 2,
> >>>>> because that is the SMMU binding for using SMR masks. It would
> >>>>> definitely
> >>>>
> >>>> I'm sorry. Yes, we have #iommu-cells = <2>.
> >>>>
> >>>>> not be OK to have some magic property trying to smuggle
> >>>>> IOMMU-driver-specific data contrary to what the IOMMU node itself
> >>>>> says. As
> >>>>> for iommu-map, I don't see what would be objectionable about
> >>>>> improving the
> >>>>> parsing to respect a real #iommu-cells value rather than hard-
> >>>>> coding an
> >>>>> assumption. Yes, we'd probably need to forbid entries with length > 1
> >>>>> targeting IOMMUs with #iommu-cells > 1, since the notion of a linear
> >>>>
> >>>> This will break e.g. PCIe on Qualcomm platforms:
> >>>>
> >>>> iommu-map = <0x0 &apps_smmu 0x1400 0x1>,
> >>>> <0x100 &apps_smmu 0x1401 0x1>;
> >>>>
> >>>>
> >>>> But this seems unlogical anyway wrt. apps_smmu having #iommu-cells =
> >>>> <2>. It depends on ARM SMMU ignoring the second cell when it's not
> >>>> present.
> >>>
> >>> Urgh, yes, that's just broken already 🙁
> >>>
> >>> At least they all seem to be a sufficiently consistent pattern that a
> >>> targeted workaround to detect old DTBs looks feasible (I'm thinking, if
> >>> iommu-map size % 4 == 0 and cells n*4 + 3 are all 1 and cells n*4 + 1
> >>> are
> >>> all the same phandle to an IOMMU with #iommu-cells == 2, then parse
> >>> as if
> >>> #iommu-cells == 1)
> >>
> >> How do we handle the case of #iommu-cells = <2>? I.e. what should be the
> >> "fixed" representation of the map above? Should we have usual cells and
> >> one extra "length" just for the sake of it?
> >
> > It's not really "for the sake of it", it is the defined format of the
> > "iommu-map" binding - IMO it would be far more horrible if each entry
> > did or didn't include a length cell depending on the size of the
> > preceding IOMMU specifier. It's also far from infeasible to have *some*
> > well-defined relationship between a non-singular input ID range and a
> > multi-cell base IOMMU specifier, it just needs more IOMMU-specific
> > interpretation in the consumer than Linux cares to bother with. Thus it
> > is appropriate for the binding to be able to describe that even though
> > Linux as a consumer continues to refuse to support it. The binding does
> > not describe Linux, or the property would be named "linux,iommu-map".
> >
> >> iommu-map = <0x0 &apps_smmu 0x1400 0x0 0x1>,
> >> <0x100 &apps_smmu 0x1401 0x0 0x1>;
> >>
> Dmitry, Wanted to understand if you did consider to include additional
> #iommu-map-cells in the above representation? or it is just based on the
> #iommu-cells?
Just #iommu-cells, as suggested by Robin.
>
> And this is the same representation for PCI devices as well or it is
> still parsed as if #iommu-cells = 1 just for old dtbs based on the
> workaround by Robin above?
>
> so, it will be like:
> iommu-map = <rid/func-id phandler sid_base <mask> len>; and if mask is
> not defined, treat this as if #iommu-cell = 1.
Yes.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2025-10-19 12:13 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-28 17:17 [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices Charan Teja Kalla
2025-09-28 17:17 ` [RFC PATCH 1/3] dtbindings: add binding for iommu-map-masked property Charan Teja Kalla
2025-09-28 17:17 ` [RFC PATCH 2/3] of: create a wrapper for of_map_id() Charan Teja Kalla
2025-09-28 17:17 ` [RFC PATCH 3/3] of: implment the 'iommu-map-masked' to represent multi-functional devices Charan Teja Kalla
2025-09-28 20:23 ` [RFC PATCH 0/3] Introduce iommu-map-masked for platform devices Rob Herring
2025-10-09 0:26 ` Krzysztof Kozlowski
2025-10-09 12:16 ` Robin Murphy
2025-10-09 13:16 ` Dmitry Baryshkov
2025-10-09 13:14 ` Dmitry Baryshkov
2025-10-09 3:05 ` Vikash Garodia
2025-09-29 10:20 ` Robin Murphy
2025-10-08 19:10 ` Charan Teja Kalla
2025-10-09 10:46 ` Robin Murphy
2025-10-09 13:19 ` Dmitry Baryshkov
2025-10-09 17:03 ` Robin Murphy
2025-10-09 18:25 ` Dmitry Baryshkov
2025-10-10 19:53 ` Charan Teja Kalla
2025-10-10 22:30 ` Rob Herring
2025-10-11 0:54 ` Dmitry Baryshkov
2025-10-12 20:44 ` Bryan O'Donoghue
2025-10-12 21:57 ` Bryan O'Donoghue
2025-10-12 22:47 ` Dmitry Baryshkov
2025-10-13 10:18 ` Bryan O'Donoghue
2025-10-13 11:20 ` Robin Murphy
2025-10-13 12:31 ` Dmitry Baryshkov
2025-10-14 14:07 ` Robin Murphy
2025-10-14 18:33 ` Dmitry Baryshkov
2025-10-15 8:32 ` Charan Teja Kalla
2025-10-19 12:13 ` Dmitry Baryshkov
2025-10-14 15:07 ` Bryan O'Donoghue
2025-10-14 18:35 ` Dmitry Baryshkov
2025-10-14 20:49 ` Bryan O'Donoghue
2025-10-14 22:18 ` Dmitry Baryshkov
2025-10-15 8:53 ` Bryan O'Donoghue
2025-10-15 21:55 ` Rob Herring
2025-10-14 22:38 ` Dmitry Baryshkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).