All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/4] powerpc/eeh: Fix stale cached primary bus
@ 2016-02-09  4:50 Gavin Shan
  2016-02-09  4:50 ` [PATCH v2 2/4] powerpc/powernv: Fix stale PE " Gavin Shan
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Gavin Shan @ 2016-02-09  4:50 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: mpe, Gavin Shan

When PE is created, its primary bus is cached to pe->bus. At later
point, the cached primary bus is returned from eeh_pe_bus_get().
However, we could get stale cached primary bus and run into kernel
crash in one case: full hotplug as part of fenced PHB error recovery
releases all PCI busses under the PHB at unplugging time and recreate
them at plugging time. pe->bus is still dereferencing the PCI bus
that was released.

This adds another PE flag (EEH_PE_PRI_BUS) to represent the validity
of pe->bus. pe->bus is updated when its first child EEH device is
online and the flag is set. Before unplugging in full hotplug for
error recovery, the flag is cleared.

Fixes: 8cdb2833 ("powerpc/eeh: Trace PCI bus from PE")
Cc: stable@vger.kernel.org #v3.11+
Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Reported-by: Pradipta Ghosh <pradghos@in.ibm.com>
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Tested-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
---
 arch/powerpc/include/asm/eeh.h               | 1 +
 arch/powerpc/kernel/eeh_driver.c             | 3 +++
 arch/powerpc/kernel/eeh_pe.c                 | 2 +-
 arch/powerpc/platforms/powernv/eeh-powernv.c | 5 ++++-
 4 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index c5eb86f..867c39b 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -81,6 +81,7 @@ struct pci_dn;
 #define EEH_PE_KEEP		(1 << 8)	/* Keep PE on hotplug	*/
 #define EEH_PE_CFG_RESTRICTED	(1 << 9)	/* Block config on error */
 #define EEH_PE_REMOVED		(1 << 10)	/* Removed permanently	*/
+#define EEH_PE_PRI_BUS		(1 << 11)	/* Cached primary bus   */
 
 struct eeh_pe {
 	int type;			/* PE type: PHB/Bus/Device	*/
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 9387421..301be31 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -564,6 +564,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus)
 	 */
 	eeh_pe_state_mark(pe, EEH_PE_KEEP);
 	if (bus) {
+		eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
 		pci_lock_rescan_remove();
 		pcibios_remove_pci_devices(bus);
 		pci_unlock_rescan_remove();
@@ -803,6 +804,7 @@ perm_error:
 	 * the their PCI config any more.
 	 */
 	if (frozen_bus) {
+		eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
 		eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
 
 		pci_lock_rescan_remove();
@@ -886,6 +888,7 @@ static void eeh_handle_special_event(void)
 					continue;
 
 				/* Notify all devices to be down */
+				eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
 				bus = eeh_pe_bus_get(phb_pe);
 				eeh_pe_dev_traverse(pe,
 					eeh_report_failure, NULL);
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 8654cb1..127d124 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -931,7 +931,7 @@ struct pci_bus *eeh_pe_bus_get(struct eeh_pe *pe)
 		bus = pe->phb->bus;
 	} else if (pe->type & EEH_PE_BUS ||
 		   pe->type & EEH_PE_DEVICE) {
-		if (pe->bus) {
+		if (pe->state & EEH_PE_PRI_BUS) {
 			bus = pe->bus;
 			goto out;
 		}
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 5f152b9..87f47e5 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -444,9 +444,12 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
 	 * PCI devices of the PE are expected to be removed prior
 	 * to PE reset.
 	 */
-	if (!edev->pe->bus)
+	if (!(edev->pe->state & EEH_PE_PRI_BUS)) {
 		edev->pe->bus = pci_find_bus(hose->global_number,
 					     pdn->busno);
+		if (edev->pe->bus)
+			edev->pe->state |= EEH_PE_PRI_BUS;
+	}
 
 	/*
 	 * Enable EEH explicitly so that we will do EEH check
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/4] powerpc/powernv: Fix stale PE primary bus
  2016-02-09  4:50 [PATCH v2 1/4] powerpc/eeh: Fix stale cached primary bus Gavin Shan
@ 2016-02-09  4:50 ` Gavin Shan
  2016-02-12  6:02   ` Andrew Donnellan
  2016-02-09  4:50 ` [PATCH v2 3/4] powerpc/eeh: Reworked eeh_pe_bus_get() Gavin Shan
  2016-02-09  4:50 ` [PATCH v2 4/4] powerpc/powernv: Simplify definitions of EEH debugfs handlers Gavin Shan
  2 siblings, 1 reply; 10+ messages in thread
From: Gavin Shan @ 2016-02-09  4:50 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: mpe, Gavin Shan

When PCI bus is unplugged during full hotplug for EEH recovery,
the platform PE instance (struct pnv_ioda_pe) isn't released and
it dereferences the stale PCI bus that has been released. It leads
to kernel crash when referring to the stale PCI bus.

This fixes the issue by correcting the PE's primary bus when it's
oneline at plugging time, in pnv_pci_dma_bus_setup() which is to
be called by pcibios_fixup_bus().

Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Reported-by: Pradipta Ghosh <pradghos@in.ibm.com>
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Tested-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
---
 arch/powerpc/platforms/powernv/pci-ioda.c |  1 +
 arch/powerpc/platforms/powernv/pci.c      | 20 ++++++++++++++++++++
 arch/powerpc/platforms/powernv/pci.h      |  1 +
 3 files changed, 22 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 573ae19..f90dc04 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -3180,6 +3180,7 @@ static void pnv_pci_ioda_shutdown(struct pci_controller *hose)
 
 static const struct pci_controller_ops pnv_pci_ioda_controller_ops = {
        .dma_dev_setup = pnv_pci_dma_dev_setup,
+       .dma_bus_setup = pnv_pci_dma_bus_setup,
 #ifdef CONFIG_PCI_MSI
        .setup_msi_irqs = pnv_setup_msi_irqs,
        .teardown_msi_irqs = pnv_teardown_msi_irqs,
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 8de0140..2441dec 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -757,6 +757,26 @@ void pnv_pci_dma_dev_setup(struct pci_dev *pdev)
 		phb->dma_dev_setup(phb, pdev);
 }
 
+void pnv_pci_dma_bus_setup(struct pci_bus *bus)
+{
+	struct pci_controller *hose = bus->sysdata;
+	struct pnv_phb *phb = hose->private_data;
+	struct pnv_ioda_pe *pe;
+
+	list_for_each_entry(pe, &phb->ioda.pe_list, list) {
+		if (!(pe->flags | (PNV_IODA_PE_BUS | PNV_IODA_PE_BUS_ALL)))
+			continue;
+
+		if (!pe->pbus)
+			continue;
+
+		if (bus->number == ((pe->rid >> 8) & 0xFF)) {
+			pe->pbus = bus;
+			break;
+		}
+	}
+}
+
 void pnv_pci_shutdown(void)
 {
 	struct pci_controller *hose;
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index 32cae3d..3f814f3 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -232,6 +232,7 @@ extern void pnv_pci_reset_secondary_bus(struct pci_dev *dev);
 extern int pnv_eeh_phb_reset(struct pci_controller *hose, int option);
 
 extern void pnv_pci_dma_dev_setup(struct pci_dev *pdev);
+extern void pnv_pci_dma_bus_setup(struct pci_bus *bus);
 extern int pnv_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type);
 extern void pnv_teardown_msi_irqs(struct pci_dev *pdev);
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/4] powerpc/eeh: Reworked eeh_pe_bus_get()
  2016-02-09  4:50 [PATCH v2 1/4] powerpc/eeh: Fix stale cached primary bus Gavin Shan
  2016-02-09  4:50 ` [PATCH v2 2/4] powerpc/powernv: Fix stale PE " Gavin Shan
