From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 71F6B34C80D for ; Sun, 3 May 2026 16:52:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777827164; cv=none; b=sA4msISlXBtljflC8UdgfCmareqMoV/uuz+eAS3IQ7AGxpow0qdiUpBNqsCBspxM6YwmfhdA6NS8u6ZYpO6X/pg1KMlVoZ3XnueD6LyrI9iu5cMV9EPNbtPM0NfZ7ngCiHKWwpo/J4tAiiI+ahINaqKK7I0NUnNmgaIS+M5g5Gs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777827164; c=relaxed/simple; bh=c9hEDcaBbMm+9MIAJ2OXdE0ANuWKycOgxmrQkGpzH1k=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=UkBPp4lDeqFrhFq5D7wGf+oseCI7majx1/2qTUcZE9sOx2Kbbmch4nzggAXKnt61QM+b5+RQRQHTEcl3MNg+8tk0fYQXWTIRt5UkxuCJ/0DRsdL0AF9K5rdDBUYFaKywSUB3ULRIuyq54dxp0HMqEpTteWCSHBXcnMjbfunuPiU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=fIDcM2mk; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="fIDcM2mk" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4A676C2BCB4; Sun, 3 May 2026 16:52:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1777827163; bh=c9hEDcaBbMm+9MIAJ2OXdE0ANuWKycOgxmrQkGpzH1k=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From; b=fIDcM2mkvjJLA02k8kx+rZTBNdsuktdGfA4OrYlZOpdtRjdR1PP+my7cCI5c2rhPR zXmQ9w6DZsdb2KYvRV7C+JS4B1ufO1gQSudOEVxDCnQOht58PhuZVmxRH+Zwx87xa5 JtH3AiNzrM8dXyb+Cp8jCbIcYcOU5CRRlVMu89I19Vxik/FzLL3iqqrITMkT4ObPNH TFrKqAW4hSYB4YT8vWL5VhcF4UnZNXNT/66UZs0zUzS5BV7BH9MpiVgAEfHNsvW7VI 6bhdz/Dqm0tP69p/GJyE28gB2mQT8vmrGZucYgzjbs+4i3i8MXJhYizecfhYE9VXtb DeKjN35BDFkPQ== From: sashiko-bot@kernel.org Subject: Re: [PATCH] PCI: cadence: Use cdns_pcie_find_capability() to get PCIe Cap offset in host driver Reply-To: sashiko@lists.linux.dev To: "Hans Zhang" <18255117159@163.com> Cc: linux-pci@vger.kernel.org In-Reply-To: <20260503161913.77878-1-18255117159@163.com> References: <20260503161913.77878-1-18255117159@163.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Sun, 03 May 2026 16:52:42 +0000 Message-Id: <20260503165243.4A676C2BCB4@smtp.kernel.org> Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Thank you for your contribution! Sashiko AI review found 1 potential issue(= s) to consider: - [Critical] cdns_pcie_find_capability() searches the wrong register space = in Host mode and its return value is not validated, leading to Root Port co= nfiguration space corruption. -- commit 09b3c64b8f5892eb365acd13030a8067652a15e8 Author: Hans Zhang <18255117159@163.com> PCI: cadence: Use cdns_pcie_find_capability() to get PCIe Cap offset in hos= t driver This commit replaces the hardcoded CDNS_PCIE_RP_CAP_OFFSET with a dynamic lookup using cdns_pcie_find_capability() in Cadence PCIe host-related functions. The goal is to ensure correct operation across various SoC desig= ns where the capability might not reside at offset 0xC0. > diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-common.c b/= drivers/pci/controller/cadence/pcie-cadence-host-common.c > index 2b0211870f02..5013d3fe23b5 100644 > --- a/drivers/pci/controller/cadence/pcie-cadence-host-common.c > +++ b/drivers/pci/controller/cadence/pcie-cadence-host-common.c > @@ -26,14 +26,14 @@ EXPORT_SYMBOL_GPL(bar_max_size); > =20 > int cdns_pcie_host_training_complete(struct cdns_pcie *pcie) > { > - u32 pcie_cap_off =3D CDNS_PCIE_RP_CAP_OFFSET; > unsigned long end_jiffies; > u16 lnk_stat; > + u8 cap =3D cdns_pcie_find_capability(pcie, PCI_CAP_ID_EXP); Does this capability search scan the correct register space when operating = in host mode? Looking at the helper function, it uses cdns_pcie_read_cfg() which reads directly from the base address without the Root Port offset: drivers/pci/controller/cadence/pcie-cadence.c:cdns_pcie_find_capability() { return PCI_FIND_NEXT_CAP(cdns_pcie_read_cfg, PCI_CAPABILITY_LIST, cap, NULL, pcie); } However, the Root Port configuration registers are physically located at pcie->reg_base + CDNS_PCIE_RP_BASE. Could this cause the search to scan the Local Management register space instead, failing to find PCI_CAP_ID_EXP and returning 0? [ ... ] > @@ -68,27 +68,26 @@ EXPORT_SYMBOL_GPL(cdns_pcie_host_wait_for_link); > int cdns_pcie_retrain(struct cdns_pcie *pcie, > cdns_pcie_linkup_func pcie_link_up) > { > - u32 lnk_cap_sls, pcie_cap_off =3D CDNS_PCIE_RP_CAP_OFFSET; > + u32 lnk_cap_sls; > u16 lnk_stat, lnk_ctl; > int ret =3D 0; > + u8 cap =3D cdns_pcie_find_capability(pcie, PCI_CAP_ID_EXP); > =20 > /* > * Set retrain bit if current speed is 2.5 GB/s, > * but the PCIe root port support is > 2.5 GB/s. > */ > =20 > - lnk_cap_sls =3D cdns_pcie_readl(pcie, (CDNS_PCIE_RP_BASE + pcie_cap_off= + > + lnk_cap_sls =3D cdns_pcie_readl(pcie, (CDNS_PCIE_RP_BASE + cap + > PCI_EXP_LNKCAP)); If the capability is not found and cap evaluates to 0, is it safe to proceed without validating the return value? Since the fallback value would be 0, evaluating cap + PCI_EXP_LNKCAP will result in an offset of 0x0C. [ ... ] > diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers= /pci/controller/cadence/pcie-cadence-host.c > index 0bc9e6e90e0e..2a3fd41c1cf4 100644 > --- a/drivers/pci/controller/cadence/pcie-cadence-host.c > +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c > @@ -147,12 +148,12 @@ static int cdns_pcie_host_init_root_port(struct cdn= s_pcie_rc *rc) > cdns_pcie_rp_writeb(pcie, PCI_CLASS_PROG, 0); > cdns_pcie_rp_writew(pcie, PCI_CLASS_DEVICE, PCI_CLASS_BRIDGE_PCI); > =20 > - value =3D cdns_pcie_rp_readl(pcie, CDNS_PCIE_RP_CAP_OFFSET + PCI_EXP_LN= KCAP); > + value =3D cdns_pcie_rp_readl(pcie, cap + PCI_EXP_LNKCAP); > if (rc->quirk_broken_aspm_l0s) > value &=3D ~PCI_EXP_LNKCAP_ASPM_L0S; > if (rc->quirk_broken_aspm_l1) > value &=3D ~PCI_EXP_LNKCAP_ASPM_L1; > - cdns_pcie_rp_writel(pcie, CDNS_PCIE_RP_CAP_OFFSET + PCI_EXP_LNKCAP, val= ue); > + cdns_pcie_rp_writel(pcie, cap + PCI_EXP_LNKCAP, value); Will this overwrite the wrong configuration registers if cap is 0? The cdns_pcie_rp_writel() function automatically adds CDNS_PCIE_RP_BASE to the destination offset. If cap is 0, writing to cap + PCI_EXP_LNKCAP will access offset 0x0C. Could this regression unintentionally clobber the standard Type 1 PCI header registers, such as the Cache Line Size and Latency Timer, and corrupt the Root Port hardware state? --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260503161913.7787= 8-1-18255117159@163.com?part=3D1