Re: [PATCH 2/5] PCI: Fix nesting windows with remainder at the left edge

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] pci: __pci_assign_resource assigns address bound min to alignment
- [High] pci: pci_resreq_check incorrectly shifts normal BARs out of alignment
--

commit 5024064a0f2ceedd425ef93754ec9f56cae0112c
Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

PCI: Fix nesting windows with remainder at the left edge

This commit attempts to fix a bridge window assignment failure in nested
topologies where the non-aligning remainder sits at the left edge. It
introduces pci_resreq_check() to wrap pcibios_align_resource() and recalculate
the start address if there are constraints, bypassing standard alignment checks.

Fixes: 9036bd0efcb6 ("PCI: Align head space better")

> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index 376f09630a4ab..eacce9e2486bf 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c

[ ... ]

> @@ -311,15 +326,54 @@ resource_size_t __weak pcibios_align_resource(void *data,
>  	return pci_align_resource(dev, res, empty_res, size, align);
>  }
>  
> +struct pci_resreq_data {
> +	struct pci_dev *dev;
> +	resource_size_t real_align;
> +};
> +
> +static resource_size_t pci_resreq_check(void *data,
> +					const struct resource *res,
> +					const struct resource *empty_res,
> +					resource_size_t size,
> +					resource_size_t min_align)
> +{
> +	struct pci_resreq_data *rr = data;
> +	resource_size_t align = rr->real_align;
> +	resource_size_t remainder, start;
> +	struct resource tmp = *res;
> +
> +	if (res->flags & IORESOURCE_MEM && !IS_ALIGNED(res->start, align)) {
> +		resource_set_range(&tmp, ALIGN(res->start, align), size);
> +		if (!__resource_contains_unbound(empty_res, &tmp)) {
> +			remainder = pci_resreq_remainder(size, align);
> +			resource_set_range(&tmp, tmp.start - remainder, size);

[Severity: High]
Does this subtraction incorrectly shift normal device BARs out of alignment?

When a user overrides alignment via the pci=resource_alignment= parameter,
the requested alignment can be much larger than the size, resulting in a
non-zero remainder. For normal device BARs, subtracting this remainder shifts
the start address out of the requested alignment boundaries.

Could this silent violation of user alignment requests break use cases like
VFIO pass-through that rely on IOMMU page alignment to isolate device memory?

> +			if (!__resource_contains_unbound(empty_res, &tmp))
> +				return tmp.start;	/* caller skips range */
> +		}
> +	}
> +
> +	start = pcibios_align_resource(rr->dev, &tmp, empty_res, size, align);
> +	WARN_ON_ONCE(!IS_ALIGNED(start, min_align));
> +
> +	return start;
> +}
> +
>  static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
>  		int resno, resource_size_t size, resource_size_t align)
>  {
> +	struct pci_resreq_data rr = { .dev = dev, .real_align = align };
>  	struct resource *res = pci_resource_n(dev, resno);
>  	resource_size_t min;
>  	int ret;
>  
>  	min = (res->flags & IORESOURCE_IO) ? PCIBIOS_MIN_IO : PCIBIOS_MIN_MEM;
>  
> +	if ((res->flags & IORESOURCE_MEM) && pci_resreq_remainder(size, align)) {
> +		align = min;
> +		if (pci_resource_is_bridge_win(resno))
> +			align = pci_min_window_alignment(bus, res->flags);
> +	}

[Severity: High]
Can this logic unintentionally catch normal device BARs with overridden
alignments and cause resource allocation to fail?

If pci=resource_alignment= is used, pci_resreq_remainder() evaluates to
non-zero for a normal device BAR. This block is then entered and align is
overwritten with min (which is PCIBIOS_MIN_MEM).

On architectures where PCIBIOS_MIN_MEM is 0 (like ARM64), this destroys the
original alignment constraint and assigns an alignment of 0. Passing a 0
alignment to allocate_resource() causes ALIGN(x, 0) to evaluate to 0, which
can trigger a catastrophic allocation failure.

Should this condition specifically check if the resource is a bridge window
before proceeding?

> +
>  	/*
>  	 * First, try exact prefetching match.  Even if a 64-bit
>  	 * prefetchable bridge window is below 4GB, we can't put a 32-bit

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