[PATCH] PCI: qcom: Honor IOMMU provider's #iommu-cells in qcom_pcie_config_sid_1_9_0()

[PATCH] PCI: qcom: Honor IOMMU provider's #iommu-cells in qcom_pcie_config_sid_1_9_0()

[Manivannan Sadhasivam]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

qcom_pcie_config_sid_1_9_0() reads the "iommu-map" property as an array
of fixed four-word {rid-base, phandle, sid, rid-len} entries to program
the BDF to SID translation table. But that layout only holds for an
IOMMU with '#iommu-cells = <1>'. The PCIe SMMUs on these SoCs use
'#iommu-cells = <2>' (SID and mask), so per the pci-iommu binding each
entry is really five cells long.

This used to work only because the DTs were themselves broken. They
described iommu-map with four-cell entries that omitted the SID mask,
which of_map_id() tolerated via its of_check_bad_map() fallback, and the
four-word parsing coincidentally matched that malformed shape.

Since commit ccb2fd725d41 ("of: Respect #{iommu,msi}-cells in maps") the
OF core parses such maps correctly, so the device trees were converted
to proper five-cell entries, e.g. commit c41749e9554d ("arm64: dts:
qcom: sm8250: Fix the PCIe iommu-map entries"). With five-cell entries
the fixed four-word stride slips by one cell for each entry after the
first, so qcom_pcie_config_sid_1_9_0() reads the endpoint mapping's RID
from the preceding entry's length cell and its SID from the phandle cell.
As the RID is the hash key, the endpoint's real RID is never programmed
into the BDF to SID table. Its DMA then hashes to an unprogrammed slot,
gets tagged with SID 0 and the SMMU faults like below on QCS8300:

  arm-smmu 15200000.iommu: Unhandled context fault: fsr=0x402, iova=0xffa00000, cbfrsynra=0x0, cb=1

To fix this, walk the map with a stride of 3 + '#iommu-cells' of the
referenced IOMMU and take the SID from the first specifier cell, which is
all the BDF to SID table needs. Validate the layout instead of trusting
the array size. Also, preserve the legacy behavior of the old DTs by
detecting the same pattern that of_check_bad_map() recognizes and
falling back to a stride of four.

Fixes: 4c9398822106 ("PCI: qcom: Add support for configuring BDF to SID mapping for SM8250")
Reported-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 90 ++++++++++++++++++--------
 1 file changed, 64 insertions(+), 26 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index b58a607b713f..42cbeef083ca 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1143,50 +1143,90 @@ static void qcom_pcie_deinit_2_7_0(struct qcom_pcie *pcie)
 
 static int qcom_pcie_config_sid_1_9_0(struct qcom_pcie *pcie)
 {
-	/* iommu map structure */
-	struct {
-		u32 bdf;
-		u32 phandle;
-		u32 smmu_sid;
-		u32 smmu_sid_len;
-	} *map;
 	void __iomem *bdf_to_sid_base = pcie->parf + PARF_BDF_TO_SID_TABLE_N;
 	struct device *dev = pcie->pci->dev;
+	struct device_node *iommu_np;
 	u8 qcom_pcie_crc8_table[CRC8_TABLE_SIZE];
-	int i, nr_map, size = 0;
-	u32 smmu_sid_base;
+	const __be32 *map;
+	u32 iommu_cells, entry_cells, phandle, smmu_sid_base;
+	int i, nr_cells, nr_map, size = 0;
 	u32 val;
 
-	of_get_property(dev->of_node, "iommu-map", &size);
-	if (!size)
+	map = of_get_property(dev->of_node, "iommu-map", &size);
+	if (!map || !size)
 		return 0;
 
+	if (size % sizeof(*map)) {
+		dev_err(dev, "Malformed iommu-map property\n");
+		return -EINVAL;
+	}
+	nr_cells = size / sizeof(*map);
+
+	/*
+	 * Each iommu-map entry is: rid-base (1 cell), phandle (1 cell),
+	 * IOMMU specifier (#iommu-cells cells), length (1 cell). Read
+	 * #iommu-cells from the IOMMU provider referenced by the first
+	 * entry to compute the per-entry stride.
+	 */
+	phandle = be32_to_cpu(map[1]);
+	iommu_np = of_find_node_by_phandle(phandle);
+	if (!iommu_np) {
+		dev_err(dev, "Failed to find IOMMU node in iommu-map\n");
+		return -ENODEV;
+	}
+
+	if (of_property_read_u32(iommu_np, "#iommu-cells", &iommu_cells))
+		iommu_cells = 1;
+	of_node_put(iommu_np);
+
+	entry_cells = 3 + iommu_cells;
+
+	/*
+	 * Retain backward compatibility with DTs that describe iommu-map
+	 * with 4-cell entries against an IOMMU declaring #iommu-cells = 2,
+	 * matching the fallback in drivers/of/base.c::of_check_bad_map().
+	 */
+	if (iommu_cells == 2 && !(nr_cells % 4)) {
+		bool legacy = true;
+
+		for (i = 0; i < nr_cells; i += 4) {
+			if (be32_to_cpu(map[i + 1]) != phandle ||
+			    be32_to_cpu(map[i + 3]) != 1) {
+				legacy = false;
+				break;
+			}
+		}
+
+		if (legacy) {
+			dev_warn_once(dev, "iommu-map has 1-cell entries targeting 2-cell #iommu-cells, treating as 1-cell output\n");
+			entry_cells = 4;
+		}
+	}
+
+	if (nr_cells % entry_cells) {
+		dev_err(dev, "Malformed iommu-map property\n");
+		return -EINVAL;
+	}
+	nr_map = nr_cells / entry_cells;
+
 	/* Enable BDF to SID translation by disabling bypass mode (default) */
 	val = readl(pcie->parf + PARF_BDF_TO_SID_CFG);
 	val &= ~BDF_TO_SID_BYPASS;
 	writel(val, pcie->parf + PARF_BDF_TO_SID_CFG);
 
-	map = kzalloc(size, GFP_KERNEL);
-	if (!map)
-		return -ENOMEM;
-
-	of_property_read_u32_array(dev->of_node, "iommu-map", (u32 *)map,
-				   size / sizeof(u32));
-
-	nr_map = size / (sizeof(*map));
-
 	crc8_populate_msb(qcom_pcie_crc8_table, QCOM_PCIE_CRC8_POLYNOMIAL);
 
 	/* Registers need to be zero out first */
 	memset_io(bdf_to_sid_base, 0, CRC8_TABLE_SIZE * sizeof(u32));
 
 	/* Extract the SMMU SID base from the first entry of iommu-map */
-	smmu_sid_base = map[0].smmu_sid;
+	smmu_sid_base = be32_to_cpu(map[2]);
 
 	/* Look for an available entry to hold the mapping */
 	for (i = 0; i < nr_map; i++) {
-		__be16 bdf_be = cpu_to_be16(map[i].bdf);
-		u32 val;
+		u32 bdf = be32_to_cpu(map[i * entry_cells]);
+		u32 sid = be32_to_cpu(map[i * entry_cells + 2]);
+		__be16 bdf_be = cpu_to_be16(bdf);
 		u8 hash;
 
 		hash = crc8(qcom_pcie_crc8_table, (u8 *)&bdf_be, sizeof(bdf_be), 0);
@@ -1208,12 +1248,10 @@ static int qcom_pcie_config_sid_1_9_0(struct qcom_pcie *pcie)
 		}
 
 		/* BDF [31:16] | SID [15:8] | NEXT [7:0] */
-		val = map[i].bdf << 16 | (map[i].smmu_sid - smmu_sid_base) << 8 | 0;
+		val = bdf << 16 | (sid - smmu_sid_base) << 8 | 0;
 		writel(val, bdf_to_sid_base + hash * sizeof(u32));
 	}
 
-	kfree(map);
-
 	return 0;
 }
 
