lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <4c28cd58-fd0d-1dff-ad31-df3c488c464f@linux.intel.com>
Date: Tue, 30 Sep 2025 19:32:34 +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 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

Hi Geert,

I really appreciate you paying attention!!

> 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".

When IORESOURCE_UNSET is set in a resource, %pR does not print the start 
and end addresses.

>     +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.

You didn't report any issues beyond textual changes in the log, I suppose 
there were none beyond the log differences?

-- 
 i.

> > --- 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);
> > +                       res->flags |= IORESOURCE_UNSET;
> > +               }
> > +       }
> > +
> >         goto out;
> >
> >
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