Message-ID: <efebb809-cbd4-4644-750f-4b42d85102f2@linux.intel.com>
Date: Wed, 22 Oct 2025 15:14:12 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Rob Herring <robh@...nel.org>
cc: linux-pci@...r.kernel.org, Bjorn Helgaas <bhelgaas@...gle.com>, 
    Geert Uytterhoeven <geert@...ux-m68k.org>, 
    Kai-Heng Feng <kaihengf@...dia.com>, LKML <linux-kernel@...r.kernel.org>, 
    Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, 
    Krzysztof Wilczyński <kw@...ux.com>, 
    Linux-Renesas <linux-renesas-soc@...r.kernel.org>
Subject: Re: [PATCH 0/3] PCI & resource: Make coalescing host bridge windows
 safer

On Wed, 22 Oct 2025, Geert Uytterhoeven wrote:
> On Tue, 21 Oct 2025 at 13:54, Ilpo Järvinen
> <ilpo.jarvinen@...ux.intel.com> wrote:
> > On Tue, 21 Oct 2025, Geert Uytterhoeven wrote:
> > > On Mon, 20 Oct 2025 at 18:20, Ilpo Järvinen
> > > <ilpo.jarvinen@...ux.intel.com> wrote:
> > > > On Mon, 20 Oct 2025, Geert Uytterhoeven wrote:
> > > > > On Fri, 10 Oct 2025 at 16:42, Ilpo Järvinen
> > > > > <ilpo.jarvinen@...ux.intel.com> wrote:
> > > > > > Here's a series for Geert to test if this fixes the improper coalescing
> > > > > > of resources as was experienced with the pci_add_resource() change (I
> > > > > > know the breaking change was pulled before 6.18 main PR but I'd want to
> > > > > > retry it later once the known issues have been addressed). The expected
> > > > > > result is there'll be two adjacent host bridge resources in the
> > > > > > resource tree as the different name should disallow coalescing them
> > > > > > together, and therefore BAR0 has a window into which it belongs to.
> > > > > >
> > > > > > Generic info for the series:
> > > > > >
> > > > > > PCI host bridge windows were coalesced in place into one of the structs
> > > > > > on the resources list. The host bridge window coalescing code does not
> > > > > > know who holds references and still needs the struct resource it's
> > > > > > coalescing from/to so it is safer to perform coalescing into entirely
> > > > > > a new struct resource instead and leave the old resource addresses as
> > > > > > they were.
> > > > > >
> > > > > > The checks when coalescing is allowed are also made stricter so that
> > > > > > only resources that have identical the metadata can be coalesced.
> > > > > >
> > > > > > As a bonus, there's also a bit of framework to easily create kunit
> > > > > > tests for resource tree functions (beyond just resource_coalesce()).
> > > > > >
> > > > > > Ilpo Järvinen (3):
> > > > > >   PCI: Refactor host bridge window coalescing loop to use prev
> > > > > >   PCI: Do not coalesce host bridge resource structs in place
> > > > > >   resource, kunit: add test case for resource_coalesce()
> > > > >
> > > > > Thanks for your series!
> > > > >
> > > > > I have applied this on top of commit 06b77d5647a4d6a7 ("PCI:
> > > > > Mark resources IORESOURCE_UNSET when outside bridge windows"), and
> > > > > gave it a a try on Koelsch (R-Car M2-W).
> > > >
> > > > So the pci_bus_add_resource() patch to rcar_pci_probe() was not included?
> > > > No coalescing would be attempted without that change.
> > >
> > > Sorry, I didn't realize you wanted that (and anything else) to be
> > > included, too.  Please tell me the exact base I should use for testing,
> > > and I will give it another run.
> >
> > I'm sorry, it's indeed a bit confusing as some of these patches never
> > have been in Linus' tree.
> >
> > So I'm interested on what's the result with these changes/series together:
> >
> > [PATCH 1/2] PCI: Setup bridge resources earlier
> > [PATCH 2/2] PCI: Resources outside their window must set IORESOURCE_UNSET
> > [PATCH 1/1] PCI: rcar-gen2: Add BAR0 into host bridge resources
> > [PATCH 1/3] PCI: Refactor host bridge window coalescing loop to use prev
> > [PATCH 2/3] PCI: Do not coalesce host bridge resource structs in place
> > [PATCH 3/3] resource, kunit: add test case for resource_coalesce()
> >
> > You might also want to change that pci_dbg() in the IORESOURCE_UNSET patch
> > to pci_info() (as otherwise dyndbg is necessary to make it visible).
> 
> Thanks, all done:
> 
>     $ git cherry -v --abbrev=1 v6.18-rc2^
>     + 211ddde0 Linux 6.18-rc2
>     + 3fdaf2 PCI: Setup bridge resources earlier
>     + 5be02e5 PCI: Resources outside their window must set IORESOURCE_UNSET
>     + adf6f11 PCI: rcar-gen2: Add BAR0 into host bridge resources
>     + eecb500 PCI: Refactor host bridge window coalescing loop to use prev
>     + 60470b3 PCI: Do not coalesce host bridge resource structs in place
>     + afe3ec resource, kunit: add test case for resource_coalesce()
>     + 487c98 Use dev_info() in drivers/pci/probe.c:__pci_read_base()
> IORESOURCE_UNSET path
> 
> Compared to v6.18-rc2, dmesg changed (for the first PCI/USB instance)
> like:
> 
>      pci-rcar-gen2 ee090000.pci: host bridge /soc/pci@...90000 ranges:
>      pci-rcar-gen2 ee090000.pci:      MEM 0x00ee080000..0x00ee08ffff
> -> 0x00ee080000
>      pci-rcar-gen2 ee090000.pci: PCI: revision 11
>      pci-rcar-gen2 ee090000.pci: PCI host bridge to bus 0000:00
>      pci_bus 0000:00: root bus resource [bus 00]
>      pci_bus 0000:00: root bus resource [mem 0xee080000-0xee08ffff]
>     +pci_bus 0000:00: root bus resource [mem 0xee090000-0xee090bff]
>      pci 0000:00:00.0: [1033:0000] type 00 class 0x060000 conventional
> PCI endpoint
>      pci 0000:00:00.0: BAR 0 [mem 0xee090800-0xee090bff]
>     -pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]
>     +pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]: no
> initial claim (no window)
>     +pci 0000:00:00.0: BAR 1 [mem size 0x40000000 pref]
>      pci 0000:00:01.0: [1033:0035] type 00 class 0x0c0310 conventional
> PCI endpoint
>     -pci 0000:00:01.0: BAR 0 [mem 0x00000000-0x00000fff]
>     +pci 0000:00:01.0: BAR 0 [mem 0x00000000-0x00000fff]: no initial
> claim (no window)
>     +pci 0000:00:01.0: BAR 0 [mem size 0x00001000]
>      pci 0000:00:01.0: supports D1 D2
>      pci 0000:00:01.0: PME# supported from D0 D1 D2 D3hot
>      pci 0000:00:02.0: [1033:00e0] type 00 class 0x0c0320 conventional
> PCI endpoint
>     -pci 0000:00:02.0: BAR 0 [mem 0x00000000-0x000000ff]
>     +pci 0000:00:02.0: BAR 0 [mem 0x00000000-0x000000ff]: no initial
> claim (no window)
>     +pci 0000:00:02.0: BAR 0 [mem size 0x00000100]
>      pci 0000:00:02.0: supports D1 D2
>      pci 0000:00:02.0: PME# supported from D0 D1 D2 D3hot
>      PCI: bus0: Fast back to back transfers disabled
>      pci 0000:00:01.0: BAR 0 [mem 0xee080000-0xee080fff]: assigned
>      pci 0000:00:02.0: BAR 0 [mem 0xee081000-0xee0810ff]: assigned
>      pci_bus 0000:00: resource 4 [mem 0xee080000-0xee08ffff]
>     +pci_bus 0000:00: resource 5 [mem 0xee090000-0xee090bff]
>      pci 0000:00:01.0: enabling device (0140 -> 0142)
>      pci 0000:00:02.0: enabling device (0140 -> 0142)
> 
> > The expected result is that those usb resources are properly parented and
> > the ee080000-ee08ffff and ee090000-ee090bff are not coalesced together (as
> > that would destroy information). So something along the lines of:
> >
> >     ee080000-ee08ffff : pci@...90000
> >       ee080000-ee080fff : 0000:00:01.0
> >         ee080000-ee080fff : ohci_hcd
> >       ee081000-ee0810ff : 0000:00:02.0
> >         ee081000-ee0810ff : ehci_hcd
> >     ee090000-ee090bff : ee090000.pci pci@...90000
> 
> Compared to v6.18-rc2, the output of "lspci -v" or "cat /proc/iomem"
> did not change.  Hence for the two PCI/USB instances:
> 
>     ee080000-ee08ffff : pci@...90000
>       ee080000-ee080fff : 0000:00:01.0
>         ee080000-ee080fff : ohci_hcd
>       ee081000-ee0810ff : 0000:00:02.0
>         ee081000-ee0810ff : ehci_hcd
>     ee090000-ee090bff : ee090000.pci pci@...90000
>     ee0c0000-ee0cffff : pci@...d0000
>       ee0c0000-ee0c0fff : 0001:01:01.0
>         ee0c0000-ee0c0fff : ohci_hcd
>       ee0c1000-ee0c10ff : 0001:01:02.0
>         ee0c1000-ee0c10ff : ehci_hcd
>     ee0d0000-ee0d0bff : ee0d0000.pci pci@...d0000

