[PATCH v2 0/7] PCI: Resource placement algorithm fixes

[PATCH v2 0/7] PCI: Resource placement algorithm fixes

[Ilpo Järvinen]

Hi,

This series generalizes resource placement algorithm. The commit
9036bd0efcb6 ("PCI: Align head space better") special cased one
composite resource remainder case to fix a regression (in a long chain
of regression fixes). Unfortunately, it introduced another regression
(or a few, to be more accurate).

Instead of building more special tricks, generalize the PCI resource
placement algorithm so it returns placements that gives better chances
for the greedy assignment algorithm to find suitable space for
subsequent assignments.

In short words, the new approach tries to find a placement for the
current resource that blocks as little of the free space range as
possible, considering both alignment and continuous free span of the
remaining free space.

In addition, the series corrects composite resource sizing to account
for gaps that have to be added due to alignment constaints and
remainder space not fully connecting (filling space all the way to
the bridge window align).

More detailed explanations in the patches themselves.

Unfortunately, this also causes yet another regression due to shuffling
resources around, even if the resulting arrangement seems just as valid
as any other. The workaround quirk is in the last patch.

I find it likely that after this series the resources are packed tight
enough to solve also
  https://bugzilla.kernel.org/show_bug.cgi?id=220016
from more than a year back but that remains to be seen (if we can still
get the reporter to test it).


I've another series coming up to improve pci=resource_alignment locking
as I realized the current approach allows parameter change to race with
the fitting and assignment algorithm.

v2:
- Add patch to cleanup parisc debug print (do dyndbg conversion)
- Add patch to prevent remainder tricks when alignment override is used
  (found by sashiko)
- Do not use PCIBIOS_MIN_IO/MEM but 1U as min align. The former seem
  address space lower bounds even if parisc arch code misleadingly
  uses them for ALIGN() call.
- Fix res->end + 1 overflow case (found by sashiko)
- Tweak debug print formatting
- Reworked the iomem black hole quirk (now based on the bridge)

Ilpo Järvinen (7):
  resource: Mark free space assigned
  PCI/parisc: Clean up resource debug print & use dynamic debug
  PCI: Honor alignment overrides
  PCI: Fix nesting windows with remainder at the left edge
  PCI: Place resources to either edge of the window
  PCI: Fix composite resource sizing
  PCI/quirks: Avoid certain address on Genoa systems

 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         |  20 +--
 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                |   5 +-
 drivers/pci/pci.h                |   4 +
 drivers/pci/quirks.c             |  35 +++++
 drivers/pci/setup-bus.c          |  83 ++++++++++-
 drivers/pci/setup-res.c          | 227 +++++++++++++++++++++++++++----
 include/linux/pci.h              |  27 +++-
 kernel/resource.c                |   1 +
 19 files changed, 385 insertions(+), 69 deletions(-)


base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.47.3

[PATCH v2 1/7] resource: Mark free space assigned

[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 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/resource.c b/kernel/resource.c
index e60539a55541..17e7fccd859c 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.
-- 
2.47.3

[PATCH v2 2/7] PCI/parisc: Clean up resource debug print & use dynamic debug

[Ilpo Järvinen]

Remove adhoc RES_DBG() and use pci_dbg() instead.

Reformat the message to the typical format used by PCI core. Print the
resources 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..518f532ccddc 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), parent %pR\n",
+		res, res->flags, (unsigned long long)size,
+		(unsigned long long)alignment, res->parent);
 
 	/* 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

[PATCH v2 3/7] PCI: Honor alignment overrides

[Ilpo Järvinen]

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.

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/
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 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                |  5 +++--
 drivers/pci/setup-res.c          | 19 +++++++++++++------
 include/linux/pci.h              | 26 ++++++++++++++++++++++++--
 15 files changed, 75 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 518f532ccddc..d582be996051 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), parent %pR\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..1b7a4469c6ca 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -6444,8 +6444,8 @@ 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();
@@ -6497,6 +6497,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..fe0233f0d9e6 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1217,12 +1217,34 @@ 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);
+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);
+}
+
 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

[PATCH v2 4/7] PCI: Fix nesting windows with remainder at the left edge

[Ilpo Järvinen]

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 fe0233f0d9e6..3594223233ac 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

[PATCH v2 5/7] PCI: Place resources to either edge of the window

[Ilpo Järvinen]

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 | 174 ++++++++++++++++++++++++++++++++++------
 3 files changed, 152 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..4c73bf6e0f70 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,89 @@ 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_two(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 <<= __ffs(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.
+ *
+ * When user has requested specific alignmnet, only candidates 1 & 3 can
+ * qualify. TODO: If the resulting alignment with the remainder at the
+ * left edge is enough to satisfy user's request, all candidates could be
+ * allowed.
  */
 resource_size_t pci_align_resource(struct pci_resreq_data *rr,
 				   const struct resource *res,
@@ -280,38 +356,84 @@ 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 && (i == 1 || i == 3)) {
+			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

[PATCH v2 6/7] PCI: Fix composite resource sizing

[Ilpo Järvinen]

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 | 79 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 76 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 7ca0e9f4ffb6..9d828a59bd00 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1164,6 +1164,76 @@ 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)
+{
+	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 (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 +1351,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 +1416,8 @@ 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, false);
+	size0 = calculate_memsize(size + gap_size, realloc_head ? 0 : add_size,
 				  0, win_align);
 
 	if (size0) {
@@ -1355,8 +1427,9 @@ 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);
+		size1 = calculate_memsize(size + gap_size, add_size,
+					  children_add_size, win_align);
 	}
 
 	if (!size0 && !size1) {
-- 
2.47.3

[PATCH v2 7/7] PCI/quirks: Avoid certain address on Genoa systems

[Ilpo Järvinen]

While testing the resource placement changes, tests hit a case where
igb fails to probe when BAR 0 is placed at 0x9c000000:

   90000000-9cffffff : PCI Bus 0000:a0
  -  90000000-902fffff : PCI Bus 0000:a1
  -    90000000-900fffff : 0000:a1:00.0
  -      90000000-900fffff : igb
  -    90100000-901fffff : 0000:a1:00.0
  -    90200000-90203fff : 0000:a1:00.0
  -      90200000-90203fff : igb
  +  9be00000-9c0fffff : PCI Bus 0000:a1
  +    9be00000-9befffff : 0000:a1:00.0
  +    9bf00000-9bf03fff : 0000:a1:00.0
  +    9c000000-9c0fffff : 0000:a1:00.0
     9c100000-9c17ffff : amd_iommu
     9c180000-9c1803ff : IOAPIC 8

  -       Region 0: Memory at 90000000 (32-bit, non-prefetchable) [size=1M]
  -       Region 3: Memory at 90200000 (32-bit, non-prefetchable) [size=16K]
  -       Expansion ROM at 90100000 [disabled] [size=1M]
  +       Region 0: Memory at 9c000000 (32-bit, non-prefetchable) [size=1M]
  +       Region 3: Memory at 9bf00000 (32-bit, non-prefetchable) [size=16K]
  +       Expansion ROM at 9be00000 [disabled] [size=1M]

igb 0000:a1:00.0 0000:a1:00.0 (uninitialized): PCIe link lost
------------[ cut here ]------------
igb: Failed to read reg 0x18!
WARNING: drivers/net/ethernet/intel/igb/igb_main.c:724 at igb_rd32.cold+0x3c/0x4f [igb], CPU#32: kworker/32:1/706
...
 igb_get_invariants_82575+0xff/0xf00 [igb]
 igb_probe+0x3c8/0x1190 [igb]
 local_pci_probe+0x3b/0x80

It turns out there is a 64kB iomem black hole at 9c000000 that returns
~0 and this is where igb's BAR 0 resides. If another BAR of the same
card is placed into that address, it is similarly black holed.

Mark the problematic 64kB range reserved using a quirk bound to the
bridge found in the problematic system.

Closes: https://bugzilla.kernel.org/show_bug.cgi?id=222074
Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---

Mario suggested the quirk to be based on the bridge instead of the 
endpoint device which certainly looks better and cleaner than the
approach used in v1.

The current plan is to try a different card in the same slot but it is
a bit hard for me to predictable on what timescale that can be done.

---
 drivers/pci/quirks.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index de9bbccda21f..5483b47d8d54 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -6288,6 +6288,41 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1536, rom_bar_overlap_defect);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1537, rom_bar_overlap_defect);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1538, rom_bar_overlap_defect);
 
