* Re: [PATCH] PCI: Validate window resource type in pbus_select_window_for_type()
2026-02-10 14:20 [PATCH] PCI: Validate window resource type in pbus_select_window_for_type() Kai-Heng Feng
@ 2026-02-10 15:19 ` Ilpo Järvinen
2026-02-10 17:26 ` Bjorn Helgaas
2026-02-12 17:09 ` Bjorn Helgaas
2 siblings, 0 replies; 5+ messages in thread
From: Ilpo Järvinen @ 2026-02-10 15:19 UTC (permalink / raw)
To: Kai-Heng Feng; +Cc: bhelgaas, linux-pci, LKML
[-- Attachment #1: Type: text/plain, Size: 2861 bytes --]
On Tue, 10 Feb 2026, Kai-Heng Feng wrote:
> Afer commit ebe091ad81e1 ("PCI: Use pbus_select_window_for_type() during
> IO window sizing") and commit ae88d0b9c57f ("PCI: Use
> pbus_select_window_for_type() during mem window sizing"), many bridge
> window can't get resource assigned:
> pci 0006:05:00.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: can't assign; no space
> pci 0006:05:00.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: failed to assign
> pci 0006:05:04.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: can't assign; no space
> pci 0006:05:04.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: failed to assign
> pci 0006:05:08.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: can't assign; no space
> pci 0006:05:08.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: failed to assign
> pci 0006:05:0c.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: can't assign; no space
> pci 0006:05:0c.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: failed to assign
>
> Those commits replace find_bus_resource_of_type() with
> pbus_select_window_for_type(), and the latter lacks resource type
> validation.
>
> This commit adds the resource type validation back to
> pbus_select_window_for_type() to match the original behavior.
>
> Fixes: 74afce3dfcba ("PCI: Add bridge window selection functions")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=221072
> Signed-off-by: Kai-Heng Feng <kaihengf@nvidia.com>
> ---
> drivers/pci/setup-bus.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index e680f75a5b5e..1b5185697011 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -224,14 +224,21 @@ static struct resource *pbus_select_window_for_type(struct pci_bus *bus,
>
> switch (iores_type) {
> case IORESOURCE_IO:
> - return pci_bus_resource_n(bus, PCI_BUS_BRIDGE_IO_WINDOW);
> + win = pci_bus_resource_n(bus, PCI_BUS_BRIDGE_IO_WINDOW);
> + if (win && (win->flags & IORESOURCE_IO))
> + return win;
> + return NULL;
>
> case IORESOURCE_MEM:
> mmio = pci_bus_resource_n(bus, PCI_BUS_BRIDGE_MEM_WINDOW);
> mmio_pref = pci_bus_resource_n(bus, PCI_BUS_BRIDGE_PREF_MEM_WINDOW);
>
> - if (!(type & IORESOURCE_PREFETCH) ||
> - !(mmio_pref->flags & IORESOURCE_MEM))
> + if (mmio && !(mmio->flags & IORESOURCE_MEM))
> + mmio = NULL;
> + if (mmio_pref && !(mmio_pref->flags & IORESOURCE_MEM))
> + mmio_pref = NULL;
> +
> + if (!(type & IORESOURCE_PREFETCH) || !mmio_pref)
> return mmio;
>
> if ((type & IORESOURCE_MEM_64) ||
Thanks for fixing this,
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] PCI: Validate window resource type in pbus_select_window_for_type()
2026-02-10 14:20 [PATCH] PCI: Validate window resource type in pbus_select_window_for_type() Kai-Heng Feng
2026-02-10 15:19 ` Ilpo Järvinen
@ 2026-02-10 17:26 ` Bjorn Helgaas
2026-02-12 15:13 ` Ilpo Järvinen
2026-02-12 17:09 ` Bjorn Helgaas
2 siblings, 1 reply; 5+ messages in thread
From: Bjorn Helgaas @ 2026-02-10 17:26 UTC (permalink / raw)
To: Kai-Heng Feng; +Cc: bhelgaas, ilpo.jarvinen, linux-pci, linux-kernel
On Tue, Feb 10, 2026 at 10:20:57PM +0800, Kai-Heng Feng wrote:
> Afer commit ebe091ad81e1 ("PCI: Use pbus_select_window_for_type() during
> IO window sizing") and commit ae88d0b9c57f ("PCI: Use
> pbus_select_window_for_type() during mem window sizing"), many bridge
> window can't get resource assigned:
> pci 0006:05:00.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: can't assign; no space
> pci 0006:05:00.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: failed to assign
> pci 0006:05:04.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: can't assign; no space
> pci 0006:05:04.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: failed to assign
> pci 0006:05:08.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: can't assign; no space
> pci 0006:05:08.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: failed to assign
> pci 0006:05:0c.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: can't assign; no space
> pci 0006:05:0c.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: failed to assign
>
> Those commits replace find_bus_resource_of_type() with
> pbus_select_window_for_type(), and the latter lacks resource type
> validation.
>
> This commit adds the resource type validation back to
> pbus_select_window_for_type() to match the original behavior.
>
> Fixes: 74afce3dfcba ("PCI: Add bridge window selection functions")
74afce3dfcba, ebe091ad81e1, and ae88d0b9c57f all appeared in v6.18.
Interesting that nobody else reported this since. In any case, this
looks like post-merge window v7.0 material after an ack from Ilpo,
probably with a stable tag.
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=221072
> Signed-off-by: Kai-Heng Feng <kaihengf@nvidia.com>
Thanks for bisecting and fixing this! From the "dmesg with fix"
attachment (https://bugzilla.kernel.org/attachment.cgi?id=309333):
pci_bus 0006:00: root bus resource [mem 0x618040000000-0x6180bfffffff window] (bus address [0x40000000-0xbfffffff])
pci_bus 0006:00: root bus resource [mem 0x6180c0000000-0x627fffffffff window]
pci 0006:05:00.0: [15b3:197b] type 01 class 0x060400 PCIe Switch Downstream Port
pci 0006:05:00.0: PCI bridge to [bus 06]
pci 0006:05:00.0: bridge window [mem 0x618040300000-0x6180403fffff]
Looks like the 32-bit (non-prefetchable) window was programmed by
firmware but the 64-bit window was not programmed or maybe programmed
incorrectly (a dmesg with "pci=earlydump" would show the raw state
Linux received):
pci 0006:05:00.0: bridge window [mem 0x00000000 64bit pref] to [bus 06] requires relaxed alignment rules
pci 0006:05:00.0: bridge window [mem 0x00100000-0x000fffff 64bit pref] to [bus 06] add_size 200000 add_align 100000
For hex_range() to print a resource with a single address like the
"relaxed alignment" line, I think the resource must have
start==end==0, which would mean a window at 0x0 that's one byte long.
A memory window res.end should always have 0xfffff in the low 20 bits
because these windows are a multiple of 1MB in size, so this suggests
another bug somewhere.
pci 0006:05:00.0: bridge window [mem 0x00100000-0x001fffff] to [bus 06] requires relaxed alignment rules
pci 0006:05:00.0: bridge window [mem 0x00100000-0x001fffff] to [bus 06] add_size 200000 add_align 100000
pci 0006:05:00.0: bridge window [mem 0x618040200000-0x6180404fffff]: assigned
I think these three lines all refer to the 32-bit window. It's a bit
confusing that the logging shows what looks like a
IORESOURCE_STARTALIGN and size (same applies to the 64-bit window
above). I wonder if resource_string() should do something different
when decoding this so it doesn't look like we actually programmed the
window to this. And maybe pci_resource_name() should explicitly say
which window this is.
Also slightly interesting that we reassigned the 32-bit window,
increasing its size from 1MB to 3MB. Maybe this is because the bridge
supports hotplug.
Unrelated nit: I think we should add "0x" prefixes for add_size and
add_align to match the addresses.
pci 0006:05:00.0: bridge window [mem 0x6180c8800000-0x6180c89fffff 64bit pref]: assigned
pci 0006:05:00.0: PCI bridge to [bus 06]
pci 0006:05:00.0: bridge window [mem 0x618040200000-0x6180404fffff]
pci 0006:05:00.0: bridge window [mem 0x6180c8800000-0x6180c89fffff 64bit pref]
> ---
> drivers/pci/setup-bus.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index e680f75a5b5e..1b5185697011 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -224,14 +224,21 @@ static struct resource *pbus_select_window_for_type(struct pci_bus *bus,
>
> switch (iores_type) {
> case IORESOURCE_IO:
> - return pci_bus_resource_n(bus, PCI_BUS_BRIDGE_IO_WINDOW);
> + win = pci_bus_resource_n(bus, PCI_BUS_BRIDGE_IO_WINDOW);
> + if (win && (win->flags & IORESOURCE_IO))
> + return win;
> + return NULL;
>
> case IORESOURCE_MEM:
> mmio = pci_bus_resource_n(bus, PCI_BUS_BRIDGE_MEM_WINDOW);
> mmio_pref = pci_bus_resource_n(bus, PCI_BUS_BRIDGE_PREF_MEM_WINDOW);
>
> - if (!(type & IORESOURCE_PREFETCH) ||
> - !(mmio_pref->flags & IORESOURCE_MEM))
> + if (mmio && !(mmio->flags & IORESOURCE_MEM))
> + mmio = NULL;
> + if (mmio_pref && !(mmio_pref->flags & IORESOURCE_MEM))
> + mmio_pref = NULL;
> +
> + if (!(type & IORESOURCE_PREFETCH) || !mmio_pref)
> return mmio;
>
> if ((type & IORESOURCE_MEM_64) ||
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] PCI: Validate window resource type in pbus_select_window_for_type()
2026-02-10 17:26 ` Bjorn Helgaas
@ 2026-02-12 15:13 ` Ilpo Järvinen
0 siblings, 0 replies; 5+ messages in thread
From: Ilpo Järvinen @ 2026-02-12 15:13 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Kai-Heng Feng, bhelgaas, linux-pci, LKML
On Tue, 10 Feb 2026, Bjorn Helgaas wrote:
> On Tue, Feb 10, 2026 at 10:20:57PM +0800, Kai-Heng Feng wrote:
> > Afer commit ebe091ad81e1 ("PCI: Use pbus_select_window_for_type() during
> > IO window sizing") and commit ae88d0b9c57f ("PCI: Use
> > pbus_select_window_for_type() during mem window sizing"), many bridge
> > window can't get resource assigned:
> > pci 0006:05:00.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: can't assign; no space
> > pci 0006:05:00.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: failed to assign
> > pci 0006:05:04.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: can't assign; no space
> > pci 0006:05:04.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: failed to assign
> > pci 0006:05:08.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: can't assign; no space
> > pci 0006:05:08.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: failed to assign
> > pci 0006:05:0c.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: can't assign; no space
> > pci 0006:05:0c.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: failed to assign
> >
> > Those commits replace find_bus_resource_of_type() with
> > pbus_select_window_for_type(), and the latter lacks resource type
> > validation.
> >
> > This commit adds the resource type validation back to
> > pbus_select_window_for_type() to match the original behavior.
> >
> > Fixes: 74afce3dfcba ("PCI: Add bridge window selection functions")
>
> 74afce3dfcba, ebe091ad81e1, and ae88d0b9c57f all appeared in v6.18.
> Interesting that nobody else reported this since. In any case, this
> looks like post-merge window v7.0 material after an ack from Ilpo,
> probably with a stable tag.
The change seemed fine to me.
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=221072
> > Signed-off-by: Kai-Heng Feng <kaihengf@nvidia.com>
>
> Thanks for bisecting and fixing this! From the "dmesg with fix"
> attachment (https://bugzilla.kernel.org/attachment.cgi?id=309333):
>
> pci_bus 0006:00: root bus resource [mem 0x618040000000-0x6180bfffffff window] (bus address [0x40000000-0xbfffffff])
> pci_bus 0006:00: root bus resource [mem 0x6180c0000000-0x627fffffffff window]
> pci 0006:05:00.0: [15b3:197b] type 01 class 0x060400 PCIe Switch Downstream Port
> pci 0006:05:00.0: PCI bridge to [bus 06]
> pci 0006:05:00.0: bridge window [mem 0x618040300000-0x6180403fffff]
>
> Looks like the 32-bit (non-prefetchable) window was programmed by
> firmware but the 64-bit window was not programmed or maybe programmed
> incorrectly (a dmesg with "pci=earlydump" would show the raw state
> Linux received):
>
> pci 0006:05:00.0: bridge window [mem 0x00000000 64bit pref] to [bus 06] requires relaxed alignment rules
> pci 0006:05:00.0: bridge window [mem 0x00100000-0x000fffff 64bit pref] to [bus 06] add_size 200000 add_align 100000
>
> For hex_range() to print a resource with a single address like the
> "relaxed alignment" line, I think the resource must have
> start==end==0, which would mean a window at 0x0 that's one byte long.
>
> A memory window res.end should always have 0xfffff in the low 20 bits
> because these windows are a multiple of 1MB in size, so this suggests
> another bug somewhere.
I tried to find places where the kernel could mess the end address up but
unfortunately couldn't come up anything, the commit 8278c6914306 ("PCI:
Preserve bridge window resource type flags") should take care
initializing the window resource always (if the bridge window is read,
obviously).
Perhaps the bridge window configuration was not read for that window,
but feels odd given it has the type. One possibility would be that
pci_bridge_check_ranges() set the type based on ->pref_window but
pci_read_bridge_bases() never executed so the resource addresses were
never filled (pci_read_bridge_windows() that fills ->pref_window only
reads the window's addresses to a dummy resource). I'd prefer to kill
pci_bridge_check_ranges() as it has chance of messing things up like this
but that requires that one series to setup the bridge windows earlier we
had to revert.
It's unclear to me if the commit 15c5867b0ae6 ("PCI: Don't print stale
information about resource") is in the kernel these logs were taken from.
It does affect in what state the resource gets printed.
> pci 0006:05:00.0: bridge window [mem 0x00100000-0x001fffff] to [bus 06] requires relaxed alignment rules
> pci 0006:05:00.0: bridge window [mem 0x00100000-0x001fffff] to [bus 06] add_size 200000 add_align 100000
> pci 0006:05:00.0: bridge window [mem 0x618040200000-0x6180404fffff]: assigned
>
> I think these three lines all refer to the 32-bit window. It's a bit
> confusing that the logging shows what looks like a
> IORESOURCE_STARTALIGN and size (same applies to the 64-bit window
> above). I wonder if resource_string() should do something different
> when decoding this so it doesn't look like we actually programmed the
> window to this.
> And maybe pci_resource_name() should explicitly say
> which window this is.
There was mention somewhere that it's not included because the resource
bits should indicate that information.
I'd much prefer that info to be visible though, it's okay to not have it
explicit when things work as expected but while troubleshooting the
ambiguity is very annoying when those dreaded ??? appear.
> Also slightly interesting that we reassigned the 32-bit window,
> increasing its size from 1MB to 3MB. Maybe this is because the bridge
> supports hotplug.
It has add_size so the expected size is resource size + add_size.
> Unrelated nit: I think we should add "0x" prefixes for add_size and
> add_align to match the addresses.
>
> pci 0006:05:00.0: bridge window [mem 0x6180c8800000-0x6180c89fffff 64bit pref]: assigned
>
> pci 0006:05:00.0: PCI bridge to [bus 06]
> pci 0006:05:00.0: bridge window [mem 0x618040200000-0x6180404fffff]
> pci 0006:05:00.0: bridge window [mem 0x6180c8800000-0x6180c89fffff 64bit pref]
>
> > ---
> > drivers/pci/setup-bus.c | 13 ++++++++++---
> > 1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> > index e680f75a5b5e..1b5185697011 100644
> > --- a/drivers/pci/setup-bus.c
> > +++ b/drivers/pci/setup-bus.c
> > @@ -224,14 +224,21 @@ static struct resource *pbus_select_window_for_type(struct pci_bus *bus,
> >
> > switch (iores_type) {
> > case IORESOURCE_IO:
> > - return pci_bus_resource_n(bus, PCI_BUS_BRIDGE_IO_WINDOW);
> > + win = pci_bus_resource_n(bus, PCI_BUS_BRIDGE_IO_WINDOW);
> > + if (win && (win->flags & IORESOURCE_IO))
> > + return win;
> > + return NULL;
> >
> > case IORESOURCE_MEM:
> > mmio = pci_bus_resource_n(bus, PCI_BUS_BRIDGE_MEM_WINDOW);
> > mmio_pref = pci_bus_resource_n(bus, PCI_BUS_BRIDGE_PREF_MEM_WINDOW);
> >
> > - if (!(type & IORESOURCE_PREFETCH) ||
> > - !(mmio_pref->flags & IORESOURCE_MEM))
> > + if (mmio && !(mmio->flags & IORESOURCE_MEM))
> > + mmio = NULL;
> > + if (mmio_pref && !(mmio_pref->flags & IORESOURCE_MEM))
> > + mmio_pref = NULL;
> > +
> > + if (!(type & IORESOURCE_PREFETCH) || !mmio_pref)
> > return mmio;
> >
> > if ((type & IORESOURCE_MEM_64) ||
> > --
> > 2.43.0
> >
>
--
i.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] PCI: Validate window resource type in pbus_select_window_for_type()
2026-02-10 14:20 [PATCH] PCI: Validate window resource type in pbus_select_window_for_type() Kai-Heng Feng
2026-02-10 15:19 ` Ilpo Järvinen
2026-02-10 17:26 ` Bjorn Helgaas
@ 2026-02-12 17:09 ` Bjorn Helgaas
2 siblings, 0 replies; 5+ messages in thread
From: Bjorn Helgaas @ 2026-02-12 17:09 UTC (permalink / raw)
To: Kai-Heng Feng; +Cc: bhelgaas, ilpo.jarvinen, linux-pci, linux-kernel
On Tue, Feb 10, 2026 at 10:20:57PM +0800, Kai-Heng Feng wrote:
> Afer commit ebe091ad81e1 ("PCI: Use pbus_select_window_for_type() during
> IO window sizing") and commit ae88d0b9c57f ("PCI: Use
> pbus_select_window_for_type() during mem window sizing"), many bridge
> window can't get resource assigned:
> pci 0006:05:00.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: can't assign; no space
> pci 0006:05:00.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: failed to assign
> pci 0006:05:04.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: can't assign; no space
> pci 0006:05:04.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: failed to assign
> pci 0006:05:08.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: can't assign; no space
> pci 0006:05:08.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: failed to assign
> pci 0006:05:0c.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: can't assign; no space
> pci 0006:05:0c.0: bridge window [??? 0x00001000-0x00001fff flags 0x20080000]: failed to assign
>
> Those commits replace find_bus_resource_of_type() with
> pbus_select_window_for_type(), and the latter lacks resource type
> validation.
>
> This commit adds the resource type validation back to
> pbus_select_window_for_type() to match the original behavior.
>
> Fixes: 74afce3dfcba ("PCI: Add bridge window selection functions")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=221072
> Signed-off-by: Kai-Heng Feng <kaihengf@nvidia.com>
Applied to for-linus for v7.0, thanks!
> ---
> drivers/pci/setup-bus.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index e680f75a5b5e..1b5185697011 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -224,14 +224,21 @@ static struct resource *pbus_select_window_for_type(struct pci_bus *bus,
>
> switch (iores_type) {
> case IORESOURCE_IO:
> - return pci_bus_resource_n(bus, PCI_BUS_BRIDGE_IO_WINDOW);
> + win = pci_bus_resource_n(bus, PCI_BUS_BRIDGE_IO_WINDOW);
> + if (win && (win->flags & IORESOURCE_IO))
> + return win;
> + return NULL;
>
> case IORESOURCE_MEM:
> mmio = pci_bus_resource_n(bus, PCI_BUS_BRIDGE_MEM_WINDOW);
> mmio_pref = pci_bus_resource_n(bus, PCI_BUS_BRIDGE_PREF_MEM_WINDOW);
>
> - if (!(type & IORESOURCE_PREFETCH) ||
> - !(mmio_pref->flags & IORESOURCE_MEM))
> + if (mmio && !(mmio->flags & IORESOURCE_MEM))
> + mmio = NULL;
> + if (mmio_pref && !(mmio_pref->flags & IORESOURCE_MEM))
> + mmio_pref = NULL;
> +
> + if (!(type & IORESOURCE_PREFETCH) || !mmio_pref)
> return mmio;
>
> if ((type & IORESOURCE_MEM_64) ||
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread