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@xxxxxxxxxxxxxxxx> wrote:
>
> From: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxx>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>

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

Tested-by: Loic Poulain <loic.poulain@xxxxxxxxxxxxxxxx>


> ---
>  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
>