[PATCH] pSeries: EEH improperly enabled for some Power4 systems

Paul,

The attached patch seems like a late-arriving candidate for 2.6.20
I've had it floating around for a few weeks, and should have  sent it
earler.

--linas

It appears that EEH is improperly enabled for some Power4 systems.
On these systems, the ibm,set-eeh-option returns a value of success
even when EEH is not supported on the given node. Thus, an explicit
check for support is required.

Signed-off-by: Linas Vepstas <redacted> 

----
 arch/powerpc/platforms/pseries/eeh.c |   19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

Index: linux-2.6.20-rc4/arch/powerpc/platforms/pseries/eeh.c
===================================================================

--- linux-2.6.20-rc4.orig/arch/powerpc/platforms/pseries/eeh.c	2007-01-11 14:15:02.000000000 -0600
+++ linux-2.6.20-rc4/arch/powerpc/platforms/pseries/eeh.c	2007-01-11 15:14:39.000000000 -0600

@@ -748,6 +748,7 @@ struct eeh_early_enable_info {
 /* Enable eeh for the given device node. */
 static void *early_enable_eeh(struct device_node *dn, void *data)
 {
+	unsigned int rets[3];
 	struct eeh_early_enable_info *info = data;
 	int ret;
 	const char *status = get_property(dn, "status", NULL);

@@ -804,16 +805,14 @@ static void *early_enable_eeh(struct dev
 		                regs[0], info->buid_hi, info->buid_lo,
 		                EEH_ENABLE);
 
+		enable = 0;
 		if (ret == 0) {
-			eeh_subsystem_enabled = 1;
-			pdn->eeh_mode |= EEH_MODE_SUPPORTED;
 			pdn->eeh_config_addr = regs[0];
 
 			/* If the newer, better, ibm,get-config-addr-info is supported, 
 			 * then use that instead. */
 			pdn->eeh_pe_config_addr = 0;
 			if (ibm_get_config_addr_info != RTAS_UNKNOWN_SERVICE) {
-				unsigned int rets[2];
 				ret = rtas_call (ibm_get_config_addr_info, 4, 2, rets, 
 					pdn->eeh_config_addr, 
 					info->buid_hi, info->buid_lo,

@@ -821,6 +820,20 @@ static void *early_enable_eeh(struct dev
 				if (ret == 0)
 					pdn->eeh_pe_config_addr = rets[0];
 			}
+
+			/* Some older systems (Power4) allow the
+			 * ibm,set-eeh-option call to succeed even on nodes
+			 * where EEH is not supported. Verify support
+			 * explicitly. */
+			ret = read_slot_reset_state(pdn, rets);
+			if ((ret == 0) && (rets[1] == 1))
+				enable = 1;
+		}
+
+		if (enable) {
+			eeh_subsystem_enabled = 1;
+			pdn->eeh_mode |= EEH_MODE_SUPPORTED;
+
 #ifdef DEBUG
 			printk(KERN_DEBUG "EEH: %s: eeh enabled, config=%x pe_config=%x\n",
 			       dn->full_name, pdn->eeh_config_addr, pdn->eeh_pe_config_addr);

Linas Vepstas writes:

It appears that EEH is improperly enabled for some Power4 systems.
On these systems, the ibm,set-eeh-option returns a value of success
even when EEH is not supported on the given node. Thus, an explicit
check for support is required.

What happens on the power4 systems when EEH is improperly enabled?

What systems has the patch been tested on?

Thanks,
Paul.

On Sat, Jan 27, 2007 at 09:30:57AM +1100, Paul Mackerras wrote:

Linas Vepstas writes:

quoted

It appears that EEH is improperly enabled for some Power4 systems.
On these systems, the ibm,set-eeh-option returns a value of success
even when EEH is not supported on the given node. Thus, an explicit
check for support is required.

What happens on the power4 systems when EEH is improperly enabled?

What systems has the patch been tested on?

Sorry, I should have said more from the get-go.

During boot, on power4, without this patch, one sees messages 
similar to:

EEH: event on unsupported device, rc=0 dn=/pci@400000000110/IBM,sp@1
EEH: event on unsupported device, rc=0 dn=/pci@400000000110/pci@2
EEH: event on unsupported device, rc=0 dn=/pci@400000000110/pci@2,2
etc.

The patch makes these go away.

Without this patch, EEH recovery does seem to work correctly for 
at least some devices (I tested ethernet e1000), but fails to 
recover others (the Emulex LightPulse LPFC, most notably). 
Off the top of my head, I don't remember why some devices are 
affected, but not others.

The PAPR indicates that the correct way to test for EEH is as 
done in this patch; its not clear to me if this was in the PAPR 
all along, or recently added; if it was there all along, its not
clear to me why this hadn't been fixed long ago. I suspect only
certain firmware levels are affected.

I've tested on one power4 and one power5; both have "old" 
firmware (firmware dating back to not long after product 
announce). It sure would be nice to test on more machines, huh? 
I don't know how to quickly test on a broad spectrum of machines.

If this makes you nervous, I suppose this patch can wait for
the 2.6.21 series.

--linas