Re: [PATCH 03/23] PCI: Stop over-estimating bridge window size

[Guenter Roeck]

Hi,

On Fri, Dec 19, 2025 at 07:40:16PM +0200, Ilpo Järvinen wrote:
> New way to calculate the bridge window head alignment produces
> tight-fit, that is, it does not leave any gaps between the resources.
> Similarly, relaxed tail alignment does not leave extra tail room.
> 
> Start to use bridge window calculation that does not over-estimate
> the size of the required window.
> 
> pbus_upstream_space_available() can be removed.
> 
> Tested-by: Malte Schröder <malte+lkml@xxxxxxxx>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> ---

Google's experimental AI code review agent produced the following feedback.
It seems real to me, but I may be missing something. PTAL and let me know
if it has a point.

If it is a real problem, it may have been fixed by a later patch which I
may have missed. If so, my apologies for the noise.

Thanks,
Guenter

---
commit 3958bf16e2fe1b1c95467e58694102122c951a31
Author: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>

    PCI: Stop over-estimating bridge window size

    New way to calculate the bridge window head alignment produces tight-fit,
    that is, it does not leave any gaps between the resources.  Similarly,
    relaxed tail alignment does not leave extra tail room.

    Link: https://patch.msgid.link/20251219174036.16738-4-ilpo.jarvinen@xxxxxxxxxxxxxxx

> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index cd12926a72af..25d6d4d3afc1 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c

[ ... ]

> @@ -1409,13 +1346,8 @@
>  				continue;
>  			}
>  			size += max(r_size, align);
> -			/*
> -			 * Exclude ranges with size > align from calculation of
> -			 * the alignment.
> -			 */
> -			if (r_size <= align)
> -				aligns[order] += align;
> -			aligns2[order] += align;
> +
> +			aligns[order] += align;
>  			if (order > max_order)
>  				max_order = order;
>  
> @@ -1429,38 +1361,19 @@
>  
>  	old_size = resource_size(b_res);
>  	win_align = window_alignment(bus, b_res->flags);
> -	min_align = calculate_mem_align(aligns, max_order);
> +	min_align = calculate_head_align(aligns, max_order);
>  	min_align = max(min_align, win_align);
> -	size0 = calculate_memsize(size, min_size, 0, 0, old_size, min_align);
> +	size0 = calculate_memsize(size, min_size, 0, 0, old_size, win_align);

Does the removal of the `r_size <= align` check cause bridge window size
underestimations if we have child resources larger than their alignment?

If we have child resources with the following requirements:
- Resource A: 4MB size, 4MB align
- Resource B: 3MB size, 1MB align
- Resource C: 1MB size, 1MB align

The requested `size` is calculated as 4 + 3 + 1 = 8MB.
`aligns[order]` for 1MB will now incorrectly include B, summing to 2MB.
`calculate_head_align()` will see the 2MB remainder and reduce `min_align`
to 2MB, assuming the remainder can perfectly fill a 2MB head gap.

If the bridge window of size 8MB is allocated at a 2MB-aligned address
(e.g., `0x10200000`), the 4MB aligned Resource A must be placed at
`0x10400000`, leaving a 2MB gap.

Resource B (3MB) cannot fit in this 2MB gap, so it is placed after A at
`0x10800000`.
Resource C (1MB) fits in the gap at `0x10200000`.

The total space used spans from `0x10200000` to `0x10AFFFFF` (9MB).
Since `size0` was calculated as 8MB, won't Resource B fail to allocate
because it exceeds the bridge window?