* [PATCH v4 1/2] pci/hotplug/pnv_php: Fix hotplug driver crash on Powernv
2024-07-01 7:45 [PATCH v4 0/2] PCI hotplug driver fixes Krishna Kumar
@ 2024-07-01 7:45 ` Krishna Kumar
2024-07-01 8:10 ` Krishna Kumar
2024-07-01 7:45 ` [PATCH v4 2/2] powerpc: hotplug driver bridge support Krishna Kumar
2024-07-09 6:48 ` [PATCH v4 0/2] PCI hotplug driver fixes Michael Ellerman
2 siblings, 1 reply; 5+ messages in thread
From: Krishna Kumar @ 2024-07-01 7:45 UTC (permalink / raw)
To: mpe, npiggin
Cc: linuxppc-dev, linux-kernel, linux-pci, brking, gbatra,
aneesh.kumar, christophe.leroy, nathanl, bhelgaas, oohall,
tpearson, Krishna Kumar, Shawn Anastasio
Description of the problem: The hotplug driver for powerpc
(pci/hotplug/pnv_php.c) gives kernel crash when we try to
hot-unplug/disable the PCIe switch/bridge from the PHB.
Root Cause of Crash: The crash is due to the reason that, though the msi
data structure has been released during disable/hot-unplug path and it
has been assigned with NULL, still during unregistration the code was
again trying to explicitly disable the MSI which causes the NULL pointer
dereference and kernel crash.
The patch fixes the check during unregistration path to prevent invoking
pci_disable_msi/msix() since its data structure is already freed.
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>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Shawn Anastasio <sanastasio@raptorengineering.com>
Signed-off-by: Krishna Kumar <krishnak@linux.ibm.com>
---
drivers/pci/hotplug/pnv_php.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index 694349be9d0a..573a41869c15 100644
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -40,7 +40,6 @@ static void pnv_php_disable_irq(struct pnv_php_slot *php_slot,
bool disable_device)
{
struct pci_dev *pdev = php_slot->pdev;
- int irq = php_slot->irq;
u16 ctrl;
if (php_slot->irq > 0) {
@@ -59,7 +58,7 @@ static void pnv_php_disable_irq(struct pnv_php_slot *php_slot,
php_slot->wq = NULL;
}
- if (disable_device || irq > 0) {
+ if (disable_device) {
if (pdev->msix_enabled)
pci_disable_msix(pdev);
else if (pdev->msi_enabled)
--
2.45.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v4 2/2] powerpc: hotplug driver bridge support
2024-07-01 7:45 [PATCH v4 0/2] PCI hotplug driver fixes Krishna Kumar
2024-07-01 7:45 ` [PATCH v4 1/2] pci/hotplug/pnv_php: Fix hotplug driver crash on Powernv Krishna Kumar
@ 2024-07-01 7:45 ` Krishna Kumar
2024-07-09 6:48 ` [PATCH v4 0/2] PCI hotplug driver fixes Michael Ellerman
2 siblings, 0 replies; 5+ messages in thread
From: Krishna Kumar @ 2024-07-01 7:45 UTC (permalink / raw)
To: mpe, npiggin
Cc: linuxppc-dev, linux-kernel, linux-pci, brking, gbatra,
aneesh.kumar, christophe.leroy, nathanl, bhelgaas, oohall,
tpearson, Krishna Kumar, Shawn Anastasio
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>
Tested-by: Shawn Anastasio <sanastasio@raptorengineering.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(-)
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);
--
2.45.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v4 0/2] PCI hotplug driver fixes
2024-07-01 7:45 [PATCH v4 0/2] PCI hotplug driver fixes Krishna Kumar
2024-07-01 7:45 ` [PATCH v4 1/2] pci/hotplug/pnv_php: Fix hotplug driver crash on Powernv Krishna Kumar
2024-07-01 7:45 ` [PATCH v4 2/2] powerpc: hotplug driver bridge support Krishna Kumar
@ 2024-07-09 6:48 ` Michael Ellerman
2 siblings, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2024-07-09 6:48 UTC (permalink / raw)
To: mpe, npiggin, Krishna Kumar
Cc: linuxppc-dev, linux-kernel, linux-pci, brking, gbatra,
aneesh.kumar, christophe.leroy, nathanl, bhelgaas, oohall,
tpearson
On Mon, 01 Jul 2024 13:15:05 +0530, Krishna Kumar wrote:
> The fix of Powerpc hotplug driver (drivers/pci/hotplug/pnv_php.c)
> addresses below two issues.
>
> 1. Kernel Crash during hot unplug of bridge/switch slot.
>
> 2. Bridge Support Enablement - Previously, when we do a hot-unplug
> operation on a bridge slot, all the ports and devices behind the
> bridge-ports would be hot-unplugged/offline, but when we do a hot-plug
> operation on the same bridge slot, all the ports and devices behind the
> bridge would not get hot-plugged/online. In this case, Only the first
> port of the bridge gets enabled and the remaining port/devices remain
> unplugged/offline. After the fix, The hot-unplug and hot-plug
> operations on the slot associated with the bridge started behaving
> correctly and became in sync. Now, after the hot plug operation on the
> same slot, all the bridge ports and devices behind the bridge become
> hot-plugged/online/restored in the same manner as it was before the
> hot-unplug operation.
>
> [...]
Applied to powerpc/next.
[1/2] pci/hotplug/pnv_php: Fix hotplug driver crash on Powernv
https://git.kernel.org/powerpc/c/335e35b748527f0c06ded9eebb65387f60647fda
[2/2] powerpc: hotplug driver bridge support
https://git.kernel.org/powerpc/c/20ce0c247b2500cb7060cb115274ba71abda2626
cheers
^ permalink raw reply [flat|nested] 5+ messages in thread