+/*
+ * TODO: Remove when/if root cause is found.
+ *
+ * Genoa appears to have a 64kB iomem black hole starting at 0x9c000000
+ * address for which all reads return ~0 blocking an overlapping BAR from
+ * working.
+ *
+ * Work around the problem by reserving the space prior to making any iomem
+ * allocations that could overlap with the black hole.
+ */
+static struct resource black_hole_res = DEFINE_RES_MEM_NAMED(0x9c000000, SZ_64K,
+							     "reserved");
+
+static void genoa_iomem_black_hole(struct pci_dev *dev)
+{
+	struct resource *r;
+
+	pci_bus_for_each_resource(dev->bus, r) {
+		if (!r || !r->flags || !resource_assigned(r))
+			continue;
+
+		if (!__resource_contains_unbound(r, &black_hole_res))
+			continue;
+
+		if (resource_assigned(&black_hole_res))
+			pci_dbg(dev, "iomem black hole workaround already applied\n");
+		else if (!insert_resource(r, &black_hole_res))
+			pci_info(dev, "iomem black hole workaround enabled\n");
+		else
+			pci_dbg(dev, "iomem black hole workaround add failed\n");
+		return;
+	}
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, 0x14ab, genoa_iomem_black_hole);
+
 #ifdef CONFIG_PCIEASPM
 /*
  * Several Intel DG2 graphics devices advertise that they can only tolerate
-- 
2.47.3

Re: [PATCH v2 4/7] PCI: Fix nesting windows with remainder at the left edge

[sashiko-bot]

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

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002113319.6652-1-ilpo.jarvinen@linux.intel.com?part=4

Re: [PATCH v2 2/7] PCI/parisc: Clean up resource debug print & use dynamic debug

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci: uninitialized stack pointer dereference in pcibios_align_resource() via %pR
--

commit e687046bde97f65f6b278cb7b13dd117863bc565
Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

PCI/parisc: Clean up resource debug print & use dynamic debug

This commit cleans up the ad-hoc RES_DBG() macro in the parisc PCI code
by replacing it with the standard pci_dbg() macro. It also updates the
format string to use %pR for printing resources, which matches the typical
format used by the PCI core.

> diff --git a/arch/parisc/kernel/pci.c b/arch/parisc/kernel/pci.c
> index b8007c7400d47..518f532ccddc5 100644
> --- a/arch/parisc/kernel/pci.c
> +++ b/arch/parisc/kernel/pci.c

[ ... ]

> @@ -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), parent %pR\n",
> +		res, res->flags, (unsigned long long)size,
> +		(unsigned long long)alignment, res->parent);

[Severity: High]
Could this format string introduce a regression by causing a kernel oops due
to an uninitialized pointer dereference?

The %pR format specifier causes the printk implementation to dereference the
resource pointer to read its boundaries and flags. However,
pcibios_align_resource() is used as an alignf callback for resource
allocation, and it can be called with a partially uninitialized stack
variable from __find_resource_space():

kernel/resource.c:__find_resource_space() {
	...
	struct resource avail;
	...
	avail.start = ALIGN(full_avail.start, constraint->align);
	avail.end = full_avail.end;
	avail.flags = new->flags;
	if (avail.start >= full_avail.start) {
		alloc.flags = avail.flags;
		if (alignf) {
			alloc.start = alignf(constraint->alignf_data,
					     &avail, &full_avail,
					     size, constraint->align);
	...
}

In this path, the avail.parent field is left uninitialized. When %pR attempts
to dereference res->parent, it will be reading stack garbage. The previous
DBG_RES() implementation used %p, which safely printed the pointer address
without dereferencing it.

Would it be safer to revert to %p for the parent pointer or ensure the
struct is fully initialized?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002113319.6652-1-ilpo.jarvinen@linux.intel.com?part=2

Re: [PATCH v2 1/7] resource: Mark free space assigned

[sashiko-bot]

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

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002113319.6652-1-ilpo.jarvinen@linux.intel.com?part=1

Re: [PATCH v2 5/7] PCI: Place resources to either edge of the window

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci: 64-bit truncation in pci_max_natural_size() on 32-bit archs
--

commit a3daecf300277b71086888c6fc930ab2932e9d1b
Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

PCI: Place resources to either edge of the window

This introduces a new greedy algorithm for PCI resource assignment that
is aware of window edges. It aims to avoid fragmenting the free space
when placing composite resources like bridge windows and VF BARs.

> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index 2ca3784aa323f..4c73bf6e0f706 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
[ ... ]
> @@ -262,16 +265,89 @@ 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_two(size);

[Severity: High]
Does pci_max_natural_size() in drivers/pci/setup-res.c silently truncate the
64-bit size to 32 bits on 32-bit architectures?

If CONFIG_PHYS_ADDR_T_64BIT=y is enabled on a 32-bit architecture and a
resource is >= 4GB, rounddown_pow_of_two() will receive a 0-truncated value
since it expects an unsigned long. Can this result in a shift by -1
(undefined behavior) and cause incorrect BAR size and alignment calculations?

> +	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 <<= __ffs(natural_start);

[Severity: High]
Can pci_max_natural_size() in drivers/pci/setup-res.c invoke undefined
behavior on 32-bit architectures if the start address is aligned to a 4GB
boundary?

The __ffs() function expects an unsigned long. If the 64-bit natural_start
has its lower 32 bits as zero, this implicitly casts to a 32-bit zero, which
triggers undefined behavior in __builtin_ctzl(0).

> +	} 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;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002113319.6652-1-ilpo.jarvinen@linux.intel.com?part=5

Re: [PATCH v2 3/7] PCI: Honor alignment overrides

[Jani Nikula]

On Fri, 02 Oct 2026, Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> 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.
>
> 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/
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

So I don't have the time to figure out what's going on here, but
assuming you get proper review, the i915 part is

Acked-by: Jani Nikula <jani.nikula@intel.com>

for merging via whichever tree makes sense.

> ---
>  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                |  5 +++--
>  drivers/pci/setup-res.c          | 19 +++++++++++++------
>  include/linux/pci.h              | 26 ++++++++++++++++++++++++--
>  15 files changed, 75 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 518f532ccddc..d582be996051 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), parent %pR\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..1b7a4469c6ca 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -6444,8 +6444,8 @@ 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();
> @@ -6497,6 +6497,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..fe0233f0d9e6 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1217,12 +1217,34 @@ 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);
> +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);
> +}
> +
>  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,

-- 
Jani Nikula, Intel

Re: [PATCH v2 7/7] PCI/quirks: Avoid certain address on Genoa systems

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci/quirks: skipped workaround and UAF on rescan due to dangling resource pointer
--

commit 988b9fe1c0d1202ac7629ed0f5250a0381f61b90
Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

PCI/quirks: Avoid certain address on Genoa systems

This commit addresses a 64kB iomem black hole on Genoa systems that causes
reads to return ~0 and blocks overlapping BARs from working. It reserves the
problematic space via a bridge quirk.

Closes: https://bugzilla.kernel.org/show_bug.cgi?id=222074

> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index de9bbccda21fd..5483b47d8d544 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c

[ ... ]

> +static struct resource black_hole_res = DEFINE_RES_MEM_NAMED(0x9c000000, SZ_64K,
> +							     "reserved");
> +
> +static void genoa_iomem_black_hole(struct pci_dev *dev)
> +{
> +	struct resource *r;
> +
> +	pci_bus_for_each_resource(dev->bus, r) {
> +		if (!r || !r->flags || !resource_assigned(r))
> +			continue;
> +
> +		if (!__resource_contains_unbound(r, &black_hole_res))
> +			continue;
> +
> +		if (resource_assigned(&black_hole_res))
> +			pci_dbg(dev, "iomem black hole workaround already applied\n");

[Severity: High]
Can this lead to a use-after-free and cause the workaround to be skipped on
rescan?

During the first probe, insert_resource() sets black_hole_res.parent to the
dynamically allocated host bridge window. If the PCI bridge is subsequently
removed, the bridge's resources are released and freed.

However, release_resource() unlinks the parent but does not clear the parent
pointers of child resources. Because black_hole_res is statically allocated,
its parent pointer is left dangling.

When the bus is rescanned, resource_assigned(&black_hole_res) evaluates to
true due to the dangling parent pointer. The code erroneously logs "already
applied" and skips inserting the reservation into the new bridge window.

This leaves the new window unprotected, allowing subsequent BAR allocations
to fall into the 0x9c000000 black hole.

> +		else if (!insert_resource(r, &black_hole_res))
> +			pci_info(dev, "iomem black hole workaround enabled\n");
> +		else
> +			pci_dbg(dev, "iomem black hole workaround add failed\n");
> +		return;
> +	}
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, 0x14ab, genoa_iomem_black_hole);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002113319.6652-1-ilpo.jarvinen@linux.intel.com?part=7

Re: [PATCH v2 3/7] PCI: Honor alignment overrides

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pci: pci_init_pci_resreq_data() confuses default alignment with user override
--

commit 9d8429783cc0cbfe47ed3509438a7591dc940ce5
Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

PCI: Honor alignment overrides

This commit introduces struct pci_resreq_data to hold device and user-given
alignment information so that pci_align_resource() can honor the
pci=resource_alignment argument. When a user-given alignment is found, any
remainder movement is skipped to avoid violating the requested alignment.

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;
> +
[Severity: Medium]
Does this inadvertently skip remainder movement for all devices on platforms
with non-zero default alignments?

On platforms where pcibios_default_alignment() returns a non-zero value
(such as PowerPC), rr->user_align might be populated with that default
rather than specifically a user-requested override. If this condition
evaluates as true for those defaults, it appears it would entirely disable
the remainder movement logic and limit PCI resource packing efficiency across
the platform.

Note that follow-up modifications to this condition (such as checking if the
index is 1 or 3) would still incorrectly trigger based on a default alignment
if the distinction between user overrides and defaults is lost.

[ ... ]

> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index d31a8d107b1ef..fe0233f0d9e6e 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1217,12 +1217,34 @@ 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);
> +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);
> +}
> +
[Severity: Medium]
Does this initialize rr->user_align with the returned alignment size while
ignoring the boolean flag that indicates if it was actually requested by
the user?

Looking at pci_specified_resource_alignment() in drivers/pci/pci.c, it
falls back to pcibios_default_alignment() if no user override matches. The
user_aligned flag is set to true only when a specific override is found.
Because the return value is assigned directly to rr->user_align, it seems
struct pci_resreq_data cannot differentiate between a default alignment and an
actual user override.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002113319.6652-1-ilpo.jarvinen@linux.intel.com?part=3

Re: [PATCH v2 6/7] PCI: Fix composite resource sizing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] pci: undersized bridge windows during hotplug due to ignored optional resources
- [High] pci: gap calculation ignores expanded sizes, undersizing optional bridge windows
- [High] pci: gap optimization incorrectly assumes perfect packing, causing allocation failures
--

commit 17592808ea429ea63c8337b7e33d67bb459cf0c2
Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Subject: PCI: Fix composite resource sizing

This commit adds a bridge window gap size calculator to correctly pad
alignments for non-aligning composite resources back to back. The gap is
calculated based on the minimal alignment for the 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..9d828a59bd002 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
[ ... ]
> +static resource_size_t calculate_win_gap_size(struct pci_bus *bus,
> +					      struct resource *b_res,
> +					      resource_size_t win_align,
> +					      bool optional)
> +{
> +	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);

[Severity: High]
During the optional sizing pass, calculate_win_gap_size() is called with
optional=true, but the gap requirement is evaluated using the unexpanded base
size from resource_size(r). It does not appear to consider the requested
add_size expansions from realloc_head.

If a child bridge's base size perfectly aligns to the parent's add_align
(resulting in a 0 gap here), but the child's expanded size is unaligned, won't
this result in an undersized parent optional window because it failed to
provision the padding gap for the expanded size?

> +			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;

[Severity: High]
Does this optimization assume perfect packing of fragmented remainders,
violating the strict alignment constraints of the resources themselves?

If both resources require win_align alignment (e.g., both have size 5MB and
alignment 4MB), the second resource cannot be placed adjacent to the first
and must be bumped to the next 4MB boundary. Wouldn't this reduction in the
gap cause the second resource to fail assignment because the bridge window
is undersized?

> +
> +	if (nonaligning >= 2)
> +		return safe_gap;
> +
> +	return 0;
> +}
[ ... ]
> @@ -1345,7 +1416,8 @@ 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, false);

[Severity: High]
When a device with optional resources (like SR-IOV BARs) is sized via a path
where realloc_head is NULL (e.g., during hotplug via pci_bus_size_bridges()),
pbus_size_mem() accumulates the sizes of those optional resources into the
base size because pbus_size_mem_optional() returns false.

However, the gap calculator is invoked here with optional=false, which
unconditionally drops optional resources. Doesn't this cause their required
alignment padding gaps to be omitted from the bridge window entirely, leaving
the bridge window undersized?

> +	size0 = calculate_memsize(size + gap_size, realloc_head ? 0 : add_size,
>  				  0, win_align);
>  
>  	if (size0) {
> @@ -1355,8 +1427,9 @@ 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);
> +		size1 = calculate_memsize(size + gap_size, add_size,
> +					  children_add_size, win_align);
>  	}
>  
>  	if (!size0 && !size1) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002113319.6652-1-ilpo.jarvinen@linux.intel.com?part=6

Re: [PATCH v2 7/7] PCI/quirks: Avoid certain address on Genoa systems

[Mario Limonciello]

+Boris

Please include him on the next version too.

On 10/2/26 06:33, Ilpo Järvinen wrote:
> While testing the resource placement changes, tests hit a case where
> igb fails to probe when BAR 0 is placed at 0x9c000000:
> 
>     90000000-9cffffff : PCI Bus 0000:a0
>    -  90000000-902fffff : PCI Bus 0000:a1
>    -    90000000-900fffff : 0000:a1:00.0
>    -      90000000-900fffff : igb
>    -    90100000-901fffff : 0000:a1:00.0
>    -    90200000-90203fff : 0000:a1:00.0
>    -      90200000-90203fff : igb
>    +  9be00000-9c0fffff : PCI Bus 0000:a1
>    +    9be00000-9befffff : 0000:a1:00.0
>    +    9bf00000-9bf03fff : 0000:a1:00.0
>    +    9c000000-9c0fffff : 0000:a1:00.0
>       9c100000-9c17ffff : amd_iommu
>       9c180000-9c1803ff : IOAPIC 8
> 
>    -       Region 0: Memory at 90000000 (32-bit, non-prefetchable) [size=1M]
>    -       Region 3: Memory at 90200000 (32-bit, non-prefetchable) [size=16K]
>    -       Expansion ROM at 90100000 [disabled] [size=1M]
>    +       Region 0: Memory at 9c000000 (32-bit, non-prefetchable) [size=1M]
>    +       Region 3: Memory at 9bf00000 (32-bit, non-prefetchable) [size=16K]
>    +       Expansion ROM at 9be00000 [disabled] [size=1M]
> 
> igb 0000:a1:00.0 0000:a1:00.0 (uninitialized): PCIe link lost
> ------------[ cut here ]------------
> igb: Failed to read reg 0x18!
> WARNING: drivers/net/ethernet/intel/igb/igb_main.c:724 at igb_rd32.cold+0x3c/0x4f [igb], CPU#32: kworker/32:1/706
> ...
>   igb_get_invariants_82575+0xff/0xf00 [igb]
>   igb_probe+0x3c8/0x1190 [igb]
>   local_pci_probe+0x3b/0x80
> 
> It turns out there is a 64kB iomem black hole at 9c000000 that returns
> ~0 and this is where igb's BAR 0 resides. If another BAR of the same
> card is placed into that address, it is similarly black holed.
> 
> Mark the problematic 64kB range reserved using a quirk bound to the
> bridge found in the problematic system.
> 
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=222074
> Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
> 
> Mario suggested the quirk to be based on the bridge instead of the
> endpoint device which certainly looks better and cleaner than the
> approach used in v1.
> 
> The current plan is to try a different card in the same slot but it is
> a bit hard for me to predictable on what timescale that can be done.
> 

FWIW - I do think this approach is pragmatic for the given situation.

> ---
>   drivers/pci/quirks.c | 35 +++++++++++++++++++++++++++++++++++

I think a better location will be arch/x86/pci/fixup.c as these quirks 
will only matter for x86.

>   1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index de9bbccda21f..5483b47d8d54 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -6288,6 +6288,41 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1536, rom_bar_overlap_defect);
>   DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1537, rom_bar_overlap_defect);
>   DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1538, rom_bar_overlap_defect);
>   
> +/*
> + * TODO: Remove when/if root cause is found.
> + *
> + * Genoa appears to have a 64kB iomem black hole starting at 0x9c000000
> + * address for which all reads return ~0 blocking an overlapping BAR from
> + * working.
> + *
> + * Work around the problem by reserving the space prior to making any iomem
> + * allocations that could overlap with the black hole.
> + */
> +static struct resource black_hole_res = DEFINE_RES_MEM_NAMED(0x9c000000, SZ_64K,
> +							     "reserved");

