[PATCH v3 16/16] PCI: dwc: ultrarisc: Use cached PCIe capability offset
sashiko-bot at kernel.org
sashiko-bot at kernel.org
Mon Jul 20 08:33:31 PDT 2026
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Critical] In Endpoint (EP) mode, `pci->pcie_cap` remains uninitialized (0), causing `dw_pcie_setup()` to silently corrupt standard PCI header registers.
Pre-existing issues:
- [High] 32-bit read-modify-write (RMW) of 16-bit PCIe Control registers accidentally clears Write-1-to-Clear (W1C) bits in the adjacent Status registers.
--
--- Patch [16]: [PATCH v3 16/16] PCI: dwc: ultrarisc: Use cached PCIe capability offset ---
commit 4d9321bed75fe3c10535cf01ea0a69ce527cb616
Author: Hans Zhang <18255117159 at 163.com>
PCI: dwc: ultrarisc: Use cached PCIe capability offset
This commit updates ultrarisc_pcie_host_init to use dw_pcie_get_pcie_cap
to obtain the PCIe capability offset, as the core has not cached it yet.
Hardware is already enabled before this point.
> diff --git a/drivers/pci/controller/dwc/pcie-ultrarisc.c b/drivers/pci/controller/dwc/pcie-ultrarisc.c
> index 6ee661ceff67a..3f6e3ab0aa697 100644
> --- a/drivers/pci/controller/dwc/pcie-ultrarisc.c
> +++ b/drivers/pci/controller/dwc/pcie-ultrarisc.c
> @@ -49,7 +49,7 @@ static int ultrarisc_pcie_host_init(struct dw_pcie_rp *pp)
> FIELD_MODIFY(PORT_FLT_SF_MASK, &val, PORT_FLT_SF_VAL_64);
> dw_pcie_writel_dbi(pci, PCIE_TIMER_CTRL_MAX_FUNC_NUM, val);
>
> - cap_exp = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> + cap_exp = dw_pcie_get_pcie_cap(pci);
[Severity: Critical]
Does caching the capability offset here expose an uninitialized value
in Endpoint mode?
If the broader patch series replaced dw_pcie_find_capability with direct
reads of pci->pcie_cap in the EP paths without initializing it,
pci->pcie_cap could remain 0.
For example, in drivers/pci/controller/dwc/pcie-designware-ep.c:
dw_pcie_ep_init_non_sticky_registers() {
if (funcs > 1) {
offset = pci->pcie_cap;
func0_lnkcap = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
[ ... ]
}
If dw_pcie_get_pcie_cap() is not called before this in the EP
initialization sequence, offset evaluates to 0. This causes
dw_pcie_setup() to write into standard PCI header registers like Cache
Line Size or BIST instead of the PCIe capability space, silently
corrupting the header.
> val = dw_pcie_readl_dbi(pci, cap_exp + PCI_EXP_LNKCTL2);
> FIELD_MODIFY(PCI_EXP_LNKCTL2_TLS, &val, PCI_EXP_LNKCTL2_TLS_16_0GT);
> dw_pcie_writel_dbi(pci, cap_exp + PCI_EXP_LNKCTL2, val);
[Severity: High]
This is a pre-existing issue, but does this 32-bit read-modify-write
sequence unintentionally clear Write-1-to-Clear bits in the adjacent
Status register?
When ultrarisc_pcie_host_init() reads and writes 32 bits starting at
PCI_EXP_LNKCTL2:
drivers/pci/controller/dwc/pcie-ultrarisc.c:ultrarisc_pcie_host_init()
val = dw_pcie_readl_dbi(pci, cap_exp + PCI_EXP_LNKCTL2);
FIELD_MODIFY(PCI_EXP_LNKCTL2_TLS, &val, PCI_EXP_LNKCTL2_TLS_16_0GT);
dw_pcie_writel_dbi(pci, cap_exp + PCI_EXP_LNKCTL2, val);
The read includes the 16-bit Link Control 2 register and the adjacent
16-bit Link Status 2 register. If any W1C bits are set in the status
register, writing the 32-bit value back will clear those events. This
can drop hardware events like Link Equalization Requests or Device Errors,
potentially breaking link training or Advanced Error Reporting.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720150619.24733-1-18255117159@163.com?part=16
More information about the linux-amlogic
mailing list