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

Bjorn Helgaas
2025-09-26T19:30:30Z

On Fri, Sep 26, 2025 at 03:21:17PM +0300, Ilpo Järvinen wrote:
> On Thu, 25 Sep 2025, Bjorn Helgaas wrote:
> > 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():

> > > @@ -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?
>
> AFAIK, there's no other print. The kernel didn't even recognize this case
> until now so how could there have been one?!

> They'd generally show up as failures later in resource assignment if the
> resource doesn't fit to the bridge window [1], which should also set
> IORESOURCE_UNSET, but good luck for inferring things from that. It's
> tedious, I know. :-) If the bridge window is large enough, the base
> address would just change where the resource fits (I think).
>
> It can be pci_info() if you think that's better. I just picked the level
> which is the least noisy. We can go with pci_info() now and if the logging
> turns out excessive when we start to see dmesgs with it, we can of course
> adjust it later so it's not permanent either way.
>
> In any case, there's not much user can do for these as it's the setup FW
> gave us.
>
> > 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.
>
> Resource reassignment is only done after the resource was initially
> assigned so I'm not sure if that inferring chain is sound.
>
> Admittedly, you didn't exactly specify how you picked up that it was
> "reassigned" so it could be just terminology that doesn't match what
> setup-bus/res.c considers as resource reassignment. That is, if BAR's
> address was simply changed from the initial, that's not "reassignment" in
> the sense used by the kernel.

Here's the case I saw (a v6.1 kernel, so old log messages):

pci 0000:00:00.0: bridge window [mem 0x80000000-0x97fffffff 64bit pref] to [bus 01-05] add_size 80000000 add_align 80000000
pci 0000:00:00.0: BAR 15: assigned [mem 0x380000000-0xcffffffff 64bit pref]
pci 0000:01:01.0: BAR 0: [mem 0xb00000000-0xbffffffff 64bit pref]
...
pci 0000:01:01.7: BAR 0: [mem 0x400000000-0x4ffffffff 64bit pref]
pci 0000:01:01.0: BAR 0: assigned [mem 0x400000000-0x4ffffffff 64bit pref]

Obviously these initial BAR 0 values don't fit in the
[0x80000000-0x97fffffff] bridge window, so I think we moved and
expanded it and then assigned the BARs to be inside.

I was thinking might get the "can't claim; no compatible bridge
window" message in pci_claim_resource() as a clue, but we didn't.

This *seems* like a firmware defect: why would firmware bother to
program these BARs at all unless it also made a bridge window that
could reach them.

Bjorn