| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e6fb7000-7aac-45b6-b4f9-c9efa2a98d57@oss.qualcomm.com>
Date: Wed, 12 Nov 2025 20:12:28 +0530
From: Charan Teja Kalla <charan.kalla@....qualcomm.com>
To: Robin Murphy <robin.murphy@....com>, will@...nel.org, joro@...tes.org,
robh@...nel.org, dmitry.baryshkov@....qualcomm.com,
konrad.dybcio@....qualcomm.com, bjorn.andersson@....qualcomm.com,
bod@...nel.org, conor+dt@...nel.org, krzk+dt@...nel.org,
saravanak@...gle.com, prakash.gupta@....qualcomm.com,
vikash.garodia@....qualcomm.com
Cc: iommu@...ts.linux.dev, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH 0/6] of: iommu-map parsing for multi-cell IOMMU
On 11/11/2025 11:57 PM, Charan Teja Kalla wrote:
>
> On 11/5/2025 10:58 PM, Robin Murphy wrote:
>>> The other motivation for this patchset is the below usecase.
>>> USECASE [1]:
>>> ------------
>>> Video IP, 32bit, have 2 hardware sub blocks(or can be called as
>>> functions) called as pixel and nonpixel blocks, that does decode and
>>> encode of the video stream. These logical blocks are configured to
>>> generate different stream IDs.
>>>
>>> With the classical approach of representing all sids with iommus= end up
>>> in using a single translation context limited to the 4GB. There are
>>> video usecases which needs larger IOVA space, like higher concurrent
>>> video sessions(eg: 32 session and 192MB per session) where 4GB of IOVA
>>> is not sufficient.
>>>
>>> For this case, each functionality is represented in the firmware(device
>>> tree) by the 'rid' field of the iommu-map property and the video driver
>>> creates sub platform devices for each of this functionality and call
>>> into IOMMU configuration. Each rid(function id) in the dt property
>>> indicates the bit that can be associated by the driver passed input id.
>>>
>>> Example:
>>> iommu {
>>> #iommu-cells = 2;
>>> };
>>>
>>> video-codec@...bar {
>>> compatible = "qcom,video";
>>> iommus = <&apps_smmu 0x1234 0xca>;
>>> iommu-map= <0x1 &iommu 0x1940 0x0 0x1>,
>>> <0x1 &iommu 0x1941 0x0 0x1>,
>>> <0x2 &iommu 0x1942 0x0 0x1>,
>>> <0x4 &iommu 0x1943 0x0 0x1>,
>>> <0x4 &iommu 0x1944 0x0 0x1>;
>>> };
>>>
>>> video-driver:
>>> #define PIXEL_FUNC (1)
>>> #define NON_PIXEL_FUNC (2)
>>> #define SECURE_FUNC (4)
>>>
>>> case1: All these functionalities requires individual contexts.
>>> Create 3 subdevices for each of this function and call
>>> of_dma_configure_id(..,id), id = 0x1, 0x2, 0x4.
>>>
>>> Case2: Secure and non-secure functionalities require individual
>>> contexts. Create 2 subdevices and call of_dma_configure_id(..,id), id =
>>> 0x3(bitmap of pixel and non-pixel), 0x4 (secure).
>>>
>>> Credits: to Dmitry for thorough discussions on the RFC patch and major
>>> help in getting the consenus on this approach, to Konrad & Bjorn for
>>> offline discussions and reviews, to Robin for his inputs on IOMMU front,
>>> to Bod, Rob and Krzysztof for all valuable inputs.
>>>
>>> [1] https://lore.kernel.org/all/20250627-video_cb-
>>> v3-0-51e18c0ffbce@...cinc.com/
>>> [2] https://lore.kernel.org/all/20250928171718.436440-1-
>>> charan.kalla@....qualcomm.com/#r
>>>
>>> Charan Teja Kalla (6):
>>> of: create a wrapper for of_map_id()
>>> of: introduce wrapper function to query the cell count
>>> of: parse #<name>-cells property to get the cell count
>>> of: detect and handle legacy iommu-map parsing
>>> of: add infra to parse iommu-map per IOMMU cell count
>>> of: use correct iommu-map parsing logic from of_iommu layer
>>>
>>> drivers/iommu/of_iommu.c | 59 +++++++--
>>> drivers/of/base.c | 269 +++++++++++++++++++++++++++++++++++----
>>> include/linux/of.h | 19 +++
>>> 3 files changed, 314 insertions(+), 33 deletions(-)
>> Hmm, I did actually have a quick go at this the other week too, and
>> while I though it was a bit clunky, it was still significantly simpler
>> than this seems to be...
>>
>> FWIW: https://gitlab.arm.com/linux-arm/linux-rm/-/commits/iommu-map - I
> Thanks a lot Robin for taking a look and sorry for the delayed reply as
> I was on vacation.
>
> stripped code_snippet from your patch:
> offset = 0;
> out_base = map + offset + 2;
> id_off = masked_id - id_base;
> if (masked_id < id_base || id_off >= id_len)
> continue;
> for (int i = 0; id_out && i < cells; i++)
> id_out[i] = id_off + be32_to_cpu(out_base[i]);
>
>
> seems way cleaner than mine...
>
> Actually, we also have a case of a device emitting 2 distinct
> identifiers, eg: a device is emitting 0x1940, 0x1944 and 0x1A20 sids and
> attached to a single context bank. If I use mask to cover all these sids
> in a single iommu-map entry, it does overlap with other device SID.
>
> I don't think that patch you shared can be used to cover the above, or
> it is?
>
> Hence I resorted to the approach where RID used as the bitmap of indices
> to cover such cases for platform devices, which it seems you clearly
> didn't like....otherwise, any otherway we can handle such cases?
Hi Robin,
Don't want to bother you with my ideas, but I can't think of other ways
to handle such cases of multi-map than the below. I just tried this code on
Qemu on top of your patches(with some nit compilation fixes) and just checked
if devices are added to the iommu groups.
----------------------8888---------------------------------------------
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index a511ecf21fcd..ac005e70de7d 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -16,6 +16,7 @@
#include <linux/pci.h>
#include <linux/slab.h>
#include <linux/fsl/mc.h>
+#include <linux/platform_device.h>
#include "iommu-priv.h"
@@ -41,6 +42,18 @@ static int of_iommu_xlate(struct device *dev,
return ret;
}
+static int of_iommu_configure_cb(void *arg, u32 *id_out)
+{
+ struct of_phandle_args *iommu_spec =
+ (struct of_phandle_args *)((void *)id_out - offsetof(struct of_phandle_args, args));
+ struct device *dev = arg;
+ int err;
+
+ err = of_iommu_xlate(dev, iommu_spec);
+ of_node_put(iommu_spec->np);
+ return err;
+}
+
static int of_iommu_configure_dev_id(struct device_node *master_np,
struct device *dev,
const u32 *id)
@@ -48,12 +61,10 @@ static int of_iommu_configure_dev_id(struct device_node *master_np,
struct of_phandle_args iommu_spec = { .args_count = 1 };
int err;
- err = of_map_iommu_id(master_np, *id, &iommu_spec.np, iommu_spec.args);
+ err = of_map_iommu_id(master_np, *id, &iommu_spec.np, iommu_spec.args,
+ dev_is_platform(dev) ? true : false, dev, of_iommu_configure_cb);
- if (err)
- return err;
-
- err = of_iommu_xlate(dev, &iommu_spec);
- of_node_put(iommu_spec.np);
return err;
}
diff --git a/drivers/of/base.c b/drivers/of/base.c
index a4fd4a4f720b..8abe36c17309 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2085,16 +2085,21 @@ static bool of_check_bad_map(const __be32 *map, int len)
*/
int of_map_id(const struct device_node *np, u32 id, const char *map_name,
const char *cells_name, const char *map_mask_name,
- struct device_node **target, u32 *id_out)
+ struct device_node **target, u32 *id_out, bool multi_map,
+ void *arg, of_map_id_cb cb)
{
u32 map_mask, masked_id;
int map_bytes, map_len, offset = 0;
bool bad_map = false;
const __be32 *map = NULL;
+ bool mapped_multi_id = false;
if (!np || !map_name || !cells_name || (!target && !id_out))
return -EINVAL;
+ if (multi_map && !cb)
+ return -EINVAL;
+
map = of_get_property(np, map_name, &map_bytes);
if (!map) {
if (target)
@@ -2189,16 +2194,29 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
pr_debug("%pOF: %s, using mask %08x, id-base: %08x, out-base: %08x, length: %08x, id: %08x -> %08x\n",
np, map_name, map_mask, id_base, be32_to_cpup(out_base),
id_len, id, id_off + be32_to_cpup(out_base));
- return 0;
+
+ if (multi_map) {
+ if (cb(arg, id_out))
+ return -EINVAL;
+
+ mapped_multi_id = true;
+ continue;
+ }
+
+ goto translated;
}
+ if (mapped_multi_id)
+ return 0;
+
pr_info("%pOF: no %s translation for id 0x%x on %pOF\n", np, map_name,
id, target && *target ? *target : NULL);
/* Bypasses translation */
if (id_out)
*id_out = id;
- return 0;
+translated:
+ return cb ? cb(arg, id_out) : 0;
err_map_len:
pr_err("%pOF: Error: Bad %s length: %d\n", np, map_name, map_bytes);
diff --git a/include/linux/of.h b/include/linux/of.h
index 183be897b088..84a24c2a1041 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -24,6 +24,7 @@
typedef u32 phandle;
typedef u32 ihandle;
+typedef int (*of_map_id_cb)(void *arg, u32 *id_out);
struct property {
char *name;
@@ -458,7 +459,8 @@ bool of_console_check(const struct device_node *dn, char *name, int index);
int of_map_id(const struct device_node *np, u32 id, const char *map_name,
const char *cells_name, const char *map_mask_name,
- struct device_node **target, u32 *id_out);
+ struct device_node **target, u32 *id_out,
+ bool multi_map, void *arg, of_map_id_cb cb);
phys_addr_t of_dma_get_max_cpu_address(struct device_node *np);
@@ -1436,17 +1438,18 @@ static inline int of_property_read_s32(const struct device_node *np,
}
static inline int of_map_iommu_id(const struct device_node *np, u32 id,
- struct device_node **target, u32 *id_out)
+ struct device_node **target, u32 *id_out,
+ bool multi_map, void *arg, of_map_id_cb cb)
{
return of_map_id(np, id, "iommu-map", "#iommu-cells", "iommu-map-mask",
- target, id_out);
+ target, id_out, multi_map, arg, cb);
}
static inline int of_map_msi_id(const struct device_node *np, u32 id,
struct device_node **target, u32 *id_out)
{
return of_map_id(np, id, "msi-map", "#msi-cells", "msi-map-mask",
- target, id_out);
+ target, id_out, false, NULL, NULL);
}
#define of_for_each_phandle(it, err, np, ln, cn, cc)
-----------------------------------------------------------------------
full patch is at: https://github.com/charan-kalla-oss/linux-next/commits/refs/for/iommu_map
Thanks,
Charan
Powered by blists - more mailing lists