Re: [PATCH v3 12/13] arm64: dts: qcom: sm8650: Add Iris context bank subnodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Dynamically created sub-devices for SMMU context banks lack PM runtime enablement, keeping the SMMU permanently active.
- [High] of_iommu_derive_resv_regions() erroneously translates IOVAs into CPU physical addresses.
--

commit 6e1de71c7ed4433a945133dd5bedc79a78bdd2e4
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>

arm64: dts: qcom: sm8650: Add Iris context bank subnodes

This commit introduces subnodes for the non-pixel and pixel VPU SMMU streams
so that each stream can be associated with its respective addressable range.
It adds an iommu-ranges property to the non-pixel stream to restrict its
allocations, addressing hardware faults caused by buffers landing below the
600MB IOVA boundary.

> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> index b679fd8afb7d5..97dad9c7f7fc4 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi

[ ... ]

> @@ -5287,6 +5288,15 @@ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
>  			 */
>  			status = "disabled";
>  
> +			non-pixel {
> +				iommus = <&apps_smmu 0x1940 0x0>;

[Severity: High]
Does adding these subnodes cause a PM runtime leak in the SMMU?

When these dummy platform devices are dynamically created for context banks by
the iris driver in drivers/media/platform/qcom/iris/iris_probe.c:

iris_create_cb_dev() {
    ...
    pdev = platform_device_register_full(&plat_dev_info);
    ...
}

The IOMMU core intercepts them, attaches them to the SMMU, and establishes a
PM runtime device link (DL_FLAG_PM_RUNTIME). Because the driver never
explicitly calls pm_runtime_enable() on these dummy devices, their PM
runtime disable depth remains greater than zero.

The PM core interprets this as the consumer being active, forcing the
supplier (the SMMU) and its interconnect power domains to resume and stay
active permanently.

> +				iommu-ranges = <0x25800000 0xba800000>;

[Severity: High]
Does the use of iommu-ranges here cause the IOMMU core to reserve the wrong
address space?

When this property is parsed in drivers/iommu/of_iommu.c to compute reserved
regions:

of_iommu_derive_resv_regions() {
    ...
    maps = of_translate_dma_region(dev->of_node, maps, &iova, &length);
    ...
}

The function of_translate_dma_region() uses dma-ranges to translate up to a
CPU physical address, rather than preserving the intended IOVA space.

This CPU physical address is then incorrectly passed as the IOVA base to
iommu_alloc_resv_region(), which can severely break the IOVA allocator and
block valid DMA allocations on architectures with non-identity DMA offsets.

> +			};
> +
> +			pixel {
> +				iommus = <&apps_smmu 0x1947 0x0>;
> +			};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-vpu_iommu_iova_handling-v3-0-2fe43ae45495@oss.qualcomm.com?part=12