I'd give it more detail for anyone looking at reservations to get a hint 
back to this function/commit.  Maybe "genoa_black_hole"?

> +
> +static void genoa_iomem_black_hole(struct pci_dev *dev)
> +{
> +	struct resource *r;
> +
> +	pci_bus_for_each_resource(dev->bus, r) {
> +		if (!r || !r->flags || !resource_assigned(r))
> +			continue;
> +
> +		if (!__resource_contains_unbound(r, &black_hole_res))
> +			continue;
> +
> +		if (resource_assigned(&black_hole_res))
> +			pci_dbg(dev, "iomem black hole workaround already applied\n");
> +		else if (!insert_resource(r, &black_hole_res))
> +			pci_info(dev, "iomem black hole workaround enabled\n");
> +		else
> +			pci_dbg(dev, "iomem black hole workaround add failed\n");
> +		return;
> +	}
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, 0x14ab, genoa_iomem_black_hole);
> +
>   #ifdef CONFIG_PCIEASPM
>   /*
>    * Several Intel DG2 graphics devices advertise that they can only tolerate

Re: [PATCH v2 7/7] PCI/quirks: Avoid certain address on Genoa systems

[Ilpo Järvinen]

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

On Fri, 2 Oct 2026, sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] pci/quirks: skipped workaround and UAF on rescan due to dangling resource pointer
> --
> 
> commit 988b9fe1c0d1202ac7629ed0f5250a0381f61b90
> Author: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> 
> PCI/quirks: Avoid certain address on Genoa systems
> 
> This commit addresses a 64kB iomem black hole on Genoa systems that causes
> reads to return ~0 and blocks overlapping BARs from working. It reserves the
> problematic space via a bridge quirk.
> 
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=222074
> 
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index de9bbccda21fd..5483b47d8d544 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> 
> [ ... ]
> 
> > +static struct resource black_hole_res = DEFINE_RES_MEM_NAMED(0x9c000000, SZ_64K,
> > +							     "reserved");
> > +
> > +static void genoa_iomem_black_hole(struct pci_dev *dev)
> > +{
> > +	struct resource *r;
> > +
> > +	pci_bus_for_each_resource(dev->bus, r) {
> > +		if (!r || !r->flags || !resource_assigned(r))
> > +			continue;
> > +
> > +		if (!__resource_contains_unbound(r, &black_hole_res))
> > +			continue;
> > +
> > +		if (resource_assigned(&black_hole_res))
> > +			pci_dbg(dev, "iomem black hole workaround already applied\n");
> 
> [Severity: High]
> Can this lead to a use-after-free and cause the workaround to be skipped on
> rescan?
> 
> During the first probe, insert_resource() sets black_hole_res.parent to the
> dynamically allocated host bridge window. If the PCI bridge is subsequently
> removed, the bridge's resources are released and freed.
> 
> However, release_resource() unlinks the parent but does not clear the parent
> pointers of child resources. Because black_hole_res is statically allocated,
> its parent pointer is left dangling.
>
> When the bus is rescanned, resource_assigned(&black_hole_res) evaluates to
> true due to the dangling parent pointer. The code erroneously logs "already
> applied" and skips inserting the reservation into the new bridge window.
>
> This leaves the new window unprotected, allowing subsequent BAR allocations
> to fall into the 0x9c000000 black hole.

Before sending this, I did test removing the PCI devices all the way to 
the Root Complex (RC included) and the black hole marker weathered the 
storm just fine.

I suppose if I'd proceed removing something from further up (something 
ACPI side?), the scenario might actually occur but why would anyone want 
to do that?

I think this goes to the territory where root has many ways to shoot 
oneself's own foot...

-- 
 i.
 
> > +		else if (!insert_resource(r, &black_hole_res))
> > +			pci_info(dev, "iomem black hole workaround enabled\n");
> > +		else
> > +			pci_dbg(dev, "iomem black hole workaround add failed\n");
> > +		return;
> > +	}
> > +}
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, 0x14ab, genoa_iomem_black_hole);
> 
> 

Re: [PATCH v2 7/7] PCI/quirks: Avoid certain address on Genoa systems

[Borislav Petkov]

On Fri, Oct 02, 2026 at 02:33:15PM +0300, Ilpo Järvinen wrote:
> While testing the resource placement changes, tests hit a case where
> igb fails to probe when BAR 0 is placed at 0x9c000000:
> 
>    90000000-9cffffff : PCI Bus 0000:a0
>   -  90000000-902fffff : PCI Bus 0000:a1
>   -    90000000-900fffff : 0000:a1:00.0
>   -      90000000-900fffff : igb
>   -    90100000-901fffff : 0000:a1:00.0
>   -    90200000-90203fff : 0000:a1:00.0
>   -      90200000-90203fff : igb
>   +  9be00000-9c0fffff : PCI Bus 0000:a1
>   +    9be00000-9befffff : 0000:a1:00.0
>   +    9bf00000-9bf03fff : 0000:a1:00.0
>   +    9c000000-9c0fffff : 0000:a1:00.0
>      9c100000-9c17ffff : amd_iommu
>      9c180000-9c1803ff : IOAPIC 8
> 
>   -       Region 0: Memory at 90000000 (32-bit, non-prefetchable) [size=1M]
>   -       Region 3: Memory at 90200000 (32-bit, non-prefetchable) [size=16K]
>   -       Expansion ROM at 90100000 [disabled] [size=1M]
>   +       Region 0: Memory at 9c000000 (32-bit, non-prefetchable) [size=1M]
>   +       Region 3: Memory at 9bf00000 (32-bit, non-prefetchable) [size=16K]
>   +       Expansion ROM at 9be00000 [disabled] [size=1M]
> 
> igb 0000:a1:00.0 0000:a1:00.0 (uninitialized): PCIe link lost
> ------------[ cut here ]------------
> igb: Failed to read reg 0x18!
> WARNING: drivers/net/ethernet/intel/igb/igb_main.c:724 at igb_rd32.cold+0x3c/0x4f [igb], CPU#32: kworker/32:1/706
> ...
>  igb_get_invariants_82575+0xff/0xf00 [igb]
>  igb_probe+0x3c8/0x1190 [igb]
>  local_pci_probe+0x3b/0x80
> 
> It turns out there is a 64kB iomem black hole at 9c000000 that returns

Can we please debug that "It turns out" thing and try to find out what really
is going on here.

Can you pls explain what your machine looks like so that we can try to
reproduce it and see what's going on?

Does it have latest BIOS, microcode, etc etc? If it doesn't, can you upgrade
and try again?

I guess dmesg, hwinfo, kernel .config would be good starters, privately is
fine too.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v2 7/7] PCI/quirks: Avoid certain address on Genoa systems

[Bjorn Helgaas]

[+cc Boris]

On Fri, Oct 02, 2026 at 02:33:15PM +0300, Ilpo Järvinen wrote:
> While testing the resource placement changes, tests hit a case where
> igb fails to probe when BAR 0 is placed at 0x9c000000:
> 
>    90000000-9cffffff : PCI Bus 0000:a0
>   -  90000000-902fffff : PCI Bus 0000:a1
>   -    90000000-900fffff : 0000:a1:00.0
>   -      90000000-900fffff : igb
>   -    90100000-901fffff : 0000:a1:00.0
>   -    90200000-90203fff : 0000:a1:00.0
>   -      90200000-90203fff : igb
>   +  9be00000-9c0fffff : PCI Bus 0000:a1
>   +    9be00000-9befffff : 0000:a1:00.0
>   +    9bf00000-9bf03fff : 0000:a1:00.0
>   +    9c000000-9c0fffff : 0000:a1:00.0
>      9c100000-9c17ffff : amd_iommu
>      9c180000-9c1803ff : IOAPIC 8
> 
>   -       Region 0: Memory at 90000000 (32-bit, non-prefetchable) [size=1M]
>   -       Region 3: Memory at 90200000 (32-bit, non-prefetchable) [size=16K]
>   -       Expansion ROM at 90100000 [disabled] [size=1M]
>   +       Region 0: Memory at 9c000000 (32-bit, non-prefetchable) [size=1M]
>   +       Region 3: Memory at 9bf00000 (32-bit, non-prefetchable) [size=16K]
>   +       Expansion ROM at 9be00000 [disabled] [size=1M]
> 
> igb 0000:a1:00.0 0000:a1:00.0 (uninitialized): PCIe link lost
> ------------[ cut here ]------------
> igb: Failed to read reg 0x18!
> WARNING: drivers/net/ethernet/intel/igb/igb_main.c:724 at igb_rd32.cold+0x3c/0x4f [igb], CPU#32: kworker/32:1/706
> ...
>  igb_get_invariants_82575+0xff/0xf00 [igb]
>  igb_probe+0x3c8/0x1190 [igb]
>  local_pci_probe+0x3b/0x80
> 
> It turns out there is a 64kB iomem black hole at 9c000000 that returns
> ~0 and this is where igb's BAR 0 resides. If another BAR of the same
> card is placed into that address, it is similarly black holed.
> 
> Mark the problematic 64kB range reserved using a quirk bound to the
> bridge found in the problematic system.

I think this is my fault, or at least it looks like it's related to
07eab0901ede ("efi/x86: Remove EfiMemoryMappedIO from E820 map").

This platform describes the [0x9c000000-0x9cffffff] range as
E820_TYPE_RESERVED in the E820 table and as EFI_MEMORY_MAPPED_IO in
the EFI memory map (from the dmesg at
https://bugzilla.kernel.org/show_bug.cgi?id=222074):

  BIOS-e820: [gap 0x0000000090000000-0x000000009bffffff]
  BIOS-e820: [mem 0x000000009c000000-0x000000009cffffff]  device reserved
  efi: Remove mem49: MMIO range=[0x9c000000-0x9cffffff] (16MB) from e820 map
  e820: remove [mem 0x9c000000-0x9cffffff] device reserved
  pci_bus 0000:a0: root bus resource [mem 0x90000000-0x9cffffff window]

/proc/iomem:

  90000000-9cffffff : PCI Bus 0000:a0
    9c100000-9c17ffff : amd_iommu
    9c180000-9c1803ff : IOAPIC 8

I haven't worked out all the details, but I bet that if 07eab0901ede
had not removed [0x9c000000-0x9cffffff] from the E820 table, it would
show up in /proc/iomem as "Reserved" and would not be available for
use by a BAR.  amd_iommu and IOAPIC 8 occupy some of that space, and I
suspect there are other devices in there that we don't know about.

It looks like the last 16MB of every 32-bit PCI host bridge window is
EFI_MEMORY_MAPPED_IO, so if you can move the igb device to a different
host bridge, I suspect the same problem would happen if you put the
BAR at 16MB below the end.

> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=222074
> Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
> 
> Mario suggested the quirk to be based on the bridge instead of the 
> endpoint device which certainly looks better and cleaner than the
> approach used in v1.
> 
> The current plan is to try a different card in the same slot but it is
> a bit hard for me to predictable on what timescale that can be done.
> 
> ---
>  drivers/pci/quirks.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index de9bbccda21f..5483b47d8d54 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -6288,6 +6288,41 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1536, rom_bar_overlap_defect);
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1537, rom_bar_overlap_defect);
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1538, rom_bar_overlap_defect);
>  
> +/*
> + * TODO: Remove when/if root cause is found.
> + *
> + * Genoa appears to have a 64kB iomem black hole starting at 0x9c000000
> + * address for which all reads return ~0 blocking an overlapping BAR from
> + * working.
> + *
> + * Work around the problem by reserving the space prior to making any iomem
> + * allocations that could overlap with the black hole.
> + */
> +static struct resource black_hole_res = DEFINE_RES_MEM_NAMED(0x9c000000, SZ_64K,
> +							     "reserved");
> +
> +static void genoa_iomem_black_hole(struct pci_dev *dev)
> +{
> +	struct resource *r;
> +
> +	pci_bus_for_each_resource(dev->bus, r) {
> +		if (!r || !r->flags || !resource_assigned(r))
> +			continue;
> +
> +		if (!__resource_contains_unbound(r, &black_hole_res))
> +			continue;
> +
> +		if (resource_assigned(&black_hole_res))
> +			pci_dbg(dev, "iomem black hole workaround already applied\n");
> +		else if (!insert_resource(r, &black_hole_res))
> +			pci_info(dev, "iomem black hole workaround enabled\n");
> +		else
> +			pci_dbg(dev, "iomem black hole workaround add failed\n");
> +		return;
> +	}
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, 0x14ab, genoa_iomem_black_hole);
> +
>  #ifdef CONFIG_PCIEASPM
>  /*
>   * Several Intel DG2 graphics devices advertise that they can only tolerate
> -- 
> 2.47.3
> 

Re: [PATCH v2 7/7] PCI/quirks: Avoid certain address on Genoa systems

[Ilpo Järvinen]

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

On Mon, 5 Oct 2026, Bjorn Helgaas wrote:

> [+cc Boris]
> 
> On Fri, Oct 02, 2026 at 02:33:15PM +0300, Ilpo Järvinen wrote:
> > While testing the resource placement changes, tests hit a case where
> > igb fails to probe when BAR 0 is placed at 0x9c000000:
> > 
> >    90000000-9cffffff : PCI Bus 0000:a0
> >   -  90000000-902fffff : PCI Bus 0000:a1
> >   -    90000000-900fffff : 0000:a1:00.0
> >   -      90000000-900fffff : igb
> >   -    90100000-901fffff : 0000:a1:00.0
> >   -    90200000-90203fff : 0000:a1:00.0
> >   -      90200000-90203fff : igb
> >   +  9be00000-9c0fffff : PCI Bus 0000:a1
> >   +    9be00000-9befffff : 0000:a1:00.0
> >   +    9bf00000-9bf03fff : 0000:a1:00.0
> >   +    9c000000-9c0fffff : 0000:a1:00.0
> >      9c100000-9c17ffff : amd_iommu
> >      9c180000-9c1803ff : IOAPIC 8
> > 
> >   -       Region 0: Memory at 90000000 (32-bit, non-prefetchable) [size=1M]
> >   -       Region 3: Memory at 90200000 (32-bit, non-prefetchable) [size=16K]
> >   -       Expansion ROM at 90100000 [disabled] [size=1M]
> >   +       Region 0: Memory at 9c000000 (32-bit, non-prefetchable) [size=1M]
> >   +       Region 3: Memory at 9bf00000 (32-bit, non-prefetchable) [size=16K]
> >   +       Expansion ROM at 9be00000 [disabled] [size=1M]
> > 
> > igb 0000:a1:00.0 0000:a1:00.0 (uninitialized): PCIe link lost
> > ------------[ cut here ]------------
> > igb: Failed to read reg 0x18!
> > WARNING: drivers/net/ethernet/intel/igb/igb_main.c:724 at igb_rd32.cold+0x3c/0x4f [igb], CPU#32: kworker/32:1/706
> > ...
> >  igb_get_invariants_82575+0xff/0xf00 [igb]
> >  igb_probe+0x3c8/0x1190 [igb]
> >  local_pci_probe+0x3b/0x80
> > 
> > It turns out there is a 64kB iomem black hole at 9c000000 that returns
> > ~0 and this is where igb's BAR 0 resides. If another BAR of the same
> > card is placed into that address, it is similarly black holed.
> > 
> > Mark the problematic 64kB range reserved using a quirk bound to the
> > bridge found in the problematic system.
> 
> I think this is my fault, or at least it looks like it's related to
> 07eab0901ede ("efi/x86: Remove EfiMemoryMappedIO from E820 map").
> 
> This platform describes the [0x9c000000-0x9cffffff] range as
> E820_TYPE_RESERVED in the E820 table and as EFI_MEMORY_MAPPED_IO in
> the EFI memory map (from the dmesg at
> https://bugzilla.kernel.org/show_bug.cgi?id=222074):
> 
>   BIOS-e820: [gap 0x0000000090000000-0x000000009bffffff]
>   BIOS-e820: [mem 0x000000009c000000-0x000000009cffffff]  device reserved
>   efi: Remove mem49: MMIO range=[0x9c000000-0x9cffffff] (16MB) from e820 map
>   e820: remove [mem 0x9c000000-0x9cffffff] device reserved
>   pci_bus 0000:a0: root bus resource [mem 0x90000000-0x9cffffff window]
> 
> /proc/iomem:
> 
>   90000000-9cffffff : PCI Bus 0000:a0
>     9c100000-9c17ffff : amd_iommu
>     9c180000-9c1803ff : IOAPIC 8
> 
> I haven't worked out all the details, but I bet that if 07eab0901ede
> had not removed [0x9c000000-0x9cffffff] from the E820 table, it would
> show up in /proc/iomem as "Reserved" and would not be available for
> use by a BAR.  amd_iommu and IOAPIC 8 occupy some of that space, and I
> suspect there are other devices in there that we don't know about.
> 
> It looks like the last 16MB of every 32-bit PCI host bridge window is
> EFI_MEMORY_MAPPED_IO, so if you can move the igb device to a different
> host bridge, I suspect the same problem would happen if you put the
> BAR at 16MB below the end.

Perhaps things could break but even in this range I've found addresses 
above 9c000000 that do work. And the problem at 9c000000 is a black hole 
(returning ~0), not something that seems a meaningful device.


With 07eab0901ede ("efi/x86: Remove EfiMemoryMappedIO from E820 map") 
reverted, I get this:

90000000-9cffffff : PCI Bus 0000:a0
  9bd00000-9bffffff : PCI Bus 0000:a1
    9bd00000-9bdfffff : 0000:a1:00.0
      9bd00000-9bdfffff : igb
    9befc000-9befffff : 0000:a1:00.0
      9befc000-9befffff : igb
    9bf00000-9bffffff : 0000:a1:00.0
  9c000000-9cffffff : Reserved
    9c100000-9c17ffff : amd_iommu
    9c180000-9c1803ff : IOAPIC 8

-- 
 i.

> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=222074
> > Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > ---
> > 
> > Mario suggested the quirk to be based on the bridge instead of the 
> > endpoint device which certainly looks better and cleaner than the
> > approach used in v1.
> > 
> > The current plan is to try a different card in the same slot but it is
> > a bit hard for me to predictable on what timescale that can be done.
> > 
> > ---
> >  drivers/pci/quirks.c | 35 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> > 
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index de9bbccda21f..5483b47d8d54 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -6288,6 +6288,41 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1536, rom_bar_overlap_defect);
> >  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1537, rom_bar_overlap_defect);
> >  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1538, rom_bar_overlap_defect);
> >  
> > +/*
> > + * TODO: Remove when/if root cause is found.
> > + *
> > + * Genoa appears to have a 64kB iomem black hole starting at 0x9c000000
> > + * address for which all reads return ~0 blocking an overlapping BAR from
> > + * working.
> > + *
> > + * Work around the problem by reserving the space prior to making any iomem
> > + * allocations that could overlap with the black hole.
> > + */
> > +static struct resource black_hole_res = DEFINE_RES_MEM_NAMED(0x9c000000, SZ_64K,
> > +							     "reserved");
> > +
> > +static void genoa_iomem_black_hole(struct pci_dev *dev)
> > +{
> > +	struct resource *r;
> > +
> > +	pci_bus_for_each_resource(dev->bus, r) {
> > +		if (!r || !r->flags || !resource_assigned(r))
> > +			continue;
> > +
> > +		if (!__resource_contains_unbound(r, &black_hole_res))
> > +			continue;
> > +
> > +		if (resource_assigned(&black_hole_res))
> > +			pci_dbg(dev, "iomem black hole workaround already applied\n");
> > +		else if (!insert_resource(r, &black_hole_res))
> > +			pci_info(dev, "iomem black hole workaround enabled\n");
> > +		else
> > +			pci_dbg(dev, "iomem black hole workaround add failed\n");
> > +		return;
> > +	}
> > +}
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, 0x14ab, genoa_iomem_black_hole);
> > +
> >  #ifdef CONFIG_PCIEASPM
> >  /*
> >   * Several Intel DG2 graphics devices advertise that they can only tolerate
> > -- 
> > 2.47.3
> > 
> 

-- 
 i.