[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250610102101.6496-3-ilpo.jarvinen@linux.intel.com>
Date: Tue, 10 Jun 2025 13:21:01 +0300
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Bjorn Helgaas <bhelgaas@...gle.com>,
linux-pci@...r.kernel.org,
Tudor Ambarus <tudor.ambarus@...aro.org>,
Rio <rio@....me>,
Krzysztof Wilczyński <kw@...ux.com>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
linux-kernel@...r.kernel.org
Cc: stable@...r.kernel.org
Subject: [PATCH 2/2] PCI: Fix pdev_resources_assignable() disparity
pdev_sort_resources() uses pdev_resources_assignable() helper to decide
if device's resources cannot be assigned. pbus_size_mem(), on the other
hand, does not do the same check. This could lead into a situation
where a resource ends up on realloc_head list but is not on the head
list, which is turn prevents emptying the resource from the
realloc_head list in __assign_resources_sorted().
A non-empty realloc_head is unacceptable because it triggers an
internal sanity check as show in this log with a device that has class
0 (PCI_CLASS_NOT_DEFINED):
pci 0001:01:00.0: [144d:a5a5] type 00 class 0x000000 PCIe Endpoint
pci 0001:01:00.0: BAR 0 [mem 0x00000000-0x000fffff 64bit]
pci 0001:01:00.0: ROM [mem 0x00000000-0x0000ffff pref]
pci 0001:01:00.0: enabling Extended Tags
pci 0001:01:00.0: PME# supported from D0 D3hot D3cold
pci 0001:01:00.0: 15.752 Gb/s available PCIe bandwidth, limited by 8.0 GT/s PCIe x2 link at 0001:00:00.0 (capable of 31.506 Gb/s with 16.0 GT/s PCIe x2 link)
pcieport 0001:00:00.0: bridge window [mem 0x00100000-0x001fffff] to [bus 01-ff] add_size 100000 add_align 100000
pcieport 0001:00:00.0: bridge window [mem 0x40000000-0x401fffff]: assigned
------------[ cut here ]------------
kernel BUG at drivers/pci/setup-bus.c:2532!
Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
...
Call trace:
pci_assign_unassigned_bus_resources+0x110/0x114 (P)
pci_rescan_bus+0x28/0x48
Use pdev_resources_assignable() also within pbus_size_mem() to skip
processing of non-assignable resources which removes the disparity in
between what resources pdev_sort_resources() and pbus_size_mem()
consider. As non-assignable resources are no longer processed, they are
not added to the realloc_head list, thus the sanity check no longer
triggers.
This disparity problem is very old but only now became apparent after
the commit 2499f5348431 ("PCI: Rework optional resource handling") that
made the ROM resources optional when calculating bridge window sizes
which required adding the resource to the realloc_head list.
Previously, bridge windows were just sized larger than necessary.
Fixes: 2499f5348431 ("PCI: Rework optional resource handling")
Reported-by: Tudor Ambarus <tudor.ambarus@...aro.org>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: <stable@...r.kernel.org>
---
The reporter was perhaps not happy with this fix as behavior of PCI core
isn't identical after this fix even if this patch fixes the problem on
the PCI core side which causes the internal sanity check to fire.
It seems that in the reporter's case, an out-of-tree driver was involved
that performed things and made assumptions a driver should not do in its
probe function such as assuming a bridge window is assigned even if there
are not child resources to be put into it (the child device in reporter's
case doesn't have a valid class and gets therefore skipped by the resource
fitting/assignment):
https://lore.kernel.org/all/bd579412-d07c-476d-8932-55c1f69adc9f@linaro.org/
In other words, the out-of-tree driver relies on the disparity in the
PCI core's resource fitting code which is now eliminated by this fix.
drivers/pci/setup-bus.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index f90d49cd07da..24863d8d0053 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1191,6 +1191,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
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))
--
2.39.5
Powered by blists - more mailing lists