Re: [PATCH 02/22] arm64: dts: qcom: kodiak: Fix the PCIe iommu-map entries

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Changing the `iommu-map` entries from 4 cells to 5 cells breaks the manual parsing logic in the QCOM PCIe driver, resulting in corrupted BDF-to-SID hardware configurations.

Pre-existing issues:
- [Medium] The patch corrects an incorrectly formatted 4-cell `iommu-map` property by adding the missing SID mask cell to match the 2-cell `apps_smmu` provider in `kodiak.dtsi`. However, it fails to address the exact same defect present in numerous other Qualcomm SoC device trees and their YAML bindings.
--

--- Patch [2]: [PATCH 02/22] arm64: dts: qcom: kodiak: Fix the PCIe iommu-map entries ---
commit 3a1ab560afad0b97b3e224b3201d2ab8bc8cd668
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

arm64: dts: qcom: kodiak: Fix the PCIe iommu-map entries
    
The IOMMU provider apps_smmu uses '#iommu-cells = <2>', but the PCIe
iommu-map entries specify only one cell for the SID, omitting the SID
mask. This went unnoticed until the OF core started warning with commit
ccb2fd725d41. This patch fixes the entries to match the provider's
'#iommu-cells' property.

> diff --git a/arch/arm64/boot/dts/qcom/kodiak.dtsi b/arch/arm64/boot/dts/qcom/kodiak.dtsi
> index fa540d8c2615d..2eb613dd67165 100644
> --- a/arch/arm64/boot/dts/qcom/kodiak.dtsi
> +++ b/arch/arm64/boot/dts/qcom/kodiak.dtsi
> @@ -2278,8 +2278,8 @@ pcie0: pcie@1c00000 {
>  				      "aggre0",
>  				      "aggre1";
>  
> -			iommu-map = <0x0   &apps_smmu 0x1c00 0x1>,
> -				    <0x100 &apps_smmu 0x1c01 0x1>;
> +			iommu-map = <0x0   &apps_smmu 0x1c00 0x0 0x1>,
> +				    <0x100 &apps_smmu 0x1c01 0x0 0x1>;

[Severity: High]
Will changing this to 5 cells break the manual parsing logic in the QCOM
PCIe driver?

Looking at drivers/pci/controller/dwc/pcie-qcom.c,
qcom_pcie_config_sid_1_9_0() manually casts the raw iommu-map property into
a 4-u32 struct array:

struct {
	u32 bdf;
	u32 phandle;
	u32 smmu_sid;
	u32 smmu_sid_len;
} *map;

If the device tree format is updated to 5 cells, will reading this array
cause out-of-sync reads? For the second entry, map[1].bdf would read the
length cell of the first entry (0x1), and map[1].smmu_sid would read the
phandle of the second entry. This could result in corrupted BDF-to-SID
hardware configurations, causing PCIe devices to use invalid SIDs for DMA.

[Severity: Medium]
This is a pre-existing issue, but does this fix need to be applied to other
Qualcomm SoCs as well?

This patch corrects an incorrectly formatted 4-cell iommu-map property by
adding the missing SID mask cell to match the 2-cell apps_smmu provider in
kodiak.dtsi, but fails to address the exact same defect present in numerous
other Qualcomm SoC device trees.

For example, in arch/arm64/boot/dts/qcom/sm8550.dtsi, pcie0 contains:

iommu-map = <0x0   &apps_smmu 0x1400 0x1>,

Other SoCs like sm8650 and sc8180x also seem to omit the SID mask. The OF
core warning will continue to be generated for all other affected Qualcomm
SoCs until they are updated.

(Note that fixing these DTs directly would also break the driver as mentioned
above, so the driver must likely be updated first to handle variable-length
cells or use standard OF core parsing before applying these DT changes).

>  
>  			resets = <&gcc GCC_PCIE_0_BCR>;
>  			reset-names = "pci";

[ ... ]

> @@ -2424,8 +2424,8 @@ pcie1: pcie@1c08000 {
>  
>  			dma-coherent;
>  
> -			iommu-map = <0x0 &apps_smmu 0x1c80 0x1>,
> -				    <0x100 &apps_smmu 0x1c81 0x1>;
> +			iommu-map = <0x0 &apps_smmu 0x1c80 0x0 0x1>,
> +				    <0x100 &apps_smmu 0x1c81 0x0 0x1>;
>  
>  			status = "disabled";
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-iommu-map-fix-v1-0-83405d37ba41@oss.qualcomm.com?part=2