Hi Rob,

I'd want to hear your opinion on the solutions me and Geert tried and
discussed in the subthread starting from this:

https://lore.kernel.org/linux-pci/CAMuHMdVtVzcL3AX0uetNhKr-gLij37Ww+fcWXxnYpO3xRAOthA@mail.gmail.com/
   
A short history/summary of the problem and solution space:

I made "PCI: Resources outside their window must set IORESOURCE_UNSET"
change that checks at the init time whether BARs belong to an upstream
window or not. If not, the resource is marked wit IORESOURCE_UNSET to 
indicate FW/platform didn't provide working addressing for those BARs.

On Geert's R-Car M2-W, it caused some BARs to be detected as not having a
an upstream window where they belong to:

     pci-rcar-gen2 ee090000.pci: host bridge /soc/pci@...90000 ranges:
     pci-rcar-gen2 ee090000.pci:      MEM 0x00ee080000..0x00ee08ffff -> 0x00ee080000
     pci-rcar-gen2 ee090000.pci: PCI: revision 11
     pci-rcar-gen2 ee090000.pci: PCI host bridge to bus 0000:00
     pci_bus 0000:00: root bus resource [bus 00]
     pci_bus 0000:00: root bus resource [mem 0xee080000-0xee08ffff]
     pci 0000:00:00.0: [1033:0000] type 00 class 0x060000 conventional PCI endpoint
    -pci 0000:00:00.0: BAR 0 [mem 0xee090800-0xee090bff]
    -pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]
    +pci 0000:00:00.0: BAR 0 [mem 0xee090800-0xee090bff]: no initial claim (no window)
    +pci 0000:00:00.0: BAR 0 [mem size 0x00000400]
    +pci 0000:00:00.0: BAR 1 [mem 0x40000000-0x7fffffff pref]: no initial claim (no window)
    +pci 0000:00:00.0: BAR 1 [mem size 0x40000000 pref]