@ 2016-02-09  4:50 ` Gavin Shan
  2016-03-09 12:51   ` [v2,3/4] " Michael Ellerman
  2016-02-09  4:50 ` [PATCH v2 4/4] powerpc/powernv: Simplify definitions of EEH debugfs handlers Gavin Shan
  2 siblings, 1 reply; 10+ messages in thread
From: Gavin Shan @ 2016-02-09  4:50 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: mpe, Gavin Shan

The original implementation is ugly: unnecessary if statements and
"out" tag. This reworks the function to avoid above weaknesses. No
functional changes introduced.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
---
 arch/powerpc/kernel/eeh_pe.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 127d124..faaf19e 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -923,25 +923,21 @@ out:
  */
 struct pci_bus *eeh_pe_bus_get(struct eeh_pe *pe)
 {
-	struct pci_bus *bus = NULL;
 	struct eeh_dev *edev;
 	struct pci_dev *pdev;
 
-	if (pe->type & EEH_PE_PHB) {
-		bus = pe->phb->bus;
-	} else if (pe->type & EEH_PE_BUS ||
-		   pe->type & EEH_PE_DEVICE) {
-		if (pe->state & EEH_PE_PRI_BUS) {
-			bus = pe->bus;
-			goto out;
-		}
+	if (pe->type & EEH_PE_PHB)
+		return pe->phb->bus;
 
-		edev = list_first_entry(&pe->edevs, struct eeh_dev, list);
-		pdev = eeh_dev_to_pci_dev(edev);
-		if (pdev)
-			bus = pdev->bus;
-	}
+	/* The primary bus might be cached during probe time */
+	if (pe->state & EEH_PE_PRI_BUS)
+		return pe->bus;
 
-out:
-	return bus;
+	/* Retrieve the parent PCI bus of first (top) PCI device */
+	edev = list_first_entry_or_null(&pe->edevs, struct eeh_dev, list);
+	pdev = eeh_dev_to_pci_dev(edev);
+	if (pdev)
+		return pdev->bus;
+
+	return NULL;
 }
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 4/4] powerpc/powernv: Simplify definitions of EEH debugfs handlers
  2016-02-09  4:50 [PATCH v2 1/4] powerpc/eeh: Fix stale cached primary bus Gavin Shan
  2016-02-09  4:50 ` [PATCH v2 2/4] powerpc/powernv: Fix stale PE " Gavin Shan
  2016-02-09  4:50 ` [PATCH v2 3/4] powerpc/eeh: Reworked eeh_pe_bus_get() Gavin Shan
@ 2016-02-09  4:50 ` Gavin Shan
  2016-02-17 12:41   ` [v2, " Michael Ellerman
  2 siblings, 1 reply; 10+ messages in thread
From: Gavin Shan @ 2016-02-09  4:50 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: mpe, Gavin Shan

The EEH debugfs handlers have same prototype. This introduces
a macro to define them, then to simplify the code. No logical
changes.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/eeh-powernv.c | 60 ++++++++++------------------
 1 file changed, 22 insertions(+), 38 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 87f47e5..8119172 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -167,42 +167,26 @@ static int pnv_eeh_dbgfs_get(void *data, int offset, u64 *val)
 	return 0;
 }
 
-static int pnv_eeh_outb_dbgfs_set(void *data, u64 val)
-{
-	return pnv_eeh_dbgfs_set(data, 0xD10, val);
-}
-
-static int pnv_eeh_outb_dbgfs_get(void *data, u64 *val)
-{
-	return pnv_eeh_dbgfs_get(data, 0xD10, val);
-}
-
-static int pnv_eeh_inbA_dbgfs_set(void *data, u64 val)
-{
-	return pnv_eeh_dbgfs_set(data, 0xD90, val);
-}
-
-static int pnv_eeh_inbA_dbgfs_get(void *data, u64 *val)
-{
-	return pnv_eeh_dbgfs_get(data, 0xD90, val);
-}
-
-static int pnv_eeh_inbB_dbgfs_set(void *data, u64 val)
-{
-	return pnv_eeh_dbgfs_set(data, 0xE10, val);
-}
-
-static int pnv_eeh_inbB_dbgfs_get(void *data, u64 *val)
-{
-	return pnv_eeh_dbgfs_get(data, 0xE10, val);
-}
+#define PNV_EEH_DBGFS_ENTRY(name, reg)				\
+static int pnv_eeh_dbgfs_set_##name(void *data, u64 val)	\
+{								\
+	return pnv_eeh_dbgfs_set(data, reg, val);		\
+}								\
+								\
+static int pnv_eeh_dbgfs_get_##name(void *data, u64 *val)	\
+{								\
+	return pnv_eeh_dbgfs_get(data, reg, val);		\
+}								\
+								\
+DEFINE_SIMPLE_ATTRIBUTE(pnv_eeh_dbgfs_ops_##name,		\
+			pnv_eeh_dbgfs_get_##name,		\
+                        pnv_eeh_dbgfs_set_##name,		\
+			"0x%llx\n")
+
+PNV_EEH_DBGFS_ENTRY(outb, 0xD10);
+PNV_EEH_DBGFS_ENTRY(inbA, 0xD90);
+PNV_EEH_DBGFS_ENTRY(inbB, 0xE10);
 
-DEFINE_SIMPLE_ATTRIBUTE(pnv_eeh_outb_dbgfs_ops, pnv_eeh_outb_dbgfs_get,
-			pnv_eeh_outb_dbgfs_set, "0x%llx\n");
-DEFINE_SIMPLE_ATTRIBUTE(pnv_eeh_inbA_dbgfs_ops, pnv_eeh_inbA_dbgfs_get,
-			pnv_eeh_inbA_dbgfs_set, "0x%llx\n");
-DEFINE_SIMPLE_ATTRIBUTE(pnv_eeh_inbB_dbgfs_ops, pnv_eeh_inbB_dbgfs_get,
-			pnv_eeh_inbB_dbgfs_set, "0x%llx\n");
 #endif /* CONFIG_DEBUG_FS */
 
 /**
@@ -268,13 +252,13 @@ static int pnv_eeh_post_init(void)
 
 		debugfs_create_file("err_injct_outbound", 0600,
 				    phb->dbgfs, hose,
-				    &pnv_eeh_outb_dbgfs_ops);
+				    &pnv_eeh_dbgfs_ops_outb);
 		debugfs_create_file("err_injct_inboundA", 0600,
 				    phb->dbgfs, hose,
-				    &pnv_eeh_inbA_dbgfs_ops);
+				    &pnv_eeh_dbgfs_ops_inbA);
 		debugfs_create_file("err_injct_inboundB", 0600,
 				    phb->dbgfs, hose,
-				    &pnv_eeh_inbB_dbgfs_ops);
+				    &pnv_eeh_dbgfs_ops_inbB);
 #endif /* CONFIG_DEBUG_FS */
 	}
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/4] powerpc/powernv: Fix stale PE primary bus
  2016-02-09  4:50 ` [PATCH v2 2/4] powerpc/powernv: Fix stale PE " Gavin Shan
