When I played with ipmi, I found problems in the initializing
watchdog.
As far as my test on HPE DL20 Gen10, the ipmi device refuses "set
watchdog timer" command if "use" value (in first byte) is none. Since
existing code uses the value which read from the device, if the value
is not set on the device, the command actually fails. The diff makes
the driver set the value IPMI_WDOG_USE_SMS_OS explicitly always.
Also existing code ignores error when "set watchdog timer" fail. I
think this is no good and some actual problems can happen.
ok?
Set "use" bits to "SMS/OS" always to prevent error. Actually "set
watchdog timer" fails on HP DL20 Gen10 with ccode 0xCC (invalid
request) if the bits was not set in advance. Also don't update the
timer period if setting watchdog timer is failed.
Index: sys/dev/ipmi.c
===================================================================
RCS file: /var/cvs/openbsd/src/sys/dev/ipmi.c,v
retrieving revision 1.109
diff -u -p -r1.109 ipmi.c
--- sys/dev/ipmi.c 18 Feb 2020 00:06:12 -0000 1.109
+++ sys/dev/ipmi.c 2 Mar 2020 05:39:34 -0000
@@ -130,7 +130,7 @@ void ipmi_cmd_wait_cb(void *);
int ipmi_watchdog(void *, int);
void ipmi_watchdog_tickle(void *);
-void ipmi_watchdog_set(void *);
+int ipmi_watchdog_set(void *, int);
struct ipmi_softc *ipmilookup(dev_t dev);
@@ -1741,8 +1741,9 @@ ipmi_watchdog(void *arg, int period)
if (period < MIN_PERIOD && period > 0)
period = MIN_PERIOD;
+ if (ipmi_watchdog_set(sc, period) == -1)
+ return (sc->sc_wdog_period);
sc->sc_wdog_period = period;
- ipmi_watchdog_set(sc);
printf("%s: watchdog %sabled\n", DEVNAME(sc),
(period == 0) ? "dis" : "en");
return (period);
@@ -1766,8 +1767,8 @@ ipmi_watchdog_tickle(void *arg)
ipmi_cmd(&c);
}
-void
-ipmi_watchdog_set(void *arg)
+int
+ipmi_watchdog_set(void *arg, int period)
{
struct ipmi_softc *sc = arg;
uint8_t wdog[IPMI_GET_WDOG_MAX];
@@ -1785,14 +1786,15 @@ ipmi_watchdog_set(void *arg)
ipmi_cmd(&c);
/* Period is 10ths/sec */
- uint16_t timo = htole16(sc->sc_wdog_period * 10);
+ uint16_t timo = htole16(period * 10);
memcpy(&wdog[IPMI_SET_WDOG_TIMOL], &timo, 2);
- wdog[IPMI_SET_WDOG_TIMER] &= ~IPMI_WDOG_DONTSTOP;
- wdog[IPMI_SET_WDOG_TIMER] |= (sc->sc_wdog_period == 0) ?
- 0 : IPMI_WDOG_DONTSTOP;
+ wdog[IPMI_SET_WDOG_TIMER] &= ~(IPMI_WDOG_DONTSTOP | IPMI_WDOG_USE_MASK);
+ wdog[IPMI_SET_WDOG_TIMER] |= IPMI_WDOG_USE_SMS_OS;
+ if (period != 0)
+ wdog[IPMI_SET_WDOG_TIMER] |= IPMI_WDOG_DONTSTOP;
wdog[IPMI_SET_WDOG_ACTION] &= ~IPMI_WDOG_MASK;
- wdog[IPMI_SET_WDOG_ACTION] |= (sc->sc_wdog_period == 0) ?
+ wdog[IPMI_SET_WDOG_ACTION] |= (period == 0) ?
IPMI_WDOG_DISABLED : IPMI_WDOG_REBOOT;
c.c_cmd = APP_SET_WATCHDOG_TIMER;
@@ -1801,6 +1803,13 @@ ipmi_watchdog_set(void *arg)
c.c_rxlen = 0;
c.c_data = wdog;
ipmi_cmd(&c);
+ if (c.c_ccode != 0) {
+ printf("%s: set watchdog timer failed: %.2x", DEVNAME(sc),
+ c.c_ccode);
+ return (-1);
+ }
+
+ return (0);
}
#if defined(__amd64__) || defined(__i386__)
Index: sys/dev/ipmivar.h
===================================================================
RCS file: /var/cvs/openbsd/src/sys/dev/ipmivar.h,v
retrieving revision 1.32
diff -u -p -r1.32 ipmivar.h
--- sys/dev/ipmivar.h 19 Dec 2019 09:01:50 -0000 1.32
+++ sys/dev/ipmivar.h 2 Mar 2020 05:39:34 -0000
@@ -136,6 +136,11 @@ struct ipmi_thread {
};
#define IPMI_WDOG_DONTSTOP 0x40
+#define IPMI_WDOG_USE_MASK 0x07
+#define IPMI_WDOG_USE_BIOS_FRB2 1
+#define IPMI_WDOG_USE_BIOS_POST 2
+#define IPMI_WDOG_USE_OS_LOAD 3
+#define IPMI_WDOG_USE_SMS_OS 4
#define IPMI_WDOG_MASK 0x03
#define IPMI_WDOG_DISABLED 0x00
