* [PATCH 00/25] PCI: Resource fitting/assignment fixes and cleanups
@ 2025-02-14 9:59 ` Xiaochun Lee
0 siblings, 0 replies; 43+ messages in thread
From: Xiaochun Lee @ 2025-02-14 9:59 UTC (permalink / raw)
To: xiaocli, 740797925, Bjorn Helgaas, linux-pci,
Michał Winiarski, Igor Mammedov
Cc: Ilpo Järvinen, linux-kernel, Mika Westerberg
From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Hi all,
>
> This series focuses on PCI resource fitting and assignment algorithms.
> I've further changes in works to enable handling resizable BARs better
> during resource fitting built on top of these, but that's still WIP and
> this series seems way too large as is to have more stuff included.
>
> First there are small tweaks and fixes to the relaxed tail alignment
> code and applying the lessons learned to other similar cases. They are
> sort of independent of the rest. Then a large set of pure cleanups and
> refactoring that are not intended to make any functional changes.
> Finally, starting from "PCI: Extend enable to check for any optional
> resource" are again patches that aim to make behavioral changes to fix
> bridge window sizing to consider expansion ROM as an optional resource
> (to fix a remove/rescan cycle issue) and improve resource fitting
> algorithm in general.
>
> The series includes one of the change from Michał Winiarski
> <michal.winiarski@intel.com> as these changes also touch the same IOV
> checks.
>
> Please let me know if you'd prefer me to order the changes differently
> or split it into smaller chunks.
>
>
> I've extensively tested this series over the hosts in our lab which
> have quite heterogeneous PCI setup each. There were no losses of any
> important resource. Without pci=realloc, there's some churn in which of
> the disabled expansion ROMs gets a scarce memory space assigned (with
> pci=realloc, they are all assigned large enough bridge window).
>
>
> Ilpo Järvinen (24):
> PCI: Remove add_align overwrite unrelated to size0
> PCI: size0 is unrelated to add_align
> PCI: Simplify size1 assignment logic
> PCI: Optional bridge window size too may need relaxing
> PCI: Fix old_size lower bound in calculate_iosize() too
> PCI: Use SZ_* instead of literals in setup-bus.c
> PCI: resource_set_range/size() conversions
> PCI: Check resource_size() separately
> PCI: Add pci_resource_num() helper
> PCI: Add dev & res local variables to resource assignment funcs
> PCI: Converge return paths in __assign_resources_sorted()
> PCI: Refactor pdev_sort_resources() & __dev_sort_resources()
> PCI: Use while loop and break instead of gotos
> PCI: Rename retval to ret
> PCI: Consolidate assignment loop next round preparation
> PCI: Remove wrong comment from pci_reassign_resource()
> PCI: Add restore_dev_resource()
> PCI: Extend enable to check for any optional resource
> PCI: Always have realloc_head in __assign_resources_sorted()
> PCI: Indicate optional resource assignment failures
> PCI: Add debug print when releasing resources before retry
> PCI: Use res->parent to check is resource is assigned
> PCI: Perform reset_resource() and build fail list in sync
> PCI: Rework optional resource handling
>
> Michał Winiarski (1):
> PCI: Add a helper to identify IOV resources
>
> drivers/pci/pci.h | 44 +++-
> drivers/pci/setup-bus.c | 566 +++++++++++++++++++++++-----------------
> drivers/pci/setup-res.c | 8 +-
> 3 files changed, 364 insertions(+), 254 deletions(-)
>
Tested-by: Xiaochun XC17 Li <lixc17@lenovo.com>
> --
> 2.39.5
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 01/25] PCI: Remove add_align overwrite unrelated to size0
2025-02-14 9:59 ` Xiaochun Lee
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
Mika Westerberg, Ilpo Järvinen, linux-kernel
The commit 566f1dd52816 ("PCI: Relax bridge window tail sizing rules")
relaxed bridge window tail alignment rule for the non-optional part
(size0, no add_size/add_align). The change, however, also overwrite
add_align that is only related to case where optional size1 related
entry is added into realloc head.
Correct this by removing the add_align overwrite.
Fixes: 566f1dd52816 ("PCI: Relax bridge window tail sizing rules")
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 5e00cecf1f1a..d92832e1f4a1 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1149,7 +1149,6 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
min_align = 1ULL << (max_order + __ffs(SZ_1M));
min_align = max(min_align, win_align);
size0 = calculate_memsize(size, min_size, 0, 0, resource_size(b_res), win_align);
- add_align = win_align;
pci_info(bus->self, "bridge window %pR to %pR requires relaxed alignment rules\n",
b_res, &bus->busn_res);
}
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 02/25] PCI: size0 is unrelated to add_align
2025-02-14 9:59 ` Xiaochun Lee
(?)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
The commit 566f1dd52816 ("PCI: Relax bridge window tail sizing rules")
relaxed bridge window tail alignment rule for the non-optional part
(size0, no add_size/add_align). The required alignment given for
pbus_upstream_space_available(), however, was add_align which relates
only to size1 alignment.
As pbus_upstream_space_available() only selects between normal and
relaxed tail alignment of the bridge window, the different alignment
only makes relaxed tail alignment to be used more often than what was
intended which should be harmless because relaxed tail alignment itself
should work in all cases.
For consistency, change pbus_upstream_space_available() call to use
min_align which is the alignment that is going to be used for the bridge
window in the case where size0 sized allocation is attempted.
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 d92832e1f4a1..09c275f8d088 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1145,7 +1145,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
if (bus->self && size0 &&
!pbus_upstream_space_available(bus, mask | IORESOURCE_PREFETCH, type,
- size0, add_align)) {
+ size0, min_align)) {
min_align = 1ULL << (max_order + __ffs(SZ_1M));
min_align = max(min_align, win_align);
size0 = calculate_memsize(size, min_size, 0, 0, resource_size(b_res), win_align);
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 03/25] PCI: Simplify size1 assignment logic
2025-02-14 9:59 ` Xiaochun Lee
` (2 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
In pbus_size_io() and pbus_size_mem(), a complex ?: operation is
performed to set size1 which becomes easier to read when decomposed.
In the case of pbus_size_mem(), simply initializing size1 to zero
ensures the size1 checks work as expected.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 09c275f8d088..7f4680a23c13 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -927,9 +927,14 @@ 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);
- size1 = (!realloc_head || (realloc_head && !add_size && !children_add_size)) ? size0 :
- calculate_iosize(size, min_size, size1, add_size, children_add_size,
- resource_size(b_res), min_align);
+
+ size1 = size0;
+ if (realloc_head && (add_size > 0 || children_add_size > 0)) {
+ size1 = calculate_iosize(size, min_size, size1, add_size,
+ children_add_size, resource_size(b_res),
+ min_align);
+ }
+
if (!size0 && !size1) {
if (bus->self && (b_res->start || b_res->end))
pci_info(bus->self, "disabling bridge window %pR to %pR (unused)\n",
@@ -1058,7 +1063,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
struct list_head *realloc_head)
{
struct pci_dev *dev;
- resource_size_t min_align, win_align, align, size, size0, size1;
+ resource_size_t min_align, win_align, align, size, size0, size1 = 0;
resource_size_t aligns[24]; /* Alignments from 1MB to 8TB */
int order, max_order;
struct resource *b_res = find_bus_resource_of_type(bus,
@@ -1153,9 +1158,11 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
b_res, &bus->busn_res);
}
- size1 = (!realloc_head || (realloc_head && !add_size && !children_add_size)) ? size0 :
- calculate_memsize(size, min_size, add_size, children_add_size,
- resource_size(b_res), add_align);
+ if (realloc_head && (add_size > 0 || children_add_size > 0)) {
+ size1 = calculate_memsize(size, min_size, add_size, children_add_size,
+ resource_size(b_res), add_align);
+ }
+
if (!size0 && !size1) {
if (bus->self && (b_res->start || b_res->end))
pci_info(bus->self, "disabling bridge window %pR to %pR (unused)\n",
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 04/25] PCI: Optional bridge window size too may need relaxing
2025-02-14 9:59 ` Xiaochun Lee
` (3 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
Mika Westerberg, Ilpo Järvinen, linux-kernel
The commit 566f1dd52816 ("PCI: Relax bridge window tail sizing rules")
relaxed the bridge window requirements for non-optional size (size0)
but pbus_size_mem() also handles optional sizes (IOV resources) using
size1. This can manifest, e.g., as a failure to resize a BAR back to
its original size after it was first shrunk when device has a VF BAR
resource because the bridge window (size1) is enlarged beyond what is
strictly required to fit the downstream resources.
Allow using relaxed bridge window tail sizing rules also with the
optional resources (size1) so that the remove/realloc cycle during BAR
resize (smaller and back to the original size) does not fail
unexpectedly due to sudden increase in bridge window size demand.
Move also add_align calculation into more logical place next to size1
assignment as they are strongly related to each other.
Fixes: 566f1dd52816 ("PCI: Relax bridge window tail sizing rules")
Reported-by: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 7f4680a23c13..31f051cdac68 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1146,7 +1146,6 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
min_align = calculate_mem_align(aligns, max_order);
min_align = max(min_align, win_align);
size0 = calculate_memsize(size, min_size, 0, 0, resource_size(b_res), min_align);
- add_align = max(min_align, add_align);
if (bus->self && size0 &&
!pbus_upstream_space_available(bus, mask | IORESOURCE_PREFETCH, type,
@@ -1159,8 +1158,21 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
}
if (realloc_head && (add_size > 0 || children_add_size > 0)) {
+ add_align = max(min_align, add_align);
size1 = calculate_memsize(size, min_size, add_size, children_add_size,
resource_size(b_res), add_align);
+
+ if (bus->self && size1 &&
+ !pbus_upstream_space_available(bus, mask | IORESOURCE_PREFETCH, type,
+ size1, add_align)) {
+ min_align = 1ULL << (max_order + __ffs(SZ_1M));
+ min_align = max(min_align, win_align);
+ size1 = calculate_memsize(size, min_size, add_size, children_add_size,
+ resource_size(b_res), win_align);
+ pci_info(bus->self,
+ "bridge window %pR to %pR requires relaxed alignment rules\n",
+ b_res, &bus->busn_res);
+ }
}
if (!size0 && !size1) {
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 05/25] PCI: Fix old_size lower bound in calculate_iosize() too
2025-02-14 9:59 ` Xiaochun Lee
` (4 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
The commit 903534fa7d30 ("PCI: Fix resource double counting on remove &
rescan") fixed double counting of mem resources because of old_size
being applied too early.
Fix a similar counting bug on the io resource side.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 31f051cdac68..ca544fb83700 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -814,11 +814,9 @@ static resource_size_t calculate_iosize(resource_size_t size,
size = (size & 0xff) + ((size & ~0xffUL) << 2);
#endif
size = size + size1;
- if (size < old_size)
- size = old_size;
- size = ALIGN(max(size, add_size) + children_add_size, align);
- return size;
+ size = max(size, add_size) + children_add_size;
+ return ALIGN(max(size, old_size), align);
}
static resource_size_t calculate_memsize(resource_size_t size,
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 06/25] PCI: Use SZ_* instead of literals in setup-bus.c
2025-02-14 9:59 ` Xiaochun Lee
` (5 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
Convert literals in setup-bus.c to SZ_* defines that make the size more
human readable.
As the code is now self-explanatory, the comments about the size can be
eliminated.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index ca544fb83700..303c4fbf2d14 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -841,9 +841,9 @@ resource_size_t __weak pcibios_window_alignment(struct pci_bus *bus,
return 1;
}
-#define PCI_P2P_DEFAULT_MEM_ALIGN 0x100000 /* 1MiB */
-#define PCI_P2P_DEFAULT_IO_ALIGN 0x1000 /* 4KiB */
-#define PCI_P2P_DEFAULT_IO_ALIGN_1K 0x400 /* 1KiB */
+#define PCI_P2P_DEFAULT_MEM_ALIGN SZ_1M
+#define PCI_P2P_DEFAULT_IO_ALIGN SZ_4K
+#define PCI_P2P_DEFAULT_IO_ALIGN_1K SZ_1K
static resource_size_t window_alignment(struct pci_bus *bus, unsigned long type)
{
@@ -908,7 +908,7 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size,
continue;
r_size = resource_size(r);
- if (r_size < 0x400)
+ if (r_size < SZ_1K)
/* Might be re-aligned for ISA */
size += r_size;
else
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 07/25] PCI: resource_set_range/size() conversions
2025-02-14 9:59 ` Xiaochun Lee
` (6 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
A few sites that could use resource_set_range/size() in setup-bus.c
were not picked up earlier due to them no matching the usual pattern.
Convert them now.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 303c4fbf2d14..dd9b06947621 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -413,10 +413,8 @@ static void __assign_resources_sorted(struct list_head *head,
* consistent.
*/
if (add_align > dev_res->res->start) {
- resource_size_t r_size = resource_size(dev_res->res);
-
- dev_res->res->start = add_align;
- dev_res->res->end = add_align + r_size - 1;
+ resource_set_range(dev_res->res, add_align,
+ resource_size(dev_res->res));
list_for_each_entry(dev_res2, head, list) {
align = pci_resource_alignment(dev_res2->dev,
@@ -1100,7 +1098,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
if (realloc_head && i >= PCI_IOV_RESOURCES &&
i <= PCI_IOV_RESOURCE_END) {
add_align = max(pci_resource_alignment(dev, r), add_align);
- r->end = r->start - 1;
+ resource_set_size(r, 0);
add_to_list(realloc_head, dev, r, r_size, 0 /* Don't care */);
children_add_size += r_size;
continue;
@@ -1180,8 +1178,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
b_res->flags = 0;
return 0;
}
- b_res->start = min_align;
- b_res->end = size0 + min_align - 1;
+
+ resource_set_range(b_res, min_align, size0);
b_res->flags |= IORESOURCE_STARTALIGN;
if (bus->self && size1 > size0 && realloc_head) {
add_to_list(realloc_head, bus->self, b_res, size1-size0, add_align);
@@ -1656,8 +1654,7 @@ static void pci_bridge_release_resources(struct pci_bus *bus,
pci_info(dev, "resource %d %pR released\n",
PCI_BRIDGE_RESOURCES + idx, r);
/* Keep the old size */
- r->end = resource_size(r) - 1;
- r->start = 0;
+ resource_set_range(r, 0, resource_size(r));
r->flags = 0;
/* Avoiding touch the one without PREF */
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 08/25] PCI: Add a helper to identify IOV resources
2025-02-14 9:59 ` Xiaochun Lee
` (7 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen, Christian König
From: Michał Winiarski <michal.winiarski@intel.com>
There are multiple places where special handling is required for IOV
resources.
Extract it to pci_resource_is_iov() helper and drop a few ifdefs.
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
drivers/pci/pci.h | 19 +++++++++++++++----
drivers/pci/setup-bus.c | 7 +++----
drivers/pci/setup-res.c | 4 +---
3 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 2e40fc63ba31..25bae4bfebea 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -621,6 +621,10 @@ void pci_iov_update_resource(struct pci_dev *dev, int resno);
resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev, int resno);
void pci_restore_iov_state(struct pci_dev *dev);
int pci_iov_bus_range(struct pci_bus *bus);
+static inline bool pci_resource_is_iov(int resno)
+{
+ return resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END;
+}
extern const struct attribute_group sriov_pf_dev_attr_group;
extern const struct attribute_group sriov_vf_dev_attr_group;
#else
@@ -630,12 +634,21 @@ static inline int pci_iov_init(struct pci_dev *dev)
}
static inline void pci_iov_release(struct pci_dev *dev) { }
static inline void pci_iov_remove(struct pci_dev *dev) { }
+static inline void pci_iov_update_resource(struct pci_dev *dev, int resno) { }
+static inline resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev,
+ int resno)
+{
+ return 0;
+}
static inline void pci_restore_iov_state(struct pci_dev *dev) { }
static inline int pci_iov_bus_range(struct pci_bus *bus)
{
return 0;
}
-
+static inline bool pci_resource_is_iov(int resno)
+{
+ return false;
+}
#endif /* CONFIG_PCI_IOV */
#ifdef CONFIG_PCIE_TPH
@@ -669,12 +682,10 @@ unsigned long pci_cardbus_resource_alignment(struct resource *);
static inline resource_size_t pci_resource_alignment(struct pci_dev *dev,
struct resource *res)
{
-#ifdef CONFIG_PCI_IOV
int resno = res - dev->resource;
- if (resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END)
+ if (pci_resource_is_iov(resno))
return pci_sriov_resource_alignment(dev, resno);
-#endif
if (dev->class >> 8 == PCI_CLASS_BRIDGE_CARDBUS)
return pci_cardbus_resource_alignment(res);
return resource_alignment(res);
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index dd9b06947621..3907930da00d 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1093,17 +1093,16 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
(r->flags & mask) != type3))
continue;
r_size = resource_size(r);
-#ifdef CONFIG_PCI_IOV
+
/* Put SRIOV requested res to the optional list */
- if (realloc_head && i >= PCI_IOV_RESOURCES &&
- i <= PCI_IOV_RESOURCE_END) {
+ if (realloc_head && pci_resource_is_iov(i)) {
add_align = max(pci_resource_alignment(dev, r), add_align);
resource_set_size(r, 0);
add_to_list(realloc_head, dev, r, r_size, 0 /* Don't care */);
children_add_size += r_size;
continue;
}
-#endif
+
/*
* aligns[0] is for 1MB (since bridge memory
* windows are always at least 1MB aligned), so
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index ca14576bf2bf..79c7ef349856 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -127,10 +127,8 @@ void pci_update_resource(struct pci_dev *dev, int resno)
{
if (resno <= PCI_ROM_RESOURCE)
pci_std_update_resource(dev, resno);
-#ifdef CONFIG_PCI_IOV
- else if (resno >= PCI_IOV_RESOURCES && resno <= PCI_IOV_RESOURCE_END)
+ else if (pci_resource_is_iov(resno))
pci_iov_update_resource(dev, resno);
-#endif
}
int pci_claim_resource(struct pci_dev *dev, int resource)
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 09/25] PCI: Check resource_size() separately
2025-02-14 9:59 ` Xiaochun Lee
` (8 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
Instead of chaining logic inside if () condition so that multiple lines
are required, make !resource_size() a separate check and use continue.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 3907930da00d..63c134b087d5 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -285,8 +285,11 @@ static void assign_requested_resources_sorted(struct list_head *head,
list_for_each_entry(dev_res, head, list) {
res = dev_res->res;
idx = res - &dev_res->dev->resource[0];
- if (resource_size(res) &&
- pci_assign_resource(dev_res->dev, idx)) {
+
+ if (!resource_size(res))
+ continue;
+
+ if (pci_assign_resource(dev_res->dev, idx)) {
if (fail_head) {
/*
* If the failed resource is a ROM BAR and
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 10/25] PCI: Add pci_resource_num() helper
2025-02-14 9:59 ` Xiaochun Lee
` (9 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
A few places in PCI code, mainly in setup-bus.c, need to reverse lookup
the index of a resource in pci_dev's resource array. Create
pci_resource_num() helper to avoid repeating the pointer arithmetic
trick used to calculate the index.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/pci.h | 24 +++++++++++++++++++++++-
drivers/pci/setup-bus.c | 10 +++++-----
2 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 25bae4bfebea..0b722d158b6a 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -330,6 +330,28 @@ void pci_walk_bus_locked(struct pci_bus *top,
const char *pci_resource_name(struct pci_dev *dev, unsigned int i);
+/**
+ * pci_resource_num - Reverse lookup resource number from device resources
+ * @dev: PCI device
+ * @res: Resource to lookup index for (MUST be a @dev's resource)
+ *
+ * Perform reverse lookup to determine the resource number for @res within
+ * @dev resource array. NOTE: The caller is responsible for ensuring @res is
+ * among @dev's resources!
+ *
+ * Returns: resource number.
+ */
+static inline int pci_resource_num(const struct pci_dev *dev,
+ const struct resource *res)
+{
+ int resno = res - &dev->resource[0];
+
+ /* Passing a resource that is not among dev's resources? */
+ WARN_ON_ONCE(resno >= PCI_NUM_RESOURCES);
+
+ return resno;
+}
+
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);
@@ -682,7 +704,7 @@ unsigned long pci_cardbus_resource_alignment(struct resource *);
static inline resource_size_t pci_resource_alignment(struct pci_dev *dev,
struct resource *res)
{
- int resno = res - dev->resource;
+ int resno = pci_resource_num(dev, res);
if (pci_resource_is_iov(resno))
return pci_sriov_resource_alignment(dev, resno);
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 63c134b087d5..8831365418d6 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -242,7 +242,7 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
if (!found_match) /* Just skip */
continue;
- idx = res - &add_res->dev->resource[0];
+ idx = pci_resource_num(add_res->dev, res);
res_name = pci_resource_name(add_res->dev, idx);
add_size = add_res->add_size;
align = add_res->min_align;
@@ -284,7 +284,7 @@ static void assign_requested_resources_sorted(struct list_head *head,
list_for_each_entry(dev_res, head, list) {
res = dev_res->res;
- idx = res - &dev_res->dev->resource[0];
+ idx = pci_resource_num(dev_res->dev, res);
if (!resource_size(res))
continue;
@@ -2211,7 +2211,7 @@ void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus)
res->flags = fail_res->flags;
if (pci_is_bridge(fail_res->dev)) {
- idx = res - &fail_res->dev->resource[0];
+ idx = pci_resource_num(fail_res->dev, res);
if (idx >= PCI_BRIDGE_RESOURCES &&
idx <= PCI_BRIDGE_RESOURCE_END)
res->flags = 0;
@@ -2295,7 +2295,7 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
res->flags = fail_res->flags;
if (pci_is_bridge(fail_res->dev)) {
- idx = res - &fail_res->dev->resource[0];
+ idx = pci_resource_num(fail_res->dev, res);
if (idx >= PCI_BRIDGE_RESOURCES &&
idx <= PCI_BRIDGE_RESOURCE_END)
res->flags = 0;
@@ -2402,7 +2402,7 @@ int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
struct resource *res = dev_res->res;
bridge = dev_res->dev;
- i = res - bridge->resource;
+ i = pci_resource_num(bridge, res);
res->start = dev_res->start;
res->end = dev_res->end;
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 11/25] PCI: Add dev & res local variables to resource assignment funcs
2025-02-14 9:59 ` Xiaochun Lee
` (10 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
Many PCI resource allocation related functions process struct
pci_dev_resource items which hold the struct pci_dev and resource
pointers. Reduce the number of lines that need indirection by adding
'dev' and 'res' local variable to hold the pointers.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 66 +++++++++++++++++++++++------------------
1 file changed, 37 insertions(+), 29 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 8831365418d6..6b4318da1147 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -216,10 +216,11 @@ static inline void reset_resource(struct resource *res)
static void reassign_resources_sorted(struct list_head *realloc_head,
struct list_head *head)
{
- struct resource *res;
- const char *res_name;
struct pci_dev_resource *add_res, *tmp;
struct pci_dev_resource *dev_res;
+ struct pci_dev *dev;
+ struct resource *res;
+ const char *res_name;
resource_size_t add_size, align;
int idx;
@@ -227,6 +228,7 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
bool found_match = false;
res = add_res->res;
+ dev = add_res->dev;
/* Skip resource that has been reset */
if (!res->flags)
@@ -242,20 +244,19 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
if (!found_match) /* Just skip */
continue;
- idx = pci_resource_num(add_res->dev, res);
- res_name = pci_resource_name(add_res->dev, idx);
+ idx = pci_resource_num(dev, res);
+ res_name = pci_resource_name(dev, idx);
add_size = add_res->add_size;
align = add_res->min_align;
if (!resource_size(res)) {
resource_set_range(res, align, add_size);
- if (pci_assign_resource(add_res->dev, idx))
+ if (pci_assign_resource(dev, idx))
reset_resource(res);
} else {
res->flags |= add_res->flags &
(IORESOURCE_STARTALIGN|IORESOURCE_SIZEALIGN);
- if (pci_reassign_resource(add_res->dev, idx,
- add_size, align))
- pci_info(add_res->dev, "%s %pR: failed to add %llx\n",
+ if (pci_reassign_resource(dev, idx, add_size, align))
+ pci_info(dev, "%s %pR: failed to add %llx\n",
res_name, res,
(unsigned long long) add_size);
}
@@ -278,18 +279,20 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
static void assign_requested_resources_sorted(struct list_head *head,
struct list_head *fail_head)
{
- struct resource *res;
struct pci_dev_resource *dev_res;
+ struct resource *res;
+ struct pci_dev *dev;
int idx;
list_for_each_entry(dev_res, head, list) {
res = dev_res->res;
- idx = pci_resource_num(dev_res->dev, res);
+ dev = dev_res->dev;
+ idx = pci_resource_num(dev, res);
if (!resource_size(res))
continue;
- if (pci_assign_resource(dev_res->dev, idx)) {
+ if (pci_assign_resource(dev, idx)) {
if (fail_head) {
/*
* If the failed resource is a ROM BAR and
@@ -298,8 +301,7 @@ static void assign_requested_resources_sorted(struct list_head *head,
*/
if (!((idx == PCI_ROM_RESOURCE) &&
(!(res->flags & IORESOURCE_ROM_ENABLE))))
- add_to_list(fail_head,
- dev_res->dev, res,
+ add_to_list(fail_head, dev, res,
0 /* don't care */,
0 /* don't care */);
}
@@ -377,6 +379,7 @@ static void __assign_resources_sorted(struct list_head *head,
LIST_HEAD(local_fail_head);
struct pci_dev_resource *save_res;
struct pci_dev_resource *dev_res, *tmp_res, *dev_res2;
+ struct resource *res;
unsigned long fail_type;
resource_size_t add_align, align;
@@ -394,8 +397,9 @@ static void __assign_resources_sorted(struct list_head *head,
/* Update res in head list with add_size in realloc_head list */
list_for_each_entry_safe(dev_res, tmp_res, head, list) {
- dev_res->res->end += get_res_add_size(realloc_head,
- dev_res->res);
+ res = dev_res->res;
+
+ res->end += get_res_add_size(realloc_head, res);
/*
* There are two kinds of additional resources in the list:
@@ -403,10 +407,10 @@ static void __assign_resources_sorted(struct list_head *head,
* 2. SR-IOV resource -- IORESOURCE_SIZEALIGN
* Here just fix the additional alignment for bridge
*/
- if (!(dev_res->res->flags & IORESOURCE_STARTALIGN))
+ if (!(res->flags & IORESOURCE_STARTALIGN))
continue;
- add_align = get_res_add_align(realloc_head, dev_res->res);
+ add_align = get_res_add_align(realloc_head, res);
/*
* The "head" list is sorted by alignment so resources with
@@ -415,9 +419,8 @@ static void __assign_resources_sorted(struct list_head *head,
* need to reorder the list by alignment to make it
* consistent.
*/
- if (add_align > dev_res->res->start) {
- resource_set_range(dev_res->res, add_align,
- resource_size(dev_res->res));
+ if (add_align > res->start) {
+ resource_set_range(res, add_align, resource_size(res));
list_for_each_entry(dev_res2, head, list) {
align = pci_resource_alignment(dev_res2->dev,
@@ -448,24 +451,29 @@ static void __assign_resources_sorted(struct list_head *head,
/* Check failed type */
fail_type = pci_fail_res_type_mask(&local_fail_head);
/* Remove not need to be released assigned res from head list etc */
- list_for_each_entry_safe(dev_res, tmp_res, head, list)
- if (dev_res->res->parent &&
- !pci_need_to_release(fail_type, dev_res->res)) {
+ list_for_each_entry_safe(dev_res, tmp_res, head, list) {
+ res = dev_res->res;
+
+ if (res->parent && !pci_need_to_release(fail_type, res)) {
/* Remove it from realloc_head list */
- remove_from_list(realloc_head, dev_res->res);
- remove_from_list(&save_head, dev_res->res);
+ remove_from_list(realloc_head, res);
+ remove_from_list(&save_head, res);
list_del(&dev_res->list);
kfree(dev_res);
}
+ }
free_list(&local_fail_head);
/* Release assigned resource */
- list_for_each_entry(dev_res, head, list)
- if (dev_res->res->parent)
- release_resource(dev_res->res);
+ list_for_each_entry(dev_res, head, list) {
+ res = dev_res->res;
+
+ if (res->parent)
+ release_resource(res);
+ }
/* Restore start/end/flags from saved list */
list_for_each_entry(save_res, &save_head, list) {
- struct resource *res = save_res->res;
+ res = save_res->res;
res->start = save_res->start;
res->end = save_res->end;
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 12/25] PCI: Converge return paths in __assign_resources_sorted()
2025-02-14 9:59 ` Xiaochun Lee
` (11 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
All return paths want to free head list in __assign_resources_sorted()
so add a label and use goto.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 6b4318da1147..ad7bc6166b23 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -444,8 +444,7 @@ static void __assign_resources_sorted(struct list_head *head,
list_for_each_entry(dev_res, head, list)
remove_from_list(realloc_head, dev_res->res);
free_list(&save_head);
- free_list(head);
- return;
+ goto out;
}
/* Check failed type */
@@ -488,6 +487,8 @@ static void __assign_resources_sorted(struct list_head *head,
/* Try to satisfy any additional optional resource requests */
if (realloc_head)
reassign_resources_sorted(realloc_head, head);
+
+out:
free_list(head);
}
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 13/25] PCI: Refactor pdev_sort_resources() & __dev_sort_resources()
2025-02-14 9:59 ` Xiaochun Lee
` (12 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
Reduce level of call nesting by calling pdev_sort_resources() directly
and by moving the tests done inside __dev_sort_resources() into
pdev_resources_assignable() helper.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 44 +++++++++++++++++++++--------------------
1 file changed, 23 insertions(+), 21 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index ad7bc6166b23..ba935a050be3 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -127,12 +127,33 @@ static resource_size_t get_res_add_align(struct list_head *head,
return dev_res ? dev_res->min_align : 0;
}
+static bool pdev_resources_assignable(struct pci_dev *dev)
+{
+ u16 class = dev->class >> 8, command;
+
+ /* Don't touch classless devices or host bridges or IOAPICs */
+ if (class == PCI_CLASS_NOT_DEFINED || class == PCI_CLASS_BRIDGE_HOST)
+ return false;
+
+ /* Don't touch IOAPIC devices already enabled by firmware */
+ if (class == PCI_CLASS_SYSTEM_PIC) {
+ pci_read_config_word(dev, PCI_COMMAND, &command);
+ if (command & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
+ return false;
+ }
+
+ return true;
+}
+
/* Sort resources by alignment */
static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head)
{
struct resource *r;
int i;
+ if (!pdev_resources_assignable(dev))
+ return;
+
pci_dev_for_each_resource(dev, r, i) {
const char *r_name = pci_resource_name(dev, i);
struct pci_dev_resource *dev_res, *tmp;
@@ -176,25 +197,6 @@ static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head)
}
}
-static void __dev_sort_resources(struct pci_dev *dev, struct list_head *head)
-{
- u16 class = dev->class >> 8;
-
- /* Don't touch classless devices or host bridges or IOAPICs */
- if (class == PCI_CLASS_NOT_DEFINED || class == PCI_CLASS_BRIDGE_HOST)
- return;
-
- /* Don't touch IOAPIC devices already enabled by firmware */
- if (class == PCI_CLASS_SYSTEM_PIC) {
- u16 command;
- pci_read_config_word(dev, PCI_COMMAND, &command);
- if (command & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
- return;
- }
-
- pdev_sort_resources(dev, head);
-}
-
static inline void reset_resource(struct resource *res)
{
res->start = 0;
@@ -498,7 +500,7 @@ static void pdev_assign_resources_sorted(struct pci_dev *dev,
{
LIST_HEAD(head);
- __dev_sort_resources(dev, &head);
+ pdev_sort_resources(dev, &head);
__assign_resources_sorted(&head, add_head, fail_head);
}
@@ -511,7 +513,7 @@ static void pbus_assign_resources_sorted(const struct pci_bus *bus,
LIST_HEAD(head);
list_for_each_entry(dev, &bus->devices, bus_list)
- __dev_sort_resources(dev, &head);
+ pdev_sort_resources(dev, &head);
__assign_resources_sorted(&head, realloc_head, fail_head);
}
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 14/25] PCI: Use while loop and break instead of gotos
2025-02-14 9:59 ` Xiaochun Lee
` (13 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
pci_assign_unassigned_root_bus_resources() and
pci_assign_unassigned_bridge_resources() contain ad-hoc loops using
backwards goto and gotos out of the loop. Replace them with while loops
and break statements.
While reindenting the loop bodies, do a few coding style tweaks (add
braces & remove parenthesis).
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 212 ++++++++++++++++++++--------------------
1 file changed, 106 insertions(+), 106 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index ba935a050be3..bbe3472cfba9 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -2162,78 +2162,79 @@ void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus)
max_depth, pci_try_num);
}
-again:
- /*
- * Last try will use add_list, otherwise will try good to have as must
- * have, so can realloc parent bridge resource
- */
- if (tried_times + 1 == pci_try_num)
- add_list = &realloc_head;
- /*
- * Depth first, calculate sizes and alignments of all subordinate buses.
- */
- __pci_bus_size_bridges(bus, add_list);
+ while (1) {
+ /*
+ * Last try will use add_list, otherwise will try good to
+ * have as must have, so can realloc parent bridge resource
+ */
+ if (tried_times + 1 == pci_try_num)
+ add_list = &realloc_head;
+ /*
+ * Depth first, calculate sizes and alignments of all
+ * subordinate buses.
+ */
+ __pci_bus_size_bridges(bus, add_list);
- pci_root_bus_distribute_available_resources(bus, add_list);
+ pci_root_bus_distribute_available_resources(bus, add_list);
- /* Depth last, allocate resources and update the hardware. */
- __pci_bus_assign_resources(bus, add_list, &fail_head);
- if (add_list)
- BUG_ON(!list_empty(add_list));
- tried_times++;
+ /* Depth last, allocate resources and update the hardware. */
+ __pci_bus_assign_resources(bus, add_list, &fail_head);
+ if (add_list)
+ BUG_ON(!list_empty(add_list));
+ tried_times++;
- /* Any device complain? */
- if (list_empty(&fail_head))
- goto dump;
+ /* Any device complain? */
+ if (list_empty(&fail_head))
+ break;
- if (tried_times >= pci_try_num) {
- if (enable_local == undefined)
- dev_info(&bus->dev, "Some PCI device resources are unassigned, try booting with pci=realloc\n");
- else if (enable_local == auto_enabled)
- dev_info(&bus->dev, "Automatically enabled pci realloc, if you have problem, try booting with pci=realloc=off\n");
+ if (tried_times >= pci_try_num) {
+ if (enable_local == undefined) {
+ dev_info(&bus->dev,
+ "Some PCI device resources are unassigned, try booting with pci=realloc\n");
+ } else if (enable_local == auto_enabled) {
+ dev_info(&bus->dev,
+ "Automatically enabled pci realloc, if you have problem, try booting with pci=realloc=off\n");
+ }
+ free_list(&fail_head);
+ break;
+ }
- free_list(&fail_head);
- goto dump;
- }
+ dev_info(&bus->dev, "No. %d try to assign unassigned res\n",
+ tried_times + 1);
- dev_info(&bus->dev, "No. %d try to assign unassigned res\n",
- tried_times + 1);
+ /* Third times and later will not check if it is leaf */
+ if (tried_times + 1 > 2)
+ rel_type = whole_subtree;
+
+ /*
+ * Try to release leaf bridge's resources that doesn't fit
+ * resource of child device under that bridge.
+ */
+ 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);
+ }
- /* Third times and later will not check if it is leaf */
- if ((tried_times + 1) > 2)
- rel_type = whole_subtree;
+ /* Restore size and flags */
+ list_for_each_entry(fail_res, &fail_head, list) {
+ struct resource *res = fail_res->res;
+ int idx;
- /*
- * Try to release leaf bridge's resources that doesn't fit resource of
- * child device under that bridge.
- */
- 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);
+ res->start = fail_res->start;
+ res->end = fail_res->end;
+ res->flags = fail_res->flags;
- /* Restore size and flags */
- list_for_each_entry(fail_res, &fail_head, list) {
- struct resource *res = fail_res->res;
- int idx;
-
- res->start = fail_res->start;
- res->end = fail_res->end;
- res->flags = fail_res->flags;
-
- if (pci_is_bridge(fail_res->dev)) {
- idx = pci_resource_num(fail_res->dev, res);
- if (idx >= PCI_BRIDGE_RESOURCES &&
- idx <= PCI_BRIDGE_RESOURCE_END)
- res->flags = 0;
+ if (pci_is_bridge(fail_res->dev)) {
+ idx = pci_resource_num(fail_res->dev, res);
+ if (idx >= PCI_BRIDGE_RESOURCES &&
+ idx <= PCI_BRIDGE_RESOURCE_END)
+ res->flags = 0;
+ }
}
+ free_list(&fail_head);
}
- free_list(&fail_head);
- goto again;
-
-dump:
- /* Dump the resource on buses */
pci_bus_dump_resources(bus);
}
@@ -2261,62 +2262,61 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
struct pci_dev_resource *fail_res;
int retval;
-again:
- __pci_bus_size_bridges(parent, &add_list);
+ while (1) {
+ __pci_bus_size_bridges(parent, &add_list);
- /*
- * Distribute remaining resources (if any) equally between hotplug
- * bridges below. This makes it possible to extend the hierarchy
- * later without running out of resources.
- */
- pci_bridge_distribute_available_resources(bridge, &add_list);
+ /*
+ * Distribute remaining resources (if any) equally between
+ * hotplug bridges below. This makes it possible to extend
+ * the hierarchy later without running out of resources.
+ */
+ pci_bridge_distribute_available_resources(bridge, &add_list);
- __pci_bridge_assign_resources(bridge, &add_list, &fail_head);
- BUG_ON(!list_empty(&add_list));
- tried_times++;
+ __pci_bridge_assign_resources(bridge, &add_list, &fail_head);
+ BUG_ON(!list_empty(&add_list));
+ tried_times++;
- if (list_empty(&fail_head))
- goto enable_all;
+ if (list_empty(&fail_head))
+ break;
- if (tried_times >= 2) {
- /* Still fail, don't need to try more */
- free_list(&fail_head);
- goto enable_all;
- }
+ if (tried_times >= 2) {
+ /* Still fail, don't need to try more */
+ free_list(&fail_head);
+ break;
+ }
- printk(KERN_DEBUG "PCI: No. %d try to assign unassigned res\n",
- tried_times + 1);
+ printk(KERN_DEBUG "PCI: No. %d try to assign unassigned res\n",
+ tried_times + 1);
- /*
- * Try to release leaf bridge's resources that aren't big 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,
- whole_subtree);
+ /*
+ * Try to release leaf bridge's resources that aren't big
+ * 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,
+ whole_subtree);
+ }
- /* Restore size and flags */
- list_for_each_entry(fail_res, &fail_head, list) {
- struct resource *res = fail_res->res;
- int idx;
-
- res->start = fail_res->start;
- res->end = fail_res->end;
- res->flags = fail_res->flags;
-
- if (pci_is_bridge(fail_res->dev)) {
- idx = pci_resource_num(fail_res->dev, res);
- if (idx >= PCI_BRIDGE_RESOURCES &&
- idx <= PCI_BRIDGE_RESOURCE_END)
- res->flags = 0;
+ /* Restore size and flags */
+ list_for_each_entry(fail_res, &fail_head, list) {
+ struct resource *res = fail_res->res;
+ int idx;
+
+ res->start = fail_res->start;
+ res->end = fail_res->end;
+ res->flags = fail_res->flags;
+
+ if (pci_is_bridge(fail_res->dev)) {
+ idx = pci_resource_num(fail_res->dev, res);
+ if (idx >= PCI_BRIDGE_RESOURCES &&
+ idx <= PCI_BRIDGE_RESOURCE_END)
+ res->flags = 0;
+ }
}
+ free_list(&fail_head);
}
- free_list(&fail_head);
-
- goto again;
-enable_all:
retval = pci_reenable_device(bridge);
if (retval)
pci_err(bridge, "Error reenabling bridge (%d)\n", retval);
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 15/25] PCI: Rename retval to ret
2025-02-14 9:59 ` Xiaochun Lee
` (14 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
Rename 'retval' to 'ret' in pci_assign_unassigned_bridge_resources().
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index bbe3472cfba9..38dbe8b99910 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -2256,11 +2256,10 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
struct pci_bus *parent = bridge->subordinate;
/* List of resources that want additional resources */
LIST_HEAD(add_list);
-
int tried_times = 0;
LIST_HEAD(fail_head);
struct pci_dev_resource *fail_res;
- int retval;
+ int ret;
while (1) {
__pci_bus_size_bridges(parent, &add_list);
@@ -2317,9 +2316,9 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
free_list(&fail_head);
}
- retval = pci_reenable_device(bridge);
- if (retval)
- pci_err(bridge, "Error reenabling bridge (%d)\n", retval);
+ ret = pci_reenable_device(bridge);
+ if (ret)
+ pci_err(bridge, "Error reenabling bridge (%d)\n", ret);
pci_set_master(bridge);
}
EXPORT_SYMBOL_GPL(pci_assign_unassigned_bridge_resources);
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 16/25] PCI: Consolidate assignment loop next round preparation
2025-02-14 9:59 ` Xiaochun Lee
` (15 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
pci_assign_unassigned_root_bus_resources() and
pci_assign_unassigned_bridge_resources() have a loop that performs a
number rounds to assign resources. The code to prepare for the next
round is identical.
Consolidate the code that prepares for the next assignment round
into pci_prepare_next_assign_round().
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 104 ++++++++++++++++------------------------
1 file changed, 42 insertions(+), 62 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 38dbe8b99910..7e5985543734 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -2135,6 +2135,45 @@ pci_root_bus_distribute_available_resources(struct pci_bus *bus,
}
}
+static void pci_prepare_next_assign_round(struct list_head *fail_head,
+ int tried_times,
+ enum release_type rel_type)
+{
+ struct pci_dev_resource *fail_res;
+
+ pr_info("PCI: No. %d try to assign unassigned res\n", tried_times + 1);
+
+ /*
+ * Try to release leaf bridge's resources that aren't big
+ * 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);
+ }
+
+ /* 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);
+
+ res->start = fail_res->start;
+ res->end = fail_res->end;
+ res->flags = fail_res->flags;
+
+ if (!pci_is_bridge(dev))
+ continue;
+
+ if (idx >= PCI_BRIDGE_RESOURCES &&
+ idx <= PCI_BRIDGE_RESOURCE_END)
+ res->flags = 0;
+ }
+
+ free_list(fail_head);
+}
+
/*
* First try will not touch PCI bridge res.
* Second and later try will clear small leaf bridge res.
@@ -2148,7 +2187,6 @@ void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus)
int tried_times = 0;
enum release_type rel_type = leaf_only;
LIST_HEAD(fail_head);
- struct pci_dev_resource *fail_res;
int pci_try_num = 1;
enum enable_type enable_local;
@@ -2199,40 +2237,11 @@ void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus)
break;
}
- dev_info(&bus->dev, "No. %d try to assign unassigned res\n",
- tried_times + 1);
-
/* Third times and later will not check if it is leaf */
if (tried_times + 1 > 2)
rel_type = whole_subtree;
- /*
- * Try to release leaf bridge's resources that doesn't fit
- * resource of child device under that bridge.
- */
- 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);
- }
-
- /* Restore size and flags */
- list_for_each_entry(fail_res, &fail_head, list) {
- struct resource *res = fail_res->res;
- int idx;
-
- res->start = fail_res->start;
- res->end = fail_res->end;
- res->flags = fail_res->flags;
-
- if (pci_is_bridge(fail_res->dev)) {
- idx = pci_resource_num(fail_res->dev, res);
- if (idx >= PCI_BRIDGE_RESOURCES &&
- idx <= PCI_BRIDGE_RESOURCE_END)
- res->flags = 0;
- }
- }
- free_list(&fail_head);
+ pci_prepare_next_assign_round(&fail_head, tried_times, rel_type);
}
pci_bus_dump_resources(bus);
@@ -2258,7 +2267,6 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
LIST_HEAD(add_list);
int tried_times = 0;
LIST_HEAD(fail_head);
- struct pci_dev_resource *fail_res;
int ret;
while (1) {
@@ -2284,36 +2292,8 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
break;
}
- printk(KERN_DEBUG "PCI: No. %d try to assign unassigned res\n",
- tried_times + 1);
-
- /*
- * Try to release leaf bridge's resources that aren't big
- * 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,
- whole_subtree);
- }
-
- /* Restore size and flags */
- list_for_each_entry(fail_res, &fail_head, list) {
- struct resource *res = fail_res->res;
- int idx;
-
- res->start = fail_res->start;
- res->end = fail_res->end;
- res->flags = fail_res->flags;
-
- if (pci_is_bridge(fail_res->dev)) {
- idx = pci_resource_num(fail_res->dev, res);
- if (idx >= PCI_BRIDGE_RESOURCES &&
- idx <= PCI_BRIDGE_RESOURCE_END)
- res->flags = 0;
- }
- }
- free_list(&fail_head);
+ pci_prepare_next_assign_round(&fail_head, tried_times,
+ whole_subtree);
}
ret = pci_reenable_device(bridge);
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 17/25] PCI: Remove wrong comment from pci_reassign_resource()
2025-02-14 9:59 ` Xiaochun Lee
` (16 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen, Yinghai Lu
The commit a4ac9fea016f ("PCI : Calculate right add_size") removed
including min_align into new_size in pci_reassign_resource() which is
correct thing to do. However, it also added a snakeoil comment that the
resource would already be aligned with min_align which is incorrect.
A resource that is assigned earlier is aligned with the old alignment,
NOT with the new requested alignment (min_align) until later deep
within the reassignment callchain. Thus, remove the incorrect comment.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
drivers/pci/setup-res.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 79c7ef349856..a862b5d769dc 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -387,7 +387,6 @@ int pci_reassign_resource(struct pci_dev *dev, int resno,
return -EINVAL;
}
- /* already aligned with min_align */
new_size = resource_size(res) + addsize;
ret = _pci_assign_resource(dev, resno, new_size, min_align);
if (ret) {
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 18/25] PCI: Add restore_dev_resource()
2025-02-14 9:59 ` Xiaochun Lee
` (17 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
Resource fitting needs to restore the saved dev resources in a few
places. Add a restore_dev_resource() helper for that.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 35 +++++++++++++++--------------------
1 file changed, 15 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 7e5985543734..d30e8a5a0311 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -127,6 +127,15 @@ static resource_size_t get_res_add_align(struct list_head *head,
return dev_res ? dev_res->min_align : 0;
}
+static void restore_dev_resource(struct pci_dev_resource *dev_res)
+{
+ struct resource *res = dev_res->res;
+
+ res->start = dev_res->start;
+ res->end = dev_res->end;
+ res->flags = dev_res->flags;
+}
+
static bool pdev_resources_assignable(struct pci_dev *dev)
{
u16 class = dev->class >> 8, command;
@@ -473,13 +482,8 @@ static void __assign_resources_sorted(struct list_head *head,
release_resource(res);
}
/* Restore start/end/flags from saved list */
- list_for_each_entry(save_res, &save_head, list) {
- res = save_res->res;
-
- res->start = save_res->start;
- res->end = save_res->end;
- res->flags = save_res->flags;
- }
+ list_for_each_entry(save_res, &save_head, list)
+ restore_dev_resource(save_res);
free_list(&save_head);
requested_and_reassign:
@@ -2159,9 +2163,7 @@ static void pci_prepare_next_assign_round(struct list_head *fail_head,
struct pci_dev *dev = fail_res->dev;
int idx = pci_resource_num(dev, res);
- res->start = fail_res->start;
- res->end = fail_res->end;
- res->flags = fail_res->flags;
+ restore_dev_resource(fail_res);
if (!pci_is_bridge(dev))
continue;
@@ -2378,13 +2380,8 @@ int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
cleanup:
/* Restore size and flags */
- list_for_each_entry(dev_res, &failed, list) {
- struct resource *res = dev_res->res;
-
- res->start = dev_res->start;
- res->end = dev_res->end;
- res->flags = dev_res->flags;
- }
+ list_for_each_entry(dev_res, &failed, list)
+ restore_dev_resource(dev_res);
free_list(&failed);
/* Revert to the old configuration */
@@ -2394,9 +2391,7 @@ int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
bridge = dev_res->dev;
i = pci_resource_num(bridge, res);
- res->start = dev_res->start;
- res->end = dev_res->end;
- res->flags = dev_res->flags;
+ restore_dev_resource(dev_res);
pci_claim_resource(bridge, i);
pci_setup_bridge(bridge->subordinate);
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 19/25] PCI: Extend enable to check for any optional resource
2025-02-14 9:59 ` Xiaochun Lee
` (18 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
pci_enable_resources() checks if device's io and mem resources are all
assigned and disallows enable if any resource failed to assign (*) but
makes an exception for the case of disabled extension ROM. There are
other optional resources, however.
Add pci_resource_is_optional() and use it instead of
pci_resource_is_disabled_rom() to cover also IOV resources that are
also optional as per pbus_size_mem().
As there will be more users of pci_resource_is_optional() inside
setup-bus.c in changes coming up after this one, the function is
placed there.
(*) In practice, resource fitting code calls reset_resource() for any
resource it fails to assign which clears resource's ->flags causing
pci_enable_resources() to never detect failed resource assignments.
This seems undesirable internal logic inconsistency, effectively
reset_resource() prevents pci_enable_resources() from functioning as
intended. This is one step of many that will be needed towards removing
reset_resource().
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/pci.h | 1 +
drivers/pci/setup-bus.c | 12 ++++++++++++
drivers/pci/setup-res.c | 3 +--
3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 0b722d158b6a..efdb9af8e256 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -329,6 +329,7 @@ void pci_walk_bus_locked(struct pci_bus *top,
void *userdata);
const char *pci_resource_name(struct pci_dev *dev, unsigned int i);
+bool pci_resource_is_optional(const struct pci_dev *dev, int resno);
/**
* pci_resource_num - Reverse lookup resource number from device resources
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index d30e8a5a0311..a4e40916e2fc 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -206,6 +206,18 @@ static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head)
}
}
+bool pci_resource_is_optional(const struct pci_dev *dev, int resno)
+{
+ const struct resource *res = pci_resource_n(dev, resno);
+
+ if (pci_resource_is_iov(resno))
+ return true;
+ if (resno == PCI_ROM_RESOURCE && !(res->flags & IORESOURCE_ROM_ENABLE))
+ return true;
+
+ return false;
+}
+
static inline void reset_resource(struct resource *res)
{
res->start = 0;
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index a862b5d769dc..041c881c3c9c 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -494,8 +494,7 @@ int pci_enable_resources(struct pci_dev *dev, int mask)
if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
continue;
- if ((i == PCI_ROM_RESOURCE) &&
- (!(r->flags & IORESOURCE_ROM_ENABLE)))
+ if (pci_resource_is_optional(dev, i))
continue;
if (r->flags & IORESOURCE_UNSET) {
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 20/25] PCI: Always have realloc_head in __assign_resources_sorted()
2025-02-14 9:59 ` Xiaochun Lee
` (19 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
Add a dummy list to always have a non-NULL realloc head in
__assign_resources_sorted() as it allows only checking list_empty().
In future, it would be good to ensure all callers provide a valid
realloc_head but that is relatively complex to do in practice and not
necessary for the subsequent optional resource handling fix.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index a4e40916e2fc..a10acf4671ef 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -400,14 +400,18 @@ static void __assign_resources_sorted(struct list_head *head,
*/
LIST_HEAD(save_head);
LIST_HEAD(local_fail_head);
+ LIST_HEAD(dummy_head);
struct pci_dev_resource *save_res;
struct pci_dev_resource *dev_res, *tmp_res, *dev_res2;
struct resource *res;
unsigned long fail_type;
resource_size_t add_align, align;
+ if (!realloc_head)
+ realloc_head = &dummy_head;
+
/* Check if optional add_size is there */
- if (!realloc_head || list_empty(realloc_head))
+ if (list_empty(realloc_head))
goto requested_and_reassign;
/* Save original start, end, flags etc at first */
@@ -503,7 +507,7 @@ static void __assign_resources_sorted(struct list_head *head,
assign_requested_resources_sorted(head, fail_head);
/* Try to satisfy any additional optional resource requests */
- if (realloc_head)
+ if (!list_empty(realloc_head))
reassign_resources_sorted(realloc_head, head);
out:
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 21/25] PCI: Indicate optional resource assignment failures
2025-02-14 9:59 ` Xiaochun Lee
` (20 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
Add pci_dbg() to tell the an assignment failure was for an optional
resource and reword existing one about resource resize to tell the
change was optional.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index a10acf4671ef..500652eef17b 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -273,13 +273,17 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
align = add_res->min_align;
if (!resource_size(res)) {
resource_set_range(res, align, add_size);
- if (pci_assign_resource(dev, idx))
+ if (pci_assign_resource(dev, idx)) {
+ pci_dbg(dev,
+ "%s %pR: ignoring failure in optional allocation\n",
+ res_name, res);
reset_resource(res);
+ }
} else {
res->flags |= add_res->flags &
(IORESOURCE_STARTALIGN|IORESOURCE_SIZEALIGN);
if (pci_reassign_resource(dev, idx, add_size, align))
- pci_info(dev, "%s %pR: failed to add %llx\n",
+ pci_info(dev, "%s %pR: failed to add optional %llx\n",
res_name, res,
(unsigned long long) add_size);
}
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 22/25] PCI: Add debug print when releasing resources before retry
2025-02-14 9:59 ` Xiaochun Lee
` (21 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
PCI resource fitting is somewhat hard to track because it performs many
actions without logging them. In the case inside
__assign_resources_sorted(), the resources are released before resource
assignment is going to be retried in a different order. That is just
one level of retries the resource fitting performs overall so tracking
it through repeated assignments or failures of a resource gets messy
rather quickly.
Simply make the release announced explicitly using pci_dbg() so it is
clear what is going on with each resource.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 500652eef17b..5a3b320f1511 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -408,6 +408,9 @@ static void __assign_resources_sorted(struct list_head *head,
struct pci_dev_resource *save_res;
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;
@@ -497,9 +500,16 @@ static void __assign_resources_sorted(struct list_head *head,
/* Release assigned resource */
list_for_each_entry(dev_res, head, list) {
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);
- if (res->parent)
- release_resource(res);
+ release_resource(res);
}
/* Restore start/end/flags from saved list */
list_for_each_entry(save_res, &save_head, list)
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 23/25] PCI: Use res->parent to check is resource is assigned
2025-02-14 9:59 ` Xiaochun Lee
` (22 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
reassign_resources_sorted() uses resource_size() to select between
pci_assign_resource() and pci_reassign_resource(). Due to twisted way
bridge window sizing in pbus_size_mem() sets resource sizes to 0, it
works to match into IOV resources but that is going to be changed by
an upcoming change.
Replace resource_size() check with res->parent check that is the true
dividing line in between whether assign or reassign function should be
used for the resource.
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 5a3b320f1511..fec7d68fb971 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -271,7 +271,7 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
res_name = pci_resource_name(dev, idx);
add_size = add_res->add_size;
align = add_res->min_align;
- if (!resource_size(res)) {
+ if (!res->parent) {
resource_set_range(res, align, add_size);
if (pci_assign_resource(dev, idx)) {
pci_dbg(dev,
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH 24/25] PCI: Perform reset_resource() and build fail list in sync
2025-02-14 9:59 ` Xiaochun Lee
` (23 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
2025-04-01 2:35 ` Guenter Roeck
-1 siblings, 1 reply; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen
Resetting resource is problematic as it prevent attempting to allocate
the resource later, unless something in between restores the resource.
Similarly, if fail_head does not contain all resources that were reset,
those resource cannot be restored later.
The entire reset/restore cycle adds complexity and leaving resources
into reseted state causes issues to other code such as for checks done
in pci_enable_resources(). Take a small step towards not resetting
resources by delaying reset until the end of resource assignment and
build failure list (fail_head) in sync with the reset to avoid leaving
behind resources that cannot be restored (for the case where the caller
provides fail_head in the first place to allow restore somewhere in the
callchain, as is not all callers pass non-NULL fail_head).
The Expansion ROM check is temporarily left in place while building the
failure list until the upcoming change which reworks optional resource
handling.
Ideally, whole resource reset could be removed but doing that in a big
step would make the impact non-tractable due to complexity of all
related code.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 35 +++++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index fec7d68fb971..b61f24a5cfa5 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -252,9 +252,14 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
res = add_res->res;
dev = add_res->dev;
+ idx = pci_resource_num(dev, res);
- /* Skip resource that has been reset */
- if (!res->flags)
+ /*
+ * Skip resource that failed the earlier assignment and is
+ * not optional as it would just fail again.
+ */
+ if (!res->parent && resource_size(res) &&
+ !pci_resource_is_optional(dev, idx))
goto out;
/* Skip this resource if not found in head list */
@@ -267,7 +272,6 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
if (!found_match) /* Just skip */
continue;
- idx = pci_resource_num(dev, res);
res_name = pci_resource_name(dev, idx);
add_size = add_res->add_size;
align = add_res->min_align;
@@ -277,7 +281,6 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
pci_dbg(dev,
"%s %pR: ignoring failure in optional allocation\n",
res_name, res);
- reset_resource(res);
}
} else {
res->flags |= add_res->flags &
@@ -332,7 +335,6 @@ static void assign_requested_resources_sorted(struct list_head *head,
0 /* don't care */,
0 /* don't care */);
}
- reset_resource(res);
}
}
}
@@ -518,13 +520,34 @@ static void __assign_resources_sorted(struct list_head *head,
requested_and_reassign:
/* Satisfy the must-have resource requests */
- assign_requested_resources_sorted(head, fail_head);
+ assign_requested_resources_sorted(head, NULL);
/* Try to satisfy any additional optional resource requests */
if (!list_empty(realloc_head))
reassign_resources_sorted(realloc_head, head);
out:
+ /* Reset any failed resource, cannot use fail_head as it can be NULL. */
+ list_for_each_entry(dev_res, head, list) {
+ res = dev_res->res;
+ dev = dev_res->dev;
+
+ if (res->parent)
+ continue;
+
+ /*
+ * If the failed resource is a ROM BAR and it will
+ * be enabled later, don't add it to the list.
+ */
+ if (fail_head && !pci_resource_is_disabled_rom(res, idx)) {
+ add_to_list(fail_head, dev, res,
+ 0 /* don't care */,
+ 0 /* don't care */);
+ }
+
+ reset_resource(res);
+ }
+
free_list(head);
}
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 24/25] PCI: Perform reset_resource() and build fail list in sync
2024-12-16 17:56 ` [PATCH 24/25] PCI: Perform reset_resource() and build fail list in sync Ilpo Järvinen
@ 2025-04-01 2:35 ` Guenter Roeck
2025-04-01 10:18 ` Ilpo Järvinen
0 siblings, 1 reply; 43+ messages in thread
From: Guenter Roeck @ 2025-04-01 2:35 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel, Mika Westerberg
Hi,
On Mon, Dec 16, 2024 at 07:56:31PM +0200, Ilpo Järvinen wrote:
> Resetting resource is problematic as it prevent attempting to allocate
> the resource later, unless something in between restores the resource.
> Similarly, if fail_head does not contain all resources that were reset,
> those resource cannot be restored later.
>
> The entire reset/restore cycle adds complexity and leaving resources
> into reseted state causes issues to other code such as for checks done
> in pci_enable_resources(). Take a small step towards not resetting
> resources by delaying reset until the end of resource assignment and
> build failure list (fail_head) in sync with the reset to avoid leaving
> behind resources that cannot be restored (for the case where the caller
> provides fail_head in the first place to allow restore somewhere in the
> callchain, as is not all callers pass non-NULL fail_head).
>
> The Expansion ROM check is temporarily left in place while building the
> failure list until the upcoming change which reworks optional resource
> handling.
>
> Ideally, whole resource reset could be removed but doing that in a big
> step would make the impact non-tractable due to complexity of all
> related code.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
With this patch in the mainline kernel, all mips:boston qemu emulations
fail when running a 64-bit little endian configuration (64r6el_defconfig).
The problem is that the PCI based IDE/ATA controller is not initialized.
There are a number of pci error messages.
pci_bus 0002:01: extended config space not accessible
pci 0002:01:00.0: [8086:2922] type 00 class 0x010601 conventional PCI endpoint
pci 0002:01:00.0: BAR 4 [io 0x0000-0x001f]
pci 0002:01:00.0: BAR 5 [mem 0x00000000-0x00000fff]
pci 0002:00:00.0: PCI bridge to [bus 01-ff]
pci_bus 0002:01: busn_res: [bus 01-ff] end is updated to 01
pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff]: assigned
pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: can't assign; no space
pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: failed to assign
pci 0002:00:00.0: bridge window [io size 0x1000]: can't assign; no space
pci 0002:00:00.0: bridge window [io size 0x1000]: failed to assign
pci 0002:00:00.0: bridge window [mem size 0x00100000]: can't assign; bogus alignment
pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff 64bit pref]: assigned
pci 0002:00:00.0: bridge window [io size 0x1000]: can't assign; no space
pci 0002:00:00.0: bridge window [io size 0x1000]: failed to assign
pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: can't assign; no space
pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: failed to assign
pci 0002:01:00.0: BAR 4 [io size 0x0020]: can't assign; no space
pci 0002:01:00.0: BAR 4 [io size 0x0020]: failed to assign
pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: can't assign; no space
pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: failed to assign
pci 0002:01:00.0: BAR 4 [io size 0x0020]: can't assign; no space
pci 0002:01:00.0: BAR 4 [io size 0x0020]: failed to assign
pci 0002:00:00.0: PCI bridge to [bus 01]
pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff 64bit pref]
pci_bus 0002:00: Some PCI device resources are unassigned, try booting with pci=realloc
pci_bus 0002:00: resource 4 [mem 0x16000000-0x160fffff]
pci_bus 0002:01: resource 2 [mem 0x16000000-0x160fffff 64bit pref]
...
pci 0002:00:00.0: enabling device (0000 -> 0002)
ahci 0002:01:00.0: probe with driver ahci failed with error -12
Bisect points to this patch. Reverting it together with "PCI: Rework
optional resource handling" fixes the problem. For comparison, after
reverting the offending patches, the log messages are as follows.
pci_bus 0002:00: root bus resource [bus 00-ff]
pci_bus 0002:00: root bus resource [mem 0x16000000-0x160fffff]
pci 0002:00:00.0: [10ee:7021] type 01 class 0x060400 PCIe Root Complex Integrated Endpoint
pci 0002:00:00.0: PCI bridge to [bus 00]
pci 0002:00:00.0: bridge window [io 0x0000-0x0fff]
pci 0002:00:00.0: bridge window [mem 0x00000000-0x000fffff]
pci 0002:00:00.0: bridge window [mem 0x00000000-0x000fffff 64bit pref]
pci 0002:00:00.0: enabling Extended Tags
pci 0002:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring
pci_bus 0002:01: extended config space not accessible
pci 0002:01:00.0: [8086:2922] type 00 class 0x010601 conventional PCI endpoint
pci 0002:01:00.0: BAR 4 [io 0x0000-0x001f]
pci 0002:01:00.0: BAR 5 [mem 0x00000000-0x00000fff]
pci 0002:00:00.0: PCI bridge to [bus 01-ff]
pci_bus 0002:01: busn_res: [bus 01-ff] end is updated to 01
pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff]: assigned
pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: can't assign; no space
pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: failed to assign
pci 0002:00:00.0: bridge window [io size 0x1000]: can't assign; no space
pci 0002:00:00.0: bridge window [io size 0x1000]: failed to assign
pci 0002:01:00.0: BAR 5 [mem 0x16000000-0x16000fff]: assigned
pci 0002:01:00.0: BAR 4 [io size 0x0020]: can't assign; no space
pci 0002:01:00.0: BAR 4 [io size 0x0020]: failed to assign
pci 0002:00:00.0: PCI bridge to [bus 01]
pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff]
pci_bus 0002:00: Some PCI device resources are unassigned, try booting with pci=realloc
pci_bus 0002:00: resource 4 [mem 0x16000000-0x160fffff]
pci_bus 0002:01: resource 1 [mem 0x16000000-0x160fffff]
...
pci 0002:00:00.0: enabling device (0000 -> 0002)
ahci 0002:01:00.0: enabling device (0000 -> 0002)
ahci 0002:01:00.0: AHCI vers 0001.0000, 32 command slots, 1.5 Gbps, SATA mode
ahci 0002:01:00.0: 6/6 ports implemented (port mask 0x3f)
ahci 0002:01:00.0: flags: 64bit ncq only
Bisect log is attached for reference.
Guenter
---
# bad: [609706855d90bcab6080ba2cd030b9af322a1f0c] Merge tag 'trace-latency-v6.15-3' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace
# good: [acb4f33713b9f6cadb6143f211714c343465411c] Merge tag 'm68knommu-for-v6.15' of git://git.kernel.org/pub/scm/linux/kernel/git/gerg/m68knommu
git bisect start 'HEAD' 'acb4f33713b9'
# good: [cf05922d63e2ae6a9b1b52ff5236a44c3b29f78c] Merge tag 'drm-intel-gt-next-2025-03-12' of https://gitlab.freedesktop.org/drm/i915/kernel into drm-next
git bisect good cf05922d63e2ae6a9b1b52ff5236a44c3b29f78c
# bad: [93d52288679e29aaa44a6f12d5a02e8a90e742c5] Merge tag 'backlight-next-6.15' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight
git bisect bad 93d52288679e29aaa44a6f12d5a02e8a90e742c5
# bad: [e5e0e6bebef3a21081fd1057c40468d4cff1a60d] Merge tag 'v6.15-p1' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6
git bisect bad e5e0e6bebef3a21081fd1057c40468d4cff1a60d
# bad: [dea140198b846f7432d78566b7b0b83979c72c2b] Merge branch 'pci/misc'
git bisect bad dea140198b846f7432d78566b7b0b83979c72c2b
# bad: [a113afb84ae63ec4c893bc3204945ef6f3bb89f7] Merge branch 'pci/endpoint'
git bisect bad a113afb84ae63ec4c893bc3204945ef6f3bb89f7
# good: [a7a8e7996c1c114b50df5599229b1e7be38be3db] Merge branch 'pci/reset'
git bisect good a7a8e7996c1c114b50df5599229b1e7be38be3db
# bad: [7d4bcc0f2631e4ee10b5bcfff24a423d1c3c02a3] PCI: Move resource reassignment func declarations into pci/pci.h
git bisect bad 7d4bcc0f2631e4ee10b5bcfff24a423d1c3c02a3
# good: [acba174d2e754346c07578ad2220258706a203e2] PCI: Use while loop and break instead of gotos
git bisect good acba174d2e754346c07578ad2220258706a203e2
# good: [8884b5637b794ae541e8d6fb72102b1d8dba2b8d] PCI: Add debug print when releasing resources before retry
git bisect good 8884b5637b794ae541e8d6fb72102b1d8dba2b8d
# bad: [5af473941b56189423a7d16c05efabaf77299847] PCI: Increase Resizable BAR support from 512 GB to 128 TB
git bisect bad 5af473941b56189423a7d16c05efabaf77299847
# bad: [96336ec702643aec2addb3b1cdb81d687fe362f0] PCI: Perform reset_resource() and build fail list in sync
git bisect bad 96336ec702643aec2addb3b1cdb81d687fe362f0
# good: [e89df6d2beae847e931d84a190b192dfac41eba7] PCI: Use res->parent to check if resource is assigned
git bisect good e89df6d2beae847e931d84a190b192dfac41eba7
# first bad commit: [96336ec702643aec2addb3b1cdb81d687fe362f0] PCI: Perform reset_resource() and build fail list in sync
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 24/25] PCI: Perform reset_resource() and build fail list in sync
2025-04-01 2:35 ` Guenter Roeck
@ 2025-04-01 10:18 ` Ilpo Järvinen
2025-04-01 12:07 ` Ilpo Järvinen
2025-04-01 13:28 ` Guenter Roeck
0 siblings, 2 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2025-04-01 10:18 UTC (permalink / raw)
To: Guenter Roeck
Cc: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
LKML, Mika Westerberg
[-- Attachment #1: Type: text/plain, Size: 9223 bytes --]
On Mon, 31 Mar 2025, Guenter Roeck wrote:
> On Mon, Dec 16, 2024 at 07:56:31PM +0200, Ilpo Järvinen wrote:
> > Resetting resource is problematic as it prevent attempting to allocate
> > the resource later, unless something in between restores the resource.
> > Similarly, if fail_head does not contain all resources that were reset,
> > those resource cannot be restored later.
> >
> > The entire reset/restore cycle adds complexity and leaving resources
> > into reseted state causes issues to other code such as for checks done
> > in pci_enable_resources(). Take a small step towards not resetting
> > resources by delaying reset until the end of resource assignment and
> > build failure list (fail_head) in sync with the reset to avoid leaving
> > behind resources that cannot be restored (for the case where the caller
> > provides fail_head in the first place to allow restore somewhere in the
> > callchain, as is not all callers pass non-NULL fail_head).
> >
> > The Expansion ROM check is temporarily left in place while building the
> > failure list until the upcoming change which reworks optional resource
> > handling.
> >
> > Ideally, whole resource reset could be removed but doing that in a big
> > step would make the impact non-tractable due to complexity of all
> > related code.
> >
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>
> With this patch in the mainline kernel, all mips:boston qemu emulations
> fail when running a 64-bit little endian configuration (64r6el_defconfig).
>
> The problem is that the PCI based IDE/ATA controller is not initialized.
> There are a number of pci error messages.
>
> pci_bus 0002:01: extended config space not accessible
> pci 0002:01:00.0: [8086:2922] type 00 class 0x010601 conventional PCI endpoint
> pci 0002:01:00.0: BAR 4 [io 0x0000-0x001f]
> pci 0002:01:00.0: BAR 5 [mem 0x00000000-0x00000fff]
> pci 0002:00:00.0: PCI bridge to [bus 01-ff]
> pci_bus 0002:01: busn_res: [bus 01-ff] end is updated to 01
> pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff]: assigned
> pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: can't assign; no space
> pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: failed to assign
> pci 0002:00:00.0: bridge window [io size 0x1000]: can't assign; no space
> pci 0002:00:00.0: bridge window [io size 0x1000]: failed to assign
> pci 0002:00:00.0: bridge window [mem size 0x00100000]: can't assign; bogus alignment
> pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff 64bit pref]: assigned
> pci 0002:00:00.0: bridge window [io size 0x1000]: can't assign; no space
> pci 0002:00:00.0: bridge window [io size 0x1000]: failed to assign
> pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: can't assign; no space
> pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: failed to assign
> pci 0002:01:00.0: BAR 4 [io size 0x0020]: can't assign; no space
> pci 0002:01:00.0: BAR 4 [io size 0x0020]: failed to assign
> pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: can't assign; no space
> pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: failed to assign
> pci 0002:01:00.0: BAR 4 [io size 0x0020]: can't assign; no space
> pci 0002:01:00.0: BAR 4 [io size 0x0020]: failed to assign
> pci 0002:00:00.0: PCI bridge to [bus 01]
> pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff 64bit pref]
> pci_bus 0002:00: Some PCI device resources are unassigned, try booting with pci=realloc
> pci_bus 0002:00: resource 4 [mem 0x16000000-0x160fffff]
> pci_bus 0002:01: resource 2 [mem 0x16000000-0x160fffff 64bit pref]
> ...
> pci 0002:00:00.0: enabling device (0000 -> 0002)
> ahci 0002:01:00.0: probe with driver ahci failed with error -12
>
> Bisect points to this patch. Reverting it together with "PCI: Rework
> optional resource handling" fixes the problem. For comparison, after
> reverting the offending patches, the log messages are as follows.
>
> pci_bus 0002:00: root bus resource [bus 00-ff]
> pci_bus 0002:00: root bus resource [mem 0x16000000-0x160fffff]
> pci 0002:00:00.0: [10ee:7021] type 01 class 0x060400 PCIe Root Complex Integrated Endpoint
> pci 0002:00:00.0: PCI bridge to [bus 00]
> pci 0002:00:00.0: bridge window [io 0x0000-0x0fff]
> pci 0002:00:00.0: bridge window [mem 0x00000000-0x000fffff]
> pci 0002:00:00.0: bridge window [mem 0x00000000-0x000fffff 64bit pref]
> pci 0002:00:00.0: enabling Extended Tags
> pci 0002:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring
> pci_bus 0002:01: extended config space not accessible
> pci 0002:01:00.0: [8086:2922] type 00 class 0x010601 conventional PCI endpoint
> pci 0002:01:00.0: BAR 4 [io 0x0000-0x001f]
> pci 0002:01:00.0: BAR 5 [mem 0x00000000-0x00000fff]
> pci 0002:00:00.0: PCI bridge to [bus 01-ff]
> pci_bus 0002:01: busn_res: [bus 01-ff] end is updated to 01
> pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff]: assigned
> pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: can't assign; no space
> pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: failed to assign
> pci 0002:00:00.0: bridge window [io size 0x1000]: can't assign; no space
> pci 0002:00:00.0: bridge window [io size 0x1000]: failed to assign
> pci 0002:01:00.0: BAR 5 [mem 0x16000000-0x16000fff]: assigned
> pci 0002:01:00.0: BAR 4 [io size 0x0020]: can't assign; no space
> pci 0002:01:00.0: BAR 4 [io size 0x0020]: failed to assign
> pci 0002:00:00.0: PCI bridge to [bus 01]
> pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff]
> pci_bus 0002:00: Some PCI device resources are unassigned, try booting with pci=realloc
> pci_bus 0002:00: resource 4 [mem 0x16000000-0x160fffff]
> pci_bus 0002:01: resource 1 [mem 0x16000000-0x160fffff]
> ...
> pci 0002:00:00.0: enabling device (0000 -> 0002)
> ahci 0002:01:00.0: enabling device (0000 -> 0002)
> ahci 0002:01:00.0: AHCI vers 0001.0000, 32 command slots, 1.5 Gbps, SATA mode
> ahci 0002:01:00.0: 6/6 ports implemented (port mask 0x3f)
> ahci 0002:01:00.0: flags: 64bit ncq only
Hi,
Thanks for reporting. Please add this to the command line to get the
resource releasing between the steps to show:
dyndbg="file drivers/pci/setup-bus.c +p"
Also, the log snippet just shows it fails but it is impossible to know
from it why the resource assigments do not fit so could you please provide
a complete dmesg logs. Also providing the contents of /proc/iomem from the
working case would save me quite a bit of decoding the iomem layout from
the dmesgs.
> Bisect log is attached for reference.
>
> Guenter
> ---
> # bad: [609706855d90bcab6080ba2cd030b9af322a1f0c] Merge tag 'trace-latency-v6.15-3' of git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace
> # good: [acb4f33713b9f6cadb6143f211714c343465411c] Merge tag 'm68knommu-for-v6.15' of git://git.kernel.org/pub/scm/linux/kernel/git/gerg/m68knommu
> git bisect start 'HEAD' 'acb4f33713b9'
> # good: [cf05922d63e2ae6a9b1b52ff5236a44c3b29f78c] Merge tag 'drm-intel-gt-next-2025-03-12' of https://gitlab.freedesktop.org/drm/i915/kernel into drm-next
> git bisect good cf05922d63e2ae6a9b1b52ff5236a44c3b29f78c
> # bad: [93d52288679e29aaa44a6f12d5a02e8a90e742c5] Merge tag 'backlight-next-6.15' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight
> git bisect bad 93d52288679e29aaa44a6f12d5a02e8a90e742c5
> # bad: [e5e0e6bebef3a21081fd1057c40468d4cff1a60d] Merge tag 'v6.15-p1' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6
> git bisect bad e5e0e6bebef3a21081fd1057c40468d4cff1a60d
> # bad: [dea140198b846f7432d78566b7b0b83979c72c2b] Merge branch 'pci/misc'
> git bisect bad dea140198b846f7432d78566b7b0b83979c72c2b
> # bad: [a113afb84ae63ec4c893bc3204945ef6f3bb89f7] Merge branch 'pci/endpoint'
> git bisect bad a113afb84ae63ec4c893bc3204945ef6f3bb89f7
> # good: [a7a8e7996c1c114b50df5599229b1e7be38be3db] Merge branch 'pci/reset'
> git bisect good a7a8e7996c1c114b50df5599229b1e7be38be3db
> # bad: [7d4bcc0f2631e4ee10b5bcfff24a423d1c3c02a3] PCI: Move resource reassignment func declarations into pci/pci.h
> git bisect bad 7d4bcc0f2631e4ee10b5bcfff24a423d1c3c02a3
> # good: [acba174d2e754346c07578ad2220258706a203e2] PCI: Use while loop and break instead of gotos
> git bisect good acba174d2e754346c07578ad2220258706a203e2
> # good: [8884b5637b794ae541e8d6fb72102b1d8dba2b8d] PCI: Add debug print when releasing resources before retry
> git bisect good 8884b5637b794ae541e8d6fb72102b1d8dba2b8d
> # bad: [5af473941b56189423a7d16c05efabaf77299847] PCI: Increase Resizable BAR support from 512 GB to 128 TB
> git bisect bad 5af473941b56189423a7d16c05efabaf77299847
> # bad: [96336ec702643aec2addb3b1cdb81d687fe362f0] PCI: Perform reset_resource() and build fail list in sync
> git bisect bad 96336ec702643aec2addb3b1cdb81d687fe362f0
> # good: [e89df6d2beae847e931d84a190b192dfac41eba7] PCI: Use res->parent to check if resource is assigned
> git bisect good e89df6d2beae847e931d84a190b192dfac41eba7
> # first bad commit: [96336ec702643aec2addb3b1cdb81d687fe362f0] PCI: Perform reset_resource() and build fail list in sync
>
--
i.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 24/25] PCI: Perform reset_resource() and build fail list in sync
2025-04-01 10:18 ` Ilpo Järvinen
@ 2025-04-01 12:07 ` Ilpo Järvinen
2025-04-01 14:15 ` Guenter Roeck
2025-04-01 13:28 ` Guenter Roeck
1 sibling, 1 reply; 43+ messages in thread
From: Ilpo Järvinen @ 2025-04-01 12:07 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Guenter Roeck, Bjorn Helgaas, linux-pci, Michał Winiarski,
Igor Mammedov, LKML, Mika Westerberg
[-- Attachment #1: Type: text/plain, Size: 10045 bytes --]
On Tue, 1 Apr 2025, Ilpo Järvinen wrote:
> On Mon, 31 Mar 2025, Guenter Roeck wrote:
> > On Mon, Dec 16, 2024 at 07:56:31PM +0200, Ilpo Järvinen wrote:
> > > Resetting resource is problematic as it prevent attempting to allocate
> > > the resource later, unless something in between restores the resource.
> > > Similarly, if fail_head does not contain all resources that were reset,
> > > those resource cannot be restored later.
> > >
> > > The entire reset/restore cycle adds complexity and leaving resources
> > > into reseted state causes issues to other code such as for checks done
> > > in pci_enable_resources(). Take a small step towards not resetting
> > > resources by delaying reset until the end of resource assignment and
> > > build failure list (fail_head) in sync with the reset to avoid leaving
> > > behind resources that cannot be restored (for the case where the caller
> > > provides fail_head in the first place to allow restore somewhere in the
> > > callchain, as is not all callers pass non-NULL fail_head).
> > >
> > > The Expansion ROM check is temporarily left in place while building the
> > > failure list until the upcoming change which reworks optional resource
> > > handling.
> > >
> > > Ideally, whole resource reset could be removed but doing that in a big
> > > step would make the impact non-tractable due to complexity of all
> > > related code.
> > >
> > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> >
> > With this patch in the mainline kernel, all mips:boston qemu emulations
> > fail when running a 64-bit little endian configuration (64r6el_defconfig).
> >
> > The problem is that the PCI based IDE/ATA controller is not initialized.
> > There are a number of pci error messages.
> >
> > pci_bus 0002:01: extended config space not accessible
> > pci 0002:01:00.0: [8086:2922] type 00 class 0x010601 conventional PCI endpoint
> > pci 0002:01:00.0: BAR 4 [io 0x0000-0x001f]
> > pci 0002:01:00.0: BAR 5 [mem 0x00000000-0x00000fff]
> > pci 0002:00:00.0: PCI bridge to [bus 01-ff]
> > pci_bus 0002:01: busn_res: [bus 01-ff] end is updated to 01
> > pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff]: assigned
> > pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: can't assign; no space
> > pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: failed to assign
> > pci 0002:00:00.0: bridge window [io size 0x1000]: can't assign; no space
> > pci 0002:00:00.0: bridge window [io size 0x1000]: failed to assign
> > pci 0002:00:00.0: bridge window [mem size 0x00100000]: can't assign; bogus alignment
> > pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff 64bit pref]: assigned
> > pci 0002:00:00.0: bridge window [io size 0x1000]: can't assign; no space
> > pci 0002:00:00.0: bridge window [io size 0x1000]: failed to assign
> > pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: can't assign; no space
> > pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: failed to assign
> > pci 0002:01:00.0: BAR 4 [io size 0x0020]: can't assign; no space
> > pci 0002:01:00.0: BAR 4 [io size 0x0020]: failed to assign
> > pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: can't assign; no space
> > pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: failed to assign
> > pci 0002:01:00.0: BAR 4 [io size 0x0020]: can't assign; no space
> > pci 0002:01:00.0: BAR 4 [io size 0x0020]: failed to assign
> > pci 0002:00:00.0: PCI bridge to [bus 01]
> > pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff 64bit pref]
> > pci_bus 0002:00: Some PCI device resources are unassigned, try booting with pci=realloc
> > pci_bus 0002:00: resource 4 [mem 0x16000000-0x160fffff]
> > pci_bus 0002:01: resource 2 [mem 0x16000000-0x160fffff 64bit pref]
> > ...
> > pci 0002:00:00.0: enabling device (0000 -> 0002)
> > ahci 0002:01:00.0: probe with driver ahci failed with error -12
> >
> > Bisect points to this patch. Reverting it together with "PCI: Rework
> > optional resource handling" fixes the problem. For comparison, after
> > reverting the offending patches, the log messages are as follows.
> >
> > pci_bus 0002:00: root bus resource [bus 00-ff]
> > pci_bus 0002:00: root bus resource [mem 0x16000000-0x160fffff]
> > pci 0002:00:00.0: [10ee:7021] type 01 class 0x060400 PCIe Root Complex Integrated Endpoint
> > pci 0002:00:00.0: PCI bridge to [bus 00]
> > pci 0002:00:00.0: bridge window [io 0x0000-0x0fff]
> > pci 0002:00:00.0: bridge window [mem 0x00000000-0x000fffff]
> > pci 0002:00:00.0: bridge window [mem 0x00000000-0x000fffff 64bit pref]
> > pci 0002:00:00.0: enabling Extended Tags
> > pci 0002:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring
> > pci_bus 0002:01: extended config space not accessible
> > pci 0002:01:00.0: [8086:2922] type 00 class 0x010601 conventional PCI endpoint
> > pci 0002:01:00.0: BAR 4 [io 0x0000-0x001f]
> > pci 0002:01:00.0: BAR 5 [mem 0x00000000-0x00000fff]
> > pci 0002:00:00.0: PCI bridge to [bus 01-ff]
> > pci_bus 0002:01: busn_res: [bus 01-ff] end is updated to 01
> > pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff]: assigned
> > pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: can't assign; no space
> > pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: failed to assign
> > pci 0002:00:00.0: bridge window [io size 0x1000]: can't assign; no space
> > pci 0002:00:00.0: bridge window [io size 0x1000]: failed to assign
> > pci 0002:01:00.0: BAR 5 [mem 0x16000000-0x16000fff]: assigned
> > pci 0002:01:00.0: BAR 4 [io size 0x0020]: can't assign; no space
> > pci 0002:01:00.0: BAR 4 [io size 0x0020]: failed to assign
> > pci 0002:00:00.0: PCI bridge to [bus 01]
> > pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff]
> > pci_bus 0002:00: Some PCI device resources are unassigned, try booting with pci=realloc
> > pci_bus 0002:00: resource 4 [mem 0x16000000-0x160fffff]
> > pci_bus 0002:01: resource 1 [mem 0x16000000-0x160fffff]
> > ...
> > pci 0002:00:00.0: enabling device (0000 -> 0002)
> > ahci 0002:01:00.0: enabling device (0000 -> 0002)
> > ahci 0002:01:00.0: AHCI vers 0001.0000, 32 command slots, 1.5 Gbps, SATA mode
> > ahci 0002:01:00.0: 6/6 ports implemented (port mask 0x3f)
> > ahci 0002:01:00.0: flags: 64bit ncq only
>
> Hi,
>
> Thanks for reporting. Please add this to the command line to get the
> resource releasing between the steps to show:
>
> dyndbg="file drivers/pci/setup-bus.c +p"
>
> Also, the log snippet just shows it fails but it is impossible to know
> from it why the resource assigments do not fit so could you please provide
> a complete dmesg logs. Also providing the contents of /proc/iomem from the
> working case would save me quite a bit of decoding the iomem layout from
> the dmesgs.
Hi again,
If you could kindly include this patch into the test with pci_dbg()
enabled so the resource reset/restore is better tracked.
From e7cb98904ab2ee235bbc13b3e981332e944dd476 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Tue, 1 Apr 2025 15:05:13 +0300
Subject: [PATCH 1/1] PCI: Log reset and restore of resources
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
PCI resource fitting and assignment is complicated to track because it
performs many actions without any logging. One of these is resource
reset (zeroing the resource) and the restore during the multi-passed
resource fitting algorithm.
Resource reset does not play well with the other PCI code if the code
later wants to reattempt assignment of that resource, knowing that a
resource was left into the reset state without a pairing restore is
useful for understanding issues that show up as resource assignment
failures.
Add pci_dbg() to both reset and restore to be better able to track
what's going on within the resource fitting algorithm.
The resource fitting and assignment tracking should eventually be
convert to use trace events to cover all changes made into the
resources fully but that requires significant effort. In the meantime,
logging resource reset and restore with pci_dbg() seems low-hanging
fruit.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 54d6f4fa3ce1..e67af19cb876 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -130,10 +130,15 @@ static resource_size_t get_res_add_align(struct list_head *head,
static void restore_dev_resource(struct pci_dev_resource *dev_res)
{
struct resource *res = dev_res->res;
+ struct pci_dev *dev = dev_res->dev;
+ int idx = pci_resource_num(dev, res);
+ const char *res_name = pci_resource_name(dev, idx);
res->start = dev_res->start;
res->end = dev_res->end;
res->flags = dev_res->flags;
+
+ pci_dbg(dev_res->dev, "%s %pR: resource restored\n", res_name, res);
}
static bool pdev_resources_assignable(struct pci_dev *dev)
@@ -218,8 +223,15 @@ bool pci_resource_is_optional(const struct pci_dev *dev, int resno)
return false;
}
-static inline void reset_resource(struct resource *res)
+static void reset_resource(struct pci_dev_resource *dev_res)
{
+ struct resource *res = dev_res->res;
+ struct pci_dev *dev = dev_res->dev;
+ int idx = pci_resource_num(dev, res);
+ const char *res_name = pci_resource_name(dev, idx);
+
+ pci_dbg(dev_res->dev, "%s %pR: resetting resource\n", res_name, res);
+
res->start = 0;
res->end = 0;
res->flags = 0;
@@ -573,7 +585,7 @@ static void __assign_resources_sorted(struct list_head *head,
0 /* don't care */);
}
- reset_resource(res);
+ reset_resource(dev_res);
}
free_list(head);
base-commit: 7d06015d936c861160803e020f68f413b5c3cd9d
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 24/25] PCI: Perform reset_resource() and build fail list in sync
2025-04-01 12:07 ` Ilpo Järvinen
@ 2025-04-01 14:15 ` Guenter Roeck
2025-04-01 17:38 ` Ilpo Järvinen
0 siblings, 1 reply; 43+ messages in thread
From: Guenter Roeck @ 2025-04-01 14:15 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
LKML, Mika Westerberg
On 4/1/25 05:07, Ilpo Järvinen wrote:
> On Tue, 1 Apr 2025, Ilpo Järvinen wrote:
>
>> On Mon, 31 Mar 2025, Guenter Roeck wrote:
>>> On Mon, Dec 16, 2024 at 07:56:31PM +0200, Ilpo Järvinen wrote:
>>>> Resetting resource is problematic as it prevent attempting to allocate
>>>> the resource later, unless something in between restores the resource.
>>>> Similarly, if fail_head does not contain all resources that were reset,
>>>> those resource cannot be restored later.
>>>>
>>>> The entire reset/restore cycle adds complexity and leaving resources
>>>> into reseted state causes issues to other code such as for checks done
>>>> in pci_enable_resources(). Take a small step towards not resetting
>>>> resources by delaying reset until the end of resource assignment and
>>>> build failure list (fail_head) in sync with the reset to avoid leaving
>>>> behind resources that cannot be restored (for the case where the caller
>>>> provides fail_head in the first place to allow restore somewhere in the
>>>> callchain, as is not all callers pass non-NULL fail_head).
>>>>
>>>> The Expansion ROM check is temporarily left in place while building the
>>>> failure list until the upcoming change which reworks optional resource
>>>> handling.
>>>>
>>>> Ideally, whole resource reset could be removed but doing that in a big
>>>> step would make the impact non-tractable due to complexity of all
>>>> related code.
>>>>
>>>> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>>>
>>> With this patch in the mainline kernel, all mips:boston qemu emulations
>>> fail when running a 64-bit little endian configuration (64r6el_defconfig).
>>>
>>> The problem is that the PCI based IDE/ATA controller is not initialized.
>>> There are a number of pci error messages.
>>>
>>> pci_bus 0002:01: extended config space not accessible
>>> pci 0002:01:00.0: [8086:2922] type 00 class 0x010601 conventional PCI endpoint
>>> pci 0002:01:00.0: BAR 4 [io 0x0000-0x001f]
>>> pci 0002:01:00.0: BAR 5 [mem 0x00000000-0x00000fff]
>>> pci 0002:00:00.0: PCI bridge to [bus 01-ff]
>>> pci_bus 0002:01: busn_res: [bus 01-ff] end is updated to 01
>>> pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff]: assigned
>>> pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: can't assign; no space
>>> pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: failed to assign
>>> pci 0002:00:00.0: bridge window [io size 0x1000]: can't assign; no space
>>> pci 0002:00:00.0: bridge window [io size 0x1000]: failed to assign
>>> pci 0002:00:00.0: bridge window [mem size 0x00100000]: can't assign; bogus alignment
>>> pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff 64bit pref]: assigned
>>> pci 0002:00:00.0: bridge window [io size 0x1000]: can't assign; no space
>>> pci 0002:00:00.0: bridge window [io size 0x1000]: failed to assign
>>> pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: can't assign; no space
>>> pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: failed to assign
>>> pci 0002:01:00.0: BAR 4 [io size 0x0020]: can't assign; no space
>>> pci 0002:01:00.0: BAR 4 [io size 0x0020]: failed to assign
>>> pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: can't assign; no space
>>> pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: failed to assign
>>> pci 0002:01:00.0: BAR 4 [io size 0x0020]: can't assign; no space
>>> pci 0002:01:00.0: BAR 4 [io size 0x0020]: failed to assign
>>> pci 0002:00:00.0: PCI bridge to [bus 01]
>>> pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff 64bit pref]
>>> pci_bus 0002:00: Some PCI device resources are unassigned, try booting with pci=realloc
>>> pci_bus 0002:00: resource 4 [mem 0x16000000-0x160fffff]
>>> pci_bus 0002:01: resource 2 [mem 0x16000000-0x160fffff 64bit pref]
>>> ...
>>> pci 0002:00:00.0: enabling device (0000 -> 0002)
>>> ahci 0002:01:00.0: probe with driver ahci failed with error -12
>>>
>>> Bisect points to this patch. Reverting it together with "PCI: Rework
>>> optional resource handling" fixes the problem. For comparison, after
>>> reverting the offending patches, the log messages are as follows.
>>>
>>> pci_bus 0002:00: root bus resource [bus 00-ff]
>>> pci_bus 0002:00: root bus resource [mem 0x16000000-0x160fffff]
>>> pci 0002:00:00.0: [10ee:7021] type 01 class 0x060400 PCIe Root Complex Integrated Endpoint
>>> pci 0002:00:00.0: PCI bridge to [bus 00]
>>> pci 0002:00:00.0: bridge window [io 0x0000-0x0fff]
>>> pci 0002:00:00.0: bridge window [mem 0x00000000-0x000fffff]
>>> pci 0002:00:00.0: bridge window [mem 0x00000000-0x000fffff 64bit pref]
>>> pci 0002:00:00.0: enabling Extended Tags
>>> pci 0002:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring
>>> pci_bus 0002:01: extended config space not accessible
>>> pci 0002:01:00.0: [8086:2922] type 00 class 0x010601 conventional PCI endpoint
>>> pci 0002:01:00.0: BAR 4 [io 0x0000-0x001f]
>>> pci 0002:01:00.0: BAR 5 [mem 0x00000000-0x00000fff]
>>> pci 0002:00:00.0: PCI bridge to [bus 01-ff]
>>> pci_bus 0002:01: busn_res: [bus 01-ff] end is updated to 01
>>> pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff]: assigned
>>> pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: can't assign; no space
>>> pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: failed to assign
>>> pci 0002:00:00.0: bridge window [io size 0x1000]: can't assign; no space
>>> pci 0002:00:00.0: bridge window [io size 0x1000]: failed to assign
>>> pci 0002:01:00.0: BAR 5 [mem 0x16000000-0x16000fff]: assigned
>>> pci 0002:01:00.0: BAR 4 [io size 0x0020]: can't assign; no space
>>> pci 0002:01:00.0: BAR 4 [io size 0x0020]: failed to assign
>>> pci 0002:00:00.0: PCI bridge to [bus 01]
>>> pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff]
>>> pci_bus 0002:00: Some PCI device resources are unassigned, try booting with pci=realloc
>>> pci_bus 0002:00: resource 4 [mem 0x16000000-0x160fffff]
>>> pci_bus 0002:01: resource 1 [mem 0x16000000-0x160fffff]
>>> ...
>>> pci 0002:00:00.0: enabling device (0000 -> 0002)
>>> ahci 0002:01:00.0: enabling device (0000 -> 0002)
>>> ahci 0002:01:00.0: AHCI vers 0001.0000, 32 command slots, 1.5 Gbps, SATA mode
>>> ahci 0002:01:00.0: 6/6 ports implemented (port mask 0x3f)
>>> ahci 0002:01:00.0: flags: 64bit ncq only
>>
>> Hi,
>>
>> Thanks for reporting. Please add this to the command line to get the
>> resource releasing between the steps to show:
>>
>> dyndbg="file drivers/pci/setup-bus.c +p"
>>
>> Also, the log snippet just shows it fails but it is impossible to know
>> from it why the resource assigments do not fit so could you please provide
>> a complete dmesg logs. Also providing the contents of /proc/iomem from the
>> working case would save me quite a bit of decoding the iomem layout from
>> the dmesgs.
>
> Hi again,
>
> If you could kindly include this patch into the test with pci_dbg()
> enabled so the resource reset/restore is better tracked.
>
Same link as before (http://server.roeck-us.net/qemu/mipsel64/).
The log with the patch applied is in bad-extra.log.
Hope this helps,
Guenter
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 24/25] PCI: Perform reset_resource() and build fail list in sync
2025-04-01 14:15 ` Guenter Roeck
@ 2025-04-01 17:38 ` Ilpo Järvinen
2025-04-11 19:37 ` Ondřej Jirman
0 siblings, 1 reply; 43+ messages in thread
From: Ilpo Järvinen @ 2025-04-01 17:38 UTC (permalink / raw)
To: Guenter Roeck
Cc: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
LKML, Mika Westerberg
[-- Attachment #1: Type: text/plain, Size: 8409 bytes --]
On Tue, 1 Apr 2025, Guenter Roeck wrote:
> On 4/1/25 05:07, Ilpo Järvinen wrote:
> > On Tue, 1 Apr 2025, Ilpo Järvinen wrote:
> >
> > > On Mon, 31 Mar 2025, Guenter Roeck wrote:
> > > > On Mon, Dec 16, 2024 at 07:56:31PM +0200, Ilpo Järvinen wrote:
> > > > > Resetting resource is problematic as it prevent attempting to allocate
> > > > > the resource later, unless something in between restores the resource.
> > > > > Similarly, if fail_head does not contain all resources that were
> > > > > reset,
> > > > > those resource cannot be restored later.
> > > > >
> > > > > The entire reset/restore cycle adds complexity and leaving resources
> > > > > into reseted state causes issues to other code such as for checks done
> > > > > in pci_enable_resources(). Take a small step towards not resetting
> > > > > resources by delaying reset until the end of resource assignment and
> > > > > build failure list (fail_head) in sync with the reset to avoid leaving
> > > > > behind resources that cannot be restored (for the case where the
> > > > > caller
> > > > > provides fail_head in the first place to allow restore somewhere in
> > > > > the
> > > > > callchain, as is not all callers pass non-NULL fail_head).
> > > > >
> > > > > The Expansion ROM check is temporarily left in place while building
> > > > > the
> > > > > failure list until the upcoming change which reworks optional resource
> > > > > handling.
> > > > >
> > > > > Ideally, whole resource reset could be removed but doing that in a big
> > > > > step would make the impact non-tractable due to complexity of all
> > > > > related code.
> > > > >
> > > > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > > >
> > > > With this patch in the mainline kernel, all mips:boston qemu emulations
> > > > fail when running a 64-bit little endian configuration
> > > > (64r6el_defconfig).
> > > >
> > > > The problem is that the PCI based IDE/ATA controller is not initialized.
> > > > There are a number of pci error messages.
> > > >
> > > > pci_bus 0002:01: extended config space not accessible
> > > > pci 0002:01:00.0: [8086:2922] type 00 class 0x010601 conventional PCI
> > > > endpoint
> > > > pci 0002:01:00.0: BAR 4 [io 0x0000-0x001f]
> > > > pci 0002:01:00.0: BAR 5 [mem 0x00000000-0x00000fff]
> > > > pci 0002:00:00.0: PCI bridge to [bus 01-ff]
> > > > pci_bus 0002:01: busn_res: [bus 01-ff] end is updated to 01
> > > > pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff]: assigned
> > > > pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: can't
> > > > assign; no space
> > > > pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: failed
> > > > to assign
> > > > pci 0002:00:00.0: bridge window [io size 0x1000]: can't assign; no
> > > > space
> > > > pci 0002:00:00.0: bridge window [io size 0x1000]: failed to assign
> > > > pci 0002:00:00.0: bridge window [mem size 0x00100000]: can't assign;
> > > > bogus alignment
> > > > pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff 64bit pref]:
> > > > assigned
> > > > pci 0002:00:00.0: bridge window [io size 0x1000]: can't assign; no
> > > > space
> > > > pci 0002:00:00.0: bridge window [io size 0x1000]: failed to assign
> > > > pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: can't assign; no space
> > > > pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: failed to assign
> > > > pci 0002:01:00.0: BAR 4 [io size 0x0020]: can't assign; no space
> > > > pci 0002:01:00.0: BAR 4 [io size 0x0020]: failed to assign
> > > > pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: can't assign; no space
> > > > pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: failed to assign
> > > > pci 0002:01:00.0: BAR 4 [io size 0x0020]: can't assign; no space
> > > > pci 0002:01:00.0: BAR 4 [io size 0x0020]: failed to assign
> > > > pci 0002:00:00.0: PCI bridge to [bus 01]
> > > > pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff 64bit pref]
> > > > pci_bus 0002:00: Some PCI device resources are unassigned, try booting
> > > > with pci=realloc
> > > > pci_bus 0002:00: resource 4 [mem 0x16000000-0x160fffff]
> > > > pci_bus 0002:01: resource 2 [mem 0x16000000-0x160fffff 64bit pref]
> > > > ...
> > > > pci 0002:00:00.0: enabling device (0000 -> 0002)
> > > > ahci 0002:01:00.0: probe with driver ahci failed with error -12
> > > >
> > > > Bisect points to this patch. Reverting it together with "PCI: Rework
> > > > optional resource handling" fixes the problem. For comparison, after
> > > > reverting the offending patches, the log messages are as follows.
> > > >
> > > > pci_bus 0002:00: root bus resource [bus 00-ff]
> > > > pci_bus 0002:00: root bus resource [mem 0x16000000-0x160fffff]
> > > > pci 0002:00:00.0: [10ee:7021] type 01 class 0x060400 PCIe Root Complex
> > > > Integrated Endpoint
> > > > pci 0002:00:00.0: PCI bridge to [bus 00]
> > > > pci 0002:00:00.0: bridge window [io 0x0000-0x0fff]
> > > > pci 0002:00:00.0: bridge window [mem 0x00000000-0x000fffff]
> > > > pci 0002:00:00.0: bridge window [mem 0x00000000-0x000fffff 64bit pref]
> > > > pci 0002:00:00.0: enabling Extended Tags
> > > > pci 0002:00:00.0: bridge configuration invalid ([bus 00-00]),
> > > > reconfiguring
> > > > pci_bus 0002:01: extended config space not accessible
> > > > pci 0002:01:00.0: [8086:2922] type 00 class 0x010601 conventional PCI
> > > > endpoint
> > > > pci 0002:01:00.0: BAR 4 [io 0x0000-0x001f]
> > > > pci 0002:01:00.0: BAR 5 [mem 0x00000000-0x00000fff]
> > > > pci 0002:00:00.0: PCI bridge to [bus 01-ff]
> > > > pci_bus 0002:01: busn_res: [bus 01-ff] end is updated to 01
> > > > pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff]: assigned
> > > > pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: can't
> > > > assign; no space
> > > > pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: failed
> > > > to assign
> > > > pci 0002:00:00.0: bridge window [io size 0x1000]: can't assign; no
> > > > space
> > > > pci 0002:00:00.0: bridge window [io size 0x1000]: failed to assign
> > > > pci 0002:01:00.0: BAR 5 [mem 0x16000000-0x16000fff]: assigned
> > > > pci 0002:01:00.0: BAR 4 [io size 0x0020]: can't assign; no space
> > > > pci 0002:01:00.0: BAR 4 [io size 0x0020]: failed to assign
> > > > pci 0002:00:00.0: PCI bridge to [bus 01]
> > > > pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff]
> > > > pci_bus 0002:00: Some PCI device resources are unassigned, try booting
> > > > with pci=realloc
> > > > pci_bus 0002:00: resource 4 [mem 0x16000000-0x160fffff]
> > > > pci_bus 0002:01: resource 1 [mem 0x16000000-0x160fffff]
> > > > ...
> > > > pci 0002:00:00.0: enabling device (0000 -> 0002)
> > > > ahci 0002:01:00.0: enabling device (0000 -> 0002)
> > > > ahci 0002:01:00.0: AHCI vers 0001.0000, 32 command slots, 1.5 Gbps, SATA
> > > > mode
> > > > ahci 0002:01:00.0: 6/6 ports implemented (port mask 0x3f)
> > > > ahci 0002:01:00.0: flags: 64bit ncq only
> > >
> > > Hi,
> > >
> > > Thanks for reporting. Please add this to the command line to get the
> > > resource releasing between the steps to show:
> > >
> > > dyndbg="file drivers/pci/setup-bus.c +p"
> > >
> > > Also, the log snippet just shows it fails but it is impossible to know
> > > from it why the resource assigments do not fit so could you please provide
> > > a complete dmesg logs. Also providing the contents of /proc/iomem from the
> > > working case would save me quite a bit of decoding the iomem layout from
> > > the dmesgs.
> >
> > Hi again,
> >
> > If you could kindly include this patch into the test with pci_dbg()
> > enabled so the resource reset/restore is better tracked.
> >
>
> Same link as before (http://server.roeck-us.net/qemu/mipsel64/).
> The log with the patch applied is in bad-extra.log.
That log wasn't taken from a bad case but it doesn't matter anymore as I
could test this with qemu myself, thanks for providing enough to make it
easy to reproduce & test it locally :-).
The problem is caused by assign+release cycle being destructive on start
aligned resources because successful assigment overwrites the start field.
I'll send a patch to fix the problem once I've given it a bit more thought
as this resource fitting code is somewhat complex.
--
i.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 24/25] PCI: Perform reset_resource() and build fail list in sync
2025-04-01 17:38 ` Ilpo Järvinen
@ 2025-04-11 19:37 ` Ondřej Jirman
2025-04-14 9:52 ` Ilpo Järvinen
0 siblings, 1 reply; 43+ messages in thread
From: Ondřej Jirman @ 2025-04-11 19:37 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Guenter Roeck, Bjorn Helgaas, linux-pci, Michał Winiarski,
Igor Mammedov, LKML, Mika Westerberg
Hello Ilpo,
On Tue, Apr 01, 2025 at 08:38:48PM +0300, Ilpo Järvinen wrote:
> That log wasn't taken from a bad case but it doesn't matter anymore as I
> could test this with qemu myself, thanks for providing enough to make it
> easy to reproduce & test it locally :-).
>
> The problem is caused by assign+release cycle being destructive on start
> aligned resources because successful assigment overwrites the start field.
> I'll send a patch to fix the problem once I've given it a bit more thought
> as this resource fitting code is somewhat complex.
BTW, same thing here on a different SoC:
https://lore.kernel.org/lkml/hrcsm2bo4ysqj2ggejndlou32cdc7yiknnm5nrlcoz4d64wall@7te4dfqsoe3y/T/#u
There are kernel logs there, too, although I don't have dynamic debug enabled
in the kernel.
Interestingly, bisect pointed me initially to a different commit. Reverting
it helped, but just on one board (QuartzPro64).
And this is iomem:
0010f000-0010f0ff : 10f000.sram sram@10f000
00200000-e2bbffff : System RAM
02010000-0474ffff : Kernel code
04750000-0498ffff : reserved
04990000-0508ffff : Kernel data
daa00000-e29fffff : reserved
e2bc0000-ecbbffff : reserved
e2bc0000-ecbbffff : reserved
ecbc0000-efffffff : System RAM
ecbc7000-ecbdffff : reserved
f0000000-f00fffff : a40000000.pcie config
f0200000-f0ffffff : pcie@fe150000
f0200000-f020ffff : 0000:00:00.0
f0300000-f03fffff : PCI Bus 0000:01
f0300000-f0303fff : 0000:01:00.0
f0300000-f0303fff : nvme
f0304000-f03040ff : 0000:01:00.0
f0304000-f03040ff : nvme
f2000000-f20fffff : a40800000.pcie config
f2200000-f2ffffff : pcie@fe170000
f2200000-f27fffff : PCI Bus 0002:21
f2200000-f220ffff : 0002:21:00.0
f2400000-f27fffff : 0002:21:00.0
f2800000-f280ffff : 0002:20:00.0
f3000000-f30fffff : a40c00000.pcie config
f3200000-f3ffffff : pcie@fe180000
f3200000-f320ffff : 0003:30:00.0
f3300000-f33fffff : PCI Bus 0003:31
f3300000-f3303fff : 0003:31:00.0
f3304000-f3304fff : 0003:31:00.0
f3304000-f3304fff : r8169
fb000000-fb1fffff : fb000000.gpu gpu@fb000000
fc00c100-fc3fffff : fc000000.usb usb@fc000000
fc400000-fc407fff : usb@fc400000
fc400000-fc407fff : xhci-hcd.10.auto usb@fc400000
fc40c100-fc7fffff : fc400000.usb usb@fc400000
fc800000-fc83ffff : fc800000.usb usb@fc800000
fc840000-fc87ffff : fc840000.usb usb@fc840000
fc880000-fc8bffff : fc880000.usb usb@fc880000
fc8c0000-fc8fffff : fc8c0000.usb usb@fc8c0000
fc900000-fc900dff : fc900000.iommu
fc910000-fc910dff : fc900000.iommu
fd600000-fd6fffff : fd600000.sram sram@fd600000
fd8a0000-fd8a00ff : fd8a0000.gpio gpio@fd8a0000
fdb50000-fdb507ff : fdb50000.video-codec video-codec@fdb50000
fdb50800-fdb5083f : fdb50800.iommu iommu@fdb50800
fdb80000-fdb8017f : fdb80000.rga rga@fdb80000
fdba0000-fdba07ff : fdba0000.video-codec video-codec@fdba0000
fdba0800-fdba083f : fdba0800.iommu iommu@fdba0800
fdba4800-fdba483f : fdba4800.iommu iommu@fdba4800
fdba8800-fdba883f : fdba8800.iommu iommu@fdba8800
fdbac800-fdbac83f : fdbac800.iommu iommu@fdbac800
fdc70000-fdc707ff : fdc70000.video-codec video-codec@fdc70000
fdd90000-fdd941ff : fdd90000.vop vop
fdd95000-fdd95fff : fdd90000.vop gamma-lut
fdd97e00-fdd97eff : fdd97e00.iommu iommu@fdd97e00
fdd97f00-fdd97fff : fdd97e00.iommu iommu@fdd97e00
fddf0000-fddf0fff : fddf0000.i2s i2s@fddf0000
fddf4000-fddf4fff : fddf4000.i2s i2s@fddf4000
fde80000-fde9ffff : fde80000.hdmi hdmi@fde80000
fdea0000-fdebffff : fdea0000.hdmi hdmi@fdea0000
fdee0000-fdee5fff : fdee0000.hdmi_receiver hdmi_receiver@fdee0000
fe060000-fe06ffff : fe060000.dfi dfi@fe060000
fe150000-fe15ffff : a40000000.pcie apb
fe170000-fe17ffff : a40800000.pcie apb
fe180000-fe18ffff : a40c00000.pcie apb
fe1b0000-fe1bffff : fe1b0000.ethernet ethernet@fe1b0000
fe210000-fe210fff : fe210000.sata sata@fe210000
fe2c0000-fe2c3fff : fe2c0000.mmc mmc@fe2c0000
fe2e0000-fe2effff : fe2e0000.mmc mmc@fe2e0000
fe470000-fe470fff : fe470000.i2s i2s@fe470000
fe600000-fe60ffff : GICD
fe680000-fe77ffff : GICR
fea10000-fea13fff : dma-controller@fea10000
fea10000-fea13fff : fea10000.dma-controller dma-controller@fea10000
fea30000-fea33fff : dma-controller@fea30000
fea30000-fea33fff : fea30000.dma-controller dma-controller@fea30000
feaa0000-feaa0fff : feaa0000.i2c i2c@feaa0000
feaf0000-feaf00ff : feaf0000.watchdog watchdog@feaf0000
feb20000-feb20fff : feb20000.spi spi@feb20000
feb50000-feb500ff : serial
fec00000-fec003ff : fec00000.tsadc tsadc@fec00000
fec10000-fec1ffff : fec10000.adc adc@fec10000
fec20000-fec200ff : fec20000.gpio gpio@fec20000
fec30000-fec300ff : fec30000.gpio gpio@fec30000
fec40000-fec400ff : fec40000.gpio gpio@fec40000
fec50000-fec500ff : fec50000.gpio gpio@fec50000
fec90000-fec90fff : fec90000.i2c i2c@fec90000
fed10000-fed13fff : dma-controller@fed10000
fed10000-fed13fff : fed10000.dma-controller dma-controller@fed10000
fed60000-fed61fff : fed60000.phy phy@fed60000
fed70000-fed71fff : fed70000.phy phy@fed70000
fed80000-fed8ffff : fed80000.phy phy@fed80000
fed90000-fed9ffff : fed90000.phy phy@fed90000
fee00000-fee000ff : fee00000.phy phy@fee00000
fee10000-fee100ff : fee10000.phy phy@fee10000
fee20000-fee200ff : fee20000.phy phy@fee20000
fee80000-fee9ffff : fee80000.phy phy@fee80000
ff001000-ff0effff : ff001000.sram sram@ff001000
100000000-3fbffffff : System RAM
3ec000000-3fbffffff : reserved
3fc500000-3ffefffff : System RAM
4f0000000-4ffffffff : System RAM
4fc611000-4fc6d0fff : reserved
4fc6d1000-4fded1fff : reserved
4fded2000-4fdf91fff : reserved
4fdf93000-4fdf96fff : reserved
4fdf97000-4fdfabfff : reserved
4fdfac000-4fe051fff : reserved
4fe052000-4ffffffff : reserved
900000000-93fffffff : pcie@fe150000
900000000-93fffffff : 0000:00:00.0
980000000-9bfffffff : pcie@fe170000
9c0000000-9ffffffff : pcie@fe180000
a40000000-a403fffff : a40000000.pcie dbi
a40800000-a40bfffff : a40800000.pcie dbi
a40c00000-a40ffffff : a40c00000.pcie dbi
Thank you,
o.
> --
> i.
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH 24/25] PCI: Perform reset_resource() and build fail list in sync
2025-04-11 19:37 ` Ondřej Jirman
@ 2025-04-14 9:52 ` Ilpo Järvinen
2025-04-14 12:19 ` Ondřej Jirman
0 siblings, 1 reply; 43+ messages in thread
From: Ilpo Järvinen @ 2025-04-14 9:52 UTC (permalink / raw)
To: Ondřej Jirman
Cc: Guenter Roeck, Bjorn Helgaas, linux-pci, Michał Winiarski,
Igor Mammedov, LKML, Mika Westerberg
[-- Attachment #1: Type: text/plain, Size: 6581 bytes --]
On Fri, 11 Apr 2025, Ondřej Jirman wrote:
> Hello Ilpo,
>
> On Tue, Apr 01, 2025 at 08:38:48PM +0300, Ilpo Järvinen wrote:
> > That log wasn't taken from a bad case but it doesn't matter anymore as I
> > could test this with qemu myself, thanks for providing enough to make it
> > easy to reproduce & test it locally :-).
> >
> > The problem is caused by assign+release cycle being destructive on start
> > aligned resources because successful assigment overwrites the start field.
> > I'll send a patch to fix the problem once I've given it a bit more thought
> > as this resource fitting code is somewhat complex.
>
> BTW, same thing here on a different SoC:
>
> https://lore.kernel.org/lkml/hrcsm2bo4ysqj2ggejndlou32cdc7yiknnm5nrlcoz4d64wall@7te4dfqsoe3y/T/#u
>
> There are kernel logs there, too, although I don't have dynamic debug enabled
> in the kernel.
>
> Interestingly, bisect pointed me initially to a different commit. Reverting
> it helped, but just on one board (QuartzPro64).
Hi,
Since you didn't mention it, I guess you haven't tried the fix:
https://patchwork.kernel.org/project/linux-pci/patch/20250403093137.1481-1-ilpo.jarvinen@linux.intel.com/
--
i.
> And this is iomem:
>
> 0010f000-0010f0ff : 10f000.sram sram@10f000
> 00200000-e2bbffff : System RAM
> 02010000-0474ffff : Kernel code
> 04750000-0498ffff : reserved
> 04990000-0508ffff : Kernel data
> daa00000-e29fffff : reserved
> e2bc0000-ecbbffff : reserved
> e2bc0000-ecbbffff : reserved
> ecbc0000-efffffff : System RAM
> ecbc7000-ecbdffff : reserved
> f0000000-f00fffff : a40000000.pcie config
> f0200000-f0ffffff : pcie@fe150000
> f0200000-f020ffff : 0000:00:00.0
> f0300000-f03fffff : PCI Bus 0000:01
> f0300000-f0303fff : 0000:01:00.0
> f0300000-f0303fff : nvme
> f0304000-f03040ff : 0000:01:00.0
> f0304000-f03040ff : nvme
> f2000000-f20fffff : a40800000.pcie config
> f2200000-f2ffffff : pcie@fe170000
> f2200000-f27fffff : PCI Bus 0002:21
> f2200000-f220ffff : 0002:21:00.0
> f2400000-f27fffff : 0002:21:00.0
> f2800000-f280ffff : 0002:20:00.0
> f3000000-f30fffff : a40c00000.pcie config
> f3200000-f3ffffff : pcie@fe180000
> f3200000-f320ffff : 0003:30:00.0
> f3300000-f33fffff : PCI Bus 0003:31
> f3300000-f3303fff : 0003:31:00.0
> f3304000-f3304fff : 0003:31:00.0
> f3304000-f3304fff : r8169
> fb000000-fb1fffff : fb000000.gpu gpu@fb000000
> fc00c100-fc3fffff : fc000000.usb usb@fc000000
> fc400000-fc407fff : usb@fc400000
> fc400000-fc407fff : xhci-hcd.10.auto usb@fc400000
> fc40c100-fc7fffff : fc400000.usb usb@fc400000
> fc800000-fc83ffff : fc800000.usb usb@fc800000
> fc840000-fc87ffff : fc840000.usb usb@fc840000
> fc880000-fc8bffff : fc880000.usb usb@fc880000
> fc8c0000-fc8fffff : fc8c0000.usb usb@fc8c0000
> fc900000-fc900dff : fc900000.iommu
> fc910000-fc910dff : fc900000.iommu
> fd600000-fd6fffff : fd600000.sram sram@fd600000
> fd8a0000-fd8a00ff : fd8a0000.gpio gpio@fd8a0000
> fdb50000-fdb507ff : fdb50000.video-codec video-codec@fdb50000
> fdb50800-fdb5083f : fdb50800.iommu iommu@fdb50800
> fdb80000-fdb8017f : fdb80000.rga rga@fdb80000
> fdba0000-fdba07ff : fdba0000.video-codec video-codec@fdba0000
> fdba0800-fdba083f : fdba0800.iommu iommu@fdba0800
> fdba4800-fdba483f : fdba4800.iommu iommu@fdba4800
> fdba8800-fdba883f : fdba8800.iommu iommu@fdba8800
> fdbac800-fdbac83f : fdbac800.iommu iommu@fdbac800
> fdc70000-fdc707ff : fdc70000.video-codec video-codec@fdc70000
> fdd90000-fdd941ff : fdd90000.vop vop
> fdd95000-fdd95fff : fdd90000.vop gamma-lut
> fdd97e00-fdd97eff : fdd97e00.iommu iommu@fdd97e00
> fdd97f00-fdd97fff : fdd97e00.iommu iommu@fdd97e00
> fddf0000-fddf0fff : fddf0000.i2s i2s@fddf0000
> fddf4000-fddf4fff : fddf4000.i2s i2s@fddf4000
> fde80000-fde9ffff : fde80000.hdmi hdmi@fde80000
> fdea0000-fdebffff : fdea0000.hdmi hdmi@fdea0000
> fdee0000-fdee5fff : fdee0000.hdmi_receiver hdmi_receiver@fdee0000
> fe060000-fe06ffff : fe060000.dfi dfi@fe060000
> fe150000-fe15ffff : a40000000.pcie apb
> fe170000-fe17ffff : a40800000.pcie apb
> fe180000-fe18ffff : a40c00000.pcie apb
> fe1b0000-fe1bffff : fe1b0000.ethernet ethernet@fe1b0000
> fe210000-fe210fff : fe210000.sata sata@fe210000
> fe2c0000-fe2c3fff : fe2c0000.mmc mmc@fe2c0000
> fe2e0000-fe2effff : fe2e0000.mmc mmc@fe2e0000
> fe470000-fe470fff : fe470000.i2s i2s@fe470000
> fe600000-fe60ffff : GICD
> fe680000-fe77ffff : GICR
> fea10000-fea13fff : dma-controller@fea10000
> fea10000-fea13fff : fea10000.dma-controller dma-controller@fea10000
> fea30000-fea33fff : dma-controller@fea30000
> fea30000-fea33fff : fea30000.dma-controller dma-controller@fea30000
> feaa0000-feaa0fff : feaa0000.i2c i2c@feaa0000
> feaf0000-feaf00ff : feaf0000.watchdog watchdog@feaf0000
> feb20000-feb20fff : feb20000.spi spi@feb20000
> feb50000-feb500ff : serial
> fec00000-fec003ff : fec00000.tsadc tsadc@fec00000
> fec10000-fec1ffff : fec10000.adc adc@fec10000
> fec20000-fec200ff : fec20000.gpio gpio@fec20000
> fec30000-fec300ff : fec30000.gpio gpio@fec30000
> fec40000-fec400ff : fec40000.gpio gpio@fec40000
> fec50000-fec500ff : fec50000.gpio gpio@fec50000
> fec90000-fec90fff : fec90000.i2c i2c@fec90000
> fed10000-fed13fff : dma-controller@fed10000
> fed10000-fed13fff : fed10000.dma-controller dma-controller@fed10000
> fed60000-fed61fff : fed60000.phy phy@fed60000
> fed70000-fed71fff : fed70000.phy phy@fed70000
> fed80000-fed8ffff : fed80000.phy phy@fed80000
> fed90000-fed9ffff : fed90000.phy phy@fed90000
> fee00000-fee000ff : fee00000.phy phy@fee00000
> fee10000-fee100ff : fee10000.phy phy@fee10000
> fee20000-fee200ff : fee20000.phy phy@fee20000
> fee80000-fee9ffff : fee80000.phy phy@fee80000
> ff001000-ff0effff : ff001000.sram sram@ff001000
> 100000000-3fbffffff : System RAM
> 3ec000000-3fbffffff : reserved
> 3fc500000-3ffefffff : System RAM
> 4f0000000-4ffffffff : System RAM
> 4fc611000-4fc6d0fff : reserved
> 4fc6d1000-4fded1fff : reserved
> 4fded2000-4fdf91fff : reserved
> 4fdf93000-4fdf96fff : reserved
> 4fdf97000-4fdfabfff : reserved
> 4fdfac000-4fe051fff : reserved
> 4fe052000-4ffffffff : reserved
> 900000000-93fffffff : pcie@fe150000
> 900000000-93fffffff : 0000:00:00.0
> 980000000-9bfffffff : pcie@fe170000
> 9c0000000-9ffffffff : pcie@fe180000
> a40000000-a403fffff : a40000000.pcie dbi
> a40800000-a40bfffff : a40800000.pcie dbi
> a40c00000-a40ffffff : a40c00000.pcie dbi
>
> Thank you,
> o.
>
> > --
> > i.
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 24/25] PCI: Perform reset_resource() and build fail list in sync
2025-04-14 9:52 ` Ilpo Järvinen
@ 2025-04-14 12:19 ` Ondřej Jirman
2025-04-14 13:15 ` Ilpo Järvinen
0 siblings, 1 reply; 43+ messages in thread
From: Ondřej Jirman @ 2025-04-14 12:19 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Guenter Roeck, Bjorn Helgaas, linux-pci, Michał Winiarski,
Igor Mammedov, LKML, Mika Westerberg
Hello Ilpo,
On Mon, Apr 14, 2025 at 12:52:15PM +0300, Ilpo Järvinen wrote:
> On Fri, 11 Apr 2025, Ondřej Jirman wrote:
>
> > Hello Ilpo,
> >
> > On Tue, Apr 01, 2025 at 08:38:48PM +0300, Ilpo Järvinen wrote:
> > > That log wasn't taken from a bad case but it doesn't matter anymore as I
> > > could test this with qemu myself, thanks for providing enough to make it
> > > easy to reproduce & test it locally :-).
> > >
> > > The problem is caused by assign+release cycle being destructive on start
> > > aligned resources because successful assigment overwrites the start field.
> > > I'll send a patch to fix the problem once I've given it a bit more thought
> > > as this resource fitting code is somewhat complex.
> >
> > BTW, same thing here on a different SoC:
> >
> > https://lore.kernel.org/lkml/hrcsm2bo4ysqj2ggejndlou32cdc7yiknnm5nrlcoz4d64wall@7te4dfqsoe3y/T/#u
> >
> > There are kernel logs there, too, although I don't have dynamic debug enabled
> > in the kernel.
> >
> > Interestingly, bisect pointed me initially to a different commit. Reverting
> > it helped, but just on one board (QuartzPro64).
>
> Hi,
>
> Since you didn't mention it, I guess you haven't tried the fix:
>
> https://patchwork.kernel.org/project/linux-pci/patch/20250403093137.1481-1-ilpo.jarvinen@linux.intel.com/
This patch works. Thank you. One difference compared to 6.14 that I'm noticing
in the kernel log is that "save config" sequences now are printed twice for
unpopulated port. Not sure if it's related to your patches. Previously it was
printed just once.
Kind regards,
o.
rockchip-dw-pcie a40800000.pcie: PCI host bridge to bus 0002:20
pci_bus 0002:20: root bus resource [bus 20-2f]
pci_bus 0002:20: root bus resource [io 0x300000-0x3fffff] (bus address [0xf2100000-0xf21fffff])
pci_bus 0002:20: root bus resource [mem 0xf2200000-0xf2ffffff]
pci_bus 0002:20: root bus resource [mem 0x980000000-0x9bfffffff] (bus address [0x40000000-0x7fffffff])
pci_bus 0002:20: scanning bus
pci 0002:20:00.0: [1d87:3588] type 01 class 0x060400 PCIe Root Port
pci 0002:20:00.0: ROM [mem 0x00000000-0x0000ffff pref]
pci 0002:20:00.0: PCI bridge to [bus 01-ff]
pci 0002:20:00.0: bridge window [io 0x0000-0x0fff]
pci 0002:20:00.0: bridge window [mem 0x00000000-0x000fffff]
pci 0002:20:00.0: bridge window [mem 0x00000000-0x000fffff 64bit pref]
pci 0002:20:00.0: supports D1 D2
pci 0002:20:00.0: PME# supported from D0 D1 D3hot
pci 0002:20:00.0: PME# disabled
pci 0002:20:00.0: Adding to iommu group 9
pci 0002:20:00.0: vgaarb: pci_notify
pci_bus 0002:20: fixups for bus
pci 0002:20:00.0: scanning [bus 01-ff] behind bridge, pass 0
pci 0002:20:00.0: Primary bus is hard wired to 0
pci 0002:20:00.0: bridge configuration invalid ([bus 01-ff]), reconfiguring
pci 0002:20:00.0: scanning [bus 00-00] behind bridge, pass 1
pci_bus 0002:20: bus scan returning with max=21
pci 0002:20:00.0: ROM [mem 0xf2200000-0xf220ffff pref]: assigned
pci 0002:20:00.0: PCI bridge to [bus 21]
pci_bus 0002:20: resource 4 [io 0x300000-0x3fffff]
pci_bus 0002:20: resource 5 [mem 0xf2200000-0xf2ffffff]
pci_bus 0002:20: resource 6 [mem 0x980000000-0x9bfffffff]
pcieport 0002:20:00.0: vgaarb: pci_notify
pcieport 0002:20:00.0: assign IRQ: got 148
pcieport 0002:20:00.0: PME: Signaling with IRQ 157
pcieport 0002:20:00.0: AER: enabled with IRQ 158
pcieport 0002:20:00.0: bwctrl: enabled with IRQ 157
pcieport 0002:20:00.0: save config 0x00: 0x35881d87
pcieport 0002:20:00.0: save config 0x04: 0x00100507
pcieport 0002:20:00.0: save config 0x08: 0x06040001
pcieport 0002:20:00.0: save config 0x0c: 0x00010000
pcieport 0002:20:00.0: save config 0x10: 0x00000000
pcieport 0002:20:00.0: save config 0x14: 0x00000000
pcieport 0002:20:00.0: save config 0x18: 0x00212120
pcieport 0002:20:00.0: save config 0x1c: 0x000000f0
pcieport 0002:20:00.0: save config 0x20: 0x0000fff0
pcieport 0002:20:00.0: save config 0x24: 0x0001fff1
pcieport 0002:20:00.0: save config 0x28: 0x00000000
pcieport 0002:20:00.0: save config 0x2c: 0x00000000
pcieport 0002:20:00.0: save config 0x30: 0x00000000
pcieport 0002:20:00.0: save config 0x34: 0x00000040
pcieport 0002:20:00.0: save config 0x38: 0x00000000
pcieport 0002:20:00.0: save config 0x3c: 0x00020194
pcieport 0002:20:00.0: vgaarb: pci_notify
pcieport 0002:20:00.0: save config 0x00: 0x35881d87
pcieport 0002:20:00.0: save config 0x04: 0x00100507
pcieport 0002:20:00.0: save config 0x08: 0x06040001
pcieport 0002:20:00.0: save config 0x0c: 0x00010000
pcieport 0002:20:00.0: save config 0x10: 0x00000000
pcieport 0002:20:00.0: save config 0x14: 0x00000000
pcieport 0002:20:00.0: save config 0x18: 0x00212120
pcieport 0002:20:00.0: save config 0x1c: 0x000000f0
pcieport 0002:20:00.0: save config 0x20: 0x0000fff0
pcieport 0002:20:00.0: save config 0x24: 0x0001fff1
pcieport 0002:20:00.0: save config 0x28: 0x00000000
pcieport 0002:20:00.0: save config 0x2c: 0x00000000
pcieport 0002:20:00.0: save config 0x30: 0x00000000
pcieport 0002:20:00.0: save config 0x34: 0x00000040
pcieport 0002:20:00.0: save config 0x38: 0x00000000
pcieport 0002:20:00.0: save config 0x3c: 0x00020194
pcieport 0002:20:00.0: PME# enabled
> --
> i.
>
> > And this is iomem:
> >
> > 0010f000-0010f0ff : 10f000.sram sram@10f000
> > 00200000-e2bbffff : System RAM
> > 02010000-0474ffff : Kernel code
> > 04750000-0498ffff : reserved
> > 04990000-0508ffff : Kernel data
> > daa00000-e29fffff : reserved
> > e2bc0000-ecbbffff : reserved
> > e2bc0000-ecbbffff : reserved
> > ecbc0000-efffffff : System RAM
> > ecbc7000-ecbdffff : reserved
> > f0000000-f00fffff : a40000000.pcie config
> > f0200000-f0ffffff : pcie@fe150000
> > f0200000-f020ffff : 0000:00:00.0
> > f0300000-f03fffff : PCI Bus 0000:01
> > f0300000-f0303fff : 0000:01:00.0
> > f0300000-f0303fff : nvme
> > f0304000-f03040ff : 0000:01:00.0
> > f0304000-f03040ff : nvme
> > f2000000-f20fffff : a40800000.pcie config
> > f2200000-f2ffffff : pcie@fe170000
> > f2200000-f27fffff : PCI Bus 0002:21
> > f2200000-f220ffff : 0002:21:00.0
> > f2400000-f27fffff : 0002:21:00.0
> > f2800000-f280ffff : 0002:20:00.0
> > f3000000-f30fffff : a40c00000.pcie config
> > f3200000-f3ffffff : pcie@fe180000
> > f3200000-f320ffff : 0003:30:00.0
> > f3300000-f33fffff : PCI Bus 0003:31
> > f3300000-f3303fff : 0003:31:00.0
> > f3304000-f3304fff : 0003:31:00.0
> > f3304000-f3304fff : r8169
> > fb000000-fb1fffff : fb000000.gpu gpu@fb000000
> > fc00c100-fc3fffff : fc000000.usb usb@fc000000
> > fc400000-fc407fff : usb@fc400000
> > fc400000-fc407fff : xhci-hcd.10.auto usb@fc400000
> > fc40c100-fc7fffff : fc400000.usb usb@fc400000
> > fc800000-fc83ffff : fc800000.usb usb@fc800000
> > fc840000-fc87ffff : fc840000.usb usb@fc840000
> > fc880000-fc8bffff : fc880000.usb usb@fc880000
> > fc8c0000-fc8fffff : fc8c0000.usb usb@fc8c0000
> > fc900000-fc900dff : fc900000.iommu
> > fc910000-fc910dff : fc900000.iommu
> > fd600000-fd6fffff : fd600000.sram sram@fd600000
> > fd8a0000-fd8a00ff : fd8a0000.gpio gpio@fd8a0000
> > fdb50000-fdb507ff : fdb50000.video-codec video-codec@fdb50000
> > fdb50800-fdb5083f : fdb50800.iommu iommu@fdb50800
> > fdb80000-fdb8017f : fdb80000.rga rga@fdb80000
> > fdba0000-fdba07ff : fdba0000.video-codec video-codec@fdba0000
> > fdba0800-fdba083f : fdba0800.iommu iommu@fdba0800
> > fdba4800-fdba483f : fdba4800.iommu iommu@fdba4800
> > fdba8800-fdba883f : fdba8800.iommu iommu@fdba8800
> > fdbac800-fdbac83f : fdbac800.iommu iommu@fdbac800
> > fdc70000-fdc707ff : fdc70000.video-codec video-codec@fdc70000
> > fdd90000-fdd941ff : fdd90000.vop vop
> > fdd95000-fdd95fff : fdd90000.vop gamma-lut
> > fdd97e00-fdd97eff : fdd97e00.iommu iommu@fdd97e00
> > fdd97f00-fdd97fff : fdd97e00.iommu iommu@fdd97e00
> > fddf0000-fddf0fff : fddf0000.i2s i2s@fddf0000
> > fddf4000-fddf4fff : fddf4000.i2s i2s@fddf4000
> > fde80000-fde9ffff : fde80000.hdmi hdmi@fde80000
> > fdea0000-fdebffff : fdea0000.hdmi hdmi@fdea0000
> > fdee0000-fdee5fff : fdee0000.hdmi_receiver hdmi_receiver@fdee0000
> > fe060000-fe06ffff : fe060000.dfi dfi@fe060000
> > fe150000-fe15ffff : a40000000.pcie apb
> > fe170000-fe17ffff : a40800000.pcie apb
> > fe180000-fe18ffff : a40c00000.pcie apb
> > fe1b0000-fe1bffff : fe1b0000.ethernet ethernet@fe1b0000
> > fe210000-fe210fff : fe210000.sata sata@fe210000
> > fe2c0000-fe2c3fff : fe2c0000.mmc mmc@fe2c0000
> > fe2e0000-fe2effff : fe2e0000.mmc mmc@fe2e0000
> > fe470000-fe470fff : fe470000.i2s i2s@fe470000
> > fe600000-fe60ffff : GICD
> > fe680000-fe77ffff : GICR
> > fea10000-fea13fff : dma-controller@fea10000
> > fea10000-fea13fff : fea10000.dma-controller dma-controller@fea10000
> > fea30000-fea33fff : dma-controller@fea30000
> > fea30000-fea33fff : fea30000.dma-controller dma-controller@fea30000
> > feaa0000-feaa0fff : feaa0000.i2c i2c@feaa0000
> > feaf0000-feaf00ff : feaf0000.watchdog watchdog@feaf0000
> > feb20000-feb20fff : feb20000.spi spi@feb20000
> > feb50000-feb500ff : serial
> > fec00000-fec003ff : fec00000.tsadc tsadc@fec00000
> > fec10000-fec1ffff : fec10000.adc adc@fec10000
> > fec20000-fec200ff : fec20000.gpio gpio@fec20000
> > fec30000-fec300ff : fec30000.gpio gpio@fec30000
> > fec40000-fec400ff : fec40000.gpio gpio@fec40000
> > fec50000-fec500ff : fec50000.gpio gpio@fec50000
> > fec90000-fec90fff : fec90000.i2c i2c@fec90000
> > fed10000-fed13fff : dma-controller@fed10000
> > fed10000-fed13fff : fed10000.dma-controller dma-controller@fed10000
> > fed60000-fed61fff : fed60000.phy phy@fed60000
> > fed70000-fed71fff : fed70000.phy phy@fed70000
> > fed80000-fed8ffff : fed80000.phy phy@fed80000
> > fed90000-fed9ffff : fed90000.phy phy@fed90000
> > fee00000-fee000ff : fee00000.phy phy@fee00000
> > fee10000-fee100ff : fee10000.phy phy@fee10000
> > fee20000-fee200ff : fee20000.phy phy@fee20000
> > fee80000-fee9ffff : fee80000.phy phy@fee80000
> > ff001000-ff0effff : ff001000.sram sram@ff001000
> > 100000000-3fbffffff : System RAM
> > 3ec000000-3fbffffff : reserved
> > 3fc500000-3ffefffff : System RAM
> > 4f0000000-4ffffffff : System RAM
> > 4fc611000-4fc6d0fff : reserved
> > 4fc6d1000-4fded1fff : reserved
> > 4fded2000-4fdf91fff : reserved
> > 4fdf93000-4fdf96fff : reserved
> > 4fdf97000-4fdfabfff : reserved
> > 4fdfac000-4fe051fff : reserved
> > 4fe052000-4ffffffff : reserved
> > 900000000-93fffffff : pcie@fe150000
> > 900000000-93fffffff : 0000:00:00.0
> > 980000000-9bfffffff : pcie@fe170000
> > 9c0000000-9ffffffff : pcie@fe180000
> > a40000000-a403fffff : a40000000.pcie dbi
> > a40800000-a40bfffff : a40800000.pcie dbi
> > a40c00000-a40ffffff : a40c00000.pcie dbi
> >
> > Thank you,
> > o.
> >
> > > --
> > > i.
> >
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 24/25] PCI: Perform reset_resource() and build fail list in sync
2025-04-14 12:19 ` Ondřej Jirman
@ 2025-04-14 13:15 ` Ilpo Järvinen
2025-04-14 13:43 ` Ondřej Jirman
0 siblings, 1 reply; 43+ messages in thread
From: Ilpo Järvinen @ 2025-04-14 13:15 UTC (permalink / raw)
To: Ondřej Jirman
Cc: Guenter Roeck, Bjorn Helgaas, linux-pci, Michał Winiarski,
Igor Mammedov, LKML, Mika Westerberg
[-- Attachment #1: Type: text/plain, Size: 12103 bytes --]
On Mon, 14 Apr 2025, Ondřej Jirman wrote:
> On Mon, Apr 14, 2025 at 12:52:15PM +0300, Ilpo Järvinen wrote:
> > On Fri, 11 Apr 2025, Ondřej Jirman wrote:
> >
> > > Hello Ilpo,
> > >
> > > On Tue, Apr 01, 2025 at 08:38:48PM +0300, Ilpo Järvinen wrote:
> > > > That log wasn't taken from a bad case but it doesn't matter anymore as I
> > > > could test this with qemu myself, thanks for providing enough to make it
> > > > easy to reproduce & test it locally :-).
> > > >
> > > > The problem is caused by assign+release cycle being destructive on start
> > > > aligned resources because successful assigment overwrites the start field.
> > > > I'll send a patch to fix the problem once I've given it a bit more thought
> > > > as this resource fitting code is somewhat complex.
> > >
> > > BTW, same thing here on a different SoC:
> > >
> > > https://lore.kernel.org/lkml/hrcsm2bo4ysqj2ggejndlou32cdc7yiknnm5nrlcoz4d64wall@7te4dfqsoe3y/T/#u
> > >
> > > There are kernel logs there, too, although I don't have dynamic debug enabled
> > > in the kernel.
> > >
> > > Interestingly, bisect pointed me initially to a different commit. Reverting
> > > it helped, but just on one board (QuartzPro64).
> >
> > Hi,
> >
> > Since you didn't mention it, I guess you haven't tried the fix:
> >
> > https://patchwork.kernel.org/project/linux-pci/patch/20250403093137.1481-1-ilpo.jarvinen@linux.intel.com/
>
> This patch works. Thank you.
Thanks for testing. If you feel confident enough, please send your
Tested-by tag to its thread (not this one).
> One difference compared to 6.14 that I'm noticing
> in the kernel log is that "save config" sequences now are printed twice for
> unpopulated port. Not sure if it's related to your patches. Previously it was
> printed just once.
I don't think it is related. The resource fitting/assignment changes
should not impact PCI save/restore AFAIK (except by preventing device from
working in the first place if necessary resources do not get assigned).
PCI state saving has always seemed like that in most logs I've seen with
dyndbg enabled, that is, the state is seemingly saved multiple times,
often right after the previous save too. So TBH, I'm not convinced it's
even something new. If you have backtraces to show those places that
invoke pci_save_state() I can take a look but I don't expect much to
come out of that.
--
i.
> Kind regards,
> o.
>
> rockchip-dw-pcie a40800000.pcie: PCI host bridge to bus 0002:20
> pci_bus 0002:20: root bus resource [bus 20-2f]
> pci_bus 0002:20: root bus resource [io 0x300000-0x3fffff] (bus address [0xf2100000-0xf21fffff])
> pci_bus 0002:20: root bus resource [mem 0xf2200000-0xf2ffffff]
> pci_bus 0002:20: root bus resource [mem 0x980000000-0x9bfffffff] (bus address [0x40000000-0x7fffffff])
> pci_bus 0002:20: scanning bus
> pci 0002:20:00.0: [1d87:3588] type 01 class 0x060400 PCIe Root Port
> pci 0002:20:00.0: ROM [mem 0x00000000-0x0000ffff pref]
> pci 0002:20:00.0: PCI bridge to [bus 01-ff]
> pci 0002:20:00.0: bridge window [io 0x0000-0x0fff]
> pci 0002:20:00.0: bridge window [mem 0x00000000-0x000fffff]
> pci 0002:20:00.0: bridge window [mem 0x00000000-0x000fffff 64bit pref]
> pci 0002:20:00.0: supports D1 D2
> pci 0002:20:00.0: PME# supported from D0 D1 D3hot
> pci 0002:20:00.0: PME# disabled
> pci 0002:20:00.0: Adding to iommu group 9
> pci 0002:20:00.0: vgaarb: pci_notify
> pci_bus 0002:20: fixups for bus
> pci 0002:20:00.0: scanning [bus 01-ff] behind bridge, pass 0
> pci 0002:20:00.0: Primary bus is hard wired to 0
> pci 0002:20:00.0: bridge configuration invalid ([bus 01-ff]), reconfiguring
> pci 0002:20:00.0: scanning [bus 00-00] behind bridge, pass 1
> pci_bus 0002:20: bus scan returning with max=21
> pci 0002:20:00.0: ROM [mem 0xf2200000-0xf220ffff pref]: assigned
> pci 0002:20:00.0: PCI bridge to [bus 21]
> pci_bus 0002:20: resource 4 [io 0x300000-0x3fffff]
> pci_bus 0002:20: resource 5 [mem 0xf2200000-0xf2ffffff]
> pci_bus 0002:20: resource 6 [mem 0x980000000-0x9bfffffff]
> pcieport 0002:20:00.0: vgaarb: pci_notify
> pcieport 0002:20:00.0: assign IRQ: got 148
> pcieport 0002:20:00.0: PME: Signaling with IRQ 157
> pcieport 0002:20:00.0: AER: enabled with IRQ 158
> pcieport 0002:20:00.0: bwctrl: enabled with IRQ 157
> pcieport 0002:20:00.0: save config 0x00: 0x35881d87
> pcieport 0002:20:00.0: save config 0x04: 0x00100507
> pcieport 0002:20:00.0: save config 0x08: 0x06040001
> pcieport 0002:20:00.0: save config 0x0c: 0x00010000
> pcieport 0002:20:00.0: save config 0x10: 0x00000000
> pcieport 0002:20:00.0: save config 0x14: 0x00000000
> pcieport 0002:20:00.0: save config 0x18: 0x00212120
> pcieport 0002:20:00.0: save config 0x1c: 0x000000f0
> pcieport 0002:20:00.0: save config 0x20: 0x0000fff0
> pcieport 0002:20:00.0: save config 0x24: 0x0001fff1
> pcieport 0002:20:00.0: save config 0x28: 0x00000000
> pcieport 0002:20:00.0: save config 0x2c: 0x00000000
> pcieport 0002:20:00.0: save config 0x30: 0x00000000
> pcieport 0002:20:00.0: save config 0x34: 0x00000040
> pcieport 0002:20:00.0: save config 0x38: 0x00000000
> pcieport 0002:20:00.0: save config 0x3c: 0x00020194
> pcieport 0002:20:00.0: vgaarb: pci_notify
> pcieport 0002:20:00.0: save config 0x00: 0x35881d87
> pcieport 0002:20:00.0: save config 0x04: 0x00100507
> pcieport 0002:20:00.0: save config 0x08: 0x06040001
> pcieport 0002:20:00.0: save config 0x0c: 0x00010000
> pcieport 0002:20:00.0: save config 0x10: 0x00000000
> pcieport 0002:20:00.0: save config 0x14: 0x00000000
> pcieport 0002:20:00.0: save config 0x18: 0x00212120
> pcieport 0002:20:00.0: save config 0x1c: 0x000000f0
> pcieport 0002:20:00.0: save config 0x20: 0x0000fff0
> pcieport 0002:20:00.0: save config 0x24: 0x0001fff1
> pcieport 0002:20:00.0: save config 0x28: 0x00000000
> pcieport 0002:20:00.0: save config 0x2c: 0x00000000
> pcieport 0002:20:00.0: save config 0x30: 0x00000000
> pcieport 0002:20:00.0: save config 0x34: 0x00000040
> pcieport 0002:20:00.0: save config 0x38: 0x00000000
> pcieport 0002:20:00.0: save config 0x3c: 0x00020194
> pcieport 0002:20:00.0: PME# enabled
>
>
> > --
> > i.
> >
> > > And this is iomem:
> > >
> > > 0010f000-0010f0ff : 10f000.sram sram@10f000
> > > 00200000-e2bbffff : System RAM
> > > 02010000-0474ffff : Kernel code
> > > 04750000-0498ffff : reserved
> > > 04990000-0508ffff : Kernel data
> > > daa00000-e29fffff : reserved
> > > e2bc0000-ecbbffff : reserved
> > > e2bc0000-ecbbffff : reserved
> > > ecbc0000-efffffff : System RAM
> > > ecbc7000-ecbdffff : reserved
> > > f0000000-f00fffff : a40000000.pcie config
> > > f0200000-f0ffffff : pcie@fe150000
> > > f0200000-f020ffff : 0000:00:00.0
> > > f0300000-f03fffff : PCI Bus 0000:01
> > > f0300000-f0303fff : 0000:01:00.0
> > > f0300000-f0303fff : nvme
> > > f0304000-f03040ff : 0000:01:00.0
> > > f0304000-f03040ff : nvme
> > > f2000000-f20fffff : a40800000.pcie config
> > > f2200000-f2ffffff : pcie@fe170000
> > > f2200000-f27fffff : PCI Bus 0002:21
> > > f2200000-f220ffff : 0002:21:00.0
> > > f2400000-f27fffff : 0002:21:00.0
> > > f2800000-f280ffff : 0002:20:00.0
> > > f3000000-f30fffff : a40c00000.pcie config
> > > f3200000-f3ffffff : pcie@fe180000
> > > f3200000-f320ffff : 0003:30:00.0
> > > f3300000-f33fffff : PCI Bus 0003:31
> > > f3300000-f3303fff : 0003:31:00.0
> > > f3304000-f3304fff : 0003:31:00.0
> > > f3304000-f3304fff : r8169
> > > fb000000-fb1fffff : fb000000.gpu gpu@fb000000
> > > fc00c100-fc3fffff : fc000000.usb usb@fc000000
> > > fc400000-fc407fff : usb@fc400000
> > > fc400000-fc407fff : xhci-hcd.10.auto usb@fc400000
> > > fc40c100-fc7fffff : fc400000.usb usb@fc400000
> > > fc800000-fc83ffff : fc800000.usb usb@fc800000
> > > fc840000-fc87ffff : fc840000.usb usb@fc840000
> > > fc880000-fc8bffff : fc880000.usb usb@fc880000
> > > fc8c0000-fc8fffff : fc8c0000.usb usb@fc8c0000
> > > fc900000-fc900dff : fc900000.iommu
> > > fc910000-fc910dff : fc900000.iommu
> > > fd600000-fd6fffff : fd600000.sram sram@fd600000
> > > fd8a0000-fd8a00ff : fd8a0000.gpio gpio@fd8a0000
> > > fdb50000-fdb507ff : fdb50000.video-codec video-codec@fdb50000
> > > fdb50800-fdb5083f : fdb50800.iommu iommu@fdb50800
> > > fdb80000-fdb8017f : fdb80000.rga rga@fdb80000
> > > fdba0000-fdba07ff : fdba0000.video-codec video-codec@fdba0000
> > > fdba0800-fdba083f : fdba0800.iommu iommu@fdba0800
> > > fdba4800-fdba483f : fdba4800.iommu iommu@fdba4800
> > > fdba8800-fdba883f : fdba8800.iommu iommu@fdba8800
> > > fdbac800-fdbac83f : fdbac800.iommu iommu@fdbac800
> > > fdc70000-fdc707ff : fdc70000.video-codec video-codec@fdc70000
> > > fdd90000-fdd941ff : fdd90000.vop vop
> > > fdd95000-fdd95fff : fdd90000.vop gamma-lut
> > > fdd97e00-fdd97eff : fdd97e00.iommu iommu@fdd97e00
> > > fdd97f00-fdd97fff : fdd97e00.iommu iommu@fdd97e00
> > > fddf0000-fddf0fff : fddf0000.i2s i2s@fddf0000
> > > fddf4000-fddf4fff : fddf4000.i2s i2s@fddf4000
> > > fde80000-fde9ffff : fde80000.hdmi hdmi@fde80000
> > > fdea0000-fdebffff : fdea0000.hdmi hdmi@fdea0000
> > > fdee0000-fdee5fff : fdee0000.hdmi_receiver hdmi_receiver@fdee0000
> > > fe060000-fe06ffff : fe060000.dfi dfi@fe060000
> > > fe150000-fe15ffff : a40000000.pcie apb
> > > fe170000-fe17ffff : a40800000.pcie apb
> > > fe180000-fe18ffff : a40c00000.pcie apb
> > > fe1b0000-fe1bffff : fe1b0000.ethernet ethernet@fe1b0000
> > > fe210000-fe210fff : fe210000.sata sata@fe210000
> > > fe2c0000-fe2c3fff : fe2c0000.mmc mmc@fe2c0000
> > > fe2e0000-fe2effff : fe2e0000.mmc mmc@fe2e0000
> > > fe470000-fe470fff : fe470000.i2s i2s@fe470000
> > > fe600000-fe60ffff : GICD
> > > fe680000-fe77ffff : GICR
> > > fea10000-fea13fff : dma-controller@fea10000
> > > fea10000-fea13fff : fea10000.dma-controller dma-controller@fea10000
> > > fea30000-fea33fff : dma-controller@fea30000
> > > fea30000-fea33fff : fea30000.dma-controller dma-controller@fea30000
> > > feaa0000-feaa0fff : feaa0000.i2c i2c@feaa0000
> > > feaf0000-feaf00ff : feaf0000.watchdog watchdog@feaf0000
> > > feb20000-feb20fff : feb20000.spi spi@feb20000
> > > feb50000-feb500ff : serial
> > > fec00000-fec003ff : fec00000.tsadc tsadc@fec00000
> > > fec10000-fec1ffff : fec10000.adc adc@fec10000
> > > fec20000-fec200ff : fec20000.gpio gpio@fec20000
> > > fec30000-fec300ff : fec30000.gpio gpio@fec30000
> > > fec40000-fec400ff : fec40000.gpio gpio@fec40000
> > > fec50000-fec500ff : fec50000.gpio gpio@fec50000
> > > fec90000-fec90fff : fec90000.i2c i2c@fec90000
> > > fed10000-fed13fff : dma-controller@fed10000
> > > fed10000-fed13fff : fed10000.dma-controller dma-controller@fed10000
> > > fed60000-fed61fff : fed60000.phy phy@fed60000
> > > fed70000-fed71fff : fed70000.phy phy@fed70000
> > > fed80000-fed8ffff : fed80000.phy phy@fed80000
> > > fed90000-fed9ffff : fed90000.phy phy@fed90000
> > > fee00000-fee000ff : fee00000.phy phy@fee00000
> > > fee10000-fee100ff : fee10000.phy phy@fee10000
> > > fee20000-fee200ff : fee20000.phy phy@fee20000
> > > fee80000-fee9ffff : fee80000.phy phy@fee80000
> > > ff001000-ff0effff : ff001000.sram sram@ff001000
> > > 100000000-3fbffffff : System RAM
> > > 3ec000000-3fbffffff : reserved
> > > 3fc500000-3ffefffff : System RAM
> > > 4f0000000-4ffffffff : System RAM
> > > 4fc611000-4fc6d0fff : reserved
> > > 4fc6d1000-4fded1fff : reserved
> > > 4fded2000-4fdf91fff : reserved
> > > 4fdf93000-4fdf96fff : reserved
> > > 4fdf97000-4fdfabfff : reserved
> > > 4fdfac000-4fe051fff : reserved
> > > 4fe052000-4ffffffff : reserved
> > > 900000000-93fffffff : pcie@fe150000
> > > 900000000-93fffffff : 0000:00:00.0
> > > 980000000-9bfffffff : pcie@fe170000
> > > 9c0000000-9ffffffff : pcie@fe180000
> > > a40000000-a403fffff : a40000000.pcie dbi
> > > a40800000-a40bfffff : a40800000.pcie dbi
> > > a40c00000-a40ffffff : a40c00000.pcie dbi
> > >
> > > Thank you,
> > > o.
> > >
> > > > --
> > > > i.
> > >
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 24/25] PCI: Perform reset_resource() and build fail list in sync
2025-04-14 13:15 ` Ilpo Järvinen
@ 2025-04-14 13:43 ` Ondřej Jirman
2025-04-14 13:52 ` Ilpo Järvinen
0 siblings, 1 reply; 43+ messages in thread
From: Ondřej Jirman @ 2025-04-14 13:43 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Guenter Roeck, Bjorn Helgaas, linux-pci, Michał Winiarski,
Igor Mammedov, LKML, Mika Westerberg
On Mon, Apr 14, 2025 at 04:15:01PM +0300, Ilpo Järvinen wrote:
> On Mon, 14 Apr 2025, Ondřej Jirman wrote:
> > On Mon, Apr 14, 2025 at 12:52:15PM +0300, Ilpo Järvinen wrote:
> > > On Fri, 11 Apr 2025, Ondřej Jirman wrote:
> > >
> > > > Hello Ilpo,
> > > >
> > > > On Tue, Apr 01, 2025 at 08:38:48PM +0300, Ilpo Järvinen wrote:
> > > > > That log wasn't taken from a bad case but it doesn't matter anymore as I
> > > > > could test this with qemu myself, thanks for providing enough to make it
> > > > > easy to reproduce & test it locally :-).
> > > > >
> > > > > The problem is caused by assign+release cycle being destructive on start
> > > > > aligned resources because successful assigment overwrites the start field.
> > > > > I'll send a patch to fix the problem once I've given it a bit more thought
> > > > > as this resource fitting code is somewhat complex.
> > > >
> > > > BTW, same thing here on a different SoC:
> > > >
> > > > https://lore.kernel.org/lkml/hrcsm2bo4ysqj2ggejndlou32cdc7yiknnm5nrlcoz4d64wall@7te4dfqsoe3y/T/#u
> > > >
> > > > There are kernel logs there, too, although I don't have dynamic debug enabled
> > > > in the kernel.
> > > >
> > > > Interestingly, bisect pointed me initially to a different commit. Reverting
> > > > it helped, but just on one board (QuartzPro64).
> > >
> > > Hi,
> > >
> > > Since you didn't mention it, I guess you haven't tried the fix:
> > >
> > > https://patchwork.kernel.org/project/linux-pci/patch/20250403093137.1481-1-ilpo.jarvinen@linux.intel.com/
> >
> > This patch works. Thank you.
>
> Thanks for testing. If you feel confident enough, please send your
> Tested-by tag to its thread (not this one).
>
> > One difference compared to 6.14 that I'm noticing
> > in the kernel log is that "save config" sequences now are printed twice for
> > unpopulated port. Not sure if it's related to your patches. Previously it was
> > printed just once.
>
> I don't think it is related. The resource fitting/assignment changes
> should not impact PCI save/restore AFAIK (except by preventing device from
> working in the first place if necessary resources do not get assigned).
>
> PCI state saving has always seemed like that in most logs I've seen with
> dyndbg enabled, that is, the state is seemingly saved multiple times,
> often right after the previous save too. So TBH, I'm not convinced it's
> even something new. If you have backtraces to show those places that
> invoke pci_save_state() I can take a look but I don't expect much to
> come out of that.
You're right. It's unrelated. The traces:
[ 1.878732] show_stack+0x28/0x80 (C)
[ 1.878751] dump_stack_lvl+0x58/0x74
[ 1.878762] dump_stack+0x14/0x1c
[ 1.878772] pci_save_state+0x34/0x1e8
[ 1.878783] pcie_portdrv_probe+0x330/0x6a8
[ 1.878794] local_pci_probe+0x3c/0xa0
[ 1.878804] pci_device_probe+0xac/0x194
[ 1.878813] really_probe+0xbc/0x388
[ 1.878825] __driver_probe_device+0x78/0x144
[ 1.878837] driver_probe_device+0x38/0x110
[ 1.878850] __device_attach_driver+0xb0/0x150
[ 1.878862] bus_for_each_drv+0x6c/0xb0
[ 1.878873] __device_attach+0x98/0x1a0
[ 1.878886] device_attach+0x10/0x20
[ 1.878897] pci_bus_add_device+0x84/0x12c
[ 1.878911] pci_bus_add_devices+0x40/0x90
[ 1.878924] pci_host_probe+0x88/0xe4
[ 1.878933] dw_pcie_host_init+0x258/0x714
[ 1.878945] rockchip_pcie_probe+0x2f0/0x3f8
[ 1.878956] platform_probe+0x64/0xcc
[ 1.878965] really_probe+0xbc/0x388
[ 1.878977] __driver_probe_device+0x78/0x144
[ 1.878989] driver_probe_device+0x38/0x110
[ 1.879001] __device_attach_driver+0xb0/0x150
[ 1.879014] bus_for_each_drv+0x6c/0xb0
[ 1.879025] __device_attach+0x98/0x1a0
[ 1.879037] device_initial_probe+0x10/0x20
[ 1.879049] bus_probe_device+0xa8/0xb8
[ 1.879061] deferred_probe_work_func+0xa4/0xf0
[ 1.879072] process_one_work+0x13c/0x2bc
[ 1.879084] worker_thread+0x284/0x460
[ 1.879094] kthread+0xe4/0x1a0
[ 1.879107] ret_from_fork+0x10/0x20
[ 1.879121] pcieport 0002:20:00.0: save config 0x00: 0x35881d87
[ 1.879158] pcieport 0002:20:00.0: save config 0x04: 0x00100507
[ 1.879168] pcieport 0002:20:00.0: save config 0x08: 0x06040001
[ 1.879177] pcieport 0002:20:00.0: save config 0x0c: 0x00010000
[ 1.879221] pcieport 0002:20:00.0: save config 0x10: 0x00000000
[ 1.879232] pcieport 0002:20:00.0: save config 0x14: 0x00000000
[ 1.879242] pcieport 0002:20:00.0: save config 0x18: 0x00212120
[ 1.879251] pcieport 0002:20:00.0: save config 0x1c: 0x000000f0
[ 1.879260] pcieport 0002:20:00.0: save config 0x20: 0x0000fff0
[ 1.879270] pcieport 0002:20:00.0: save config 0x24: 0x0001fff1
[ 1.879279] pcieport 0002:20:00.0: save config 0x28: 0x00000000
[ 1.879289] pcieport 0002:20:00.0: save config 0x2c: 0x00000000
[ 1.879298] pcieport 0002:20:00.0: save config 0x30: 0x00000000
[ 1.879307] pcieport 0002:20:00.0: save config 0x34: 0x00000040
[ 1.879316] pcieport 0002:20:00.0: save config 0x38: 0x00000000
[ 1.879325] pcieport 0002:20:00.0: save config 0x3c: 0x00020194
[ 1.879376] pcieport 0002:20:00.0: vgaarb: pci_notify
second time it's from:
[ 2.004478] Workqueue: pm pm_runtime_work
[ 2.004488] Call trace:
[ 2.004491] show_stack+0x28/0x80 (C)
[ 2.004500] dump_stack_lvl+0x58/0x74
[ 2.004506] dump_stack+0x14/0x1c
[ 2.004511] pci_save_state+0x34/0x1e8
[ 2.004516] pci_pm_runtime_suspend+0xa8/0x1e0
[ 2.004521] __rpm_callback+0x3c/0x220
[ 2.004527] rpm_callback+0x6c/0x80
[ 2.004532] rpm_suspend+0xec/0x674
[ 2.004537] pm_runtime_work+0x104/0x114
[ 2.004542] process_one_work+0x13c/0x2bc
[ 2.004548] worker_thread+0x284/0x460
[ 2.004552] kthread+0xe4/0x1a0
[ 2.004559] ret_from_fork+0x10/0x20
[ 2.004567] pcieport 0002:20:00.0: save config 0x00: 0x35881d87
[ 2.004578] pcieport 0002:20:00.0: save config 0x04: 0x00100507
[ 2.004583] pcieport 0002:20:00.0: save config 0x08: 0x06040001
[ 2.004587] pcieport 0002:20:00.0: save config 0x0c: 0x00010000
[ 2.004591] pcieport 0002:20:00.0: save config 0x10: 0x00000000
[ 2.004596] pcieport 0002:20:00.0: save config 0x14: 0x00000000
[ 2.004600] pcieport 0002:20:00.0: save config 0x18: 0x00212120
[ 2.004604] pcieport 0002:20:00.0: save config 0x1c: 0x000000f0
[ 2.004608] pcieport 0002:20:00.0: save config 0x20: 0x0000fff0
[ 2.004613] pcieport 0002:20:00.0: save config 0x24: 0x0001fff1
[ 2.004617] pcieport 0002:20:00.0: save config 0x28: 0x00000000
[ 2.004621] pcieport 0002:20:00.0: save config 0x2c: 0x00000000
[ 2.004625] pcieport 0002:20:00.0: save config 0x30: 0x00000000
[ 2.004629] pcieport 0002:20:00.0: save config 0x34: 0x00000040
[ 2.004633] pcieport 0002:20:00.0: save config 0x38: 0x00000000
[ 2.004638] pcieport 0002:20:00.0: save config 0x3c: 0x00020194
[ 2.004662] pcieport 0002:20:00.0: PME# enabled
on 6.15-rc:
cat /sys/class/pci_bus/0002:20/device/0002:20:00.0/power/runtime_status OK
suspended
on 6.14:
cat /sys/class/pci_bus/0002:20/device/0002:20:00.0/power/runtime_status OK
active
So some other unrelated change in 6.15-rc just enabled RPM to suspend the unused
port.
Thnak you,
o.
> --
> i.
>
> > Kind regards,
> > o.
> >
> > rockchip-dw-pcie a40800000.pcie: PCI host bridge to bus 0002:20
> > pci_bus 0002:20: root bus resource [bus 20-2f]
> > pci_bus 0002:20: root bus resource [io 0x300000-0x3fffff] (bus address [0xf2100000-0xf21fffff])
> > pci_bus 0002:20: root bus resource [mem 0xf2200000-0xf2ffffff]
> > pci_bus 0002:20: root bus resource [mem 0x980000000-0x9bfffffff] (bus address [0x40000000-0x7fffffff])
> > pci_bus 0002:20: scanning bus
> > pci 0002:20:00.0: [1d87:3588] type 01 class 0x060400 PCIe Root Port
> > pci 0002:20:00.0: ROM [mem 0x00000000-0x0000ffff pref]
> > pci 0002:20:00.0: PCI bridge to [bus 01-ff]
> > pci 0002:20:00.0: bridge window [io 0x0000-0x0fff]
> > pci 0002:20:00.0: bridge window [mem 0x00000000-0x000fffff]
> > pci 0002:20:00.0: bridge window [mem 0x00000000-0x000fffff 64bit pref]
> > pci 0002:20:00.0: supports D1 D2
> > pci 0002:20:00.0: PME# supported from D0 D1 D3hot
> > pci 0002:20:00.0: PME# disabled
> > pci 0002:20:00.0: Adding to iommu group 9
> > pci 0002:20:00.0: vgaarb: pci_notify
> > pci_bus 0002:20: fixups for bus
> > pci 0002:20:00.0: scanning [bus 01-ff] behind bridge, pass 0
> > pci 0002:20:00.0: Primary bus is hard wired to 0
> > pci 0002:20:00.0: bridge configuration invalid ([bus 01-ff]), reconfiguring
> > pci 0002:20:00.0: scanning [bus 00-00] behind bridge, pass 1
> > pci_bus 0002:20: bus scan returning with max=21
> > pci 0002:20:00.0: ROM [mem 0xf2200000-0xf220ffff pref]: assigned
> > pci 0002:20:00.0: PCI bridge to [bus 21]
> > pci_bus 0002:20: resource 4 [io 0x300000-0x3fffff]
> > pci_bus 0002:20: resource 5 [mem 0xf2200000-0xf2ffffff]
> > pci_bus 0002:20: resource 6 [mem 0x980000000-0x9bfffffff]
> > pcieport 0002:20:00.0: vgaarb: pci_notify
> > pcieport 0002:20:00.0: assign IRQ: got 148
> > pcieport 0002:20:00.0: PME: Signaling with IRQ 157
> > pcieport 0002:20:00.0: AER: enabled with IRQ 158
> > pcieport 0002:20:00.0: bwctrl: enabled with IRQ 157
> > pcieport 0002:20:00.0: save config 0x00: 0x35881d87
> > pcieport 0002:20:00.0: save config 0x04: 0x00100507
> > pcieport 0002:20:00.0: save config 0x08: 0x06040001
> > pcieport 0002:20:00.0: save config 0x0c: 0x00010000
> > pcieport 0002:20:00.0: save config 0x10: 0x00000000
> > pcieport 0002:20:00.0: save config 0x14: 0x00000000
> > pcieport 0002:20:00.0: save config 0x18: 0x00212120
> > pcieport 0002:20:00.0: save config 0x1c: 0x000000f0
> > pcieport 0002:20:00.0: save config 0x20: 0x0000fff0
> > pcieport 0002:20:00.0: save config 0x24: 0x0001fff1
> > pcieport 0002:20:00.0: save config 0x28: 0x00000000
> > pcieport 0002:20:00.0: save config 0x2c: 0x00000000
> > pcieport 0002:20:00.0: save config 0x30: 0x00000000
> > pcieport 0002:20:00.0: save config 0x34: 0x00000040
> > pcieport 0002:20:00.0: save config 0x38: 0x00000000
> > pcieport 0002:20:00.0: save config 0x3c: 0x00020194
> > pcieport 0002:20:00.0: vgaarb: pci_notify
> > pcieport 0002:20:00.0: save config 0x00: 0x35881d87
> > pcieport 0002:20:00.0: save config 0x04: 0x00100507
> > pcieport 0002:20:00.0: save config 0x08: 0x06040001
> > pcieport 0002:20:00.0: save config 0x0c: 0x00010000
> > pcieport 0002:20:00.0: save config 0x10: 0x00000000
> > pcieport 0002:20:00.0: save config 0x14: 0x00000000
> > pcieport 0002:20:00.0: save config 0x18: 0x00212120
> > pcieport 0002:20:00.0: save config 0x1c: 0x000000f0
> > pcieport 0002:20:00.0: save config 0x20: 0x0000fff0
> > pcieport 0002:20:00.0: save config 0x24: 0x0001fff1
> > pcieport 0002:20:00.0: save config 0x28: 0x00000000
> > pcieport 0002:20:00.0: save config 0x2c: 0x00000000
> > pcieport 0002:20:00.0: save config 0x30: 0x00000000
> > pcieport 0002:20:00.0: save config 0x34: 0x00000040
> > pcieport 0002:20:00.0: save config 0x38: 0x00000000
> > pcieport 0002:20:00.0: save config 0x3c: 0x00020194
> > pcieport 0002:20:00.0: PME# enabled
> >
> >
> > > --
> > > i.
> > >
> > > > And this is iomem:
> > > >
> > > > 0010f000-0010f0ff : 10f000.sram sram@10f000
> > > > 00200000-e2bbffff : System RAM
> > > > 02010000-0474ffff : Kernel code
> > > > 04750000-0498ffff : reserved
> > > > 04990000-0508ffff : Kernel data
> > > > daa00000-e29fffff : reserved
> > > > e2bc0000-ecbbffff : reserved
> > > > e2bc0000-ecbbffff : reserved
> > > > ecbc0000-efffffff : System RAM
> > > > ecbc7000-ecbdffff : reserved
> > > > f0000000-f00fffff : a40000000.pcie config
> > > > f0200000-f0ffffff : pcie@fe150000
> > > > f0200000-f020ffff : 0000:00:00.0
> > > > f0300000-f03fffff : PCI Bus 0000:01
> > > > f0300000-f0303fff : 0000:01:00.0
> > > > f0300000-f0303fff : nvme
> > > > f0304000-f03040ff : 0000:01:00.0
> > > > f0304000-f03040ff : nvme
> > > > f2000000-f20fffff : a40800000.pcie config
> > > > f2200000-f2ffffff : pcie@fe170000
> > > > f2200000-f27fffff : PCI Bus 0002:21
> > > > f2200000-f220ffff : 0002:21:00.0
> > > > f2400000-f27fffff : 0002:21:00.0
> > > > f2800000-f280ffff : 0002:20:00.0
> > > > f3000000-f30fffff : a40c00000.pcie config
> > > > f3200000-f3ffffff : pcie@fe180000
> > > > f3200000-f320ffff : 0003:30:00.0
> > > > f3300000-f33fffff : PCI Bus 0003:31
> > > > f3300000-f3303fff : 0003:31:00.0
> > > > f3304000-f3304fff : 0003:31:00.0
> > > > f3304000-f3304fff : r8169
> > > > fb000000-fb1fffff : fb000000.gpu gpu@fb000000
> > > > fc00c100-fc3fffff : fc000000.usb usb@fc000000
> > > > fc400000-fc407fff : usb@fc400000
> > > > fc400000-fc407fff : xhci-hcd.10.auto usb@fc400000
> > > > fc40c100-fc7fffff : fc400000.usb usb@fc400000
> > > > fc800000-fc83ffff : fc800000.usb usb@fc800000
> > > > fc840000-fc87ffff : fc840000.usb usb@fc840000
> > > > fc880000-fc8bffff : fc880000.usb usb@fc880000
> > > > fc8c0000-fc8fffff : fc8c0000.usb usb@fc8c0000
> > > > fc900000-fc900dff : fc900000.iommu
> > > > fc910000-fc910dff : fc900000.iommu
> > > > fd600000-fd6fffff : fd600000.sram sram@fd600000
> > > > fd8a0000-fd8a00ff : fd8a0000.gpio gpio@fd8a0000
> > > > fdb50000-fdb507ff : fdb50000.video-codec video-codec@fdb50000
> > > > fdb50800-fdb5083f : fdb50800.iommu iommu@fdb50800
> > > > fdb80000-fdb8017f : fdb80000.rga rga@fdb80000
> > > > fdba0000-fdba07ff : fdba0000.video-codec video-codec@fdba0000
> > > > fdba0800-fdba083f : fdba0800.iommu iommu@fdba0800
> > > > fdba4800-fdba483f : fdba4800.iommu iommu@fdba4800
> > > > fdba8800-fdba883f : fdba8800.iommu iommu@fdba8800
> > > > fdbac800-fdbac83f : fdbac800.iommu iommu@fdbac800
> > > > fdc70000-fdc707ff : fdc70000.video-codec video-codec@fdc70000
> > > > fdd90000-fdd941ff : fdd90000.vop vop
> > > > fdd95000-fdd95fff : fdd90000.vop gamma-lut
> > > > fdd97e00-fdd97eff : fdd97e00.iommu iommu@fdd97e00
> > > > fdd97f00-fdd97fff : fdd97e00.iommu iommu@fdd97e00
> > > > fddf0000-fddf0fff : fddf0000.i2s i2s@fddf0000
> > > > fddf4000-fddf4fff : fddf4000.i2s i2s@fddf4000
> > > > fde80000-fde9ffff : fde80000.hdmi hdmi@fde80000
> > > > fdea0000-fdebffff : fdea0000.hdmi hdmi@fdea0000
> > > > fdee0000-fdee5fff : fdee0000.hdmi_receiver hdmi_receiver@fdee0000
> > > > fe060000-fe06ffff : fe060000.dfi dfi@fe060000
> > > > fe150000-fe15ffff : a40000000.pcie apb
> > > > fe170000-fe17ffff : a40800000.pcie apb
> > > > fe180000-fe18ffff : a40c00000.pcie apb
> > > > fe1b0000-fe1bffff : fe1b0000.ethernet ethernet@fe1b0000
> > > > fe210000-fe210fff : fe210000.sata sata@fe210000
> > > > fe2c0000-fe2c3fff : fe2c0000.mmc mmc@fe2c0000
> > > > fe2e0000-fe2effff : fe2e0000.mmc mmc@fe2e0000
> > > > fe470000-fe470fff : fe470000.i2s i2s@fe470000
> > > > fe600000-fe60ffff : GICD
> > > > fe680000-fe77ffff : GICR
> > > > fea10000-fea13fff : dma-controller@fea10000
> > > > fea10000-fea13fff : fea10000.dma-controller dma-controller@fea10000
> > > > fea30000-fea33fff : dma-controller@fea30000
> > > > fea30000-fea33fff : fea30000.dma-controller dma-controller@fea30000
> > > > feaa0000-feaa0fff : feaa0000.i2c i2c@feaa0000
> > > > feaf0000-feaf00ff : feaf0000.watchdog watchdog@feaf0000
> > > > feb20000-feb20fff : feb20000.spi spi@feb20000
> > > > feb50000-feb500ff : serial
> > > > fec00000-fec003ff : fec00000.tsadc tsadc@fec00000
> > > > fec10000-fec1ffff : fec10000.adc adc@fec10000
> > > > fec20000-fec200ff : fec20000.gpio gpio@fec20000
> > > > fec30000-fec300ff : fec30000.gpio gpio@fec30000
> > > > fec40000-fec400ff : fec40000.gpio gpio@fec40000
> > > > fec50000-fec500ff : fec50000.gpio gpio@fec50000
> > > > fec90000-fec90fff : fec90000.i2c i2c@fec90000
> > > > fed10000-fed13fff : dma-controller@fed10000
> > > > fed10000-fed13fff : fed10000.dma-controller dma-controller@fed10000
> > > > fed60000-fed61fff : fed60000.phy phy@fed60000
> > > > fed70000-fed71fff : fed70000.phy phy@fed70000
> > > > fed80000-fed8ffff : fed80000.phy phy@fed80000
> > > > fed90000-fed9ffff : fed90000.phy phy@fed90000
> > > > fee00000-fee000ff : fee00000.phy phy@fee00000
> > > > fee10000-fee100ff : fee10000.phy phy@fee10000
> > > > fee20000-fee200ff : fee20000.phy phy@fee20000
> > > > fee80000-fee9ffff : fee80000.phy phy@fee80000
> > > > ff001000-ff0effff : ff001000.sram sram@ff001000
> > > > 100000000-3fbffffff : System RAM
> > > > 3ec000000-3fbffffff : reserved
> > > > 3fc500000-3ffefffff : System RAM
> > > > 4f0000000-4ffffffff : System RAM
> > > > 4fc611000-4fc6d0fff : reserved
> > > > 4fc6d1000-4fded1fff : reserved
> > > > 4fded2000-4fdf91fff : reserved
> > > > 4fdf93000-4fdf96fff : reserved
> > > > 4fdf97000-4fdfabfff : reserved
> > > > 4fdfac000-4fe051fff : reserved
> > > > 4fe052000-4ffffffff : reserved
> > > > 900000000-93fffffff : pcie@fe150000
> > > > 900000000-93fffffff : 0000:00:00.0
> > > > 980000000-9bfffffff : pcie@fe170000
> > > > 9c0000000-9ffffffff : pcie@fe180000
> > > > a40000000-a403fffff : a40000000.pcie dbi
> > > > a40800000-a40bfffff : a40800000.pcie dbi
> > > > a40c00000-a40ffffff : a40c00000.pcie dbi
> > > >
> > > > Thank you,
> > > > o.
> > > >
> > > > > --
> > > > > i.
> > > >
> >
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 24/25] PCI: Perform reset_resource() and build fail list in sync
2025-04-14 13:43 ` Ondřej Jirman
@ 2025-04-14 13:52 ` Ilpo Järvinen
0 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2025-04-14 13:52 UTC (permalink / raw)
To: Ondřej Jirman
Cc: Guenter Roeck, Bjorn Helgaas, linux-pci, Michał Winiarski,
Igor Mammedov, LKML, Mika Westerberg
[-- Attachment #1: Type: text/plain, Size: 8202 bytes --]
On Mon, 14 Apr 2025, Ondřej Jirman wrote:
> On Mon, Apr 14, 2025 at 04:15:01PM +0300, Ilpo Järvinen wrote:
> > On Mon, 14 Apr 2025, Ondřej Jirman wrote:
> > > On Mon, Apr 14, 2025 at 12:52:15PM +0300, Ilpo Järvinen wrote:
> > > > On Fri, 11 Apr 2025, Ondřej Jirman wrote:
> > > >
> > > > > Hello Ilpo,
> > > > >
> > > > > On Tue, Apr 01, 2025 at 08:38:48PM +0300, Ilpo Järvinen wrote:
> > > > > > That log wasn't taken from a bad case but it doesn't matter anymore as I
> > > > > > could test this with qemu myself, thanks for providing enough to make it
> > > > > > easy to reproduce & test it locally :-).
> > > > > >
> > > > > > The problem is caused by assign+release cycle being destructive on start
> > > > > > aligned resources because successful assigment overwrites the start field.
> > > > > > I'll send a patch to fix the problem once I've given it a bit more thought
> > > > > > as this resource fitting code is somewhat complex.
> > > > >
> > > > > BTW, same thing here on a different SoC:
> > > > >
> > > > > https://lore.kernel.org/lkml/hrcsm2bo4ysqj2ggejndlou32cdc7yiknnm5nrlcoz4d64wall@7te4dfqsoe3y/T/#u
> > > > >
> > > > > There are kernel logs there, too, although I don't have dynamic debug enabled
> > > > > in the kernel.
> > > > >
> > > > > Interestingly, bisect pointed me initially to a different commit. Reverting
> > > > > it helped, but just on one board (QuartzPro64).
> > > >
> > > > Hi,
> > > >
> > > > Since you didn't mention it, I guess you haven't tried the fix:
> > > >
> > > > https://patchwork.kernel.org/project/linux-pci/patch/20250403093137.1481-1-ilpo.jarvinen@linux.intel.com/
> > >
> > > This patch works. Thank you.
> >
> > Thanks for testing. If you feel confident enough, please send your
> > Tested-by tag to its thread (not this one).
> >
> > > One difference compared to 6.14 that I'm noticing
> > > in the kernel log is that "save config" sequences now are printed twice for
> > > unpopulated port. Not sure if it's related to your patches. Previously it was
> > > printed just once.
> >
> > I don't think it is related. The resource fitting/assignment changes
> > should not impact PCI save/restore AFAIK (except by preventing device from
> > working in the first place if necessary resources do not get assigned).
> >
> > PCI state saving has always seemed like that in most logs I've seen with
> > dyndbg enabled, that is, the state is seemingly saved multiple times,
> > often right after the previous save too. So TBH, I'm not convinced it's
> > even something new. If you have backtraces to show those places that
> > invoke pci_save_state() I can take a look but I don't expect much to
> > come out of that.
>
> You're right. It's unrelated. The traces:
>
> [ 1.878732] show_stack+0x28/0x80 (C)
> [ 1.878751] dump_stack_lvl+0x58/0x74
> [ 1.878762] dump_stack+0x14/0x1c
> [ 1.878772] pci_save_state+0x34/0x1e8
> [ 1.878783] pcie_portdrv_probe+0x330/0x6a8
> [ 1.878794] local_pci_probe+0x3c/0xa0
> [ 1.878804] pci_device_probe+0xac/0x194
> [ 1.878813] really_probe+0xbc/0x388
> [ 1.878825] __driver_probe_device+0x78/0x144
> [ 1.878837] driver_probe_device+0x38/0x110
> [ 1.878850] __device_attach_driver+0xb0/0x150
> [ 1.878862] bus_for_each_drv+0x6c/0xb0
> [ 1.878873] __device_attach+0x98/0x1a0
> [ 1.878886] device_attach+0x10/0x20
> [ 1.878897] pci_bus_add_device+0x84/0x12c
> [ 1.878911] pci_bus_add_devices+0x40/0x90
> [ 1.878924] pci_host_probe+0x88/0xe4
> [ 1.878933] dw_pcie_host_init+0x258/0x714
> [ 1.878945] rockchip_pcie_probe+0x2f0/0x3f8
> [ 1.878956] platform_probe+0x64/0xcc
> [ 1.878965] really_probe+0xbc/0x388
> [ 1.878977] __driver_probe_device+0x78/0x144
> [ 1.878989] driver_probe_device+0x38/0x110
> [ 1.879001] __device_attach_driver+0xb0/0x150
> [ 1.879014] bus_for_each_drv+0x6c/0xb0
> [ 1.879025] __device_attach+0x98/0x1a0
> [ 1.879037] device_initial_probe+0x10/0x20
> [ 1.879049] bus_probe_device+0xa8/0xb8
> [ 1.879061] deferred_probe_work_func+0xa4/0xf0
> [ 1.879072] process_one_work+0x13c/0x2bc
> [ 1.879084] worker_thread+0x284/0x460
> [ 1.879094] kthread+0xe4/0x1a0
> [ 1.879107] ret_from_fork+0x10/0x20
> [ 1.879121] pcieport 0002:20:00.0: save config 0x00: 0x35881d87
> [ 1.879158] pcieport 0002:20:00.0: save config 0x04: 0x00100507
> [ 1.879168] pcieport 0002:20:00.0: save config 0x08: 0x06040001
> [ 1.879177] pcieport 0002:20:00.0: save config 0x0c: 0x00010000
> [ 1.879221] pcieport 0002:20:00.0: save config 0x10: 0x00000000
> [ 1.879232] pcieport 0002:20:00.0: save config 0x14: 0x00000000
> [ 1.879242] pcieport 0002:20:00.0: save config 0x18: 0x00212120
> [ 1.879251] pcieport 0002:20:00.0: save config 0x1c: 0x000000f0
> [ 1.879260] pcieport 0002:20:00.0: save config 0x20: 0x0000fff0
> [ 1.879270] pcieport 0002:20:00.0: save config 0x24: 0x0001fff1
> [ 1.879279] pcieport 0002:20:00.0: save config 0x28: 0x00000000
> [ 1.879289] pcieport 0002:20:00.0: save config 0x2c: 0x00000000
> [ 1.879298] pcieport 0002:20:00.0: save config 0x30: 0x00000000
> [ 1.879307] pcieport 0002:20:00.0: save config 0x34: 0x00000040
> [ 1.879316] pcieport 0002:20:00.0: save config 0x38: 0x00000000
> [ 1.879325] pcieport 0002:20:00.0: save config 0x3c: 0x00020194
> [ 1.879376] pcieport 0002:20:00.0: vgaarb: pci_notify
>
> second time it's from:
>
> [ 2.004478] Workqueue: pm pm_runtime_work
> [ 2.004488] Call trace:
> [ 2.004491] show_stack+0x28/0x80 (C)
> [ 2.004500] dump_stack_lvl+0x58/0x74
> [ 2.004506] dump_stack+0x14/0x1c
> [ 2.004511] pci_save_state+0x34/0x1e8
> [ 2.004516] pci_pm_runtime_suspend+0xa8/0x1e0
> [ 2.004521] __rpm_callback+0x3c/0x220
> [ 2.004527] rpm_callback+0x6c/0x80
> [ 2.004532] rpm_suspend+0xec/0x674
> [ 2.004537] pm_runtime_work+0x104/0x114
> [ 2.004542] process_one_work+0x13c/0x2bc
> [ 2.004548] worker_thread+0x284/0x460
> [ 2.004552] kthread+0xe4/0x1a0
> [ 2.004559] ret_from_fork+0x10/0x20
> [ 2.004567] pcieport 0002:20:00.0: save config 0x00: 0x35881d87
> [ 2.004578] pcieport 0002:20:00.0: save config 0x04: 0x00100507
> [ 2.004583] pcieport 0002:20:00.0: save config 0x08: 0x06040001
> [ 2.004587] pcieport 0002:20:00.0: save config 0x0c: 0x00010000
> [ 2.004591] pcieport 0002:20:00.0: save config 0x10: 0x00000000
> [ 2.004596] pcieport 0002:20:00.0: save config 0x14: 0x00000000
> [ 2.004600] pcieport 0002:20:00.0: save config 0x18: 0x00212120
> [ 2.004604] pcieport 0002:20:00.0: save config 0x1c: 0x000000f0
> [ 2.004608] pcieport 0002:20:00.0: save config 0x20: 0x0000fff0
> [ 2.004613] pcieport 0002:20:00.0: save config 0x24: 0x0001fff1
> [ 2.004617] pcieport 0002:20:00.0: save config 0x28: 0x00000000
> [ 2.004621] pcieport 0002:20:00.0: save config 0x2c: 0x00000000
> [ 2.004625] pcieport 0002:20:00.0: save config 0x30: 0x00000000
> [ 2.004629] pcieport 0002:20:00.0: save config 0x34: 0x00000040
> [ 2.004633] pcieport 0002:20:00.0: save config 0x38: 0x00000000
> [ 2.004638] pcieport 0002:20:00.0: save config 0x3c: 0x00020194
> [ 2.004662] pcieport 0002:20:00.0: PME# enabled
>
> on 6.15-rc:
>
> cat /sys/class/pci_bus/0002:20/device/0002:20:00.0/power/runtime_status OK
> suspended
>
> on 6.14:
>
> cat /sys/class/pci_bus/0002:20/device/0002:20:00.0/power/runtime_status OK
> active
>
> So some other unrelated change in 6.15-rc just enabled RPM to suspend the unused
> port.
Ah right, makes sense, now that you mention I recall D3 became allowed for
PCI bridge on non-x86 archs but I just couldn't make the connection in my
mind.
--
i.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 24/25] PCI: Perform reset_resource() and build fail list in sync
2025-04-01 10:18 ` Ilpo Järvinen
2025-04-01 12:07 ` Ilpo Järvinen
@ 2025-04-01 13:28 ` Guenter Roeck
1 sibling, 0 replies; 43+ messages in thread
From: Guenter Roeck @ 2025-04-01 13:28 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
LKML, Mika Westerberg
On 4/1/25 03:18, Ilpo Järvinen wrote:
> On Mon, 31 Mar 2025, Guenter Roeck wrote:
>> On Mon, Dec 16, 2024 at 07:56:31PM +0200, Ilpo Järvinen wrote:
>>> Resetting resource is problematic as it prevent attempting to allocate
>>> the resource later, unless something in between restores the resource.
>>> Similarly, if fail_head does not contain all resources that were reset,
>>> those resource cannot be restored later.
>>>
>>> The entire reset/restore cycle adds complexity and leaving resources
>>> into reseted state causes issues to other code such as for checks done
>>> in pci_enable_resources(). Take a small step towards not resetting
>>> resources by delaying reset until the end of resource assignment and
>>> build failure list (fail_head) in sync with the reset to avoid leaving
>>> behind resources that cannot be restored (for the case where the caller
>>> provides fail_head in the first place to allow restore somewhere in the
>>> callchain, as is not all callers pass non-NULL fail_head).
>>>
>>> The Expansion ROM check is temporarily left in place while building the
>>> failure list until the upcoming change which reworks optional resource
>>> handling.
>>>
>>> Ideally, whole resource reset could be removed but doing that in a big
>>> step would make the impact non-tractable due to complexity of all
>>> related code.
>>>
>>> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>>
>> With this patch in the mainline kernel, all mips:boston qemu emulations
>> fail when running a 64-bit little endian configuration (64r6el_defconfig).
>>
>> The problem is that the PCI based IDE/ATA controller is not initialized.
>> There are a number of pci error messages.
>>
>> pci_bus 0002:01: extended config space not accessible
>> pci 0002:01:00.0: [8086:2922] type 00 class 0x010601 conventional PCI endpoint
>> pci 0002:01:00.0: BAR 4 [io 0x0000-0x001f]
>> pci 0002:01:00.0: BAR 5 [mem 0x00000000-0x00000fff]
>> pci 0002:00:00.0: PCI bridge to [bus 01-ff]
>> pci_bus 0002:01: busn_res: [bus 01-ff] end is updated to 01
>> pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff]: assigned
>> pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: can't assign; no space
>> pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: failed to assign
>> pci 0002:00:00.0: bridge window [io size 0x1000]: can't assign; no space
>> pci 0002:00:00.0: bridge window [io size 0x1000]: failed to assign
>> pci 0002:00:00.0: bridge window [mem size 0x00100000]: can't assign; bogus alignment
>> pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff 64bit pref]: assigned
>> pci 0002:00:00.0: bridge window [io size 0x1000]: can't assign; no space
>> pci 0002:00:00.0: bridge window [io size 0x1000]: failed to assign
>> pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: can't assign; no space
>> pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: failed to assign
>> pci 0002:01:00.0: BAR 4 [io size 0x0020]: can't assign; no space
>> pci 0002:01:00.0: BAR 4 [io size 0x0020]: failed to assign
>> pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: can't assign; no space
>> pci 0002:01:00.0: BAR 5 [mem size 0x00001000]: failed to assign
>> pci 0002:01:00.0: BAR 4 [io size 0x0020]: can't assign; no space
>> pci 0002:01:00.0: BAR 4 [io size 0x0020]: failed to assign
>> pci 0002:00:00.0: PCI bridge to [bus 01]
>> pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff 64bit pref]
>> pci_bus 0002:00: Some PCI device resources are unassigned, try booting with pci=realloc
>> pci_bus 0002:00: resource 4 [mem 0x16000000-0x160fffff]
>> pci_bus 0002:01: resource 2 [mem 0x16000000-0x160fffff 64bit pref]
>> ...
>> pci 0002:00:00.0: enabling device (0000 -> 0002)
>> ahci 0002:01:00.0: probe with driver ahci failed with error -12
>>
>> Bisect points to this patch. Reverting it together with "PCI: Rework
>> optional resource handling" fixes the problem. For comparison, after
>> reverting the offending patches, the log messages are as follows.
>>
>> pci_bus 0002:00: root bus resource [bus 00-ff]
>> pci_bus 0002:00: root bus resource [mem 0x16000000-0x160fffff]
>> pci 0002:00:00.0: [10ee:7021] type 01 class 0x060400 PCIe Root Complex Integrated Endpoint
>> pci 0002:00:00.0: PCI bridge to [bus 00]
>> pci 0002:00:00.0: bridge window [io 0x0000-0x0fff]
>> pci 0002:00:00.0: bridge window [mem 0x00000000-0x000fffff]
>> pci 0002:00:00.0: bridge window [mem 0x00000000-0x000fffff 64bit pref]
>> pci 0002:00:00.0: enabling Extended Tags
>> pci 0002:00:00.0: bridge configuration invalid ([bus 00-00]), reconfiguring
>> pci_bus 0002:01: extended config space not accessible
>> pci 0002:01:00.0: [8086:2922] type 00 class 0x010601 conventional PCI endpoint
>> pci 0002:01:00.0: BAR 4 [io 0x0000-0x001f]
>> pci 0002:01:00.0: BAR 5 [mem 0x00000000-0x00000fff]
>> pci 0002:00:00.0: PCI bridge to [bus 01-ff]
>> pci_bus 0002:01: busn_res: [bus 01-ff] end is updated to 01
>> pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff]: assigned
>> pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: can't assign; no space
>> pci 0002:00:00.0: bridge window [mem size 0x00100000 64bit pref]: failed to assign
>> pci 0002:00:00.0: bridge window [io size 0x1000]: can't assign; no space
>> pci 0002:00:00.0: bridge window [io size 0x1000]: failed to assign
>> pci 0002:01:00.0: BAR 5 [mem 0x16000000-0x16000fff]: assigned
>> pci 0002:01:00.0: BAR 4 [io size 0x0020]: can't assign; no space
>> pci 0002:01:00.0: BAR 4 [io size 0x0020]: failed to assign
>> pci 0002:00:00.0: PCI bridge to [bus 01]
>> pci 0002:00:00.0: bridge window [mem 0x16000000-0x160fffff]
>> pci_bus 0002:00: Some PCI device resources are unassigned, try booting with pci=realloc
>> pci_bus 0002:00: resource 4 [mem 0x16000000-0x160fffff]
>> pci_bus 0002:01: resource 1 [mem 0x16000000-0x160fffff]
>> ...
>> pci 0002:00:00.0: enabling device (0000 -> 0002)
>> ahci 0002:01:00.0: enabling device (0000 -> 0002)
>> ahci 0002:01:00.0: AHCI vers 0001.0000, 32 command slots, 1.5 Gbps, SATA mode
>> ahci 0002:01:00.0: 6/6 ports implemented (port mask 0x3f)
>> ahci 0002:01:00.0: flags: 64bit ncq only
>
> Hi,
>
> Thanks for reporting. Please add this to the command line to get the
> resource releasing between the steps to show:
>
> dyndbg="file drivers/pci/setup-bus.c +p"
>
> Also, the log snippet just shows it fails but it is impossible to know
> from it why the resource assigments do not fit so could you please provide
> a complete dmesg logs. Also providing the contents of /proc/iomem from the
> working case would save me quite a bit of decoding the iomem layout from
> the dmesgs.
>
You should find everything you need at http://server.roeck-us.net/qemu/mipsel64/.
Please let me know if you need anything else.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 25/25] PCI: Rework optional resource handling
2025-02-14 9:59 ` Xiaochun Lee
` (24 preceding siblings ...)
(?)
@ 2024-12-16 17:56 ` Ilpo Järvinen
-1 siblings, 0 replies; 43+ messages in thread
From: Ilpo Järvinen @ 2024-12-16 17:56 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel
Cc: Mika Westerberg, Ilpo Järvinen, Jia Yao
Remove and rescan cycle can result in not failure to assign a bridge
windows if it becomes larger than before the remove. The bridge window
size will include space for disabled Expansion ROM, which can causes
the bridge window to not fit anymore into the same address space slot
on rescan if the Expansion ROM resource was not assigned before the
remove. In addition, the optional resource handling is not internally
consistent.
The resource fitting logic supports three main types of optional
resources:
- IOV BARs
- Expansion ROMs
- Bridge window size variation due to optional resources
In addition to the above, resizable BARs beyond their current size will
require handling optional variation in resource sizes within the
resource fitting algorithm (not yet done by the resource fitting code).
There are multiple inconsistencies related to optional resource
handling:
a) The allocation failure of disabled expansion ROM requires special
case inside assign_requested_resources_sorted().
b) The optionality of disabled expansion ROM is not considered during
bridge window sizing in pbus_size_mem().
c) Setting resource size to zero for optional resource in
pbus_size_mem() is problematic because it makes also the alignment
invalid, which is checked by pdev_sort_resources().
Optional IOV resources have their size set to zero by
pbus_size_mem() but the information about size is stored externally
into the struct pci_sriov and complex call-chain trickery in
pci_resource_alignment() ensures IOV resources return a valid
alignment despite having zero resource size. A solution that is
specific to IOV resources makes it hard to use the same solution for
other types of resources such as expansion ROM.
Simply changing only pbus_size_mem() is not sufficient to fully address
the main issue because it would introduce disparity between bridge
window sizing and resource allocation. Due size based ordering of
resource list during assignment loop, an Expansion ROM resource could
steal space from some other resource and make the other resource not
fit if the Expansion ROM is larger than the other resource. Thus, the
resource assignment functions need to be changed as well.
Make optional resource handling more straightforwards. Use
pci_resource_is_optional() to determine if a resource is optional or
not in both bridge window sizing and assignment failure classification
to ensure they always align. Indicate with a parameter to
assign_requested_resources_sorted() whether it should attempt to
allocate optional resources or not.
Always try first to assign all resources (also when realloc_head is not
provided). This is required for calls from
pci_assign_unassigned_root_bus_resources() that provides realloc_head
only with some of its iterations.
Non-bridge-window optional resources in realloc_head now have add_size
0. This condition has to be detected in reassign_resources_sorted()
before reassigning them (which would fail as there is no size change).
Removing add_size=0 optional resources entirely from realloc_head might
eventually be doable but further rework in __assign_resources_sorted()
is needed first to support such a change.
Reported-by: Jia Yao <jia.yao@intel.com>
Tested-by: Jia Yao <jia.yao@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/setup-bus.c | 89 +++++++++++++++++++++++++++--------------
1 file changed, 58 insertions(+), 31 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index b61f24a5cfa5..802643a4de47 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -276,13 +276,14 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
add_size = add_res->add_size;
align = add_res->min_align;
if (!res->parent) {
- resource_set_range(res, align, add_size);
+ resource_set_range(res, align,
+ resource_size(res) + add_size);
if (pci_assign_resource(dev, idx)) {
pci_dbg(dev,
"%s %pR: ignoring failure in optional allocation\n",
res_name, res);
}
- } else {
+ } else if (add_size > 0) {
res->flags |= add_res->flags &
(IORESOURCE_STARTALIGN|IORESOURCE_SIZEALIGN);
if (pci_reassign_resource(dev, idx, add_size, align))
@@ -302,38 +303,38 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
* @head: Head of the list tracking requests for resources
* @fail_head: Head of the list tracking requests that could not be
* allocated
+ * @optional: Assign also optional resources
*
* Satisfy resource requests of each element in the list. Add requests that
* could not be satisfied to the failed_list.
*/
static void assign_requested_resources_sorted(struct list_head *head,
- struct list_head *fail_head)
+ struct list_head *fail_head,
+ bool optional)
{
struct pci_dev_resource *dev_res;
struct resource *res;
struct pci_dev *dev;
+ bool optional_res;
int idx;
list_for_each_entry(dev_res, head, list) {
res = dev_res->res;
dev = dev_res->dev;
idx = pci_resource_num(dev, res);
+ optional_res = pci_resource_is_optional(dev, idx);
if (!resource_size(res))
continue;
+ if (!optional && optional_res)
+ continue;
+
if (pci_assign_resource(dev, idx)) {
if (fail_head) {
- /*
- * If the failed resource is a ROM BAR and
- * it will be enabled later, don't add it
- * to the list.
- */
- if (!((idx == PCI_ROM_RESOURCE) &&
- (!(res->flags & IORESOURCE_ROM_ENABLE))))
- add_to_list(fail_head, dev, res,
- 0 /* don't care */,
- 0 /* don't care */);
+ add_to_list(fail_head, dev, res,
+ 0 /* don't care */,
+ 0 /* don't care */);
}
}
}
@@ -379,6 +380,20 @@ static bool pci_need_to_release(unsigned long mask, struct resource *res)
return false; /* Should not get here */
}
+/* Return: @true if assignment of a required resource failed. */
+static bool pci_required_resource_failed(struct list_head *fail_head)
+{
+ struct pci_dev_resource *fail_res;
+
+ list_for_each_entry(fail_res, fail_head, list) {
+ int idx = pci_resource_num(fail_res->dev, fail_res->res);
+
+ if (!pci_resource_is_optional(fail_res->dev, idx))
+ return true;
+ }
+ return false;
+}
+
static void __assign_resources_sorted(struct list_head *head,
struct list_head *realloc_head,
struct list_head *fail_head)
@@ -388,9 +403,11 @@ static void __assign_resources_sorted(struct list_head *head,
* adjacent, so later reassign can not reallocate them one by one in
* parent resource window.
*
- * Try to assign requested + add_size at beginning. If could do that,
- * could get out early. If could not do that, we still try to assign
- * requested at first, then try to reassign add_size for some resources.
+ * Try to assign required and any optional resources at beginning
+ * (add_size included). If all required resources were successfully
+ * assigned, get out early. If could not do that, we still try to
+ * assign required at first, then try to reassign some optional
+ * resources.
*
* Separate three resource type checking if we need to release
* assigned resource after requested + add_size try.
@@ -421,13 +438,13 @@ static void __assign_resources_sorted(struct list_head *head,
/* Check if optional add_size is there */
if (list_empty(realloc_head))
- goto requested_and_reassign;
+ goto assign;
/* Save original start, end, flags etc at first */
list_for_each_entry(dev_res, head, list) {
if (add_to_list(&save_head, dev_res->dev, dev_res->res, 0, 0)) {
free_list(&save_head);
- goto requested_and_reassign;
+ goto assign;
}
}
@@ -471,10 +488,10 @@ static void __assign_resources_sorted(struct list_head *head,
}
- /* Try updated head list with add_size added */
- assign_requested_resources_sorted(head, &local_fail_head);
+assign:
+ assign_requested_resources_sorted(head, &local_fail_head, true);
- /* All assigned with add_size? */
+ /* All non-optional resources assigned? */
if (list_empty(&local_fail_head)) {
/* Remove head list from realloc_head list */
list_for_each_entry(dev_res, head, list)
@@ -483,6 +500,22 @@ static void __assign_resources_sorted(struct list_head *head,
goto out;
}
+ /* Without realloc_head and only optional fails, nothing more to do. */
+ if (!pci_required_resource_failed(&local_fail_head) &&
+ list_empty(realloc_head)) {
+ list_for_each_entry(save_res, &save_head, list) {
+ struct resource *res = save_res->res;
+
+ if (res->parent)
+ continue;
+
+ restore_dev_resource(save_res);
+ }
+ free_list(&local_fail_head);
+ free_list(&save_head);
+ goto out;
+ }
+
/* Check failed type */
fail_type = pci_fail_res_type_mask(&local_fail_head);
/* Remove not need to be released assigned res from head list etc */
@@ -518,9 +551,8 @@ static void __assign_resources_sorted(struct list_head *head,
restore_dev_resource(save_res);
free_list(&save_head);
-requested_and_reassign:
/* Satisfy the must-have resource requests */
- assign_requested_resources_sorted(head, NULL);
+ assign_requested_resources_sorted(head, NULL, false);
/* Try to satisfy any additional optional resource requests */
if (!list_empty(realloc_head))
@@ -535,11 +567,7 @@ static void __assign_resources_sorted(struct list_head *head,
if (res->parent)
continue;
- /*
- * If the failed resource is a ROM BAR and it will
- * be enabled later, don't add it to the list.
- */
- if (fail_head && !pci_resource_is_disabled_rom(res, idx)) {
+ if (fail_head) {
add_to_list(fail_head, dev, res,
0 /* don't care */,
0 /* don't care */);
@@ -1166,10 +1194,9 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
r_size = resource_size(r);
/* Put SRIOV requested res to the optional list */
- if (realloc_head && pci_resource_is_iov(i)) {
+ if (realloc_head && pci_resource_is_optional(dev, i)) {
add_align = max(pci_resource_alignment(dev, r), add_align);
- resource_set_size(r, 0);
- add_to_list(realloc_head, dev, r, r_size, 0 /* Don't care */);
+ add_to_list(realloc_head, dev, r, 0, 0 /* Don't care */);
children_add_size += r_size;
continue;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH 00/25] PCI: Resource fitting/assignment fixes and cleanups
2025-02-14 9:59 ` Xiaochun Lee
` (25 preceding siblings ...)
(?)
@ 2025-02-13 21:46 ` Bjorn Helgaas
-1 siblings, 0 replies; 43+ messages in thread
From: Bjorn Helgaas @ 2025-02-13 21:46 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Bjorn Helgaas, linux-pci, Michał Winiarski, Igor Mammedov,
linux-kernel, Mika Westerberg
On Mon, Dec 16, 2024 at 07:56:07PM +0200, Ilpo Järvinen wrote:
> Hi all,
>
> This series focuses on PCI resource fitting and assignment algorithms.
> I've further changes in works to enable handling resizable BARs better
> during resource fitting built on top of these, but that's still WIP and
> this series seems way too large as is to have more stuff included.
>
> First there are small tweaks and fixes to the relaxed tail alignment
> code and applying the lessons learned to other similar cases. They are
> sort of independent of the rest. Then a large set of pure cleanups and
> refactoring that are not intended to make any functional changes.
> Finally, starting from "PCI: Extend enable to check for any optional
> resource" are again patches that aim to make behavioral changes to fix
> bridge window sizing to consider expansion ROM as an optional resource
> (to fix a remove/rescan cycle issue) and improve resource fitting
> algorithm in general.
>
> The series includes one of the change from Michał Winiarski
> <michal.winiarski@intel.com> as these changes also touch the same IOV
> checks.
>
> Please let me know if you'd prefer me to order the changes differently
> or split it into smaller chunks.
>
>
> I've extensively tested this series over the hosts in our lab which
> have quite heterogeneous PCI setup each. There were no losses of any
> important resource. Without pci=realloc, there's some churn in which of
> the disabled expansion ROMs gets a scarce memory space assigned (with
> pci=realloc, they are all assigned large enough bridge window).
>
>
> Ilpo Järvinen (24):
> PCI: Remove add_align overwrite unrelated to size0
> PCI: size0 is unrelated to add_align
> PCI: Simplify size1 assignment logic
> PCI: Optional bridge window size too may need relaxing
> PCI: Fix old_size lower bound in calculate_iosize() too
> PCI: Use SZ_* instead of literals in setup-bus.c
> PCI: resource_set_range/size() conversions
> PCI: Check resource_size() separately
> PCI: Add pci_resource_num() helper
> PCI: Add dev & res local variables to resource assignment funcs
> PCI: Converge return paths in __assign_resources_sorted()
> PCI: Refactor pdev_sort_resources() & __dev_sort_resources()
> PCI: Use while loop and break instead of gotos
> PCI: Rename retval to ret
> PCI: Consolidate assignment loop next round preparation
> PCI: Remove wrong comment from pci_reassign_resource()
> PCI: Add restore_dev_resource()
> PCI: Extend enable to check for any optional resource
> PCI: Always have realloc_head in __assign_resources_sorted()
> PCI: Indicate optional resource assignment failures
> PCI: Add debug print when releasing resources before retry
> PCI: Use res->parent to check is resource is assigned
> PCI: Perform reset_resource() and build fail list in sync
> PCI: Rework optional resource handling
>
> Michał Winiarski (1):
> PCI: Add a helper to identify IOV resources
>
> drivers/pci/pci.h | 44 +++-
> drivers/pci/setup-bus.c | 566 +++++++++++++++++++++++-----------------
> drivers/pci/setup-res.c | 8 +-
> 3 files changed, 364 insertions(+), 254 deletions(-)
Applied to pci/resource for v6.15, thanks, Ilpo!
^ permalink raw reply [flat|nested] 43+ messages in thread* RE: [PATCH 00/25] PCI: Resource fitting/assignment fixes and cleanups
2025-02-14 9:59 ` Xiaochun Lee
` (26 preceding siblings ...)
(?)
@ 2025-02-14 8:18 ` Xiaochun XC17 Li | 李小春 Xavier
2025-02-14 11:53 ` Ilpo Järvinen
-1 siblings, 1 reply; 43+ messages in thread
From: Xiaochun XC17 Li | 李小春 Xavier @ 2025-02-14 8:18 UTC (permalink / raw)
To: Ilpo Järvinen, Bjorn Helgaas, linux-pci@vger.kernel.org,
Michał Winiarski, Igor Mammedov
Cc: linux-kernel@vger.kernel.org, Mika Westerberg
On Mon, Dec 16, 2024 at 7:56:45PM +0200 Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>
> Hi all,
>
> This series focuses on PCI resource fitting and assignment algorithms.
> I've further changes in works to enable handling resizable BARs better during
> resource fitting built on top of these, but that's still WIP and this series seems
> way too large as is to have more stuff included.
>
> First there are small tweaks and fixes to the relaxed tail alignment code and
> applying the lessons learned to other similar cases. They are sort of
> independent of the rest. Then a large set of pure cleanups and refactoring that
> are not intended to make any functional changes.
> Finally, starting from "PCI: Extend enable to check for any optional resource" are
> again patches that aim to make behavioral changes to fix bridge window sizing
> to consider expansion ROM as an optional resource (to fix a remove/rescan
> cycle issue) and improve resource fitting algorithm in general.
>
> The series includes one of the change from Michał Winiarski
> <michal.winiarski@intel.com> as these changes also touch the same IOV checks.
>
> Please let me know if you'd prefer me to order the changes differently or split it
> into smaller chunks.
>
>
> I've extensively tested this series over the hosts in our lab which have quite
> heterogeneous PCI setup each. There were no losses of any important resource.
> Without pci=realloc, there's some churn in which of the disabled expansion
> ROMs gets a scarce memory space assigned (with pci=realloc, they are all
> assigned large enough bridge window).
>
>
> Ilpo Järvinen (24):
> PCI: Remove add_align overwrite unrelated to size0
> PCI: size0 is unrelated to add_align
> PCI: Simplify size1 assignment logic
> PCI: Optional bridge window size too may need relaxing
> PCI: Fix old_size lower bound in calculate_iosize() too
> PCI: Use SZ_* instead of literals in setup-bus.c
> PCI: resource_set_range/size() conversions
> PCI: Check resource_size() separately
> PCI: Add pci_resource_num() helper
> PCI: Add dev & res local variables to resource assignment funcs
> PCI: Converge return paths in __assign_resources_sorted()
> PCI: Refactor pdev_sort_resources() & __dev_sort_resources()
> PCI: Use while loop and break instead of gotos
> PCI: Rename retval to ret
> PCI: Consolidate assignment loop next round preparation
> PCI: Remove wrong comment from pci_reassign_resource()
> PCI: Add restore_dev_resource()
> PCI: Extend enable to check for any optional resource
> PCI: Always have realloc_head in __assign_resources_sorted()
> PCI: Indicate optional resource assignment failures
> PCI: Add debug print when releasing resources before retry
> PCI: Use res->parent to check is resource is assigned
> PCI: Perform reset_resource() and build fail list in sync
> PCI: Rework optional resource handling
>
> Michał Winiarski (1):
> PCI: Add a helper to identify IOV resources
>
> drivers/pci/pci.h | 44 +++-
> drivers/pci/setup-bus.c | 566 +++++++++++++++++++++++-----------------
> drivers/pci/setup-res.c | 8 +-
> 3 files changed, 364 insertions(+), 254 deletions(-)
>
Hi, all
This series has undergone testing on the following configurations:
- Lenovo ThinkSystem SR630 V4 equipped with Intel Granite Rapids CPUs.
- The latest upstream kernel v6.14-rc2 and the stable kernel 6.13.2.
- With "pci=realloc" appended to the kernel command line.
- Red Hat Enterprise Linux 10.0 Beta.
Test results:
- All patches were applied cleanly and the build process was successful.
- The assignment for the ROM BAR of downstream devices was successful.
- The bridge window has been adjusted to fit the downstream resources.
Tested-by: Xiaochun Lee <lixc17@lenovo.com>
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 43+ messages in thread* RE: [PATCH 00/25] PCI: Resource fitting/assignment fixes and cleanups
2025-02-14 8:18 ` Xiaochun XC17 Li | 李小春 Xavier
@ 2025-02-14 11:53 ` Ilpo Järvinen
2025-02-14 21:28 ` Bjorn Helgaas
0 siblings, 1 reply; 43+ messages in thread
From: Ilpo Järvinen @ 2025-02-14 11:53 UTC (permalink / raw)
To: Xiaochun XC17 Li | 李小春 Xavier, Bjorn Helgaas
Cc: linux-pci@vger.kernel.org, Michał Winiarski, Igor Mammedov,
linux-kernel@vger.kernel.org, Mika Westerberg
[-- Attachment #1: Type: text/plain, Size: 4465 bytes --]
Hi Bjorn,
Can you please add to the last patch of the series:
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219547
...And also Xiaochun's tested by tag from below to the series.
On Fri, 14 Feb 2025, Xiaochun XC17 Li | 李小春 Xavier wrote:
> On Mon, Dec 16, 2024 at 7:56:45PM +0200 Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> > From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> >
> > Hi all,
> >
> > This series focuses on PCI resource fitting and assignment algorithms.
> > I've further changes in works to enable handling resizable BARs better during
> > resource fitting built on top of these, but that's still WIP and this series seems
> > way too large as is to have more stuff included.
> >
> > First there are small tweaks and fixes to the relaxed tail alignment code and
> > applying the lessons learned to other similar cases. They are sort of
> > independent of the rest. Then a large set of pure cleanups and refactoring that
> > are not intended to make any functional changes.
> > Finally, starting from "PCI: Extend enable to check for any optional resource" are
> > again patches that aim to make behavioral changes to fix bridge window sizing
> > to consider expansion ROM as an optional resource (to fix a remove/rescan
> > cycle issue) and improve resource fitting algorithm in general.
> >
> > The series includes one of the change from Michał Winiarski
> > <michal.winiarski@intel.com> as these changes also touch the same IOV checks.
> >
> > Please let me know if you'd prefer me to order the changes differently or split it
> > into smaller chunks.
> >
> >
> > I've extensively tested this series over the hosts in our lab which have quite
> > heterogeneous PCI setup each. There were no losses of any important resource.
> > Without pci=realloc, there's some churn in which of the disabled expansion
> > ROMs gets a scarce memory space assigned (with pci=realloc, they are all
> > assigned large enough bridge window).
> >
> >
> > Ilpo Järvinen (24):
> > PCI: Remove add_align overwrite unrelated to size0
> > PCI: size0 is unrelated to add_align
> > PCI: Simplify size1 assignment logic
> > PCI: Optional bridge window size too may need relaxing
> > PCI: Fix old_size lower bound in calculate_iosize() too
> > PCI: Use SZ_* instead of literals in setup-bus.c
> > PCI: resource_set_range/size() conversions
> > PCI: Check resource_size() separately
> > PCI: Add pci_resource_num() helper
> > PCI: Add dev & res local variables to resource assignment funcs
> > PCI: Converge return paths in __assign_resources_sorted()
> > PCI: Refactor pdev_sort_resources() & __dev_sort_resources()
> > PCI: Use while loop and break instead of gotos
> > PCI: Rename retval to ret
> > PCI: Consolidate assignment loop next round preparation
> > PCI: Remove wrong comment from pci_reassign_resource()
> > PCI: Add restore_dev_resource()
> > PCI: Extend enable to check for any optional resource
> > PCI: Always have realloc_head in __assign_resources_sorted()
> > PCI: Indicate optional resource assignment failures
> > PCI: Add debug print when releasing resources before retry
> > PCI: Use res->parent to check is resource is assigned
I also noticed there's a typo here, should be "if resource is assigned"
My apologies for the extra work.
> > PCI: Perform reset_resource() and build fail list in sync
> > PCI: Rework optional resource handling
> >
> > Michał Winiarski (1):
> > PCI: Add a helper to identify IOV resources
> >
> > drivers/pci/pci.h | 44 +++-
> > drivers/pci/setup-bus.c | 566 +++++++++++++++++++++++-----------------
> > drivers/pci/setup-res.c | 8 +-
> > 3 files changed, 364 insertions(+), 254 deletions(-)
> >
> Hi, all
> This series has undergone testing on the following configurations:
> - Lenovo ThinkSystem SR630 V4 equipped with Intel Granite Rapids CPUs.
> - The latest upstream kernel v6.14-rc2 and the stable kernel 6.13.2.
> - With "pci=realloc" appended to the kernel command line.
> - Red Hat Enterprise Linux 10.0 Beta.
>
> Test results:
> - All patches were applied cleanly and the build process was successful.
> - The assignment for the ROM BAR of downstream devices was successful.
> - The bridge window has been adjusted to fit the downstream resources.
>
> Tested-by: Xiaochun Lee <lixc17@lenovo.com>
--
i.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 00/25] PCI: Resource fitting/assignment fixes and cleanups
2025-02-14 11:53 ` Ilpo Järvinen
@ 2025-02-14 21:28 ` Bjorn Helgaas
0 siblings, 0 replies; 43+ messages in thread
From: Bjorn Helgaas @ 2025-02-14 21:28 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Xiaochun XC17 Li | 李小春 Xavier, Bjorn Helgaas,
linux-pci@vger.kernel.org, Michał Winiarski, Igor Mammedov,
linux-kernel@vger.kernel.org, Mika Westerberg
On Fri, Feb 14, 2025 at 01:53:24PM +0200, Ilpo Järvinen wrote:
> Hi Bjorn,
>
> Can you please add to the last patch of the series:
>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219547
>
> ...And also Xiaochun's tested by tag from below to the series.
Done! How exciting to have somebody step up to work on this resource
assignment mess, thank you!
Bjorn
^ permalink raw reply [flat|nested] 43+ messages in thread