| 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: <hunwsdkmeo6efpv3yt3izkgmarelnubd74dywj3scryxrreq6p@njijwtoyjh46>
Date: Tue, 27 Jan 2026 13:45:27 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Vikash Garodia <vikash.garodia@....qualcomm.com>
Cc: Dikshita Agarwal <dikshita.agarwal@....qualcomm.com>,
Abhinav Kumar <abhinav.kumar@...ux.dev>,
Bryan O'Donoghue <bod@...nel.org>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Saravana Kannan <saravanak@...nel.org>, Joerg Roedel <joro@...tes.org>,
Will Deacon <will@...nel.org>, Robin Murphy <robin.murphy@....com>,
Stefan Schmidt <stefan.schmidt@...aro.org>,
Hans Verkuil <hverkuil@...nel.org>,
Krzysztof Kozlowski <krzk@...nel.org>,
Vishnu Reddy <busanna.reddy@....qualcomm.com>,
Hans Verkuil <hverkuil+cisco@...nel.org>,
linux-arm-msm@...r.kernel.org, linux-media@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
iommu@...ts.linux.dev, Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
Charan Teja Kalla <charan.kalla@....qualcomm.com>,
Vijayanand Jitta <vijayanand.jitta@....qualcomm.com>
Subject: Re: [PATCH 3/7] of/iommu: add multi-map support
On Mon, Jan 26, 2026 at 05:55:46PM +0530, Vikash Garodia wrote:
> From: Charan Teja Kalla <charan.kalla@....qualcomm.com>
>
> When multiple mappings are present for an input id, linux matches just
> the first one. There is a usecase[1] where all the mappings are to be
> maintained in parallel for an iommu-map entry of a same input id.
This contradicts the IOMMU idealogy (at least as far as I understood it
fom the maintainers): the device (driver) doesn't control which IOMMUs
are getting used. Instead _all_ defined entries should get used. For
iommu-map it means that if the map defines several entries for a single
function, then all entries should always get mapped.
>
> Whether multi-map is needed is reported by the callers through the
> callback function passed, which is called for every input id match.
>
> Since the requirement in the usecase[1] is for platform devices, not
> sure if it is really clean to maintain this decision on the bus type at
> the of_iommu layer or further to be from the respective
> iommu_driver->impl_ops().
This doesn't tell us, why do you want to control, which entries of the
map get used.
>
> [1] https://lore.kernel.org/all/20250627-video_cb-v3-0-51e18c0ffbce@quicinc.com/
>
> Signed-off-by: Charan Teja Kalla <charan.kalla@....qualcomm.com>
> Signed-off-by: Vijayanand Jitta <vijayanand.jitta@....qualcomm.com>
> Signed-off-by: Vikash Garodia <vikash.garodia@....qualcomm.com>
> ---
> drivers/iommu/of_iommu.c | 36 ++++++++++++++++++++++++++++--------
> drivers/of/base.c | 38 ++++++++++++++++++++++++++++----------
> include/linux/of.h | 6 ++++++
> 3 files changed, 62 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 768eaddf927b0700b2497b08ea21611b1a1b5688..067bb2298973671e1eaf01bb2ea52df3d2a52a44 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -16,6 +16,7 @@
> #include <linux/pci.h>
> #include <linux/slab.h>
> #include <linux/fsl/mc.h>
> +#include <linux/platform_device.h>
>
> #include "iommu-priv.h"
>
> @@ -41,22 +42,41 @@ static int of_iommu_xlate(struct device *dev,
> return ret;
> }
>
> +/*
> + * Callback to be called from of_map_id(), that tells if
> + * all the mappings for an input id to be maintained in
> + * parallel. Should this decission be from further layers,
> + * iommu_driver->impl_ops?
> + */
> +static int of_iommu_configure_cb(struct of_map_id_arg *arg)
> +{
> + struct of_phandle_args *iommu_spec = &arg->map_args;
> + struct device *dev = arg->dev;
> + int err;
> +
> + err = of_iommu_xlate(dev, iommu_spec);
> + of_node_put(iommu_spec->np);
> +
> + /* !iommu_spec->np may be from the bypassed translations */
> + if (!err)
> + err = (!arg->multi_map || !iommu_spec->np) ? 0 : -EAGAIN;
> +
> + return err;
> +}
> +
> static int of_iommu_configure_dev_id(struct device_node *master_np,
> struct device *dev,
> const u32 *id)
> {
> struct of_map_id_arg arg = {
> .map_args = {},
> + .cb = of_iommu_configure_cb,
> + .dev = dev,
> + /* Should this be pushed to iommu_driver->impl_ops? */
> + .multi_map = dev_is_platform(dev),
> };
> - int err;
> -
> - err = of_map_iommu_id(master_np, *id, &arg);
> - if (err)
> - return err;
>
> - err = of_iommu_xlate(dev, &arg.map_args);
> - of_node_put(arg.map_args.np);
> - return err;
> + return of_map_iommu_id(master_np, *id, &arg);
> }
>
> static int of_iommu_configure_dev(struct device_node *master_np,
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 606bef4f90e7d13bae4f7b0c45acd1755ad89826..a1c3c5954ec7e8eb3753c8fd782a1570f9eb9c17 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2122,14 +2122,21 @@ static bool of_check_bad_map(const __be32 *map, int len)
> return true;
> }
>
> -static int of_map_id_fill_output(struct of_map_id_arg *arg,
> - struct device_node *phandle_node, u32 id_or_offset,
> - const __be32 *out_base, u32 cells,
> - bool bypass)
> +/*
> + * Fill the id_out and target for the of_map_id() caller. Also
> + * call the callback passed to the of_map_id() as part of the arg
> + * that decides if to continue further search.
> + */
> +static int of_map_id_fill_arg(struct of_map_id_arg *arg,
> + struct device_node *phandle_node, u32 id_or_offset,
> + const __be32 *out_base, u32 cells,
> + bool bypass, bool *multi_id_map)
> {
> + int ret;
> +
> if (bypass) {
> arg->map_args.args[0] = id_or_offset;
> - return 0;
> + goto output;
> }
>
> if (arg->map_args.np)
> @@ -2145,7 +2152,14 @@ static int of_map_id_fill_output(struct of_map_id_arg *arg,
>
> arg->map_args.args_count = cells;
>
> - return 0;
> +output:
> + /* pass the output for the callback, callers may further decide */
> + ret = arg->cb ? arg->cb(arg) : 0;
> +
> + if (multi_id_map && ret == -EAGAIN)
> + *multi_id_map = true;
> +
> + return ret;
> }
>
> /**
> @@ -2179,6 +2193,7 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
> int map_bytes, map_len, offset = 0;
> bool bad_map = false;
> const __be32 *map = NULL;
> + bool multi_id_map = false;
>
> if (!np || !map_name || !arg)
> return -EINVAL;
> @@ -2264,23 +2279,26 @@ int of_map_id(const struct device_node *np, u32 id, const char *map_name,
> if (masked_id < id_base || id_off >= id_len)
> continue;
>
> - ret = of_map_id_fill_output(arg, phandle_node, id_off, out_base, cells, false);
> + ret = of_map_id_fill_arg(arg, phandle_node, id_off, out_base,
> + cells, false, &multi_id_map);
> if (ret == -EAGAIN)
> continue;
>
> 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;
> + return ret;
> }
>
> + if (multi_id_map)
> + return 0;
> +
> pr_info("%pOF: no %s translation for id 0x%x on %pOF\n", np, map_name,
> id, arg->map_args.np ? arg->map_args.np : NULL);
>
> bypass_translation:
> /* Bypasses translation */
> - return of_map_id_fill_output(arg, NULL, id, 0, 0, true);
> -
> + return of_map_id_fill_arg(arg, NULL, id, 0, 0, true, NULL);
> err_map_len:
> pr_err("%pOF: Error: Bad %s length: %d\n", np, map_name, map_bytes);
> return -EINVAL;
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 9efa6f93712c6024f05476f9fd39f3294f942ec1..abab73a76682351f5635c1127a6c899917525050 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -25,6 +25,9 @@
> typedef u32 phandle;
> typedef u32 ihandle;
>
> +struct of_map_id_arg;
> +typedef int (*of_map_id_cb)(struct of_map_id_arg *arg);
> +
> struct property {
> char *name;
> int length;
> @@ -76,6 +79,9 @@ struct of_phandle_args {
>
> struct of_map_id_arg {
> struct of_phandle_args map_args;
> + of_map_id_cb cb;
> + struct device *dev;
> + bool multi_map;
> };
>
> struct of_phandle_iterator {
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
Powered by blists - more mailing lists