Skip to contentSkip to conversation

2.6 PPC64: Another log buffer length patch.

PATCH In review Linas Vepstas 2 participants 3 messages Aug 25, 2004 – Aug 27, 2004

lore
Share & tools
lore

Linas Vepstas

author
#0 lore raw

PATCH 1/1 2.6 PPC64: Another log buffer length patch.

Paul, Anton,

During the last round of patches fixing the RTAS buffer length, the due diligence wasn't as diligent as it should have been. Here's one more. Please review and forward upstream.

This patch uses the firmware-defined error log buffer length
for calls to the firmware routine 'check-exception'. It also
simplifies code in rtasd.c that is attempting to obtain the error log length.

Signed-off-by Linas Vepstas <[email protected]>

--linas

p.s. please be sure to cc me directly on replies, it seems I *still* cannot get email from the linuxppc64-dev mailing list.

===== arch/ppc64/kernel/ras.c 1.15 vs edited =====

arch/ppc64/kernel/ras.c +5 -2
@@ -108,6 +108,7 @@
 
 	ras_get_sensor_state_token = rtas_token("get-sensor-state");
 	ras_check_exception_token = rtas_token("check-exception");
+	rtas_get_error_log_max();
 
 	/* Internal Errors */
 	np = of_find_node_by_path("/event-sources/internal-errors");

@@ -161,7 +162,8 @@
 			   RAS_VECTOR_OFFSET,
 			   virt_irq_to_real(irq_offset_down(irq)),
 			   RTAS_EPOW_WARNING | RTAS_POWERMGM_EVENTS,
-			   critical, __pa(&ras_log_buf), RTAS_ERROR_LOG_MAX);
+			   critical, __pa(&ras_log_buf), 
+				rtas_get_error_log_max());
 
 	udbg_printf("EPOW <0x%lx 0x%x 0x%x>\n",
 		    *((unsigned long *)&ras_log_buf), status, state);

@@ -196,7 +198,8 @@
 			   RAS_VECTOR_OFFSET,
 			   virt_irq_to_real(irq_offset_down(irq)),
 			   RTAS_INTERNAL_ERROR, 1 /*Time Critical */,
-			   __pa(&ras_log_buf), RTAS_ERROR_LOG_MAX);
+			   __pa(&ras_log_buf), 
+				rtas_get_error_log_max());
 
 	rtas_elog = (struct rtas_error_log *)ras_log_buf;

===== arch/ppc64/kernel/rtasd.c 1.26 vs edited =====

arch/ppc64/kernel/rtasd.c +1 -14
@@ -318,21 +318,8 @@
 	rtas_event_scan_rate = *ip;
 	DEBUG("rtas-event-scan-rate %d\n", rtas_event_scan_rate);
 
-	ip = (int *)get_property(node, "rtas-error-log-max", NULL);
-	if (ip == NULL) {
-		printk(KERN_ERR "rtasd: no rtas-error-log-max\n");
-		of_node_put(node);
-		return -1;
-	}
-	rtas_error_log_max = *ip;
-	DEBUG("rtas-error-log-max %d\n", rtas_error_log_max);
-
-	if (rtas_error_log_max > RTAS_ERROR_LOG_MAX) {
-		printk(KERN_ERR "rtasd: truncated error log from %d to %d bytes\n", rtas_error_log_max, RTAS_ERROR_LOG_MAX);
-		rtas_error_log_max = RTAS_ERROR_LOG_MAX;
-	}
-
 	/* Make room for the sequence number */
+	rtas_error_log_max = rtas_get_error_log_max();
 	rtas_error_log_buffer_max = rtas_error_log_max + sizeof(int);
 
 	of_node_put(node);

===== include/asm-ppc64/rtas.h 1.20 vs edited =====

include/asm-ppc64/rtas.h +21 -2
@@ -1,6 +1,7 @@
 #ifndef _PPC64_RTAS_H
 #define _PPC64_RTAS_H
 
+#include <linux/kernel.h>
 #include <linux/spinlock.h>
 #include <asm/page.h>
 

@@ -199,8 +200,26 @@
 #define RTAS_DEBUG KERN_DEBUG "RTAS: "
  
 #define RTAS_ERROR_LOG_MAX 2048
- 
- 
+    
+/** Return the firmware-specified size of the error log buffer
+ *  for all rtas calls that require an error buffer argument.
+ *  This includes 'check-exception' and 'rtas-last-error'.
+ */
+static inline int rtas_get_error_log_max (void)
+{
+	static int rtas_error_log_max;
+	if (rtas_error_log_max)
+		return rtas_error_log_max;
+	
+	rtas_error_log_max = rtas_token ("rtas-error-log-max");
+	if ((rtas_error_log_max == RTAS_UNKNOWN_SERVICE) ||
+	    (rtas_error_log_max > RTAS_ERROR_LOG_MAX)) {
+		printk (KERN_WARNING "RTAS: bad log buffer size %d\n", rtas_error_log_max);
+		rtas_error_log_max = RTAS_ERROR_LOG_MAX;
+	}
+	return rtas_error_log_max;
+}
+    
 /* Event Scan Parameters */
 #define EVENT_SCAN_ALL_EVENTS	0xf0000000
 #define SURVEILLANCE_TOKEN	9000
#1 lore raw

Linas,

arch/ppc64/kernel/ras.c line 111

Quoted 8 lines

===== arch/ppc64/kernel/ras.c 1.15 vs edited =====

arch/ppc64/kernel/ras.c +1 -0
@@ -108,6 +108,7 @@
 
 	ras_get_sensor_state_token = rtas_token("get-sensor-state");
 	ras_check_exception_token = rtas_token("check-exception");
+	rtas_get_error_log_max();

Why do we do this call, given that we don't use the result? Is there something time-critical about some future call, such that we want to have fetched the value at this point? If there is, it needs a comment here, if not, let's remove this call.

Regards,
Paul.

author
#2 lore raw

On Thu, Aug 26, 2004 at 12:09:31PM +1000, Paul Mackerras was heard to remark:

Quoted 8 lines

Linas,

Quoted 5 lines

===== arch/ppc64/kernel/ras.c 1.15 vs edited =====

arch/ppc64/kernel/ras.c +0 -0
@@ -108,6 +108,7 @@
Quoted 8 lines

arch/ppc64/kernel/ras.c line 111

Quoted 3 lines
arch/ppc64/kernel/ras.c +1 -0
 	ras_get_sensor_state_token = rtas_token("get-sensor-state");
 	ras_check_exception_token = rtas_token("check-exception");
+	rtas_get_error_log_max();

Why do we do this call, given that we don't use the result? Is there something time-critical about some future call, such that we want to have fetched the value at this point? If there is, it needs a comment here, if not, let's remove this call.

Clearly, I'm being too clever, and too conservative, belt-n-suspenders. I was actually expecting that there might be objections, cause the usage isn't 'typical' :)

The call sets a global var. I figured that it would be best to just go ahead and initialize it early, rather than hoping it all works at our
moment of 'dire need'. Removing this call shouldn't affect operation,
except during some bizarro failure situation which can only be imagined after some creative thinking.

--linas