sparclinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: "Andreas Larsson" <andreas@gaisler.com>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Geert Uytterhoeven" <geert@linux-m68k.org>,
	linux-m68k@lists.linux-m68k.org, linux-mips@vger.kernel.org,
	linux-pci@vger.kernel.org, sparclinux@vger.kernel.org,
	"Thomas Bogendoerfer" <tsbogend@alpha.franken.de>,
	"Christian König" <christian.koenig@amd.com>,
	"Yinghai Lu" <yinghai@kernel.org>,
	"Igor Mammedov" <imammedo@redhat.com>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	"Jonathan Cameron" <Jonathan.Cameron@huawei.com>,
	"Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	linux-kernel@vger.kernel.org
Cc: "Michał Winiarski" <michal.winiarski@intel.com>,
	linuxppc-dev@lists.ozlabs.org,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Subject: [PATCH 10/24] PCI: Preserve bridge window resource type flags
Date: Fri, 22 Aug 2025 17:55:51 +0300	[thread overview]
Message-ID: <20250822145605.18172-11-ilpo.jarvinen@linux.intel.com> (raw)
In-Reply-To: <20250822145605.18172-1-ilpo.jarvinen@linux.intel.com>

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 setup 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 ended up fixing an issue which too 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 | 88 ++++++++++++++++++++++++++---------------
 drivers/pci/setup-res.c |  3 ++
 5 files changed, 89 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..223f0e025407 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,7 +1799,6 @@ 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;
@@ -1767,8 +1807,6 @@ static void pci_bridge_release_resources(struct pci_bus *bus,
 	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 +2284,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


  parent reply	other threads:[~2025-08-22 14:57 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-22 14:55 [PATCH 00/24] PCI: Bridge window selection improvements Ilpo Järvinen
2025-08-22 14:55 ` [PATCH 01/24] m68k/PCI: Use pci_enable_resources() in pcibios_enable_device() Ilpo Järvinen
2025-08-22 14:55 ` [PATCH 02/24] sparc/PCI: Remove pcibios_enable_device() as they do nothing extra Ilpo Järvinen
2025-08-22 14:55 ` [PATCH 03/24] MIPS: PCI: Use pci_enable_resources() Ilpo Järvinen
2025-08-28  6:57   ` Thomas Bogendoerfer
2025-08-22 14:55 ` [PATCH 04/24] PCI: Move find_bus_resource_of_type() earlier Ilpo Järvinen
2025-08-22 14:55 ` [PATCH 05/24] PCI: Refactor find_bus_resource_of_type() logic checks Ilpo Järvinen
2025-08-22 14:55 ` [PATCH 06/24] PCI: Always claim bridge window before its setup Ilpo Järvinen
2025-08-22 14:55 ` [PATCH 07/24] PCI: Disable non-claimed bridge window Ilpo Järvinen
2025-08-22 14:55 ` [PATCH 08/24] PCI: Use pci_release_resource() instead of release_resource() Ilpo Järvinen
2025-08-22 14:55 ` [PATCH 09/24] PCI: Enable bridge even if bridge window fails to assign Ilpo Järvinen
2025-08-22 14:55 ` Ilpo Järvinen [this message]
2025-08-22 14:55 ` [PATCH 11/24] PCI: Add defines for bridge window indexing Ilpo Järvinen
2025-08-22 14:55 ` [PATCH 12/24] PCI: Add bridge window selection functions Ilpo Järvinen
2025-08-22 14:55 ` [PATCH 13/24] PCI: Fix finding bridge window in pci_reassign_bridge_resources() Ilpo Järvinen
2025-08-22 14:55 ` [PATCH 14/24] PCI: Warn if bridge window cannot be released when resizing BAR Ilpo Järvinen
2025-08-22 14:55 ` [PATCH 15/24] PCI: Use pbus_select_window() during BAR resize Ilpo Järvinen
2025-08-22 14:55 ` [PATCH 16/24] PCI: Use pbus_select_window_for_type() during IO window sizing Ilpo Järvinen
2025-08-22 14:55 ` [PATCH 17/24] PCI: Rename resource variable from r to res Ilpo Järvinen
2025-08-22 14:55 ` [PATCH 18/24] PCI: Use pbus_select_window() in space available checker Ilpo Järvinen
2025-08-22 14:56 ` [PATCH 19/24] PCI: Use pbus_select_window_for_type() during mem window sizing Ilpo Järvinen
2025-08-22 14:56 ` [PATCH 20/24] PCI: Refactor distributing available memory to use loops Ilpo Järvinen
2025-08-22 14:56 ` [PATCH 21/24] PCI: Refactor remove_dev_resources() to use pbus_select_window() Ilpo Järvinen
2025-08-22 14:56 ` [PATCH 22/24] PCI: Add pci_setup_one_bridge_window() Ilpo Järvinen
2025-08-22 14:56 ` [PATCH 23/24] PCI: Pass bridge window to pci_bus_release_bridge_resources() Ilpo Järvinen
2025-08-22 14:56 ` [PATCH 24/24] PCI: Alter misleading recursion " Ilpo Järvinen
2025-08-22 15:04 ` [PATCH 00/24] PCI: Bridge window selection improvements Ilpo Järvinen
2025-08-27 22:36 ` Bjorn Helgaas
2025-08-28 16:47   ` Ilpo Järvinen
2025-08-28 17:31     ` Bjorn Helgaas
2025-09-01  8:38   ` Geert Uytterhoeven

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=20250822145605.18172-11-ilpo.jarvinen@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=andreas@gaisler.com \
    --cc=bhelgaas@google.com \
    --cc=christian.koenig@amd.com \
    --cc=davem@davemloft.net \
    --cc=geert@linux-m68k.org \
    --cc=imammedo@redhat.com \
    --cc=kw@linux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-m68k@lists.linux-m68k.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=michal.winiarski@intel.com \
    --cc=rafael@kernel.org \
    --cc=sparclinux@vger.kernel.org \
    --cc=tsbogend@alpha.franken.de \
    --cc=yinghai@kernel.org \
    /path/to/YOUR_REPLY

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

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