[PATCH v2 00/24] PCI: Bridge window selection improvements

[PATCH v2 00/24] PCI: Bridge window selection improvements

[Ilpo Järvinen]

This series is based on top of the three resource fitting and assignment
algorithm fixes already in the pci/resource branch. I've tried to compare
these patch with the commits in the pci/resource branch to retain the minor
spelling/grammar corrections Bjorn made while applying v1.

v2 is just to fix two small issues within the series intermediate patches.
These corrections attempt to ensure this series is bisectable if
troubleshooting requires that in the future.

In addition, a few corrections to changelog texts were made.

I'm left to wonder though if the added double spaces after some stops
within the commit messages in the pci/resource branch were intentional or
not (I did remove them for v2).

As the changes are very minimal, I'm only sending this to lists and Bjorn
to spare people's inboxes. If somebody provides a Tested-by tag for v1, it
should be counted in for this v2 (v1 vs v2 difference does not matter if
testing the entire series).

v2:
- In pci_bridge_release_resources():
    - Keep type assignment in until removing the type hack.
    - Introduce res_name in the patch it is used avoid compiler warning
      about unused variable. Place it into the block that needs it.
- Minor corrections to changelog texts

Ilpo Järvinen (24):
  m68k/PCI: Use pci_enable_resources() in pcibios_enable_device()
  sparc/PCI: Remove pcibios_enable_device() as they do nothing extra
  MIPS: PCI: Use pci_enable_resources()
  PCI: Move find_bus_resource_of_type() earlier
  PCI: Refactor find_bus_resource_of_type() logic checks
  PCI: Always claim bridge window before its setup
  PCI: Disable non-claimed bridge window
  PCI: Use pci_release_resource() instead of release_resource()
  PCI: Enable bridge even if bridge window fails to assign
  PCI: Preserve bridge window resource type flags
  PCI: Add defines for bridge window indexing
  PCI: Add bridge window selection functions
  PCI: Fix finding bridge window in pci_reassign_bridge_resources()
  PCI: Warn if bridge window cannot be released when resizing BAR
  PCI: Use pbus_select_window() during BAR resize
  PCI: Use pbus_select_window_for_type() during IO window sizing
  PCI: Rename resource variable from r to res
  PCI: Use pbus_select_window() in space available checker
  PCI: Use pbus_select_window_for_type() during mem window sizing
  PCI: Refactor distributing available memory to use loops
  PCI: Refactor remove_dev_resources() to use pbus_select_window()
  PCI: Add pci_setup_one_bridge_window()
  PCI: Pass bridge window to pci_bus_release_bridge_resources()
  PCI: Alter misleading recursion to pci_bus_release_bridge_resources()

 arch/m68k/kernel/pcibios.c   |  39 +-
 arch/mips/pci/pci-legacy.c   |  38 +-
 arch/sparc/kernel/leon_pci.c |  27 --
 arch/sparc/kernel/pci.c      |  27 --
 arch/sparc/kernel/pcic.c     |  27 --
 drivers/pci/bus.c            |   3 +
 drivers/pci/pci-sysfs.c      |  27 +-
 drivers/pci/pci.h            |   8 +-
 drivers/pci/probe.c          |  35 +-
 drivers/pci/setup-bus.c      | 798 ++++++++++++++++++-----------------
 drivers/pci/setup-res.c      |  46 +-
 include/linux/pci.h          |   5 +-
 12 files changed, 504 insertions(+), 576 deletions(-)


base-commit: 295524c65d8b4850efbb809f12176eb1262a5aba
-- 
2.39.5

[PATCH v2 01/24] m68k/PCI: Use pci_enable_resources() in pcibios_enable_device()

[Ilpo Järvinen]

m68k has a resource enable (check) loop in its pcibios_enable_device()
which for some reason differs from pci_enable_resources(). This could lead
to inconsistencies in behavior, especially now as pci_enable_resources()
and the bridge window resource flags behavior are going to be altered by
upcoming changes.

The check for !r->start && r->end is already covered by the more generic
checks done in pci_enable_resources().

