1. Patchew
  2. linux
  3. [PATCH v2 0/3] PCI: mediatek: Add support for EcoNet SoCs
  4. View patch

[PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported

Caleb James DeLisle posted 3 patches 3 weeks ago
There is a newer version of this series
[PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported
Posted by Caleb James DeLisle 3 weeks ago 
pci_read_bridge_io() and pci_read_bridge_mmio_pref() read bridge window
registers unconditionally. If the registers are hardwired to zero
(not implemented), both base and limit will be 0. Since (0 <= 0) is
true, a bogus window [mem 0x00000000-0x000fffff] or [io 0x0000-0x0fff]
gets created.

pci_read_bridge_windows() already detects unsupported windows by
testing register writability and sets io_window/pref_window flags
accordingly. Check these flags at the start of pci_read_bridge_io()
and pci_read_bridge_mmio_pref() to skip reading registers when the
window is not supported.

Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Link: https://lore.kernel.org/all/20260113210259.GA715789@bhelgaas/
Signed-off-by: Ahmed Naseef <naseefkm@gmail.com>
Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
---
 drivers/pci/probe.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index bccc7a4bdd79..4eacb741b4ec 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -395,6 +395,9 @@ static void pci_read_bridge_io(struct pci_dev *dev, struct resource *res,
 	unsigned long io_mask, io_granularity, base, limit;
 	struct pci_bus_region region;
 
+	if (!dev->io_window)
+		return;
+
 	io_mask = PCI_IO_RANGE_MASK;
 	io_granularity = 0x1000;
 	if (dev->io_window_1k) {
@@ -465,6 +468,9 @@ static void pci_read_bridge_mmio_pref(struct pci_dev *dev, struct resource *res,
 	pci_bus_addr_t base, limit;
 	struct pci_bus_region region;
 
+	if (!dev->pref_window)
+		return;
+
 	pci_read_config_word(dev, PCI_PREF_MEMORY_BASE, &mem_base_lo);
 	pci_read_config_word(dev, PCI_PREF_MEMORY_LIMIT, &mem_limit_lo);
 	base64 = (mem_base_lo & PCI_PREF_RANGE_MASK) << 16;
-- 
2.39.5
Re: [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported
Posted by Bjorn Helgaas 2 weeks, 5 days ago 
On Mon, Mar 16, 2026 at 03:51:57PM +0000, Caleb James DeLisle wrote:
> pci_read_bridge_io() and pci_read_bridge_mmio_pref() read bridge window
> registers unconditionally. If the registers are hardwired to zero
> (not implemented), both base and limit will be 0. Since (0 <= 0) is
> true, a bogus window [mem 0x00000000-0x000fffff] or [io 0x0000-0x0fff]
> gets created.
> 
> pci_read_bridge_windows() already detects unsupported windows by
> testing register writability and sets io_window/pref_window flags
> accordingly. Check these flags at the start of pci_read_bridge_io()
> and pci_read_bridge_mmio_pref() to skip reading registers when the
> window is not supported.
> 
> Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
> Link: https://lore.kernel.org/all/20260113210259.GA715789@bhelgaas/
> Signed-off-by: Ahmed Naseef <naseefkm@gmail.com>
> Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>

I applied this to pci/resource for v7.1 with the following commit log:

  PCI: Prevent assignment to unsupported bridge windows

  Previously, pci_read_bridge_io() and pci_read_bridge_mmio_pref()
  unconditionally set resource type flags (IORESOURCE_IO or IORESOURCE_MEM |
  IORESOURCE_PREFETCH) when reading bridge window registers. For windows that
  are not implemented in hardware, this may cause the allocator to assign
  space for a window that doesn't exist.

  For example, the EcoNET EN7528 SoC Root Port doesn't support the
  prefetchable window, but since a downstream device had a prefetchable BAR,
  the allocator mistakenly assigned a prefetchable window:

    pci 0001:00:01.0: [14c3:0811] type 01 class 0x060400 PCIe Root Port
    pci 0001:00:01.0: PCI bridge to [bus 01-ff]
    pci 0001:00:01.0: bridge window [mem 0x28000000-0x280fffff]: assigned
    pci 0001:00:01.0: bridge window [mem 0x28100000-0x282fffff pref]: assigned
    pci 0001:01:00.0: BAR 0 [mem 0x28100000-0x281fffff 64bit pref]: assigned

  pci_read_bridge_windows() already detects unsupported windows by testing
  register writability and sets dev->io_window/pref_window accordingly.

  Check dev->io_window/pref_window so we don't set the resource flags for
  unsupported windows, which prevents the allocator from assigning space to
  them.

  After this commit, the prefetchable BAR is correctly allocated from the
  non-prefetchable window:

    pci 0001:00:01.0: bridge window [mem 0x28000000-0x281fffff]: assigned
    pci 0001:01:00.0: BAR 0 [mem 0x28000000-0x280fffff 64bit pref]: assigned

I also set the author to "Ahmed Naseef <naseefkm@gmail.com>" per
https://lore.kernel.org/all/abRQYM1If/6Vv/tI@DESKTOP-TIT0J8O.localdomain

You can make this work correctly next time by including a
"From: Ahmed Naseef <naseefkm@gmail.com>" line as the very first line
in the message body; see
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=v6.19#n723

> ---
>  drivers/pci/probe.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index bccc7a4bdd79..4eacb741b4ec 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -395,6 +395,9 @@ static void pci_read_bridge_io(struct pci_dev *dev, struct resource *res,
>  	unsigned long io_mask, io_granularity, base, limit;
>  	struct pci_bus_region region;
>  
> +	if (!dev->io_window)
> +		return;
> +
>  	io_mask = PCI_IO_RANGE_MASK;
>  	io_granularity = 0x1000;
>  	if (dev->io_window_1k) {
> @@ -465,6 +468,9 @@ static void pci_read_bridge_mmio_pref(struct pci_dev *dev, struct resource *res,
>  	pci_bus_addr_t base, limit;
>  	struct pci_bus_region region;
>  
> +	if (!dev->pref_window)
> +		return;
> +
>  	pci_read_config_word(dev, PCI_PREF_MEMORY_BASE, &mem_base_lo);
>  	pci_read_config_word(dev, PCI_PREF_MEMORY_LIMIT, &mem_limit_lo);
>  	base64 = (mem_base_lo & PCI_PREF_RANGE_MASK) << 16;
> -- 
> 2.39.5
>
Re: [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported
Posted by Caleb James DeLisle 2 weeks, 5 days ago 
On 18/03/2026 22:58, Bjorn Helgaas wrote:
> On Mon, Mar 16, 2026 at 03:51:57PM +0000, Caleb James DeLisle wrote:
>> pci_read_bridge_io() and pci_read_bridge_mmio_pref() read bridge window
>> registers unconditionally. If the registers are hardwired to zero
>> (not implemented), both base and limit will be 0. Since (0 <= 0) is
>> true, a bogus window [mem 0x00000000-0x000fffff] or [io 0x0000-0x0fff]
>> gets created.
>>
>> pci_read_bridge_windows() already detects unsupported windows by
>> testing register writability and sets io_window/pref_window flags
>> accordingly. Check these flags at the start of pci_read_bridge_io()
>> and pci_read_bridge_mmio_pref() to skip reading registers when the
>> window is not supported.
>>
>> Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
>> Link: https://lore.kernel.org/all/20260113210259.GA715789@bhelgaas/
>> Signed-off-by: Ahmed Naseef <naseefkm@gmail.com>
>> Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
> I applied this to pci/resource for v7.1 with the following commit log:
>
>    PCI: Prevent assignment to unsupported bridge windows
>
>    Previously, pci_read_bridge_io() and pci_read_bridge_mmio_pref()
>    unconditionally set resource type flags (IORESOURCE_IO or IORESOURCE_MEM |
>    IORESOURCE_PREFETCH) when reading bridge window registers. For windows that
>    are not implemented in hardware, this may cause the allocator to assign
>    space for a window that doesn't exist.
>
>    For example, the EcoNET EN7528 SoC Root Port doesn't support the
>    prefetchable window, but since a downstream device had a prefetchable BAR,
>    the allocator mistakenly assigned a prefetchable window:
>
>      pci 0001:00:01.0: [14c3:0811] type 01 class 0x060400 PCIe Root Port
>      pci 0001:00:01.0: PCI bridge to [bus 01-ff]
>      pci 0001:00:01.0: bridge window [mem 0x28000000-0x280fffff]: assigned
>      pci 0001:00:01.0: bridge window [mem 0x28100000-0x282fffff pref]: assigned
>      pci 0001:01:00.0: BAR 0 [mem 0x28100000-0x281fffff 64bit pref]: assigned
>
>    pci_read_bridge_windows() already detects unsupported windows by testing
>    register writability and sets dev->io_window/pref_window accordingly.
>
>    Check dev->io_window/pref_window so we don't set the resource flags for
>    unsupported windows, which prevents the allocator from assigning space to
>    them.
>
>    After this commit, the prefetchable BAR is correctly allocated from the
>    non-prefetchable window:
>
>      pci 0001:00:01.0: bridge window [mem 0x28000000-0x281fffff]: assigned
>      pci 0001:01:00.0: BAR 0 [mem 0x28000000-0x280fffff 64bit pref]: assigned
>
> I also set the author to "Ahmed Naseef <naseefkm@gmail.com>" per
> https://lore.kernel.org/all/abRQYM1If/6Vv/tI@DESKTOP-TIT0J8O.localdomain
>
> You can make this work correctly next time by including a
> "From: Ahmed Naseef <naseefkm@gmail.com>" line as the very first line
> in the message body; see
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=v6.19#n723

Thank you very much. I wasn't quite sure how to submit this so thanks 
and I will keep that in mind.

Thanks,

Caleb

>> ---
>>   drivers/pci/probe.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index bccc7a4bdd79..4eacb741b4ec 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -395,6 +395,9 @@ static void pci_read_bridge_io(struct pci_dev *dev, struct resource *res,
>>   	unsigned long io_mask, io_granularity, base, limit;
>>   	struct pci_bus_region region;
>>   
>> +	if (!dev->io_window)
>> +		return;
>> +
>>   	io_mask = PCI_IO_RANGE_MASK;
>>   	io_granularity = 0x1000;
>>   	if (dev->io_window_1k) {
>> @@ -465,6 +468,9 @@ static void pci_read_bridge_mmio_pref(struct pci_dev *dev, struct resource *res,
>>   	pci_bus_addr_t base, limit;
>>   	struct pci_bus_region region;
>>   
>> +	if (!dev->pref_window)
>> +		return;
>> +
>>   	pci_read_config_word(dev, PCI_PREF_MEMORY_BASE, &mem_base_lo);
>>   	pci_read_config_word(dev, PCI_PREF_MEMORY_LIMIT, &mem_limit_lo);
>>   	base64 = (mem_base_lo & PCI_PREF_RANGE_MASK) << 16;
>> -- 
>> 2.39.5
>>
Re: [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported
Posted by Bjorn Helgaas 2 weeks, 6 days ago 
On Mon, Mar 16, 2026 at 03:51:57PM +0000, Caleb James DeLisle wrote:
> pci_read_bridge_io() and pci_read_bridge_mmio_pref() read bridge window
> registers unconditionally. If the registers are hardwired to zero
> (not implemented), both base and limit will be 0. Since (0 <= 0) is
> true, a bogus window [mem 0x00000000-0x000fffff] or [io 0x0000-0x0fff]
> gets created.
> 
> pci_read_bridge_windows() already detects unsupported windows by
> testing register writability and sets io_window/pref_window flags
> accordingly. Check these flags at the start of pci_read_bridge_io()
> and pci_read_bridge_mmio_pref() to skip reading registers when the
> window is not supported.

The fundamental problem here is that assigned space to a bridge window
that isn't implemented.  I wish we understood the connection between
this "read window" path and the assignment path.

Maybe this patch fixes it because we enter pci_read_bridge_mmio_pref()
with res->flags being NULL, and we set IORESOURCE_MEM |
IORESOURCE_PREFETCH again, which makes it look like we can assign
space for it?

If that's the case, I think it would improve the commit log to mention
the actual mechanism by which we avoid assigning space.

> Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
> Link: https://lore.kernel.org/all/20260113210259.GA715789@bhelgaas/
> Signed-off-by: Ahmed Naseef <naseefkm@gmail.com>
> Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
> ---
>  drivers/pci/probe.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index bccc7a4bdd79..4eacb741b4ec 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -395,6 +395,9 @@ static void pci_read_bridge_io(struct pci_dev *dev, struct resource *res,
>  	unsigned long io_mask, io_granularity, base, limit;
>  	struct pci_bus_region region;
>  
> +	if (!dev->io_window)
> +		return;
> +
>  	io_mask = PCI_IO_RANGE_MASK;
>  	io_granularity = 0x1000;
>  	if (dev->io_window_1k) {
> @@ -465,6 +468,9 @@ static void pci_read_bridge_mmio_pref(struct pci_dev *dev, struct resource *res,
>  	pci_bus_addr_t base, limit;
>  	struct pci_bus_region region;
>  
> +	if (!dev->pref_window)
> +		return;
> +
>  	pci_read_config_word(dev, PCI_PREF_MEMORY_BASE, &mem_base_lo);
>  	pci_read_config_word(dev, PCI_PREF_MEMORY_LIMIT, &mem_limit_lo);
>  	base64 = (mem_base_lo & PCI_PREF_RANGE_MASK) << 16;
> -- 
> 2.39.5
>
Re: [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported
Posted by Ahmed Naseef 2 weeks, 5 days ago 
On Tue, Mar 17, 2026 at 04:29:08PM -0500, Bjorn Helgaas wrote:
> On Mon, Mar 16, 2026 at 03:51:57PM +0000, Caleb James DeLisle wrote:
> > pci_read_bridge_io() and pci_read_bridge_mmio_pref() read bridge window
> > registers unconditionally. If the registers are hardwired to zero
> > (not implemented), both base and limit will be 0. Since (0 <= 0) is
> > true, a bogus window [mem 0x00000000-0x000fffff] or [io 0x0000-0x0fff]
> > gets created.
> > 
> > pci_read_bridge_windows() already detects unsupported windows by
> > testing register writability and sets io_window/pref_window flags
> > accordingly. Check these flags at the start of pci_read_bridge_io()
> > and pci_read_bridge_mmio_pref() to skip reading registers when the
> > window is not supported.
> 
> The fundamental problem here is that assigned space to a bridge window
> that isn't implemented.  I wish we understood the connection between
> this "read window" path and the assignment path.
> 
> Maybe this patch fixes it because we enter pci_read_bridge_mmio_pref()
> with res->flags being NULL, and we set IORESOURCE_MEM |
> IORESOURCE_PREFETCH again, which makes it look like we can assign
> space for it?

Yes, that's exactly right.

> 
> If that's the case, I think it would improve the commit log to mention
> the actual mechanism by which we avoid assigning space.
> 

How about this:

  pci_read_bridge_io() and pci_read_bridge_mmio_pref() read
  bridge window registers unconditionally. If the registers
  are hardwired to zero (not implemented), both base and limit
  will be 0. Since (0 <= 0) is true, these functions set
  IORESOURCE_IO or IORESOURCE_MEM | IORESOURCE_PREFETCH on
  the bridge resource. This causes the allocator to assign
  space for the window even though the hardware can't
  implement it.

  pci_read_bridge_windows() already detects unsupported windows
  by testing register writability and sets io_window/pref_window
  flags accordingly. Check these flags at the start of
  pci_read_bridge_io() and pci_read_bridge_mmio_pref() to skip
  reading registers when the window is not supported, so the
  resource flags remain clear and the allocator does not assign
  space for non-existent windows.


Ahmed Naseef

> > Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
> > Link: https://lore.kernel.org/all/20260113210259.GA715789@bhelgaas/
> > Signed-off-by: Ahmed Naseef <naseefkm@gmail.com>
> > Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
> > ---
> >  drivers/pci/probe.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index bccc7a4bdd79..4eacb741b4ec 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -395,6 +395,9 @@ static void pci_read_bridge_io(struct pci_dev *dev, struct resource *res,
> >  	unsigned long io_mask, io_granularity, base, limit;
> >  	struct pci_bus_region region;
> >  
> > +	if (!dev->io_window)
> > +		return;
> > +
> >  	io_mask = PCI_IO_RANGE_MASK;
> >  	io_granularity = 0x1000;
> >  	if (dev->io_window_1k) {
> > @@ -465,6 +468,9 @@ static void pci_read_bridge_mmio_pref(struct pci_dev *dev, struct resource *res,
> >  	pci_bus_addr_t base, limit;
> >  	struct pci_bus_region region;
> >  
> > +	if (!dev->pref_window)
> > +		return;
> > +
> >  	pci_read_config_word(dev, PCI_PREF_MEMORY_BASE, &mem_base_lo);
> >  	pci_read_config_word(dev, PCI_PREF_MEMORY_LIMIT, &mem_limit_lo);
> >  	base64 = (mem_base_lo & PCI_PREF_RANGE_MASK) << 16;
> > -- 
> > 2.39.5
> >
Re: [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported
Posted by Ilpo Järvinen 2 weeks, 5 days ago 
On Wed, 18 Mar 2026, Ahmed Naseef wrote:

> On Tue, Mar 17, 2026 at 04:29:08PM -0500, Bjorn Helgaas wrote:
> > On Mon, Mar 16, 2026 at 03:51:57PM +0000, Caleb James DeLisle wrote:
> > > pci_read_bridge_io() and pci_read_bridge_mmio_pref() read bridge window
> > > registers unconditionally. If the registers are hardwired to zero
> > > (not implemented), both base and limit will be 0. Since (0 <= 0) is
> > > true, a bogus window [mem 0x00000000-0x000fffff] or [io 0x0000-0x0fff]
> > > gets created.
> > > 
> > > pci_read_bridge_windows() already detects unsupported windows by
> > > testing register writability and sets io_window/pref_window flags
> > > accordingly. Check these flags at the start of pci_read_bridge_io()
> > > and pci_read_bridge_mmio_pref() to skip reading registers when the
> > > window is not supported.
> > 
> > The fundamental problem here is that assigned space to a bridge window
> > that isn't implemented.  I wish we understood the connection between
> > this "read window" path and the assignment path.
> > 
> > Maybe this patch fixes it because we enter pci_read_bridge_mmio_pref()
> > with res->flags being NULL, and we set IORESOURCE_MEM |
> > IORESOURCE_PREFETCH again, which makes it look like we can assign
> > space for it?
> 
> Yes, that's exactly right.
> 
> > 
> > If that's the case, I think it would improve the commit log to mention
> > the actual mechanism by which we avoid assigning space.
> > 
> 
> How about this:
> 
>   pci_read_bridge_io() and pci_read_bridge_mmio_pref() read
>   bridge window registers unconditionally. If the registers
>   are hardwired to zero (not implemented), both base and limit
>   will be 0. Since (0 <= 0) is true, these functions set
>   IORESOURCE_IO or IORESOURCE_MEM | IORESOURCE_PREFETCH on
>   the bridge resource. This causes the allocator to assign
>   space for the window even though the hardware can't
>   implement it.
> 
>   pci_read_bridge_windows() already detects unsupported windows
>   by testing register writability and sets io_window/pref_window
>   flags accordingly. Check these flags at the start of
>   pci_read_bridge_io() and pci_read_bridge_mmio_pref() to skip
>   reading registers when the window is not supported, so the
>   resource flags remain clear and the allocator does not assign
>   space for non-existent windows.

At least to me the proposed text reads much better than the original.
The original text required reading between the lines to connect the dots, 
whereas this new one clearly explains what causes what.

--
 i.

> Ahmed Naseef
> 
> > > Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
> > > Link: https://lore.kernel.org/all/20260113210259.GA715789@bhelgaas/
> > > Signed-off-by: Ahmed Naseef <naseefkm@gmail.com>
> > > Signed-off-by: Caleb James DeLisle <cjd@cjdns.fr>
> > > ---
> > >  drivers/pci/probe.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > > index bccc7a4bdd79..4eacb741b4ec 100644
> > > --- a/drivers/pci/probe.c
> > > +++ b/drivers/pci/probe.c
> > > @@ -395,6 +395,9 @@ static void pci_read_bridge_io(struct pci_dev *dev, struct resource *res,
> > >  	unsigned long io_mask, io_granularity, base, limit;
> > >  	struct pci_bus_region region;
> > >  
> > > +	if (!dev->io_window)
> > > +		return;
> > > +
> > >  	io_mask = PCI_IO_RANGE_MASK;
> > >  	io_granularity = 0x1000;
> > >  	if (dev->io_window_1k) {
> > > @@ -465,6 +468,9 @@ static void pci_read_bridge_mmio_pref(struct pci_dev *dev, struct resource *res,
> > >  	pci_bus_addr_t base, limit;
> > >  	struct pci_bus_region region;
> > >  
> > > +	if (!dev->pref_window)
> > > +		return;
> > > +
> > >  	pci_read_config_word(dev, PCI_PREF_MEMORY_BASE, &mem_base_lo);
> > >  	pci_read_config_word(dev, PCI_PREF_MEMORY_LIMIT, &mem_limit_lo);
> > >  	base64 = (mem_base_lo & PCI_PREF_RANGE_MASK) << 16;
> > > -- 
> > > 2.39.5
> > > 
>
Re: [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported
Posted by Ilpo Järvinen 2 weeks, 5 days ago 
On Wed, 18 Mar 2026, Ilpo Järvinen wrote:

> On Wed, 18 Mar 2026, Ahmed Naseef wrote:
> 
> > On Tue, Mar 17, 2026 at 04:29:08PM -0500, Bjorn Helgaas wrote:
> > > On Mon, Mar 16, 2026 at 03:51:57PM +0000, Caleb James DeLisle wrote:
> > > > pci_read_bridge_io() and pci_read_bridge_mmio_pref() read bridge window
> > > > registers unconditionally. If the registers are hardwired to zero
> > > > (not implemented), both base and limit will be 0. Since (0 <= 0) is
> > > > true, a bogus window [mem 0x00000000-0x000fffff] or [io 0x0000-0x0fff]
> > > > gets created.
> > > > 
> > > > pci_read_bridge_windows() already detects unsupported windows by
> > > > testing register writability and sets io_window/pref_window flags
> > > > accordingly. Check these flags at the start of pci_read_bridge_io()
> > > > and pci_read_bridge_mmio_pref() to skip reading registers when the
> > > > window is not supported.
> > > 
> > > The fundamental problem here is that assigned space to a bridge window
> > > that isn't implemented.  I wish we understood the connection between
> > > this "read window" path and the assignment path.
> > > 
> > > Maybe this patch fixes it because we enter pci_read_bridge_mmio_pref()
> > > with res->flags being NULL, and we set IORESOURCE_MEM |
> > > IORESOURCE_PREFETCH again, which makes it look like we can assign
> > > space for it?
> > 
> > Yes, that's exactly right.
> > 
> > > 
> > > If that's the case, I think it would improve the commit log to mention
> > > the actual mechanism by which we avoid assigning space.
> > > 
> > 
> > How about this:
> > 
> >   pci_read_bridge_io() and pci_read_bridge_mmio_pref() read
> >   bridge window registers unconditionally. If the registers
> >   are hardwired to zero (not implemented), both base and limit
> >   will be 0. Since (0 <= 0) is true, these functions set
> >   IORESOURCE_IO or IORESOURCE_MEM | IORESOURCE_PREFETCH on
> >   the bridge resource. This causes the allocator to assign
> >   space for the window even though the hardware can't
> >   implement it.
> > 
> >   pci_read_bridge_windows() already detects unsupported windows
> >   by testing register writability and sets io_window/pref_window
> >   flags accordingly. Check these flags at the start of
> >   pci_read_bridge_io() and pci_read_bridge_mmio_pref() to skip
> >   reading registers when the window is not supported, so the
> >   resource flags remain clear and the allocator does not assign
> >   space for non-existent windows.
> 
> At least to me the proposed text reads much better than the original.
> The original text required reading between the lines to connect the dots, 
> whereas this new one clearly explains what causes what.

Hi again,

Reading the code I think the entire 0 <= 0 part is a red herring 
when it comes to the current code, the flags are always set by the 
functions!

The code would only add IORESOURCE_UNSET | IORESOURCE_DISABLED if the 
base <= limit check fails but that's still wrong because it says to the 
resource allocation code that it can enable that bridge window if it needs 
to.

Prior to the commit 8278c6914306 ("PCI: Preserve bridge window resource 
type flags") the base <= limit check did play some role (maybe the 
original commit message was based on some older tree than the most current 
one).

-- 
 i.
Re: [PATCH v2 3/3] PCI: Skip bridge window reads when window is not supported
Posted by Ahmed Naseef 2 weeks, 5 days ago 
On Wed, Mar 18, 2026 at 02:18:22PM +0200, Ilpo Järvinen wrote:
> On Wed, 18 Mar 2026, Ilpo Järvinen wrote:
> 
> > On Wed, 18 Mar 2026, Ahmed Naseef wrote:
> > 
> > > On Tue, Mar 17, 2026 at 04:29:08PM -0500, Bjorn Helgaas wrote:
> > > > On Mon, Mar 16, 2026 at 03:51:57PM +0000, Caleb James DeLisle wrote:
> > > > > pci_read_bridge_io() and pci_read_bridge_mmio_pref() read bridge window
> > > > > registers unconditionally. If the registers are hardwired to zero
> > > > > (not implemented), both base and limit will be 0. Since (0 <= 0) is
> > > > > true, a bogus window [mem 0x00000000-0x000fffff] or [io 0x0000-0x0fff]
> > > > > gets created.
> > > > > 
> > > > > pci_read_bridge_windows() already detects unsupported windows by
> > > > > testing register writability and sets io_window/pref_window flags
> > > > > accordingly. Check these flags at the start of pci_read_bridge_io()
> > > > > and pci_read_bridge_mmio_pref() to skip reading registers when the
> > > > > window is not supported.
> > > > 
> > > > The fundamental problem here is that assigned space to a bridge window
> > > > that isn't implemented.  I wish we understood the connection between
> > > > this "read window" path and the assignment path.
> > > > 
> > > > Maybe this patch fixes it because we enter pci_read_bridge_mmio_pref()
> > > > with res->flags being NULL, and we set IORESOURCE_MEM |
> > > > IORESOURCE_PREFETCH again, which makes it look like we can assign
> > > > space for it?
> > > 
> > > Yes, that's exactly right.
> > > 
> > > > 
> > > > If that's the case, I think it would improve the commit log to mention
> > > > the actual mechanism by which we avoid assigning space.
> > > > 
> > > 
> > > How about this:
> > > 
> > >   pci_read_bridge_io() and pci_read_bridge_mmio_pref() read
> > >   bridge window registers unconditionally. If the registers
> > >   are hardwired to zero (not implemented), both base and limit
> > >   will be 0. Since (0 <= 0) is true, these functions set
> > >   IORESOURCE_IO or IORESOURCE_MEM | IORESOURCE_PREFETCH on
> > >   the bridge resource. This causes the allocator to assign
> > >   space for the window even though the hardware can't
> > >   implement it.
> > > 
> > >   pci_read_bridge_windows() already detects unsupported windows
> > >   by testing register writability and sets io_window/pref_window
> > >   flags accordingly. Check these flags at the start of
> > >   pci_read_bridge_io() and pci_read_bridge_mmio_pref() to skip
> > >   reading registers when the window is not supported, so the
> > >   resource flags remain clear and the allocator does not assign
> > >   space for non-existent windows.
> > 
> > At least to me the proposed text reads much better than the original.
> > The original text required reading between the lines to connect the dots, 
> > whereas this new one clearly explains what causes what.
> 
> Hi again,
> 
> Reading the code I think the entire 0 <= 0 part is a red herring 
> when it comes to the current code, the flags are always set by the 
> functions!
> 
> The code would only add IORESOURCE_UNSET | IORESOURCE_DISABLED if the 
> base <= limit check fails but that's still wrong because it says to the 
> resource allocation code that it can enable that bridge window if it needs 
> to.
> 
> Prior to the commit 8278c6914306 ("PCI: Preserve bridge window resource 
> type flags") the base <= limit check did play some role (maybe the 
> original commit message was based on some older tree than the most current 
> one).

Thank you for catching that. We were testing on LTS kernel
6.12 (downstream OpenWrt) where the commit 8278c6914306
("PCI: Preserve bridge window resource type flags") is not
present . In that tree the flags are still only set inside
the base <= limit check, which is why the commit message
focused on that path.
  
For current mainline, how about this for the commit message:

  pci_read_bridge_io() and pci_read_bridge_mmio_pref()
  unconditionally set resource type flags (IORESOURCE_IO
  or IORESOURCE_MEM | IORESOURCE_PREFETCH) when reading
  bridge window registers. For windows that are not
  implemented in hardware, this causes the allocator to
  assign space for a window that doesn't exist.

  pci_read_bridge_windows() already detects unsupported
  windows by testing register writability and sets
  io_window/pref_window flags accordingly. Check these
  flags at the start of pci_read_bridge_io() and
  pci_read_bridge_mmio_pref() to skip them entirely when
  the window is not supported, so the resource flags
  remain clear and the allocator does not assign space
  for non-existent windows.


Ahmed Naseef

> 
> -- 
>  i.

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.