Re: [PATCH v3 2/2] powerpc: hotplug driver bridge support

2024-06-27 Thread Shawn Anastasio
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 
> Cc: Nicholas Piggin 
> Cc: Christophe Leroy 
> Cc: "Aneesh Kumar K.V" 
> Cc: Bjorn Helgaas 
> Cc: Gaurav Batra 
> Cc: Nathan Lynch 
> Cc: Brian King 
> 
> Signed-off-by: Krishna Kumar 

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 

Thanks,
Shawn


[PATCH v3 2/2] powerpc: hotplug driver bridge support

2024-06-24 Thread Krishna Kumar
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 
Cc: Nicholas Piggin 
Cc: Christophe Leroy 
Cc: "Aneesh Kumar K.V" 
Cc: Bjorn Helgaas 
Cc: Gaurav Batra 
Cc: Nathan Lynch 
Cc: Brian King 

Signed-off-by: Krishna Kumar 
---
 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(-)

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", )) {
+   /* 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", )) {
+   /* 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);
-- 
2.45.0