| 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: <95ea1872a0afb10265aa675817647d830de42d70.camel@ndufresne.ca>
Date: Tue, 27 Jan 2026 08:51:17 -0500
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>, 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
Hi,
Le mardi 27 janvier 2026 à 13:45 +0200, Dmitry Baryshkov a écrit :
> 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/
Since its for an M2M V4L2 device, and its all about having more address space
according to this link, you may want to use a different iommu domain per m2m
instances, rather then per device (what is done implicitly). On top of which, it
will ensure memory isolation between instances, making this driver suitable for
virtualisation. 4Gb per instance (which is also per stream) seems like a lot of
memory to me. But like Dmitry says, there is little clarity in what you are
trying to achieve.
Nicolas
> >
> > 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..067bb2298973671e1eaf01bb2ea52df3d2
> > a52a44 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..a1c3c5954ec7e8eb3753c8fd782a1570f9
> > eb9c17 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..abab73a76682351f5635c1127a6c899917
> > 525050 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
> >
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists