Re: [PATCH v2] PCI: mediatek: Fix integer truncation in fls() and clamp size

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci: silent hardware window truncation causes unmapped BARs
--

commit 3807ae15ec6ca796b24e06b510d2cb2153d9a5c8
Author: Sreeraj S Kurup <sreekuttan2156239@gmail.com>

PCI: mediatek: Fix integer truncation in fls() and clamp size

This commit fixes an integer truncation issue by using fls64() for 64-bit
resource sizes. It also clamps the resulting size_order to a maximum of 31
to avoid overflowing the 5-bit register field used in AHB2PCIE_SIZE().

> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index 3a3e91cd0c211..ddc7ed28ef843 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
[ ... ]
> @@ -753,8 +756,13 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
>  		mtk_pcie_enable_msi(port);
>  
>  	/* Set AHB to PCIe translation windows */
> +	size = resource_size(mem);
> +	size_order = fls64(size);
> +	if (size_order > 31)
> +		size_order = 31;
> +

[Severity: High]
Does clamping the size order here in mtk_pcie_startup_port_v2() cause
unmapped BARs?

The hardware translation window size is clamped to size order 31, but the
patch does not adjust the OS-level mem resource (e.g., mem->end). If the
device tree provides a PCIe memory resource of 4 GiB or larger, the Linux
PCI core will allocate BARs within the full original window. Because the
hardware only translates the truncated portion, will CPU accesses to BARs
placed above the clamped limit fail with a data abort or bus error?

Additionally, using fls64(size) on a power-of-two size computes
ilog2(size) + 1, which may evaluate to a larger window than requested.

Should the driver reject oversized windows entirely, or shrink the OS
resource size to match the hardware reality?

>  	val = lower_32_bits(mem->start) |
> -	      AHB2PCIE_SIZE(fls(resource_size(mem)));
> +	      AHB2PCIE_SIZE(size_order);
>  	writel(val, port->base + PCIE_AHB_TRANS_BASE0_L);
>  
>  	val = upper_32_bits(mem->start);
[ ... ]
> @@ -829,8 +839,13 @@ static int mtk_pcie_startup_port_en7528(struct mtk_pcie_port *port)
>  		mtk_pcie_enable_msi(port);
>  
>  	/* Set AHB to PCIe translation windows */
> +	size = resource_size(mem);
> +	size_order = fls64(size);
> +	if (size_order > 31)
> +		size_order = 31;
> +

[Severity: High]
Does this suffer from the same silent hardware window truncation issue as
mtk_pcie_startup_port_v2() above? The hardware translation window is
clamped without adjusting the corresponding OS-level resource.

>  	val = lower_32_bits(mem->start) |
> -	      AHB2PCIE_SIZE(fls(resource_size(mem)));
> +	      AHB2PCIE_SIZE(size_order);
>  	writel(val, port->base + PCIE_AHB_TRANS_BASE0_L);
>  
>  	val = upper_32_bits(mem->start);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930105435.3660-1-sreekuttan2156239@gmail.com?part=1