Re: [PATCH 2/2] PCI: Resources outside their window must set IORESOURCE_UNSET

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Sep 24, 2025 at 04:42:28PM +0300, Ilpo Järvinen wrote:
> PNP resources are checked for conflicts with the other resource in the
> system by quirk_system_pci_resources() that walks through all PCI
> resources. quirk_system_pci_resources() correctly filters out resource
> with IORESOURCE_UNSET.
> 
> Resources that do not reside within their bridge window, however, are
> not properly initialized with IORESOURCE_UNSET resulting in bogus
> conflicts detected in quirk_system_pci_resources():
> 
> pci 0000:00:02.0: VF BAR 2 [mem 0x00000000-0x1fffffff 64bit pref]
> pci 0000:00:02.0: VF BAR 2 [mem 0x00000000-0xdfffffff 64bit pref]: contains BAR 2 for 7 VFs
> ...
> pci 0000:03:00.0: VF BAR 2 [mem 0x00000000-0x1ffffffff 64bit pref]
> pci 0000:03:00.0: VF BAR 2 [mem 0x00000000-0x3dffffffff 64bit pref]: contains BAR 2 for 31 VFs
> ...
> pnp 00:04: disabling [mem 0xfc000000-0xfc00ffff] because it overlaps 0000:03:00.0 BAR 9 [mem 0x00000000-0x3dffffffff 64bit pref]
> pnp 00:05: disabling [mem 0xc0000000-0xcfffffff] because it overlaps 0000:00:02.0 BAR 9 [mem 0x00000000-0xdfffffff 64bit pref]
> pnp 00:05: disabling [mem 0xfedc0000-0xfedc7fff] because it overlaps 0000:03:00.0 BAR 9 [mem 0x00000000-0x3dffffffff 64bit pref]
> pnp 00:05: disabling [mem 0xfeda0000-0xfeda0fff] because it overlaps 0000:03:00.0 BAR 9 [mem 0x00000000-0x3dffffffff 64bit pref]
> pnp 00:05: disabling [mem 0xfeda1000-0xfeda1fff] because it overlaps 0000:03:00.0 BAR 9 [mem 0x00000000-0x3dffffffff 64bit pref]
> pnp 00:05: disabling [mem 0xc0000000-0xcfffffff disabled] because it overlaps 0000:03:00.0 BAR 9 [mem 0x00000000-0x3dffffffff 64bit pref]
> pnp 00:05: disabling [mem 0xfed20000-0xfed7ffff] because it overlaps 0000:03:00.0 BAR 9 [mem 0x00000000-0x3dffffffff 64bit pref]
> pnp 00:05: disabling [mem 0xfed90000-0xfed93fff] because it overlaps 0000:03:00.0 BAR 9 [mem 0x00000000-0x3dffffffff 64bit pref]
> pnp 00:05: disabling [mem 0xfed45000-0xfed8ffff] because it overlaps 0000:03:00.0 BAR 9 [mem 0x00000000-0x3dffffffff 64bit pref]
> pnp 00:05: disabling [mem 0xfee00000-0xfeefffff] because it overlaps 0000:03:00.0 BAR 9 [mem 0x00000000-0x3dffffffff 64bit pref]
> 
> Mark resources that are not contained within their bridge window with
> IORESOURCE_UNSET in __pci_read_base() which resolves the false
> positives for the overlap check in quirk_system_pci_resources().
> 
> Fixes: f7834c092c42 ("PNP: Don't check for overlaps with unassigned PCI BARs")
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
> 
> This change uses resource_contains() which will reject partial overlaps.
> I don't know for sure if partial overlaps should be allowed or not (but
> they feel as something FW didn't set things up properly so I chose to
> mark them UNSET as well).
> 
>  drivers/pci/probe.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 7f9da8c41620..097389f25853 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -205,6 +205,26 @@ static void __pci_size_rom(struct pci_dev *dev, unsigned int pos, u32 *sizes)
>  	__pci_size_bars(dev, 1, pos, sizes, true);
>  }
>  
> +static struct resource *pbus_select_window_for_res_addr(
> +					const struct pci_bus *bus,
> +					const struct resource *res)
> +{
> +	unsigned long type = res->flags & IORESOURCE_TYPE_BITS;
> +	struct resource *r;
> +
> +	pci_bus_for_each_resource(bus, r) {
> +		if (!r || r == &ioport_resource || r == &iomem_resource)
> +			continue;
> +
> +		if ((r->flags & IORESOURCE_TYPE_BITS) != type)
> +			continue;
> +
> +		if (resource_contains(r, res))
> +			return r;
> +	}
> +	return NULL;
> +}
> +
>  /**
>   * __pci_read_base - Read a PCI BAR
>   * @dev: the PCI device
> @@ -329,6 +349,18 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
>  			 res_name, (unsigned long long)region.start);
>  	}
>  
> +	if (!(res->flags & IORESOURCE_UNSET)) {
> +		struct resource *b_res;
> +
> +		b_res = pbus_select_window_for_res_addr(dev->bus, res);
> +		if (!b_res ||
> +		    b_res->flags & (IORESOURCE_UNSET | IORESOURCE_DISABLED)) {
> +			pci_dbg(dev, "%s %pR: no initial claim (no window)\n",
> +				res_name, res);

Should this be pci_info()?  Or is there somewhere else that we
complain about a child resource that's not contained in a bridge
window?

I recently got an internal report of child BARs being reassigned, I
think because they weren't inside a bridge window, and the dmesg log
(from an older kernel) showed the BAR reassignments, but didn't say
anything about the *reason* for the reassignment.

> +			res->flags |= IORESOURCE_UNSET;
> +		}
> +	}
> +
>  	goto out;
>  
>  
> -- 
> 2.39.5
>