Re: [PATCH v3 01/16] PCI: dwc: Add pcie_cap field and helper in designware header

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Data race and logically flawed caching in dw_pcie_get_pcie_cap() due to unprotected read-modify-write and failure to cache 'not found' state.
--

--- Patch [1]: [PATCH v3 01/16] PCI: dwc: Add pcie_cap field and helper in designware header ---
commit ab22dc6f534d77b34df13c99dd929771547f5ff1
Author: Hans Zhang <[email protected]>

PCI: dwc: Add pcie_cap field and helper in designware header

This commit adds a pcie_cap field to struct dw_pcie to cache the offset of
the PCI Express Capability structure. It introduces dw_pcie_get_pcie_cap()
to perform the capability search on the first call and cache the result.

> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
[ ... ]
> @@ -829,6 +831,21 @@ static inline void dw_pcie_dbi_ro_wr_dis(struct dw_pcie *pci)
>  	dw_pcie_writel_dbi(pci, reg, val);
>  }
>  
> +/**
> + * dw_pcie_get_pcie_cap() - Return cached PCIe Capability offset
> + * @pci: DWC instance
> + *
> + * Finds and caches the offset of PCI_CAP_ID_EXP on first call.
> + * Returns 0 if the capability is not present.
> + */
> +static inline u8 dw_pcie_get_pcie_cap(struct dw_pcie *pci)
> +{
> +	if (!pci->pcie_cap)
> +		pci->pcie_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);

[Severity: Medium]
Will this repeatedly search the configuration space if the capability is
not found? If dw_pcie_find_capability() returns 0 when the capability is
missing, pci->pcie_cap will remain 0, causing the search to be repeated
on every subsequent call instead of being cached.

Also, since this helper might be called concurrently by different threads,
could the unprotected read and write to pci->pcie_cap cause a data race?

> +
> +	return pci->pcie_cap;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic