diff mbox series

[v5,01/12] powerpc/64: Adjust order in pcibios_init()

Message ID 4506b0489eabd0921a3587d90bd44c7683f3472d.1565930772.git.sbobroff@linux.ibm.com (mailing list archive)
State Accepted
Commit 3f068aae7a958555533847af88705b5629f31600
Headers show
Series [v5,01/12] powerpc/64: Adjust order in pcibios_init() | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch next (c9633332103e55bc73d80d07ead28b95a22a85a3)
snowpatch_ozlabs/checkpatch success total: 0 errors, 0 warnings, 0 checks, 43 lines checked

Commit Message

Sam Bobroff Aug. 16, 2019, 4:48 a.m. UTC
The pcibios_init() function for PowerPC 64 currently calls
pci_bus_add_devices() before pcibios_resource_survey(). This means
that at boot time, when the pcibios_bus_add_device() hooks are called
by pci_bus_add_devices(), device resources have not been allocated and
they are unable to perform EEH setup, so a separate pass is needed.

This patch adjusts that order so that it will become possible to
consolidate the EEH setup work into a single location.

The only functional change is to execute pcibios_resource_survey()
(excepting ppc_md.pcibios_fixup(), see below) before
pci_bus_add_devices() instead of after it.

Because pcibios_scan_phb() and pci_bus_add_devices() are called
together in a loop, this must be broken into one loop for each call.
Then the call to pcibios_resource_survey() is moved up in between
them. This changes the ordering but because pcibios_resource_survey()
also calls ppc_md.pcibios_fixup(), that call is extracted out into
pcibios_init() to where pcibios_resource_survey() was, so that it is
not moved.

The only other caller of pcibios_resource_survey() is the PowerPC 32
version of pcibios_init(), and therefore, that is modified to call
ppc_md.pcibios_fixup() right after pcibios_resource_survey() so that
there is no functional change there at all.

The re-arrangement will cause very few side-effects because at this
stage in the boot, pci_bus_add_devices() does very little:
- pci_create_sysfs_dev_files() does nothing (no sysfs yet)
- pci_proc_attach_device() does nothing (no proc yet)
- device_attach() does nothing (no drivers yet)
This leaves only the pci_final_fixup calls, D3 support, and marking
the device as added. Of those, only the pci_final_fixup calls have the
potential to be affected by resource allocation.

The only pci_final_fixup handlers that touch resources seem to be one
for x86 (pci_amd_enable_64bit_bar()), and a PowerPC 32 platform driver
(quirk_final_uli1575()), neither of which use this pcibios_init()
function. Even if they did, it would almost certainly be a bug, under
the current ordering, to rely on or make changes to resources before
they were allocated.

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
v5 - Complete rewrite of commit message based on more research.

 arch/powerpc/kernel/pci-common.c |  4 ----
 arch/powerpc/kernel/pci_32.c     |  4 ++++
 arch/powerpc/kernel/pci_64.c     | 12 +++++++++---
 3 files changed, 13 insertions(+), 7 deletions(-)

Comments

Michael Ellerman Aug. 28, 2019, 4:24 a.m. UTC | #1
On Fri, 2019-08-16 at 04:48:05 UTC, Sam Bobroff wrote:
> The pcibios_init() function for PowerPC 64 currently calls
> pci_bus_add_devices() before pcibios_resource_survey(). This means
> that at boot time, when the pcibios_bus_add_device() hooks are called
> by pci_bus_add_devices(), device resources have not been allocated and
> they are unable to perform EEH setup, so a separate pass is needed.
> 
> This patch adjusts that order so that it will become possible to
> consolidate the EEH setup work into a single location.
> 
> The only functional change is to execute pcibios_resource_survey()
> (excepting ppc_md.pcibios_fixup(), see below) before
> pci_bus_add_devices() instead of after it.
> 
> Because pcibios_scan_phb() and pci_bus_add_devices() are called
> together in a loop, this must be broken into one loop for each call.
> Then the call to pcibios_resource_survey() is moved up in between
> them. This changes the ordering but because pcibios_resource_survey()
> also calls ppc_md.pcibios_fixup(), that call is extracted out into
> pcibios_init() to where pcibios_resource_survey() was, so that it is
> not moved.
> 
> The only other caller of pcibios_resource_survey() is the PowerPC 32
> version of pcibios_init(), and therefore, that is modified to call
> ppc_md.pcibios_fixup() right after pcibios_resource_survey() so that
> there is no functional change there at all.
> 
> The re-arrangement will cause very few side-effects because at this
> stage in the boot, pci_bus_add_devices() does very little:
> - pci_create_sysfs_dev_files() does nothing (no sysfs yet)
> - pci_proc_attach_device() does nothing (no proc yet)
> - device_attach() does nothing (no drivers yet)
> This leaves only the pci_final_fixup calls, D3 support, and marking
> the device as added. Of those, only the pci_final_fixup calls have the
> potential to be affected by resource allocation.
> 
> The only pci_final_fixup handlers that touch resources seem to be one
> for x86 (pci_amd_enable_64bit_bar()), and a PowerPC 32 platform driver
> (quirk_final_uli1575()), neither of which use this pcibios_init()
> function. Even if they did, it would almost certainly be a bug, under
> the current ordering, to rely on or make changes to resources before
> they were allocated.
> 
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/3f068aae7a958555533847af88705b5629f31600

cheers
diff mbox series

Patch

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index f627e15bb43c..1c448cf25506 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1379,10 +1379,6 @@  void __init pcibios_resource_survey(void)
 		pr_debug("PCI: Assigning unassigned resources...\n");
 		pci_assign_unassigned_resources();
 	}
-
-	/* Call machine dependent fixup */
-	if (ppc_md.pcibios_fixup)
-		ppc_md.pcibios_fixup();
 }
 
 /* This is used by the PCI hotplug driver to allocate resource
diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index 50942a1d1a5f..b49e1060a3bf 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -263,6 +263,10 @@  static int __init pcibios_init(void)
 	/* Call common code to handle resource allocation */
 	pcibios_resource_survey();
 
+	/* Call machine dependent fixup */
+	if (ppc_md.pcibios_fixup)
+		ppc_md.pcibios_fixup();
+
 	/* Call machine dependent post-init code */
 	if (ppc_md.pcibios_after_init)
 		ppc_md.pcibios_after_init();
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index b7030b1189d0..f83d1f69b1dd 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -54,14 +54,20 @@  static int __init pcibios_init(void)
 	pci_add_flags(PCI_ENABLE_PROC_DOMAINS | PCI_COMPAT_DOMAIN_0);
 
 	/* Scan all of the recorded PCI controllers.  */
-	list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
+	list_for_each_entry_safe(hose, tmp, &hose_list, list_node)
 		pcibios_scan_phb(hose);
-		pci_bus_add_devices(hose->bus);
-	}
 
 	/* Call common code to handle resource allocation */
 	pcibios_resource_survey();
 
+	/* Add devices. */
+	list_for_each_entry_safe(hose, tmp, &hose_list, list_node)
+		pci_bus_add_devices(hose->bus);
+
+	/* Call machine dependent fixup */
+	if (ppc_md.pcibios_fixup)
+		ppc_md.pcibios_fixup();
+
 	printk(KERN_DEBUG "PCI: Probing PCI hardware done\n");
 
 	return 0;