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

[Neil Armstrong <neil.armstrong@linaro.org>]

On 7/28/26 12:39, Vijayanand Jitta wrote:
> 
> 
> On 7/28/2026 3:34 PM, Robin Murphy wrote:
>> 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 <robin.murphy@arm.com>
>>>>>
>>>>> 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 <robin.murphy@arm.com>
>>>>> Signed-off-by: Charan Teja Kalla <charan.kalla@oss.qualcomm.com>
>>>>> Signed-off-by: Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
>>>>> ---
>>>>>     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.
> 
> Right, I get it now, so when it tried to parse with iommu-cells as 2,
> 
> iommu-map = <0     &apps_smmu 0x1480 0x1>,
>              <0x100 &apps_smmu 0x1481 0x1>;
> 
> Above tuples would look something like <0  &apps_smmu 0x1480 0x1 0x100>, where 0x100 from next tuple would be seen as length. Hence, the above error log.
> And the next tuple <&apps_smmu 0x1481 0x1> won't be able to get parsed as you mentioned.

Thanks for the explanation, clearer now.

Neil

> 
> 
> Thanks,
> Vijay
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
>