Message-ID: <aWiOT0Q2rCMHXsIx@DESKTOP-TIT0J8O.localdomain>
Date: Thu, 15 Jan 2026 10:50:55 +0400
From: Ahmed Naseef <naseefkm@...il.com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [QUESTION] pci_read_bridge_bases: skip prefetch window if
 pref_window not set?

On Wed, Jan 14, 2026 at 11:55:50AM -0600, Bjorn Helgaas wrote:
> On Wed, Jan 14, 2026 at 11:34:23AM +0400, Ahmed Naseef wrote:
> > On Tue, Jan 13, 2026 at 03:02:59PM -0600, Bjorn Helgaas wrote:
> > > On Mon, Jan 12, 2026 at 01:41:00PM +0400, Ahmed Naseef wrote:
> > > >   [  160.238227] mtk-pcie 1fb83000.pcie: EN7528: port1 link trained to Gen2
> > > 
> > > Can we look forward to a patch to add support for EN7528?
> > 
> > Definitely. Someone else is working on PCIe support for EN751221
> > (same SoC family). Once everything is consolidated, we plan to submit
> > a unified patch to drivers/pci/controller/pcie-mediatek.c.
> 
> Great, will watch for that!
> 
> > > > PCI_PREF_MEMORY_LIMIT which both return 0x0000. This results in base = 0
> > > > and limit = 0. The condition "if (base <= limit)" evaluates to true
> > > > (since 0 <= 0), so a bogus prefetch window [mem 0x00000000-0x000fffff pref]
> > > > is created.
> > > 
> > > It's too bad we didn't log this in dmesg.  It looks like we claimed
> > > there was a [mem 0x28100000-0x282fffff pref] window.
> > 
> > Should I add logging for this as part of the patch? If so, could you
> > suggest where and what format would be appropriate?
> 
> Apparently there's a place where we figured out that 01:00.0 has a
> prefetchable window at [mem 0x28100000-0x282fffff pref]:
> 
>   [  160.546094] pci 0001:00:01.0:   bridge window [mem 0x28100000-0x282fffff pref]
> 
> I don't know how we figured that out if PCI_PREF_MEMORY_BASE and
> PCI_PREF_MEMORY_LIMIT are hardwired to zero.  Another mystery worth
> exploring.
> 
> In any event, it sounds like there's another place later where we make
> a bogus [mem 0x00000000-0x000fffff pref] window?  That point, where we
> change the window, is where I would think about adding a message.
> 

If my understanding of the code flow is correct (please correct me if
I'm mistaken), the order is actually reversed - the bogus window is
created first, then the proper assignment happens later:

1. pci_read_bridge_windows() tests register writability and finds
pref_window is not supported (pref_window=0)

2. Later, pci_read_bridge_bases() unconditionally calls
pci_read_bridge_mmio_pref() with log=false. Since the registers
return 0, we get base=0, limit=0, and (0 <= 0) creates the bogus
window [mem 0x0-0xfffff pref]. This sets res->flags with
IORESOURCE_PREFETCH and res->start=0, res->end=0xfffff. But because
log=false, this is not logged to dmesg.

3. Then pci_assign_unassigned_root_bus_resources() sizes and assigns
bridge windows based on downstream device BAR requirements. The mt7615e
(device 14c3:7663) has prefetchable BARs:

	[  160.332147] pci 0001:01:00.0: [14c3:7663] type 00 class 0x000280 PCIe Endpoint
	[  160.339730] pci 0001:01:00.0: BAR 0 [mem 0x00000000-0x000fffff 64bit pref]
	[  160.346757] pci 0001:01:00.0: BAR 2 [mem 0x00000000-0x00003fff 64bit pref]
	[  160.353833] pci 0001:01:00.0: BAR 4 [mem 0x00000000-0x00000fff 64bit pref]

 The bridge prefetch resource already has IORESOURCE_PREFETCH flag set
from step 2. The sizing phase calculates the required window size based
on these device BARs. Then the assignment phase allocates addresses from
the available address space, overwriting the bogus res->start and
res->end with proper values.

	[  160.487241] pci 0001:00:01.0: bridge window [mem 0x28100000-0x282fffff pref]: assigned

4. Finally, pci_setup_bridge_mmio_pref() attempts to program the bridge
registers and logs the window address. However, since the registers
are hardwired to zero, the values won't stick.

	[  160.546094] pci 0001:00:01.0:   bridge window [mem 0x28100000-0x282fffff pref]

The reason for the confusion is that step 2 (bogus window creation) has
log=false, so it's silent in dmesg, while steps 3 and 4 log the proper
window address. This makes it appear as if the proper window came first.

In contrast, the mt7603 (device 14c3:7603) on port 0 has no pref flags:

	pci 0000:01:00.0: [14c3:7603] type 00 class 0x028000 PCIe Endpoint
	pci 0000:01:00.0: BAR 0 [mem 0x00000000-0x000fffff]

Since mt7603 has no prefetchable BAR requirements, no prefetch bridge
window assignment is needed for that port and it works without any issues.

> > From: Ahmed Naseef <naseefkm@...il.com>
> > Subject: [PATCH] PCI: Skip bridge window reads when window is not supported
> > 
> > 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@...nel.org>
> > Signed-off-by: Ahmed Naseef <naseefkm@...il.com>
> > ---
> > 
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -351,6 +351,9 @@ static void pci_read_bridge_io(struct pc
> >         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) {
> > @@ -412,6 +415,9 @@ static void pci_read_bridge_mmio_pref(st
> >         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;
> 
> This looks good.  But I guess I would like to understand better how we
> figured out the addresses for the prefetchable window.  Things like
> that niggle at me.
> 

Does above explanation clarify the flow? I'm still learning the PCI subsystem internals,
so please let me know if any part of my understanding is incorrect.

Best regards,
Ahmed Naseef

Powered by blists - more mailing lists