...In the log above, there's no root bus resource that covers
BAR0's 0xee090800-0xee090bff address range (which itself comes from DT 
"reg"), and thus it got marked IORESOURCE_UNSET with as it does not 
have window where it belongs to.

It's unclear to me whether DT ranges should have included BAR0 so that 
the root bus resources would cover that range?

I was then told the updaing ranges now will not be enough due to DT 
backwards compatibility requirements so it looks we have to resort to a 
change like this:

https://lore.kernel.org/linux-pci/7640a03e-dfea-db9c-80f5-d80fa2c505b7@linux.intel.com/

(+ a few supporting changes as that change exposed brokenness in PCI core.)

Does that look correct solution? That is, should these be added on 
case-by-case basis as additional root bus resources or should there be 
something more generic in the OF PCI code to do it?

There's also that BAR1 which seems to be related to dma_ranges and I don't 
know what to make of it. This resource comes with the added complication 
that this same address appears more than once (in the full log there's 
more than one PCI/USB instance). Again, this BAR1 is not covered by any 
root bus resource and thus gets flagged with IORESOURCE_UNSET. 

So I'm interested what is the "correct" solution for these resources that 
appear as BARs but do not have a backing root bus resource, is it having 
DT "ranges" cover them (I'm ignoring backwards compatibility aspect here) 
or something else?


In addition, is there something special/non-ordinary with these BARs and 
PCI core should treat them somehow differently? If that's the case, how 
can I identify such BARs from "normal" ones to avoid messing with them?


-- 
 i.

Powered by blists - more mailing lists