mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Ard Biesheuvel <ardb+git@google.com>,
	linux-pci@vger.kernel.org,  LKML <linux-kernel@vger.kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	 Lorenzo Pieralisi <lpieralisi@kernel.org>
Subject: Re: [PATCH v3 2/2] PCI: Allow 64-bit non-prefetchable BARs in prefetchable windows
Date: Fri, 2 Oct 2026 17:06:18 +0300 (EEST)	[thread overview]
Message-ID: <c5371d2b-60aa-5658-ac57-4953705dc0b0@linux.intel.com> (raw)
In-Reply-To: <a7a847db-b9e2-a94b-540a-5cdd829dec18@linux.intel.com>

[-- Attachment #1: Type: text/plain, Size: 14118 bytes --]

On Fri, 2 Oct 2026, Ilpo Järvinen wrote:

> On Fri, 2 Oct 2026, Ard Biesheuvel wrote:
> 
> > 
> > On Wed, 30 Sep 2026, at 20:50, Ilpo Järvinen wrote:
> > > On Wed, 30 Sep 2026, Ard Biesheuvel wrote:
> > >
> > >> From: Ard Biesheuvel <ardb@kernel.org>
> > >> 
> > >> The non-prefetchable memory window of a PCI-to-PCI bridge can only
> > >> decode 32-bit addresses, and so non-prefetchable BARs of devices below a
> > >> bridge can only be allocated from the part of the host bridge memory
> > >> space below 4 GB. This is the case even for 64-bit BARs, which could
> > >> easily be placed above 4 GB if there was a bridge window to put them in,
> > >> and on many platforms, 32-bit addressable MMIO space is scarce.
> > >> 
> > >> The PCIe spec addresses this in the implementation note "Additional
> > >> Guidance on the Prefetchable Bit in Memory Space BARs" (PCIe r7.0, sec
> > >> 7.5.1.2.1): on PCIe, setting the Prefetchable bit of a BAR still permits
> > >> correct operation even if the range has read side effects or cannot
> > >> tolerate write merging, as long as the entire path from the host to the
> > >> device is PCIe, given that PCIe Memory Reads always carry an explicit
> > >> length, and PCIe Switches never prefetch or merge writes. The same
> > >> reasoning applies when it is the OS that places a non-prefetchable BAR
> > >> in a prefetchable bridge window: PCIe Root Ports and Switch Ports
> > >> forward requests that hit either window in exactly the same way. Hence,
> > >> the prefetchable window of a PCIe Root Port or Switch Port, which may be
> > >> 64-bit, can serve as a 64-bit window for non-prefetchable BARs too. [0]
> > >> 
> > >> So add pci_bus_placement_flags(), which returns the flags of a resource
> > >> on a given bus with IORESOURCE_PREFETCH set if it is a 64-bit
> > >> non-prefetchable memory resource, the bus is not a root bus, all bridges
> > >> between the bus and the root bus are PCIe Root Ports or PCIe Switch
> > >> Ports, and the host bridge has no prefetchable memory window, but does
> > >> have one that extends above 4 GB in PCI bus address space. [1]
> > >> 
> > >> Evaluate the conditions on the bridges and the host bridge only once
> > >> per bus, when it is added, and record the result in a new bus flag,
> > >> PCI_BUS_FLAGS_NO_NP_BARS_IN_P_WINDOWS: pci_register_host_bridge() sets
> > >> it on the root bus if the host bridge does not qualify, child buses
> > >> inherit it, and pci_alloc_child_bus() sets it on the secondary bus of
> > >> any bridge that is not a PCIe Root Port or Switch Port.
> > >> 
> > >> Use pci_bus_placement_flags() wherever the resource allocator decides
> > >> which bridge window a device resource (including an SR-IOV VF BAR)
> > >> belongs in:
> > >> 
> > >> - in pbus_select_window_for_type(), which is used when sizing bridge
> > >>   windows, and when releasing them to retry failed assignments or to
> > >>   resize a BAR;
> > >> - when allocating the resource in __pci_assign_resource(), by passing
> > >>   its result to __pci_bus_alloc_resource();
> > >> - when claiming a resource assigned by firmware, in
> > >>   pci_find_parent_resource();
> > >> - when deciding which assigned resources to release after a failed
> > >>   assignment, and which failures are relevant to a resized BAR.
> > >> 
> > >> As a result, an eligible 64-bit non-prefetchable BAR is handled exactly
> > >> like a 64-bit prefetchable BAR: it is placed in the prefetchable window
> > >> of the upstream bridge, which may be above 4 GB, and allocation falls
> > >> back to the non-prefetchable window if the prefetchable one has no
> > >> space. [2]
> > >> 
> > >> 32-bit BARs are not affected, and neither are bridge windows, as only
> > >> prefetchable bridge windows can be 64-bit. Devices below conventional
> > >> PCI or CardBus bridges, below PCIe to PCI/PCI-X bridges (in either
> > >> direction), or below host bridges that have a prefetchable window or no
> > >> window above 4 GB are not affected either, and neither are devices on a
> > >> root bus, as there is no prefetchable window for their BARs to go to.
> > >> 
> > >> [0] This reasoning does not extend to the host bridge, though: how it
> > >>     treats the windows that firmware describes as prefetchable is
> > >>     platform specific. For instance, the V3 Semiconductor V360EPC
> > >>     (pci-v3-semi) enables prefetching for its prefetchable window, the
> > >>     MPC52xx uses Memory Read Multiple for it, and Freescale PCI/PCIe
> > >>     host bridges (fsl_pci) enable relaxed ordering for it. However, if
> > >>     the host bridge has no prefetchable windows at all (as appears to be
> > >>     the case on many x86 PCs), all prefetchable bridge windows are
> > >>     carved out of its non-prefetchable windows, and so the host bridge
> > >>     does not treat them any differently.
> > >> 
> > >> [1] Placing non-prefetchable BARs in prefetchable bridge windows only
> > >>     helps if one of those non-prefetchable host bridge windows extends
> > >>     above 4 GB. Otherwise, the prefetchable bridge windows end up below
> > >>     4 GB as well, and BARs would merely move between two bridge windows
> > >>     that are carved out of the same 32-bit space.
> > >> 
> > >> [2] Note that this only concerns where a BAR is placed. How it is mapped
> > >>     is decided by the driver and by the attributes of the BAR itself
> > >>     (e.g., pci_iomap_wc() and the sysfs resource<N>_wc files only honour
> > >>     IORESOURCE_PREFETCH on the BAR), and this change does not modify the
> > >>     flags of any BAR.)
> > >> 
> > >> Assisted-by: LLM
> > >> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > >> ---
> > >> Tested on QEMU arm64 'virt' (DT, all resources assigned by Linux) with
> > >> a qemu-xhci below a Root Port, an NVMe below a Switch, a qemu-xhci
> > >> below a PCIe-to-PCI bridge and another qemu-xhci on the root bus: the
> > >> 64-bit non-prefetchable BARs of the first two (and of the PCIe-to-PCI
> > >> bridge itself) move from the 32-bit non-prefetchable windows into the
> > >> 64-bit prefetchable windows above 4 GB, while the others stay where
> > >> they were. When the 64-bit host bridge window is marked prefetchable
> > >> in the DT, or removed from it, all resources are assigned exactly as
> > >> without this patch. Both drivers work, also after hot removal and
> > >> rescan of the endpoints and of the Switch. Also tested on QEMU x86_64
> > >> q35 with SeaBIOS, which assigns all resources itself: the firmware
> > >> assignment is claimed as before, and after hot removal and rescan, the
> > >> eligible BARs are placed in the prefetchable windows.
> > >> ---
> > >>  drivers/pci/pci.c       | 43 +++++++++++++++++++-
> > >>  drivers/pci/pci.h       |  1 +
> > >>  drivers/pci/probe.c     | 35 ++++++++++++++++
> > >>  drivers/pci/setup-bus.c | 23 ++++++++---
> > >>  drivers/pci/setup-res.c | 30 +++++++++-----
> > >>  include/linux/pci.h     |  1 +
> > >>  6 files changed, 115 insertions(+), 18 deletions(-)
> > >> 
> > >> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > >> index b2879a6be5f8..51e96611d734 100644
> > >> --- a/drivers/pci/pci.c
> > >> +++ b/drivers/pci/pci.c
> > >> @@ -736,6 +736,43 @@ static bool pci_dev_config_accessible(struct pci_dev *dev, char *msg)
> > >>  	return true;
> > >>  }
> > >>  
> > >> +/**
> > >> + * pci_bus_placement_flags - Get the flags to use for placing a resource
> > >> + * @bus: PCI bus of the device that owns the resource
> > >> + * @flags: Resource flags
> > >> + *
> > >> + * The non-prefetchable window of a PCI-to-PCI bridge can only decode 32-bit
> > >> + * addresses, so 64-bit non-prefetchable BARs of devices below a bridge have to
> > >> + * compete for space below 4GB. However, the PCIe spec notes that setting the
> > >> + * Prefetchable bit of a BAR permits correct operation even if the range has
> > >> + * read side effects or cannot tolerate write merging, as long as the entire
> > >> + * path from the host to the device is PCIe: PCIe Memory Reads always carry an
> > >> + * explicit length, and PCIe Switches never prefetch or merge writes (PCIe
> > >> + * r7.0, sec 7.5.1.2.1, Implementation Note "Additional Guidance on the
> > >> + * Prefetchable Bit in Memory Space BARs").
> > >> + *
> > >> + * So if all bridges between @bus and the root bus are PCIe Root Ports or PCIe
> > >> + * Switch Ports, handle 64-bit non-prefetchable resources on @bus like 64-bit
> > >> + * prefetchable ones, so that they can be placed in the prefetchable window of
> > >> + * the bridge above @bus, which may be above 4GB. Other bridges, such as
> > >> + * conventional PCI bridges, may prefetch from their prefetchable window.
> > >> + *
> > >> + * Return: @flags, with IORESOURCE_PREFETCH set if a resource with @flags on
> > >> + * @bus may be placed in a prefetchable bridge window.
> > >> + */
> > >> +unsigned long pci_bus_placement_flags(struct pci_bus *bus, unsigned long flags)
> > >> +{
> > >> +	if ((flags & (IORESOURCE_TYPE_BITS | IORESOURCE_PREFETCH |
> > >> +		      IORESOURCE_MEM_64)) != (IORESOURCE_MEM | IORESOURCE_MEM_64))
> > >
> > > PCI_RES_TYPE_MASK
> > >
> > 
> > OK. Its definition will need to move, though, but I guess that's fine.
> 
> Yes. If you want to have it in a consistent place with my work, here's 
> where I put it:
> 
> https://lore.kernel.org/linux-pci/20261002113319.6652-6-ilpo.jarvinen@linux.intel.com/
> 
> > >> +		return flags;
> > >> +
> > >> +	if (pci_is_root_bus(bus) ||
> > >> +	    (bus->bus_flags & PCI_BUS_FLAGS_NO_NP_BARS_IN_P_WINDOWS))
> > >> +		return flags;
> > >> +
> > >> +	return flags | IORESOURCE_PREFETCH;
> > >> +}
> > >> +
> > >>  /**
> > >>   * pci_find_parent_resource - return resource region of parent bus of given
> > >>   *			      region
> > >> @@ -749,6 +786,7 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
> > >>  					  struct resource *res)
> > >>  {
> > >>  	const struct pci_bus *bus = dev->bus;
> > >> +	unsigned long flags = pci_bus_placement_flags(dev->bus, res->flags);
> > >>  	struct resource *r;
> > >>  
> > >>  	pci_bus_for_each_resource(bus, r) {
> > >> @@ -758,10 +796,11 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
> > >>  
> > >>  			/*
> > >>  			 * If the window is prefetchable but the BAR is
> > >> -			 * not, the allocator made a mistake.
> > >> +			 * not (and may not be treated as such), the
> > >> +			 * allocator made a mistake.
> > >>  			 */
> > >>  			if (r->flags & IORESOURCE_PREFETCH &&
> > >> -			    !(res->flags & IORESOURCE_PREFETCH))
> > >> +			    !(flags & IORESOURCE_PREFETCH))
> > >>  				return NULL;
> > >>  
> > >>  			/*
> > >> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> > >> index 8297cfb5dcd5..989b9c1f1580 100644
> > >> --- a/drivers/pci/pci.h
> > >> +++ b/drivers/pci/pci.h
> > >> @@ -567,6 +567,7 @@ static inline int pci_resource_num(const struct pci_dev *dev,
> > >>  	return resno;
> > >>  }
> > >>  
> > >> +unsigned long pci_bus_placement_flags(struct pci_bus *bus, unsigned long flags);
> > >>  int __pci_bus_alloc_resource(struct pci_bus *bus, struct resource *res,
> > >>  			     unsigned long flags, resource_size_t size,
> > >>  			     resource_size_t align, resource_size_t min,
> > >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > >> index 27008e2ea5af..8f00ea3321c0 100644
> > >> --- a/drivers/pci/probe.c
> > >> +++ b/drivers/pci/probe.c
> > >> @@ -990,6 +990,32 @@ static bool pci_preserve_config(struct pci_host_bridge *host_bridge)
> > >>  	return false;
> > >>  }
> > >>  
> > >> +/*
> > >> + * Return true if all memory windows of @bridge are non-prefetchable, and at
> > >> + * least one of them extends above 4GB in PCI bus address space (see
> > >> + * pci_bus_placement_flags()).
> > >> + */
> > >> +static bool pci_host_np_only_with_high_window(struct pci_host_bridge *bridge)
> > >> +{
> > >> +	struct resource_entry *window;
> > >> +	bool high = false;
> > >> +
> > >> +	resource_list_for_each_entry(window, &bridge->windows) {
> > >> +		struct resource *res = window->res;
> > >> +
> > >> +		if (resource_type(res) != IORESOURCE_MEM)
> > >> +			continue;
> > >> +
> > >> +		if (res->flags & IORESOURCE_PREFETCH)
> > >> +			return false;
> > >> +
> > >> +		if (upper_32_bits(res->end - window->offset))
> > >
> > > Should this use pcibios_resource_to_bus() for consistency with the rest of 
> > > the code?
> > >
> > 
> > We are walking the window resources of an apriori known root bus here,
> > and pcibios_resource_to_bus() takes a bus, walks up to the root, iterates
> > over all window resources again to find the given resource, and then
> > applies the offset.
> > 
> > I agree it would be better to respect the layering here, but using
> > pcibios_resource_to_bus() is a bit of a kludge. Could we split it up
> > perhaps?
> 
> I guess that possible but TBH, it would probably be best to just ignore 
> my comment instead as I failed to take account the complications you came 
> across.
> 
> 
> FYI, I'm planning on trying this series with my pci=realloc recalc 
> changes to see if they together resolve the 64-bit VF BARs not appearing 
> on 64-bit side, hopefully I get that done today.

It seems to generally work better in pci=realloc case but there are still 
some disabled Expansion ROMs that do lose their upstream bridge windows 
(win gets disabled) and some cases with mixed pref & non-pref VF BARs 
where the latter do not appear.

norealloc case looks messy but I account that to changes made in my series 
as it alters auto switch to pci=realloc to happen less frequently, which 
can of course cause more assignment failures but it also tries to retain 
the original setup from FW better.

I don't know at this point if the problem is in my series or yours, I will 
have to look deeper but that won't be until next week.


-- 
 i.

  reply	other threads:[~2026-10-02 14:06 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-30 10:30 [PATCH v3 0/2] PCI: Allow " Ard Biesheuvel
2026-09-30 10:30 ` [PATCH v3 1/2] PCI: Add __pci_bus_alloc_resource() to allocate with explicit flags Ard Biesheuvel
2026-09-30 10:30 ` [PATCH v3 2/2] PCI: Allow 64-bit non-prefetchable BARs in prefetchable windows Ard Biesheuvel
2026-09-30 18:50   ` Ilpo Järvinen
2026-10-02 10:13     ` Ard Biesheuvel
2026-10-02 11:36       ` Ilpo Järvinen
2026-10-02 14:06         ` Ilpo Järvinen [this message]
2026-09-30 18:31 ` [PATCH v3 0/2] PCI: Allow " Ilpo Järvinen

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=c5371d2b-60aa-5658-ac57-4953705dc0b0@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=ardb+git@google.com \
    --cc=ardb@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lpieralisi@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®