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

[Ahmed Naseef]

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.