Message-ID: <7640a03e-dfea-db9c-80f5-d80fa2c505b7@linux.intel.com>
Date: Thu, 2 Oct 2025 19:59:09 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Geert Uytterhoeven <geert@...ux-m68k.org>
cc: linux-pci@...r.kernel.org, Bjorn Helgaas <bhelgaas@...gle.com>, 
    Krzysztof Wilczyński <kw@...ux.com>, 
    "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>, 
    LKML <linux-kernel@...r.kernel.org>, 
    Lucas De Marchi <lucas.demarchi@...el.com>, 
    Linux-Renesas <linux-renesas-soc@...r.kernel.org>
Subject: Re: [PATCH 2/2] PCI: Resources outside their window must set
 IORESOURCE_UNSET

On Thu, 2 Oct 2025, Geert Uytterhoeven wrote:

> Hi Ilpo,
> 
> On Thu, 2 Oct 2025 at 16:54, Ilpo Järvinen
> <ilpo.jarvinen@...ux.intel.com> wrote:
> > On Wed, 1 Oct 2025, Geert Uytterhoeven wrote:
> > > On Wed, 1 Oct 2025 at 15:06, Ilpo Järvinen
> > > <ilpo.jarvinen@...ux.intel.com> wrote:
> > > > On Wed, 1 Oct 2025, Geert Uytterhoeven wrote:
> > > > > On Tue, 30 Sept 2025 at 18:32, Ilpo Järvinen
> > > > > <ilpo.jarvinen@...ux.intel.com> wrote:
> > > > > > On Tue, 30 Sep 2025, Geert Uytterhoeven wrote:
> > > > > > > On Fri, 26 Sept 2025 at 04:40, Ilpo Järvinen
> > > > > > > <ilpo.jarvinen@...ux.intel.com> 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@...ux.intel.com>
> > > > > > >
> > > > > > > Thanks for your patch, which is now commit 06b77d5647a4d6a7 ("PCI:
> > > > > > > Mark resources IORESOURCE_UNSET when outside bridge windows") in
> > > > > > > linux-next/master next-20250929 pci/next
> > > > >
> > > > > > > This replaces the actual resources by their sizes in the boot log on
> > > > > > > e.g. on R-Car M2-W:
> > > > > > >
> > > > > > >      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]
> > > > > >
> > > > > > What is going to be the parent of these resources? They don't seem to fall
> > > > > > under the root bus resource above in which case the output change is
> > > > > > intentional so they don't appear as if address range would be "okay".
> > > > >
> > > > > >From /proc/iomem:
> > > > >
> > > > >     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
> > > >
> > > > Okay, so this seem to appear in the resource tree but not among the root
> > > > bus resources.
> > > >
> > > > >     ee0c0000-ee0cffff : pci@...d0000
> > > > >       ee0c0000-ee0c0fff : 0001:01:01.0
> > > > >         ee0c0000-ee0c0fff : ohci_hcd
> > > > >       ee0c1000-ee0c10ff : 0001:01:02.0
> > > > >         ee0c1000-ee0c10ff : ehci_hcd
> > > > >
> > > > > > When IORESOURCE_UNSET is set in a resource, %pR does not print the start
> > > > > > and end addresses.
> > > > >
> > > > > Yeah, that's how I found this commit, without bisecting ;-)
> > > > >
> > > > > > >     +pci 0000:00:00.0: BAR 0 [mem size 0x00000400]
> > > > > > >     +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 size 0x00001000]
> > > > > >
> > > > > > For this resource, it's very much intentional. It's a zero-based BAR which
> > > > > > was left without IORESOURCE_UNSET prior to my patch leading to others part
> > > > > > of the kernel to think that resource range valid and in use (in your
> > > > > > case it's so small it wouldn't collide with anything but it wasn't
> > > > > > properly set up resource, nonetheless).
> > > > > >
> > > > > > >      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 size 0x00000100]
> > > > > >
> > > > > > And this as well is very much intentional.
> > > > > >
> > > > > > >      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]
> > > > > > >
> > > > > > > Is that intentional?
> > > > > >
> > > > > > There's also that pci_dbg() in the patch which would show the original
> > > > > > addresses (print the resource before setting IORESOURCE_UNSET) but to see
> > > > > > it one would need to enable it with dyndbg=... Bjorn was thinking of
> > > > > > making that pci_info() though so it would appear always.
> > > > > >
> > > > > > Was this the entire PCI related diff? I don't see those 0000:00:00.0 BARs
> > > > > > getting assigned anywhere.
> > > > >
> > > > > The above log is almost complete (lacked enabling the device afterwards).
> > > > >
> > > > > AFAIU, the BARs come from the reg and ranges properties in the
> > > > > PCI controller nodes;
> > > > > https://elixir.bootlin.com/linux/v6.17/source/arch/arm/boot/dts/renesas/r8a7791.dtsi#L1562
> > > >
> > > > Thanks, although I had already found this line by grep. I was just
> > > > expecting the address appear among root bus resources too.
> > > >
> > > > Curiously enough, pci_register_host_bridge() seems to try to add some
> > > > resource from that list into bus and it's what prints those "root bus
> > > > resource" lines and ee090000 is not among the printed lines despite
> > > > appearing in /proc/iomem. As is, the resource tree and PCI bus view on the
> > > > resources seems to be in disagreement and I'm not sure what to make of it.
> > > >
> > > > But before considering going into that direction or figuring out why this
> > > > ee090000 resource does not appear among root bus resources, could you
> > > > check if the attached patch changes behavior for the resource which are
> > > > non-zero based?
> > >
> > > This patch has no impact on dmesg, lspci output, or /proc/iomem
> > > for pci-rcar-gen2.
> >
> > It would have been too easy... :-(
> >
> > I'm sorry I don't really know these platform well and I'm currently trying
> > to understand what adds that ee090000 resource into the resource tree
> > and so far I've not been very successful.
> >
> > Perhaps it would be easiest to print a stacktrace when the resource is
> > added but there are many possible functions. I think all of them
> > converge in __request_resource() so I suggest adding:
> >
> > WARN_ON(new->start == 0xee090000);
> >
> > before __request_resource() does anything.
> 
>     Call trace:
>      unwind_backtrace from show_stack+0x10/0x14
>      show_stack from dump_stack_lvl+0x7c/0xb0
>      dump_stack_lvl from __warn+0x80/0x198
>      __warn from warn_slowpath_fmt+0xc0/0x124
>      warn_slowpath_fmt from __request_resource+0x38/0xc8
>      __request_resource from __request_region+0xc4/0x1e8
>      __request_region from __devm_request_region+0x80/0xac
>      __devm_request_region from __devm_ioremap_resource+0xcc/0x160
>      __devm_ioremap_resource from rcar_pci_probe+0x58/0x350
>      rcar_pci_probe from platform_probe+0x58/0x90
> 
> I.e. the call to devm_platform_get_and_ioremap_resource() in
> drivers/pci/controller/pci-rcar-gen2.c:rcar_pci_probe().

Thanks, the patch below might help BAR0 (but I'm not sure if it's the 
correct solution due to being so unfamiliar with these kind of platforms 
and how they're initialized).

If this works, we'll also have to decide what to do with the BAR1 (it 
didn't appear in your (partial?) /proc/iomem quote so I'm left unsure how 
to approach it).

I also noticed that || COMPILE_TEST is made ineffective for this driver by 
the depends on ARM on the other line but it built just fine on x86 after 
changing the depends on to:

depends on (ARCH_RENESAS && ARM) || COMPILE_TEST


--
[PATCH 1/1] PCI: rcar-gen2: Add BAR0 into host bridge resources

On R-Car M2-W, 0000:00:00.0 has BAR0 address that does not fall under
any root bus resource.

 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]

The same resource, however, appears in the resource tree (from
/proc/iomem):

ee090000-ee090bff : ee090000.pci pci@...90000

This discrepancy between the resource tree and PCI bus resources for
the root bus causes issues with the commit 06b77d5647a4 ("PCI: Mark
resources IORESOURCE_UNSET when outside bridge windows") as BAR0 is
outside of any bus resource, and therefore marked with
IORESOURCE_UNSET.

The resource is added into the resource tree by rcar_pci_probe().
Add the resource also into host bridge resources.

Reported-by: Geert Uytterhoeven <geert@...ux-m68k.org>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
---
 drivers/pci/controller/pci-rcar-gen2.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/controller/pci-rcar-gen2.c b/drivers/pci/controller/pci-rcar-gen2.c
index d29866485361..72ed44fdcde4 100644
--- a/drivers/pci/controller/pci-rcar-gen2.c
+++ b/drivers/pci/controller/pci-rcar-gen2.c
@@ -294,6 +294,8 @@ static int rcar_pci_probe(struct platform_device *pdev)
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
+	pci_add_resource(&bridge->windows, cfg_res);
+
 	mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
 	if (!mem_res || !mem_res->start)
 		return -ENODEV;

base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
-- 
2.39.5

Powered by blists - more mailing lists