[PATCH 0/5] PCI: Finally make pci_root_buses private

Hi all!

The ominous warning about pci_root_buses in drivers/pci/probe.c caught
my attention. Looking closer, I found that there are uses in four
arch-specific files left before we can stop exposing that symbol outside
of drivers/pci.

Finish off the job that Yinghai Lu started in 2013 - see
https://msgid.link/1359265003-16166-23-git-send-email-yinghai@kernel.org/

The entire series has been compile-tested only - with defconfigs on
alpha, arm, powerpc, and x86.

Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
---
Gerd Bayer (5):
      alpha/pci: Use official API to iterate over PCI buses
      arm/pci: Use official API to iterate over PCI buses
      powerpc/pci: Use official API to iterate over PCI buses
      x86/pci: Use official API to iterate over PCI buses
      PCI: Make pci_root_buses private to PCI core

 arch/alpha/kernel/pci.c          |  4 ++--
 arch/arm/kernel/bios32.c         |  4 ++--
 arch/powerpc/kernel/pci-common.c |  7 ++++---
 arch/powerpc/kernel/pci_64.c     |  4 ++--
 arch/x86/pci/i386.c              | 14 ++++++++------
 drivers/pci/pci.h                |  3 +++
 drivers/pci/probe.c              |  2 --
 include/linux/pci.h              |  4 ----
 8 files changed, 21 insertions(+), 21 deletions(-)
---
base-commit: 5d6919055dec134de3c40167a490f33c74c12581
change-id: 20260508-priv_root_buses-0263ef2679ad

Best regards,
-- 
Gerd Bayer [off-list ref]

Replace iterating over pci_root_buses with the official
pci_find_next_bus() call provided by PCI core. This allows to make
pci_root_buses private to PCI core.

Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
---
 arch/powerpc/kernel/pci-common.c | 7 ++++---
 arch/powerpc/kernel/pci_64.c     | 4 ++--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 8efe95a0c4ff..1e0be7bcaa56 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c