@ 2016-02-12  6:02   ` Andrew Donnellan
  2016-02-12  6:09     ` Gavin Shan
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Donnellan @ 2016-02-12  6:02 UTC (permalink / raw)
  To: Gavin Shan, linuxppc-dev, mpe

On 09/02/16 15:50, Gavin Shan wrote:
> When PCI bus is unplugged during full hotplug for EEH recovery,
> the platform PE instance (struct pnv_ioda_pe) isn't released and
> it dereferences the stale PCI bus that has been released. It leads
> to kernel crash when referring to the stale PCI bus.
>
> This fixes the issue by correcting the PE's primary bus when it's
> oneline at plugging time, in pnv_pci_dma_bus_setup() which is to
> be called by pcibios_fixup_bus().
>
> Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> Reported-by: Pradipta Ghosh <pradghos@in.ibm.com>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> Tested-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

I realise this has already been merged, but the following was found by 
Coverity:

> +void pnv_pci_dma_bus_setup(struct pci_bus *bus)
> +{
> +	struct pci_controller *hose = bus->sysdata;
> +	struct pnv_phb *phb = hose->private_data;
> +	struct pnv_ioda_pe *pe;
> +
> +	list_for_each_entry(pe, &phb->ioda.pe_list, list) {
> +		if (!(pe->flags | (PNV_IODA_PE_BUS | PNV_IODA_PE_BUS_ALL)))
> +			continue;

This condition is always false. I think the first "|" is supposed to be "&".

-- 
Andrew Donnellan              Software Engineer, OzLabs
andrew.donnellan@au1.ibm.com  Australia Development Lab, Canberra
+61 2 6201 8874 (work)        IBM Australia Limited

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/4] powerpc/powernv: Fix stale PE primary bus
  2016-02-12  6:02   ` Andrew Donnellan
@ 2016-02-12  6:09     ` Gavin Shan
  2016-02-15 10:38       ` Michael Ellerman
  0 siblings, 1 reply; 10+ messages in thread
From: Gavin Shan @ 2016-02-12  6:09 UTC (permalink / raw)
  To: Andrew Donnellan; +Cc: Gavin Shan, linuxppc-dev, mpe

On Fri, Feb 12, 2016 at 05:02:46PM +1100, Andrew Donnellan wrote:
>On 09/02/16 15:50, Gavin Shan wrote:
>>When PCI bus is unplugged during full hotplug for EEH recovery,
>>the platform PE instance (struct pnv_ioda_pe) isn't released and
>>it dereferences the stale PCI bus that has been released. It leads
>>to kernel crash when referring to the stale PCI bus.
>>
>>This fixes the issue by correcting the PE's primary bus when it's
>>oneline at plugging time, in pnv_pci_dma_bus_setup() which is to
>>be called by pcibios_fixup_bus().
>>
>>Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>>Reported-by: Pradipta Ghosh <pradghos@in.ibm.com>
>>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>>Tested-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>
>I realise this has already been merged, but the following was found by
>Coverity:
>
>>+void pnv_pci_dma_bus_setup(struct pci_bus *bus)
>>+{
>>+	struct pci_controller *hose = bus->sysdata;
>>+	struct pnv_phb *phb = hose->private_data;
>>+	struct pnv_ioda_pe *pe;
>>+
>>+	list_for_each_entry(pe, &phb->ioda.pe_list, list) {
>>+		if (!(pe->flags | (PNV_IODA_PE_BUS | PNV_IODA_PE_BUS_ALL)))
>>+			continue;
>
>This condition is always false. I think the first "|" is supposed to be "&".
>

Yeah, that should be "&". I think the problem isn't found when doing
testing in non-SRIOV environment. Thanks for pointing it out.

Michael, please let me know if I need send a follow-up revision to
correct this one? I found the first two patches have been put into
linux-next branch. I think we probably just need repost the correct
version for this one only.

Thanks,
Gavin

>-- 
>Andrew Donnellan              Software Engineer, OzLabs
>andrew.donnellan@au1.ibm.com  Australia Development Lab, Canberra
>+61 2 6201 8874 (work)        IBM Australia Limited

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/4] powerpc/powernv: Fix stale PE primary bus
  2016-02-12  6:09     ` Gavin Shan
@ 2016-02-15 10:38       ` Michael Ellerman
  2016-02-15 22:43         ` Gavin Shan
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Ellerman @ 2016-02-15 10:38 UTC (permalink / raw)
  To: Gavin Shan, Andrew Donnellan; +Cc: linuxppc-dev

On Fri, 2016-02-12 at 17:09 +1100, Gavin Shan wrote:

> On Fri, Feb 12, 2016 at 05:02:46PM +1100, Andrew Donnellan wrote:

> > On 09/02/16 15:50, Gavin Shan wrote:

> > > When PCI bus is unplugged during full hotplug for EEH recovery,
> > > the platform PE instance (struct pnv_ioda_pe) isn't released and
> > > it dereferences the stale PCI bus that has been released. It leads
> > > to kernel crash when referring to the stale PCI bus.
> > > 
> > > This fixes the issue by correcting the PE's primary bus when it's
> > > oneline at plugging time, in pnv_pci_dma_bus_setup() which is to
> > > be called by pcibios_fixup_bus().
> > > 
> > > Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> > > Reported-by: Pradipta Ghosh <pradghos@in.ibm.com>
> > > Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> > > Tested-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> > 
> > I realise this has already been merged, but the following was found by
> > Coverity:
> > 

> > > +void pnv_pci_dma_bus_setup(struct pci_bus *bus)
> > > +{
> > > +	struct pci_controller *hose = bus->sysdata;
> > > +	struct pnv_phb *phb = hose->private_data;
> > > +	struct pnv_ioda_pe *pe;
> > > +
> > > +	list_for_each_entry(pe, &phb->ioda.pe_list, list) {
> > > +		if (!(pe->flags | (PNV_IODA_PE_BUS | PNV_IODA_PE_BUS_ALL)))
> > > +			continue;
> > 
> > This condition is always false. I think the first "|" is supposed to be "&".
> > 
> 
> Yeah, that should be "&". I think the problem isn't found when doing
> testing in non-SRIOV environment. Thanks for pointing it out.
> 
> Michael, please let me know if I need send a follow-up revision to
> correct this one? I found the first two patches have been put into
> linux-next branch. I think we probably just need repost the correct
> version for this one only.

As it happens I needed to rebase my fixes branch anyway, so I've updated it and
rebased. No further action required :)

cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/4] powerpc/powernv: Fix stale PE primary bus
  2016-02-15 10:38       ` Michael Ellerman
@ 2016-02-15 22:43         ` Gavin Shan
  0 siblings, 0 replies; 10+ messages in thread
From: Gavin Shan @ 2016-02-15 22:43 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Gavin Shan, Andrew Donnellan, linuxppc-dev

On Mon, Feb 15, 2016 at 09:38:35PM +1100, Michael Ellerman wrote:
>On Fri, 2016-02-12 at 17:09 +1100, Gavin Shan wrote:
>
>> On Fri, Feb 12, 2016 at 05:02:46PM +1100, Andrew Donnellan wrote:
>
>> > On 09/02/16 15:50, Gavin Shan wrote:
>
>> > > When PCI bus is unplugged during full hotplug for EEH recovery,
>> > > the platform PE instance (struct pnv_ioda_pe) isn't released and
>> > > it dereferences the stale PCI bus that has been released. It leads
>> > > to kernel crash when referring to the stale PCI bus.
>> > > 
>> > > This fixes the issue by correcting the PE's primary bus when it's
>> > > oneline at plugging time, in pnv_pci_dma_bus_setup() which is to
>> > > be called by pcibios_fixup_bus().
>> > > 
>> > > Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>> > > Reported-by: Pradipta Ghosh <pradghos@in.ibm.com>
>> > > Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> > > Tested-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>> > 
>> > I realise this has already been merged, but the following was found by
>> > Coverity:
>> > 
>
>> > > +void pnv_pci_dma_bus_setup(struct pci_bus *bus)
>> > > +{
>> > > +	struct pci_controller *hose = bus->sysdata;
>> > > +	struct pnv_phb *phb = hose->private_data;
>> > > +	struct pnv_ioda_pe *pe;
>> > > +
>> > > +	list_for_each_entry(pe, &phb->ioda.pe_list, list) {
>> > > +		if (!(pe->flags | (PNV_IODA_PE_BUS | PNV_IODA_PE_BUS_ALL)))
>> > > +			continue;
>> > 
>> > This condition is always false. I think the first "|" is supposed to be "&".
>> > 
>> 
>> Yeah, that should be "&". I think the problem isn't found when doing
>> testing in non-SRIOV environment. Thanks for pointing it out.
>> 
>> Michael, please let me know if I need send a follow-up revision to
>> correct this one? I found the first two patches have been put into
>> linux-next branch. I think we probably just need repost the correct
>> version for this one only.
>
>As it happens I needed to rebase my fixes branch anyway, so I've updated it and
>rebased. No further action required :)
>

Thanks Michael. Sorry for the rebase :-)

Thanks,
Gavin

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v2, 4/4] powerpc/powernv: Simplify definitions of EEH debugfs handlers
  2016-02-09  4:50 ` [PATCH v2 4/4] powerpc/powernv: Simplify definitions of EEH debugfs handlers Gavin Shan
@ 2016-02-17 12:41   ` Michael Ellerman
  0 siblings, 0 replies; 10+ messages in thread
