[PATCH 1/3] powerpc/eeh: Fix PE state format

[PATCH 1/3] powerpc/eeh: Fix PE state format

[Gavin Shan]

Obviously I had wrong format given to the PE state output from
/sys/bus/pci/devices/xxxx/eeh_pe_state with some typoes, which
was introduced by commit 2013add4 ("powerpc/eeh: Show hex prefix
for PE state sysfs"). The patch fixes it up.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/eeh_sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/eeh_sysfs.c b/arch/powerpc/kernel/eeh_sysfs.c
index f19b1e5..1ceecdd 100644
--- a/arch/powerpc/kernel/eeh_sysfs.c
+++ b/arch/powerpc/kernel/eeh_sysfs.c
@@ -65,7 +65,7 @@ static ssize_t eeh_pe_state_show(struct device *dev,
 		return -ENODEV;
 
 	state = eeh_ops->get_state(edev->pe, NULL);
-	return sprintf(buf, "%0x08x %0x08x\n",
+	return sprintf(buf, "0x%08x 0x%08x\n",
 		       state, edev->pe->state);
 }
 
-- 
1.8.3.2

[PATCH 2/3] powerpc/powernv: Replace OPAL_DEASSERT_RESET with EEH_RESET_DEACTIVATE

[Gavin Shan]

The flag passed to ioda_eeh_phb_reset() should be EEH_RESET_DEACTIVATE,
which is translated to OPAL_DEASSERT_RESET or something else by the
EEH backend accordingly.

The patch replaces OPAL_DEASSERT_RESET with EEH_RESET_DEACTIVATE for
ioda_eeh_phb_reset().

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/pci-ioda.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index d035035..ba1b4cb 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -2102,7 +2102,7 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,
 	if (is_kdump_kernel()) {
 		pr_info("  Issue PHB reset ...\n");
 		ioda_eeh_phb_reset(hose, EEH_RESET_FUNDAMENTAL);
-		ioda_eeh_phb_reset(hose, OPAL_DEASSERT_RESET);
+		ioda_eeh_phb_reset(hose, EEH_RESET_DEACTIVATE);
 	}
 
 	/* Remove M64 resource if we can't configure it successfully */
-- 
1.8.3.2

Re: [2/3] powerpc/powernv: Replace OPAL_DEASSERT_RESET with EEH_RESET_DEACTIVATE

[Michael Ellerman]

On Mon, 2014-24-11 at 22:26:59 UTC, Gavin Shan wrote:
> The flag passed to ioda_eeh_phb_reset() should be EEH_RESET_DEACTIVATE,
> which is translated to OPAL_DEASSERT_RESET or something else by the
> EEH backend accordingly.
> 
> The patch replaces OPAL_DEASSERT_RESET with EEH_RESET_DEACTIVATE for
> ioda_eeh_phb_reset().

What is the symptom? Does it not work at all or is it just a cosmetic issue?
ie. should I send this to Linus ASAP or can it wait for the next release.

Looks like this went in as 361f2a2a1536 ("powrpc/powernv: Reset PHB in kdump
kernel") in 3.16. Should it go to stable?

cheers

Re: [2/3] powerpc/powernv: Replace OPAL_DEASSERT_RESET with EEH_RESET_DEACTIVATE

[Gavin Shan]

On Wed, Nov 26, 2014 at 03:07:51PM +1100, Michael Ellerman wrote:
>On Mon, 2014-24-11 at 22:26:59 UTC, Gavin Shan wrote:
>> The flag passed to ioda_eeh_phb_reset() should be EEH_RESET_DEACTIVATE,
>> which is translated to OPAL_DEASSERT_RESET or something else by the
>> EEH backend accordingly.
>> 
>> The patch replaces OPAL_DEASSERT_RESET with EEH_RESET_DEACTIVATE for
>> ioda_eeh_phb_reset().
>
>What is the symptom? Does it not work at all or is it just a cosmetic issue?
>ie. should I send this to Linus ASAP or can it wait for the next release.
>
>Looks like this went in as 361f2a2a1536 ("powrpc/powernv: Reset PHB in kdump
>kernel") in 3.16. Should it go to stable?
>

It's just a cosmetic issue and please pick it up for next release.

Thanks,
Gavin

>cheers
>

[PATCH 3/3] powerpc/eeh: Fix missed PE#0 on P7IOC

[Gavin Shan]

PE#0 should be regarded as valid for P7IOC, while it's invalid for
PHB3. The patch adds flag EEH_VALID_PE_ZERO to differentiate those
two cases. Without the patch, we possibly see frozen PE#0 state is
cleared without EEH recovery taken on P7IOC as following kernel logs
indicate:

[root@ltcfbl8eb ~]# dmesg
       :
pci 0000:00     : [PE# 000] Secondary bus 0 associated with PE#0
pci 0000:01     : [PE# 001] Secondary bus 1 associated with PE#1
pci 0001:00     : [PE# 000] Secondary bus 0 associated with PE#0
pci 0001:01     : [PE# 001] Secondary bus 1 associated with PE#1
pci 0002:00     : [PE# 000] Secondary bus 0 associated with PE#0
pci 0002:01     : [PE# 001] Secondary bus 1 associated with PE#1
pci 0003:00     : [PE# 000] Secondary bus 0 associated with PE#0
pci 0003:01     : [PE# 001] Secondary bus 1 associated with PE#1
pci 0003:20     : [PE# 002] Secondary bus 32..63 associated with PE#2
       :
EEH: Clear non-existing PHB#3-PE#0
EEH: PHB location: U78AE.001.WZS00M9-P1-002

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/eeh.h               |  5 +++--
 arch/powerpc/kernel/eeh_pe.c                 | 14 +++++++++++---
 arch/powerpc/platforms/powernv/eeh-powernv.c | 11 +++++++++++
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 0652ebe..9c11d1e 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -38,8 +38,9 @@ struct device_node;
 #define EEH_FORCE_DISABLED	0x02	/* EEH disabled		*/
 #define EEH_PROBE_MODE_DEV	0x04	/* From PCI device	*/
 #define EEH_PROBE_MODE_DEVTREE	0x08	/* From device tree	*/
-#define EEH_ENABLE_IO_FOR_LOG	0x10	/* Enable IO for log	*/
-#define EEH_EARLY_DUMP_LOG	0x20	/* Dump log immediately	*/
+#define EEH_VALID_PE_ZERO	0x10	/* PE#0 is valid	*/
+#define EEH_ENABLE_IO_FOR_LOG	0x20	/* Enable IO for log	*/
+#define EEH_EARLY_DUMP_LOG	0x40	/* Dump log immediately	*/
 
 /*
  * Delay for PE reset, all in ms
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 5a63e2b..fa950fb 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -239,10 +239,18 @@ static void *__eeh_pe_get(void *data, void *flag)
 	if (pe->type & EEH_PE_PHB)
 		return NULL;
 
-	/* We prefer PE address */
-	if (edev->pe_config_addr &&
-	   (edev->pe_config_addr == pe->addr))
+	/*
+	 * We prefer PE address. For most cases, we should
+	 * have non-zero PE address
+	 */
+	if (eeh_has_flag(EEH_VALID_PE_ZERO)) {
+		if (edev->pe_config_addr == pe->addr)
+			return pe;
+	} else {
+		if (edev->pe_config_addr &&
+		    (edev->pe_config_addr == pe->addr))
 		return pe;
+	}
 
 	/* Try BDF address */
 	if (edev->config_addr &&
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 1d19e79..e261869 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -68,6 +68,17 @@ static int powernv_eeh_init(void)
 
 		if (phb->model == PNV_PHB_MODEL_P7IOC)
 			eeh_add_flag(EEH_ENABLE_IO_FOR_LOG);
+
+		/*
+		 * PE#0 should be regarded as valid by EEH core
+		 * if it's not the reserved one. Currently, we
+		 * have the reserved PE#0 and PE#127 for PHB3
+		 * and P7IOC separately. So we should regard
+		 * PE#0 as valid for P7IOC.
+		 */
+		if (phb->ioda.reserved_pe != 0)
+			eeh_add_flag(EEH_VALID_PE_ZERO);
+
 		break;
 	}
 
-- 
1.8.3.2

Re: [3/3] powerpc/eeh: Fix missed PE#0 on P7IOC

[Michael Ellerman]

On Mon, 2014-24-11 at 22:27:00 UTC, Gavin Shan wrote:
> PE#0 should be regarded as valid for P7IOC, while it's invalid for
> PHB3. The patch adds flag EEH_VALID_PE_ZERO to differentiate those
> two cases. Without the patch, we possibly see frozen PE#0 state is
> cleared without EEH recovery taken on P7IOC as following kernel logs
> indicate:

This sounds like an urgent fix but I'm not sure.

cheers

Re: [3/3] powerpc/eeh: Fix missed PE#0 on P7IOC

[Gavin Shan]

On Wed, Nov 26, 2014 at 03:09:10PM +1100, Michael Ellerman wrote:
>On Mon, 2014-24-11 at 22:27:00 UTC, Gavin Shan wrote:
>> PE#0 should be regarded as valid for P7IOC, while it's invalid for
>> PHB3. The patch adds flag EEH_VALID_PE_ZERO to differentiate those
>> two cases. Without the patch, we possibly see frozen PE#0 state is
>> cleared without EEH recovery taken on P7IOC as following kernel logs
>> indicate:
>
>This sounds like an urgent fix but I'm not sure.
>

You can pick it up for next release. Usually, we shouldn't see errors
from PE#0. Something I did to force that for testing purpose: disable
memory windows of the root complex and then access one of memory BARs
on one adapter behind the root complex.

Thanks,
Gavin

>cheers
>