Re: [PATCH v2 11/16] PCI: dwc: qcom: Use cached PCIe capability offset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Uninitialized cached PCIe capability offset causes memory corruption in PCI configuration space.
--

commit 1d7762e21cba69c340ccc430485c442e06488028
Author: Hans Zhang <18255117159@163.com>

PCI: dwc: qcom: Use cached PCIe capability offset

This patch replaces calls to dw_pcie_find_capability() with the cached
pcie_cap offset in the designware PCIe driver for Qualcomm. It aims to
optimize access to PCIe capabilities by reusing the offset.

>     For .init itself, we must call dw_pcie_get_pcie_cap() inside
>     qcom_pcie_host_init() to obtain the offset (hardware is already enabled).

[Severity: High]
Does the patch actually make this call? It appears this was left out of the
code changes.

Without this call, pci->pcie_cap remains uninitialized (0) when the
.post_init callbacks run.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 11fc604898926..80783353d5398 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c

[ ... ]

> @@ -370,9 +369,9 @@ static void qcom_pcie_set_slot_nccs(struct dw_pcie *pci)
>  	 * notifications for the Hot-Plug commands. So set the NCCS field to
>  	 * avoid waiting for the completions.
>  	 */
> -	val = readl(pci->dbi_base + offset + PCI_EXP_SLTCAP);
> +	val = readl(pci->dbi_base + pci->pcie_cap + PCI_EXP_SLTCAP);
>  	val |= PCI_EXP_SLTCAP_NCCS;
> -	writel(val, pci->dbi_base + offset + PCI_EXP_SLTCAP);
> +	writel(val, pci->dbi_base + pci->pcie_cap + PCI_EXP_SLTCAP);

[Severity: High]
Will pci->pcie_cap be 0 here?

Since dw_pcie_get_pcie_cap() is not called prior to this, pci->pcie_cap
evaluates to 0. This causes the code to write to offset 0 + PCI_EXP_SLTCAP
(0x14, which is PCI_BASE_ADDRESS_1) instead of the actual capability,
corrupting the Root Port hardware configuration.

[ ... ]

> @@ -935,7 +934,7 @@ static int qcom_pcie_init_2_3_3(struct qcom_pcie *pcie)
>  static int qcom_pcie_post_init_2_3_3(struct qcom_pcie *pcie)
>  {
>  	struct dw_pcie *pci = pcie->pci;
> -	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +	u8 offset = pci->pcie_cap;

[Severity: High]
Does this also suffer from the uninitialized pci->pcie_cap?

Because offset becomes 0, subsequent writes using this offset (like writing
PCIE_CAP_SLOT_VAL to PCI_EXP_SLTCAP) will also corrupt BAR1 instead of updating
the PCIe capability register.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530153101.695580-1-18255117159@163.com?part=11