Re: [PATCH v15 3/3] of: Respect #{iommu,msi}-cells in maps

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Strict requirement for `#iommu-cells` or `#msi-cells` breaks MSI allocations on standard-compliant controllers.
- [High] Array desynchronization when parsing legacy 0-cell maps padded to 4 words per entry.
--

commit 23cece64ecb1dcea6290dbab34facbdac53ff293
Author: Robin Murphy <robin.murphy@arm.com>
Date:   Wed May 20 08:02:42 2026 +0000

of: Respect #{iommu,msi}-cells in maps

This commit adds support for parsing target nodes' #cells values for
iommu-map and msi-map properties, instead of assuming output specifiers
always have exactly 1 cell. It updates of_map_id() to parse multi-cell and
0-cell output specifiers and reflect the actual number of output specifier
cells.

> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index d658c26201353..f436e26763818 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c

[ ... ]

> @@ -2176,39 +2206,93 @@ int of_map_id(const struct device_node *np, u32 id,

[ ... ]

> +		phandle = be32_to_cpup(map + offset + 1);
>  		phandle_node = of_find_node_by_phandle(phandle);
>  		if (!phandle_node)
>  			return -ENODEV;
>  
> +		if (bad_map) {
> +			cells = 1;
> +		} else if (of_property_read_u32(phandle_node, cells_name, &cells)) {
> +			pr_err("%pOF: missing %s property\n", phandle_node, cells_name);
> +			of_node_put(phandle_node);
> +			return -EINVAL;
> +		}

[Severity: High]
Does this strict requirement for #iommu-cells or #msi-cells break MSI
allocations on standard-compliant controllers?

Several MSI controllers validly lack a #msi-cells property. For instance,
arm,gic-v2m-frame nodes do not require it. In
arch/arm64/boot/dts/qcom/ipq5018.dtsi, the pcie0 node defines:

msi-map = <0x0 &v2m0 0x0 0xff8>;

but v2m0 does not have #msi-cells.

Prior to this patch, of_map_id() implicitly consumed 4 words per entry and
succeeded. Will it now fail to find #msi-cells, print an error, and
return -EINVAL, propagating the error through of_msi_xlate() and breaking
MSI initialization on affected platforms?

> +
> +		if (cells > MAX_PHANDLE_ARGS) {

[ ... ]

> +		if (map_len - offset < 3 + cells) {
> +			of_node_put(phandle_node);
> +			goto err_map_len;
> +		}
> +
> +		out_base = map + offset + 2;
> +		offset += 3 + cells;

[Severity: High]
Could this cause array desynchronization when parsing legacy 0-cell maps
that were padded to 4 words per entry?

Prior to this patch, of_map_id() mandated that the map property length must
be a multiple of 16 bytes:

if (!map_len || map_len % (4 * sizeof(*map)))
    return -EINVAL;

Consequently, if a legacy device tree targeted an MSI controller that
explicitly defined #msi-cells = <0> (such as RISC-V IMSIC nodes), it was
structurally forced to pad the 3-word map entry with a dummy 4th word to
satisfy the kernel's length check.

Since the new logic computes the entry stride dynamically as 3 + cells,
when parsing a legacy padded map with cells=0, does the code advance by 3
and incorrectly interpret the 4th dummy word of the first entry as the
id_base of the second entry?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520-parse_iommu_cells-v15-0-b5f99ad4e7e8@oss.qualcomm.com?part=3