kern/130512: [patch] Various mistakes in IPMI watchdog handling

[Dmitrij Tejblum]

>Number:         130512
>Category:       kern
>Synopsis:       [patch] Various mistakes in IPMI watchdog handling
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Tue Jan 13 18:40:00 UTC 2009
>Closed-Date:
>Last-Modified:
>Originator:     Dmitrij Tejblum
>Release:        FreeBSD 7.1-PRERELEASE i386
>Organization:
OOO Yandex
>Environment:

>Description:

Watchdog handling in the ipmi driver has several problems.

1. Incorrect timeout setting:
To get the most significant byte of a 2-byte number (i.e. sec*10) you 
should divide the number by 256, not 2550.
Also, a second contains 1000000000 nanoseconds, not 1800000000 nanoseconds.
(This change is verified by the "ipmitool" program and by testing when the 
watchdog actually fire.)

2. Due to rounding error, setting watchdog to a really small timeout 
(< 1 sec) was turning the watchdog off. It should set the watchdog to 
a small timeout instead.

3. The error checking was missed.

>How-To-Repeat:

Use ipmitool program from ports/sysutils/ipmitool
(ipmitool bmc watchdog get) to see actual watchdog settings.
Use different values of '-s' option to watchdogd to see when the timeout
actually fires.

>Fix:

--- dev/ipmi/ipmi.c	2008-10-13 18:43:46.000000000 +0400
+++ dev/ipmi/ipmi.c	2009-01-13 21:15:49.000000000 +0300
@@ -588,7 +588,7 @@
  * Watchdog event handler.
  */
 
-static void
+static int
 ipmi_set_watchdog(struct ipmi_softc *sc, int sec)
 {
 	struct ipmi_request *req;
@@ -604,7 +604,7 @@
 		req->ir_request[2] = 0;
 		req->ir_request[3] = 0;	/* Timer use */
 		req->ir_request[4] = (sec * 10) & 0xff;
-		req->ir_request[5] = (sec * 10) / 2550;
+		req->ir_request[5] = (sec * 10) / 256;
 	} else {
 		req->ir_request[0] = IPMI_SET_WD_TIMER_SMS_OS;
 		req->ir_request[1] = 0;
@@ -631,6 +631,7 @@
 	}
 
 	ipmi_free_request(req);
+	return error;
 	/*
 	dump_watchdog(sc);
 	*/
@@ -641,14 +642,22 @@
 {
 	struct ipmi_softc *sc = arg;
 	unsigned int timeout;
+	int e;
 
 	cmd &= WD_INTERVAL;
 	if (cmd > 0 && cmd <= 63) {
-		timeout = ((uint64_t)1 << cmd) / 1800000000;
-		ipmi_set_watchdog(sc, timeout);
-		*error = 0;
+		timeout = ((uint64_t)1 << cmd) / 1000000000;
+		if (timeout == 0)
+			timeout = 1;
+		e = ipmi_set_watchdog(sc, timeout);
+		if (e == 0)
+			*error = 0;
+		else
+			ipmi_set_watchdog(sc, 0);
 	} else {
-		ipmi_set_watchdog(sc, 0);
+		e = ipmi_set_watchdog(sc, 0);
+		if (e != 0)
+			*error = EOPNOTSUPP;
 	}
 }
 
>Release-Note:
>Audit-Trail:
>Unformatted: