Re: [PATCH 12/13] PCI: brcmstb: Use order-0 indexing for inbound BAR window array

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The shift from 1-based to 0-based array indexing for `inbound_wins` broke the MSI target address calculation. `brcm_pcie_setup()` was not updated and continues to evaluate the hardcoded index 2, which now points to a 0-sized disabled window (or uninitialized memory), forcing an incorrect MSI target address.
--

commit cc21218055610569186c9058228a2b11645c3501
Author: Jim Quinlan <james.quinlan@broadcom.com>

PCI: brcmstb: Use order-0 indexing for inbound BAR window array

The register names for the inbound BAR registers use order-1 indexing,
e.g. the first is BAR1. The code followed this nomenclature by not using
the 0th element of the inbound array. This commit undoes this approach
and uses order-0 indexing.

> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 8bca98e6f5fa4..53984cfa2c96f 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
[ ... ]
> @@ -1202,11 +1195,11 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
>  	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE_MASK);
>  	writel(tmp, base + PCIE_MISC_MISC_CTRL);
>  
> -	num_inbound_wins = brcm_pcie_get_inbound_wins(pcie, inbound_wins);
> +	num_inbound_wins = brcm_pcie_get_ib_wins(pcie, inbound_wins);

[Severity: High]
By changing the inbound_wins array indexing from 1-based to 0-based, the main
inbound window for non-7712 chips has shifted from index 2 to index 1.

Does this break the MSI target address configuration further down in
brcm_pcie_setup()?

Later in brcm_pcie_setup(), the code still unconditionally evaluates index 2:

	if (inbound_wins[2].pci_offset >= SZ_4G ||
	    (inbound_wins[2].size + inbound_wins[2].pci_offset) < SZ_4G)
		pcie->msi_target_addr = BRCM_MSI_TARGET_ADDR_LT_4GB;
	else
		pcie->msi_target_addr = BRCM_MSI_TARGET_ADDR_GT_4GB;

For non-7712 chips, inbound_wins[2] will now point to the disabled zero-size
window, forcing the condition to always evaluate to true. This incorrectly
forces BRCM_MSI_TARGET_ADDR_LT_4GB and breaks >4GB MSI allocation.

Additionally, for BCM7712 chips with fewer than three dma-ranges,
inbound_wins[2] is never initialized and will result in an uninitialized stack
memory read. This issue remains unfixed in subsequent patches of the series.

>  	if (num_inbound_wins < 0)
>  		return num_inbound_wins;
>  
> -	set_inbound_win_registers(pcie, inbound_wins, num_inbound_wins);
> +	brcm_pcie_set_ib_win_registers(pcie, inbound_wins, num_inbound_wins);
>  
>  	if (!brcm_pcie_rc_mode(pcie)) {
>  		dev_err(pcie->dev, "PCIe RC controller misconfigured as Endpoint\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911233541.1650895-1-james.quinlan@broadcom.com?part=12