Re: [PATCH v16 3/3] of: Respect #{iommu,msi}-cells in maps
Robin Murphy <[email protected]> Tue, 28 Jul 2026 11:04:27 +0100
| Newsgroups | dev.linux.lists.iommu,dev.linux.lists.imx,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci,org.xenproject.lists.xen-devel |
|---|---|
| Message-ID | <[email protected]> |
On 28/07/2026 5:05 am, Vijayanand Jitta wrote: > > > On 7/23/2026 6:47 PM, Neil Armstrong wrote: >> Hi, >> >> On 6/3/26 09:13, Vijayanand Jitta wrote: >>> From: Robin Murphy <[email protected]> >>> >>> So far our parsing of {iommu,msi}-map properties has always blindly >>> assumed that the output specifiers will always have exactly 1 cell. >>> This typically does happen to be the case, but is not actually enforced >>> (and the PCI msi-map binding even explicitly states support for 0 or 1 >>> cells) - as a result we've now ended up with dodgy DTs out in the field >>> which depend on this behaviour to map a 1-cell specifier for a 2-cell >>> provider, despite that being bogus per the bindings themselves. >>> >>> Since there is some potential use in being able to map at least single >>> input IDs to multi-cell output specifiers (and properly support 0-cell >>> outputs as well), add support for properly parsing and using the target >>> nodes' #cells values, albeit with the unfortunate complication of still >>> having to work around expectations of the old behaviour too. >>> >>> Since there are multi-cell output specifiers, the callers of of_map_id() >>> may need to get the exact cell output value for further processing. >>> Update of_map_id() to set args_count in the output to reflect the actual >>> number of output specifier cells. >>> >>> Signed-off-by: Robin Murphy <[email protected]> >>> Signed-off-by: Charan Teja Kalla <[email protected]> >>> Signed-off-by: Vijayanand Jitta <[email protected]> >>> --- >>> drivers/of/base.c | 168 +++++++++++++++++++++++++++++++++++++++++------------ >>> include/linux/of.h | 6 +- >>> 2 files changed, 135 insertions(+), 39 deletions(-) >>> >>> diff --git a/drivers/of/base.c b/drivers/of/base.c >>> index d658c2620135..ac7961cbab94 100644 >>> --- a/drivers/of/base.c >>> +++ b/drivers/of/base.c >>> @@ -2116,19 +2116,49 @@ int of_find_last_cache_level(unsigned int cpu) >>> return cache_level; >>> } >>> +/* >>> + * Some DTs have an iommu-map targeting a 2-cell IOMMU node while >>> + * specifying only 1 cell. Fortunately they all consist of value '1' >>> + * as the 2nd cell entry with the same target, so check for that pattern. >>> + * >>> + * Example: >>> + * IOMMU node: >>> + * #iommu-cells = <2>; >>> + * >>> + * Device node: >>> + * iommu-map = <0x0000 &smmu 0x0000 0x1>, >>> + * <0x0100 &smmu 0x0100 0x1>; >> >> So the sm8650 PCIe controllers has: >> >> pcie@1c08000: >> iommu-map = <0 &apps_smmu 0x1480 0x1>, >> <0x100 &apps_smmu 0x1481 0x1>; >> >> and >> >> pcie@1c00000: >> >> iommu-map = <0 &apps_smmu 0x1400 0x1>, >> <0x100 &apps_smmu 0x1401 0x1>; >> >> and apps_smmu has #iommu-cells = <2>, but gets flagged at wrong: >> >> [ 7.538800] OF: /soc@0/pcie@1c08000: iommu-map has 1-cell entries targeting 2-cell #iommu-cells, treating as 1-cell output >> >> Returning false in of_check_bad_map() triggers: >> >> [ 7.642680] OF: /soc@0/pcie@1c08000: Unsupported iommu-map - cannot handle 256-ID range with 2-cell output specifier >> >> I don't understand the issue here, we use 2 cells as expected by >> the iommu-cells, so why is it wrong ? can somebody explain in >> comprehensive words ? I'm super confused, it worked like a charm until now. >> >> Neil >> > > Hi Neil, > > iommu-map = <0 &apps_smmu 0x1480 0x1>, > <0x100 &apps_smmu 0x1481 0x1>; > > > Entries here are not 2-cell format, Even though apps_smmu declares #iommu-cells = <2>, > this DT only supplies one output cell (0x1480/0x1481) — the trailing 0x1 is the length field, > not a second output cell. (<id-base phandle out-base length>) > > The new code detects exactly this pattern (same target phandle across all entries, length always 1) > and falls back to treating the map as 1-cell output for backward compatibility — hence the pr_warn_once. > It's harmless and expected, your RIDs still resolve to the correct SIDs (0 → 0x1480, 0x100 → 0x1481). > > The second message is a different case and shouldn't be coming from this same map — once the 1-cell > fallback triggers on the first entry, it applies to the whole map, so you shouldn't hit both warnings > together on the same node. That error only fires for a genuine 2-cell output specifier combined with > an id_len > 1, e.g.: > > iommu-map = <0x0 &apps_smmu 0x1480 0x1 0x100>; > > (<id-base, phandle, out0, out1, length=256>) — which isn't supported, since there's no way to > linearly scale a multi-cell output specifier across a range of IDs. > > Are you seeing that second error on the same pcie node, or a different one? > If it's the same node, can you share the exact iommu-map entry that triggers it? I think Neil is saying he bypassed the fallback check so that it *did* try to parse the given map with the real #iommu-cells=2 in precisely the way you've shown - so even if it could have got past that point, it would have then blown trying to parse the second "entry" of just <&apps_smmu 0x1481 0x1>, since 0x1481 almost certainly isn't a valid phandle to read an #iommu-cells value from. Cheers, Robin.