The entire pcibios_enable_device() suspiciously looks copy-paste from some
other arch as also indicated by the preceding comment. However, it also
enables PCI_COMMAND_IO | PCI_COMMAND_MEMORY always for bridges. It is not
clear why that is being done as the commit e93a6bbeb5a5 ("m68k: common PCI
support definitions and code") introducing this code states "Nothing
specific to any PCI implementation in any m68k class CPU hardware yet".

Replace the resource enable loop with a call to pci_enable_resources() and
adjust the Command Register afterwards as it's unclear if that is necessary
or not so keep it for now.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 arch/m68k/kernel/pcibios.c | 39 +++++++++++---------------------------
 1 file changed, 11 insertions(+), 28 deletions(-)

diff --git a/arch/m68k/kernel/pcibios.c b/arch/m68k/kernel/pcibios.c
index 9504eb19d73a..e6ab3f9ff5d8 100644
--- a/arch/m68k/kernel/pcibios.c
+++ b/arch/m68k/kernel/pcibios.c
@@ -44,41 +44,24 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
  */
 int pcibios_enable_device(struct pci_dev *dev, int mask)
 {
-	struct resource *r;
 	u16 cmd, newcmd;
-	int idx;
+	int ret;
 
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	newcmd = cmd;
-
-	for (idx = 0; idx < 6; idx++) {
-		/* Only set up the requested stuff */
-		if (!(mask & (1 << idx)))
-			continue;
-
-		r = dev->resource + idx;
-		if (!r->start && r->end) {
-			pr_err("PCI: Device %s not available because of resource collisions\n",
-				pci_name(dev));
-			return -EINVAL;
-		}
-		if (r->flags & IORESOURCE_IO)
-			newcmd |= PCI_COMMAND_IO;
-		if (r->flags & IORESOURCE_MEM)
-			newcmd |= PCI_COMMAND_MEMORY;
-	}
+	ret = pci_enable_resources(dev, mask);
+	if (ret < 0)
+		return ret;
 
 	/*
 	 * Bridges (eg, cardbus bridges) need to be fully enabled
 	 */
-	if ((dev->class >> 16) == PCI_BASE_CLASS_BRIDGE)
+	if ((dev->class >> 16) == PCI_BASE_CLASS_BRIDGE) {
+		pci_read_config_word(dev, PCI_COMMAND, &cmd);
 		newcmd |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY;
-
-
-	if (newcmd != cmd) {
-		pr_info("PCI: enabling device %s (0x%04x -> 0x%04x)\n",
-			pci_name(dev), cmd, newcmd);
-		pci_write_config_word(dev, PCI_COMMAND, newcmd);
+		if (newcmd != cmd) {
+			pr_info("PCI: enabling bridge %s (0x%04x -> 0x%04x)\n",
+				pci_name(dev), cmd, newcmd);
+			pci_write_config_word(dev, PCI_COMMAND, newcmd);
+		}
 	}
 	return 0;
 }
-- 
2.39.5

[PATCH v2 02/24] sparc/PCI: Remove pcibios_enable_device() as they do nothing extra

[Ilpo Järvinen]

Under arch/sparc/ there are multiple copies of pcibios_enable_device() but
none of those seem to do anything extra beyond what pci_enable_resources()
is supposed to do. These functions could lead to inconsistencies in
behavior, especially now as pci_enable_resources() and the bridge window
resource flags behavior are going to be altered by upcoming changes.

Remove all pcibios_enable_device() from arch/sparc/ so that PCI core can
simply call into pci_enable_resources() instead using its __weak version
of pcibios_enable_device().

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 arch/sparc/kernel/leon_pci.c | 27 ---------------------------
 arch/sparc/kernel/pci.c      | 27 ---------------------------
 arch/sparc/kernel/pcic.c     | 27 ---------------------------
 3 files changed, 81 deletions(-)

diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
index 8de6646e9ce8..10934dfa987a 100644
--- a/arch/sparc/kernel/leon_pci.c
+++ b/arch/sparc/kernel/leon_pci.c
@@ -60,30 +60,3 @@ void leon_pci_init(struct platform_device *ofdev, struct leon_pci_info *info)
 	pci_assign_unassigned_resources();
 	pci_bus_add_devices(root_bus);
 }
-
-int pcibios_enable_device(struct pci_dev *dev, int mask)
-{
-	struct resource *res;
-	u16 cmd, oldcmd;
-	int i;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	oldcmd = cmd;
-
-	pci_dev_for_each_resource(dev, res, i) {
-		/* Only set up the requested stuff */
-		if (!(mask & (1<<i)))
-			continue;
-
-		if (res->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (res->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-
-	if (cmd != oldcmd) {
-		pci_info(dev, "enabling device (%04x -> %04x)\n", oldcmd, cmd);
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-	return 0;
-}
diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
index ddac216a2aff..a9448088e762 100644
--- a/arch/sparc/kernel/pci.c
+++ b/arch/sparc/kernel/pci.c
@@ -722,33 +722,6 @@ struct pci_bus *pci_scan_one_pbm(struct pci_pbm_info *pbm,
 	return bus;
 }
 
-int pcibios_enable_device(struct pci_dev *dev, int mask)
-{
-	struct resource *res;
-	u16 cmd, oldcmd;
-	int i;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	oldcmd = cmd;
-
-	pci_dev_for_each_resource(dev, res, i) {
-		/* Only set up the requested stuff */
-		if (!(mask & (1<<i)))
-			continue;
-
-		if (res->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (res->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-
-	if (cmd != oldcmd) {
-		pci_info(dev, "enabling device (%04x -> %04x)\n", oldcmd, cmd);
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-	return 0;
-}
-
 /* Platform support for /proc/bus/pci/X/Y mmap()s. */
 int pci_iobar_pfn(struct pci_dev *pdev, int bar, struct vm_area_struct *vma)
 {
diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index 25fe0a061732..3d54ad5656a4 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -641,33 +641,6 @@ void pcibios_fixup_bus(struct pci_bus *bus)
 	}
 }
 
-int pcibios_enable_device(struct pci_dev *dev, int mask)
-{
-	struct resource *res;
-	u16 cmd, oldcmd;
-	int i;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	oldcmd = cmd;
-
-	pci_dev_for_each_resource(dev, res, i) {
-		/* Only set up the requested stuff */
-		if (!(mask & (1<<i)))
-			continue;
-
-		if (res->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (res->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-
-	if (cmd != oldcmd) {
-		pci_info(dev, "enabling device (%04x -> %04x)\n", oldcmd, cmd);
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-	return 0;
-}
-
 /* Makes compiler happy */
 static volatile int pcic_timer_dummy;
 
-- 
2.39.5

[PATCH v2 03/24] MIPS: PCI: Use pci_enable_resources()

[Ilpo Järvinen]

pci-legacy.c under MIPS has a copy of pci_enable_resources() named as
pcibios_enable_resources(). Having own copy of same functionality could
lead to inconsistencies in behavior, especially now as
pci_enable_resources() and the bridge window resource flags behavior are
going to be altered by upcoming changes.

The check for !r->start && r->end is already covered by the more generic
checks done in pci_enable_resources().

Call pci_enable_resources() from MIPS's pcibios_enable_device() and remove
pcibios_enable_resources().

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
---
 arch/mips/pci/pci-legacy.c | 38 ++------------------------------------
 1 file changed, 2 insertions(+), 36 deletions(-)

diff --git a/arch/mips/pci/pci-legacy.c b/arch/mips/pci/pci-legacy.c
index 66898fd182dc..d04b7c1294b6 100644
--- a/arch/mips/pci/pci-legacy.c
+++ b/arch/mips/pci/pci-legacy.c
@@ -249,45 +249,11 @@ static int __init pcibios_init(void)
 
 subsys_initcall(pcibios_init);
 
-static int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
-	u16 cmd, old_cmd;
-	int idx;
-	struct resource *r;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	old_cmd = cmd;
-	pci_dev_for_each_resource(dev, r, idx) {
-		/* Only set up the requested stuff */
-		if (!(mask & (1<<idx)))
-			continue;
-
-		if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
-			continue;
-		if ((idx == PCI_ROM_RESOURCE) &&
-				(!(r->flags & IORESOURCE_ROM_ENABLE)))
-			continue;
-		if (!r->start && r->end) {
-			pci_err(dev,
-				"can't enable device: resource collisions\n");
-			return -EINVAL;
-		}
-		if (r->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (r->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-	if (cmd != old_cmd) {
-		pci_info(dev, "enabling device (%04x -> %04x)\n", old_cmd, cmd);
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-	return 0;
-}
-
 int pcibios_enable_device(struct pci_dev *dev, int mask)
 {
-	int err = pcibios_enable_resources(dev, mask);
+	int err;
 
+	err = pci_enable_resources(dev, mask);
 	if (err < 0)
 		return err;
 
-- 
2.39.5

[PATCH v2 04/24] PCI: Move find_bus_resource_of_type() earlier

[Ilpo Järvinen]

Move find_bus_resource_of_type() earlier in setup-bus.c to be able to call
it in upcoming changes.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/setup-bus.c | 56 ++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index def29506700e..4097d8703b8f 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -140,6 +140,34 @@ static void restore_dev_resource(struct pci_dev_resource *dev_res)
 	res->flags = dev_res->flags;
 }
 
+/*
+ * Helper function for sizing routines.  Assigned resources have non-NULL
+ * parent resource.
+ *
+ * Return first unassigned resource of the correct type.  If there is none,
+ * return first assigned resource of the correct type.  If none of the
+ * above, return NULL.
+ *
+ * Returning an assigned resource of the correct type allows the caller to
+ * distinguish between already assigned and no resource of the correct type.
+ */
+static struct resource *find_bus_resource_of_type(struct pci_bus *bus,
+						  unsigned long type_mask,
+						  unsigned long type)
+{
+	struct resource *r, *r_assigned = NULL;
+
+	pci_bus_for_each_resource(bus, r) {
+		if (r == &ioport_resource || r == &iomem_resource)
+			continue;
+		if (r && (r->flags & type_mask) == type && !r->parent)
+			return r;
+		if (r && (r->flags & type_mask) == type && !r_assigned)
+			r_assigned = r;
+	}
+	return r_assigned;
+}
+
 static bool pdev_resources_assignable(struct pci_dev *dev)
 {
 	u16 class = dev->class >> 8, command;
@@ -876,34 +904,6 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
 	}
 }
 
-/*
- * Helper function for sizing routines.  Assigned resources have non-NULL
- * parent resource.
- *
- * Return first unassigned resource of the correct type.  If there is none,
- * return first assigned resource of the correct type.  If none of the
- * above, return NULL.
- *
- * Returning an assigned resource of the correct type allows the caller to
- * distinguish between already assigned and no resource of the correct type.
- */
-static struct resource *find_bus_resource_of_type(struct pci_bus *bus,
-						  unsigned long type_mask,
-						  unsigned long type)
-{
-	struct resource *r, *r_assigned = NULL;
-
-	pci_bus_for_each_resource(bus, r) {
-		if (r == &ioport_resource || r == &iomem_resource)
-			continue;
-		if (r && (r->flags & type_mask) == type && !r->parent)
-			return r;
-		if (r && (r->flags & type_mask) == type && !r_assigned)
-			r_assigned = r;
-	}
-	return r_assigned;
-}
-
 static resource_size_t calculate_iosize(resource_size_t size,
 					resource_size_t min_size,
 					resource_size_t size1,
-- 
2.39.5

[PATCH v2 05/24] PCI: Refactor find_bus_resource_of_type() logic checks

[Ilpo Järvinen]

Reorder the logic checks in find_bus_resource_of_type() to simplify them.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/setup-bus.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 4097d8703b8f..c5fc4e2825be 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -158,11 +158,15 @@ static struct resource *find_bus_resource_of_type(struct pci_bus *bus,
 	struct resource *r, *r_assigned = NULL;
 
 	pci_bus_for_each_resource(bus, r) {
-		if (r == &ioport_resource || r == &iomem_resource)
+		if (!r || r == &ioport_resource || r == &iomem_resource)
 			continue;
-		if (r && (r->flags & type_mask) == type && !r->parent)
+
+		if ((r->flags & type_mask) != type)
+			continue;
+
+		if (!r->parent)
 			return r;
-		if (r && (r->flags & type_mask) == type && !r_assigned)
+		if (!r_assigned)
 			r_assigned = r;
 	}
 	return r_assigned;
-- 
2.39.5

[PATCH v2 06/24] PCI: Always claim bridge window before its setup

[Ilpo Järvinen]

When the claim of a resource fails for the full range in
pci_claim_bridge_resource(), clipping the resource to a smaller size is
attempted. If clipping is successful, the new bridge window is programmed
and only as the last step the code attempts to claim the resource again.
The order of the last two steps is slightly illogical and inconsistent with
the assignment call chains.

If claiming the bridge window after clipping fails, the bridge window that
was set up is left in place.

Rework the logic such that the bridge window is claimed before calling the
relevant bridge setup function. This make the behavior consistent with
resource fitting call chains that always assign the bridge window before
programming it.

If claiming the bridge window fails, the clipped bridge window is no longer
set up but pci_claim_bridge_resource() returns without writing the bridge
window at all.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/setup-bus.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index c5fc4e2825be..b477f68b236c 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -857,9 +857,16 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i)
 	if ((bridge->class >> 8) != PCI_CLASS_BRIDGE_PCI)
 		return 0;
 
+	if (i > PCI_BRIDGE_PREF_MEM_WINDOW)
+		return -EINVAL;
+
+	/* Try to clip the resource and claim the smaller window */
 	if (!pci_bus_clip_resource(bridge, i))
 		return -EINVAL;	/* Clipping didn't change anything */
 
+	if (!pci_claim_resource(bridge, i))
+		return -EINVAL;
+
 	switch (i) {
 	case PCI_BRIDGE_IO_WINDOW:
 		pci_setup_bridge_io(bridge);
@@ -874,10 +881,7 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i)
 		return -EINVAL;
 	}
 
-	if (pci_claim_resource(bridge, i) == 0)
-		return 0;	/* Claimed a smaller window */
-
-	return -EINVAL;
+	return 0;
 }
 
 /*
-- 
2.39.5

[PATCH v2 07/24] PCI: Disable non-claimed bridge window

[Ilpo Järvinen]

If clipping or claiming the bridge window fails, the bridge window is left
in a state that does not match the kernel's view on what the bridge window
is.

Disable the bridge window by writing the magic disable value into the Base
and Limit Registers if clipping or claiming failed. To detect if claiming
the resource was successful, add res->parent checks into the bridge setup
functions.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/setup-bus.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index b477f68b236c..6bdc1af887da 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -660,7 +660,7 @@ void pci_setup_cardbus(struct pci_bus *bus)
 
 	res = bus->resource[0];
 	pcibios_resource_to_bus(bridge->bus, &region, res);
-	if (res->flags & IORESOURCE_IO) {
+	if (res->parent && res->flags & IORESOURCE_IO) {
 		/*
 		 * The IO resource is allocated a range twice as large as it
 		 * would normally need.  This allows us to set both IO regs.
@@ -674,7 +674,7 @@ void pci_setup_cardbus(struct pci_bus *bus)
 
 	res = bus->resource[1];
 	pcibios_resource_to_bus(bridge->bus, &region, res);
-	if (res->flags & IORESOURCE_IO) {
+	if (res->parent && res->flags & IORESOURCE_IO) {
 		pci_info(bridge, "  bridge window %pR\n", res);
 		pci_write_config_dword(bridge, PCI_CB_IO_BASE_1,
 					region.start);
@@ -684,7 +684,7 @@ void pci_setup_cardbus(struct pci_bus *bus)
 
 	res = bus->resource[2];
 	pcibios_resource_to_bus(bridge->bus, &region, res);
-	if (res->flags & IORESOURCE_MEM) {
+	if (res->parent && res->flags & IORESOURCE_MEM) {
 		pci_info(bridge, "  bridge window %pR\n", res);
 		pci_write_config_dword(bridge, PCI_CB_MEMORY_BASE_0,
 					region.start);
@@ -694,7 +694,7 @@ void pci_setup_cardbus(struct pci_bus *bus)
 
 	res = bus->resource[3];
 	pcibios_resource_to_bus(bridge->bus, &region, res);
-	if (res->flags & IORESOURCE_MEM) {
+	if (res->parent && res->flags & IORESOURCE_MEM) {
 		pci_info(bridge, "  bridge window %pR\n", res);
 		pci_write_config_dword(bridge, PCI_CB_MEMORY_BASE_1,
 					region.start);
@@ -735,7 +735,7 @@ static void pci_setup_bridge_io(struct pci_dev *bridge)
 	res = &bridge->resource[PCI_BRIDGE_IO_WINDOW];
 	res_name = pci_resource_name(bridge, PCI_BRIDGE_IO_WINDOW);
 	pcibios_resource_to_bus(bridge->bus, &region, res);
-	if (res->flags & IORESOURCE_IO) {
+	if (res->parent && res->flags & IORESOURCE_IO) {
 		pci_read_config_word(bridge, PCI_IO_BASE, &l);
 		io_base_lo = (region.start >> 8) & io_mask;
 		io_limit_lo = (region.end >> 8) & io_mask;
@@ -767,7 +767,7 @@ static void pci_setup_bridge_mmio(struct pci_dev *bridge)
 	res = &bridge->resource[PCI_BRIDGE_MEM_WINDOW];
 	res_name = pci_resource_name(bridge, PCI_BRIDGE_MEM_WINDOW);
 	pcibios_resource_to_bus(bridge->bus, &region, res);
-	if (res->flags & IORESOURCE_MEM) {
+	if (res->parent && res->flags & IORESOURCE_MEM) {
 		l = (region.start >> 16) & 0xfff0;
 		l |= region.end & 0xfff00000;
 		pci_info(bridge, "  %s %pR\n", res_name, res);
@@ -796,7 +796,7 @@ static void pci_setup_bridge_mmio_pref(struct pci_dev *bridge)
 	res = &bridge->resource[PCI_BRIDGE_PREF_MEM_WINDOW];
 	res_name = pci_resource_name(bridge, PCI_BRIDGE_PREF_MEM_WINDOW);
 	pcibios_resource_to_bus(bridge->bus, &region, res);
-	if (res->flags & IORESOURCE_PREFETCH) {
+	if (res->parent && res->flags & IORESOURCE_PREFETCH) {
 		l = (region.start >> 16) & 0xfff0;
 		l |= region.end & 0xfff00000;
 		if (res->flags & IORESOURCE_MEM_64) {
@@ -848,6 +848,8 @@ static void pci_setup_bridge(struct pci_bus *bus)
 
 int pci_claim_bridge_resource(struct pci_dev *bridge, int i)
 {
+	int ret = -EINVAL;
+
 	if (i < PCI_BRIDGE_RESOURCES || i > PCI_BRIDGE_RESOURCE_END)
 		return 0;
 
@@ -861,11 +863,8 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i)
 		return -EINVAL;
 
 	/* Try to clip the resource and claim the smaller window */
-	if (!pci_bus_clip_resource(bridge, i))
-		return -EINVAL;	/* Clipping didn't change anything */
-
-	if (!pci_claim_resource(bridge, i))
-		return -EINVAL;
+	if (pci_bus_clip_resource(bridge, i))
+		ret = pci_claim_resource(bridge, i);
 
 	switch (i) {
 	case PCI_BRIDGE_IO_WINDOW:
@@ -881,7 +880,7 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i)
 		return -EINVAL;
 	}
 
-	return 0;
+	return ret;
 }
 
 /*
-- 
2.39.5

[PATCH v2 08/24] PCI: Use pci_release_resource() instead of release_resource()

[Ilpo Järvinen]

A few places in setup-bus.c call release_resource() directly and end up
duplicating functionality from pci_release_resource() such as parent check,
logging, and clearing the resource. Worse yet, the way the resource is
cleared is inconsistent between different sites.

Convert release_resource() calls into pci_release_resource() to remove code
duplication. This will also make the resource start, end, and flags
behavior consistent, i.e., start address is cleared, and only
IORESOURCE_UNSET is asserted for the resource.

While at it, eliminate the unnecessary initialization of idx variable in
pci_bridge_release_resources().

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/setup-bus.c | 46 +++++++++++++----------------------------
 drivers/pci/setup-res.c | 11 +++++++---
 include/linux/pci.h     |  2 +-
 3 files changed, 23 insertions(+), 36 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 6bdc1af887da..b62465665abc 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -473,8 +473,6 @@ static void __assign_resources_sorted(struct list_head *head,
 	struct pci_dev_resource *dev_res, *tmp_res, *dev_res2;
 	struct resource *res;
 	struct pci_dev *dev;
-	const char *res_name;
-	int idx;
 	unsigned long fail_type;
 	resource_size_t add_align, align;
 
@@ -582,14 +580,7 @@ static void __assign_resources_sorted(struct list_head *head,
 		res = dev_res->res;
 		dev = dev_res->dev;
 
-		if (!res->parent)
-			continue;
-
-		idx = pci_resource_num(dev, res);
-		res_name = pci_resource_name(dev, idx);
-		pci_dbg(dev, "%s %pR: releasing\n", res_name, res);
-
-		release_resource(res);
+		pci_release_resource(dev, pci_resource_num(dev, res));
 		restore_dev_resource(dev_res);
 	}
 	/* Restore start/end/flags from saved list */
@@ -1732,7 +1723,7 @@ static void pci_bridge_release_resources(struct pci_bus *bus,
 	struct resource *r;
 	unsigned int old_flags;
 	struct resource *b_res;
-	int idx = 1;
+	int idx, ret;
 
 	b_res = &dev->resource[PCI_BRIDGE_RESOURCES];
 
@@ -1766,21 +1757,18 @@ static void pci_bridge_release_resources(struct pci_bus *bus,
 
 	/* If there are children, release them all */
 	release_child_resources(r);
-	if (!release_resource(r)) {
-		type = old_flags = r->flags & PCI_RES_TYPE_MASK;
-		pci_info(dev, "resource %d %pR released\n",
-			 PCI_BRIDGE_RESOURCES + idx, r);
-		/* Keep the old size */
-		resource_set_range(r, 0, resource_size(r));
-		r->flags = 0;
 
-		/* Avoiding touch the one without PREF */
-		if (type & IORESOURCE_PREFETCH)
-			type = IORESOURCE_PREFETCH;
-		__pci_setup_bridge(bus, type);
-		/* For next child res under same bridge */
-		r->flags = old_flags;
-	}
+	type = old_flags = r->flags & PCI_RES_TYPE_MASK;
+	ret = pci_release_resource(dev, PCI_BRIDGE_RESOURCES + idx);
+	if (ret)
+		return;
+
+	/* Avoiding touch the one without PREF */
+	if (type & IORESOURCE_PREFETCH)
+		type = IORESOURCE_PREFETCH;
+	__pci_setup_bridge(bus, type);
+	/* For next child res under same bridge */
+	r->flags = old_flags;
 }
 
 enum release_type {
@@ -2425,7 +2413,6 @@ int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
 		for (i = PCI_BRIDGE_RESOURCES; i < PCI_BRIDGE_RESOURCE_END;
 		     i++) {
 			struct resource *res = &bridge->resource[i];
-			const char *res_name = pci_resource_name(bridge, i);
 
 			if ((res->flags ^ type) & PCI_RES_TYPE_MASK)
 				continue;
@@ -2438,12 +2425,7 @@ int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
 			if (ret)
 				goto cleanup;
 
-			pci_info(bridge, "%s %pR: releasing\n", res_name, res);
-
-			if (res->parent)
-				release_resource(res);
-			res->start = 0;
-			res->end = 0;
+			pci_release_resource(bridge, i);
 			break;
 		}
 		if (i == PCI_BRIDGE_RESOURCE_END)
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index d2b3ed51e880..0468c058b598 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -406,20 +406,25 @@ int pci_reassign_resource(struct pci_dev *dev, int resno,
 	return 0;
 }
 
-void pci_release_resource(struct pci_dev *dev, int resno)
+int pci_release_resource(struct pci_dev *dev, int resno)
 {
 	struct resource *res = pci_resource_n(dev, resno);
 	const char *res_name = pci_resource_name(dev, resno);
+	int ret;
 
 	if (!res->parent)
-		return;
+		return 0;
 
 	pci_info(dev, "%s %pR: releasing\n", res_name, res);
 
-	release_resource(res);
+	ret = release_resource(res);
+	if (ret)
+		return ret;
 	res->end = resource_size(res) - 1;
 	res->start = 0;
 	res->flags |= IORESOURCE_UNSET;
+
+	return 0;
 }
 EXPORT_SYMBOL(pci_release_resource);
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 59876de13860..275df4058767 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1417,7 +1417,7 @@ void pci_reset_secondary_bus(struct pci_dev *dev);
 void pcibios_reset_secondary_bus(struct pci_dev *dev);
 void pci_update_resource(struct pci_dev *dev, int resno);
 int __must_check pci_assign_resource(struct pci_dev *dev, int i);
-void pci_release_resource(struct pci_dev *dev, int resno);
+int pci_release_resource(struct pci_dev *dev, int resno);
 static inline int pci_rebar_bytes_to_size(u64 bytes)
 {
 	bytes = roundup_pow_of_two(bytes);
-- 
2.39.5

[PATCH v2 09/24] PCI: Enable bridge even if bridge window fails to assign

[Ilpo Järvinen]

A normal PCI bridge has multiple bridge windows and not all of them are
always required by devices underneath the bridge. If a Root Port or bridge
does not have a device underneath, no bridge windows get assigned. Yet,
pci_enable_resources() is set to fail indiscriminantly on any resource
assignment failure if the resource is not known to be optional.

In practice, the code in pci_enable_resources() is currently largely
dormant. The kernel sets resource flags to zero for any unused bridge
window and resets flags to zero in case of an resource assignment failure,
which short-circuits pci_enable_resources() because of this check:

  if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
    continue;

However, an upcoming change to resource flags will alter how bridge window
resource flags behave activating these long dormants checks in
pci_enable_resources().

While complex logic could be built to selectively enable a bridge only
under some conditions, a few versions of such logic were tried during
development of this change and none of them worked satisfactorily. Thus, I
just gave up and decided to enable any bridge regardless of the bridge
windows as there seems to be no clear benefit from not enabling it, but a
major downside as pcieport will not be probed for the bridge if it's not
enabled.

Therefore, change pci_enable_resources() to not check if bridge window
resources remain unassigned. Resource assignment failures are pretty noisy
already so there is no need to log that for bridge windows in
pci_enable_resources().

Ignoring bridge window failures hopefully prevents an obvious source of
regressions when the upcoming change that no longer clears resource flags
for bridge windows is enacted. I've hit this problem even during my own
testing on multiple occasions so I expect it to be a quite common problem.

This can always be revisited later if somebody thinks the enable check for
bridges is not strict enough, but expect a mind-boggling number of
regressions from such a change.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/setup-res.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 0468c058b598..4e0e60256f04 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -527,22 +527,26 @@ int pci_enable_resources(struct pci_dev *dev, int mask)
 		if (pci_resource_is_optional(dev, i))
 			continue;
 
-		if (r->flags & IORESOURCE_UNSET) {
-			pci_err(dev, "%s %pR: not assigned; can't enable device\n",
-				r_name, r);
-			return -EINVAL;
+		if (i < PCI_BRIDGE_RESOURCES) {
+			if (r->flags & IORESOURCE_UNSET) {
+				pci_err(dev, "%s %pR: not assigned; can't enable device\n",
+					r_name, r);
+				return -EINVAL;
+			}
+
+			if (!r->parent) {
+				pci_err(dev, "%s %pR: not claimed; can't enable device\n",
+					r_name, r);
+				return -EINVAL;
+			}
 		}
 
-		if (!r->parent) {
-			pci_err(dev, "%s %pR: not claimed; can't enable device\n",
-				r_name, r);
-			return -EINVAL;
+		if (r->parent) {
+			if (r->flags & IORESOURCE_IO)
+				cmd |= PCI_COMMAND_IO;
+			if (r->flags & IORESOURCE_MEM)
+				cmd |= PCI_COMMAND_MEMORY;
 		}
-
-		if (r->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (r->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
 	}
 
 	if (cmd != old_cmd) {
-- 
2.39.5

[PATCH v2 10/24] PCI: Preserve bridge window resource type flags

[Ilpo Järvinen]

When a bridge window is found unused or fails to assign, the flags of the
associated resource are cleared. Clearing flags is problematic as it also
removes the type information of the resource which is needed later.

Thus, always preserve the bridge window type flags and use IORESOURCE_UNSET
and IORESOURCE_DISABLED to indicate the status of the bridge window. Also,
when initializing resources, make sure all valid bridge windows do get
their type flags set.

Change various places that relied on resource flags being cleared to check
for IORESOURCE_UNSET and IORESOURCE_DISABLED to allow bridge window
resource to retain their type flags. Add pdev_resource_assignable() and
pdev_resource_should_fit() helpers to filter out disabled bridge windows
during resource fitting; the latter combines more common checks into the
helper.

When reading the bridge windows from the registers, instead of leaving the
resource flags cleared for bridge windows that are not enabled, always
set up the flags and set IORESOURCE_UNSET | IORESOURCE_DISABLED as needed.

When resource fitting or assignment fails for a bridge window resource, or
the bridge window is not needed, mark the resource with IORESOURCE_UNSET or
IORESOURCE_DISABLED, respectively.

Use dummy zero resource in resource_show() for backwards compatibility as
lspci will otherwise misrepresent disabled bridge windows.

This change fixes an issue which highlights the importance of keeping the
resource type flags intact:

  At the end of __assign_resources_sorted(), reset_resource() is called,
  previously clearing the flags. Later, pci_prepare_next_assign_round()
  attempted to release bridge resources using
  pci_bus_release_bridge_resources() that calls into
  pci_bridge_release_resources() that assumes type flags are still present.
  As type flags were cleared, IORESOURCE_MEM_64 was not set leading to
  resources under an incorrect bridge window to be released (idx = 1
  instead of idx = 2). While the assignments performed later covered this
  problem so that the wrongly released resources got assigned in the end,
  it was still causing extra release+assign pairs.

There are other reasons why the resource flags should be retained in
upcoming changes too.

Removing the flag reset for non-bridge window resource is left as future
work, in part because it has a much higher regression potential due to
pci_enable_resources() that will start to work also for those resources
then and due to what endpoint drivers might assume about resources.

Despite the Fixes tag, backporting this (at least any time soon) is highly
discouraged. The issue fixed is borderline cosmetic as the later
assignments normally cover the problem entirely. Also there might be
non-obvious dependencies.

Fixes: 5b28541552ef ("PCI: Restrict 64-bit prefetchable bridge windows to 64-bit resources")
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/bus.c       |  3 ++
 drivers/pci/pci-sysfs.c |  7 ++++
 drivers/pci/probe.c     | 25 +++++++++---
 drivers/pci/setup-bus.c | 89 +++++++++++++++++++++++++++--------------
 drivers/pci/setup-res.c |  3 ++
 5 files changed, 90 insertions(+), 37 deletions(-)

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index b77fd30bbfd9..58b5388423ee 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -204,6 +204,9 @@ static int pci_bus_alloc_from_region(struct pci_bus *bus, struct resource *res,
 		if (!r)
 			continue;
 
+		if (r->flags & (IORESOURCE_UNSET|IORESOURCE_DISABLED))
+			continue;
+
 		/* type_mask must match */
 		if ((res->flags ^ r->flags) & type_mask)
 			continue;
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 5eea14c1f7f5..162a5241c7f7 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -177,6 +177,13 @@ static ssize_t resource_show(struct device *dev, struct device_attribute *attr,
 
 	for (i = 0; i < max; i++) {
 		struct resource *res =  &pci_dev->resource[i];
+		struct resource zerores = {};
+
+		/* For backwards compatibility */
+		if (i >= PCI_BRIDGE_RESOURCES && i <= PCI_BRIDGE_RESOURCE_END &&
+		    res->flags & (IORESOURCE_UNSET | IORESOURCE_DISABLED))
+			res = &zerores;
+
 		pci_resource_to_user(pci_dev, i, res, &start, &end);
 		len += sysfs_emit_at(buf, len, "0x%016llx 0x%016llx 0x%016llx\n",
 				     (unsigned long long)start,
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index f41128f91ca7..f31d27c7708a 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -419,13 +419,17 @@ static void pci_read_bridge_io(struct pci_dev *dev, struct resource *res,
 		limit |= ((unsigned long) io_limit_hi << 16);
 	}
 
+	res->flags = (io_base_lo & PCI_IO_RANGE_TYPE_MASK) | IORESOURCE_IO;
+
 	if (base <= limit) {
-		res->flags = (io_base_lo & PCI_IO_RANGE_TYPE_MASK) | IORESOURCE_IO;
 		region.start = base;
 		region.end = limit + io_granularity - 1;
 		pcibios_bus_to_resource(dev->bus, res, &region);
 		if (log)
 			pci_info(dev, "  bridge window %pR\n", res);
+	} else {
+		resource_set_range(res, 0, 0);
+		res->flags |= IORESOURCE_UNSET | IORESOURCE_DISABLED;
 	}
 }
 
@@ -440,13 +444,18 @@ static void pci_read_bridge_mmio(struct pci_dev *dev, struct resource *res,
 	pci_read_config_word(dev, PCI_MEMORY_LIMIT, &mem_limit_lo);
 	base = ((unsigned long) mem_base_lo & PCI_MEMORY_RANGE_MASK) << 16;
 	limit = ((unsigned long) mem_limit_lo & PCI_MEMORY_RANGE_MASK) << 16;
+
+	res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM;
+
 	if (base <= limit) {
-		res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM;
 		region.start = base;
 		region.end = limit + 0xfffff;
 		pcibios_bus_to_resource(dev->bus, res, &region);
 		if (log)
 			pci_info(dev, "  bridge window %pR\n", res);
+	} else {
+		resource_set_range(res, 0, 0);
+		res->flags |= IORESOURCE_UNSET | IORESOURCE_DISABLED;
 	}
 }
 
@@ -489,16 +498,20 @@ static void pci_read_bridge_mmio_pref(struct pci_dev *dev, struct resource *res,
 		return;
 	}
 
+	res->flags = (mem_base_lo & PCI_PREF_RANGE_TYPE_MASK) | IORESOURCE_MEM |
+		     IORESOURCE_PREFETCH;
+	if (res->flags & PCI_PREF_RANGE_TYPE_64)
+		res->flags |= IORESOURCE_MEM_64;
+
 	if (base <= limit) {
-		res->flags = (mem_base_lo & PCI_PREF_RANGE_TYPE_MASK) |
-					 IORESOURCE_MEM | IORESOURCE_PREFETCH;
-		if (res->flags & PCI_PREF_RANGE_TYPE_64)
-			res->flags |= IORESOURCE_MEM_64;
 		region.start = base;
 		region.end = limit + 0xfffff;
 		pcibios_bus_to_resource(dev->bus, res, &region);
 		if (log)
 			pci_info(dev, "  bridge window %pR\n", res);
+	} else {
+		resource_set_range(res, 0, 0);
+		res->flags |= IORESOURCE_UNSET | IORESOURCE_DISABLED;
 	}
 }
 
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index b62465665abc..70b210ed200d 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -190,6 +190,31 @@ static bool pdev_resources_assignable(struct pci_dev *dev)
 	return true;
 }
 
+static bool pdev_resource_assignable(struct pci_dev *dev, struct resource *res)
+{
+	int idx = pci_resource_num(dev, res);
+
+	if (!res->flags)
+		return false;
+
+	if (idx >= PCI_BRIDGE_RESOURCES && idx <= PCI_BRIDGE_RESOURCE_END &&
+	    res->flags & IORESOURCE_DISABLED)
+		return false;
+
+	return true;
+}
+
+static bool pdev_resource_should_fit(struct pci_dev *dev, struct resource *res)
+{
+	if (res->parent)
+		return false;
+
+	if (res->flags & IORESOURCE_PCI_FIXED)
+		return false;
+
+	return pdev_resource_assignable(dev, res);
+}
+
 /* Sort resources by alignment */
 static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head)
 {
@@ -205,10 +230,7 @@ static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head)
 		resource_size_t r_align;
 		struct list_head *n;
 
-		if (r->flags & IORESOURCE_PCI_FIXED)
-			continue;
-
-		if (!(r->flags) || r->parent)
+		if (!pdev_resource_should_fit(dev, r))
 			continue;
 
 		r_align = pci_resource_alignment(dev, r);
@@ -257,8 +279,15 @@ bool pci_resource_is_optional(const struct pci_dev *dev, int resno)
 	return false;
 }
 
-static inline void reset_resource(struct resource *res)
+static inline void reset_resource(struct pci_dev *dev, struct resource *res)
 {
+	int idx = pci_resource_num(dev, res);
+
+	if (idx >= PCI_BRIDGE_RESOURCES && idx <= PCI_BRIDGE_RESOURCE_END) {
+		res->flags |= IORESOURCE_UNSET;
+		return;
+	}
+
 	res->start = 0;
 	res->end = 0;
 	res->flags = 0;
@@ -610,7 +639,7 @@ static void __assign_resources_sorted(struct list_head *head,
 				    0 /* don't care */);
 		}
 
-		reset_resource(res);
+		reset_resource(dev, res);
 	}
 
 	free_list(head);
@@ -1014,8 +1043,11 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size,
 
 			if (r->parent || !(r->flags & IORESOURCE_IO))
 				continue;
-			r_size = resource_size(r);
 
+			if (!pdev_resource_assignable(dev, r))
+				continue;
+
+			r_size = resource_size(r);
 			if (r_size < SZ_1K)
 				/* Might be re-aligned for ISA */
 				size += r_size;
@@ -1034,6 +1066,9 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size,
 	size0 = calculate_iosize(size, min_size, size1, 0, 0,
 			resource_size(b_res), min_align);
 
+	if (size0)
+		b_res->flags &= ~IORESOURCE_DISABLED;
+
 	size1 = size0;
 	if (realloc_head && (add_size > 0 || children_add_size > 0)) {
 		size1 = calculate_iosize(size, min_size, size1, add_size,
@@ -1045,13 +1080,14 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size,
 		if (bus->self && (b_res->start || b_res->end))
 			pci_info(bus->self, "disabling bridge window %pR to %pR (unused)\n",
 				 b_res, &bus->busn_res);
-		b_res->flags = 0;
+		b_res->flags |= IORESOURCE_DISABLED;
 		return;
 	}
 
 	resource_set_range(b_res, min_align, size0);
 	b_res->flags |= IORESOURCE_STARTALIGN;
 	if (bus->self && size1 > size0 && realloc_head) {
+		b_res->flags &= ~IORESOURCE_DISABLED;
 		add_to_list(realloc_head, bus->self, b_res, size1-size0,
 			    min_align);
 		pci_info(bus->self, "bridge window %pR to %pR add_size %llx\n",
@@ -1198,11 +1234,13 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			const char *r_name = pci_resource_name(dev, i);
 			resource_size_t r_size;
 
-			if (r->parent || (r->flags & IORESOURCE_PCI_FIXED) ||
-			    !pdev_resources_assignable(dev) ||
-			    ((r->flags & mask) != type &&
-			     (r->flags & mask) != type2 &&
-			     (r->flags & mask) != type3))
+			if (!pdev_resources_assignable(dev) ||
+			    !pdev_resource_should_fit(dev, r))
+				continue;
+
+			if ((r->flags & mask) != type &&
+			    (r->flags & mask) != type2 &&
+			    (r->flags & mask) != type3)
 				continue;
 			r_size = resource_size(r);
 
@@ -1253,6 +1291,9 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 	min_align = max(min_align, win_align);
 	size0 = calculate_memsize(size, min_size, 0, 0, resource_size(b_res), min_align);
 
+	if (size0)
+		b_res->flags &= ~IORESOURCE_DISABLED;
+
 	if (bus->self && size0 &&
 	    !pbus_upstream_space_available(bus, mask | IORESOURCE_PREFETCH, type,
 					   size0, min_align)) {
@@ -1287,13 +1328,14 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 		if (bus->self && (b_res->start || b_res->end))
 			pci_info(bus->self, "disabling bridge window %pR to %pR (unused)\n",
 				 b_res, &bus->busn_res);
-		b_res->flags = 0;
+		b_res->flags |= IORESOURCE_DISABLED;
 		return 0;
 	}
 
 	resource_set_range(b_res, min_align, size0);
 	b_res->flags |= IORESOURCE_STARTALIGN;
 	if (bus->self && size1 > size0 && realloc_head) {
+		b_res->flags &= ~IORESOURCE_DISABLED;
 		add_to_list(realloc_head, bus->self, b_res, size1-size0, add_align);
 		pci_info(bus->self, "bridge window %pR to %pR add_size %llx add_align %llx\n",
 			   b_res, &bus->busn_res,
@@ -1721,7 +1763,6 @@ static void pci_bridge_release_resources(struct pci_bus *bus,
 {
 	struct pci_dev *dev = bus->self;
 	struct resource *r;
-	unsigned int old_flags;
 	struct resource *b_res;
 	int idx, ret;
 
@@ -1758,17 +1799,15 @@ static void pci_bridge_release_resources(struct pci_bus *bus,
 	/* If there are children, release them all */
 	release_child_resources(r);
 
-	type = old_flags = r->flags & PCI_RES_TYPE_MASK;
 	ret = pci_release_resource(dev, PCI_BRIDGE_RESOURCES + idx);
 	if (ret)
 		return;
 
+	type = r->flags & PCI_RES_TYPE_MASK;
 	/* Avoiding touch the one without PREF */
 	if (type & IORESOURCE_PREFETCH)
 		type = IORESOURCE_PREFETCH;
 	__pci_setup_bridge(bus, type);
-	/* For next child res under same bridge */
-	r->flags = old_flags;
 }
 
 enum release_type {
@@ -2246,21 +2285,9 @@ static void pci_prepare_next_assign_round(struct list_head *fail_head,
 	}
 
 	/* Restore size and flags */
-	list_for_each_entry(fail_res, fail_head, list) {
-		struct resource *res = fail_res->res;
-		struct pci_dev *dev = fail_res->dev;
-		int idx = pci_resource_num(dev, res);
-
+	list_for_each_entry(fail_res, fail_head, list)
 		restore_dev_resource(fail_res);
 
-		if (!pci_is_bridge(dev))
-			continue;
-
-		if (idx >= PCI_BRIDGE_RESOURCES &&
-		    idx <= PCI_BRIDGE_RESOURCE_END)
-			res->flags = 0;
-	}
-
 	free_list(fail_head);
 }
 
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 4e0e60256f04..21f77e5c647c 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -359,6 +359,9 @@ int pci_assign_resource(struct pci_dev *dev, int resno)
 
 	res->flags &= ~IORESOURCE_UNSET;
 	res->flags &= ~IORESOURCE_STARTALIGN;
+	if (resno >= PCI_BRIDGE_RESOURCES && resno <= PCI_BRIDGE_RESOURCE_END)
+		res->flags &= ~IORESOURCE_DISABLED;
+
 	pci_info(dev, "%s %pR: assigned\n", res_name, res);
 	if (resno < PCI_BRIDGE_RESOURCES)
 		pci_update_resource(dev, resno);
-- 
2.39.5

[PATCH v2 11/24] PCI: Add defines for bridge window indexing

[Ilpo Järvinen]

include/linux/pci.h provides PCI_BRIDGE_{IO,MEM,PREF_MEM}_WINDOW defines,
however, they're based on the resource array indexing in the pci_dev
struct. The struct pci_bus also has pointers to those same resources but
they start from zeroth index.

Add PCI_BUS_BRIDGE_{IO,MEM,PREF_MEM}_WINDOW defines to get rid of literal
indexing.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/pci.h   |  4 ++++
 drivers/pci/probe.c | 10 +++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 34f65d69662e..1dc8a8066761 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -81,6 +81,10 @@ struct pcie_tlp_log;
 #define PCIE_MSG_CODE_DEASSERT_INTC	0x26
 #define PCIE_MSG_CODE_DEASSERT_INTD	0x27
 
+#define PCI_BUS_BRIDGE_IO_WINDOW	0
+#define PCI_BUS_BRIDGE_MEM_WINDOW	1
+#define PCI_BUS_BRIDGE_PREF_MEM_WINDOW	2
+
 extern const unsigned char pcie_link_speed[];
 extern bool pci_early_dump;
 
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index f31d27c7708a..eaeb66bec433 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -598,9 +598,13 @@ void pci_read_bridge_bases(struct pci_bus *child)
 	for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; i++)
 		child->resource[i] = &dev->resource[PCI_BRIDGE_RESOURCES+i];
 
-	pci_read_bridge_io(child->self, child->resource[0], false);
-	pci_read_bridge_mmio(child->self, child->resource[1], false);
-	pci_read_bridge_mmio_pref(child->self, child->resource[2], false);
+	pci_read_bridge_io(child->self,
+			   child->resource[PCI_BUS_BRIDGE_IO_WINDOW], false);
+	pci_read_bridge_mmio(child->self,
+			     child->resource[PCI_BUS_BRIDGE_MEM_WINDOW], false);
+	pci_read_bridge_mmio_pref(child->self,
+				  child->resource[PCI_BUS_BRIDGE_PREF_MEM_WINDOW],
+				  false);
 
 	if (!dev->transparent)
 		return;
-- 
2.39.5

[PATCH v2 12/24] PCI: Add bridge window selection functions

[Ilpo Järvinen]

Various places in the PCI core code independently decide into which bridge
window a child resource should be placed. It is hard to see whether these
decisions always end up in agreement, especially in the corner cases, and
in some places it requires complex logic to pass multiple resource types
and/or bridge windows around.

Add pbus_select_window() and pbus_select_window_for_type() for cases where
the former cannot be used so that eventually the same helper can be used to
select the bridge window everywhere. Using the same function ensures the
selected bridge window remains always the same and it can be easily
recalculated in-situ allowing simplifying the interfaces between internal
functions in upcoming changes.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/pci.h       |   2 +
 drivers/pci/setup-bus.c | 101 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 1dc8a8066761..cbd40f05c39c 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -385,6 +385,8 @@ static inline int pci_resource_num(const struct pci_dev *dev,
 	return resno;
 }
 
+struct resource *pbus_select_window(struct pci_bus *bus,
+				    const struct resource *res);
 void pci_reassigndev_resource_alignment(struct pci_dev *dev);
 void pci_disable_bridge_window(struct pci_dev *dev);
 struct pci_bus *pci_bus_get(struct pci_bus *bus);
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 70b210ed200d..913fd41e1d0d 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -172,6 +172,107 @@ static struct resource *find_bus_resource_of_type(struct pci_bus *bus,
 	return r_assigned;
 }
 
+/**
+ * pbus_select_window_for_type - Select bridge window for a resource type
+ * @bus: PCI bus
+ * @type: Resource type (resource flags can be passed as is)
+ *
+ * Select the bridge window based on a resource @type.
+ *
+ * For memory resources, the selection is done as follows:
+ *
+ * Any non-prefetchable resource is put into the non-prefetchable window.
+ *
+ * If there is no prefetchable MMIO window, put all memory resources into the
+ * non-prefetchable window.
+ *
+ * If there's a 64-bit prefetchable MMIO window, put all 64-bit prefetchable
+ * resources into it and place 32-bit prefetchable memory into the
+ * non-prefetchable window.
+ *
+ * Otherwise, put all prefetchable resources into the prefetchable window.
+ *
+ * Return: the bridge window resource or NULL if no bridge window is found.
+ */
+static struct resource *pbus_select_window_for_type(struct pci_bus *bus,
+						    unsigned long type)
+{
+	int iores_type = type & IORESOURCE_TYPE_BITS;	/* w/o 64bit & pref */
+	struct resource *mmio, *mmio_pref, *win;
+
+	type &= PCI_RES_TYPE_MASK;			/* with 64bit & pref */
+
+	if ((iores_type != IORESOURCE_IO) && (iores_type != IORESOURCE_MEM))
+		return NULL;
+
+	if (pci_is_root_bus(bus)) {
+		win = find_bus_resource_of_type(bus, type, type);
+		if (win)
+			return win;
+
+		type &= ~IORESOURCE_MEM_64;
+		win = find_bus_resource_of_type(bus, type, type);
+		if (win)
+			return win;
+
+		type &= ~IORESOURCE_PREFETCH;
+		return find_bus_resource_of_type(bus, type, type);
+	}
+
+	switch (iores_type) {
+	case IORESOURCE_IO:
+		return pci_bus_resource_n(bus, PCI_BUS_BRIDGE_IO_WINDOW);
+
+	case IORESOURCE_MEM:
+		mmio = pci_bus_resource_n(bus, PCI_BUS_BRIDGE_MEM_WINDOW);
+		mmio_pref = pci_bus_resource_n(bus, PCI_BUS_BRIDGE_PREF_MEM_WINDOW);
+
+		if (!(type & IORESOURCE_PREFETCH) ||
+		    !(mmio_pref->flags & IORESOURCE_MEM))
+			return mmio;
+
+		if ((type & IORESOURCE_MEM_64) ||
+		    !(mmio_pref->flags & IORESOURCE_MEM_64))
+			return mmio_pref;
+
+		return mmio;
+	default:
+		return NULL;
+	}
+}
+
+/**
+ * pbus_select_window - Select bridge window for a resource
+ * @bus: PCI bus
+ * @res: Resource
+ *
+ * Select the bridge window for @res. If the resource is already assigned,
+ * return the current bridge window.
+ *
+ * For memory resources, the selection is done as follows:
+ *
+ * Any non-prefetchable resource is put into the non-prefetchable window.
+ *
+ * If there is no prefetchable MMIO window, put all memory resources into the
+ * non-prefetchable window.
+ *
+ * If there's a 64-bit prefetchable MMIO window, put all 64-bit prefetchable
+ * resources into it and place 32-bit prefetchable memory into the
+ * non-prefetchable window.
+ *
+ * Otherwise, put all prefetchable resources into the prefetchable window.
+ *
+ * Return: the bridge window resource or NULL if no bridge window is found.
+ */
+struct resource *pbus_select_window(struct pci_bus *bus,
+				    const struct resource *res)
+{
+	if (res->parent)
+		return res->parent;
+
+	return pbus_select_window_for_type(bus, res->flags);
+}
+
 static bool pdev_resources_assignable(struct pci_dev *dev)
 {
 	u16 class = dev->class >> 8, command;
-- 
2.39.5

[PATCH v2 13/24] PCI: Fix finding bridge window in pci_reassign_bridge_resources()

[Ilpo Järvinen]

pci_reassign_bridge_resources() walks upwards in the PCI bus hierarchy,
locates the relevant bridge window on each level using flags check, and
attempts to release the bridge window. The flags-based check is fragile due
to various fallbacks in the bridge window selection logic. As such, the
algorithm might not locate the correct bridge window.

Refactor pci_reassign_bridge_resources() to determine the correct bridge
window using pbus_select_window(), which contains logic to handle all
fallback cases correctly. Change function prefix to pbus as it now inputs
struct bus and resource for which to locate the bridge window.

The main purpose is to make bridge window selection logic consistent across
the entire PCI core (one step at a time). While this technically also fixes
the commit 8bb705e3e79d ("PCI: Add pci_resize_resource() for resizing
BARs") making the bridge window walk algorithm more robust, the normal
setup having a 64-bit resizable BAR underneath bridge(s) with 64-bit
prefetchable windows does not need to use any fallbacks. As such, the
practical impact is low (requiring BAR resize use case and a non-typical
bridge device).

The way to detect if unrelated resource failed again is left to use the
type based approximation which should not behave worse than before.

Fixes: 8bb705e3e79d ("PCI: Add pci_resize_resource() for resizing BARs")
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/pci.h       |  2 +-
 drivers/pci/setup-bus.c | 38 ++++++++++++++++++--------------------
 drivers/pci/setup-res.c |  2 +-
 3 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index cbd40f05c39c..0d96a9141227 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -334,7 +334,7 @@ struct device *pci_get_host_bridge_device(struct pci_dev *dev);
 void pci_put_host_bridge_device(struct device *dev);
 
 unsigned int pci_rescan_bus_bridge_resize(struct pci_dev *bridge);
-int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type);
+int pbus_reassign_bridge_resources(struct pci_bus *bus, struct resource *res);
 int __must_check pci_reassign_resource(struct pci_dev *dev, int i, resource_size_t add_size, resource_size_t align);
 
 int pci_configure_extended_tags(struct pci_dev *dev, void *ign);
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 913fd41e1d0d..4b08b9cecab3 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -2522,10 +2522,16 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
 }
 EXPORT_SYMBOL_GPL(pci_assign_unassigned_bridge_resources);
 
-int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
+/*
+ * Walk to the root bus, find the bridge window relevant for @res and
+ * release it when possible. If the bridge window contains assigned
+ * resources, it cannot be released.
+ */
+int pbus_reassign_bridge_resources(struct pci_bus *bus, struct resource *res)
 {
+	unsigned long type = res->flags;
 	struct pci_dev_resource *dev_res;
-	struct pci_dev *next;
+	struct pci_dev *bridge;
 	LIST_HEAD(saved);
 	LIST_HEAD(added);
 	LIST_HEAD(failed);
@@ -2534,33 +2540,25 @@ int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
 
 	down_read(&pci_bus_sem);
 
-	/* Walk to the root hub, releasing bridge BARs when possible */
-	next = bridge;
-	do {
-		bridge = next;
-		for (i = PCI_BRIDGE_RESOURCES; i < PCI_BRIDGE_RESOURCE_END;
-		     i++) {
-			struct resource *res = &bridge->resource[i];
-
-			if ((res->flags ^ type) & PCI_RES_TYPE_MASK)
-				continue;
+	while (!pci_is_root_bus(bus)) {
+		bridge = bus->self;
+		res = pbus_select_window(bus, res);
+		if (!res)
+			break;
 
-			/* Ignore BARs which are still in use */
-			if (res->child)
-				continue;
+		i = pci_resource_num(bridge, res);
 
+		/* Ignore BARs which are still in use */
+		if (!res->child) {
 			ret = add_to_list(&saved, bridge, res, 0, 0);
 			if (ret)
 				goto cleanup;
 
 			pci_release_resource(bridge, i);
-			break;
 		}
-		if (i == PCI_BRIDGE_RESOURCE_END)
-			break;
 
-		next = bridge->bus ? bridge->bus->self : NULL;
-	} while (next);
+		bus = bus->parent;
+	}
 
 	if (list_empty(&saved)) {
 		up_read(&pci_bus_sem);
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 21f77e5c647c..c3ba4ccecd43 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -496,7 +496,7 @@ int pci_resize_resource(struct pci_dev *dev, int resno, int size)
 
 	/* Check if the new config works by trying to assign everything. */
 	if (dev->bus->self) {
-		ret = pci_reassign_bridge_resources(dev->bus->self, res->flags);
+		ret = pbus_reassign_bridge_resources(dev->bus, res);
 		if (ret)
 			goto error_resize;
 	}
-- 
2.39.5

[PATCH v2 14/24] PCI: Warn if bridge window cannot be released when resizing BAR

[Ilpo Järvinen]

BAR resizing calls to pci_reassign_bridge_resources(), which attempts to
release any upstream bridge window to allow them to accommodate the new BAR
size. The release can only be performed if there are no other child
resources for the bridge window. Previously the code continued silently
when other child resources were detected.

Add pci_warn() to inform user that a bridge window could not be released
because of child resources. As a small bridge window is often the reason
why BAR resize fails, this warning will help to pinpoint to the cause.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/setup-bus.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 4b08b9cecab3..47f1a4747607 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -2555,6 +2555,12 @@ int pbus_reassign_bridge_resources(struct pci_bus *bus, struct resource *res)
 				goto cleanup;
 
 			pci_release_resource(bridge, i);
+		} else {
+			const char *res_name = pci_resource_name(bridge, i);
+
+			pci_warn(bridge,
+				 "%s %pR: was not released (still contains assigned resources)\n",
+				 res_name, res);
 		}
 
 		bus = bus->parent;
-- 
2.39.5

[PATCH v2 15/24] PCI: Use pbus_select_window() during BAR resize

[Ilpo Järvinen]

Prior to a BAR resize, __resource_resize_store() loops through the normal
resources of the PCI device and releases those that match to the flags of
the BAR to be resized. This is necessary to allow resizing also the
upstream bridge window as only childless bridge windows can be resized.

While the flags check (mostly) works (if corner cases are ignored), the
more straightforward way is to check if the resources share the bridge
window. Change __resource_resize_store() to do the check using
pbus_select_window().

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/pci-sysfs.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 162a5241c7f7..ce3923c4aa80 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1562,13 +1562,19 @@ static ssize_t __resource_resize_store(struct device *dev, int n,
 				       const char *buf, size_t count)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
-	unsigned long size, flags;
+	struct pci_bus *bus = pdev->bus;
+	struct resource *b_win, *res;
+	unsigned long size;
 	int ret, i;
 	u16 cmd;
 
 	if (kstrtoul(buf, 0, &size) < 0)
 		return -EINVAL;
 
+	b_win = pbus_select_window(bus, pci_resource_n(pdev, n));
+	if (!b_win)
+		return -EINVAL;
+
 	device_lock(dev);
 	if (dev->driver || pci_num_vf(pdev)) {
 		ret = -EBUSY;
@@ -1588,19 +1594,19 @@ static ssize_t __resource_resize_store(struct device *dev, int n,
 	pci_write_config_word(pdev, PCI_COMMAND,
 			      cmd & ~PCI_COMMAND_MEMORY);
 
-	flags = pci_resource_flags(pdev, n);
-
 	pci_remove_resource_files(pdev);
 
-	for (i = 0; i < PCI_BRIDGE_RESOURCES; i++) {
-		if (pci_resource_len(pdev, i) &&
-		    pci_resource_flags(pdev, i) == flags)
+	pci_dev_for_each_resource(pdev, res, i) {
+		if (i >= PCI_BRIDGE_RESOURCES)
+			break;
+
+		if (b_win == pbus_select_window(bus, res))
 			pci_release_resource(pdev, i);
 	}
 
 	ret = pci_resize_resource(pdev, n, size);
 
-	pci_assign_unassigned_bus_resources(pdev->bus);
+	pci_assign_unassigned_bus_resources(bus);
 
 	if (pci_create_resource_files(pdev))
 		pci_warn(pdev, "Failed to recreate resource files after BAR resizing\n");
-- 
2.39.5

[PATCH v2 16/24] PCI: Use pbus_select_window_for_type() during IO window sizing

[Ilpo Järvinen]

Convert pbus_size_io() to use pbus_select_window_for_type().

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/setup-bus.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 47f1a4747607..a21d6367e525 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1122,8 +1122,7 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size,
 			 struct list_head *realloc_head)
 {
 	struct pci_dev *dev;
-	struct resource *b_res = find_bus_resource_of_type(bus, IORESOURCE_IO,
-							   IORESOURCE_IO);
+	struct resource *b_res = pbus_select_window_for_type(bus, IORESOURCE_IO);
 	resource_size_t size = 0, size0 = 0, size1 = 0;
 	resource_size_t children_add_size = 0;
 	resource_size_t min_align, align;
-- 
2.39.5

[PATCH v2 17/24] PCI: Rename resource variable from r to res

[Ilpo Järvinen]

Resource is going to be passed in as argument aften an upcoming change.
Rename the struct resource variable from "r" to "res" to avoid using one
letter variable name in a function argument.

This rename is made separately to reduce churn in the upcoming change.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/setup-bus.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index a21d6367e525..5ec446c2b779 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1241,24 +1241,24 @@ static bool pbus_upstream_space_available(struct pci_bus *bus, unsigned long mas
 		.align = align,
 	};
 	struct pci_bus *downstream = bus;
-	struct resource *r;
+	struct resource *res;
 
 	while ((bus = bus->parent)) {
 		if (pci_is_root_bus(bus))
 			break;
 
-		pci_bus_for_each_resource(bus, r) {
-			if (!r || !r->parent || (r->flags & mask) != type)
+		pci_bus_for_each_resource(bus, res) {
+			if (!res || !res->parent || (res->flags & mask) != type)
 				continue;
 
-			if (resource_size(r) >= size) {
+			if (resource_size(res) >= size) {
 				struct resource gap = {};
 
-				if (find_resource_space(r, &gap, size, &constraint) == 0) {
+				if (find_resource_space(res, &gap, size, &constraint) == 0) {
 					gap.flags = type;
 					pci_dbg(bus->self,
 						"Assigned bridge window %pR to %pR free space at %pR\n",
-						r, &bus->busn_res, &gap);
+						res, &bus->busn_res, &gap);
 					return true;
 				}
 			}
@@ -1266,7 +1266,7 @@ static bool pbus_upstream_space_available(struct pci_bus *bus, unsigned long mas
 			if (bus->self) {
 				pci_info(bus->self,
 					 "Assigned bridge window %pR to %pR cannot fit 0x%llx required for %s bridging to %pR\n",
-					 r, &bus->busn_res,
+					 res, &bus->busn_res,
 					 (unsigned long long)size,
 					 pci_name(downstream->self),
 					 &downstream->busn_res);
-- 
2.39.5

[PATCH v2 18/24] PCI: Use pbus_select_window() in space available checker

[Ilpo Järvinen]

pbus_upstream_space_available() figures out the upstream bridge window
resources on its own. Migrate it to use pbus_select_window().

Note: pbus_select_window() -> pbus_select_window_for_type() calls
find_bus_resource_of_type() for root bus, which does not do parent check
similar to what pbus_upstream_space_available() did earlier, but the
difference does not matter because pbus_upstream_space_available() itself
stops when it encounters the root bus.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/setup-bus.c | 65 ++++++++++++++++++++---------------------
 1 file changed, 32 insertions(+), 33 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 5ec446c2b779..865bacae9cac 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1221,19 +1221,20 @@ static inline resource_size_t calculate_mem_align(resource_size_t *aligns,
 /**
  * pbus_upstream_space_available - Check no upstream resource limits allocation
  * @bus:	The bus
- * @mask:	Mask the resource flag, then compare it with type
- * @type:	The type of resource from bridge
+ * @res:	The resource to help select the correct bridge window
  * @size:	The size required from the bridge window
  * @align:	Required alignment for the resource
  *
- * Checks that @size can fit inside the upstream bridge resources that are
- * already assigned.
+ * Check that @size can fit inside the upstream bridge resources that are
+ * already assigned. Select the upstream bridge window based on the type of
+ * @res.
  *
  * Return: %true if enough space is available on all assigned upstream
  * resources.
  */
-static bool pbus_upstream_space_available(struct pci_bus *bus, unsigned long mask,
-					  unsigned long type, resource_size_t size,
+static bool pbus_upstream_space_available(struct pci_bus *bus,
+					  struct resource *res,
+					  resource_size_t size,
 					  resource_size_t align)
 {
 	struct resource_constraint constraint = {
@@ -1241,39 +1242,39 @@ static bool pbus_upstream_space_available(struct pci_bus *bus, unsigned long mas
 		.align = align,
 	};
 	struct pci_bus *downstream = bus;
-	struct resource *res;
 
 	while ((bus = bus->parent)) {
 		if (pci_is_root_bus(bus))
 			break;
 
-		pci_bus_for_each_resource(bus, res) {
-			if (!res || !res->parent || (res->flags & mask) != type)
-				continue;
-
-			if (resource_size(res) >= size) {
-				struct resource gap = {};
+		res = pbus_select_window(bus, res);
+		if (!res)
+			return false;
+		if (!res->parent)
+			continue;
 
-				if (find_resource_space(res, &gap, size, &constraint) == 0) {
-					gap.flags = type;
-					pci_dbg(bus->self,
-						"Assigned bridge window %pR to %pR free space at %pR\n",
-						res, &bus->busn_res, &gap);
-					return true;
-				}
-			}
+		if (resource_size(res) >= size) {
+			struct resource gap = {};
 
-			if (bus->self) {
-				pci_info(bus->self,
-					 "Assigned bridge window %pR to %pR cannot fit 0x%llx required for %s bridging to %pR\n",
-					 res, &bus->busn_res,
-					 (unsigned long long)size,
-					 pci_name(downstream->self),
-					 &downstream->busn_res);
+			if (find_resource_space(res, &gap, size, &constraint) == 0) {
+				gap.flags = res->flags;
+				pci_dbg(bus->self,
+					"Assigned bridge window %pR to %pR free space at %pR\n",
+					res, &bus->busn_res, &gap);
+				return true;
 			}
+		}
 
-			return false;
+		if (bus->self) {
+			pci_info(bus->self,
+				 "Assigned bridge window %pR to %pR cannot fit 0x%llx required for %s bridging to %pR\n",
+				 res, &bus->busn_res,
+				 (unsigned long long)size,
+				 pci_name(downstream->self),
+				 &downstream->busn_res);
 		}
+
+		return false;
 	}
 
 	return true;
@@ -1395,8 +1396,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 		b_res->flags &= ~IORESOURCE_DISABLED;
 
 	if (bus->self && size0 &&
-	    !pbus_upstream_space_available(bus, mask | IORESOURCE_PREFETCH, type,
-					   size0, min_align)) {
+	    !pbus_upstream_space_available(bus, b_res, size0, min_align)) {
 		relaxed_align = 1ULL << (max_order + __ffs(SZ_1M));
 		relaxed_align = max(relaxed_align, win_align);
 		min_align = min(min_align, relaxed_align);
@@ -1411,8 +1411,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 					  resource_size(b_res), add_align);
 
 		if (bus->self && size1 &&
-		    !pbus_upstream_space_available(bus, mask | IORESOURCE_PREFETCH, type,
-						   size1, add_align)) {
+		    !pbus_upstream_space_available(bus, b_res, size1, add_align)) {
 			relaxed_align = 1ULL << (max_order + __ffs(SZ_1M));
 			relaxed_align = max(relaxed_align, win_align);
 			min_align = min(min_align, relaxed_align);
-- 
2.39.5

[PATCH v2 19/24] PCI: Use pbus_select_window_for_type() during mem window sizing

[Ilpo Järvinen]

__pci_bus_size_bridges() goes to great lengths of helping pbus_size_mem()
in which types it should put into a particular bridge window, requiring
passing up to three resource type into pbus_size_mem().

Instead of having complex logic in __pci_bus_size_bridges() and a
non-straightforward interface between those functions, use
pbus_select_window_for_type() and pbus_select_window() to find the correct
bridge window and compare if the resources belong to that window.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/setup-bus.c | 111 +++++++++-------------------------------
 1 file changed, 24 insertions(+), 87 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 865bacae9cac..720159bca54d 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1284,24 +1284,22 @@ static bool pbus_upstream_space_available(struct pci_bus *bus,
  * pbus_size_mem() - Size the memory window of a given bus
  *
  * @bus:		The bus
- * @mask:		Mask the resource flag, then compare it with type
- * @type:		The type of free resource from bridge
- * @type2:		Second match type
- * @type3:		Third match type
+ * @type:		The type of bridge resource
  * @min_size:		The minimum memory window that must be allocated
  * @add_size:		Additional optional memory window
  * @realloc_head:	Track the additional memory window on this list
  *
- * Calculate the size of the bus and minimal alignment which guarantees
- * that all child resources fit in this size.
+ * Calculate the size of the bus resource for @type and minimal alignment
+ * which guarantees that all child resources fit in this size.
  *
- * Return -ENOSPC if there's no available bus resource of the desired
- * type.  Otherwise, set the bus resource start/end to indicate the
- * required size, add things to realloc_head (if supplied), and return 0.
+ * Set the bus resource start/end to indicate the required size if there an
+ * available unassigned bus resource of the desired @type.
+ *
+ * Add optional resource requests to the @realloc_head list if it is
+ * supplied.
  */
-static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
-			 unsigned long type, unsigned long type2,
-			 unsigned long type3, resource_size_t min_size,
+static void pbus_size_mem(struct pci_bus *bus, unsigned long type,
+			 resource_size_t min_size,
 			 resource_size_t add_size,
 			 struct list_head *realloc_head)
 {
@@ -1309,19 +1307,18 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 	resource_size_t min_align, win_align, align, size, size0, size1 = 0;
 	resource_size_t aligns[28]; /* Alignments from 1MB to 128TB */
 	int order, max_order;
-	struct resource *b_res = find_bus_resource_of_type(bus,
-					mask | IORESOURCE_PREFETCH, type);
+	struct resource *b_res = pbus_select_window_for_type(bus, type);
 	resource_size_t children_add_size = 0;
 	resource_size_t children_add_align = 0;
 	resource_size_t add_align = 0;
 	resource_size_t relaxed_align;
 
 	if (!b_res)
-		return -ENOSPC;
+		return;
 
 	/* If resource is already assigned, nothing more to do */
 	if (b_res->parent)
-		return 0;
+		return;
 
 	memset(aligns, 0, sizeof(aligns));
 	max_order = 0;
@@ -1338,11 +1335,9 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			if (!pdev_resources_assignable(dev) ||
 			    !pdev_resource_should_fit(dev, r))
 				continue;
-
-			if ((r->flags & mask) != type &&
-			    (r->flags & mask) != type2 &&
-			    (r->flags & mask) != type3)
+			if (b_res != pbus_select_window(bus, r))
 				continue;
+
 			r_size = resource_size(r);
 
 			/* Put SRIOV requested res to the optional list */
@@ -1428,7 +1423,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			pci_info(bus->self, "disabling bridge window %pR to %pR (unused)\n",
 				 b_res, &bus->busn_res);
 		b_res->flags |= IORESOURCE_DISABLED;
-		return 0;
+		return;
 	}
 
 	resource_set_range(b_res, min_align, size0);
@@ -1441,7 +1436,6 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			   (unsigned long long) (size1 - size0),
 			   (unsigned long long) add_align);
 	}
-	return 0;
 }
 
 unsigned long pci_cardbus_resource_alignment(struct resource *res)
@@ -1546,12 +1540,11 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
 void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
 {
 	struct pci_dev *dev;
-	unsigned long mask, prefmask, type2 = 0, type3 = 0;
 	resource_size_t additional_io_size = 0, additional_mmio_size = 0,
 			additional_mmio_pref_size = 0;
 	struct resource *pref;
 	struct pci_host_bridge *host;
-	int hdr_type, ret;
+	int hdr_type;
 
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		struct pci_bus *b = dev->subordinate;
@@ -1601,71 +1594,15 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
 		pbus_size_io(bus, realloc_head ? 0 : additional_io_size,
 			     additional_io_size, realloc_head);
 
-		/*
-		 * If there's a 64-bit prefetchable MMIO window, compute
-		 * the size required to put all 64-bit prefetchable
-		 * resources in it.
-		 */
-		mask = IORESOURCE_MEM;
-		prefmask = IORESOURCE_MEM | IORESOURCE_PREFETCH;
-		if (pref && (pref->flags & IORESOURCE_MEM_64)) {
-			prefmask |= IORESOURCE_MEM_64;
-			ret = pbus_size_mem(bus, prefmask, prefmask,
-				prefmask, prefmask,
-				realloc_head ? 0 : additional_mmio_pref_size,
-				additional_mmio_pref_size, realloc_head);
-
-			/*
-			 * If successful, all non-prefetchable resources
-			 * and any 32-bit prefetchable resources will go in
-			 * the non-prefetchable window.
-			 */
-			if (ret == 0) {
-				mask = prefmask;
-				type2 = prefmask & ~IORESOURCE_MEM_64;
-				type3 = prefmask & ~IORESOURCE_PREFETCH;
-			}
-		}
-
-		/*
-		 * If there is no 64-bit prefetchable window, compute the
-		 * size required to put all prefetchable resources in the
-		 * 32-bit prefetchable window (if there is one).
-		 */
-		if (!type2) {
-			prefmask &= ~IORESOURCE_MEM_64;
-			ret = pbus_size_mem(bus, prefmask, prefmask,
-				prefmask, prefmask,
-				realloc_head ? 0 : additional_mmio_pref_size,
-				additional_mmio_pref_size, realloc_head);
-
-			/*
-			 * If successful, only non-prefetchable resources
-			 * will go in the non-prefetchable window.
-			 */
-			if (ret == 0)
-				mask = prefmask;
-			else
-				additional_mmio_size += additional_mmio_pref_size;
-
-			type2 = type3 = IORESOURCE_MEM;
+		if (pref) {
+			pbus_size_mem(bus,
+				      IORESOURCE_MEM | IORESOURCE_PREFETCH |
+				      (pref->flags & IORESOURCE_MEM_64),
+				      realloc_head ? 0 : additional_mmio_pref_size,
+				      additional_mmio_pref_size, realloc_head);
 		}
 
-		/*
-		 * Compute the size required to put everything else in the
-		 * non-prefetchable window. This includes:
-		 *
-		 *   - all non-prefetchable resources
-		 *   - 32-bit prefetchable resources if there's a 64-bit
-		 *     prefetchable window or no prefetchable window at all
-		 *   - 64-bit prefetchable resources if there's no prefetchable
-		 *     window at all
-		 *
-		 * Note that the strategy in __pci_assign_resource() must match
-		 * that used here. Specifically, we cannot put a 32-bit
-		 * prefetchable resource in a 64-bit prefetchable window.
-		 */
-		pbus_size_mem(bus, mask, IORESOURCE_MEM, type2, type3,
+		pbus_size_mem(bus, IORESOURCE_MEM,
 			      realloc_head ? 0 : additional_mmio_size,
 			      additional_mmio_size, realloc_head);
 		break;
-- 
2.39.5

[PATCH v2 20/24] PCI: Refactor distributing available memory to use loops

[Ilpo Järvinen]

pci_bus_distribute_available_resources() and
pci_bridge_distribute_available_resources() retain bridge window resources
and related data needed for distributing the available window in
independent variables for io, memory, and prefetchable memory windows. The
code is essentially the same for all of them and therefore repeated three
times with different variable names.

Refactor pci_bus_distribute_available_resources() to take an array. This
is complicated slightly by the function taking advantage of passing the
struct as value, which cannot be done for arrays in C. Therefore, copy the
data into a local array in the stack in the first loop.

Variable names are (hopefully) improved slightly as well.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/setup-bus.c | 162 ++++++++++++++++++----------------------
 include/linux/pci.h     |   3 +-
 2 files changed, 74 insertions(+), 91 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 720159bca54d..3bc329b1b923 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -2059,15 +2059,16 @@ static void remove_dev_resource(struct resource *avail, struct pci_dev *dev,
 	avail->start = min(avail->start + tmp, avail->end + 1);
 }
 
-static void remove_dev_resources(struct pci_dev *dev, struct resource *io,
-				 struct resource *mmio,
-				 struct resource *mmio_pref)
+static void remove_dev_resources(struct pci_dev *dev,
+				 struct resource available[PCI_P2P_BRIDGE_RESOURCE_NUM])
 {
+	struct resource *mmio_pref = &available[PCI_BUS_BRIDGE_PREF_MEM_WINDOW];
 	struct resource *res;
 
 	pci_dev_for_each_resource(dev, res) {
 		if (resource_type(res) == IORESOURCE_IO) {
-			remove_dev_resource(io, dev, res);
+			remove_dev_resource(&available[PCI_BUS_BRIDGE_IO_WINDOW],
+					    dev, res);
 		} else if (resource_type(res) == IORESOURCE_MEM) {
 
 			/*
@@ -2081,10 +2082,13 @@ static void remove_dev_resources(struct pci_dev *dev, struct resource *io,
 			 */
 			if ((res->flags & IORESOURCE_PREFETCH) &&
 			    ((res->flags & IORESOURCE_MEM_64) ==
-			     (mmio_pref->flags & IORESOURCE_MEM_64)))
-				remove_dev_resource(mmio_pref, dev, res);
-			else
-				remove_dev_resource(mmio, dev, res);
+			     (mmio_pref->flags & IORESOURCE_MEM_64))) {
+				remove_dev_resource(&available[PCI_BUS_BRIDGE_PREF_MEM_WINDOW],
+						    dev, res);
+			} else {
+				remove_dev_resource(&available[PCI_BUS_BRIDGE_MEM_WINDOW],
+						    dev, res);
+			}
 		}
 	}
 }
@@ -2099,45 +2103,39 @@ static void remove_dev_resources(struct pci_dev *dev, struct resource *io,
  * shared with the bridges.
  */
 static void pci_bus_distribute_available_resources(struct pci_bus *bus,
-					    struct list_head *add_list,
-					    struct resource io,
-					    struct resource mmio,
-					    struct resource mmio_pref)
+		    struct list_head *add_list,
+		    struct resource available_in[PCI_P2P_BRIDGE_RESOURCE_NUM])
 {
+	struct resource available[PCI_P2P_BRIDGE_RESOURCE_NUM];
 	unsigned int normal_bridges = 0, hotplug_bridges = 0;
-	struct resource *io_res, *mmio_res, *mmio_pref_res;
 	struct pci_dev *dev, *bridge = bus->self;
-	resource_size_t io_per_b, mmio_per_b, mmio_pref_per_b, align;
+	resource_size_t per_bridge[PCI_P2P_BRIDGE_RESOURCE_NUM];
+	resource_size_t align;
+	int i;
 
-	io_res = &bridge->resource[PCI_BRIDGE_IO_WINDOW];
-	mmio_res = &bridge->resource[PCI_BRIDGE_MEM_WINDOW];
-	mmio_pref_res = &bridge->resource[PCI_BRIDGE_PREF_MEM_WINDOW];
+	for (i = 0; i < PCI_P2P_BRIDGE_RESOURCE_NUM; i++) {
+		struct resource *res = pci_bus_resource_n(bus, i);
 
-	/*
-	 * The alignment of this bridge is yet to be considered, hence it must
-	 * be done now before extending its bridge window.
-	 */
-	align = pci_resource_alignment(bridge, io_res);
-	if (!io_res->parent && align)
-		io.start = min(ALIGN(io.start, align), io.end + 1);
-
-	align = pci_resource_alignment(bridge, mmio_res);
-	if (!mmio_res->parent && align)
-		mmio.start = min(ALIGN(mmio.start, align), mmio.end + 1);
+		available[i] = available_in[i];
 
-	align = pci_resource_alignment(bridge, mmio_pref_res);
-	if (!mmio_pref_res->parent && align)
-		mmio_pref.start = min(ALIGN(mmio_pref.start, align),
-			mmio_pref.end + 1);
+		/*
+		 * The alignment of this bridge is yet to be considered,
+		 * hence it must be done now before extending its bridge
+		 * window.
+		 */
+		align = pci_resource_alignment(bridge, res);
+		if (!res->parent && align)
+			available[i].start = min(ALIGN(available[i].start, align),
+						 available[i].end + 1);
 
-	/*
-	 * Now that we have adjusted for alignment, update the bridge window
-	 * resources to fill as much remaining resource space as possible.
-	 */
-	adjust_bridge_window(bridge, io_res, add_list, resource_size(&io));
-	adjust_bridge_window(bridge, mmio_res, add_list, resource_size(&mmio));
-	adjust_bridge_window(bridge, mmio_pref_res, add_list,
-			     resource_size(&mmio_pref));
+		/*
+		 * Now that we have adjusted for alignment, update the
+		 * bridge window resources to fill as much remaining
+		 * resource space as possible.
+		 */
+		adjust_bridge_window(bridge, res, add_list,
+				     resource_size(&available[i]));
+	}
 
 	/*
 	 * Calculate how many hotplug bridges and normal bridges there
@@ -2161,7 +2159,7 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
 	 */
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		if (!dev->is_virtfn)
-			remove_dev_resources(dev, &io, &mmio, &mmio_pref);
+			remove_dev_resources(dev, available);
 	}
 
 	/*
@@ -2173,16 +2171,9 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
 	 * split between non-hotplug bridges. This is to allow possible
 	 * hotplug bridges below them to get the extra space as well.
 	 */
-	if (hotplug_bridges) {
-		io_per_b = div64_ul(resource_size(&io), hotplug_bridges);
-		mmio_per_b = div64_ul(resource_size(&mmio), hotplug_bridges);
-		mmio_pref_per_b = div64_ul(resource_size(&mmio_pref),
-					   hotplug_bridges);
-	} else {
-		io_per_b = div64_ul(resource_size(&io), normal_bridges);
-		mmio_per_b = div64_ul(resource_size(&mmio), normal_bridges);
-		mmio_pref_per_b = div64_ul(resource_size(&mmio_pref),
-					   normal_bridges);
+	for (i = 0; i < PCI_P2P_BRIDGE_RESOURCE_NUM; i++) {
+		per_bridge[i] = div64_ul(resource_size(&available[i]),
+					 hotplug_bridges ?: normal_bridges);
 	}
 
 	for_each_pci_bridge(dev, bus) {
@@ -2195,49 +2186,41 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
 		if (hotplug_bridges && !dev->is_hotplug_bridge)
 			continue;
 
-		res = &dev->resource[PCI_BRIDGE_IO_WINDOW];
+		for (i = 0; i < PCI_P2P_BRIDGE_RESOURCE_NUM; i++) {
+			res = pci_bus_resource_n(bus, i);
 
-		/*
-		 * Make sure the split resource space is properly aligned
-		 * for bridge windows (align it down to avoid going above
-		 * what is available).
-		 */
-		align = pci_resource_alignment(dev, res);
-		resource_set_size(&io, ALIGN_DOWN_IF_NONZERO(io_per_b, align));
-
-		/*
-		 * The x_per_b holds the extra resource space that can be
-		 * added for each bridge but there is the minimal already
-		 * reserved as well so adjust x.start down accordingly to
-		 * cover the whole space.
-		 */
-		io.start -= resource_size(res);
-
-		res = &dev->resource[PCI_BRIDGE_MEM_WINDOW];
-		align = pci_resource_alignment(dev, res);
-		resource_set_size(&mmio,
-				  ALIGN_DOWN_IF_NONZERO(mmio_per_b,align));
-		mmio.start -= resource_size(res);
+			/*
+			 * Make sure the split resource space is properly
+			 * aligned for bridge windows (align it down to
+			 * avoid going above what is available).
+			 */
+			align = pci_resource_alignment(dev, res);
+			resource_set_size(&available[i],
+					  ALIGN_DOWN_IF_NONZERO(per_bridge[i],
+								align));
 
-		res = &dev->resource[PCI_BRIDGE_PREF_MEM_WINDOW];
-		align = pci_resource_alignment(dev, res);
-		resource_set_size(&mmio_pref,
-				  ALIGN_DOWN_IF_NONZERO(mmio_pref_per_b, align));
-		mmio_pref.start -= resource_size(res);
+			/*
+			 * The per_bridge holds the extra resource space
+			 * that can be added for each bridge but there is
+			 * the minimal already reserved as well so adjust
+			 * x.start down accordingly to cover the whole
+			 * space.
+			 */
+			available[i].start -= resource_size(res);
+		}
 
-		pci_bus_distribute_available_resources(b, add_list, io, mmio,
-						       mmio_pref);
+		pci_bus_distribute_available_resources(b, add_list, available);
 
-		io.start += io.end + 1;
-		mmio.start += mmio.end + 1;
-		mmio_pref.start += mmio_pref.end + 1;
+		for (i = 0; i < PCI_P2P_BRIDGE_RESOURCE_NUM; i++)
+			available[i].start += available[i].end + 1;
 	}
 }
 
 static void pci_bridge_distribute_available_resources(struct pci_dev *bridge,
 						      struct list_head *add_list)
 {
-	struct resource available_io, available_mmio, available_mmio_pref;
+	struct resource *res, available[PCI_P2P_BRIDGE_RESOURCE_NUM];
+	unsigned int i;
 
 	if (!bridge->is_hotplug_bridge)
 		return;
@@ -2245,14 +2228,13 @@ static void pci_bridge_distribute_available_resources(struct pci_dev *bridge,
 	pci_dbg(bridge, "distributing available resources\n");
 
 	/* Take the initial extra resources from the hotplug port */
-	available_io = bridge->resource[PCI_BRIDGE_IO_WINDOW];
-	available_mmio = bridge->resource[PCI_BRIDGE_MEM_WINDOW];
-	available_mmio_pref = bridge->resource[PCI_BRIDGE_PREF_MEM_WINDOW];
+	for (i = 0; i < PCI_P2P_BRIDGE_RESOURCE_NUM; i++) {
+		res = pci_resource_n(bridge, PCI_BRIDGE_RESOURCES + i);
+		available[i] = *res;
+	}
 
 	pci_bus_distribute_available_resources(bridge->subordinate,
-					       add_list, available_io,
-					       available_mmio,
-					       available_mmio_pref);
+					       add_list, available);
 }
 
 static bool pci_bridge_resources_not_assigned(struct pci_dev *dev)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 275df4058767..723e9cede69d 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -119,7 +119,8 @@ enum {
 #define PCI_CB_BRIDGE_MEM_1_WINDOW	(PCI_BRIDGE_RESOURCES + 3)
 
 /* Total number of bridge resources for P2P and CardBus */
-#define PCI_BRIDGE_RESOURCE_NUM 4
+#define PCI_P2P_BRIDGE_RESOURCE_NUM	3
+#define PCI_BRIDGE_RESOURCE_NUM		4
 
 	/* Resources assigned to buses behind the bridge */
 	PCI_BRIDGE_RESOURCES,
-- 
2.39.5

[PATCH v2 21/24] PCI: Refactor remove_dev_resources() to use pbus_select_window()

[Ilpo Järvinen]

Convert remove_dev_resources() to use pbus_select_window(). As 'available'
is not the real resources, the index has to be adjusted as only bridge
resource counterparts are present in the 'available' array.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/setup-bus.c | 34 +++++++++-------------------------
 1 file changed, 9 insertions(+), 25 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 3bc329b1b923..cb91c6cb4d32 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -2062,34 +2062,18 @@ static void remove_dev_resource(struct resource *avail, struct pci_dev *dev,
 static void remove_dev_resources(struct pci_dev *dev,
 				 struct resource available[PCI_P2P_BRIDGE_RESOURCE_NUM])
 {
-	struct resource *mmio_pref = &available[PCI_BUS_BRIDGE_PREF_MEM_WINDOW];
-	struct resource *res;
+	struct resource *res, *b_win;
+	int idx;
 
 	pci_dev_for_each_resource(dev, res) {
-		if (resource_type(res) == IORESOURCE_IO) {
-			remove_dev_resource(&available[PCI_BUS_BRIDGE_IO_WINDOW],
-					    dev, res);
-		} else if (resource_type(res) == IORESOURCE_MEM) {
+		b_win = pbus_select_window(dev->bus, res);
+		if (!b_win)
+			continue;
 
-			/*
-			 * Make sure prefetchable memory is reduced from
-			 * the correct resource. Specifically we put 32-bit
-			 * prefetchable memory in non-prefetchable window
-			 * if there is a 64-bit prefetchable window.
-			 *
-			 * See comments in __pci_bus_size_bridges() for
-			 * more information.
-			 */
-			if ((res->flags & IORESOURCE_PREFETCH) &&
-			    ((res->flags & IORESOURCE_MEM_64) ==
-			     (mmio_pref->flags & IORESOURCE_MEM_64))) {
-				remove_dev_resource(&available[PCI_BUS_BRIDGE_PREF_MEM_WINDOW],
-						    dev, res);
-			} else {
-				remove_dev_resource(&available[PCI_BUS_BRIDGE_MEM_WINDOW],
-						    dev, res);
-			}
-		}
+		idx = pci_resource_num(dev->bus->self, b_win);
+		idx -= PCI_BRIDGE_RESOURCES;
+
+		remove_dev_resource(&available[idx], dev, res);
 	}
 }
 
-- 
2.39.5

[PATCH v2 22/24] PCI: Add pci_setup_one_bridge_window()

[Ilpo Järvinen]

pci_bridge_release_resources() contains a resource type hack to work
around the unsuitable __pci_setup_bridge() interface. Extract the
switch statement that picks the correct bridge window setup function
from pci_claim_bridge_resource() into pci_setup_one_bridge_window() and
use it also in pci_bridge_release_resources().

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/setup-bus.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index cb91c6cb4d32..031ad682aca1 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -953,6 +953,23 @@ static void __pci_setup_bridge(struct pci_bus *bus, unsigned long type)
 	pci_write_config_word(bridge, PCI_BRIDGE_CONTROL, bus->bridge_ctl);
 }
 
+static void pci_setup_one_bridge_window(struct pci_dev *bridge, int resno)
+{
+	switch (resno) {
+	case PCI_BRIDGE_IO_WINDOW:
+		pci_setup_bridge_io(bridge);
+		break;
+	case PCI_BRIDGE_MEM_WINDOW:
+		pci_setup_bridge_mmio(bridge);
+		break;
+	case PCI_BRIDGE_PREF_MEM_WINDOW:
+		pci_setup_bridge_mmio_pref(bridge);
+		break;
+	default:
+		return;
+	}
+}
+
 void __weak pcibios_setup_bridge(struct pci_bus *bus, unsigned long type)
 {
 }
@@ -987,19 +1004,7 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i)
 	if (pci_bus_clip_resource(bridge, i))
 		ret = pci_claim_resource(bridge, i);
 
-	switch (i) {
-	case PCI_BRIDGE_IO_WINDOW:
-		pci_setup_bridge_io(bridge);
-		break;
-	case PCI_BRIDGE_MEM_WINDOW:
-		pci_setup_bridge_mmio(bridge);
-		break;
-	case PCI_BRIDGE_PREF_MEM_WINDOW:
-		pci_setup_bridge_mmio_pref(bridge);
-		break;
-	default:
-		return -EINVAL;
-	}
+	pci_setup_one_bridge_window(bridge, i);
 
 	return ret;
 }
@@ -1839,11 +1844,7 @@ static void pci_bridge_release_resources(struct pci_bus *bus,
 	if (ret)
 		return;
 
-	type = r->flags & PCI_RES_TYPE_MASK;
-	/* Avoiding touch the one without PREF */
-	if (type & IORESOURCE_PREFETCH)
-		type = IORESOURCE_PREFETCH;
-	__pci_setup_bridge(bus, type);
+	pci_setup_one_bridge_window(dev, PCI_BRIDGE_RESOURCES + idx);
 }
 
 enum release_type {
-- 
2.39.5

[PATCH v2 23/24] PCI: Pass bridge window to pci_bus_release_bridge_resources()

[Ilpo Järvinen]

pci_bus_release_bridge_resources() takes type, which is converted into a
bridge window resource in pci_bridge_release_resources().

Find out the correct bridge window for resource whose assignment failed.
Pass that bridge window to pci_bus_release_bridge_resources() instead of
passing the type. When recursing to subordinate, check which bridge windows
have to be released and recurse for each.

For now, use pbus_select_window_for_type() instead of pbus_select_window()
because non-bridge window resources still have their flags reset which
destroys the type information from the struct resource. The struct
pci_dev_resource holds a copy of the flags which are used instead.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/setup-bus.c | 69 ++++++++++++++++-------------------------
 1 file changed, 27 insertions(+), 42 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 031ad682aca1..4ce747b5dea3 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1800,51 +1800,24 @@ static void __pci_bridge_assign_resources(const struct pci_dev *bridge,
 }
 
 static void pci_bridge_release_resources(struct pci_bus *bus,
-					 unsigned long type)
+					 struct resource *b_win)
 {
 	struct pci_dev *dev = bus->self;
-	struct resource *r;
-	struct resource *b_res;
 	int idx, ret;
 
-	b_res = &dev->resource[PCI_BRIDGE_RESOURCES];
-
-	/*
-	 * 1. If IO port assignment fails, release bridge IO port.
-	 * 2. If non pref MMIO assignment fails, release bridge nonpref MMIO.
-	 * 3. If 64bit pref MMIO assignment fails, and bridge pref is 64bit,
-	 *    release bridge pref MMIO.
-	 * 4. If pref MMIO assignment fails, and bridge pref is 32bit,
-	 *    release bridge pref MMIO.
-	 * 5. If pref MMIO assignment fails, and bridge pref is not
-	 *    assigned, release bridge nonpref MMIO.
-	 */
-	if (type & IORESOURCE_IO)
-		idx = 0;
-	else if (!(type & IORESOURCE_PREFETCH))
-		idx = 1;
-	else if ((type & IORESOURCE_MEM_64) &&
-		 (b_res[2].flags & IORESOURCE_MEM_64))
-		idx = 2;
-	else if (!(b_res[2].flags & IORESOURCE_MEM_64) &&
-		 (b_res[2].flags & IORESOURCE_PREFETCH))
-		idx = 2;
-	else
-		idx = 1;
-
-	r = &b_res[idx];
-
-	if (!r->parent)
+	if (!b_win->parent)
 		return;
 
+	idx = pci_resource_num(dev, b_win);
+
 	/* If there are children, release them all */
-	release_child_resources(r);
+	release_child_resources(b_win);
 
-	ret = pci_release_resource(dev, PCI_BRIDGE_RESOURCES + idx);
+	ret = pci_release_resource(dev, idx);
 	if (ret)
 		return;
 
-	pci_setup_one_bridge_window(dev, PCI_BRIDGE_RESOURCES + idx);
+	pci_setup_one_bridge_window(dev, idx);
 }
 
 enum release_type {
@@ -1857,7 +1830,7 @@ enum release_type {
  * a larger window later.
  */
 static void pci_bus_release_bridge_resources(struct pci_bus *bus,
-					     unsigned long type,
+					     struct resource *b_win,
 					     enum release_type rel_type)
 {
 	struct pci_dev *dev;
@@ -1865,6 +1838,8 @@ static void pci_bus_release_bridge_resources(struct pci_bus *bus,
 
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		struct pci_bus *b = dev->subordinate;
+		struct resource *res;
+
 		if (!b)
 			continue;
 
@@ -1873,9 +1848,15 @@ static void pci_bus_release_bridge_resources(struct pci_bus *bus,
 		if ((dev->class >> 8) != PCI_CLASS_BRIDGE_PCI)
 			continue;
 
-		if (rel_type == whole_subtree)
-			pci_bus_release_bridge_resources(b, type,
-						 whole_subtree);
+		if (rel_type != whole_subtree)
+			continue;
+
+		pci_bus_for_each_resource(b, res) {
+			if (res->parent != b_win)
+				continue;
+
+			pci_bus_release_bridge_resources(b, res, whole_subtree);
+		}
 	}
 
 	if (pci_is_root_bus(bus))
@@ -1885,7 +1866,7 @@ static void pci_bus_release_bridge_resources(struct pci_bus *bus,
 		return;
 
 	if ((rel_type == whole_subtree) || is_leaf_bridge)
-		pci_bridge_release_resources(bus, type);
+		pci_bridge_release_resources(bus, b_win);
 }
 
 static void pci_bus_dump_res(struct pci_bus *bus)
@@ -2282,9 +2263,13 @@ static void pci_prepare_next_assign_round(struct list_head *fail_head,
 	 * enough to contain child device resources.
 	 */
 	list_for_each_entry(fail_res, fail_head, list) {
-		pci_bus_release_bridge_resources(fail_res->dev->bus,
-						 fail_res->flags & PCI_RES_TYPE_MASK,
-						 rel_type);
+		struct pci_bus *bus = fail_res->dev->bus;
+		struct resource *b_win;
+
+		b_win = pbus_select_window_for_type(bus, fail_res->flags);
+		if (!b_win)
+			continue;
+		pci_bus_release_bridge_resources(bus, b_win, rel_type);
 	}
 
 	/* Restore size and flags */
-- 
2.39.5

[PATCH v2 24/24] PCI: Alter misleading recursion to pci_bus_release_bridge_resources()

[Ilpo Järvinen]

Recursing into pci_bus_release_bridge_resources() should not alter rel_type
because it makes no sense to change the release type within the recursion
call chain. A literal "whole_subtree" is passed into the recursion instead
of "rel_type" parameter which is misleading as the release type should
remain the same throughout the entire operation.

This is not a correctness issue because of the preceding if () that only
allows the recursion to happen if rel_type is "whole_subtree". Still,
replace the non-intuitive parameter with direct passing of "rel_type".

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/setup-bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 4ce747b5dea3..d264f16772b9 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1855,7 +1855,7 @@ static void pci_bus_release_bridge_resources(struct pci_bus *bus,
 			if (res->parent != b_win)
 				continue;
 
-			pci_bus_release_bridge_resources(b, res, whole_subtree);
+			pci_bus_release_bridge_resources(b, res, rel_type);
 		}
 	}
 
-- 
2.39.5

Re: [PATCH v2 00/24] PCI: Bridge window selection improvements

[Ilpo Järvinen]

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

On Fri, 29 Aug 2025, Ilpo Järvinen wrote:

> This series is based on top of the three resource fitting and assignment
> algorithm fixes already in the pci/resource branch. I've tried to compare
> these patch with the commits in the pci/resource branch to retain the minor
> spelling/grammar corrections Bjorn made while applying v1.
> 
> v2 is just to fix two small issues within the series intermediate patches.
> These corrections attempt to ensure this series is bisectable if
> troubleshooting requires that in the future.
> 
> In addition, a few corrections to changelog texts were made.
> 
> I'm left to wonder though if the added double spaces after some stops
> within the commit messages in the pci/resource branch were intentional or
> not (I did remove them for v2).
> 
> As the changes are very minimal, I'm only sending this to lists and Bjorn
> to spare people's inboxes. If somebody provides a Tested-by tag for v1, it
> should be counted in for this v2 (v1 vs v2 difference does not matter if
> testing the entire series).
> 
> v2:
> - In pci_bridge_release_resources():
>     - Keep type assignment in until removing the type hack.
>     - Introduce res_name in the patch it is used avoid compiler warning
>       about unused variable. Place it into the block that needs it.
> - Minor corrections to changelog texts

Hi Bjorn,

Just a reminder that v2 improves bisectability of this series which might 
turn out very useful in case people hit any issues due to one of these 
changes so I'd prefer pci/resource content to be upgrade from v1 to v2.

--
 i.

> Ilpo Järvinen (24):
>   m68k/PCI: Use pci_enable_resources() in pcibios_enable_device()
>   sparc/PCI: Remove pcibios_enable_device() as they do nothing extra
>   MIPS: PCI: Use pci_enable_resources()
>   PCI: Move find_bus_resource_of_type() earlier
>   PCI: Refactor find_bus_resource_of_type() logic checks
>   PCI: Always claim bridge window before its setup
>   PCI: Disable non-claimed bridge window
>   PCI: Use pci_release_resource() instead of release_resource()
>   PCI: Enable bridge even if bridge window fails to assign
>   PCI: Preserve bridge window resource type flags
>   PCI: Add defines for bridge window indexing
>   PCI: Add bridge window selection functions
>   PCI: Fix finding bridge window in pci_reassign_bridge_resources()
>   PCI: Warn if bridge window cannot be released when resizing BAR
>   PCI: Use pbus_select_window() during BAR resize
>   PCI: Use pbus_select_window_for_type() during IO window sizing
>   PCI: Rename resource variable from r to res
>   PCI: Use pbus_select_window() in space available checker
>   PCI: Use pbus_select_window_for_type() during mem window sizing
>   PCI: Refactor distributing available memory to use loops
>   PCI: Refactor remove_dev_resources() to use pbus_select_window()
>   PCI: Add pci_setup_one_bridge_window()
>   PCI: Pass bridge window to pci_bus_release_bridge_resources()
>   PCI: Alter misleading recursion to pci_bus_release_bridge_resources()
> 
>  arch/m68k/kernel/pcibios.c   |  39 +-
>  arch/mips/pci/pci-legacy.c   |  38 +-
>  arch/sparc/kernel/leon_pci.c |  27 --
>  arch/sparc/kernel/pci.c      |  27 --
>  arch/sparc/kernel/pcic.c     |  27 --
>  drivers/pci/bus.c            |   3 +
>  drivers/pci/pci-sysfs.c      |  27 +-
>  drivers/pci/pci.h            |   8 +-
>  drivers/pci/probe.c          |  35 +-
>  drivers/pci/setup-bus.c      | 798 ++++++++++++++++++-----------------
>  drivers/pci/setup-res.c      |  46 +-
>  include/linux/pci.h          |   5 +-
>  12 files changed, 504 insertions(+), 576 deletions(-)
> 
> 
> base-commit: 295524c65d8b4850efbb809f12176eb1262a5aba
> 

Re: [PATCH v2 00/24] PCI: Bridge window selection improvements

[Bjorn Helgaas]

On Fri, Aug 29, 2025 at 04:10:49PM +0300, Ilpo Järvinen wrote:
> This series is based on top of the three resource fitting and assignment
> algorithm fixes already in the pci/resource branch. I've tried to compare
> these patch with the commits in the pci/resource branch to retain the minor
> spelling/grammar corrections Bjorn made while applying v1.
> 
> v2 is just to fix two small issues within the series intermediate patches.
> These corrections attempt to ensure this series is bisectable if
> troubleshooting requires that in the future.
> 
> In addition, a few corrections to changelog texts were made.
> 
> I'm left to wonder though if the added double spaces after some stops
> within the commit messages in the pci/resource branch were intentional or
> not (I did remove them for v2).
> 
> As the changes are very minimal, I'm only sending this to lists and Bjorn
> to spare people's inboxes. If somebody provides a Tested-by tag for v1, it
> should be counted in for this v2 (v1 vs v2 difference does not matter if
> testing the entire series).
> 
> v2:
> - In pci_bridge_release_resources():
>     - Keep type assignment in until removing the type hack.
>     - Introduce res_name in the patch it is used avoid compiler warning
>       about unused variable. Place it into the block that needs it.
> - Minor corrections to changelog texts
> 
> Ilpo Järvinen (24):
>   m68k/PCI: Use pci_enable_resources() in pcibios_enable_device()
>   sparc/PCI: Remove pcibios_enable_device() as they do nothing extra
>   MIPS: PCI: Use pci_enable_resources()
>   PCI: Move find_bus_resource_of_type() earlier
>   PCI: Refactor find_bus_resource_of_type() logic checks
>   PCI: Always claim bridge window before its setup
>   PCI: Disable non-claimed bridge window
>   PCI: Use pci_release_resource() instead of release_resource()
>   PCI: Enable bridge even if bridge window fails to assign
>   PCI: Preserve bridge window resource type flags
>   PCI: Add defines for bridge window indexing
>   PCI: Add bridge window selection functions
>   PCI: Fix finding bridge window in pci_reassign_bridge_resources()
>   PCI: Warn if bridge window cannot be released when resizing BAR
>   PCI: Use pbus_select_window() during BAR resize
>   PCI: Use pbus_select_window_for_type() during IO window sizing
>   PCI: Rename resource variable from r to res
>   PCI: Use pbus_select_window() in space available checker
>   PCI: Use pbus_select_window_for_type() during mem window sizing
>   PCI: Refactor distributing available memory to use loops
>   PCI: Refactor remove_dev_resources() to use pbus_select_window()
>   PCI: Add pci_setup_one_bridge_window()
>   PCI: Pass bridge window to pci_bus_release_bridge_resources()
>   PCI: Alter misleading recursion to pci_bus_release_bridge_resources()
> 
>  arch/m68k/kernel/pcibios.c   |  39 +-
>  arch/mips/pci/pci-legacy.c   |  38 +-
>  arch/sparc/kernel/leon_pci.c |  27 --
>  arch/sparc/kernel/pci.c      |  27 --
>  arch/sparc/kernel/pcic.c     |  27 --
>  drivers/pci/bus.c            |   3 +
>  drivers/pci/pci-sysfs.c      |  27 +-
>  drivers/pci/pci.h            |   8 +-
>  drivers/pci/probe.c          |  35 +-
>  drivers/pci/setup-bus.c      | 798 ++++++++++++++++++-----------------
>  drivers/pci/setup-res.c      |  46 +-
>  include/linux/pci.h          |   5 +-
>  12 files changed, 504 insertions(+), 576 deletions(-)

Updated pci/resource with this v2, thanks for the reminder!

Re: [PATCH v2 20/24] PCI: Refactor distributing available memory to use loops

[john_chen_chn]

Hi Ilpo Järvinen,

This patch makes Thunderbolt unusable on my AMD Strix Halo platform.

Here's the dmesg output:

[ 4.212389] ------------[ cut here ]------------
[ 4.212391] WARNING: CPU: 6 PID: 272 at drivers/pci/pci.h:471 pci_bus_distribute_available_resources+0x6ad/0x6d0
[ 4.212400] Modules linked in: raid6_pq(+) hid_generic uas usb_storage scsi_mod usbhid hid scsi_common amdgpu amdxcp drm_panel_backlight_quirks gpu_sched drm_buddy drm_ttm_helper ttm drm_exec i2c_algo_bit drm_suballoc_helper drm_display_helper cec rc_core drm_client_lib drm_kms_helper sdhci_pci sdhci_uhs2 xhci_pci sp5100_tco xhci_hcd r8169 drm nvme sdhci watchdog realtek usbcore thunderbolt cqhci atlantic nvme_core mdio_devres psmouse libphy mmc_core nvme_keyring video i2c_piix4 macsec nvme_auth serio_raw mdio_bus i2c_smbus usb_common crc16 hkdf wmi
[ 4.212443] CPU: 6 UID: 0 PID: 272 Comm: irq/33-pciehp Not tainted 6.17.0+ #1 PREEMPT(voluntary)
[ 4.212447] Hardware name: PELADN YO Series/YO1, BIOS 1.04 05/15/2025
[ 4.212449] RIP: 0010:pci_bus_distribute_available_resources+0x6ad/0x6d0
[ 4.212453] Code: ff e9 a2 48 c7 c7 b8 b7 83 a3 4c 89 4c 24 18 e8 a9 2a fb ff 4c 8b 4c 24 18 e9 ca fd ff ff 48 8b 05 60 53 47 01 e9 94 fe ff ff <0f> 0b e9 5d fe ff ff 48 8b 05 55 53 47 01 e9 81 fe ff ff e8 4b 87
[ 4.212455] RSP: 0018:ffffaffcc0d4f9a8 EFLAGS: 00010206
[ 4.212458] RAX: 00000000000000cd RBX: ffff9721a687f800 RCX: ffff9721a687c828
[ 4.212459] RDX: 0000000000000000 RSI: 00000000000000cd RDI: ffff97218bc8a3c0
[ 4.212461] RBP: ffff9721a687c828 R08: ffffaffcc0d4f9f8 R09: 0000000000000001
[ 4.212462] R10: ffff97218bc8d700 R11: 0000000000000000 R12: ffffaffcc0d4f9f8
[ 4.212462] R13: ffffaffcc0d4f9f8 R14: 0000000000000000 R15: ffff97218bc8a000
[ 4.212464] FS: 0000000000000000(0000) GS:ffff973ee1ad9000(0000) knlGS:0000000000000000
[ 4.212465] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 4.212467] CR2: 00005640b0f29360 CR3: 0000000ccb224000 CR4: 0000000000f50ef0
[ 4.212469] PKRU: 55555554
[ 4.212470] Call Trace:
[ 4.212473] <TASK>
[ 4.212478] pci_bus_distribute_available_resources+0x590/0x6d0
[ 4.212483] pci_bridge_distribute_available_resources+0x62/0xb0
[ 4.212487] pci_assign_unassigned_bridge_resources+0x65/0x1b0
[ 4.212490] pciehp_configure_device+0x92/0x160
[ 4.212495] pciehp_handle_presence_or_link_change+0x1b5/0x350
[ 4.212498] pciehp_ist+0x147/0x1c0
[ 4.212502] irq_thread_fn+0x20/0x60
[ 4.212508] irq_thread+0x1cc/0x360
[ 4.212511] ? __pfx_irq_thread_fn+0x10/0x10
[ 4.212515] ? __pfx_irq_thread_dtor+0x10/0x10
[ 4.212518] ? __pfx_irq_thread+0x10/0x10
[ 4.212521] kthread+0xf9/0x240
[ 4.212525] ? __pfx_kthread+0x10/0x10
[ 4.212528] ret_from_fork+0x195/0x1d0
[ 4.212533] ? __pfx_kthread+0x10/0x10
[ 4.212536] ret_from_fork_asm+0x1a/0x30
[ 4.212540] </TASK>
[ 4.212541] ---[ end trace 0000000000000000 ]—

Perhaps there's something you forgot to modify while copying and
pasting, as shown below:

> On 29 Aug 2025, at 21:11, Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> 
> pci_bus_distribute_available_resources() and
> pci_bridge_distribute_available_resources() retain bridge window resources
> and related data needed for distributing the available window in
> independent variables for io, memory, and prefetchable memory windows. The
> code is essentially the same for all of them and therefore repeated three
> times with different variable names.
> 
> Refactor pci_bus_distribute_available_resources() to take an array. This
> is complicated slightly by the function taking advantage of passing the
> struct as value, which cannot be done for arrays in C. Therefore, copy the
> data into a local array in the stack in the first loop.
> 
> Variable names are (hopefully) improved slightly as well.
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
> drivers/pci/setup-bus.c | 162 ++++++++++++++++++----------------------
> include/linux/pci.h     |   3 +-
> 2 files changed, 74 insertions(+), 91 deletions(-)
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 720159bca54d..3bc329b1b923 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -2059,15 +2059,16 @@ static void remove_dev_resource(struct resource *avail, struct pci_dev *dev,
> avail->start = min(avail->start + tmp, avail->end + 1);
> }
> 
> -static void remove_dev_resources(struct pci_dev *dev, struct resource *io,
> - struct resource *mmio,
> - struct resource *mmio_pref)
> +static void remove_dev_resources(struct pci_dev *dev,
> + struct resource available[PCI_P2P_BRIDGE_RESOURCE_NUM])
> {
> + struct resource *mmio_pref = &available[PCI_BUS_BRIDGE_PREF_MEM_WINDOW];
> struct resource *res;
> 
> pci_dev_for_each_resource(dev, res) {
> if (resource_type(res) == IORESOURCE_IO) {
> - remove_dev_resource(io, dev, res);
> + remove_dev_resource(&available[PCI_BUS_BRIDGE_IO_WINDOW],
> +    dev, res);
> } else if (resource_type(res) == IORESOURCE_MEM) {
> 
> /*
> @@ -2081,10 +2082,13 @@ static void remove_dev_resources(struct pci_dev *dev, struct resource *io,
> */
> if ((res->flags & IORESOURCE_PREFETCH) &&
>    ((res->flags & IORESOURCE_MEM_64) ==
> -     (mmio_pref->flags & IORESOURCE_MEM_64)))
> - remove_dev_resource(mmio_pref, dev, res);
> - else
> - remove_dev_resource(mmio, dev, res);
> +     (mmio_pref->flags & IORESOURCE_MEM_64))) {
> + remove_dev_resource(&available[PCI_BUS_BRIDGE_PREF_MEM_WINDOW],
> +    dev, res);
> + } else {
> + remove_dev_resource(&available[PCI_BUS_BRIDGE_MEM_WINDOW],
> +    dev, res);
> + }
> }
> }
> }
> @@ -2099,45 +2103,39 @@ static void remove_dev_resources(struct pci_dev *dev, struct resource *io,
>  * shared with the bridges.
>  */
> static void pci_bus_distribute_available_resources(struct pci_bus *bus,
> -    struct list_head *add_list,
> -    struct resource io,
> -    struct resource mmio,
> -    struct resource mmio_pref)
> +    struct list_head *add_list,
> +    struct resource available_in[PCI_P2P_BRIDGE_RESOURCE_NUM])
> {
> + struct resource available[PCI_P2P_BRIDGE_RESOURCE_NUM];
> unsigned int normal_bridges = 0, hotplug_bridges = 0;
> - struct resource *io_res, *mmio_res, *mmio_pref_res;
> struct pci_dev *dev, *bridge = bus->self;
> - resource_size_t io_per_b, mmio_per_b, mmio_pref_per_b, align;
> + resource_size_t per_bridge[PCI_P2P_BRIDGE_RESOURCE_NUM];
> + resource_size_t align;
> + int i;
> 
> - io_res = &bridge->resource[PCI_BRIDGE_IO_WINDOW];
> - mmio_res = &bridge->resource[PCI_BRIDGE_MEM_WINDOW];
> - mmio_pref_res = &bridge->resource[PCI_BRIDGE_PREF_MEM_WINDOW];
> + for (i = 0; i < PCI_P2P_BRIDGE_RESOURCE_NUM; i++) {
> + struct resource *res = pci_bus_resource_n(bus, i);

Here should be: pci_resource_n(bridge, PCI_BRIDGE_RESOURCES + i);

> 
> - /*
> - * The alignment of this bridge is yet to be considered, hence it must
> - * be done now before extending its bridge window.
> - */
> - align = pci_resource_alignment(bridge, io_res);
> - if (!io_res->parent && align)
> - io.start = min(ALIGN(io.start, align), io.end + 1);
> -
> - align = pci_resource_alignment(bridge, mmio_res);
> - if (!mmio_res->parent && align)
> - mmio.start = min(ALIGN(mmio.start, align), mmio.end + 1);
> + available[i] = available_in[i];
> 
> - align = pci_resource_alignment(bridge, mmio_pref_res);
> - if (!mmio_pref_res->parent && align)
> - mmio_pref.start = min(ALIGN(mmio_pref.start, align),
> - mmio_pref.end + 1);
> + /*
> + * The alignment of this bridge is yet to be considered,
> + * hence it must be done now before extending its bridge
> + * window.
> + */
> + align = pci_resource_alignment(bridge, res);
> + if (!res->parent && align)
> + available[i].start = min(ALIGN(available[i].start, align),
> + available[i].end + 1);
> 
> - /*
> - * Now that we have adjusted for alignment, update the bridge window
> - * resources to fill as much remaining resource space as possible.
> - */
> - adjust_bridge_window(bridge, io_res, add_list, resource_size(&io));
> - adjust_bridge_window(bridge, mmio_res, add_list, resource_size(&mmio));
> - adjust_bridge_window(bridge, mmio_pref_res, add_list,
> -     resource_size(&mmio_pref));
> + /*
> + * Now that we have adjusted for alignment, update the
> + * bridge window resources to fill as much remaining
> + * resource space as possible.
> + */
> + adjust_bridge_window(bridge, res, add_list,
> +     resource_size(&available[i]));
> + }
> 
> /*
> * Calculate how many hotplug bridges and normal bridges there
> @@ -2161,7 +2159,7 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
> */
> list_for_each_entry(dev, &bus->devices, bus_list) {
> if (!dev->is_virtfn)
> - remove_dev_resources(dev, &io, &mmio, &mmio_pref);
> + remove_dev_resources(dev, available);
> }
> 
> /*
> @@ -2173,16 +2171,9 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
> * split between non-hotplug bridges. This is to allow possible
> * hotplug bridges below them to get the extra space as well.
> */
> - if (hotplug_bridges) {
> - io_per_b = div64_ul(resource_size(&io), hotplug_bridges);
> - mmio_per_b = div64_ul(resource_size(&mmio), hotplug_bridges);
> - mmio_pref_per_b = div64_ul(resource_size(&mmio_pref),
> -   hotplug_bridges);
> - } else {
> - io_per_b = div64_ul(resource_size(&io), normal_bridges);
> - mmio_per_b = div64_ul(resource_size(&mmio), normal_bridges);
> - mmio_pref_per_b = div64_ul(resource_size(&mmio_pref),
> -   normal_bridges);
> + for (i = 0; i < PCI_P2P_BRIDGE_RESOURCE_NUM; i++) {
> + per_bridge[i] = div64_ul(resource_size(&available[i]),
> + hotplug_bridges ?: normal_bridges);
> }
> 
> for_each_pci_bridge(dev, bus) {
> @@ -2195,49 +2186,41 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
> if (hotplug_bridges && !dev->is_hotplug_bridge)
> continue;
> 
> - res = &dev->resource[PCI_BRIDGE_IO_WINDOW];
> + for (i = 0; i < PCI_P2P_BRIDGE_RESOURCE_NUM; i++) {
> + res = pci_bus_resource_n(bus, i);

Same here, should be: pci_resource_n(dev, PCI_BRIDGE_RESOURCES + i);

I have written a patch to resolve these issues:
https://lore.kernel.org/lkml/tencent_8C54420E1B0FF8D804C1B4651DF970716309@qq.com/

Thanks,
Yangyu Chen

> 
> - /*
> - * Make sure the split resource space is properly aligned
> - * for bridge windows (align it down to avoid going above
> - * what is available).
> - */
> - align = pci_resource_alignment(dev, res);
> - resource_set_size(&io, ALIGN_DOWN_IF_NONZERO(io_per_b, align));
> -
> - /*
> - * The x_per_b holds the extra resource space that can be
> - * added for each bridge but there is the minimal already
> - * reserved as well so adjust x.start down accordingly to
> - * cover the whole space.
> - */
> - io.start -= resource_size(res);
> -
> - res = &dev->resource[PCI_BRIDGE_MEM_WINDOW];
> - align = pci_resource_alignment(dev, res);
> - resource_set_size(&mmio,
> -  ALIGN_DOWN_IF_NONZERO(mmio_per_b,align));
> - mmio.start -= resource_size(res);
> + /*
> + * Make sure the split resource space is properly
> + * aligned for bridge windows (align it down to
> + * avoid going above what is available).
> + */
> + align = pci_resource_alignment(dev, res);
> + resource_set_size(&available[i],
> +  ALIGN_DOWN_IF_NONZERO(per_bridge[i],
> + align));
> 
> - res = &dev->resource[PCI_BRIDGE_PREF_MEM_WINDOW];
> - align = pci_resource_alignment(dev, res);
> - resource_set_size(&mmio_pref,
> -  ALIGN_DOWN_IF_NONZERO(mmio_pref_per_b, align));
> - mmio_pref.start -= resource_size(res);
> + /*
> + * The per_bridge holds the extra resource space
> + * that can be added for each bridge but there is
> + * the minimal already reserved as well so adjust
> + * x.start down accordingly to cover the whole
> + * space.
> + */
> + available[i].start -= resource_size(res);
> + }
> 
> - pci_bus_distribute_available_resources(b, add_list, io, mmio,
> -       mmio_pref);
> + pci_bus_distribute_available_resources(b, add_list, available);
> 
> - io.start += io.end + 1;
> - mmio.start += mmio.end + 1;
> - mmio_pref.start += mmio_pref.end + 1;
> + for (i = 0; i < PCI_P2P_BRIDGE_RESOURCE_NUM; i++)
> + available[i].start += available[i].end + 1;
> }
> }
> 
> static void pci_bridge_distribute_available_resources(struct pci_dev *bridge,
>      struct list_head *add_list)
> {
> - struct resource available_io, available_mmio, available_mmio_pref;
> + struct resource *res, available[PCI_P2P_BRIDGE_RESOURCE_NUM];
> + unsigned int i;
> 
> if (!bridge->is_hotplug_bridge)
> return;
> @@ -2245,14 +2228,13 @@ static void pci_bridge_distribute_available_resources(struct pci_dev *bridge,
> pci_dbg(bridge, "distributing available resources\n");
> 
> /* Take the initial extra resources from the hotplug port */
> - available_io = bridge->resource[PCI_BRIDGE_IO_WINDOW];
> - available_mmio = bridge->resource[PCI_BRIDGE_MEM_WINDOW];
> - available_mmio_pref = bridge->resource[PCI_BRIDGE_PREF_MEM_WINDOW];
> + for (i = 0; i < PCI_P2P_BRIDGE_RESOURCE_NUM; i++) {
> + res = pci_resource_n(bridge, PCI_BRIDGE_RESOURCES + i);
> + available[i] = *res;
> + }
> 
> pci_bus_distribute_available_resources(bridge->subordinate,
> -       add_list, available_io,
> -       available_mmio,
> -       available_mmio_pref);
> +       add_list, available);
> }
> 
> static bool pci_bridge_resources_not_assigned(struct pci_dev *dev)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 275df4058767..723e9cede69d 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -119,7 +119,8 @@ enum {
> #define PCI_CB_BRIDGE_MEM_1_WINDOW (PCI_BRIDGE_RESOURCES + 3)
> 
> /* Total number of bridge resources for P2P and CardBus */
> -#define PCI_BRIDGE_RESOURCE_NUM 4
> +#define PCI_P2P_BRIDGE_RESOURCE_NUM 3
> +#define PCI_BRIDGE_RESOURCE_NUM 4
> 
> /* Resources assigned to buses behind the bridge */
> PCI_BRIDGE_RESOURCES,
> -- 
> 2.39.5
> 
> 

Re: [PATCH v2 03/24] MIPS: PCI: Use pci_enable_resources()

[Guenter Roeck]

Hi,

On Fri, Aug 29, 2025 at 04:10:52PM +0300, Ilpo Järvinen wrote:
> pci-legacy.c under MIPS has a copy of pci_enable_resources() named as
> pcibios_enable_resources(). Having own copy of same functionality could
> lead to inconsistencies in behavior, especially now as
> pci_enable_resources() and the bridge window resource flags behavior are
> going to be altered by upcoming changes.
> 
> The check for !r->start && r->end is already covered by the more generic
> checks done in pci_enable_resources().
> 
> Call pci_enable_resources() from MIPS's pcibios_enable_device() and remove
> pcibios_enable_resources().
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>

This patch causes boot failures when trying to boot mips images from
ide drive in qemu. As far as I can see the interface no longer instantiates.

Reverting this patch fixes the problem. Bisect log attached for reference.

Guenter

---
# bad: [3a8660878839faadb4f1a6dd72c3179c1df56787] Linux 6.18-rc1
# good: [e5f0a698b34ed76002dc5cff3804a61c80233a7a] Linux 6.17
git bisect start 'HEAD' 'v6.17'
# good: [58809f614e0e3f4e12b489bddf680bfeb31c0a20] Merge tag 'drm-next-2025-10-01' of https://gitlab.freedesktop.org/drm/kernel
git bisect good 58809f614e0e3f4e12b489bddf680bfeb31c0a20
# good: [bed0653fe2aacb0ca8196075cffc9e7062e74927] Merge tag 'iommu-updates-v6.18' of git://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux
git bisect good bed0653fe2aacb0ca8196075cffc9e7062e74927
# good: [6a74422b9710e987c7d6b85a1ade7330b1e61626] Merge tag 'mips_6.18' of git://git.kernel.org/pub/scm/linux/kernel/git/mips/linux
git bisect good 6a74422b9710e987c7d6b85a1ade7330b1e61626
# bad: [522ba450b56fff29f868b1552bdc2965f55de7ed] Merge tag 'clk-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux
git bisect bad 522ba450b56fff29f868b1552bdc2965f55de7ed
# bad: [256e3417065b2721f77bcd37331796b59483ef3b] Merge tag 'for-linus' of git://git.kernel.org/pub/scm/virt/kvm/kvm
git bisect bad 256e3417065b2721f77bcd37331796b59483ef3b
# bad: [2f2c7254931f41b5736e3ba12aaa9ac1bbeeeb92] Merge tag 'pci-v6.18-changes' of git://git.kernel.org/pub/scm/linux/kernel/git/pci/pci
git bisect bad 2f2c7254931f41b5736e3ba12aaa9ac1bbeeeb92
# bad: [531abff0fa53bc3a2f7f69b2693386eb6bda96e5] Merge branch 'pci/controller/qcom'
git bisect bad 531abff0fa53bc3a2f7f69b2693386eb6bda96e5
# bad: [fead6a0b15bf3b33dba877efec6b4e7b4cc4abc3] Merge branch 'pci/resource'
git bisect bad fead6a0b15bf3b33dba877efec6b4e7b4cc4abc3
# good: [0bb65e32495e6235a069b60e787140da99e9c122] Merge branch 'pci/p2pdma'
git bisect good 0bb65e32495e6235a069b60e787140da99e9c122
# bad: [ebe091ad81e1d3e5cbb1592ebc18175b5ca3d2bd] PCI: Use pbus_select_window_for_type() during IO window sizing
git bisect bad ebe091ad81e1d3e5cbb1592ebc18175b5ca3d2bd
# bad: [2ee33aa14d3f2e92ba8ae80443f2cd9b575f08cb] PCI: Always claim bridge window before its setup
git bisect bad 2ee33aa14d3f2e92ba8ae80443f2cd9b575f08cb
# good: [2657a0c982239fecc41d0df5a69091ca4297647c] m68k/PCI: Use pci_enable_resources() in pcibios_enable_device()
git bisect good 2657a0c982239fecc41d0df5a69091ca4297647c
# bad: [ae81aad5c2e17fd1fafd930e75b81aedc837f705] MIPS: PCI: Use pci_enable_resources()
git bisect bad ae81aad5c2e17fd1fafd930e75b81aedc837f705
# good: [754babaaf33349d9ef27bb1ac6f5d9d5a503a2a6] sparc/PCI: Remove pcibios_enable_device() as they do nothing extra
git bisect good 754babaaf33349d9ef27bb1ac6f5d9d5a503a2a6
# first bad commit: [ae81aad5c2e17fd1fafd930e75b81aedc837f705] MIPS: PCI: Use pci_enable_resources()

Re: [PATCH v2 03/24] MIPS: PCI: Use pci_enable_resources()

[Bjorn Helgaas]

[+cc regressions]

On Mon, Oct 13, 2025 at 12:54:25PM -0700, Guenter Roeck wrote:
> On Fri, Aug 29, 2025 at 04:10:52PM +0300, Ilpo Järvinen wrote:
> > pci-legacy.c under MIPS has a copy of pci_enable_resources() named as
> > pcibios_enable_resources(). Having own copy of same functionality could
> > lead to inconsistencies in behavior, especially now as
> > pci_enable_resources() and the bridge window resource flags behavior are
> > going to be altered by upcoming changes.
> > 
> > The check for !r->start && r->end is already covered by the more generic
> > checks done in pci_enable_resources().
> > 
> > Call pci_enable_resources() from MIPS's pcibios_enable_device() and remove
> > pcibios_enable_resources().
> > 
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> 
> This patch causes boot failures when trying to boot mips images from
> ide drive in qemu. As far as I can see the interface no longer instantiates.
> 
> Reverting this patch fixes the problem. Bisect log attached for reference.
> 
> Guenter
> 
> ---
> # bad: [3a8660878839faadb4f1a6dd72c3179c1df56787] Linux 6.18-rc1
> # good: [e5f0a698b34ed76002dc5cff3804a61c80233a7a] Linux 6.17
> git bisect start 'HEAD' 'v6.17'
> # good: [58809f614e0e3f4e12b489bddf680bfeb31c0a20] Merge tag 'drm-next-2025-10-01' of https://gitlab.freedesktop.org/drm/kernel
> git bisect good 58809f614e0e3f4e12b489bddf680bfeb31c0a20
> # good: [bed0653fe2aacb0ca8196075cffc9e7062e74927] Merge tag 'iommu-updates-v6.18' of git://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux
> git bisect good bed0653fe2aacb0ca8196075cffc9e7062e74927
> # good: [6a74422b9710e987c7d6b85a1ade7330b1e61626] Merge tag 'mips_6.18' of git://git.kernel.org/pub/scm/linux/kernel/git/mips/linux
> git bisect good 6a74422b9710e987c7d6b85a1ade7330b1e61626
> # bad: [522ba450b56fff29f868b1552bdc2965f55de7ed] Merge tag 'clk-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux
> git bisect bad 522ba450b56fff29f868b1552bdc2965f55de7ed
> # bad: [256e3417065b2721f77bcd37331796b59483ef3b] Merge tag 'for-linus' of git://git.kernel.org/pub/scm/virt/kvm/kvm
> git bisect bad 256e3417065b2721f77bcd37331796b59483ef3b
> # bad: [2f2c7254931f41b5736e3ba12aaa9ac1bbeeeb92] Merge tag 'pci-v6.18-changes' of git://git.kernel.org/pub/scm/linux/kernel/git/pci/pci
> git bisect bad 2f2c7254931f41b5736e3ba12aaa9ac1bbeeeb92
> # bad: [531abff0fa53bc3a2f7f69b2693386eb6bda96e5] Merge branch 'pci/controller/qcom'
> git bisect bad 531abff0fa53bc3a2f7f69b2693386eb6bda96e5
> # bad: [fead6a0b15bf3b33dba877efec6b4e7b4cc4abc3] Merge branch 'pci/resource'
> git bisect bad fead6a0b15bf3b33dba877efec6b4e7b4cc4abc3
> # good: [0bb65e32495e6235a069b60e787140da99e9c122] Merge branch 'pci/p2pdma'
> git bisect good 0bb65e32495e6235a069b60e787140da99e9c122
> # bad: [ebe091ad81e1d3e5cbb1592ebc18175b5ca3d2bd] PCI: Use pbus_select_window_for_type() during IO window sizing
> git bisect bad ebe091ad81e1d3e5cbb1592ebc18175b5ca3d2bd
> # bad: [2ee33aa14d3f2e92ba8ae80443f2cd9b575f08cb] PCI: Always claim bridge window before its setup
> git bisect bad 2ee33aa14d3f2e92ba8ae80443f2cd9b575f08cb
> # good: [2657a0c982239fecc41d0df5a69091ca4297647c] m68k/PCI: Use pci_enable_resources() in pcibios_enable_device()
> git bisect good 2657a0c982239fecc41d0df5a69091ca4297647c
> # bad: [ae81aad5c2e17fd1fafd930e75b81aedc837f705] MIPS: PCI: Use pci_enable_resources()
> git bisect bad ae81aad5c2e17fd1fafd930e75b81aedc837f705
> # good: [754babaaf33349d9ef27bb1ac6f5d9d5a503a2a6] sparc/PCI: Remove pcibios_enable_device() as they do nothing extra
> git bisect good 754babaaf33349d9ef27bb1ac6f5d9d5a503a2a6
> # first bad commit: [ae81aad5c2e17fd1fafd930e75b81aedc837f705] MIPS: PCI: Use pci_enable_resources()

#regzbot introduced: ae81aad5c2e1 ("MIPS: PCI: Use pci_enable_resources()")

Re: [PATCH v2 03/24] MIPS: PCI: Use pci_enable_resources()

[Thomas Bogendoerfer]

On Mon, Oct 13, 2025 at 12:54:25PM -0700, Guenter Roeck wrote:
> Hi,
> 
> On Fri, Aug 29, 2025 at 04:10:52PM +0300, Ilpo Järvinen wrote:
> > pci-legacy.c under MIPS has a copy of pci_enable_resources() named as
> > pcibios_enable_resources(). Having own copy of same functionality could
> > lead to inconsistencies in behavior, especially now as
> > pci_enable_resources() and the bridge window resource flags behavior are
> > going to be altered by upcoming changes.
> > 
> > The check for !r->start && r->end is already covered by the more generic
> > checks done in pci_enable_resources().
> > 
> > Call pci_enable_resources() from MIPS's pcibios_enable_device() and remove
> > pcibios_enable_resources().
> > 
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> 
> This patch causes boot failures when trying to boot mips images from
> ide drive in qemu. As far as I can see the interface no longer instantiates.
> 
> Reverting this patch fixes the problem. Bisect log attached for reference.

Patch below fixes my qemu malta setup. Now I'm wondering, why this is
needed. It was added with commit

aa0980b80908 ("Fixes for system controllers for Atlas/Malta core cards.")

Maciej, do you remember why this is needed ?

Thomas.

diff --git a/arch/mips/pci/pci-malta.c b/arch/mips/pci/pci-malta.c
index 6aefdf20ca05..e0270b818b03 100644
--- a/arch/mips/pci/pci-malta.c
+++ b/arch/mips/pci/pci-malta.c
@@ -229,10 +229,6 @@ void __init mips_pcibios_init(void)
 		return;
 	}
 
-	/* PIIX4 ACPI starts at 0x1000 */
-	if (controller->io_resource->start < 0x00001000UL)
-		controller->io_resource->start = 0x00001000UL;
-
 	iomem_resource.end &= 0xfffffffffULL;			/* 64 GB */
 	ioport_resource.end = controller->io_resource->end;
 
-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH v2 03/24] MIPS: PCI: Use pci_enable_resources()

[Maciej W. Rozycki]

On Mon, 13 Oct 2025, Thomas Bogendoerfer wrote:

> > This patch causes boot failures when trying to boot mips images from
> > ide drive in qemu. As far as I can see the interface no longer instantiates.
> > 
> > Reverting this patch fixes the problem. Bisect log attached for reference.
> 
> Patch below fixes my qemu malta setup. Now I'm wondering, why this is
> needed. It was added with commit
> 
> aa0980b80908 ("Fixes for system controllers for Atlas/Malta core cards.")
> 
> Maciej, do you remember why this is needed ?

 I do.  The reason is preventing PCI port I/O mappings below 0x100, which 
interferes badly with how the PIIX4 decodes port I/O cycles.  That did 
happen in the field, wreaking havoc and prompting my change.

 By the look of the code it would definitely trigger for the Bonito64 
system controller, which has a fixed port I/O target address range and, 
depending on the settings left by the firmware, it might also trigger for 
the Galileo GT64120A and SOC-it 101 system controllers, which have 
variable port I/O target address ranges.

 Here's an example map of Malta port I/O resources (SOC-it 101 variant):

00000000-0000001f : dma1
00000020-00000021 : pic1
00000040-0000005f : timer
00000060-0000006f : keyboard
00000070-00000077 : rtc0
00000080-0000008f : dma page reg
000000a0-000000a1 : pic2
000000c0-000000df : dma2
00000170-00000177 : ata_piix
000001f0-000001f7 : ata_piix
000002f8-000002ff : serial
00000376-00000376 : ata_piix
00000378-0000037a : parport0
0000037b-0000037f : parport0
000003f6-000003f6 : ata_piix
000003f8-000003ff : serial
00001000-00ffffff : MSC PCI I/O
  00001000-0000103f : 0000:00:0a.3
  00001040-0000105f : 0000:00:0a.2
    00001040-0000105f : uhci_hcd
  00001060-0000107f : 0000:00:0b.0
    00001060-0000107f : pcnet32_probe_pci
  00001080-000010ff : 0000:00:12.0
    00001080-000010ff : defxx
  00001100-0000110f : 0000:00:0a.3
  00001400-000014ff : 0000:00:13.0
  00001800-0000180f : 0000:00:0a.1
    00001800-0000180f : ata_piix

As you can see there are holes in the map below 0x100, so e.g. if the bus 
master IDE I/O space registers (claimed last in the list by `ata_piix') 
were assigned to 00000030-0000003f, then all hell would break loose.  It 
is exactly the mapping that happened in the absence of the code piece in 
question IIRC.

 The choice of 0x1000 as the lower boundary IIRC has something to do with 
alignment; I think the decoding base has to be a multiple of 0x1000 and 
given that the ACPI resource is decoded by a non-standard BAR at 0x40 in 
the configuration space (set up by `malta_piix_func3_base_fixup' BTW) we 
just need to match its setting.

 Can you please check what the port I/O map looks like with your setup 
with and without your patch applied?

 NB there is still something fishy with the setup of SOC-it 101's PCI 
decoding windows, which is why I have forced `defxx' with a patch to use 
port I/O, as reported above.  The driver uses MMIO unconditionally on PCI 
systems nowadays, but using MMIO prevents it from working with the SOC-it 
101 system controller and I yet need to debug it.  Conversely MMIO used to 
work just fine with the Galileo GT64120A system controller while I still 
had one operational.

 HTH,

  Maciej

Re: [PATCH v2 03/24] MIPS: PCI: Use pci_enable_resources()

[Ilpo Järvinen]

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

On Tue, 14 Oct 2025, Maciej W. Rozycki wrote:

> On Mon, 13 Oct 2025, Thomas Bogendoerfer wrote:
> 
> > > This patch causes boot failures when trying to boot mips images from
> > > ide drive in qemu. As far as I can see the interface no longer instantiates.
> > > 
> > > Reverting this patch fixes the problem. Bisect log attached for reference.
> > 
> > Patch below fixes my qemu malta setup. Now I'm wondering, why this is
> > needed. It was added with commit
> > 
> > aa0980b80908 ("Fixes for system controllers for Atlas/Malta core cards.")
> > 
> > Maciej, do you remember why this is needed ?
> 
>  I do.  The reason is preventing PCI port I/O mappings below 0x100, which 
> interferes badly with how the PIIX4 decodes port I/O cycles.  That did 
> happen in the field, wreaking havoc and prompting my change.
> 
>  By the look of the code it would definitely trigger for the Bonito64 
> system controller, which has a fixed port I/O target address range and, 
> depending on the settings left by the firmware, it might also trigger for 
> the Galileo GT64120A and SOC-it 101 system controllers, which have 
> variable port I/O target address ranges.
> 
>  Here's an example map of Malta port I/O resources (SOC-it 101 variant):
> 
> 00000000-0000001f : dma1
> 00000020-00000021 : pic1
> 00000040-0000005f : timer
> 00000060-0000006f : keyboard
> 00000070-00000077 : rtc0
> 00000080-0000008f : dma page reg
> 000000a0-000000a1 : pic2
> 000000c0-000000df : dma2
> 00000170-00000177 : ata_piix
> 000001f0-000001f7 : ata_piix
> 000002f8-000002ff : serial
> 00000376-00000376 : ata_piix
> 00000378-0000037a : parport0
> 0000037b-0000037f : parport0
> 000003f6-000003f6 : ata_piix
> 000003f8-000003ff : serial
> 00001000-00ffffff : MSC PCI I/O
>   00001000-0000103f : 0000:00:0a.3
>   00001040-0000105f : 0000:00:0a.2
>     00001040-0000105f : uhci_hcd
>   00001060-0000107f : 0000:00:0b.0
>     00001060-0000107f : pcnet32_probe_pci
>   00001080-000010ff : 0000:00:12.0
>     00001080-000010ff : defxx
>   00001100-0000110f : 0000:00:0a.3
>   00001400-000014ff : 0000:00:13.0
>   00001800-0000180f : 0000:00:0a.1
>     00001800-0000180f : ata_piix
> 
> As you can see there are holes in the map below 0x100, so e.g. if the bus 
> master IDE I/O space registers (claimed last in the list by `ata_piix') 
> were assigned to 00000030-0000003f, then all hell would break loose.  It 
> is exactly the mapping that happened in the absence of the code piece in 
> question IIRC.
> 
>  The choice of 0x1000 as the lower boundary IIRC has something to do with 
> alignment; I think the decoding base has to be a multiple of 0x1000 and 
> given that the ACPI resource is decoded by a non-standard BAR at 0x40 in 
> the configuration space (set up by `malta_piix_func3_base_fixup' BTW) we 
> just need to match its setting.
> 
>  Can you please check what the port I/O map looks like with your setup 
> with and without your patch applied?
> 
>  NB there is still something fishy with the setup of SOC-it 101's PCI 
> decoding windows, which is why I have forced `defxx' with a patch to use 
> port I/O, as reported above.  The driver uses MMIO unconditionally on PCI 
> systems nowadays, but using MMIO prevents it from working with the SOC-it 
> 101 system controller and I yet need to debug it.  Conversely MMIO used to 
> work just fine with the Galileo GT64120A system controller while I still 
> had one operational.

Are you sure pci-malta.c has to do anything like this as 
pcibios_align_resource() does lower bound IO resource start addresses if 
PCIBIOS_MIN_IO is set?

How about this patch below?

(I'm not sure if it should actually be
	PCIBIOS_MIN_IO = 0x1000 - hose->io_resource->start;
to allow resources starting from 0x1000 if ->start is not at 0.)

--
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Tue, 14 Oct 2025 13:47:49 +0300
Subject: [PATCH 1/1] MIPS: Malta: Use pcibios_align_resource() to block io range
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

According to Maciej W. Rozycki <macro@orcam.me.uk>, the
mips_pcibios_init() for malta adjusts root bus IO resource start
address to prevent interfering with PIIX4 I/O cycle decoding. Adjusting
lower bound leaves PIIX4 IO resources outside of the root bus resource
and assign_fixed_resource_on_bus() does not put the resources into the
resource tree.

Prior to commit ae81aad5c2e1 ("MIPS: PCI: Use pci_enable_resources()")
the arch specific pcibios_enable_resources() did not check if the
resources were assigned which diverges from what PCI core checks,
effectively hiding the PIIX4 IO resources were not properly within the
resource tree. After starting to use pcibios_enable_resources() from
PCI core, enabling PIIX4 fails:

ata_piix 0000:00:0a.1: BAR 0 [io  0x01f0-0x01f7]: not claimed; can't enable device
ata_piix 0000:00:0a.1: probe with driver ata_piix failed with error -22

MIPS PCI code already has support for enforcing lower bounds using
PCIBIOS_MIN_IO in pcibios_align_resource(). Make malta PCI code too to
use PCIBIOS_MIN_IO.

Fixes: ae81aad5c2e1 ("MIPS: PCI: Use pci_enable_resources()")
Fixes: aa0980b80908 ("Fixes for system controllers for Atlas/Malta core cards.")
Link: https://lore.kernel.org/linux-pci/9085ab12-1559-4462-9b18-f03dcb9a4088@roeck-us.net/
Link: https://lore.kernel.org/linux-pci/alpine.DEB.2.21.2510132229120.39634@angie.orcam.me.uk/
Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 arch/mips/pci/pci-malta.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/pci/pci-malta.c b/arch/mips/pci/pci-malta.c
index 6aefdf20ca05..f4ea1c99852f 100644
--- a/arch/mips/pci/pci-malta.c
+++ b/arch/mips/pci/pci-malta.c
@@ -231,7 +231,7 @@ void __init mips_pcibios_init(void)
 
 	/* PIIX4 ACPI starts at 0x1000 */
 	if (controller->io_resource->start < 0x00001000UL)
-		controller->io_resource->start = 0x00001000UL;
+		PCIBIOS_MIN_IO = 0x1000;
 
 	iomem_resource.end &= 0xfffffffffULL;			/* 64 GB */
 	ioport_resource.end = controller->io_resource->end;

base-commit: 2f2c7254931f41b5736e3ba12aaa9ac1bbeeeb92
-- 
2.39.5

Re: [PATCH v2 03/24] MIPS: PCI: Use pci_enable_resources()

[Maciej W. Rozycki]

On Tue, 14 Oct 2025, Ilpo Järvinen wrote:

> > As you can see there are holes in the map below 0x100, so e.g. if the bus 
> > master IDE I/O space registers (claimed last in the list by `ata_piix') 
> > were assigned to 00000030-0000003f, then all hell would break loose.  It 
> > is exactly the mapping that happened in the absence of the code piece in 
> > question IIRC.
> 
> Are you sure pci-malta.c has to do anything like this as 
> pcibios_align_resource() does lower bound IO resource start addresses if 
> PCIBIOS_MIN_IO is set?

 Well, PCIBIOS_MIN_IO is never set for Malta and therefore stays at 0.  I 
could boot 2.6.11 with the hunk reverted and see what happens, not a big 
deal (I should have old GCC somewhere as a kernel such old would surely be 
a pain to build with modern GCC).  This stuff was badly broken before 
commit ae81aad5c2e1 (and there was support there too for the Atlas board, 
a weird one with a Philips SAA9730 southbridge and supporting a subset of 
the same CPU core cards as the Malta board does).

> How about this patch below?
> 
> (I'm not sure if it should actually be
> 	PCIBIOS_MIN_IO = 0x1000 - hose->io_resource->start;
> to allow resources starting from 0x1000 if ->start is not at 0.)

 I'd have to go through the relevant datasheets to see whether it can 
actually happen in reality.  Perhaps we can just hardwire PCIBIOS_MIN_IO 
to 0x1000 instead, similarly to what other MIPS platforms do.  After all 
Malta's southbridge is on the mainboard and therefore always the same, 
regardless of the northbridge (system controller in MIPS-speak) which 
comes with the pluggable CPU core card.

 NB there are commit c5de50dada14 ("MIPS: Malta: Change start address to 
avoid conflicts.") and commit 27547abf36af ("MIPS: malta: Incorporate 
PIIX4 ACPI I/O region in PCI controller resources") that fiddled with this 
code piece.  Especially the latter one refers additional commits that may 
give further insights.  And the former one removed a "FIXME" annotation, 
which suggests I didn't consider the solution perfect back 20 years ago, 
but given how long it stayed there it was surely good enough for its time.

  Maciej

Re: [PATCH v2 03/24] MIPS: PCI: Use pci_enable_resources()

[Ilpo Järvinen]

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

On Tue, 14 Oct 2025, Maciej W. Rozycki wrote:

> On Tue, 14 Oct 2025, Ilpo Järvinen wrote:
> 
> > > As you can see there are holes in the map below 0x100, so e.g. if the bus 
> > > master IDE I/O space registers (claimed last in the list by `ata_piix') 
> > > were assigned to 00000030-0000003f, then all hell would break loose.  It 
> > > is exactly the mapping that happened in the absence of the code piece in 
> > > question IIRC.
> > 
> > Are you sure pci-malta.c has to do anything like this as 
> > pcibios_align_resource() does lower bound IO resource start addresses if 
> > PCIBIOS_MIN_IO is set?
> 
>  Well, PCIBIOS_MIN_IO is never set for Malta and therefore stays at 0.

I meant whether pci-malta.c has to play with the ->start address at all 
if it would use PCIBIOS_MIN_IO.

> I could boot 2.6.11 with the hunk reverted and see what happens, not a big 
> deal (I should have old GCC somewhere as a kernel such old would surely be 
> a pain to build with modern GCC).  This stuff was badly broken before 
> commit ae81aad5c2e1 (and there was support there too for the Atlas board, 
> a weird one with a Philips SAA9730 southbridge and supporting a subset of 
> the same CPU core cards as the Malta board does).
> 
> > How about this patch below?
> > 
> > (I'm not sure if it should actually be
> > 	PCIBIOS_MIN_IO = 0x1000 - hose->io_resource->start;
> > to allow resources starting from 0x1000 if ->start is not at 0.)
> 
>  I'd have to go through the relevant datasheets to see whether it can 
> actually happen in reality.  Perhaps we can just hardwire PCIBIOS_MIN_IO 
> to 0x1000 instead, similarly to what other MIPS platforms do.

My patch did hardcode set it to 0x1000, I just noted before the patch that 
I'm not sure if the code should actually try to align the resulting "real 
start address" to 0x1000 if hose->io_resource->start != 0.

Or are you saying also the the if () check should be removed as well?

> After all 
> Malta's southbridge is on the mainboard and therefore always the same, 
> regardless of the northbridge (system controller in MIPS-speak) which 
> comes with the pluggable CPU core card.
> 
>  NB there are commit c5de50dada14 ("MIPS: Malta: Change start address to 
> avoid conflicts.") and commit 27547abf36af ("MIPS: malta: Incorporate 
> PIIX4 ACPI I/O region in PCI controller resources") that fiddled with this 
> code piece.  Especially the latter one refers additional commits that may 
> give further insights.  And the former one removed a "FIXME" annotation, 
> which suggests I didn't consider the solution perfect back 20 years ago, 
> but given how long it stayed there it was surely good enough for its time.

It was "good enough" only because the arch specific 
pcibios_enable_resources() was lacking the check for whether the resource 
truly got assigned or not. The PIIX4 driver must worked just fine without 
those IO resources which is what most drivers do despite using 
pci(m)_enable_device() and not pci_enable_device_mem() (the latter 
doesn't even seem to come with m variant).

-- 
 i.

Re: [PATCH v2 03/24] MIPS: PCI: Use pci_enable_resources()

[Maciej W. Rozycki]

On Tue, 14 Oct 2025, Ilpo Järvinen wrote:

> >  Well, PCIBIOS_MIN_IO is never set for Malta and therefore stays at 0.
> 
> I meant whether pci-malta.c has to play with the ->start address at all 
> if it would use PCIBIOS_MIN_IO.

 Yes, we need either, not both.

> >  I'd have to go through the relevant datasheets to see whether it can 
> > actually happen in reality.  Perhaps we can just hardwire PCIBIOS_MIN_IO 
> > to 0x1000 instead, similarly to what other MIPS platforms do.
> 
> My patch did hardcode set it to 0x1000, I just noted before the patch that 
> I'm not sure if the code should actually try to align the resulting "real 
> start address" to 0x1000 if hose->io_resource->start != 0.
> 
> Or are you saying also the the if () check should be removed as well?

 That's what I meant, sorry to be unclear.

> >  NB there are commit c5de50dada14 ("MIPS: Malta: Change start address to 
> > avoid conflicts.") and commit 27547abf36af ("MIPS: malta: Incorporate 
> > PIIX4 ACPI I/O region in PCI controller resources") that fiddled with this 
> > code piece.  Especially the latter one refers additional commits that may 
> > give further insights.  And the former one removed a "FIXME" annotation, 
> > which suggests I didn't consider the solution perfect back 20 years ago, 
> > but given how long it stayed there it was surely good enough for its time.
> 
> It was "good enough" only because the arch specific 
> pcibios_enable_resources() was lacking the check for whether the resource 
> truly got assigned or not. The PIIX4 driver must worked just fine without 
> those IO resources which is what most drivers do despite using 
> pci(m)_enable_device() and not pci_enable_device_mem() (the latter 
> doesn't even seem to come with m variant).

 As /proc/ioport contents indicate the resources did get assigned or there 
would be no claiming driver reported.  I'm sure I did double-check it back 
in the day too.

  Maciej

Re: [PATCH v2 03/24] MIPS: PCI: Use pci_enable_resources()

[Thomas Bogendoerfer]

On Tue, Oct 14, 2025 at 03:41:42PM +0300, Ilpo Järvinen wrote:
> [...]
> It was "good enough" only because the arch specific 
> pcibios_enable_resources() was lacking the check for whether the resource 
> truly got assigned or not. The PIIX4 driver must worked just fine without 
> those IO resources which is what most drivers do despite using 
> pci(m)_enable_device() and not pci_enable_device_mem() (the latter 
> doesn't even seem to come with m variant).

will you send a v2 of the patch ?

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH v2 03/24] MIPS: PCI: Use pci_enable_resources()

[Ilpo Järvinen]

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

On Fri, 17 Oct 2025, Thomas Bogendoerfer wrote:

> On Tue, Oct 14, 2025 at 03:41:42PM +0300, Ilpo Järvinen wrote:
> > [...]
> > It was "good enough" only because the arch specific 
> > pcibios_enable_resources() was lacking the check for whether the resource 
> > truly got assigned or not. The PIIX4 driver must worked just fine without 
> > those IO resources which is what most drivers do despite using 
> > pci(m)_enable_device() and not pci_enable_device_mem() (the latter 
> > doesn't even seem to come with m variant).
> 
> will you send a v2 of the patch ?

Without the the if ()? I can do that, I was unsure how people wanted to 
progress with this.

-- 
 i.

Re: [PATCH v2 03/24] MIPS: PCI: Use pci_enable_resources()

[Thomas Bogendoerfer]

On Fri, Oct 17, 2025 at 01:58:13PM +0300, Ilpo Järvinen wrote:
> On Fri, 17 Oct 2025, Thomas Bogendoerfer wrote:
> 
> > On Tue, Oct 14, 2025 at 03:41:42PM +0300, Ilpo Järvinen wrote:
> > > [...]
> > > It was "good enough" only because the arch specific 
> > > pcibios_enable_resources() was lacking the check for whether the resource 
> > > truly got assigned or not. The PIIX4 driver must worked just fine without 
> > > those IO resources which is what most drivers do despite using 
> > > pci(m)_enable_device() and not pci_enable_device_mem() (the latter 
> > > doesn't even seem to come with m variant).
> > 
> > will you send a v2 of the patch ?
> 
> Without the the if ()? I can do that, I was unsure how people wanted to 
> progress with this.

yes without the if(), thank you

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

WARNING at drivers/pci/setup-bus.c:2373, bisected to "PCI: Use pbus_select_window_for_type() during mem window sizing"

[Klaus Kudielka]

On Fri, 2025-08-29 at 16:11 +0300, Ilpo Järvinen wrote:
> __pci_bus_size_bridges() goes to great lengths of helping pbus_size_mem()
> in which types it should put into a particular bridge window, requiring
> passing up to three resource type into pbus_size_mem().
> 
> Instead of having complex logic in __pci_bus_size_bridges() and a
> non-straightforward interface between those functions, use
> pbus_select_window_for_type() and pbus_select_window() to find the correct
> bridge window and compare if the resources belong to that window.
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>


Hi,


this batch became mainline commit ae88d0b9c57f, and causes a warning when booting
on my Turris Omnia.

Device tree: arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts
PCI driver: pci-mvebu
Hardware status: The joint mPCIe / mSATA slot carries an mSATA drive, the other
two mPCIe slots carry WiFi cards.

As far as I can tell, hardware is operating nominally, so the warning looks like
a false positive.


*** relevant section of the boot log, at ae88d0b9c57f (first bad commit) ***

[    0.024347] mvebu-pcie soc:pcie: host bridge /soc/pcie ranges:
[    0.024372] mvebu-pcie soc:pcie:      MEM 0x00f1080000..0x00f1081fff -> 0x0000080000
[    0.024388] mvebu-pcie soc:pcie:      MEM 0x00f1040000..0x00f1041fff -> 0x0000040000
[    0.024401] mvebu-pcie soc:pcie:      MEM 0x00f1044000..0x00f1045fff -> 0x0000044000
[    0.024414] mvebu-pcie soc:pcie:      MEM 0x00f1048000..0x00f1049fff -> 0x0000048000
[    0.024427] mvebu-pcie soc:pcie:      MEM 0xffffffffffffffff..0x00fffffffe -> 0x0100000000
[    0.024439] mvebu-pcie soc:pcie:       IO 0xffffffffffffffff..0x00fffffffe -> 0x0100000000
[    0.024452] mvebu-pcie soc:pcie:      MEM 0xffffffffffffffff..0x00fffffffe -> 0x0200000000
[    0.024464] mvebu-pcie soc:pcie:       IO 0xffffffffffffffff..0x00fffffffe -> 0x0200000000
[    0.024476] mvebu-pcie soc:pcie:      MEM 0xffffffffffffffff..0x00fffffffe -> 0x0300000000
[    0.024488] mvebu-pcie soc:pcie:       IO 0xffffffffffffffff..0x00fffffffe -> 0x0300000000
[    0.024500] mvebu-pcie soc:pcie:      MEM 0xffffffffffffffff..0x00fffffffe -> 0x0400000000
[    0.024508] mvebu-pcie soc:pcie:       IO 0xffffffffffffffff..0x00fffffffe -> 0x0400000000
[    0.024698] mvebu-pcie soc:pcie: pcie1.0: Slot power limit 10.0W
[    0.024890] mvebu-pcie soc:pcie: pcie2.0: Slot power limit 10.0W
[    0.025099] mvebu-pcie soc:pcie: PCI host bridge to bus 0000:00
[    0.025107] pci_bus 0000:00: root bus resource [bus 00-ff]
[    0.025114] pci_bus 0000:00: root bus resource [mem 0xf1080000-0xf1081fff] (bus address [0x00080000-0x00081fff])
[    0.025120] pci_bus 0000:00: root bus resource [mem 0xf1040000-0xf1041fff] (bus address [0x00040000-0x00041fff])
[    0.025125] pci_bus 0000:00: root bus resource [mem 0xf1044000-0xf1045fff] (bus address [0x00044000-0x00045fff])
[    0.025135] pci_bus 0000:00: root bus resource [mem 0xf1048000-0xf1049fff] (bus address [0x00048000-0x00049fff])
[    0.025139] pci_bus 0000:00: root bus resource [mem 0xe0000000-0xe7ffffff]
[    0.025143] pci_bus 0000:00: root bus resource [io  0x1000-0xeffff]
[    0.025262] pci 0000:00:02.0: [11ab:6820] type 01 class 0x060400 PCIe Root Port
[    0.025277] pci 0000:00:02.0: PCI bridge to [bus 00]
[    0.025284] pci 0000:00:02.0:   bridge window [io  0x0000-0x0fff]
[    0.025289] pci 0000:00:02.0:   bridge window [mem 0x00000000-0x000fffff]
[    0.025484] /soc/pcie/pcie@2,0: Fixed dependency cycle(s) with /soc/pcie/pcie@2,0/interrupt-controller
[    0.025524] pci 0000:00:03.0: [11ab:6820] type 01 class 0x060400 PCIe Root Port
[    0.025537] pci 0000:00:03.0: PCI bridge to [bus 00]
[    0.025543] pci 0000:00:03.0:   bridge window [io  0x0000-0x0fff]
[    0.025547] pci 0000:00:03.0:   bridge window [mem 0x00000000-0x000fffff]
[    0.025665] /soc/pcie/pcie@3,0: Fixed dependency cycle(s) with /soc/pcie/pcie@3,0/interrupt-controller
[    0.026453] PCI: bus0: Fast back to back transfers disabled
[    0.026459] pci 0000:00:02.0: bridge configuration invalid ([bus 00-00]), reconfiguring
[    0.026466] pci 0000:00:03.0: bridge configuration invalid ([bus 00-00]), reconfiguring
[    0.026538] pci 0000:01:00.0: [168c:002e] type 00 class 0x028000 PCIe Legacy Endpoint
[    0.026577] pci 0000:01:00.0: BAR 0 [mem 0xc0000000-0xc000ffff 64bit]
[    0.026669] pci 0000:01:00.0: supports D1
[    0.026673] pci 0000:01:00.0: PME# supported from D0 D1 D3hot
[    0.026783] PCI: bus1: Fast back to back transfers disabled
[    0.026788] pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
[    0.026860] pci 0000:02:00.0: [168c:003c] type 00 class 0x028000 PCIe Endpoint
[    0.026898] pci 0000:02:00.0: BAR 0 [mem 0xc8000000-0xc81fffff 64bit]
[    0.026909] pci 0000:02:00.0: ROM [mem 0xc8200000-0xc820ffff pref]
[    0.026987] pci 0000:02:00.0: supports D1 D2
[    0.027083] PCI: bus2: Fast back to back transfers disabled
[    0.027088] pci_bus 0000:02: busn_res: [bus 02-ff] end is updated to 02
[    0.027107] pci 0000:00:03.0: bridge window [mem 0x00200000-0x003fffff] to [bus 02] add_size 200000 add_align 200000
[    0.027115] pci 0000:00:03.0: bridge window [mem 0x00200000-0x003fffff] to [bus 02] add_size 200000 add_align 200000
[    0.027131] pci 0000:00:03.0: bridge window [mem 0xe0000000-0xe03fffff]: assigned
[    0.027138] pci 0000:00:02.0: bridge window [mem 0xe0400000-0xe04fffff]: assigned
[    0.027146] pci 0000:01:00.0: BAR 0 [mem 0xe0400000-0xe040ffff 64bit]: assigned
[    0.027158] pci 0000:00:02.0: PCI bridge to [bus 01]
[    0.027165] pci 0000:00:02.0:   bridge window [mem 0xe0400000-0xe04fffff]
[    0.027178] pci 0000:02:00.0: BAR 0 [mem 0xe0000000-0xe01fffff 64bit]: assigned
[    0.027188] pci 0000:02:00.0: ROM [mem 0xe0200000-0xe020ffff pref]: assigned
[    0.027194] pci 0000:00:03.0: PCI bridge to [bus 02]
[    0.027199] pci 0000:00:03.0:   bridge window [mem 0xe0000000-0xe03fffff]
[    0.027208] ------------[ cut here ]------------
[    0.027211] WARNING: CPU: 0 PID: 1 at drivers/pci/setup-bus.c:2373 pci_assign_unassigned_root_bus_resources+0x1bc/0x234
[    0.027230] Modules linked in:
[    0.027238] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.17.0-rc1+ #49 NONE 
[    0.027246] Hardware name: Marvell Armada 380/385 (Device Tree)
[    0.027250] Call trace: 
[    0.027258]  unwind_backtrace from show_stack+0x10/0x14
[    0.027278]  show_stack from dump_stack_lvl+0x50/0x64
[    0.027288]  dump_stack_lvl from __warn+0x7c/0xd4
[    0.027301]  __warn from warn_slowpath_fmt+0x158/0x15c
[    0.027314]  warn_slowpath_fmt from pci_assign_unassigned_root_bus_resources+0x1bc/0x234
[    0.027328]  pci_assign_unassigned_root_bus_resources from pci_host_probe+0x50/0xb8
[    0.027341]  pci_host_probe from platform_probe+0x48/0x84
[    0.027351]  platform_probe from really_probe+0xc8/0x2c8
[    0.027364]  really_probe from driver_probe_device+0x38/0x114
[    0.027378]  driver_probe_device from __driver_attach+0x9c/0x194
[    0.027391]  __driver_attach from bus_for_each_dev+0x60/0x94
[    0.027404]  bus_for_each_dev from bus_add_driver+0xc8/0x1e8
[    0.027417]  bus_add_driver from driver_register+0x84/0x138
[    0.027426]  driver_register from do_one_initcall+0x44/0x268
[    0.027433]  do_one_initcall from kernel_init_freeable+0x258/0x2c8
[    0.027445]  kernel_init_freeable from kernel_init+0x1c/0x130
[    0.027458]  kernel_init from ret_from_fork+0x14/0x28
[    0.027466] Exception stack(0xf0831fb0 to 0xf0831ff8)
[    0.027472] 1fa0:                                     00000000 00000000 00000000 00000000
[    0.027477] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    0.027481] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    0.027485] ---[ end trace 0000000000000000 ]---
[    0.027489] pci_bus 0000:00: resource 4 [mem 0xf1080000-0xf1081fff]
[    0.027495] pci_bus 0000:00: resource 5 [mem 0xf1040000-0xf1041fff]
[    0.027500] pci_bus 0000:00: resource 6 [mem 0xf1044000-0xf1045fff]
[    0.027504] pci_bus 0000:00: resource 7 [mem 0xf1048000-0xf1049fff]
[    0.027508] pci_bus 0000:00: resource 8 [mem 0xe0000000-0xe7ffffff]
[    0.027512] pci_bus 0000:00: resource 9 [io  0x1000-0xeffff]
[    0.027517] pci_bus 0000:01: resource 1 [mem 0xe0400000-0xe04fffff]
[    0.027522] pci_bus 0000:02: resource 1 [mem 0xe0000000-0xe03fffff]




*** relevant section of the boot log, at 13016e15d595 (last good commit) ***

[    0.024666] mvebu-pcie soc:pcie: host bridge /soc/pcie ranges:
[    0.024690] mvebu-pcie soc:pcie:      MEM 0x00f1080000..0x00f1081fff -> 0x0000080000
[    0.024706] mvebu-pcie soc:pcie:      MEM 0x00f1040000..0x00f1041fff -> 0x0000040000
[    0.024719] mvebu-pcie soc:pcie:      MEM 0x00f1044000..0x00f1045fff -> 0x0000044000
[    0.024732] mvebu-pcie soc:pcie:      MEM 0x00f1048000..0x00f1049fff -> 0x0000048000
[    0.024745] mvebu-pcie soc:pcie:      MEM 0xffffffffffffffff..0x00fffffffe -> 0x0100000000
[    0.024757] mvebu-pcie soc:pcie:       IO 0xffffffffffffffff..0x00fffffffe -> 0x0100000000
[    0.024770] mvebu-pcie soc:pcie:      MEM 0xffffffffffffffff..0x00fffffffe -> 0x0200000000
[    0.024782] mvebu-pcie soc:pcie:       IO 0xffffffffffffffff..0x00fffffffe -> 0x0200000000
[    0.024794] mvebu-pcie soc:pcie:      MEM 0xffffffffffffffff..0x00fffffffe -> 0x0300000000
[    0.024806] mvebu-pcie soc:pcie:       IO 0xffffffffffffffff..0x00fffffffe -> 0x0300000000
[    0.024818] mvebu-pcie soc:pcie:      MEM 0xffffffffffffffff..0x00fffffffe -> 0x0400000000
[    0.024827] mvebu-pcie soc:pcie:       IO 0xffffffffffffffff..0x00fffffffe -> 0x0400000000
[    0.025022] mvebu-pcie soc:pcie: pcie1.0: Slot power limit 10.0W
[    0.025210] mvebu-pcie soc:pcie: pcie2.0: Slot power limit 10.0W
[    0.025451] mvebu-pcie soc:pcie: PCI host bridge to bus 0000:00
[    0.025459] pci_bus 0000:00: root bus resource [bus 00-ff]
[    0.025466] pci_bus 0000:00: root bus resource [mem 0xf1080000-0xf1081fff] (bus address [0x00080000-0x00081fff])
[    0.025472] pci_bus 0000:00: root bus resource [mem 0xf1040000-0xf1041fff] (bus address [0x00040000-0x00041fff])
[    0.025477] pci_bus 0000:00: root bus resource [mem 0xf1044000-0xf1045fff] (bus address [0x00044000-0x00045fff])
[    0.025482] pci_bus 0000:00: root bus resource [mem 0xf1048000-0xf1049fff] (bus address [0x00048000-0x00049fff])
[    0.025487] pci_bus 0000:00: root bus resource [mem 0xe0000000-0xe7ffffff]
[    0.025491] pci_bus 0000:00: root bus resource [io  0x1000-0xeffff]
[    0.025617] pci 0000:00:02.0: [11ab:6820] type 01 class 0x060400 PCIe Root Port
[    0.025633] pci 0000:00:02.0: PCI bridge to [bus 00]
[    0.025639] pci 0000:00:02.0:   bridge window [io  0x0000-0x0fff]
[    0.025644] pci 0000:00:02.0:   bridge window [mem 0x00000000-0x000fffff]
[    0.025794] /soc/pcie/pcie@2,0: Fixed dependency cycle(s) with /soc/pcie/pcie@2,0/interrupt-controller
[    0.025832] pci 0000:00:03.0: [11ab:6820] type 01 class 0x060400 PCIe Root Port
[    0.025844] pci 0000:00:03.0: PCI bridge to [bus 00]
[    0.025851] pci 0000:00:03.0:   bridge window [io  0x0000-0x0fff]
[    0.025855] pci 0000:00:03.0:   bridge window [mem 0x00000000-0x000fffff]
[    0.025968] /soc/pcie/pcie@3,0: Fixed dependency cycle(s) with /soc/pcie/pcie@3,0/interrupt-controller
[    0.026757] PCI: bus0: Fast back to back transfers disabled
[    0.026762] pci 0000:00:02.0: bridge configuration invalid ([bus 00-00]), reconfiguring
[    0.026769] pci 0000:00:03.0: bridge configuration invalid ([bus 00-00]), reconfiguring
[    0.026845] pci 0000:01:00.0: [168c:002e] type 00 class 0x028000 PCIe Legacy Endpoint
[    0.026884] pci 0000:01:00.0: BAR 0 [mem 0xc0000000-0xc000ffff 64bit]
[    0.026976] pci 0000:01:00.0: supports D1
[    0.026980] pci 0000:01:00.0: PME# supported from D0 D1 D3hot
[    0.027082] PCI: bus1: Fast back to back transfers disabled
[    0.027087] pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
[    0.027163] pci 0000:02:00.0: [168c:003c] type 00 class 0x028000 PCIe Endpoint
[    0.027201] pci 0000:02:00.0: BAR 0 [mem 0xc8000000-0xc81fffff 64bit]
[    0.027212] pci 0000:02:00.0: ROM [mem 0xc8200000-0xc820ffff pref]
[    0.027290] pci 0000:02:00.0: supports D1 D2
[    0.027381] PCI: bus2: Fast back to back transfers disabled
[    0.027386] pci_bus 0000:02: busn_res: [bus 02-ff] end is updated to 02
[    0.027405] pci 0000:00:03.0: bridge window [mem 0x00200000-0x003fffff] to [bus 02] add_size 200000 add_align 200000
[    0.027423] pci 0000:00:03.0: bridge window [mem 0xe0000000-0xe03fffff]: assigned
[    0.027430] pci 0000:00:02.0: bridge window [mem 0xe0400000-0xe04fffff]: assigned
[    0.027438] pci 0000:01:00.0: BAR 0 [mem 0xe0400000-0xe040ffff 64bit]: assigned
[    0.027450] pci 0000:00:02.0: PCI bridge to [bus 01]
[    0.027457] pci 0000:00:02.0:   bridge window [mem 0xe0400000-0xe04fffff]
[    0.027470] pci 0000:02:00.0: BAR 0 [mem 0xe0000000-0xe01fffff 64bit]: assigned
[    0.027481] pci 0000:02:00.0: ROM [mem 0xe0200000-0xe020ffff pref]: assigned
[    0.027487] pci 0000:00:03.0: PCI bridge to [bus 02]
[    0.027492] pci 0000:00:03.0:   bridge window [mem 0xe0000000-0xe03fffff]
[    0.027502] pci_bus 0000:00: resource 4 [mem 0xf1080000-0xf1081fff]
[    0.027507] pci_bus 0000:00: resource 5 [mem 0xf1040000-0xf1041fff]
[    0.027511] pci_bus 0000:00: resource 6 [mem 0xf1044000-0xf1045fff]
[    0.027515] pci_bus 0000:00: resource 7 [mem 0xf1048000-0xf1049fff]
[    0.027519] pci_bus 0000:00: resource 8 [mem 0xe0000000-0xe7ffffff]
[    0.027523] pci_bus 0000:00: resource 9 [io  0x1000-0xeffff]
[    0.027528] pci_bus 0000:01: resource 1 [mem 0xe0400000-0xe04fffff]
[    0.027532] pci_bus 0000:02: resource 1 [mem 0xe0000000-0xe03fffff]


#regzbot introduced: ae88d0b9c57f


Thanks, Klaus

Re: [PATCH v2 03/24] MIPS: PCI: Use pci_enable_resources()

[Maciej W. Rozycki]

On Tue, 14 Oct 2025, Maciej W. Rozycki wrote:

> > > As you can see there are holes in the map below 0x100, so e.g. if the bus 
> > > master IDE I/O space registers (claimed last in the list by `ata_piix') 
> > > were assigned to 00000030-0000003f, then all hell would break loose.  It 
> > > is exactly the mapping that happened in the absence of the code piece in 
> > > question IIRC.
> > 
> > Are you sure pci-malta.c has to do anything like this as 
> > pcibios_align_resource() does lower bound IO resource start addresses if 
> > PCIBIOS_MIN_IO is set?
> 
>  Well, PCIBIOS_MIN_IO is never set for Malta and therefore stays at 0.  I 
> could boot 2.6.11 with the hunk reverted and see what happens, not a big 
> deal (I should have old GCC somewhere as a kernel such old would surely be 
> a pain to build with modern GCC).  This stuff was badly broken before 
> commit ae81aad5c2e1 (and there was support there too for the Atlas board, 
> a weird one with a Philips SAA9730 southbridge and supporting a subset of 
> the same CPU core cards as the Malta board does).

 Well, it is a big deal after all, since I've lost my old CoreLV CPU card
and the replacement Core74K one is too new for 2.6.x both in terms of the 
CPU and the system controller.  No doubt with some patching it should be 
able to get it booted, but it's not worth the effort.

 So instead I've just removed the hunk with my most recent compilation and 
what I've got is:

ata1: PATA max UDMA/33 cmd 0x1f0 ctl 0x3f6 bmdma 0x30 irq 14 lpm-pol 0
ata2: PATA max UDMA/33 cmd 0x170 ctl 0x376 bmdma 0x38 irq 15 lpm-pol 0

(notice the 0x30/0x38 bmdma allocations, which just confirms my memory) 
and then a temporary hang, a couple of more lines printed and the system 
silently rebooted back into YAMON.

 Having configured with ATA and PCNET32 disabled I get this:

00000000-00ffffff : MSC PCI I/O
  00000000-0000001f : 0000:00:0a.2
  00000020-00000021 : pic1
  00000030-0000003f : 0000:00:0a.1
  00000040-0000005f : 0000:00:0b.0
  00000060-0000006f : i8042
  00000070-00000077 : rtc0
  000000a0-000000a1 : pic2
  00000170-00000177 : 0000:00:0a.1
  000001f0-000001f7 : 0000:00:0a.1
  000002f8-000002ff : serial
  00000376-00000376 : 0000:00:0a.1
  00000378-0000037a : parport0
  0000037b-0000037f : parport0
  000003f6-000003f6 : 0000:00:0a.1
  000003f8-000003ff : serial
  00000400-000004ff : 0000:00:13.0
  00000800-0000087f : 0000:00:12.0
    00000800-0000087f : defxx
  00001000-0000103f : 0000:00:0a.3
  00001100-0000110f : 0000:00:0a.3

which does look nasty (although technically correctly shows southbridge 
resources within the system controller's PCI I/O window).  At least the 
defxx driver still works with the FDDI network interface at its port I/O 
assignment so the system boots multiuser NFS-rooted.

  Maciej

Re: WARNING at drivers/pci/setup-bus.c:2373, bisected to "PCI: Use pbus_select_window_for_type() during mem window sizing"

[Klaus Kudielka]

On Sat, 2025-10-18 at 10:14 +0200, Klaus Kudielka wrote:

[...]


> Device tree: arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts
> PCI driver: pci-mvebu
> Hardware status: The joint mPCIe / mSATA slot carries an mSATA drive, the other
> two mPCIe slots carry WiFi cards.
> 
> As far as I can tell, hardware is operating nominally, so the warning looks like
> a false positive.


In the meantime, I stared a bit at the logs, and at the code.


WITH the offending commit, I see TWO identical lines before the WARNING:

> [    0.027107] pci 0000:00:03.0: bridge window [mem 0x00200000-0x003fffff] to [bus 02] add_size 200000 add_align 200000
> [    0.027115] pci 0000:00:03.0: bridge window [mem 0x00200000-0x003fffff] to [bus 02] add_size 200000 add_align 200000

So, this part of  pbus_size_mem() now seems to be called *TWICE* for the same bridge window:

		add_to_list(realloc_head, bus->self, b_res, size1-size0, add_align);
		pci_info(bus->self, "bridge window %pR to %pR add_size %llx add_align %llx\n",
			   b_res, &bus->busn_res,
			   (unsigned long long) (size1 - size0),
			   (unsigned long long) add_align);



WITHOUT the offending commit, I see only one line, and no WARNING.
> [    0.027405] pci 0000:00:03.0: bridge window [mem 0x00200000-0x003fffff] to [bus 02] add_size 200000 add_align 200000


This behavior change really looks suspicious to me (maybe resulting in two identical entries in the realloc_list).
Does that ring any bell?


Thanks, Klaus

Re: WARNING at drivers/pci/setup-bus.c:2373, bisected to "PCI: Use pbus_select_window_for_type() during mem window sizing"

[Klaus Kudielka]

On Sat, 2025-10-25 at 12:11 +0200, Klaus Kudielka wrote:
> 
> > [    0.027107] pci 0000:00:03.0: bridge window [mem 0x00200000-0x003fffff] to [bus 02] add_size 200000 add_align 200000
> > [    0.027115] pci 0000:00:03.0: bridge window [mem 0x00200000-0x003fffff] to [bus 02] add_size 200000 add_align 200000
> 
> So, this part of  pbus_size_mem() now seems to be called *TWICE* for the same bridge window:
> 
> 		add_to_list(realloc_head, bus->self, b_res, size1-size0, add_align);
> 		pci_info(bus->self, "bridge window %pR to %pR add_size %llx add_align %llx\n",
> 			   b_res, &bus->busn_res,
> 			   (unsigned long long) (size1 - size0),
> 			   (unsigned long long) add_align);
> 
> 
> 
> WITHOUT the offending commit, I see only one line, and no WARNING.
> > [    0.027405] pci 0000:00:03.0: bridge window [mem 0x00200000-0x003fffff] to [bus 02] add_size 200000 add_align 200000
> 
> 


After some more testing, I think I know what is going on.

- My device seems to have only non-prefetchable IO resources.
- In pci_bus_size_bridges(), pbus_size_mem() is called twice, once with IORESOURCE_PREFETCH, once without.
- This seems to be the intended behaviour (with or without the offending commit).

- What DOES make the difference, is the use of pbus_select_window_for_type() inside pbus_size_mem().
- On my device, that function returns the ***non-prefetchable*** resource, even if being asked for a prefetchable one.
- End result: b_res is valid (and identical) in both calls to pbus_size_mem().
- Honestly, that does not look right to me.


Indeed, my device goes back to the original behaviour (without WARNING), if I go back to the original use of
find_bus_resource_of_type():


--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1312,7 +1312,9 @@ static void pbus_size_mem(struct pci_bus *bus, unsigned long type,
        resource_size_t min_align, win_align, align, size, size0, size1 = 0;
        resource_size_t aligns[28]; /* Alignments from 1MB to 128TB */
        int order, max_order;
-       struct resource *b_res = pbus_select_window_for_type(bus, type);
+       struct resource *b_res = find_bus_resource_of_type(bus,
+                       IORESOURCE_MEM | IORESOURCE_PREFETCH | IORESOURCE_MEM_64,
+                       type);
        resource_size_t children_add_size = 0;
        resource_size_t children_add_align = 0;
        resource_size_t add_align = 0;



Comments?

Re: WARNING at drivers/pci/setup-bus.c:2373, bisected to "PCI: Use pbus_select_window_for_type() during mem window sizing"

[Ilpo Järvinen]

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

On Sat, 25 Oct 2025, Klaus Kudielka wrote:

> On Sat, 2025-10-25 at 12:11 +0200, Klaus Kudielka wrote:
> > 
> > > [    0.027107] pci 0000:00:03.0: bridge window [mem 0x00200000-0x003fffff] to [bus 02] add_size 200000 add_align 200000
> > > [    0.027115] pci 0000:00:03.0: bridge window [mem 0x00200000-0x003fffff] to [bus 02] add_size 200000 add_align 200000
> > 
> > So, this part of  pbus_size_mem() now seems to be called *TWICE* for the same bridge window:
> > 
> > 		add_to_list(realloc_head, bus->self, b_res, size1-size0, add_align);
> > 		pci_info(bus->self, "bridge window %pR to %pR add_size %llx add_align %llx\n",
> > 			   b_res, &bus->busn_res,
> > 			   (unsigned long long) (size1 - size0),
> > 			   (unsigned long long) add_align);
> > 
> > 
> > 
> > WITHOUT the offending commit, I see only one line, and no WARNING.
> > > [    0.027405] pci 0000:00:03.0: bridge window [mem 0x00200000-0x003fffff] to [bus 02] add_size 200000 add_align 200000
> > 
> > 
> 
> 
> After some more testing, I think I know what is going on.
> 
> - My device seems to have only non-prefetchable IO resources.
> - In pci_bus_size_bridges(), pbus_size_mem() is called twice, once with IORESOURCE_PREFETCH, once without.
> - This seems to be the intended behaviour (with or without the offending commit).
> 
> - What DOES make the difference, is the use of pbus_select_window_for_type() inside pbus_size_mem().
> - On my device, that function returns the ***non-prefetchable*** resource, even if being asked for a prefetchable one.
> - End result: b_res is valid (and identical) in both calls to pbus_size_mem().
> - Honestly, that does not look right to me.
> 
> 
> Indeed, my device goes back to the original behaviour (without WARNING), if I go back to the original use of
> find_bus_resource_of_type():
> 
> 
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1312,7 +1312,9 @@ static void pbus_size_mem(struct pci_bus *bus, unsigned long type,
>         resource_size_t min_align, win_align, align, size, size0, size1 = 0;
>         resource_size_t aligns[28]; /* Alignments from 1MB to 128TB */
>         int order, max_order;
> -       struct resource *b_res = pbus_select_window_for_type(bus, type);
> +       struct resource *b_res = find_bus_resource_of_type(bus,
> +                       IORESOURCE_MEM | IORESOURCE_PREFETCH | IORESOURCE_MEM_64,
> +                       type);
>         resource_size_t children_add_size = 0;
>         resource_size_t children_add_align = 0;
>         resource_size_t add_align = 0;
> 
> 
> 
> Comments?

Hi Klaus,

I'm sorry this ended up falling through cracks until now (this is far from 
the only regression I've on my table at the moment).

Big kudos for you from figuring out what went wrong! For a bridge which 
doesn't have a prefetchable window, pbus_size_mem() should not be called for it.

I've sent a fix patch separately. Please test it.

-- 
 i.

Re: [PATCH v2 03/24] MIPS: PCI: Use pci_enable_resources()

[Bjorn Helgaas]

On Mon, Oct 13, 2025 at 04:02:36PM -0500, Bjorn Helgaas wrote:
> On Mon, Oct 13, 2025 at 12:54:25PM -0700, Guenter Roeck wrote:
> > On Fri, Aug 29, 2025 at 04:10:52PM +0300, Ilpo Järvinen wrote:
> > > pci-legacy.c under MIPS has a copy of pci_enable_resources() named as
> > > pcibios_enable_resources(). Having own copy of same functionality could
> > > lead to inconsistencies in behavior, especially now as
> > > pci_enable_resources() and the bridge window resource flags behavior are
> > > going to be altered by upcoming changes.
> > > 
> > > The check for !r->start && r->end is already covered by the more generic
> > > checks done in pci_enable_resources().
> > > 
> > > Call pci_enable_resources() from MIPS's pcibios_enable_device() and remove
> > > pcibios_enable_resources().
> > > 
> > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > > Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > 
> > This patch causes boot failures when trying to boot mips images from
> > ide drive in qemu. As far as I can see the interface no longer instantiates.
> > 
> > Reverting this patch fixes the problem. Bisect log attached for reference.
> ...

> > # first bad commit: [ae81aad5c2e17fd1fafd930e75b81aedc837f705] MIPS: PCI: Use pci_enable_resources()
> 
> #regzbot introduced: ae81aad5c2e1 ("MIPS: PCI: Use pci_enable_resources()")

#regzbot fix: 1d5d1663619d ("MIPS: Malta: Fix PCI southbridge legacy resource reservations")
#regzbot fix: f294a5fd34db ("MIPS: Malta: Use pcibios_align_resource() to block io range")