linux-kernel - Re: [QUESTION] pci_read_bridge_bases: skip prefetch window if pref

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260114175550.GA825847@bhelgaas>
Date: Wed, 14 Jan 2026 11:55:50 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Ahmed Naseef <naseefkm@...il.com>
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: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.

> 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.

Bjorn