Re: [PATCH v10 3/5] PCI: tegra: Add Tegra264 support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Incorrect register offset for `XTL_RC_PCIE_CFG_LINK_CAPS` causes `readw()` to fetch the wrong bits, leading to bogus link capabilities extraction.
- [High] Unvalidated PCIe Link Speed index extracted from hardware registers leads to an out-of-bounds read of the `pcie_link_speed` array.
- [Medium] The driver directly dereferences `pcie->cfg->win` as I/O memory, which is invalid on 32-bit platforms where `per_bus_mapping` is enabled for ECAM.
--

--- Patch [3]: [PATCH v10 3/5] PCI: tegra: Add Tegra264 support ---
commit af57b50d38f94a70c577cb3fb66bd77477d89532
Author: Thierry Reding <treding@nvidia.com>
Subject: PCI: tegra: Add Tegra264 support

Add a driver for the PCIe controller found on NVIDIA Tegra264 SoCs. The
driver is very small, with its main purpose being to set up the address
translation registers and then creating a standard PCI host using ECAM.

> diff --git a/drivers/pci/controller/pcie-tegra264.c b/drivers/pci/controller/pcie-tegra264.c
> new file mode 100644
> index 0000000000000..7774ce10544ab
> --- /dev/null
> +++ b/drivers/pci/controller/pcie-tegra264.c

[ ... ]

> +/* XTL registers */
> +#define XTL_RC_PCIE_CFG_LINK_CAPS		0x56

[Severity: High]
Is the offset for XTL_RC_PCIE_CFG_LINK_CAPS correct? Since Link Status is at
0x5a, should Link Capabilities be at 0x54 instead of 0x56? Reading from 0x56
with readw() in tegra264_pcie_icc_set() would fetch the upper 16 bits of the
32-bit Link Capabilities register, resulting in incorrect values for link
speed and width.

> +#define XTL_RC_PCIE_CFG_LINK_STATUS		0x5a
> +
> +#define XTL_RC_MGMT_PERST_CONTROL		0x218

[ ... ]

> +static void tegra264_pcie_icc_set(struct tegra264_pcie *pcie)
> +{
> +	u32 value, speed, width;
> +	int err;

[ ... ]

> +	if (pcie->link_up) {
> +		value = readw(pcie->ecam + XTL_RC_PCIE_CFG_LINK_STATUS);
> +		speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, value);
> +		width = FIELD_GET(PCI_EXP_LNKSTA_NLW, value);
> +	} else {
> +		value = readw(pcie->ecam + XTL_RC_PCIE_CFG_LINK_CAPS);
> +		speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, value);
> +		width = FIELD_GET(PCI_EXP_LNKCAP_MLW, value);
> +	}
> +
> +	value = Mbps_to_icc(width * PCIE_SPEED2MBS_ENC(pcie_link_speed[speed]));

[Severity: High]
Can the speed index extracted from the hardware registers exceed the bounds
of the pcie_link_speed array?

If the register contains an unexpected value (such as 0xffff when the device
is inaccessible, or corrupted data due to an incorrect register offset), the
extracted speed could be greater than 6, causing an out-of-bounds read on the
array.

> +
> +	/*
> +	 * We don't want to error out here because a boot-critical device

[ ... ]

> +static bool tegra264_pcie_link_up(struct tegra264_pcie *pcie,
> +				  enum pci_bus_speed *speed)
> +{
> +	u16 value = readw(pcie->ecam + XTL_RC_PCIE_CFG_LINK_STATUS);
> +
> +	if (value & PCI_EXP_LNKSTA_DLLLA) {
> +		if (speed)
> +			*speed = pcie_link_speed[FIELD_GET(PCI_EXP_LNKSTA_CLS,
> +							   value)];

[Severity: High]
Similar to tegra264_pcie_icc_set() above, could this field extraction result
in an out-of-bounds read of the pcie_link_speed array if the status register
contains an unexpected value?

> +
> +		return true;
> +	}
> +
> +	return false;
> +}

[ ... ]

> +static int tegra264_pcie_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;

[ ... ]

> +	bridge->ops = (struct pci_ops *)&pci_generic_ecam_ops.pci_ops;
> +	bridge->sysdata = pcie->cfg;
> +	pcie->ecam = pcie->cfg->win;

[Severity: Medium]
Does this direct assignment of pcie->cfg->win work correctly on 32-bit
platforms where per_bus_mapping might be enabled?

When per_bus_mapping is true, cfg->win is an unmapped kzalloc'd pointer
array (cfg->winp) due to their union structure, not device I/O memory.
Accessing it via readw() later in tegra264_pcie_link_up() would yield
zero-initialized garbage data instead of I/O memory.

> +
> +	tegra264_pcie_init(pcie);
> +
> +	/*

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814-tegra264-pcie-v10-0-05d08c592b05@nvidia.com?part=3