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

Bjorn Helgaas
2025-09-25T21:21:19Z

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
>