Re: [PATCH 4/5] PCI: Fix composite resource sizing

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

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

On Wed, 23 Sep 2026, sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] pci: bridge window gap calculation Rule 2 underestimates required size
> --
> 
> commit f9f2d66b1a5c96f99a75fc82cb40a5f073968dfb
> Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> 
> PCI: Fix composite resource sizing
> 
> This commit modifies the bridge window sizing algorithm to add a gap size
> calculator. The calculator determines the empty space required because of
> non-aligning composite child resources, such as VF BARs and bridge windows,
> to ensure alignment constraints are met when resources are placed.
> 
> Link: https://lore.kernel.org/linux-pci/20260903063124.9316-1-nbritton@exabit.io/
> 
> > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> > index 7ca0e9f4ffb63..9d828a59bd002 100644
> > --- a/drivers/pci/setup-bus.c
> > +++ b/drivers/pci/setup-bus.c
> [ ... ]
> > @@ -1164,6 +1164,76 @@ static inline resource_size_t calculate_mem_align(resource_size_t *aligns,
> [ ... ]
> > +static resource_size_t calculate_win_gap_size(struct pci_bus *bus,
> > +					      struct resource *b_res,
> > +					      resource_size_t win_align,
> > +					      bool optional)
> > +{
> > +	resource_size_t safe_gap = 0, remainders = 0;
> > +	unsigned int nonaligning = 0;
> > +	struct pci_dev *dev;
> [ ... ]
> > +			nonaligning++;
> > +			remainders += remainder;
> > +			safe_gap += win_align - remainder;
> > +		}
> > +	}
> > +
> > +	if (nonaligning == 2 && (remainders <= win_align))
> > +		return win_align - remainders;
> 
> [Severity: High]
> Does this logic underestimate the required gap size by assuming two remainder
> parts can perfectly share a single alignment block's padding without verifying
> if their individual alignment constraints allow tight packing?
> 
> If a parent PCI bus contains exactly two child resources (e.g., downstream
> bridge windows) whose sizes are not multiples of the bus's max alignment, and
> their size remainders sum to less than or equal to the max alignment, this
> bypass avoids the safe_gap fallback.
> 
> For example, if two 10MB windows require 4MB alignment, this calculates a gap
> of (4 - (2+2)) = 0MB, resulting in a 20MB parent window. However, because both
> resources require 4MB alignment, if the first is placed at 0..10MB, the second
> must be placed at 12MB. This requires a 22MB total span, which exceeds the

This is bogus, sashiko doesn't understand how the new placement algorithm 
works when it comes to remainder parts. The remainder for the second 
resource would start at 10MB and the aligning part will start exactly at 
12MB filling all the constraints and fitting to 20MB.

Thus, the calculation is correct. I suspect this case would work even 
without this patch because no gap is needed in the first place (untested).

> calculated 20MB window.
>
> Could this underestimation cause PCI resource assignment to fail during
> enumeration for devices or downstream bridges, leaving them uninitialized?
>
> > +
> > +	if (nonaligning >= 2)
> > +		return safe_gap;
> > +
> > +	return 0;
> > +}

-- 
 i.