Re: WARNING at drivers/pci/setup-bus.c:2373, bisected to "PCI: Use pbus_select_window_for_type() during mem window sizing"

Ilpo Järvinen
2025-10-27T13:29:45Z

[-- Attachment #1: Type: text/plain, Size: 2988 bytes --]

On Sat, 25 Oct 2025, Klaus Kudielka wrote:

> On Sat, 2025-10-25 at 12:11 +0200, Klaus Kudielka wrote:
> >
> > > [    0.027107] pci 0000:00:03.0: bridge window [mem 0x00200000-0x003fffff] to [bus 02] add_size 200000 add_align 200000
> > > [    0.027115] pci 0000:00:03.0: bridge window [mem 0x00200000-0x003fffff] to [bus 02] add_size 200000 add_align 200000
> >
> > So, this part of  pbus_size_mem() now seems to be called *TWICE* for the same bridge window:
> >
> > add_to_list(realloc_head, bus->self, b_res, size1-size0, add_align);
> > pci_info(bus->self, "bridge window %pR to %pR add_size %llx add_align %llx\n",
> >    b_res, &bus->busn_res,
> >    (unsigned long long) (size1 - size0),
> >    (unsigned long long) add_align);
> >
> >
> >
> > WITHOUT the offending commit, I see only one line, and no WARNING.
> > > [    0.027405] pci 0000:00:03.0: bridge window [mem 0x00200000-0x003fffff] to [bus 02] add_size 200000 add_align 200000
> >
> >
>
>
> After some more testing, I think I know what is going on.
>
> - My device seems to have only non-prefetchable IO resources.
> - In pci_bus_size_bridges(), pbus_size_mem() is called twice, once with IORESOURCE_PREFETCH, once without.
> - This seems to be the intended behaviour (with or without the offending commit).
>
> - What DOES make the difference, is the use of pbus_select_window_for_type() inside pbus_size_mem().
> - On my device, that function returns the ***non-prefetchable*** resource, even if being asked for a prefetchable one.
> - End result: b_res is valid (and identical) in both calls to pbus_size_mem().
> - Honestly, that does not look right to me.
>
>
> Indeed, my device goes back to the original behaviour (without WARNING), if I go back to the original use of
> find_bus_resource_of_type():
>
>
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1312,7 +1312,9 @@ static void pbus_size_mem(struct pci_bus *bus, unsigned long type,
> resource_size_t min_align, win_align, align, size, size0, size1 = 0;
> resource_size_t aligns[28]; /* Alignments from 1MB to 128TB */
> int order, max_order;
> - struct resource *b_res = pbus_select_window_for_type(bus, type);
> + struct resource *b_res = find_bus_resource_of_type(bus,
> + IORESOURCE_MEM | IORESOURCE_PREFETCH | IORESOURCE_MEM_64,
> + type);
> resource_size_t children_add_size = 0;
> resource_size_t children_add_align = 0;
> resource_size_t add_align = 0;
>
>
>
> Comments?

Hi Klaus,

I'm sorry this ended up falling through cracks until now (this is far from
the only regression I've on my table at the moment).

Big kudos for you from figuring out what went wrong! For a bridge which
doesn't have a prefetchable window, pbus_size_mem() should not be called for it.

I've sent a fix patch separately. Please test it.

--
i.