| Message ID | 20240624121052.233232-3-krishnak@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI hotplug driver fixes | expand |
Hi Krishna, On 6/24/24 7:09 AM, Krishna Kumar wrote: > There is an issue with the hotplug operation when it's done on the > bridge/switch slot. The bridge-port and devices behind the bridge, which > become offline by hot-unplug operation, don't get hot-plugged/enabled by > doing hot-plug operation on that slot. Only the first port of the bridge > gets enabled and the remaining port/devices remain unplugged. The hot > plug/unplug operation is done by the hotplug driver > (drivers/pci/hotplug/pnv_php.c). > > Root Cause Analysis: This behavior is due to missing code for the > switch/bridge. The existing driver depends on pci_hp_add_devices() > function for device enablement. This function calls pci_scan_slot() on > only one device-node/port of the bridge, not on all the siblings' > device-node/port. > > The missing code needs to be added which will find all the sibling > device-nodes/bridge-ports and will run explicit pci_scan_slot() on > those. A new function has been added for this purpose which gets > invoked from pci_hp_add_devices(). This new function > pci_traverse_sibling_nodes_and_scan_slot() gets all the sibling > bridge-ports by traversal and explicitly invokes pci_scan_slot on them. > > Cc: Michael Ellerman <mpe@ellerman.id.au> > Cc: Nicholas Piggin <npiggin@gmail.com> > Cc: Christophe Leroy <christophe.leroy@csgroup.eu> > Cc: "Aneesh Kumar K.V" <aneesh.kumar@kernel.org> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Gaurav Batra <gbatra@linux.ibm.com> > Cc: Nathan Lynch <nathanl@linux.ibm.com> > Cc: Brian King <brking@linux.vnet.ibm.com> > > Signed-off-by: Krishna Kumar <krishnak@linux.ibm.com> Other than the case with NVMe devices failing that we discussed in v1's thread, I can confirm that this patch resolves many of the issues we've encountered with PCIe hotplug on POWER9. Tested-by: Shawn Anastasio <sanastasio@raptorengineering.com> Thanks, Shawn
diff --git a/arch/powerpc/include/asm/ppc-pci.h b/arch/powerpc/include/asm/ppc-pci.h index a8b7e8682f5b..83db8d0798ac 100644 --- a/arch/powerpc/include/asm/ppc-pci.h +++ b/arch/powerpc/include/asm/ppc-pci.h @@ -28,6 +28,10 @@ struct pci_dn; void *pci_traverse_device_nodes(struct device_node *start, void *(*fn)(struct device_node *, void *), void *data); + +void pci_traverse_sibling_nodes_and_scan_slot(struct device_node *start, + struct pci_bus *bus); + extern void pci_devs_phb_init_dynamic(struct pci_controller *phb); #if defined(CONFIG_IOMMU_API) && (defined(CONFIG_PPC_PSERIES) || \ diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c index 0fe251c6ac2c..639a3d592fe2 100644 --- a/arch/powerpc/kernel/pci-hotplug.c +++ b/arch/powerpc/kernel/pci-hotplug.c @@ -106,7 +106,7 @@ EXPORT_SYMBOL_GPL(pci_hp_remove_devices); */ void pci_hp_add_devices(struct pci_bus *bus) { - int slotno, mode, max; + int mode, max; struct pci_dev *dev; struct pci_controller *phb; struct device_node *dn = pci_bus_to_OF_node(bus); @@ -129,8 +129,7 @@ void pci_hp_add_devices(struct pci_bus *bus) * order for fully rescan all the way down to pick them up. * They can have been removed during partial hotplug. */ - slotno = PCI_SLOT(PCI_DN(dn->child)->devfn); - pci_scan_slot(bus, PCI_DEVFN(slotno, 0)); + pci_traverse_sibling_nodes_and_scan_slot(dn, bus); max = bus->busn_res.start; /* * Scan bridges that are already configured. We don't touch diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c index 38561d6a2079..bea612759832 100644 --- a/arch/powerpc/kernel/pci_dn.c +++ b/arch/powerpc/kernel/pci_dn.c @@ -493,4 +493,36 @@ static void pci_dev_pdn_setup(struct pci_dev *pdev) pdn = pci_get_pdn(pdev); pdev->dev.archdata.pci_data = pdn; } + +void pci_traverse_sibling_nodes_and_scan_slot(struct device_node *start, struct pci_bus *bus) +{ + struct device_node *dn; + int slotno; + + u32 class = 0; + + if (!of_property_read_u32(start->child, "class-code", &class)) { + /* Call of pci_scan_slot for non-bridge/EP case */ + if (!((class >> 8) == PCI_CLASS_BRIDGE_PCI)) { + slotno = PCI_SLOT(PCI_DN(start->child)->devfn); + pci_scan_slot(bus, PCI_DEVFN(slotno, 0)); + return; + } + } + + /* Iterate all siblings */ + for_each_child_of_node(start, dn) { + class = 0; + + if (!of_property_read_u32(start->child, "class-code", &class)) { + /* Call of pci_scan_slot on each sibling-nodes/bridge-ports */ + if ((class >> 8) == PCI_CLASS_BRIDGE_PCI) { + slotno = PCI_SLOT(PCI_DN(dn)->devfn); + pci_scan_slot(bus, PCI_DEVFN(slotno, 0)); + } + } + } + +} + DECLARE_PCI_FIXUP_EARLY(PCI_ANY_ID, PCI_ANY_ID, pci_dev_pdn_setup);
There is an issue with the hotplug operation when it's done on the bridge/switch slot. The bridge-port and devices behind the bridge, which become offline by hot-unplug operation, don't get hot-plugged/enabled by doing hot-plug operation on that slot. Only the first port of the bridge gets enabled and the remaining port/devices remain unplugged. The hot plug/unplug operation is done by the hotplug driver (drivers/pci/hotplug/pnv_php.c). Root Cause Analysis: This behavior is due to missing code for the switch/bridge. The existing driver depends on pci_hp_add_devices() function for device enablement. This function calls pci_scan_slot() on only one device-node/port of the bridge, not on all the siblings' device-node/port. The missing code needs to be added which will find all the sibling device-nodes/bridge-ports and will run explicit pci_scan_slot() on those. A new function has been added for this purpose which gets invoked from pci_hp_add_devices(). This new function pci_traverse_sibling_nodes_and_scan_slot() gets all the sibling bridge-ports by traversal and explicitly invokes pci_scan_slot on them. Cc: Michael Ellerman <mpe@ellerman.id.au> Cc: Nicholas Piggin <npiggin@gmail.com> Cc: Christophe Leroy <christophe.leroy@csgroup.eu> Cc: "Aneesh Kumar K.V" <aneesh.kumar@kernel.org> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Gaurav Batra <gbatra@linux.ibm.com> Cc: Nathan Lynch <nathanl@linux.ibm.com> Cc: Brian King <brking@linux.vnet.ibm.com> Signed-off-by: Krishna Kumar <krishnak@linux.ibm.com> --- arch/powerpc/include/asm/ppc-pci.h | 4 ++++ arch/powerpc/kernel/pci-hotplug.c | 5 ++--- arch/powerpc/kernel/pci_dn.c | 32 ++++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 3 deletions(-)