mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Lorenzo Pieralisi <lpieralisi@kernel.org>
To: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: Ard Biesheuvel <ardb@kernel.org>,
	Ard Biesheuvel <ardb+git@google.com>,
	linux-pci@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>
Subject: Re: [RFC PATCH] PCI: Tolerate non-prefetchable 64-bit BARs in prefetchable windows
Date: Fri, 11 Sep 2026 12:14:21 +0200	[thread overview]
Message-ID: <aqPUfUuu1YKCvKnI@red-moon> (raw)
In-Reply-To: <2e392b1b-3cbe-5cf8-e190-b1c82d69562f@linux.intel.com>

On Fri, Sep 11, 2026 at 12:27:25PM +0300, Ilpo Järvinen wrote:
> On Thu, 10 Sep 2026, Ard Biesheuvel wrote:
> 
> > 
> > On Thu, 10 Sep 2026, at 19:31, Ilpo Järvinen wrote:
> > > On Thu, 10 Sep 2026, Ard Biesheuvel wrote:
> > >
> > >> From: Ard Biesheuvel <ardb@kernel.org>
> > >> 
> > >> The prefetchable vs. non-prefetchable distinction is a relic of
> > >> conventional PCI, to denote from which regions PCI-PCI bridges were
> > >> permitted to perform speculative readahead.
> > >> 
> > >> For software compatibility reasons, PCI Express inherited the Type 1
> > >> header and models PCIe root ports as PCI-PCI bridges. However, this
> > >> readahead behavior does not exist in PCIe, and so this distinction has
> > >> mostly become meaningless on the bridge level.
> > >> 
> > >> As per the PCIe r6.3 ECN "Removing Prefetchable Terminology", the
> > >> 'prefetchable' designation has been removed from the specification
> > >> entirely, on the basis that it is obsolete, and is being abused to
> > >> inform memory mapping attributes and other device/BAR level properties
> > >> that it was never intended for.
> > >> 
> > >> Given the limited range for non-prefetchable windows in the Type 1
> > >> header, and the fact that the distinction no longer exists for PCIe,
> > >> resource allocation performed by firmware may result in non-prefetchable
> > >> 64-bit BARs being allocated inside prefetchable bridge windows.
> > >> 
> > >> Linux rejects such allocations ("can't claim; no compatible bridge
> > >> window") when it encounters them, but will usually fail to produce an
> > >> alternative allocation, given that firmware wouldn't have placed them
> > >> there in the first place if there was sufficient space in the
> > >> non-prefetchable window.
> > >> 
> > >> So at the very least, let's not reject such allocations when they were
> > >> made by the firmware.
> > >> 
> > >> Cc: Bjorn Helgaas <bhelgaas@google.com>
> > >> Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
> > >> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > >> ---
> > >> Link: https://github.com/tianocore/edk2/issues/13104
> > >> 
> > >>  drivers/pci/pci.c   | 3 ++-
> > >>  include/linux/pci.h | 4 ++--
> > >>  2 files changed, 4 insertions(+), 3 deletions(-)
> > >> 
> > >> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > >> index b2879a6be5f8..e33eb9f3a139 100644
> > >> --- a/drivers/pci/pci.c
> > >> +++ b/drivers/pci/pci.c
> > >> @@ -761,7 +761,8 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
> > >>  			 * not, the allocator made a mistake.
> > >>  			 */
> > >>  			if (r->flags & IORESOURCE_PREFETCH &&
> > >> -			    !(res->flags & IORESOURCE_PREFETCH))
> > >> +			    !(res->flags & IORESOURCE_PREFETCH) &&
> > >> +			    !pci_is_pcie(dev))
> > >>  				return NULL;
> > >>  
> > >>  			/*
> > >
> > > I was more thinking along the lines of always setting IORESOURCE_PREFETCH 
> > > for 64-bit BARs on PCIe devices but I've not had time to look at that/test 
> > > how many things would break as a result. ...It would seem much simpler 
> > > solution to differentiate PCI from PCIe while keeping the existing logic 
> > > without adding similar pci_is_pcie() checks everywhere.
> > >
> > 
> > I agree that the code changes would be much simpler.
> > 
> > However, would this impact the PCI metadata observed by all consumers,
> > including userspace,
> 
> Yes, it will impact userspace. But quoting you from above:
> 
> "is being abused to inform memory mapping attributes and other device/BAR 
> level properties that it was never intended for."
> 
> What does the userspace then do with the information? Does it qualify 
> under "it was never intended for"?

The issue is the resulting mmap in the kernel- we can end up mapping
MMIO regions that don't tolerate side-effects with NormalNC attributes on arm64
(that's what WC is on arm64 and in turn WC is detected in places - eg sysfs -
through IORESOURCE_PREFETCH), which can end up in mayhem because those mappings
allow eg speculative reads.

Hopefully, this is not an issue in kernel drivers because drivers *should* know
what a BAR is and should ignore the resource flag as far as mem mappings are
concerned.

> > and drivers that may expect a certain BAR layout,
> 
> ??? Would that even be spec compliant??
> 
> > and/or base decisions about memory attributes on this?
> 
> The point is to consider them 64-bit window eligible so yes, kernel would 
> definitely be basing decision on that but that's intentional.
> 
> > In particular, I am concerned about non-prefetchable BARs that actually
> > have side effects on read, being mapped with WC (or Normal-NC on arm64)
> > semantics, where the interconnect may widen, combine or reorder accesses.
> 
> So on a more concrete terms, you're referring to the check in 
> proc_bus_pci_mmap()? And the one in __pci_resource_attr_is_visible() + 
> pci_dev_resource_wc_is_visible()? I suppose that wouldn't work then.

Indeed, I don't want to speak for Ard but that's exactly a couple of places
where things can go haywire. Then there is PCI drivers even though I don't
think we should have a problem there but as Ard replied, who knows, see above.

Lorenzo

> So if just setting IORESOURCE_PREFETCH is not workable, how about adding a 
> getter for res->flags which adds IORESOURCE_PREFETCH into the returned 
> flags if it's PCIe device and (in the end) use the raw value only in those 
> places that actually care about wc distinction. What I don't want to see 
> us adding that pci_is_pcie() everywhere.
> 
> -- 
>  i.


      parent reply	other threads:[~2026-09-11 10:14 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-10 14:34 Ard Biesheuvel
2026-09-10 17:31 ` Ilpo Järvinen
2026-09-10 21:04   ` Ard Biesheuvel
2026-09-11  9:27     ` Ilpo Järvinen
2026-09-11 10:01       ` Ard Biesheuvel
2026-09-11 10:14       ` Lorenzo Pieralisi [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aqPUfUuu1YKCvKnI@red-moon \
    --to=lpieralisi@kernel.org \
    --cc=ardb+git@google.com \
    --cc=ardb@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®