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

[sashiko-bot@kernel.org]

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
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;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923131757.7792-1-ilpo.jarvinen@linux.intel.com?part=4