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

[Ilpo Järvinen]

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 at kernel.org>
> > > Link: https://lore.kernel.org/all/20260113210259.GA715789@bhelgaas/
> > > Signed-off-by: Ahmed Naseef <naseefkm at gmail.com>
> > > Signed-off-by: Caleb James DeLisle <cjd at 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
> > > 
>