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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 4408 bytes --]

On Thu, 5 Mar 2026, Guenter Roeck wrote:
> 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@tnxip.de>
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > ---
> 
> 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. It looks a real problem to me.

> Thanks,
> Guenter
> 
> ---
> commit 3958bf16e2fe1b1c95467e58694102122c951a31
> Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> 
>     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@linux.intel.com
> 
> > 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?

It's not really what I'd call underestimation but an unwanted gap what is 
the end result here.

I'll think this over the weekend, maybe the best solution for now is to 
put that check back. With the check, those resources will end up into tail 
space more often but anything else I can think of would require more 
complex data structures.

I'd eventually want to use better data structure anyway as I'd want to 
make it possible to put as much stuff into the head side while still 
producing gapless fit to generally produce tighter fit for windows that 
are next to each other in the address space if both have resources 
smaller than the max order.

> 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?
> 

-- 
 i.