* [PATCH v4 1/7] resource: Mark free space assigned
2026-10-21 12:49 [PATCH v4 0/7] PCI: Resource placement algorithm fixe Ilpo Järvinen
@ 2026-10-21 12:49 ` Ilpo Järvinen
2026-10-21 12:56 ` sashiko-bot
2026-10-21 12:49 ` [PATCH v4 2/7] PCI/parisc: Clean up resource debug print & use dynamic debug Ilpo Järvinen
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Ilpo Järvinen @ 2026-10-21 12:49 UTC (permalink / raw)
To: Maciej Grochowski, Nikolas Joshua Britton, Geramy Loveless,
Eric Auger, Alexey Fomenko, Bjorn Helgaas, linux-pci,
Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński,
linux-kernel
Cc: Bradley Morgan, Ilpo Järvinen
Free space ranges are (part of) assigned ranges. Since new resources
have IORESOURCE_UNSET, the flag gets copied also to full_avail.flags,
which hampers printing them with %pR.
Clear IORESOURCE_UNSET for the free space indicator resource.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Bradley Morgan <brads@mainlining.org>
---
kernel/resource.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/resource.c b/kernel/resource.c
index e60539a55541..9e5fb021b199 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -731,6 +731,7 @@ static int __find_resource_space(struct resource *root, struct resource *old,
resource_alignf alignf = constraint->alignf;
full_avail.start = root->start;
+ full_avail.flags &= ~IORESOURCE_UNSET;
/*
* Skip past an allocated resource that starts at 0, since the assignment
* of this->start - 1 to full_avail->end below would cause an underflow.
@@ -754,7 +755,7 @@ static int __find_resource_space(struct resource *root, struct resource *old,
/* Check for overflow after ALIGN() */
avail.start = ALIGN(full_avail.start, constraint->align);
avail.end = full_avail.end;
- avail.flags = new->flags;
+ avail.flags = full_avail.flags;
if (avail.start >= full_avail.start) {
alloc.flags = avail.flags;
if (alignf) {
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v4 2/7] PCI/parisc: Clean up resource debug print & use dynamic debug
2026-10-21 12:49 [PATCH v4 0/7] PCI: Resource placement algorithm fixe Ilpo Järvinen
2026-10-21 12:49 ` [PATCH v4 1/7] resource: Mark free space assigned Ilpo Järvinen
@ 2026-10-21 12:49 ` Ilpo Järvinen
2026-10-21 12:56 ` sashiko-bot
2026-10-21 12:49 ` [PATCH v4 3/7] PCI: Honor alignment overrides Ilpo Järvinen
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Ilpo Järvinen @ 2026-10-21 12:49 UTC (permalink / raw)
To: Maciej Grochowski, Nikolas Joshua Britton, Geramy Loveless,
Eric Auger, Alexey Fomenko, Bjorn Helgaas, linux-pci,
Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński,
James E.J. Bottomley, Helge Deller, linux-parisc, linux-kernel
Cc: Bradley Morgan, Ilpo Järvinen
Remove adhoc RES_DBG() and use pci_dbg() instead.
Remove printing parent resource as the passed is a temporary resource
not within the resource tree so accessing ->parent does not make sense.
Reformat the message to the typical format used by PCI core. Print the
resource using %pR.
Correct format specifiers and remove non-sensical res->flags int cast.
Dynamic debug prints can be enabled by providing dyndbg command line
parameter or through /sys/kernel/debug/dynamic_debug/control so this
is more versatile way to debug things than the previous compile time
decision which could only be enable by changing the source code.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
arch/parisc/kernel/pci.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/arch/parisc/kernel/pci.c b/arch/parisc/kernel/pci.c
index b8007c7400d4..96950fe9c2b7 100644
--- a/arch/parisc/kernel/pci.c
+++ b/arch/parisc/kernel/pci.c
@@ -19,7 +19,6 @@
#include <asm/io.h>
#include <asm/superio.h>
-#define DEBUG_RESOURCES 0
#define DEBUG_CONFIG 0
#if DEBUG_CONFIG
@@ -28,13 +27,6 @@
# define DBGC(x...)
#endif
-
-#if DEBUG_RESOURCES
-#define DBG_RES(x...) printk(KERN_DEBUG x)
-#else
-#define DBG_RES(x...)
-#endif
-
struct pci_port_ops *pci_port __ro_after_init;
struct pci_bios_ops *pci_bios __ro_after_init;
@@ -204,10 +196,9 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
struct pci_dev *dev = data;
resource_size_t align, start = res->start;
- DBG_RES("pcibios_align_resource(%s, (%p) [%lx,%lx]/%x, 0x%lx, 0x%lx)\n",
- pci_name(((struct pci_dev *) data)),
- res->parent, res->start, res->end,
- (int) res->flags, size, alignment);
+ pci_dbg(dev, "%pR: pcibios_align_resource(%lx, 0x%llx, 0x%llx)\n",
+ res, res->flags, (unsigned long long)size,
+ (unsigned long long)alignment);
/* If it's not IO, then it's gotta be MEM */
align = (res->flags & IORESOURCE_IO) ? PCIBIOS_MIN_IO : PCIBIOS_MIN_MEM;
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v4 3/7] PCI: Honor alignment overrides
2026-10-21 12:49 [PATCH v4 0/7] PCI: Resource placement algorithm fixe Ilpo Järvinen
2026-10-21 12:49 ` [PATCH v4 1/7] resource: Mark free space assigned Ilpo Järvinen
2026-10-21 12:49 ` [PATCH v4 2/7] PCI/parisc: Clean up resource debug print & use dynamic debug Ilpo Järvinen
@ 2026-10-21 12:49 ` Ilpo Järvinen
2026-10-21 13:22 ` sashiko-bot
2026-10-21 12:49 ` [PATCH v4 4/7] PCI: Fix nesting windows with remainder at the left edge Ilpo Järvinen
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Ilpo Järvinen @ 2026-10-21 12:49 UTC (permalink / raw)
To: Maciej Grochowski, Nikolas Joshua Britton, Geramy Loveless,
Eric Auger, Alexey Fomenko, Bjorn Helgaas, linux-pci,
Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński,
Richard Henderson, Matt Turner, Magnus Lindholm, Russell King,
Geert Uytterhoeven, Thomas Bogendoerfer, James E.J. Bottomley,
Helge Deller, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Yoshinori Sato,
Rich Felker, John Paul Adrian Glaubitz, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Chris Zankel, Max Filippov, David Airlie, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Simona Vetter,
Ilpo Järvinen, linux-alpha, linux-kernel, linux-arm-kernel,
linux-m68k, linux-mips, linux-parisc, linuxppc-dev, linux-sh,
dri-devel, intel-gfx
Cc: Bradley Morgan, Sashiko
pci=resource_alignment argument can override the default alignment for
the resource. The remainder code introduced in the commit 9036bd0efcb6
("PCI: Align head space better") can move remainder space (non-aligning
part of the size) before the aligning left edge which results in
violating the requested alignment.
Introduce struct pci_resreq_data to hold device and user-given alignment
to be able to honor it in pci_align_resource(). When user-given alignment
is found, any remainder movement is skipped.
This also makes pci_specified_resource_alignment() always return a
valid 'resize' value so that callers can simply rely on its value.
Fixes: 9036bd0efcb6 ("PCI: Align head space better")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/linux-pci/20260923133202.07DF61F000FF@smtp.kernel.org/
Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
IMO, PPC rigging the user alignment override to enforce arch specific
min alignment is mixing things wrongly. It would be more appropriate
to do arch alignment overrides by overriding pci_resource_alignment().
Jani Nikula <jani.nikula@intel.com> did ack this with a condition that
there is a proper review done by somebody else so I've not yet included
his tag.
---
arch/alpha/kernel/pci.c | 3 ++-
arch/arm/kernel/bios32.c | 5 +++--
arch/m68k/kernel/pcibios.c | 4 ++--
arch/mips/pci/pci-generic.c | 5 +++--
arch/mips/pci/pci-legacy.c | 5 +++--
arch/parisc/kernel/pci.c | 5 +++--
arch/powerpc/kernel/pci-common.c | 5 +++--
arch/sh/drivers/pci/pci.c | 5 +++--
arch/x86/pci/i386.c | 5 +++--
arch/xtensa/kernel/pci.c | 5 +++--
drivers/char/agp/intel-gtt.c | 5 ++++-
drivers/gpu/drm/i915/i915_gmch.c | 5 +++--
drivers/pci/pci.c | 7 +++++--
drivers/pci/setup-res.c | 19 +++++++++++++------
include/linux/pci.h | 32 ++++++++++++++++++++++++++++++--
15 files changed, 83 insertions(+), 32 deletions(-)
diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
index 11df411b1d18..c5d725a9781f 100644
--- a/arch/alpha/kernel/pci.c
+++ b/arch/alpha/kernel/pci.c
@@ -128,7 +128,8 @@ pcibios_align_resource(void *data, const struct resource *res,
const struct resource *empty_res,
resource_size_t size, resource_size_t align)
{
- struct pci_dev *dev = data;
+ struct pci_resreq_data *rr = data;
+ struct pci_dev *dev = rr->dev;
struct pci_controller *hose = dev->sysdata;
unsigned long alignto;
resource_size_t start = res->start;
diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index ac0e890510da..3e61826f07fb 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -564,7 +564,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
resource_size_t size,
resource_size_t align)
{
- struct pci_dev *dev = data;
+ struct pci_resreq_data *rr = data;
+ struct pci_dev *dev = rr->dev;
resource_size_t start = res->start;
struct pci_host_bridge *host_bridge;
@@ -578,7 +579,7 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
start, size, align);
if (res->flags & IORESOURCE_MEM)
- return pci_align_resource(dev, res, empty_res, size, align);
+ return pci_align_resource(rr, res, empty_res, size, align);
return start;
}
diff --git a/arch/m68k/kernel/pcibios.c b/arch/m68k/kernel/pcibios.c
index 7a9e60df79c5..3024408412bc 100644
--- a/arch/m68k/kernel/pcibios.c
+++ b/arch/m68k/kernel/pcibios.c
@@ -31,14 +31,14 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
resource_size_t size,
resource_size_t align)
{
- struct pci_dev *dev = data;
+ struct pci_resreq_data *rr = data;
resource_size_t start = res->start;
if ((res->flags & IORESOURCE_IO) && (start & 0x300))
start = (start + 0x3ff) & ~0x3ff;
if (res->flags & IORESOURCE_MEM)
- return pci_align_resource(dev, res, empty_res, size, align);
+ return pci_align_resource(rr, res, empty_res, size, align);
return start;
}
diff --git a/arch/mips/pci/pci-generic.c b/arch/mips/pci/pci-generic.c
index c2e23d0c1d77..7f3ecb91b85b 100644
--- a/arch/mips/pci/pci-generic.c
+++ b/arch/mips/pci/pci-generic.c
@@ -25,7 +25,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
const struct resource *empty_res,
resource_size_t size, resource_size_t align)
{
- struct pci_dev *dev = data;
+ struct pci_resreq_data *rr = data;
+ struct pci_dev *dev = rr->dev;
resource_size_t start = res->start;
struct pci_host_bridge *host_bridge;
@@ -39,7 +40,7 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
start, size, align);
if (res->flags & IORESOURCE_MEM)
- return pci_align_resource(dev, res, empty_res, size, align);
+ return pci_align_resource(rr, res, empty_res, size, align);
return start;
}
diff --git a/arch/mips/pci/pci-legacy.c b/arch/mips/pci/pci-legacy.c
index dae6dafdd6e0..82d4b01db64e 100644
--- a/arch/mips/pci/pci-legacy.c
+++ b/arch/mips/pci/pci-legacy.c
@@ -55,7 +55,8 @@ pcibios_align_resource(void *data, const struct resource *res,
const struct resource *empty_res,
resource_size_t size, resource_size_t align)
{
- struct pci_dev *dev = data;
+ struct pci_resreq_data *rr = data;
+ struct pci_dev *dev = rr->dev;
struct pci_controller *hose = dev->sysdata;
resource_size_t start = res->start;
@@ -70,7 +71,7 @@ pcibios_align_resource(void *data, const struct resource *res,
if (start & 0x300)
start = (start + 0x3ff) & ~0x3ff;
} else if (res->flags & IORESOURCE_MEM) {
- start = pci_align_resource(dev, res, empty_res, size, align);
+ start = pci_align_resource(rr, res, empty_res, size, align);
/* Make sure we start at our min on all hoses */
if (start < PCIBIOS_MIN_MEM + hose->mem_resource->start)
diff --git a/arch/parisc/kernel/pci.c b/arch/parisc/kernel/pci.c
index 96950fe9c2b7..36a3a439f609 100644
--- a/arch/parisc/kernel/pci.c
+++ b/arch/parisc/kernel/pci.c
@@ -193,7 +193,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
resource_size_t size,
resource_size_t alignment)
{
- struct pci_dev *dev = data;
+ struct pci_resreq_data *rr = data;
+ struct pci_dev *dev = rr->dev;
resource_size_t align, start = res->start;
pci_dbg(dev, "%pR: pcibios_align_resource(%lx, 0x%llx, 0x%llx)\n",
@@ -205,7 +206,7 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
if (align > alignment)
start = ALIGN(start, align);
else
- start = pci_align_resource(dev, res, empty_res, size, alignment);
+ start = pci_align_resource(rr, res, empty_res, size, alignment);
return start;
}
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 4fc52c21fe5d..23594759cfe2 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1131,7 +1131,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
resource_size_t size,
resource_size_t align)
{
- struct pci_dev *dev = data;
+ struct pci_resreq_data *rr = data;
+ struct pci_dev *dev = rr->dev;
resource_size_t start = res->start;
if (res->flags & IORESOURCE_IO) {
@@ -1140,7 +1141,7 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
if (start & 0x300)
start = (start + 0x3ff) & ~0x3ff;
} else if (res->flags & IORESOURCE_MEM) {
- start = pci_align_resource(dev, res, empty_res, size, align);
+ start = pci_align_resource(rr, res, empty_res, size, align);
}
return start;
diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
index 878a27a1acfb..279f2770ff4b 100644
--- a/arch/sh/drivers/pci/pci.c
+++ b/arch/sh/drivers/pci/pci.c
@@ -172,7 +172,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
resource_size_t size,
resource_size_t align)
{
- struct pci_dev *dev = data;
+ struct pci_resreq_data *rr = data;
+ struct pci_dev *dev = rr->dev;
struct pci_channel *hose = dev->sysdata;
resource_size_t start = res->start;
@@ -186,7 +187,7 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
if (start & 0x300)
start = (start + 0x3ff) & ~0x3ff;
} else if (res->flags & IORESOURCE_MEM) {
- start = pci_align_resource(dev, res, empty_res, size, align);
+ start = pci_align_resource(rr, res, empty_res, size, align);
}
return start;
diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index e2de26b82940..4178ae4380c2 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c
@@ -156,7 +156,8 @@ pcibios_align_resource(void *data, const struct resource *res,
const struct resource *empty_res,
resource_size_t size, resource_size_t align)
{
- struct pci_dev *dev = data;
+ struct pci_resreq_data *rr = data;
+ struct pci_dev *dev = rr->dev;
resource_size_t start = res->start;
if (res->flags & IORESOURCE_IO) {
@@ -165,7 +166,7 @@ pcibios_align_resource(void *data, const struct resource *res,
if (start & 0x300)
start = (start + 0x3ff) & ~0x3ff;
} else if (res->flags & IORESOURCE_MEM) {
- start = pci_align_resource(dev, res, empty_res, size, align);
+ start = pci_align_resource(rr, res, empty_res, size, align);
/* The low 1MB range is reserved for ISA cards */
if (start < BIOS_END)
diff --git a/arch/xtensa/kernel/pci.c b/arch/xtensa/kernel/pci.c
index 305031551136..4e8ad4a3c9fe 100644
--- a/arch/xtensa/kernel/pci.c
+++ b/arch/xtensa/kernel/pci.c
@@ -42,7 +42,8 @@ pcibios_align_resource(void *data, const struct resource *res,
const struct resource *empty_res,
resource_size_t size, resource_size_t align)
{
- struct pci_dev *dev = data;
+ struct pci_resreq_data *rr = data;
+ struct pci_dev *dev = rr->dev;
resource_size_t start = res->start;
if (res->flags & IORESOURCE_IO) {
@@ -55,7 +56,7 @@ pcibios_align_resource(void *data, const struct resource *res,
if (start & 0x300)
start = (start + 0x3ff) & ~0x3ff;
} else if (res->flags & IORESOURCE_MEM) {
- start = pci_align_resource(dev, res, empty_res, size, align);
+ start = pci_align_resource(rr, res, empty_res, size, align);
}
return start;
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index bcc26785175d..2214347906a6 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -1038,10 +1038,13 @@ static struct agp_memory *intel_fake_agp_alloc_by_type(size_t pg_count,
static int intel_alloc_chipset_flush_resource(void)
{
+ struct pci_resreq_data rr;
int ret;
+
+ pci_init_pci_resreq_data(&rr, intel_private.bridge_dev);
ret = pci_bus_alloc_resource(intel_private.bridge_dev->bus, &intel_private.ifp_resource, PAGE_SIZE,
PAGE_SIZE, PCIBIOS_MIN_MEM, 0,
- pcibios_align_resource, intel_private.bridge_dev);
+ pcibios_align_resource, &rr);
return ret;
}
diff --git a/drivers/gpu/drm/i915/i915_gmch.c b/drivers/gpu/drm/i915/i915_gmch.c
index b0ef6ef577a3..d35097bebc0f 100644
--- a/drivers/gpu/drm/i915/i915_gmch.c
+++ b/drivers/gpu/drm/i915/i915_gmch.c
@@ -38,6 +38,7 @@ static int mchbar_reg(struct drm_i915_private *i915)
static int
intel_alloc_mchbar_resource(struct drm_i915_private *i915)
{
+ struct pci_resreq_data rr;
u32 temp_lo, temp_hi = 0;
u64 mchbar_addr;
int ret;
@@ -55,12 +56,12 @@ intel_alloc_mchbar_resource(struct drm_i915_private *i915)
/* Get some space for it */
i915->gmch.mch_res.name = "i915 MCHBAR";
i915->gmch.mch_res.flags = IORESOURCE_MEM;
+ pci_init_pci_resreq_data(&rr, i915->gmch.pdev);
ret = pci_bus_alloc_resource(i915->gmch.pdev->bus,
&i915->gmch.mch_res,
MCHBAR_SIZE, MCHBAR_SIZE,
PCIBIOS_MIN_MEM,
- 0, pcibios_align_resource,
- i915->gmch.pdev);
+ 0, pcibios_align_resource, &rr);
if (ret) {
drm_dbg(&i915->drm, "failed bus alloc: %d\n", ret);
i915->gmch.mch_res.start = 0;
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b2879a6be5f8..499ccb13c2c4 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -6444,14 +6444,16 @@ static DEFINE_SPINLOCK(resource_alignment_lock);
* RETURNS: Resource alignment if it is specified.
* Zero if it is not specified.
*/
-static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
- bool *resize)
+resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
+ bool *resize)
{
int align_order, count;
resource_size_t align = pcibios_default_alignment();
const char *p;
int ret;
+ *resize = false;
+
spin_lock(&resource_alignment_lock);
p = resource_alignment_param;
if (!p || !*p)
@@ -6497,6 +6499,7 @@ static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
spin_unlock(&resource_alignment_lock);
return align;
}
+EXPORT_SYMBOL_GPL(pci_specified_resource_alignment);
static void pci_request_resource_alignment(struct pci_dev *dev, int bar,
resource_size_t align, bool resize)
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 376f09630a4a..441a62807719 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -265,17 +265,21 @@ resource_size_t pci_resource_alignment(const struct pci_dev *dev,
* before res->start if there's enough free space there. This enables
* tighter packing for resources.
*/
-resource_size_t pci_align_resource(struct pci_dev *dev,
+resource_size_t pci_align_resource(struct pci_resreq_data *rr,
const struct resource *res,
const struct resource *empty_res,
resource_size_t size,
resource_size_t align)
{
+ struct pci_dev *dev = rr->dev;
resource_size_t remainder, start_addr;
if (!(res->flags & IORESOURCE_MEM))
return res->start;
+ if (rr->user_align)
+ return res->start;
+
if (IS_ALIGNED(size, align))
return res->start;
@@ -306,18 +310,21 @@ resource_size_t __weak pcibios_align_resource(void *data,
resource_size_t size,
resource_size_t align)
{
- struct pci_dev *dev = data;
+ struct pci_resreq_data *rr = data;
- return pci_align_resource(dev, res, empty_res, size, align);
+ return pci_align_resource(rr, res, empty_res, size, align);
}
static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
int resno, resource_size_t size, resource_size_t align)
{
struct resource *res = pci_resource_n(dev, resno);
+ struct pci_resreq_data rr;
resource_size_t min;
int ret;
+ pci_init_pci_resreq_data(&rr, dev);
+
min = (res->flags & IORESOURCE_IO) ? PCIBIOS_MIN_IO : PCIBIOS_MIN_MEM;
/*
@@ -329,7 +336,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
*/
ret = pci_bus_alloc_resource(bus, res, size, align, min,
IORESOURCE_PREFETCH | IORESOURCE_MEM_64,
- pcibios_align_resource, dev);
+ pcibios_align_resource, &rr);
if (ret == 0)
return 0;
@@ -341,7 +348,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
(IORESOURCE_PREFETCH | IORESOURCE_MEM_64)) {
ret = pci_bus_alloc_resource(bus, res, size, align, min,
IORESOURCE_PREFETCH,
- pcibios_align_resource, dev);
+ pcibios_align_resource, &rr);
if (ret == 0)
return 0;
}
@@ -354,7 +361,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
*/
if (res->flags & (IORESOURCE_PREFETCH | IORESOURCE_MEM_64))
ret = pci_bus_alloc_resource(bus, res, size, align, min, 0,
- pcibios_align_resource, dev);
+ pcibios_align_resource, &rr);
return ret;
}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index d31a8d107b1e..ea7d75871269 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1217,12 +1217,40 @@ int __must_check pcibios_enable_device(struct pci_dev *, int mask);
/* Architecture-specific versions may override this (weak) */
char *pcibios_setup(char *str);
-/* Used only when drivers/pci/setup.c is used */
+/* Resource assignment request handling */
+/**
+ * struct pci_resreq_data - Data passed to pcibios_align_resource()
+ * @dev: PCI device
+ * @user_align: User provided alignment override (pci=resource_alignment param)
+ */
+struct pci_resreq_data {
+ struct pci_dev *dev;
+ resource_size_t user_align;
+};
+
+resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
+ bool *resize);
+/*
+ * TODO: Improve PCI resource assignment interface so that callers outside PCI
+ * core do not need to know about this detail.
+ */
+static inline void pci_init_pci_resreq_data(struct pci_resreq_data *rr,
+ struct pci_dev *dev)
+{
+ bool user_aligned;
+
+ memset(rr, 0, sizeof(*rr));
+ rr->dev = dev;
+ rr->user_align = pci_specified_resource_alignment(dev, &user_aligned);
+ if (!user_aligned)
+ rr->user_align = 0;
+}
+
resource_size_t pcibios_align_resource(void *data, const struct resource *res,
const struct resource *empty_res,
resource_size_t size,
resource_size_t align);
-resource_size_t pci_align_resource(struct pci_dev *dev,
+resource_size_t pci_align_resource(struct pci_resreq_data *rr,
const struct resource *res,
const struct resource *empty_res,
resource_size_t size,
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v4 3/7] PCI: Honor alignment overrides
2026-10-21 12:49 ` [PATCH v4 3/7] PCI: Honor alignment overrides Ilpo Järvinen
@ 2026-10-21 13:22 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-10-21 13:22 UTC (permalink / raw)
To: Ilpo Järvinen, linux-pci, dri-devel, intel-gfx
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] PCI: pci_align_resource() still misaligns resources when size < align
--
commit b332469defd2a6e720c73527f0af741c6fe04a04
Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
PCI: Honor alignment overrides
This commit introduces struct pci_resreq_data to pass the PCI device and
user-specified resource alignment into pci_align_resource() so that
remainder movement is skipped when a user alignment override is active.
Link: https://lore.kernel.org/linux-pci/20260923133202.07DF61F000FF@smtp.kernel.org/
[ ... ]
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index 376f09630a4ab..441a62807719d 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -265,17 +265,21 @@ resource_size_t pci_resource_alignment(const struct pci_dev *dev,
> * before res->start if there's enough free space there. This enables
> * tighter packing for resources.
> */
> -resource_size_t pci_align_resource(struct pci_dev *dev,
> +resource_size_t pci_align_resource(struct pci_resreq_data *rr,
> const struct resource *res,
> const struct resource *empty_res,
> resource_size_t size,
> resource_size_t align)
> {
> + struct pci_dev *dev = rr->dev;
> resource_size_t remainder, start_addr;
>
> if (!(res->flags & IORESOURCE_MEM))
> return res->start;
>
> + if (rr->user_align)
> + return res->start;
> +
> if (IS_ALIGNED(size, align))
> return res->start;
>
[ ... ]
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index d31a8d107b1ef..ea7d758712691 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1217,12 +1217,40 @@ int __must_check pcibios_enable_device(struct pci_dev *, int mask);
> /* Architecture-specific versions may override this (weak) */
> char *pcibios_setup(char *str);
>
> -/* Used only when drivers/pci/setup.c is used */
> +/* Resource assignment request handling */
> +/**
> + * struct pci_resreq_data - Data passed to pcibios_align_resource()
> + * @dev: PCI device
> + * @user_align: User provided alignment override (pci=resource_alignment param)
> + */
> +struct pci_resreq_data {
> + struct pci_dev *dev;
> + resource_size_t user_align;
> +};
> +
> +resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
> + bool *resize);
> +/*
> + * TODO: Improve PCI resource assignment interface so that callers outside PCI
> + * core do not need to know about this detail.
> + */
> +static inline void pci_init_pci_resreq_data(struct pci_resreq_data *rr,
> + struct pci_dev *dev)
> +{
> + bool user_aligned;
> +
> + memset(rr, 0, sizeof(*rr));
> + rr->dev = dev;
> + rr->user_align = pci_specified_resource_alignment(dev, &user_aligned);
> + if (!user_aligned)
> + rr->user_align = 0;
> +}
[Severity: High]
Can pci_align_resource() still misalign resources when size < align?
Looking at pci_reassigndev_resource_alignment() ->
pci_request_resource_alignment() in drivers/pci/pci.c:
if (resize) {
r->start = 0;
r->end = align - 1;
} else {
r->flags &= ~IORESOURCE_SIZEALIGN;
r->flags |= IORESOURCE_STARTALIGN;
resource_set_range(r, align, size);
}
When a device matches pci=resource_alignment= so resize (user_aligned) is
true, BARs in 0..PCI_ROM_RESOURCE with size < align are resized so that
size == align. For those BARs, IS_ALIGNED(size, align) in
pci_align_resource() is already true, so rr->user_align is not needed to
skip remainder calculation for them.
Conversely, when resize (user_aligned) is false because align comes from
pcibios_default_alignment() (such as pnv_pci_default_alignment() returning
PAGE_SIZE on PowerNV), pci_request_resource_alignment() takes the else
branch and sets IORESOURCE_STARTALIGN while keeping size < align. However,
pci_init_pci_resreq_data() clears rr->user_align = 0 when !user_aligned.
Whenever pci_align_resource() in drivers/pci/setup-res.c is called with
size < align and rr->user_align == 0 (whether for IORESOURCE_STARTALIGN
BARs via pcibios_default_alignment(), even if moved to
pci_resource_alignment(), or for a bridge window whose alignment was raised
above its size):
if (IS_ALIGNED(size, align))
return res->start;
remainder = size - ALIGN_DOWN(size, align);
ALIGN_DOWN(size, align) evaluates to 0 and remainder equals size. Because
there is no aligning body, subtracting remainder shifts the entire resource
to res->start - size (and in the later commits "PCI: Fix nesting windows
with remainder at the left edge" and "PCI: Place resources to either edge
of the window", lowers constraint->align to 1U and selects candidate[1] or
candidate[3] at aligned_edge - size), placing the entire resource below the
aligned boundary at an address that is only size-aligned rather than
align-aligned.
Additionally, when a PCI-to-PCI bridge is specified in
pci=resource_alignment= to allow expanding its bridge windows,
pci_reassigndev_resource_alignment() only unsets
PCI_BRIDGE_RESOURCES..PCI_NUM_RESOURCES - 1 without requesting user_align
on those windows. Because pci_init_pci_resreq_data() sets rr->user_align on
the bridge pci_dev for all of its resources, wouldn't this disable
remainder handling on the bridge windows (and in the later commit "PCI:
Place resources to either edge of the window", reject bridge window
candidates not aligned to rr->user_align)?
Would checking size <= align in pci_align_resource() (or restricting
remainder relocation to composite bridge window resources where size >
align) avoid shifting resources with size < align below their alignment
boundary?
[ ... ]
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261021124956.4250-1-ilpo.jarvinen@linux.intel.com?part=3
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v4 4/7] PCI: Fix nesting windows with remainder at the left edge
2026-10-21 12:49 [PATCH v4 0/7] PCI: Resource placement algorithm fixe Ilpo Järvinen
` (2 preceding siblings ...)
2026-10-21 12:49 ` [PATCH v4 3/7] PCI: Honor alignment overrides Ilpo Järvinen
@ 2026-10-21 12:49 ` Ilpo Järvinen
2026-10-21 13:32 ` sashiko-bot
2026-10-21 12:49 ` [PATCH v4 5/7] linux/log2.h: Add 64-bit roundup/down_pow_of_two64() variants Ilpo Järvinen
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Ilpo Järvinen @ 2026-10-21 12:49 UTC (permalink / raw)
To: Maciej Grochowski, Nikolas Joshua Britton, Geramy Loveless,
Eric Auger, Alexey Fomenko, Bjorn Helgaas, linux-pci,
Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński,
Ilpo Järvinen, linux-kernel
Cc: Bradley Morgan
If a bridge window has its non-aligning remainder at the left edge of
the window, assigning a nested bridge window of same size will fail
because resource side code will only allow starting a candicate range
from an aligning address. In such case, aligned address is somewhere in
the middle, not at the left edge of the window. Since the entire
available space is required for full-sized nested bridge window,
resource side code rejects the free space.
To solve this problem, PCI side code has to bypass the resource side
alignment code by giving a smaller alignment and must check the
aligment requirement itself. This allows resource side code to consider
the entire available space as candidate to fit the required size.
Wrap pcibios_align_resource() with PCI core function that performs the
alignment checks and left edge recalculation if needed. As the
alignment given for the resource side code is artificially small, the
real alignment has to be passed through alignf_data.
Fixes: 9036bd0efcb6 ("PCI: Align head space better")
Tested-by: Nikolas Joshua Britton <nbritton@exabit.io>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
I've not seen a report about this problem (thus no log quotes) but with
nested thunderbolt topologies it seems well within realms of possibility
of occurring.
---
drivers/pci/setup-res.c | 60 +++++++++++++++++++++++++++++++++++++----
include/linux/pci.h | 1 +
2 files changed, 56 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 441a62807719..2ca3784aa323 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -13,6 +13,8 @@
* Resource sorting
*/
+#include <linux/align.h>
+#include <linux/bug.h>
#include <linux/kernel.h>
#include <linux/export.h>
#include <linux/pci.h>
@@ -260,6 +262,12 @@ resource_size_t pci_resource_alignment(const struct pci_dev *dev,
return resource_alignment(res);
}
+static resource_size_t pci_resreq_remainder(resource_size_t size,
+ resource_size_t align)
+{
+ return size - ALIGN_DOWN(size, align);
+}
+
/*
* For mem bridge windows, try to relocate tail remainder space to space
* before res->start if there's enough free space there. This enables
@@ -280,10 +288,17 @@ resource_size_t pci_align_resource(struct pci_resreq_data *rr,
if (rr->user_align)
return res->start;
- if (IS_ALIGNED(size, align))
+ remainder = pci_resreq_remainder(size, align);
+ if (!remainder)
+ return res->start;
+
+ /*
+ * Size constraints forced an early start move in
+ * pci_check_and_align_resource()?
+ */
+ if (!IS_ALIGNED(res->start, align))
return res->start;
- remainder = size - ALIGN_DOWN(size, align);
/* Don't mess with size that doesn't align with window size granularity */
if (!IS_ALIGNED(remainder, pci_min_window_alignment(dev->bus, res->flags)))
return res->start;
@@ -315,6 +330,33 @@ resource_size_t __weak pcibios_align_resource(void *data,
return pci_align_resource(rr, res, empty_res, size, align);
}
+static resource_size_t pci_resreq_check(void *data,
+ const struct resource *res,
+ const struct resource *empty_res,
+ resource_size_t size,
+ resource_size_t min_align)
+{
+ struct pci_resreq_data *rr = data;
+ resource_size_t align = rr->real_align;
+ resource_size_t remainder, start;
+ struct resource tmp = *res;
+
+ if (res->flags & IORESOURCE_MEM && !IS_ALIGNED(res->start, align)) {
+ resource_set_range(&tmp, ALIGN(res->start, align), size);
+ if (!__resource_contains_unbound(empty_res, &tmp)) {
+ remainder = pci_resreq_remainder(size, align);
+ resource_set_range(&tmp, tmp.start - remainder, size);
+ if (!__resource_contains_unbound(empty_res, &tmp))
+ return tmp.start; /* caller skips range */
+ }
+ }
+
+ start = pcibios_align_resource(rr, &tmp, empty_res, size, align);
+ WARN_ON_ONCE(!IS_ALIGNED(start, min_align));
+
+ return start;
+}
+
static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
int resno, resource_size_t size, resource_size_t align)
{
@@ -324,9 +366,17 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
int ret;
pci_init_pci_resreq_data(&rr, dev);
+ rr.real_align = align;
min = (res->flags & IORESOURCE_IO) ? PCIBIOS_MIN_IO : PCIBIOS_MIN_MEM;
+ if (!rr.user_align &&
+ (res->flags & IORESOURCE_MEM) && pci_resreq_remainder(size, align)) {
+ align = 1U;
+ if (pci_resource_is_bridge_win(resno))
+ align = pci_min_window_alignment(bus, res->flags);
+ }
+
/*
* First, try exact prefetching match. Even if a 64-bit
* prefetchable bridge window is below 4GB, we can't put a 32-bit
@@ -336,7 +386,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
*/
ret = pci_bus_alloc_resource(bus, res, size, align, min,
IORESOURCE_PREFETCH | IORESOURCE_MEM_64,
- pcibios_align_resource, &rr);
+ pci_resreq_check, &rr);
if (ret == 0)
return 0;
@@ -348,7 +398,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
(IORESOURCE_PREFETCH | IORESOURCE_MEM_64)) {
ret = pci_bus_alloc_resource(bus, res, size, align, min,
IORESOURCE_PREFETCH,
- pcibios_align_resource, &rr);
+ pci_resreq_check, &rr);
if (ret == 0)
return 0;
}
@@ -361,7 +411,7 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
*/
if (res->flags & (IORESOURCE_PREFETCH | IORESOURCE_MEM_64))
ret = pci_bus_alloc_resource(bus, res, size, align, min, 0,
- pcibios_align_resource, &rr);
+ pci_resreq_check, &rr);
return ret;
}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index ea7d75871269..d7b431c1ac0d 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1226,6 +1226,7 @@ char *pcibios_setup(char *str);
struct pci_resreq_data {
struct pci_dev *dev;
resource_size_t user_align;
+ resource_size_t real_align;
};
resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v4 4/7] PCI: Fix nesting windows with remainder at the left edge
2026-10-21 12:49 ` [PATCH v4 4/7] PCI: Fix nesting windows with remainder at the left edge Ilpo Järvinen
@ 2026-10-21 13:32 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-10-21 13:32 UTC (permalink / raw)
To: Ilpo Järvinen, linux-pci
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] PCI: pci_resreq_remainder() treats size < align as tail remainder
- [Medium] PCI: wrong bus passed to pci_min_window_alignment() in __pci_assign_resource()
--
commit d393d2dd34eb6d31504106da364759753902d029
Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
PCI: Fix nesting windows with remainder at the left edge
This commit wraps pcibios_align_resource() with pci_resreq_check() and passes
a smaller alignment constraint to the resource allocator when a memory
resource has a non-zero remainder. This allows nested bridge windows with a
remainder at the left edge to fit into parent windows of the same size while
preserving the real alignment in pci_resreq_data.
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index 441a62807719d..2ca3784aa323f 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
[ ... ]
> @@ -260,6 +262,12 @@ resource_size_t pci_resource_alignment(const struct pci_dev *dev,
> return resource_alignment(res);
> }
>
> +static resource_size_t pci_resreq_remainder(resource_size_t size,
> + resource_size_t align)
> +{
> + return size - ALIGN_DOWN(size, align);
> +}
> +
> /*
> * For mem bridge windows, try to relocate tail remainder space to space
> * before res->start if there's enough free space there. This enables
> @@ -280,10 +288,17 @@ resource_size_t pci_align_resource(struct pci_resreq_data *rr,
> if (rr->user_align)
> return res->start;
>
> - if (IS_ALIGNED(size, align))
> + remainder = pci_resreq_remainder(size, align);
> + if (!remainder)
> + return res->start;
> +
> + /*
> + * Size constraints forced an early start move in
> + * pci_check_and_align_resource()?
> + */
> + if (!IS_ALIGNED(res->start, align))
> return res->start;
>
> - remainder = size - ALIGN_DOWN(size, align);
> /* Don't mess with size that doesn't align with window size granularity */
> if (!IS_ALIGNED(remainder, pci_min_window_alignment(dev->bus, res->flags)))
> return res->start;
> @@ -315,6 +330,33 @@ resource_size_t __weak pcibios_align_resource(void *data,
> return pci_align_resource(rr, res, empty_res, size, align);
> }
>
> +static resource_size_t pci_resreq_check(void *data,
> + const struct resource *res,
> + const struct resource *empty_res,
> + resource_size_t size,
> + resource_size_t min_align)
> +{
> + struct pci_resreq_data *rr = data;
> + resource_size_t align = rr->real_align;
> + resource_size_t remainder, start;
> + struct resource tmp = *res;
> +
> + if (res->flags & IORESOURCE_MEM && !IS_ALIGNED(res->start, align)) {
> + resource_set_range(&tmp, ALIGN(res->start, align), size);
> + if (!__resource_contains_unbound(empty_res, &tmp)) {
> + remainder = pci_resreq_remainder(size, align);
> + resource_set_range(&tmp, tmp.start - remainder, size);
> + if (!__resource_contains_unbound(empty_res, &tmp))
> + return tmp.start; /* caller skips range */
> + }
> + }
[Severity: High]
What happens in pci_resreq_remainder() and pci_resreq_check() when a memory
resource has size < align and rr.user_align is 0?
When size < align, ALIGN_DOWN(size, align) evaluates to 0, so
pci_resreq_remainder(size, align) returns size instead of 0. That treats the
entire resource as a movable remainder even though it has no align-sized
portion.
This can occur with rr.user_align == 0 in paths such as:
1. On PowerNV, pcibios_default_alignment() -> pnv_pci_default_alignment()
returns PAGE_SIZE (for example, 64 KiB). When no pci=resource_alignment=
parameter matches the device, pci_reassigndev_resource_alignment() ->
pci_specified_resource_alignment(dev, &resize) returns align = PAGE_SIZE
with resize = false. For a 4 KiB BAR, pci_request_resource_alignment()
sets IORESOURCE_STARTALIGN with r->start = PAGE_SIZE while keeping
size = 4 KiB, and pci_init_pci_resreq_data() leaves rr.user_align = 0.
2. For a CardBus bridge (PCI_CLASS_BRIDGE_CARDBUS), BAR 0 has size = 4 KiB
while pci_resource_alignment() -> pci_cardbus_resource_alignment()
returns 64 MiB with rr.user_align == 0.
In __pci_assign_resource(), because pci_resreq_remainder(size, align) is
non-zero and !pci_resource_is_bridge_win(resno), the alignment constraint
passed to pci_bus_alloc_resource() is lowered to 1U.
Then in pci_resreq_check(), if an unaligned free range empty_res cannot fit
[ALIGN(res->start, align), ALIGN(res->start, align) + size - 1], subtracting
remainder (which equals size) shifts tmp.start to
ALIGN(res->start, align) - size. If that range fits within empty_res, the
resource is assigned just below the alignment boundary at an unaligned
address (for example, 0x8000f000 in [0x80001000, 0x8000ffff] after an earlier
4 KiB BAR at 0x80000000), violating the required alignment.
Later in the series, commit "PCI: Place resources to either edge of the
window" also uses pci_resreq_remainder() in pci_align_resource() to construct
candidates at ALIGN(empty_res->start, align) - remainder and
ALIGN_DOWN(empty_res->end + 1, align) - size, which similarly selects
unaligned addresses when size < align.
Should pci_resreq_remainder() return 0 when size < align?
> +
> + start = pcibios_align_resource(rr, &tmp, empty_res, size, align);
> + WARN_ON_ONCE(!IS_ALIGNED(start, min_align));
> +
> + return start;
> +}
> +
> static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
> int resno, resource_size_t size, resource_size_t align)
> {
> @@ -324,9 +366,17 @@ static int __pci_assign_resource(struct pci_bus *bus, struct pci_dev *dev,
> int ret;
>
> pci_init_pci_resreq_data(&rr, dev);
> + rr.real_align = align;
>
> min = (res->flags & IORESOURCE_IO) ? PCIBIOS_MIN_IO : PCIBIOS_MIN_MEM;
>
> + if (!rr.user_align &&
> + (res->flags & IORESOURCE_MEM) && pci_resreq_remainder(size, align)) {
> + align = 1U;
> + if (pci_resource_is_bridge_win(resno))
> + align = pci_min_window_alignment(bus, res->flags);
> + }
[Severity: Medium]
Should pci_min_window_alignment() here (and in pci_align_resource()) be
passed dev->subordinate instead of bus?
When sizing bridge windows in pbus_size_io() and pbus_size_mem(),
pci_min_window_alignment() is called with the bridge's subordinate bus
(where bus->self is the bridge device dev):
drivers/pci/setup-bus.c:pbus_size_mem() {
...
win_align = pci_min_window_alignment(bus, b_res->flags);
}
On PowerNV, pnv_pci_window_alignment() walks up from bridge = bus->self
counting PCI_EXP_TYPE_PCI_BRIDGE devices:
arch/powerpc/platforms/powernv/pci-ioda.c:pnv_pci_window_alignment() {
...
bridge = bus->self;
while (bridge) {
if (pci_pcie_type(bridge) == PCI_EXP_TYPE_PCI_BRIDGE) {
num_pci_bridges++;
if (num_pci_bridges >= 2)
return 1;
}
bridge = bridge->bus->self;
}
...
}
In __pci_assign_resource(), dev is the bridge whose window res is being
assigned, while bus is dev->bus (or an ancestor bus). For a second-level
PCI_EXP_TYPE_PCI_BRIDGE bridge on PowerNV, pbus_size_mem(dev->subordinate,
...) sizes the window with a 1 MiB minimum window alignment
(num_pci_bridges >= 2), whereas calling pci_min_window_alignment(bus,
res->flags) with dev->bus sees num_pci_bridges == 1 and returns m32_segsize
or m64_segsize (for example, 16 MiB or 64 MiB) as min_align.
If pci_resreq_check() (or pci_align_resource() in the subsequent commit
"PCI: Place resources to either edge of the window") places the window at an
address shifted by remainder that is aligned to dev->subordinate's 1 MiB
window alignment but not to the parent bus's segment size, can that reject
valid candidates or trigger WARN_ON_ONCE(!IS_ALIGNED(start, min_align)) in
pci_resreq_check()?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261021124956.4250-1-ilpo.jarvinen@linux.intel.com?part=4
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v4 5/7] linux/log2.h: Add 64-bit roundup/down_pow_of_two64() variants
2026-10-21 12:49 [PATCH v4 0/7] PCI: Resource placement algorithm fixe Ilpo Järvinen
` (3 preceding siblings ...)
2026-10-21 12:49 ` [PATCH v4 4/7] PCI: Fix nesting windows with remainder at the left edge Ilpo Järvinen
@ 2026-10-21 12:49 ` Ilpo Järvinen
2026-10-21 12:57 ` sashiko-bot
2026-10-21 12:49 ` [PATCH v4 6/7] PCI: Place resources to either edge of the window Ilpo Järvinen
2026-10-21 12:49 ` [PATCH v4 7/7] PCI: Fix composite resource sizing Ilpo Järvinen
6 siblings, 1 reply; 15+ messages in thread
From: Ilpo Järvinen @ 2026-10-21 12:49 UTC (permalink / raw)
To: Maciej Grochowski, Nikolas Joshua Britton, Geramy Loveless,
Eric Auger, Alexey Fomenko, Bjorn Helgaas, linux-pci,
Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński,
linux-kernel
Cc: Bradley Morgan, Ilpo Järvinen
Generic sounding roundup_pow_of_two() and rounddown_pow_of_two() are
internally based on fls() that limits types to unsigned long. The type
limitation is unexpected and undocumented.
Worse yet, the typing seems also inconsistent between
__builtin_constant_p() and __roundup/down_pow_of_two() branches, the
former using full 64-bit size and the latter truncating type.
The most sensible way seems to be eventually changing types such that
the return type for roundup/down_pow_of_two() is same as input but it
requires auditing all the existing callers that have mixed input/output
types.
As a stop gap solution, add roundup/down_pow_of_two64() that do not
unexpectedly truncate 64-bit input.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
include/linux/log2.h | 52 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/include/linux/log2.h b/include/linux/log2.h
index e17ceb32e0c9..180e5702f677 100644
--- a/include/linux/log2.h
+++ b/include/linux/log2.h
@@ -57,6 +57,16 @@ unsigned long __roundup_pow_of_two(unsigned long n)
return 1UL << fls_long(n - 1);
}
+/**
+ * __roundup_pow_of_two64() - round up to nearest power of two
+ * @n: value to round up
+ */
+static inline __attribute__((const))
+u64 __roundup_pow_of_two64(u64 n)
+{
+ return 1ULL << fls64(n - 1);
+}
+
/**
* __rounddown_pow_of_two() - round down to nearest power of two
* @n: value to round down
@@ -67,6 +77,16 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
return 1UL << (fls_long(n) - 1);
}
+/**
+ * __rounddown_pow_of_two64() - round down to nearest power of two
+ * @n: value to round down
+ */
+static inline __attribute__((const))
+u64 __rounddown_pow_of_two64(u64 n)
+{
+ return 1ULL << (fls64(n) - 1);
+}
+
/**
* const_ilog2 - log base 2 of 32-bit or a 64-bit constant unsigned value
* @n: parameter
@@ -163,6 +183,23 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
__ilog2_u64(n) \
)
+/**
+ * roundup_pow_of_two64 - round the given value up to nearest power of two
+ * @n: parameter
+ *
+ * round the given value up to the nearest power of two
+ * - the result is undefined when n == 0
+ * - this can be used to initialise global variables from constant data
+ */
+#define roundup_pow_of_two64(n) \
+( \
+ __builtin_constant_p(n) ? ( \
+ ((n) == 1) ? 1 : \
+ (1UL << (ilog2((n) - 1) + 1)) \
+ ) : \
+ __roundup_pow_of_two64(n) \
+ )
+
/**
* roundup_pow_of_two - round the given value up to nearest power of two
* @n: parameter
@@ -180,6 +217,21 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
__roundup_pow_of_two(n) \
)
+/**
+ * rounddown_pow_of_two64 - round the given value down to nearest power of two
+ * @n: parameter
+ *
+ * round the given value down to the nearest power of two
+ * - the result is undefined when n == 0
+ * - this can be used to initialise global variables from constant data
+ */
+#define rounddown_pow_of_two64(n) \
+( \
+ __builtin_constant_p(n) ? ( \
+ (1UL << ilog2(n))) : \
+ __rounddown_pow_of_two64(n) \
+ )
+
/**
* rounddown_pow_of_two - round the given value down to nearest power of two
* @n: parameter
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v4 5/7] linux/log2.h: Add 64-bit roundup/down_pow_of_two64() variants
2026-10-21 12:49 ` [PATCH v4 5/7] linux/log2.h: Add 64-bit roundup/down_pow_of_two64() variants Ilpo Järvinen
@ 2026-10-21 12:57 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-10-21 12:57 UTC (permalink / raw)
To: Ilpo Järvinen, linux-pci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] log2: 32-bit 1UL shift overflow in roundup/down_pow_of_two64()
--
commit 43925e5ad2f800f4d000698aad5a8426cb02653d
Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
linux/log2.h: Add 64-bit roundup/down_pow_of_two64() variants
This commit adds 64-bit roundup_pow_of_two64() and rounddown_pow_of_two64()
helpers in include/linux/log2.h so 64-bit inputs are not truncated to
unsigned long on 32-bit architectures.
> diff --git a/include/linux/log2.h b/include/linux/log2.h
> index e17ceb32e0c97..180e5702f6779 100644
> --- a/include/linux/log2.h
> +++ b/include/linux/log2.h
[ ... ]
> @@ -163,6 +183,23 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
> __ilog2_u64(n) \
> )
>
> +/**
> + * roundup_pow_of_two64 - round the given value up to nearest power of two
> + * @n: parameter
> + *
> + * round the given value up to the nearest power of two
> + * - the result is undefined when n == 0
> + * - this can be used to initialise global variables from constant data
> + */
> +#define roundup_pow_of_two64(n) \
> +( \
> + __builtin_constant_p(n) ? ( \
> + ((n) == 1) ? 1 : \
> + (1UL << (ilog2((n) - 1) + 1)) \
> + ) : \
> + __roundup_pow_of_two64(n) \
> + )
> +
> /**
> * roundup_pow_of_two - round the given value up to nearest power of two
> * @n: parameter
> @@ -180,6 +217,21 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
> __roundup_pow_of_two(n) \
> )
>
> +/**
> + * rounddown_pow_of_two64 - round the given value down to nearest power of two
> + * @n: parameter
> + *
> + * round the given value down to the nearest power of two
> + * - the result is undefined when n == 0
> + * - this can be used to initialise global variables from constant data
> + */
> +#define rounddown_pow_of_two64(n) \
> +( \
> + __builtin_constant_p(n) ? ( \
> + (1UL << ilog2(n))) : \
> + __rounddown_pow_of_two64(n) \
> + )
[Severity: Medium]
In include/linux/log2.h, __roundup_pow_of_two64() and
__rounddown_pow_of_two64() shift 1ULL, but the __builtin_constant_p(n)
branches in roundup_pow_of_two64() and rounddown_pow_of_two64() still shift
1UL:
(1UL << (ilog2((n) - 1) + 1))
and:
(1UL << ilog2(n))
On 32-bit architectures where unsigned long is 32 bits wide, the left shift
is evaluated in the type of its left operand (unsigned long) before the
conditional expression is converted to u64. When n is a compile-time constant
with n > (1ULL << 31) in roundup_pow_of_two64(n) or n >= (1ULL << 32) in
rounddown_pow_of_two64(n), the shift count is at least 32, which overflows
the 32-bit 1UL operand and triggers a shift-count-overflow warning while
truncating the result.
Should both __builtin_constant_p(n) branches use 1ULL instead of 1UL (and
1ULL for the (n) == 1 case in roundup_pow_of_two64())?
> +
> /**
> * rounddown_pow_of_two - round the given value down to nearest power of two
> * @n: parameter
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261021124956.4250-1-ilpo.jarvinen@linux.intel.com?part=5
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v4 6/7] PCI: Place resources to either edge of the window
2026-10-21 12:49 [PATCH v4 0/7] PCI: Resource placement algorithm fixe Ilpo Järvinen
` (4 preceding siblings ...)
2026-10-21 12:49 ` [PATCH v4 5/7] linux/log2.h: Add 64-bit roundup/down_pow_of_two64() variants Ilpo Järvinen
@ 2026-10-21 12:49 ` Ilpo Järvinen
2026-10-21 13:30 ` sashiko-bot
2026-10-21 12:49 ` [PATCH v4 7/7] PCI: Fix composite resource sizing Ilpo Järvinen
6 siblings, 1 reply; 15+ messages in thread
From: Ilpo Järvinen @ 2026-10-21 12:49 UTC (permalink / raw)
To: Maciej Grochowski, Nikolas Joshua Britton, Geramy Loveless,
Eric Auger, Alexey Fomenko, Bjorn Helgaas, linux-pci,
Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński,
Ilpo Järvinen, linux-kernel
Cc: Bradley Morgan, Eric Auger
PCI resource assignment phase based on a greedy algorithm. The
resources are assigned in descending order of required alignment.
Because the PCI BARs are power-of-two sized resources, it mostly works
(bridge windows and VF BARs are composite resources that might not
necessarily have power-of-two size but internally they are still
composed of BARs).
The resource assignment prior to the commit 9036bd0efcb6 ("PCI: Align
head space better") placed resource to/towards the left edge of the
window. The commit 9036bd0efcb6 ("PCI: Align head space better")
altered assignment for resources whose size is not a perfect multiple
of the required alignment by moving the remainder before the left edge
of the window if possible.
The behavior after the commit 9036bd0efcb6 ("PCI: Align head space
better") may result in problems when a bridge window consists of one
large alignment resource and a composite one with a smaller alignment
(this is a typical setup for GPUs PF BAR and VF BARs). The resource
with largest alignment is assigned first and placed such that the
remainder space is left of the assigned resource, which effectively
splits the remaining space into two. While the VF BAR could fit to the
remaining space, it requires continuous free space that is no longer
there because the large resource is now in the middle.
In this log exceprt, BAR 2 is placed in the middle of the window blocking
the larger VF BAR 2 from fitting anywhere:
pci 0000:03:01.0: bridge window [mem 0xa9f8000000-0xbfffffffff 64bit pref]: assigned
pci 0000:04:00.0: BAR 2 [mem 0xb000000000-0xb7ffffffff 64bit pref]: assigned
pci 0000:04:00.0: VF BAR 2 [mem size 0xe00000000 64bit pref]: can't assign; no space
pci 0000:04:00.0: VF BAR 2 [mem size 0xe00000000 64bit pref]: failed to assign
The old behavior, despite being greedy, naturally consumed space from
the left edge leaving the remainder space adjacent to the other free
space. When remainder space is at the left edge of the window, the
greedy algorithm should instead assign to the right edge of the window.
(If both ends do align, either end works equally.)
While defining window edge aware resource assignment algorithm, one
additional thing is useful to note. In DT setups the bridge
windows/root bus resources are often very precisely sized, with right
edge of the window having much smaller alignment that the left edge.
They also often come with a small BAR that should be placed to the
right edge to not block bridge window placed to the left edge of the
window. The bridge window may be entirely optional at this point
because of hotplug. The resource assignment fallback phase assigns
mandatory resources first and in such a case, small BAR gets assigned
first.
One example where mandatory BAR 0 blocks bridge windows from fitting
(both bridge windows wouldn't fit because of qemu not providing enough
space for both bridge windows but one should fit like it was originally
setup by the platform):
pci_bus 0000:0a: root bus resource [mem 0x10a00000-0x10c00fff window]
pci 0000:0a:00.0: [1b36:000c] type 01 class 0x060400 PCIe Root Port
pci 0000:0a:00.0: BAR 0 [mem 0x10c00000-0x10c00fff]
pci 0000:0a:00.0: PCI bridge to [bus 0b-0d]
pci 0000:0a:00.0: bridge window [mem 0x10a00000-0x10bfffff]
pci 0000:0a:00.0: enabling Extended Tags
pci 0000:0a:00.0: bridge window [mem 0x00100000-0x000fffff 64bit pref] to [bus 0b-0d] add_size 200000 add_align 100000
pci 0000:0a:00.0: bridge window [mem 0x00100000-0x000fffff] to [bus 0b-0d] add_size 200000 add_align 100000
pci 0000:0a:00.0: bridge window [mem 0x10a00000-0x10bfffff]: assigned
pci 0000:0a:00.0: bridge window [mem size 0x00200000 64bit pref]: can't assign; no space
pci 0000:0a:00.0: bridge window [mem size 0x00200000 64bit pref]: failed to assign
pci 0000:0a:00.0: BAR 0 [mem 0x10c00000-0x10c00fff]: assigned
pci 0000:0a:00.0: bridge window [mem 0x10a00000-0x10bfffff]: releasing
pci 0000:0a:00.0: BAR 0 [mem 0x10c00000-0x10c00fff]: releasing
pci 0000:0a:00.0: BAR 0 [mem 0x10a00000-0x10a00fff]: assigned
pci 0000:0a:00.0: bridge window [mem size 0x00200000]: can't assign; no space
pci 0000:0a:00.0: bridge window [mem size 0x00200000]: failed to assign
pci 0000:0a:00.0: bridge window [mem size 0x00200000 64bit pref]: can't assign; no space
pci 0000:0a:00.0: bridge window [mem size 0x00200000 64bit pref]: failed to assign
pci_bus 0000:0a: Some PCI device resources are unassigned, try booting with pci=realloc
To achieve best generalization of the approach, the solution should
also avoid consuming space from the window edge that can fit largest
aligning resources in the future, whenever possible.
Fixes: 9036bd0efcb6 ("PCI: Align head space better")
Reported-by: Alexey Fomenko <alexey.fomenko@intel.com>
Tested-by: Alexey Fomenko <alexey.fomenko@intel.com>
Reported-by: Eric Auger <eauger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Nikolas Joshua Britton <nbritton@exabit.io>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
Both reports are private discussions (Eric's report started as public
and produced another fix but further problem was only visible in the
privately send logs after that). Thus no links.
---
drivers/pci/pci.h | 4 +
drivers/pci/setup-bus.c | 4 -
drivers/pci/setup-res.c | 170 ++++++++++++++++++++++++++++++++++------
3 files changed, 148 insertions(+), 30 deletions(-)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index ba3c3fddddc2..82a5fd267f5b 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -106,6 +106,10 @@ struct pcie_tlp_log;
#define PCI_BUS_BRIDGE_MEM_WINDOW 1
#define PCI_BUS_BRIDGE_PREF_MEM_WINDOW 2
+#define PCI_RES_TYPE_MASK \
+ (IORESOURCE_IO | IORESOURCE_MEM | IORESOURCE_PREFETCH |\
+ IORESOURCE_MEM_64)
+
#define PCI_EXP_AER_FLAGS (PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | \
PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index e8c94aa1d3c1..7ca0e9f4ffb6 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -31,10 +31,6 @@
#include <linux/acpi.h>
#include "pci.h"
-#define PCI_RES_TYPE_MASK \
- (IORESOURCE_IO | IORESOURCE_MEM | IORESOURCE_PREFETCH |\
- IORESOURCE_MEM_64)
-
unsigned int pci_flags;
EXPORT_SYMBOL_GPL(pci_flags);
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 2ca3784aa323..0833ea88bfa1 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -17,6 +17,9 @@
#include <linux/bug.h>
#include <linux/kernel.h>
#include <linux/export.h>
+#include <linux/limits.h>
+#include <linux/log2.h>
+#include <linux/minmax.h>
#include <linux/pci.h>
#include <linux/errno.h>
#include <linux/ioport.h>
@@ -262,16 +265,84 @@ resource_size_t pci_resource_alignment(const struct pci_dev *dev,
return resource_alignment(res);
}
+static resource_size_t pci_max_natural_size(const struct resource *res,
+ resource_size_t *max_align)
+{
+ resource_size_t size = resource_size(res);
+ resource_size_t powof2, natural_start;
+
+ *max_align = 1;
+ if (!size)
+ return 0;
+
+ powof2 = rounddown_pow_of_two64(size);
+ natural_start = ALIGN(res->start, powof2);
+ /* end = ~0 first overflows, then -1 brings it back */
+ if (natural_start >= ALIGN_DOWN(res->end + 1, powof2) - 1) {
+ powof2 = max(powof2 / 2, 1U);
+ natural_start = ALIGN(res->start, powof2);
+ }
+
+ if (natural_start) {
+ *max_align <<= __ffs64(natural_start);
+ } else {
+ /*
+ * Zero address has infinite alignment, return the largest
+ * representable number even if it's not a power of two.
+ */
+ *max_align = RESOURCE_SIZE_MAX;
+ }
+
+ return powof2;
+}
+
static resource_size_t pci_resreq_remainder(resource_size_t size,
resource_size_t align)
{
return size - ALIGN_DOWN(size, align);
}
+/**
+ * pci_align_resource - Places resource into empty space range
+ * @rr: PCI resource request data
+ * @res: Candidate range calculated by caller (not among @dev's resources!)
+ * @empty_res: Full free space range
+ * @size: Required size for the resource
+ * @align: Required alignment (see below for details)
+ *
+ * Places resource inside @empty_res honoring @size and @align.
+ *
+ * Returns: start address for the resource.
+ */
/*
- * For mem bridge windows, try to relocate tail remainder space to space
- * before res->start if there's enough free space there. This enables
- * tighter packing for resources.
+ * Following candidate logic only applies to mem resources currently. For
+ * composite resources not divisable by @align, @align does not apply to
+ * non-aligning remainder part giving some leeway for its placement. There
+ * are 4 candidate positions:
+ *
+ * W0WWW0WWW0WWW
+ * 1. AAAAr
+ * 2. rAAAA
+ * 3. AAAAr
+ * 4. rAAAA
+ *
+ * W = bridge window
+ * 0 = bridge window offset matching align
+ * A = aligning part of size (size rounded down by align)
+ * r = non-aligning remainder
+ *
+ * Cases 1 & 3 and 2 & 4 may degenerate to the same candidate.
+ *
+ * Select resource placement based on the remaining free space. Pick the
+ * candidate with which the remaining free space has (in decreasing order of
+ * priority):
+ *
+ * 1. the largest naturally aligning power-of-two-sized address range within,
+ * 2. the largest continous free space,
+ * 3. the largest alignment of the start address for the naturally aligning
+ * free space range (from check 1).
+ *
+ * Cases 1 & 2 check only right edge free space and 3 & 4 the left edge.
*/
resource_size_t pci_align_resource(struct pci_resreq_data *rr,
const struct resource *res,
@@ -280,38 +351,85 @@ resource_size_t pci_align_resource(struct pci_resreq_data *rr,
resource_size_t align)
{
struct pci_dev *dev = rr->dev;
- resource_size_t remainder, start_addr;
+ unsigned long type = res->flags & PCI_RES_TYPE_MASK;
+ resource_size_t best_natural_size = 0, best_size = 0, best_maxalign = 0;
+ resource_size_t aligning, remainder;
+ struct resource candidate[4];
+ unsigned int i;
+ int best = -1;
if (!(res->flags & IORESOURCE_MEM))
return res->start;
- if (rr->user_align)
- return res->start;
-
remainder = pci_resreq_remainder(size, align);
- if (!remainder)
- return res->start;
+ aligning = size - remainder;
+
+ candidate[0] = DEFINE_RES(ALIGN(empty_res->start, align), size, type);
+ candidate[1] = DEFINE_RES(ALIGN(empty_res->start, align) - remainder,
+ size, type);
+ candidate[2] = DEFINE_RES(ALIGN_DOWN(empty_res->end + 1 - remainder, align) -
+ aligning, size, type);
+ candidate[3] = DEFINE_RES(ALIGN_DOWN(empty_res->end + 1, align) - size,
+ size, type);
+
+ for (i = 0; i < ARRAY_SIZE(candidate); i++) {
+ struct resource remaining;
+ resource_size_t natural_size, size, maxalign;
+
+ if ((candidate[i].start > candidate[i].end) ||
+ !__resource_contains_unbound(empty_res, &candidate[i])) {
+ pci_dbg(dev, "%pR: candidate %u not within free space %pR\n",
+ &candidate[i], i, empty_res);
+ continue;
+ }
- /*
- * Size constraints forced an early start move in
- * pci_check_and_align_resource()?
- */
- if (!IS_ALIGNED(res->start, align))
- return res->start;
+ if (rr->user_align &&
+ !IS_ALIGNED(candidate[i].start, rr->user_align)) {
+ pci_dbg(dev, "%pR: candidate %u skipped due to resource_alignment=%llx\n",
+ &candidate[i], i,
+ (unsigned long long)rr->user_align);
+ continue;
+ }
- /* Don't mess with size that doesn't align with window size granularity */
- if (!IS_ALIGNED(remainder, pci_min_window_alignment(dev->bus, res->flags)))
- return res->start;
- /* Try to place remainder that doesn't fill align before */
- if (res->start < remainder)
- return res->start;
- start_addr = res->start - remainder;
- if (empty_res->start > start_addr)
+ remaining.flags = type;
+ if (i <= 1) {
+ remaining.start = candidate[i].end + 1;
+ remaining.end = empty_res->end;
+ } else {
+ remaining.start = empty_res->start;
+ remaining.end = candidate[i].start - 1;
+ }
+
+ natural_size = pci_max_natural_size(&remaining, &maxalign);
+ size = resource_size(&remaining);
+ pci_dbg(dev, "%pR: candidate %u free space naturalsize=%llx size=%llx maxalign=%llx\n",
+ &candidate[i], i,
+ (unsigned long long)natural_size,
+ (unsigned long long)size,
+ (unsigned long long)maxalign);
+ if ((best < 0) ||
+ (natural_size > best_natural_size) ||
+ (natural_size == best_natural_size && size > best_size) ||
+ (natural_size == best_natural_size && size == best_size &&
+ maxalign > best_maxalign)) {
+ best = i;
+ best_natural_size = natural_size;
+ best_size = size;
+ best_maxalign = maxalign;
+ }
+ }
+
+ /* None fits? Return some address and let the caller deal with it. */
+ if (best == -1)
return res->start;
- pci_dbg(dev, "%pR: moving candidate start address below align to %llx\n",
- res, (unsigned long long)start_addr);
- return start_addr;
+ pci_dbg(dev, "%pR: candidate %u selected (free space %pR), size: %llx + %llx, align: %llx\n",
+ &candidate[best], best, empty_res,
+ (unsigned long long)aligning,
+ (unsigned long long)remainder,
+ (unsigned long long)align);
+
+ return candidate[best].start;
}
/*
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v4 6/7] PCI: Place resources to either edge of the window
2026-10-21 12:49 ` [PATCH v4 6/7] PCI: Place resources to either edge of the window Ilpo Järvinen
@ 2026-10-21 13:30 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-10-21 13:30 UTC (permalink / raw)
To: Ilpo Järvinen, linux-pci
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] pci: unaligned placement in pci_align_resource() when size is less than align
- [Medium] pci: pci_max_natural_size() underreports max_align after halving powof2
- [Medium] pci: WARN_ON_ONCE() in pci_resreq_check() when user_align is set with remainder
- [Medium] pci: pci_align_resource() bypasses user_align when returning on best == -1
- [Medium] PCI: unaligned candidate start addresses in pci_align_resource()
--
commit 19d57daaa9c86d1bd38d377d9e720319c17c02d3
Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
PCI: Place resources to either edge of the window
This commit updates pci_align_resource() to evaluate four candidate positions
at the left and right edges of an available free space range. It selects the
candidate that leaves the largest naturally aligned power-of-two range, the
largest contiguous free space, and the highest start alignment in the
remaining window space.
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index 2ca3784aa323f..0833ea88bfa1b 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
[ ... ]
> @@ -262,16 +265,84 @@ resource_size_t pci_resource_alignment(const struct pci_dev *dev,
> return resource_alignment(res);
> }
>
> +static resource_size_t pci_max_natural_size(const struct resource *res,
> + resource_size_t *max_align)
> +{
> + resource_size_t size = resource_size(res);
> + resource_size_t powof2, natural_start;
> +
> + *max_align = 1;
> + if (!size)
> + return 0;
> +
> + powof2 = rounddown_pow_of_two64(size);
> + natural_start = ALIGN(res->start, powof2);
> + /* end = ~0 first overflows, then -1 brings it back */
> + if (natural_start >= ALIGN_DOWN(res->end + 1, powof2) - 1) {
> + powof2 = max(powof2 / 2, 1U);
> + natural_start = ALIGN(res->start, powof2);
> + }
> +
> + if (natural_start) {
> + *max_align <<= __ffs64(natural_start);
> + } else {
> + /*
> + * Zero address has infinite alignment, return the largest
> + * representable number even if it's not a power of two.
> + */
> + *max_align = RESOURCE_SIZE_MAX;
> + }
> +
> + return powof2;
> +}
[Severity: Medium]
When the initial powof2 does not fit within res and pci_max_natural_size()
halves powof2, could unconditionally overwriting natural_start with
ALIGN(res->start, powof2) underreport max_align?
If the remaining range contains two naturally aligned blocks of the halved
powof2 size (for example, when res starts at an odd multiple of the halved
powof2 and has size equal to two such blocks), ALIGN(res->start, powof2)
selects the first block at the odd multiple (which only has alignment equal
to the halved powof2) rather than the second block at the original
natural_start (which is at an even multiple and has at least twice the
alignment).
For example, if a root bus window is [0x10900000-0x10b00fff] and a 4KB BAR 0
is assigned before a 2MB, 1MB-aligned bridge window:
1. candidate[0] places BAR 0 at the left edge [0x10900000-0x10900fff],
leaving remaining = [0x10901000-0x10b00fff] (size 0x200000). Halving
powof2 to 0x100000 gives natural_start = ALIGN(0x10901000, 0x100000) =
0x10a00000, so natural_size = 0x100000, size = 0x200000, and
maxalign = 0x200000.
2. candidate[2] places BAR 0 at the right edge [0x10b00000-0x10b00fff],
leaving remaining = [0x10900000-0x10afffff] (size 0x200000), which
preserves both 1MB blocks. However, halving powof2 to 0x100000 overwrites
natural_start with ALIGN(0x10900000, 0x100000) = 0x10900000, reporting
natural_size = 0x100000, size = 0x200000, and maxalign = 0x100000.
Because natural_size and size tie, maxalign breaks the tie in favor of
candidate[0], placing BAR 0 at the left edge and blocking the 2MB bridge
window from fitting. Should pci_max_natural_size() keep the original
natural_start when a halved powof2 block starting at that address still fits
within res?
> +
> static resource_size_t pci_resreq_remainder(resource_size_t size,
> resource_size_t align)
> {
> return size - ALIGN_DOWN(size, align);
> }
[ ... ]
> @@ -280,38 +351,85 @@ resource_size_t pci_align_resource(struct pci_resreq_data *rr,
> resource_size_t align)
> {
> struct pci_dev *dev = rr->dev;
> - resource_size_t remainder, start_addr;
> + unsigned long type = res->flags & PCI_RES_TYPE_MASK;
> + resource_size_t best_natural_size = 0, best_size = 0, best_maxalign = 0;
> + resource_size_t aligning, remainder;
> + struct resource candidate[4];
> + unsigned int i;
> + int best = -1;
>
> if (!(res->flags & IORESOURCE_MEM))
> return res->start;
>
> - if (rr->user_align)
> - return res->start;
> -
> remainder = pci_resreq_remainder(size, align);
> - if (!remainder)
> - return res->start;
> + aligning = size - remainder;
> +
> + candidate[0] = DEFINE_RES(ALIGN(empty_res->start, align), size, type);
> + candidate[1] = DEFINE_RES(ALIGN(empty_res->start, align) - remainder,
> + size, type);
> + candidate[2] = DEFINE_RES(ALIGN_DOWN(empty_res->end + 1 - remainder, align) -
> + aligning, size, type);
> + candidate[3] = DEFINE_RES(ALIGN_DOWN(empty_res->end + 1, align) - size,
> + size, type);
[Severity: High]
What happens here when a memory resource has size < align and
rr->user_align == 0?
In pci_resreq_remainder(size, align), ALIGN_DOWN(size, align) evaluates to 0
whenever size < align, so pci_resreq_remainder() returns size instead of 0:
drivers/pci/setup-res.c:pci_resreq_remainder() {
return size - ALIGN_DOWN(size, align);
}
Because pci_resreq_remainder(size, align) is non-zero and rr.user_align == 0,
__pci_assign_resource() lowers the alignment constraint passed to
pci_bus_alloc_resource() to 1U (or pci_min_window_alignment() for bridge
windows):
drivers/pci/setup-res.c:__pci_assign_resource() {
if (!rr.user_align &&
(res->flags & IORESOURCE_MEM) && pci_resreq_remainder(size, align)) {
align = 1U;
if (pci_resource_is_bridge_win(resno))
align = pci_min_window_alignment(bus, res->flags);
}
}
Then in pci_align_resource(), remainder = size and aligning = 0, which treats
the entire resource as a non-aligning remainder with a zero-sized aligning
part:
- candidate[1] is placed at ALIGN(empty_res->start, align) - size
- candidate[3] is placed at ALIGN_DOWN(empty_res->end + 1, align) - size
Neither candidate is aligned to align, and candidate[3] leaves align - size
more contiguous free space on the left than the aligned candidate[2] while
preserving empty_res->start (giving a higher maxalign than candidate[0]), so
pci_align_resource() selects candidate[3] (or candidate[1]) and assigns the
resource to an unaligned start address.
This can be reached when size < align with rr->user_align == 0 via:
- Platforms implementing pcibios_default_alignment() (such as PowerNV
pnv_pci_default_alignment() returning 64KB), where
pci_reassigndev_resource_alignment() -> pci_request_resource_alignment()
with resize == false sets IORESOURCE_STARTALIGN to 64KB while keeping a
smaller BAR size (e.g., 4KB) and leaving rr->user_align == 0.
- CardBus bridges, where pci_resource_alignment() returns
pci_cardbus_resource_alignment() (64MB) for all memory resources including
the 4KB BAR 0.
- SR-IOV VF BARs where pcibios_iov_resource_alignment() returns an alignment
larger than total_VFs * vf_bar_size, or resources in
reassign_resources_sorted() where add_align > size.
Should pci_resreq_remainder() return 0 when size < align?
> +
> + for (i = 0; i < ARRAY_SIZE(candidate); i++) {
> + struct resource remaining;
> + resource_size_t natural_size, size, maxalign;
> +
> + if ((candidate[i].start > candidate[i].end) ||
> + !__resource_contains_unbound(empty_res, &candidate[i])) {
> + pci_dbg(dev, "%pR: candidate %u not within free space %pR\n",
> + &candidate[i], i, empty_res);
> + continue;
> + }
>
> - /*
> - * Size constraints forced an early start move in
> - * pci_check_and_align_resource()?
> - */
> - if (!IS_ALIGNED(res->start, align))
> - return res->start;
> + if (rr->user_align &&
> + !IS_ALIGNED(candidate[i].start, rr->user_align)) {
> + pci_dbg(dev, "%pR: candidate %u skipped due to resource_alignment=%llx\n",
> + &candidate[i], i,
> + (unsigned long long)rr->user_align);
> + continue;
> + }
[Severity: Medium]
Can removing the early return on rr->user_align trigger the WARN_ON_ONCE() in
pci_resreq_check() when rr->user_align is set on a device that also has a
composite resource with a non-zero remainder?
In __pci_assign_resource(), min_align (align) is only lowered when
!rr.user_align:
drivers/pci/setup-res.c:__pci_assign_resource() {
if (!rr.user_align &&
(res->flags & IORESOURCE_MEM) && pci_resreq_remainder(size, align)) {
align = 1U;
if (pci_resource_is_bridge_win(resno))
align = pci_min_window_alignment(bus, res->flags);
}
}
If pci=resource_alignment=... matches a bridge or SR-IOV PF device with
rr->user_align smaller than rr->real_align (for example, rr->user_align = 4KB
while assigning a bridge window or VF BAR with real_align = 2MB, size = 3MB,
and remainder = 1MB), __pci_assign_resource() leaves min_align at 2MB.
In pci_align_resource(), candidate[1] and candidate[3] have 1MB-aligned start
addresses, which pass IS_ALIGNED(candidate[i].start, rr->user_align), so
pci_align_resource() can return a 1MB-aligned start address to
pci_resreq_check():
drivers/pci/setup-res.c:pci_resreq_check() {
start = pcibios_align_resource(rr, &tmp, empty_res, size, align);
WARN_ON_ONCE(!IS_ALIGNED(start, min_align));
}
With start aligned to 1MB and min_align equal to 2MB, won't this trigger
WARN_ON_ONCE(!IS_ALIGNED(start, min_align))?
>
> - /* Don't mess with size that doesn't align with window size granularity */
> - if (!IS_ALIGNED(remainder, pci_min_window_alignment(dev->bus, res->flags)))
> - return res->start;
[Severity: Medium]
Is it safe to remove the pci_min_window_alignment() check on remainder without
checking candidate alignment for bridge windows?
In drivers/pci/setup-bus.c:pci_bus_distribute_available_resources(),
remove_dev_resources() advances available[i].start by non-bridge BAR sizes on
the bus (such as a 4KB BAR 0 on a PCIe port). When available[i] is split
across downstream bridges via resource_set_size(), available[i].end becomes
unaligned by that sub-1MB offset, and the recursive call for the child bus
aligns available[i].start up before calling adjust_bridge_window():
drivers/pci/setup-bus.c:pci_bus_distribute_available_resources() {
align = pci_resource_alignment(bridge, res);
if (!resource_assigned(res) && align)
available[i].start = min(ALIGN(available[i].start, align),
available[i].end + 1);
...
adjust_bridge_window(bridge, res, add_list,
resource_size(&available[i]));
}
This can leave a child bridge window with a size whose remainder is not a
multiple of pci_min_window_alignment(dev->bus, res->flags) (1MB).
When __pci_assign_resource() assigns that bridge window, it sets min_align to
pci_min_window_alignment(bus, res->flags). In pci_align_resource(),
candidate[1] (ALIGN(empty_res->start, align) - remainder) and candidate[3]
(ALIGN_DOWN(empty_res->end + 1, align) - size) will have start addresses that
are not aligned to pci_min_window_alignment(). If candidate[1] or
candidate[3] is selected, pci_resreq_check() will trigger
WARN_ON_ONCE(!IS_ALIGNED(start, min_align)) and assign a bridge window whose
base address is not 1MB-aligned.
> - /* Try to place remainder that doesn't fill align before */
> - if (res->start < remainder)
> - return res->start;
> - start_addr = res->start - remainder;
> - if (empty_res->start > start_addr)
> + remaining.flags = type;
> + if (i <= 1) {
> + remaining.start = candidate[i].end + 1;
> + remaining.end = empty_res->end;
> + } else {
> + remaining.start = empty_res->start;
> + remaining.end = candidate[i].start - 1;
> + }
> +
> + natural_size = pci_max_natural_size(&remaining, &maxalign);
> + size = resource_size(&remaining);
> + pci_dbg(dev, "%pR: candidate %u free space naturalsize=%llx size=%llx maxalign=%llx\n",
> + &candidate[i], i,
> + (unsigned long long)natural_size,
> + (unsigned long long)size,
> + (unsigned long long)maxalign);
> + if ((best < 0) ||
> + (natural_size > best_natural_size) ||
> + (natural_size == best_natural_size && size > best_size) ||
> + (natural_size == best_natural_size && size == best_size &&
> + maxalign > best_maxalign)) {
> + best = i;
> + best_natural_size = natural_size;
> + best_size = size;
> + best_maxalign = maxalign;
> + }
> + }
> +
> + /* None fits? Return some address and let the caller deal with it. */
> + if (best == -1)
> return res->start;
[Severity: Medium]
If all candidates in empty_res are rejected because none of them satisfies
IS_ALIGNED(candidate[i].start, rr->user_align), won't returning res->start
here cause the caller to accept an unaligned allocation?
When pci_resreq_check() calls pcibios_align_resource() -> pci_align_resource(),
res is tmp, which has already been aligned to align inside empty_res. If
empty_res is large enough to contain [res->start, res->start + size - 1],
returning res->start hands back a range that lies completely within
empty_res (full_avail).
In kernel/resource.c:__find_resource_space(), the caller only checks whether
the returned range fits inside full_avail:
kernel/resource.c:__find_resource_space() {
alloc.start = alignf(alignf_data, &avail, size, align);
alloc.end = alloc.start + size - 1;
if (alloc.start <= alloc.end &&
__resource_contains_unbound(&full_avail, &alloc)) {
new->start = alloc.start;
new->end = alloc.end;
return 0;
}
}
Because [res->start, res->start + size - 1] is contained in full_avail, won't
__find_resource_space() succeed and allocate the resource at res->start in
violation of rr->user_align instead of skipping empty_res?
>
> - pci_dbg(dev, "%pR: moving candidate start address below align to %llx\n",
> - res, (unsigned long long)start_addr);
> - return start_addr;
> + pci_dbg(dev, "%pR: candidate %u selected (free space %pR), size: %llx + %llx, align: %llx\n",
> + &candidate[best], best, empty_res,
> + (unsigned long long)aligning,
> + (unsigned long long)remainder,
> + (unsigned long long)align);
> +
> + return candidate[best].start;
> }
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261021124956.4250-1-ilpo.jarvinen@linux.intel.com?part=6
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v4 7/7] PCI: Fix composite resource sizing
2026-10-21 12:49 [PATCH v4 0/7] PCI: Resource placement algorithm fixe Ilpo Järvinen
` (5 preceding siblings ...)
2026-10-21 12:49 ` [PATCH v4 6/7] PCI: Place resources to either edge of the window Ilpo Järvinen
@ 2026-10-21 12:49 ` Ilpo Järvinen
2026-10-21 13:25 ` sashiko-bot
6 siblings, 1 reply; 15+ messages in thread
From: Ilpo Järvinen @ 2026-10-21 12:49 UTC (permalink / raw)
To: Maciej Grochowski, Nikolas Joshua Britton, Geramy Loveless,
Eric Auger, Alexey Fomenko, Bjorn Helgaas, linux-pci,
Lorenzo Pieralisi, Rob Herring, Krzysztof Wilczyński,
Ilpo Järvinen, linux-kernel
Cc: Bradley Morgan
The bridge window sizing algorithm aims to pack child resources back to
back. With multiple composite child resources whose sizes do not align
to the calculated minimal alignment for the bridge window, back-to-back
placement may not be possible. The non-aligning remainder placement is
limited because it must be adjacent to the rest of the composite
resource.
Effectively, two remainder parts may be placed into the same align
sized block, but sum of their size might not match align. In such case,
a gap is required to meet the alignment requirement of both resources.
Add bridge window gap size calculator. Basic rules:
1) Gaps are only necessary if there is more than one non-aligning
composite resource within a single bridge window.
2) If there are only two remainder parts that amount to less than align
together, the required gap is the difference of align and the sum of
remainder sizes.
3) On other cases, round each remainder part to align to get the gap
size. Sometimes, smaller size may be possible but due to how sizing
and assignment are made in different phases, it is not always
possible to predict where each resource is assigned. Thus, the
sizing has to play safe.
The gap is calculated based on the minimal alignment for the bridge
window, which may be different for the case with only required
resources and the case with optional resources.
Fixes: 9036bd0efcb6 ("PCI: Align head space better")
Reported-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Bjorn Helgaas <bhelgaas@google.com>
Reported-by: Nikolas Joshua Britton <nbritton@exabit.io>
Tested-by: Nikolas Joshua Britton <nbritton@exabit.io>
Link: https://lore.kernel.org/linux-pci/20260903063124.9316-1-nbritton@exabit.io/
Reported-by: Maciej Grochowski <Maciej.Grochowski@sony.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 84 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 81 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 7ca0e9f4ffb6..2980b499ef75 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1164,6 +1164,79 @@ static inline resource_size_t calculate_mem_align(resource_size_t *aligns,
return min_align;
}
+/*
+ * Bridge window gap size calculator.
+ *
+ * Calculates gap (empty space) necessary because of non-aligning composite
+ * resources (VF BARs, bridge windows).
+ *
+ * Rules:
+ *
+ * 1) Gaps are only necessary if there is more than one non-aligning
+ * composite resource within a single bridge window.
+ *
+ * 2) If there are only two remainder parts that amount to less than win_align
+ * together, the required gap is the difference of win_align and the sum of
+ * remainder sizes.
+ *
+ * 3) On other cases, round each remainder part to win_align to get the gap
+ * size. Sometimes, tighter packing might be possible but due to how
+ * sizing and assignment are made in different phases, it is not always
+ * possible to predict where each resource is assigned. Thus, the sizing
+ * has to play safe even if it may overestimate in some cases.
+ */
+static resource_size_t calculate_win_gap_size(struct pci_bus *bus,
+ struct resource *b_res,
+ resource_size_t win_align,
+ bool optional,
+ struct list_head *realloc_head)
+{
+ resource_size_t safe_gap = 0, remainders = 0;
+ unsigned int nonaligning = 0;
+ struct pci_dev *dev;
+
+ list_for_each_entry(dev, &bus->devices, bus_list) {
+ struct resource *r;
+ int i;
+
+ pci_dev_for_each_resource(dev, r, i) {
+ resource_size_t r_size, remainder, aligning;
+
+ if (!pdev_resources_assignable(dev) ||
+ !pdev_resource_should_fit(dev, r))
+ continue;
+ if (b_res != pbus_select_window(bus, r))
+ continue;
+
+ if (!optional && pci_resource_is_optional(dev, i))
+ continue;
+
+ r_size = resource_size(r);
+ if (realloc_head && optional)
+ r_size += get_res_add_size(realloc_head, r);
+ if (r_size <= win_align)
+ continue;
+
+ aligning = ALIGN_DOWN(r_size, win_align);
+ remainder = r_size - aligning;
+ if (!remainder)
+ continue;
+
+ nonaligning++;
+ remainders += remainder;
+ safe_gap += win_align - remainder;
+ }
+ }
+
+ if (nonaligning == 2 && (remainders <= win_align))
+ return win_align - remainders;
+
+ if (nonaligning >= 2)
+ return safe_gap;
+
+ return 0;
+}
+
/*
* Calculate bridge window head alignment that leaves no gaps in between
* resources.
@@ -1281,6 +1354,7 @@ static void pbus_size_mem(struct pci_bus *bus, struct resource *b_res,
int order, max_order;
resource_size_t children_add_size = 0;
resource_size_t add_align = 0;
+ resource_size_t gap_size;
if (!b_res)
return;
@@ -1345,7 +1419,9 @@ static void pbus_size_mem(struct pci_bus *bus, struct resource *b_res,
win_align = pci_min_window_alignment(bus, b_res->flags);
min_align = calculate_head_align(aligns, max_order);
min_align = max(min_align, win_align);
- size0 = calculate_memsize(size, realloc_head ? 0 : add_size,
+ gap_size = calculate_win_gap_size(bus, b_res, min_align, !realloc_head,
+ realloc_head);
+ size0 = calculate_memsize(size + gap_size, realloc_head ? 0 : add_size,
0, win_align);
if (size0) {
@@ -1355,8 +1431,10 @@ static void pbus_size_mem(struct pci_bus *bus, struct resource *b_res,
if (realloc_head && (add_size > 0 || children_add_size > 0)) {
add_align = max(min_align, add_align);
- size1 = calculate_memsize(size, add_size, children_add_size,
- win_align);
+ gap_size = calculate_win_gap_size(bus, b_res, add_align, true,
+ realloc_head);
+ size1 = calculate_memsize(size + gap_size, add_size,
+ children_add_size, win_align);
}
if (!size0 && !size1) {
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v4 7/7] PCI: Fix composite resource sizing
2026-10-21 12:49 ` [PATCH v4 7/7] PCI: Fix composite resource sizing Ilpo Järvinen
@ 2026-10-21 13:25 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-10-21 13:25 UTC (permalink / raw)
To: Ilpo Järvinen, linux-pci
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] PCI: missing bridge window gap in calculate_win_gap_size() when r_align != win_align
- [Medium] PCI: missing bridge window gap in calculate_win_gap_size() for single remainder
- [Medium] PCI: calculate_win_gap_size() underestimates gap with small resources
--
commit b396fbe58c9de8a971b93378e647d16638aa6dbe
Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
PCI: Fix composite resource sizing
This commit adds calculate_win_gap_size() to pbus_size_mem() to account for
alignment gaps required when packing multiple non-aligning composite child
resources into a PCI bridge window.
Link: https://lore.kernel.org/linux-pci/20260903063124.9316-1-nbritton@exabit.io/
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 7ca0e9f4ffb63..2980b499ef759 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1164,6 +1164,79 @@ static inline resource_size_t calculate_mem_align(resource_size_t *aligns,
> return min_align;
> }
>
> +/*
> + * Bridge window gap size calculator.
> + *
> + * Calculates gap (empty space) necessary because of non-aligning composite
> + * resources (VF BARs, bridge windows).
> + *
> + * Rules:
> + *
> + * 1) Gaps are only necessary if there is more than one non-aligning
> + * composite resource within a single bridge window.
> + *
> + * 2) If there are only two remainder parts that amount to less than win_align
> + * together, the required gap is the difference of win_align and the sum of
> + * remainder sizes.
> + *
> + * 3) On other cases, round each remainder part to win_align to get the gap
> + * size. Sometimes, tighter packing might be possible but due to how
> + * sizing and assignment are made in different phases, it is not always
> + * possible to predict where each resource is assigned. Thus, the sizing
> + * has to play safe even if it may overestimate in some cases.
> + */
> +static resource_size_t calculate_win_gap_size(struct pci_bus *bus,
> + struct resource *b_res,
> + resource_size_t win_align,
> + bool optional,
> + struct list_head *realloc_head)
> +{
> + resource_size_t safe_gap = 0, remainders = 0;
> + unsigned int nonaligning = 0;
> + struct pci_dev *dev;
> +
> + list_for_each_entry(dev, &bus->devices, bus_list) {
> + struct resource *r;
> + int i;
> +
> + pci_dev_for_each_resource(dev, r, i) {
> + resource_size_t r_size, remainder, aligning;
> +
> + if (!pdev_resources_assignable(dev) ||
> + !pdev_resource_should_fit(dev, r))
> + continue;
> + if (b_res != pbus_select_window(bus, r))
> + continue;
> +
> + if (!optional && pci_resource_is_optional(dev, i))
> + continue;
> +
> + r_size = resource_size(r);
> + if (realloc_head && optional)
> + r_size += get_res_add_size(realloc_head, r);
> + if (r_size <= win_align)
> + continue;
> +
> + aligning = ALIGN_DOWN(r_size, win_align);
> + remainder = r_size - aligning;
> + if (!remainder)
> + continue;
[Severity: High]
Should calculate_win_gap_size() check and split each child resource against
its own required alignment pci_resource_alignment(dev, r) rather than the
parent bridge window's win_align?
During resource assignment in __pci_bus_assign_resources() ->
pbus_assign_resources_sorted() -> pci_assign_resource() ->
pci_align_resource(), each child resource r is aligned and split into an
aligning part and an adjacent remainder using its own alignment r_align =
pci_resource_alignment(dev, r).
However, during sizing in __pci_bus_size_bridges() -> pbus_size_mem(),
resources with r_size > align are excluded from aligns[order] while still
updating max_order, and pbus_size_mem() then passes min_align (for size0) or
add_align (for size1) as win_align to calculate_win_gap_size():
drivers/pci/setup-bus.c:pbus_size_mem() {
...
if (r_size <= align)
aligns[order] += align;
if (order > max_order)
max_order = order;
}
}
win_align = pci_min_window_alignment(bus, b_res->flags);
min_align = calculate_head_align(aligns, max_order);
min_align = max(min_align, win_align);
gap_size = calculate_win_gap_size(bus, b_res, min_align, !realloc_head,
realloc_head);
...
if (realloc_head && (add_size > 0 || children_add_size > 0)) {
add_align = max(min_align, add_align);
gap_size = calculate_win_gap_size(bus, b_res, add_align, true,
realloc_head);
}
When a child resource's r_align differs from win_align, this can omit the
required gap in three ways and cause pci_assign_resource() to fail with
-ENOSPC:
1. r_align < r_size <= win_align: Suppose a bus has a mandatory 64M BAR
(r_size = 64M, r_align = 64M) and two mandatory child bridge windows each
with r_size = 20M and r_align = 16M. pbus_size_mem() computes size = 104M
and min_align = 64M. In calculate_win_gap_size(), both 20M composite windows
are skipped by:
if (r_size <= win_align)
continue;
because 20M <= 64M, returning gap_size = 0 and sizing b_res to 104M instead
of the required 112M (64M + 20M + 20M + 8M gap). During assignment in a
104M window, the 64M BAR consumes the 64M-aligned region and leaves a 40M
region; placing the first 20M@16M window leaves no 16M-aligned block with
adjacent room for the second 20M@16M window.
2. min_align < r_size <= add_align when sizing optional resources for size1:
Suppose a bus has two mandatory 20M@16M child bridge windows (or three
18M@16M windows) plus an optional resource (such as a 32M SR-IOV BAR) that
raises add_align to 32M. The first call with min_align = 16M includes the
required 8M (or 42M) gap in size0, but the second call with add_align = 32M
skips the mandatory 20M (or 18M) windows because r_size <= 32M, dropping
their required gap from size1.
3. win_align < r_align when calculate_head_align() reduces min_align:
Suppose a bus has two mandatory composite windows of r_size = 80M (or 96M)
with r_align = 64M and one 32M BAR (r_size = 32M, r_align = 32M). Because
the composite windows have r_size > 64M, aligns[6] remains 0 while
max_order = 6 and aligns[5] = 32M, so calculate_head_align() reduces
min_align from 64M to 32M. Passing min_align = 32M as win_align computes
remainders modulo 32M (16M + 16M = 32M -> win_align - remainders = 0 for
80M, or remainder = 0 for 96M), returning gap_size = 0 even though two
64M-aligned composite resources and the 32M BAR cannot fit into the
resulting 192M window.
> +
> + nonaligning++;
> + remainders += remainder;
> + safe_gap += win_align - remainder;
> + }
> + }
> +
> + if (nonaligning == 2 && (remainders <= win_align))
> + return win_align - remainders;
[Severity: Medium]
Can returning win_align - remainders here underestimate the required gap
when smaller resources with r_size <= win_align are also present in the same
bridge window?
When a bridge window contains two non-aligning composite resources whose
remainders r1 and r2 sum to <= win_align plus one or more smaller resources
(skipped by r_size <= win_align) whose total size S is less than
min_align / 2, calculate_head_align() leaves min_align unchanged and Rule 2
sizes the bridge window to A1 + A2 + win_align + S. This assumes both
remainders r1 and r2 will share a single win_align block and leave a
contiguous S-sized region for the smaller resources.
However, during pbus_assign_resources_sorted() -> pci_assign_resource() ->
pci_align_resource(), candidate[2] in drivers/pci/setup-res.c evaluates
placing the first composite resource's remainder at the right end of the
window:
drivers/pci/setup-res.c:pci_align_resource() {
...
candidate[2] = DEFINE_RES(ALIGN_DOWN(empty_res->end + 1 - remainder, align) -
aligning, size, type);
}
When r1 <= S and r2 + S > win_align, candidate[2] places the first composite
resource's remainder r1 into the trailing S-sized tail because doing so
maximizes the power-of-two natural_size of the remaining free space on the
left. Once the second composite resource is placed, the space intended for S
is split into two fragments (S - r1 at the right tail and win_align - r2 on
the left), neither of which is large enough to fit the smaller resource,
causing its assignment to fail with -EBUSY.
> +
> + if (nonaligning >= 2)
> + return safe_gap;
> +
> + return 0;
> +}
[Severity: Medium]
Is an alignment gap also needed when nonaligning == 1 and the single
composite resource's remainder is not aligned to a smaller sibling
resource's alignment?
Consider a bus with one composite child bridge window r1 of size 84M
(64M + 16M + 4M, align = 64M) and one normal sibling BAR r2 of size 16M
(align = 16M). In pbus_size_mem(), r1 has r_size > align so aligns[6] is 0
with max_order = 6, while r2 sets aligns[4] = 16M. Because 16M < 64M / 2,
calculate_head_align() returns min_align = 64M.
In calculate_win_gap_size(), r1 has remainder = 20M (nonaligning = 1) and r2
is skipped because 16M <= 64M. Because nonaligning < 2, it returns 0 and
pbus_size_mem() sizes b_res to 84M + 16M = 100M with min_align = 64M.
When b_res (size 100M, align 64M, aligning = 64M, remainder = 36M) is
assigned by pci_align_resource():
drivers/pci/setup-res.c:pci_align_resource() {
...
candidate[0] = DEFINE_RES(ALIGN(empty_res->start, align), size, type);
candidate[1] = DEFINE_RES(ALIGN(empty_res->start, align) - remainder,
size, type);
candidate[2] = DEFINE_RES(ALIGN_DOWN(empty_res->end + 1 - remainder, align) -
aligning, size, type);
candidate[3] = DEFINE_RES(ALIGN_DOWN(empty_res->end + 1, align) - size,
size, type);
}
every candidate places b_res at either [K, K + 100M) or [K - 36M, K + 64M)
for some 64M-aligned address K, so b_res contains only a single 64M-aligned
block [K, K + 64M).
During pbus_assign_resources_sorted(), r1 (align 64M) is assigned before r2
(align 16M), consuming [K, K + 64M) and placing its adjacent 20M remainder
at [K + 64M, K + 84M) (or [K - 20M, K)). That leaves only [K + 84M, K + 100M)
(or [K - 36M, K - 20M)) for r2; both 16M intervals are shifted by 20M (only
4M-aligned) and straddle a 16M boundary, so __find_resource_space() fails to
assign r2 with -EBUSY.
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261021124956.4250-1-ilpo.jarvinen@linux.intel.com?part=7
^ permalink raw reply [flat|nested] 15+ messages in thread