linux-kernel - Re: [PATCH 2/2] PCI: Resources outside their window must set IORESOURCE

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] [day] [month] [year] [list]

Message-ID: <20250926193029.GA2254976@bhelgaas>
Date: Fri, 26 Sep 2025 14:30:29 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: linux-pci@...r.kernel.org, Bjorn Helgaas <bhelgaas@...gle.com>,
	Krzysztof Wilczyński <kw@...ux.com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Lucas De Marchi <lucas.demarchi@...el.com>
Subject: Re: [PATCH 2/2] PCI: Resources outside their window must set
 IORESOURCE_UNSET

On Fri, Sep 26, 2025 at 03:21:17PM +0300, Ilpo Järvinen wrote:
> On Thu, 25 Sep 2025, Bjorn Helgaas wrote:
> > On Wed, Sep 24, 2025 at 04:42:28PM +0300, Ilpo Järvinen wrote:
> > > PNP resources are checked for conflicts with the other resource in the
> > > system by quirk_system_pci_resources() that walks through all PCI
> > > resources. quirk_system_pci_resources() correctly filters out resource
> > > with IORESOURCE_UNSET.
> > > 
> > > Resources that do not reside within their bridge window, however, are
> > > not properly initialized with IORESOURCE_UNSET resulting in bogus
> > > conflicts detected in quirk_system_pci_resources():

> > > @@ -329,6 +349,18 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
> > >  			 res_name, (unsigned long long)region.start);
> > >  	}
> > >  
> > > +	if (!(res->flags & IORESOURCE_UNSET)) {
> > > +		struct resource *b_res;
> > > +
> > > +		b_res = pbus_select_window_for_res_addr(dev->bus, res);
> > > +		if (!b_res ||
> > > +		    b_res->flags & (IORESOURCE_UNSET | IORESOURCE_DISABLED)) {
> > > +			pci_dbg(dev, "%s %pR: no initial claim (no window)\n",
> > > +				res_name, res);
> > 
> > Should this be pci_info()?  Or is there somewhere else that we
> > complain about a child resource that's not contained in a bridge
> > window?
> 
> AFAIK, there's no other print. The kernel didn't even recognize this case 
> until now so how could there have been one?!

> They'd generally show up as failures later in resource assignment if the 
> resource doesn't fit to the bridge window [1], which should also set 
> IORESOURCE_UNSET, but good luck for inferring things from that. It's 
> tedious, I know. :-) If the bridge window is large enough, the base 
> address would just change where the resource fits (I think).
> 
> It can be pci_info() if you think that's better. I just picked the level 
> which is the least noisy. We can go with pci_info() now and if the logging 
> turns out excessive when we start to see dmesgs with it, we can of course 
> adjust it later so it's not permanent either way.
> 
> In any case, there's not much user can do for these as it's the setup FW 
> gave us.
> 
> > I recently got an internal report of child BARs being reassigned, I
> > think because they weren't inside a bridge window, and the dmesg log
> > (from an older kernel) showed the BAR reassignments, but didn't say
> > anything about the *reason* for the reassignment.
> 
> Resource reassignment is only done after the resource was initially 
> assigned so I'm not sure if that inferring chain is sound.
> 
> Admittedly, you didn't exactly specify how you picked up that it was 
> "reassigned" so it could be just terminology that doesn't match what 
> setup-bus/res.c considers as resource reassignment. That is, if BAR's 
> address was simply changed from the initial, that's not "reassignment" in 
> the sense used by the kernel.

Here's the case I saw (a v6.1 kernel, so old log messages):

  pci 0000:00:00.0: bridge window [mem 0x80000000-0x97fffffff 64bit pref] to [bus 01-05] add_size 80000000 add_align 80000000
  pci 0000:00:00.0: BAR 15: assigned [mem 0x380000000-0xcffffffff 64bit pref]
  pci 0000:01:01.0: BAR 0: [mem 0xb00000000-0xbffffffff 64bit pref]
  ...
  pci 0000:01:01.7: BAR 0: [mem 0x400000000-0x4ffffffff 64bit pref]
  pci 0000:01:01.0: BAR 0: assigned [mem 0x400000000-0x4ffffffff 64bit pref]

Obviously these initial BAR 0 values don't fit in the
[0x80000000-0x97fffffff] bridge window, so I think we moved and
expanded it and then assigned the BARs to be inside.

I was thinking might get the "can't claim; no compatible bridge
window" message in pci_claim_resource() as a clue, but we didn't.

This *seems* like a firmware defect: why would firmware bother to
program these BARs at all unless it also made a bridge window that
could reach them.

Bjorn