From: Michael Ellerman @ 2016-02-17 12:41 UTC (permalink / raw)
  To: Gavin Shan, linuxppc-dev; +Cc: Gavin Shan

On Tue, 2016-09-02 at 04:50:24 UTC, Gavin Shan wrote:
> The EEH debugfs handlers have same prototype. This introduces
> a macro to define them, then to simplify the code. No logical
> changes.
> 
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/ccc9662da5494a7c4ff5ed5d16

cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v2,3/4] powerpc/eeh: Reworked eeh_pe_bus_get()
  2016-02-09  4:50 ` [PATCH v2 3/4] powerpc/eeh: Reworked eeh_pe_bus_get() Gavin Shan
@ 2016-03-09 12:51   ` Michael Ellerman
  0 siblings, 0 replies; 10+ messages in thread
From: Michael Ellerman @ 2016-03-09 12:51 UTC (permalink / raw)
  To: Gavin Shan, linuxppc-dev; +Cc: Gavin Shan

On Tue, 2016-09-02 at 04:50:23 UTC, Gavin Shan wrote:
> The original implementation is ugly: unnecessary if statements and
> "out" tag. This reworks the function to avoid above weaknesses. No
> functional changes introduced.
> 
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/4eb0799ff93b54dd40c0c5bb06

cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-03-09 12:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-09  4:50 [PATCH v2 1/4] powerpc/eeh: Fix stale cached primary bus Gavin Shan
2016-02-09  4:50 ` [PATCH v2 2/4] powerpc/powernv: Fix stale PE " Gavin Shan
2016-02-12  6:02   ` Andrew Donnellan
2016-02-12  6:09     ` Gavin Shan
2016-02-15 10:38       ` Michael Ellerman
2016-02-15 22:43         ` Gavin Shan
2016-02-09  4:50 ` [PATCH v2 3/4] powerpc/eeh: Reworked eeh_pe_bus_get() Gavin Shan
2016-03-09 12:51   ` [v2,3/4] " Michael Ellerman
2016-02-09  4:50 ` [PATCH v2 4/4] powerpc/powernv: Simplify definitions of EEH debugfs handlers Gavin Shan
2016-02-17 12:41   ` [v2, " Michael Ellerman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.