-- 
2.43.0

Re: [PATCH] PCI: qcom: Honor IOMMU provider's #iommu-cells in qcom_pcie_config_sid_1_9_0()

[sashiko-bot]

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

New issues:
- [High] Out-of-bounds memory read when `iommu-map` property length is less than 2 cells.
- [High] Unvalidated `#iommu-cells` property causes integer overflow, leading to out-of-bounds memory reads or divide-by-zero undefined behavior.

Pre-existing issues:
- [High] Infinite loop if the number of `iommu-map` entries exceeds the fixed BDF-to-SID hardware table capacity.
--

commit 5e6355134714e5a9f21664a5ac644c1347a1924b
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

PCI: qcom: Honor IOMMU provider's #iommu-cells in qcom_pcie_config_sid_1_9_0()

This commit updates qcom_pcie_config_sid_1_9_0() to properly handle
iommu-map entries where the IOMMU provider uses more than one iommu-cell.
It dynamically calculates the per-entry stride based on the IOMMU's
#iommu-cells property instead of assuming a fixed four-word stride.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index b58a607b713f5..42cbeef083caf 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
[ ... ]
> @@ -1143,50 +1143,90 @@ static void qcom_pcie_deinit_2_7_0(struct qcom_pcie *pcie)
>  
>  static int qcom_pcie_config_sid_1_9_0(struct qcom_pcie *pcie)
>  {
[ ... ]
> +	if (size % sizeof(*map)) {
> +		dev_err(dev, "Malformed iommu-map property\n");
> +		return -EINVAL;
> +	}
> +	nr_cells = size / sizeof(*map);
> +
> +	/*
> +	 * Each iommu-map entry is: rid-base (1 cell), phandle (1 cell),
> +	 * IOMMU specifier (#iommu-cells cells), length (1 cell). Read
> +	 * #iommu-cells from the IOMMU provider referenced by the first
> +	 * entry to compute the per-entry stride.
> +	 */
> +	phandle = be32_to_cpu(map[1]);

[Severity: High]
Can this cause an out-of-bounds read if the iommu-map property is too short?

The function verifies the property size is a multiple of 4 bytes, but fails
to verify the minimum required length (at least 2 cells) before accessing
map[1] to extract the phandle. A malformed device tree with an iommu-map
property of 4 bytes (1 cell) could trigger an out-of-bounds memory read
during probe.

> +	iommu_np = of_find_node_by_phandle(phandle);
> +	if (!iommu_np) {
> +		dev_err(dev, "Failed to find IOMMU node in iommu-map\n");
> +		return -ENODEV;
> +	}
> +
> +	if (of_property_read_u32(iommu_np, "#iommu-cells", &iommu_cells))
> +		iommu_cells = 1;
> +	of_node_put(iommu_np);
> +
> +	entry_cells = 3 + iommu_cells;

[Severity: High]
Does this code allow an unvalidated #iommu-cells property to cause an
integer overflow?

If a device tree specifies an excessively large value for #iommu-cells,
such as 0xFFFFFFFE, entry_cells = 3 + iommu_cells will wrap around to 1.
If the value is 0xFFFFFFFD, it wraps to 0.

> +
> +	/*
> +	 * Retain backward compatibility with DTs that describe iommu-map
> +	 * with 4-cell entries against an IOMMU declaring #iommu-cells = 2,
> +	 * matching the fallback in drivers/of/base.c::of_check_bad_map().
> +	 */
> +	if (iommu_cells == 2 && !(nr_cells % 4)) {
[ ... ]
> +	}
> +
> +	if (nr_cells % entry_cells) {

[Severity: High]
Could the above integer overflow lead to a divide-by-zero here?

If entry_cells wrapped to 0 due to an overflow, this modulo operation will
trigger undefined behavior (divide-by-zero). If it wrapped to a small
non-zero number like 1, it could pass this check and result in
nr_map = nr_cells / 1, potentially causing out-of-bounds reads in the
subsequent loop.

> +		dev_err(dev, "Malformed iommu-map property\n");
> +		return -EINVAL;
> +	}
> +	nr_map = nr_cells / entry_cells;
> +
>  	/* Enable BDF to SID translation by disabling bypass mode (default) */
>  	val = readl(pcie->parf + PARF_BDF_TO_SID_CFG);
[ ... ]
>  	/* Look for an available entry to hold the mapping */
>  	for (i = 0; i < nr_map; i++) {
> -		__be16 bdf_be = cpu_to_be16(map[i].bdf);
> -		u32 val;
> +		u32 bdf = be32_to_cpu(map[i * entry_cells]);
> +		u32 sid = be32_to_cpu(map[i * entry_cells + 2]);
> +		__be16 bdf_be = cpu_to_be16(bdf);
>  		u8 hash;
>  
>  		hash = crc8(qcom_pcie_crc8_table, (u8 *)&bdf_be, sizeof(bdf_be), 0);

[Severity: High]
This is a pre-existing issue, but can this loop cause a kernel hard lockup?

Just below this in qcom_pcie_config_sid_1_9_0(), there is a linear probing
loop to handle hash collisions:

		while (val) {
			u8 current_hash = hash++;
			...
			val = readl(bdf_to_sid_base + hash * sizeof(u32));
		}

The hardware table has a capacity of 256 slots (CRC8_TABLE_SIZE). If a device
tree specifies more than 256 entries in iommu-map (or if hash collisions fill
all slots), the while (val) loop continuously increments the 8-bit hash index
and reads occupied slots, wrapping indefinitely. It lacks a termination
condition for a full translation table.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907143349.317495-1-mani@kernel.org?part=1

Re: [PATCH] PCI: qcom: Honor IOMMU provider's #iommu-cells in qcom_pcie_config_sid_1_9_0()

[Loic Poulain]

On Mon, Sep 7, 2026 at 4:34 PM Manivannan Sadhasivam
<manivannan.sadhasivam@oss.qualcomm.com> wrote:
>
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>
> qcom_pcie_config_sid_1_9_0() reads the "iommu-map" property as an array
> of fixed four-word {rid-base, phandle, sid, rid-len} entries to program
> the BDF to SID translation table. But that layout only holds for an
> IOMMU with '#iommu-cells = <1>'. The PCIe SMMUs on these SoCs use
> '#iommu-cells = <2>' (SID and mask), so per the pci-iommu binding each
> entry is really five cells long.
>
> This used to work only because the DTs were themselves broken. They
> described iommu-map with four-cell entries that omitted the SID mask,
> which of_map_id() tolerated via its of_check_bad_map() fallback, and the
> four-word parsing coincidentally matched that malformed shape.
>
> Since commit ccb2fd725d41 ("of: Respect #{iommu,msi}-cells in maps") the
> OF core parses such maps correctly, so the device trees were converted
> to proper five-cell entries, e.g. commit c41749e9554d ("arm64: dts:
> qcom: sm8250: Fix the PCIe iommu-map entries"). With five-cell entries
> the fixed four-word stride slips by one cell for each entry after the
> first, so qcom_pcie_config_sid_1_9_0() reads the endpoint mapping's RID
> from the preceding entry's length cell and its SID from the phandle cell.
> As the RID is the hash key, the endpoint's real RID is never programmed
> into the BDF to SID table. Its DMA then hashes to an unprogrammed slot,
> gets tagged with SID 0 and the SMMU faults like below on QCS8300:
>
>   arm-smmu 15200000.iommu: Unhandled context fault: fsr=0x402, iova=0xffa00000, cbfrsynra=0x0, cb=1
>
> To fix this, walk the map with a stride of 3 + '#iommu-cells' of the
> referenced IOMMU and take the SID from the first specifier cell, which is
> all the BDF to SID table needs. Validate the layout instead of trusting
> the array size. Also, preserve the legacy behavior of the old DTs by
> detecting the same pattern that of_check_bad_map() recognizes and
> falling back to a stride of four.
>
> Fixes: 4c9398822106 ("PCI: qcom: Add support for configuring BDF to SID mapping for SM8250")
> Reported-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

Arduino Monza (Monaco) ATH11k/WiFi was broken for me. This fixes the issue.

Tested-by: Loic Poulain <loic.poulain@oss.qualcomm.com>


> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 90 ++++++++++++++++++--------
>  1 file changed, 64 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index b58a607b713f..42cbeef083ca 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1143,50 +1143,90 @@ static void qcom_pcie_deinit_2_7_0(struct qcom_pcie *pcie)
>
>  static int qcom_pcie_config_sid_1_9_0(struct qcom_pcie *pcie)
>  {
> -       /* iommu map structure */
> -       struct {
> -               u32 bdf;
> -               u32 phandle;
> -               u32 smmu_sid;
> -               u32 smmu_sid_len;
> -       } *map;
>         void __iomem *bdf_to_sid_base = pcie->parf + PARF_BDF_TO_SID_TABLE_N;
>         struct device *dev = pcie->pci->dev;
> +       struct device_node *iommu_np;
>         u8 qcom_pcie_crc8_table[CRC8_TABLE_SIZE];
> -       int i, nr_map, size = 0;
> -       u32 smmu_sid_base;
> +       const __be32 *map;
> +       u32 iommu_cells, entry_cells, phandle, smmu_sid_base;
> +       int i, nr_cells, nr_map, size = 0;
>         u32 val;
>
> -       of_get_property(dev->of_node, "iommu-map", &size);
> -       if (!size)
> +       map = of_get_property(dev->of_node, "iommu-map", &size);
> +       if (!map || !size)
>                 return 0;
>
> +       if (size % sizeof(*map)) {
> +               dev_err(dev, "Malformed iommu-map property\n");
> +               return -EINVAL;
> +       }
> +       nr_cells = size / sizeof(*map);
> +
> +       /*
> +        * Each iommu-map entry is: rid-base (1 cell), phandle (1 cell),
> +        * IOMMU specifier (#iommu-cells cells), length (1 cell). Read
> +        * #iommu-cells from the IOMMU provider referenced by the first
> +        * entry to compute the per-entry stride.
> +        */
> +       phandle = be32_to_cpu(map[1]);
> +       iommu_np = of_find_node_by_phandle(phandle);
> +       if (!iommu_np) {
> +               dev_err(dev, "Failed to find IOMMU node in iommu-map\n");
> +               return -ENODEV;
> +       }
> +
> +       if (of_property_read_u32(iommu_np, "#iommu-cells", &iommu_cells))
> +               iommu_cells = 1;
> +       of_node_put(iommu_np);
> +
> +       entry_cells = 3 + iommu_cells;
> +
> +       /*
> +        * Retain backward compatibility with DTs that describe iommu-map
> +        * with 4-cell entries against an IOMMU declaring #iommu-cells = 2,
> +        * matching the fallback in drivers/of/base.c::of_check_bad_map().
> +        */
> +       if (iommu_cells == 2 && !(nr_cells % 4)) {
> +               bool legacy = true;
> +
> +               for (i = 0; i < nr_cells; i += 4) {
> +                       if (be32_to_cpu(map[i + 1]) != phandle ||
> +                           be32_to_cpu(map[i + 3]) != 1) {
> +                               legacy = false;
> +                               break;
> +                       }
> +               }
> +
> +               if (legacy) {
> +                       dev_warn_once(dev, "iommu-map has 1-cell entries targeting 2-cell #iommu-cells, treating as 1-cell output\n");
> +                       entry_cells = 4;
> +               }
> +       }
> +
> +       if (nr_cells % entry_cells) {
> +               dev_err(dev, "Malformed iommu-map property\n");
> +               return -EINVAL;
> +       }
> +       nr_map = nr_cells / entry_cells;
> +
>         /* Enable BDF to SID translation by disabling bypass mode (default) */
>         val = readl(pcie->parf + PARF_BDF_TO_SID_CFG);
>         val &= ~BDF_TO_SID_BYPASS;
>         writel(val, pcie->parf + PARF_BDF_TO_SID_CFG);
>
> -       map = kzalloc(size, GFP_KERNEL);
> -       if (!map)
> -               return -ENOMEM;
> -
> -       of_property_read_u32_array(dev->of_node, "iommu-map", (u32 *)map,
> -                                  size / sizeof(u32));
> -
> -       nr_map = size / (sizeof(*map));
> -
>         crc8_populate_msb(qcom_pcie_crc8_table, QCOM_PCIE_CRC8_POLYNOMIAL);
>
>         /* Registers need to be zero out first */
>         memset_io(bdf_to_sid_base, 0, CRC8_TABLE_SIZE * sizeof(u32));
>
>         /* Extract the SMMU SID base from the first entry of iommu-map */
> -       smmu_sid_base = map[0].smmu_sid;
> +       smmu_sid_base = be32_to_cpu(map[2]);
>
>         /* Look for an available entry to hold the mapping */
>         for (i = 0; i < nr_map; i++) {
> -               __be16 bdf_be = cpu_to_be16(map[i].bdf);
> -               u32 val;
> +               u32 bdf = be32_to_cpu(map[i * entry_cells]);
> +               u32 sid = be32_to_cpu(map[i * entry_cells + 2]);
> +               __be16 bdf_be = cpu_to_be16(bdf);
>                 u8 hash;
>
>                 hash = crc8(qcom_pcie_crc8_table, (u8 *)&bdf_be, sizeof(bdf_be), 0);
> @@ -1208,12 +1248,10 @@ static int qcom_pcie_config_sid_1_9_0(struct qcom_pcie *pcie)
>                 }
>
>                 /* BDF [31:16] | SID [15:8] | NEXT [7:0] */
> -               val = map[i].bdf << 16 | (map[i].smmu_sid - smmu_sid_base) << 8 | 0;
> +               val = bdf << 16 | (sid - smmu_sid_base) << 8 | 0;
>                 writel(val, bdf_to_sid_base + hash * sizeof(u32));
>         }
>
> -       kfree(map);
> -
>         return 0;
>  }
>
> --
> 2.43.0
>

Re: [PATCH] PCI: qcom: Honor IOMMU provider's #iommu-cells in qcom_pcie_config_sid_1_9_0()

[Neil Armstrong]

On 9/7/26 16:33, Manivannan Sadhasivam wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> 
> qcom_pcie_config_sid_1_9_0() reads the "iommu-map" property as an array
> of fixed four-word {rid-base, phandle, sid, rid-len} entries to program
> the BDF to SID translation table. But that layout only holds for an
> IOMMU with '#iommu-cells = <1>'. The PCIe SMMUs on these SoCs use
> '#iommu-cells = <2>' (SID and mask), so per the pci-iommu binding each
> entry is really five cells long.
> 
> This used to work only because the DTs were themselves broken. They
> described iommu-map with four-cell entries that omitted the SID mask,
> which of_map_id() tolerated via its of_check_bad_map() fallback, and the
> four-word parsing coincidentally matched that malformed shape.
> 
> Since commit ccb2fd725d41 ("of: Respect #{iommu,msi}-cells in maps") the
> OF core parses such maps correctly, so the device trees were converted
> to proper five-cell entries, e.g. commit c41749e9554d ("arm64: dts:
> qcom: sm8250: Fix the PCIe iommu-map entries"). With five-cell entries
> the fixed four-word stride slips by one cell for each entry after the
> first, so qcom_pcie_config_sid_1_9_0() reads the endpoint mapping's RID
> from the preceding entry's length cell and its SID from the phandle cell.
> As the RID is the hash key, the endpoint's real RID is never programmed
> into the BDF to SID table. Its DMA then hashes to an unprogrammed slot,
> gets tagged with SID 0 and the SMMU faults like below on QCS8300:
> 
>    arm-smmu 15200000.iommu: Unhandled context fault: fsr=0x402, iova=0xffa00000, cbfrsynra=0x0, cb=1
> 
> To fix this, walk the map with a stride of 3 + '#iommu-cells' of the
> referenced IOMMU and take the SID from the first specifier cell, which is
> all the BDF to SID table needs. Validate the layout instead of trusting
> the array size. Also, preserve the legacy behavior of the old DTs by
> detecting the same pattern that of_check_bad_map() recognizes and
> falling back to a stride of four.
> 
> Fixes: 4c9398822106 ("PCI: qcom: Add support for configuring BDF to SID mapping for SM8250")
> Reported-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>   drivers/pci/controller/dwc/pcie-qcom.c | 90 ++++++++++++++++++--------
>   1 file changed, 64 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index b58a607b713f..42cbeef083ca 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1143,50 +1143,90 @@ static void qcom_pcie_deinit_2_7_0(struct qcom_pcie *pcie)
>   
>   static int qcom_pcie_config_sid_1_9_0(struct qcom_pcie *pcie)
>   {
> -	/* iommu map structure */
> -	struct {
> -		u32 bdf;
> -		u32 phandle;
> -		u32 smmu_sid;
> -		u32 smmu_sid_len;
> -	} *map;
>   	void __iomem *bdf_to_sid_base = pcie->parf + PARF_BDF_TO_SID_TABLE_N;
>   	struct device *dev = pcie->pci->dev;
> +	struct device_node *iommu_np;
>   	u8 qcom_pcie_crc8_table[CRC8_TABLE_SIZE];
> -	int i, nr_map, size = 0;
> -	u32 smmu_sid_base;
> +	const __be32 *map;
> +	u32 iommu_cells, entry_cells, phandle, smmu_sid_base;
> +	int i, nr_cells, nr_map, size = 0;
>   	u32 val;
>   
> -	of_get_property(dev->of_node, "iommu-map", &size);
> -	if (!size)
> +	map = of_get_property(dev->of_node, "iommu-map", &size);
> +	if (!map || !size)
>   		return 0;
>   
> +	if (size % sizeof(*map)) {
> +		dev_err(dev, "Malformed iommu-map property\n");
> +		return -EINVAL;
> +	}
> +	nr_cells = size / sizeof(*map);
> +
> +	/*
> +	 * Each iommu-map entry is: rid-base (1 cell), phandle (1 cell),
> +	 * IOMMU specifier (#iommu-cells cells), length (1 cell). Read
> +	 * #iommu-cells from the IOMMU provider referenced by the first
> +	 * entry to compute the per-entry stride.
> +	 */
> +	phandle = be32_to_cpu(map[1]);
> +	iommu_np = of_find_node_by_phandle(phandle);
> +	if (!iommu_np) {
> +		dev_err(dev, "Failed to find IOMMU node in iommu-map\n");
> +		return -ENODEV;
> +	}
> +
> +	if (of_property_read_u32(iommu_np, "#iommu-cells", &iommu_cells))
> +		iommu_cells = 1;
> +	of_node_put(iommu_np);
> +
> +	entry_cells = 3 + iommu_cells;
> +
> +	/*
> +	 * Retain backward compatibility with DTs that describe iommu-map
> +	 * with 4-cell entries against an IOMMU declaring #iommu-cells = 2,
> +	 * matching the fallback in drivers/of/base.c::of_check_bad_map().
> +	 */
> +	if (iommu_cells == 2 && !(nr_cells % 4)) {
> +		bool legacy = true;
> +
> +		for (i = 0; i < nr_cells; i += 4) {
> +			if (be32_to_cpu(map[i + 1]) != phandle ||
> +			    be32_to_cpu(map[i + 3]) != 1) {
> +				legacy = false;
> +				break;
> +			}
> +		}
> +
> +		if (legacy) {
> +			dev_warn_once(dev, "iommu-map has 1-cell entries targeting 2-cell #iommu-cells, treating as 1-cell output\n");
> +			entry_cells = 4;
> +		}
> +	}
> +
> +	if (nr_cells % entry_cells) {
> +		dev_err(dev, "Malformed iommu-map property\n");
> +		return -EINVAL;
> +	}
> +	nr_map = nr_cells / entry_cells;
> +
>   	/* Enable BDF to SID translation by disabling bypass mode (default) */
>   	val = readl(pcie->parf + PARF_BDF_TO_SID_CFG);
>   	val &= ~BDF_TO_SID_BYPASS;
>   	writel(val, pcie->parf + PARF_BDF_TO_SID_CFG);
>   
> -	map = kzalloc(size, GFP_KERNEL);
> -	if (!map)
> -		return -ENOMEM;
> -
> -	of_property_read_u32_array(dev->of_node, "iommu-map", (u32 *)map,
> -				   size / sizeof(u32));
> -
> -	nr_map = size / (sizeof(*map));
> -
>   	crc8_populate_msb(qcom_pcie_crc8_table, QCOM_PCIE_CRC8_POLYNOMIAL);
>   
>   	/* Registers need to be zero out first */
>   	memset_io(bdf_to_sid_base, 0, CRC8_TABLE_SIZE * sizeof(u32));
>   
>   	/* Extract the SMMU SID base from the first entry of iommu-map */
> -	smmu_sid_base = map[0].smmu_sid;
> +	smmu_sid_base = be32_to_cpu(map[2]);
>   
>   	/* Look for an available entry to hold the mapping */
>   	for (i = 0; i < nr_map; i++) {
> -		__be16 bdf_be = cpu_to_be16(map[i].bdf);
> -		u32 val;
> +		u32 bdf = be32_to_cpu(map[i * entry_cells]);
> +		u32 sid = be32_to_cpu(map[i * entry_cells + 2]);
> +		__be16 bdf_be = cpu_to_be16(bdf);
>   		u8 hash;
>   
>   		hash = crc8(qcom_pcie_crc8_table, (u8 *)&bdf_be, sizeof(bdf_be), 0);
> @@ -1208,12 +1248,10 @@ static int qcom_pcie_config_sid_1_9_0(struct qcom_pcie *pcie)
>   		}
>   
>   		/* BDF [31:16] | SID [15:8] | NEXT [7:0] */
> -		val = map[i].bdf << 16 | (map[i].smmu_sid - smmu_sid_base) << 8 | 0;
> +		val = bdf << 16 | (sid - smmu_sid_base) << 8 | 0;
>   		writel(val, bdf_to_sid_base + hash * sizeof(u32));
>   	}
>   
> -	kfree(map);
> -
>   	return 0;
>   }
>   

Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-HDK

Thanks,
Neil

Re: [PATCH] PCI: qcom: Honor IOMMU provider's #iommu-cells in qcom_pcie_config_sid_1_9_0()

[Konrad Dybcio]

On 9/7/26 4:33 PM, Manivannan Sadhasivam wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> 
> qcom_pcie_config_sid_1_9_0() reads the "iommu-map" property as an array
> of fixed four-word {rid-base, phandle, sid, rid-len} entries to program
> the BDF to SID translation table. But that layout only holds for an
> IOMMU with '#iommu-cells = <1>'. The PCIe SMMUs on these SoCs use
> '#iommu-cells = <2>' (SID and mask), so per the pci-iommu binding each
> entry is really five cells long.

[...]

> +	/*
> +	 * Retain backward compatibility with DTs that describe iommu-map
> +	 * with 4-cell entries against an IOMMU declaring #iommu-cells = 2,
> +	 * matching the fallback in drivers/of/base.c::of_check_bad_map().

Can we export it from there instead?
Perhaps as a follow-up change given this is a fix to a current
breakage..

I was hoping this whole logic could be simplified a bit by reusing
the map parsing logic from of/property.c, but alas it doesn't seem
feasible given we essentially need more data

Konrad