@@ -1417,10 +1417,10 @@ static void __init pcibios_reserve_legacy_regions(struct pci_bus *bus)
 
 void __init pcibios_resource_survey(void)
 {
-	struct pci_bus *b;
+	struct pci_bus *b = NULL;
 
 	/* Allocate and assign resources */
-	list_for_each_entry(b, &pci_root_buses, node)
+	while ((b = pci_find_next_bus(b)) != NULL)
 		pcibios_allocate_bus_resources(b);
 	if (!pci_has_flag(PCI_REASSIGN_ALL_RSRC)) {
 		pcibios_allocate_resources(0);

@@ -1432,7 +1432,8 @@ void __init pcibios_resource_survey(void)
 	 * bus available resources to avoid allocating things on top of them
 	 */
 	if (!pci_has_flag(PCI_PROBE_ONLY)) {
-		list_for_each_entry(b, &pci_root_buses, node)
+		b = NULL; /* Start all over */
+		while ((b = pci_find_next_bus(b)) != NULL)
 			pcibios_reserve_legacy_regions(b);
 	}
 

diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index e27342ef128b..f816d063b984 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c

@@ -227,7 +227,7 @@ SYSCALL_DEFINE3(pciconfig_iobase, long, which, unsigned long, in_bus,
 			  unsigned long, in_devfn)
 {
 	struct pci_controller* hose;
-	struct pci_bus *tmp_bus, *bus = NULL;
+	struct pci_bus *tmp_bus = NULL, *bus = NULL;
 	struct device_node *hose_node;
 
 	/* Argh ! Please forgive me for that hack, but that's the

@@ -248,7 +248,7 @@ SYSCALL_DEFINE3(pciconfig_iobase, long, which, unsigned long, in_bus,
 	 * used on pre-domains setup. We return the first match
 	 */
 
-	list_for_each_entry(tmp_bus, &pci_root_buses, node) {
+	while ((tmp_bus = pci_find_next_bus(tmp_bus)) != NULL) {
 		if (in_bus >= tmp_bus->number &&
 		    in_bus <= tmp_bus->busn_res.end) {
 			bus = tmp_bus;

-- 
2.54.0

Replace iterating over pci_root_buses with the official
pci_find_next_bus() call provided by PCI core. This allows to make
pci_root_buses private to PCI core.

Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
---
 arch/arm/kernel/bios32.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index ac0e890510da..35642c9ba054 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c

@@ -59,9 +59,9 @@ static void pcibios_bus_report_status(struct pci_bus *bus, u_int status_mask, in
 
 void pcibios_report_status(u_int status_mask, int warn)
 {
-	struct pci_bus *bus;
+	struct pci_bus *bus = NULL;
 
-	list_for_each_entry(bus, &pci_root_buses, node)
+	while ((bus = pci_find_next_bus(bus)) != NULL)
 		pcibios_bus_report_status(bus, status_mask, warn);
 }
 

-- 
2.54.0

Replace iterating over pci_root_buses with the official
pci_find_next_bus() call provided by PCI core. This allows to make
pci_root_buses private to PCI core.

Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
---
 arch/alpha/kernel/pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
index 11df411b1d18..02ec4dbb3ac6 100644
--- a/arch/alpha/kernel/pci.c
+++ b/arch/alpha/kernel/pci.c

@@ -312,9 +312,9 @@ pcibios_claim_one_bus(struct pci_bus *b)
 static void __init
 pcibios_claim_console_setup(void)
 {
-	struct pci_bus *b;
+	struct pci_bus *b = NULL;
 
-	list_for_each_entry(b, &pci_root_buses, node)
+	while ((b = pci_find_next_bus(b)) != NULL)
 		pcibios_claim_one_bus(b);
 }
 

-- 
2.54.0

Replace iterating over pci_root_buses with the official
pci_find_next_bus() call provided by PCI core. This allows to make
pci_root_buses private to PCI core.

Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
---
 arch/x86/pci/i386.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index e2de26b82940..194d0fa3cec8 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c

@@ -357,10 +357,10 @@ static void pcibios_allocate_rom_resources(struct pci_bus *bus)
 
 static int __init pcibios_assign_resources(void)
 {
-	struct pci_bus *bus;
+	struct pci_bus *bus = NULL;
 
 	if (!(pci_probe & PCI_ASSIGN_ROMS))
-		list_for_each_entry(bus, &pci_root_buses, node)
+		while ((bus = pci_find_next_bus(bus)) != NULL)
 			pcibios_allocate_rom_resources(bus);
 
 	pci_assign_unassigned_resources();

@@ -390,16 +390,18 @@ void pcibios_resource_survey_bus(struct pci_bus *bus)
 
 void __init pcibios_resource_survey(void)
 {
-	struct pci_bus *bus;
+	struct pci_bus *bus = NULL;
 
 	DBG("PCI: Allocating resources\n");
 
-	list_for_each_entry(bus, &pci_root_buses, node)
+	while ((bus = pci_find_next_bus(bus)) != NULL)
 		pcibios_allocate_bus_resources(bus);
 
-	list_for_each_entry(bus, &pci_root_buses, node)
+	bus = NULL; /* start all over */
+	while ((bus = pci_find_next_bus(bus)) != NULL)
 		pcibios_allocate_resources(bus, 0);
-	list_for_each_entry(bus, &pci_root_buses, node)
+	bus = NULL; /* start all over */
+	while ((bus = pci_find_next_bus(bus)) != NULL)
 		pcibios_allocate_resources(bus, 1);
 
 	e820__reserve_resources_late();

-- 
2.54.0

After all users of pci_root_buses external to PCI core have been
converted to using pci_find_next_bus(), move its declaration to the
PCI core code and stop exporting the symbol.

Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
---
 drivers/pci/pci.h   | 3 +++
 drivers/pci/probe.c | 2 --
 include/linux/pci.h | 4 ----
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4a14f88e543a..1f36d400c9e0 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h

@@ -366,6 +366,9 @@ static inline void pci_create_legacy_files(struct pci_bus *bus) { }
 static inline void pci_remove_legacy_files(struct pci_bus *bus) { }
 #endif
 
+/* List of all known PCI buses */
+extern struct list_head pci_root_buses;
+
 /* Lock for read/write access to pci device and bus lists */
 extern struct rw_semaphore pci_bus_sem;
 extern struct mutex pci_slot_mutex;

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index b63cd0c310bc..2e97ab125ead 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c

@@ -33,9 +33,7 @@ static struct resource busn_resource = {
 	.flags	= IORESOURCE_BUS,
 };
 
-/* Ugh.  Need to stop exporting this to modules. */
 LIST_HEAD(pci_root_buses);
-EXPORT_SYMBOL(pci_root_buses);
 
 static LIST_HEAD(pci_domain_busn_res_list);
 

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 2c4454583c11..1c4610848b5c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h

@@ -1192,10 +1192,6 @@ extern enum pcie_bus_config_types pcie_bus_config;
 
 extern const struct bus_type pci_bus_type;
 
-/* Do NOT directly access these two variables, unless you are arch-specific PCI
- * code, or PCI core code. */
-extern struct list_head pci_root_buses;	/* List of all known PCI buses */
-
 void pcibios_resource_survey_bus(struct pci_bus *bus);
 void pcibios_bus_add_device(struct pci_dev *pdev);
 void pcibios_add_bus(struct pci_bus *bus);

-- 
2.54.0

On 5/15/26 07:22, Gerd Bayer wrote:

What's with the 'bus = NULL'? I thought there was some crazy macro magic
going on or something, but pci_find_next_bus() looks like a normal
function that's just taking a pointer and not _modifying_ the pointer value.

Also, wouldn't this be a more readable way of writing what you have?

	while (bus = pci_find_next_bus(bus))

For that matter isn't the kernel idiom for these things:

	for_each_pci_bus(bus) {
		// do bus stuff
	}

I'm kinda surprised there isn't one of those already.

On Fri, 2026-05-15 at 08:13 -0700, Dave Hansen wrote:

On 5/15/26 07:22, Gerd Bayer wrote:

quoted

 static int __init pcibios_assign_resources(void)
 {
-	struct pci_bus *bus;
+	struct pci_bus *bus = NULL;
 
 	if (!(pci_probe & PCI_ASSIGN_ROMS))
-		list_for_each_entry(bus, &pci_root_buses, node)
+		while ((bus = pci_find_next_bus(bus)) != NULL)
 			pcibios_allocate_rom_resources(bus);

What's with the 'bus = NULL'? I thought there was some crazy macro magic
going on or something, but pci_find_next_bus() looks like a normal
function that's just taking a pointer and not _modifying_ the pointer value.

Initializing 'bus = NULL" makes sure, that pci_find_next_bus() starts
at the list head; list_for_each_entry() did that implicitly. I didn't
want to rely on implicit zero-init for local var's on all the various
architectures. But I'm fine to drop it here, if you prefer.

Also, wouldn't this be a more readable way of writing what you have?

	while (bus = pci_find_next_bus(bus))

Yeah, another occasion of me being (overly?) verbose.
arch/sparc/kernel/pci.c was my blueprint. Again, something that I'm ok
to drop.

For that matter isn't the kernel idiom for these things:

	for_each_pci_bus(bus) {
		// do bus stuff
	}

I'm kinda surprised there isn't one of those already.

Just guessing: There was too little use of pci_find_next_bus() to
warrant that short-cut. But I can make a proposal in the next
iteration.

Thanks,
Gerd

On Fri, 2026-05-15 at 16:22 +0200, Gerd Bayer wrote:

Hi Russell,

Sashiko
https://sashiko.dev/#/message/20260515145940.E85AAC2BCB0%40smtp.kernel.org
reported:

Since pci_find_next_bus() unconditionally acquires the pci_bus_sem read-write
semaphore using down_read(), this introduces a blocking operation into that
atomic path:

dc21285_abort_irq() [hardirq context]
  pcibios_report_status()
    pci_find_next_bus()
      down_read(&pci_bus_sem) [sleeps]

Does this path need an alternative approach to safely iterate over the buses
without taking a sleeping lock?

IMHO, it looks like this entire pcibios_report_status() iterating over
all PCI buses and all their devices would be better off if moved
outside of the hardirq context?

Or could pcibios_report_status() be converted to use
for_each_pci_device()?

Any suggestions welcome...
Gerd