Re: [PATCH v2 04/16] PCI: dwc: layerscape-ep: Use cached PCIe capability offset

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Sleeping in atomic context due to mutex_lock() inside hardirq handler
- [High] Race condition in DBI register access leading to silent PCIe configuration failures
--

commit 68e2e619ed44719a7c9ec1273ed94120c179ed09
Author: Hans Zhang <18255117159@163.com>

PCI: dwc: layerscape-ep: Use cached PCIe capability offset
    
Replace these with dw_pcie_get_pcie_cap(). The hardware is already enabled
by the driver before ls_pcie_ep_probe() and before the interrupt handler
runs. dw_pcie_get_pcie_cap() will cache the offset on first call, and
subsequent calls (including inside dw_pcie_ep_init) will use the cached
value without re-searching.

> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index 8936975ff104e..b2d0b51df3c53 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -83,9 +82,6 @@ static irqreturn_t ls_pcie_ep_event_handler(int irq, void *dev_id)
>  		return IRQ_NONE;
>  
>  	if (val & PEX_PF0_PME_MES_DR_LUD) {
> -
> -		offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> -
>  		/*
>  		 * The values of the Maximum Link Width and Supported Link
>  		 * Speed from the Link Capabilities Register will be lost
> @@ -93,7 +89,8 @@ static irqreturn_t ls_pcie_ep_event_handler(int irq, void *dev_id)
>  		 * that configured by the Reset Configuration Word (RCW).
>  		 */
>  		dw_pcie_dbi_ro_wr_en(pci);
> -		dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, pcie->lnkcap);
> +		dw_pcie_writel_dbi(pci, pci->pcie_cap + PCI_EXP_LNKCAP,
> +				   pcie->lnkcap);
>  		dw_pcie_dbi_ro_wr_dis(pci);

[Severity: High]
This isn't a bug introduced by this patch, but does using
dw_pcie_dbi_ro_wr_en() and dw_pcie_dbi_ro_wr_dis() in hardirq context
here create a race condition with process-context operations?

Since dw_pcie_dbi_ro_wr_dis() unconditionally clears PCIE_DBI_RO_WR_EN:

static inline void dw_pcie_dbi_ro_wr_dis(struct dw_pcie *pci)
{
	...
	val &= ~PCIE_DBI_RO_WR_EN;
	dw_pcie_writel_dbi(pci, reg, val);
}

If this hardirq interrupts a process-context operation (like
dw_pcie_ep_set_bar_programmable()) right after it enabled RO_WR_EN but
before it writes to a DBI register, will this unconditionally clear the
flag, causing the interrupted thread's subsequent writes to be silently
ignored by the hardware?

>  
>  		cfg = ls_pcie_pf_lut_readl(pcie, PEX_PF0_CONFIG);

[Severity: High]
This is a pre-existing issue, but does the code in this same hardirq
handler sleep in atomic context?

Shortly after this point in ls_pcie_ep_event_handler(), the code calls:

	dw_pcie_ep_linkup(&pci->ep);

Which invokes pci_epc_linkup() in the EPC core, doing:

void pci_epc_linkup(struct pci_epc *epc)
{
	mutex_lock(&epc->list_lock);
	...
}

Will attempting to acquire this sleepable mutex from hardirq context
trigger a BUG for scheduling while atomic?

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