From owner-freebsd-current@FreeBSD.ORG Sun Dec 10 10:04:43 2006 Return-Path: X-Original-To: current@FreeBSD.ORG Delivered-To: freebsd-current@FreeBSD.ORG Received: from mx1.FreeBSD.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 3329416A40F for ; Sun, 10 Dec 2006 10:04:43 +0000 (UTC) (envelope-from nick@van-laarhoven.org) Received: from smtp-4.orange.nl (smtp-4.orange.nl [193.252.22.249]) by mx1.FreeBSD.org (Postfix) with ESMTP id 24AE243C9E for ; Sun, 10 Dec 2006 10:03:33 +0000 (GMT) (envelope-from nick@van-laarhoven.org) Received: from me-wanadoo.net (localhost [127.0.0.1]) by mwinf6309.orange.nl (SMTP Server) with ESMTP id 2A34C1C00083 for ; Sun, 10 Dec 2006 11:04:41 +0100 (CET) Received: from uitsmijter.van-laarhoven.org (ap-zvhz-13f05.adsl.wanadoo.nl [81.69.93.5]) by mwinf6309.orange.nl (SMTP Server) with ESMTP id CD9A21C00081 for ; Sun, 10 Dec 2006 11:04:40 +0100 (CET) X-ME-UUID: 20061210100440842.CD9A21C00081@mwinf6309.orange.nl Received: (qmail 22902 invoked from network); 10 Dec 2006 10:04:40 -0000 Received: from 10.66.0.143 by uitsmijter.van-laarhoven.org (envelope-from , uid 82) with qmail-scanner-1.25 (clamdscan: 0.88.4/2187. f-prot: 4.6.6/3.16.14. spamassassin: 3.1.7. Clear:RC:1(10.66.0.143):. Processed in 0.537084 secs); 10 Dec 2006 10:04:40 -0000 X-Qmail-Scanner-Mail-From: nick@van-laarhoven.org via uitsmijter.van-laarhoven.org X-Qmail-Scanner: 1.25 (Clear:RC:1(10.66.0.143):. Processed in 0.537084 secs) Received: from unknown (HELO van-laarhoven.org) (nick@10.66.0.143) by uitsmijter.van-laarhoven.org with SMTP; 10 Dec 2006 10:04:39 -0000 Received: (nullmailer pid 48569 invoked by uid 1001); Sun, 10 Dec 2006 10:04:39 -0000 Date: Sun, 10 Dec 2006 11:04:39 +0100 (CET) From: Nick Hibma X-X-Sender: nick@localhost To: FreeBSD CURRENT Mailing List Message-ID: <20061210110419.H42195@localhost> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: Subject: Slight interface change on the watchdog fido X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 10 Dec 2006 10:04:43 -0000 I'm planning on committing the following change to make the implementations of the various hardware watchdogs more consistent. A timeout of 0 passed to the ioctl is now a valid input and will disable the watchdog. Previously it produced an error for the Elan chip watchdog, which is _not_ what you want to see when disarming a watchdog :-) I'd appreciate review of the individual changes in the files by the following people. The change as a whole was discussed with phk. cognet@freebsd.org i80321_wdog.c (*) des ichwd.c (**) ambrisko ipmi.c marius mk48txx.c phk kern_clock.c, elan-mmcr.c, watchdog.c (**) (*) The i80321_wdog.c cannot be disarmed. Is this correct? (**) These have been tested to arm, disarm, and fire. Others have only been compile tested. This change has been tested on 6.2-STABLE and 7-CURRENT. Commit message: Convert all watchdog event handlers to the following format: - If the timeout value passed is >0 and acceptable arm the watchdog and set the *error to 0 (a watchdog is armed). - Otherwise disarm the watchdog and leave *error as is (valid disarm). - If the timeout value passed is 0, disarm the watchdog. set *error to an error if disarm failed. The default of *error in the ioctl is a failed arm and a succesful disarm. Also: - Return an error if WD_PASSIVE is passed in to the ioctl as only WD_ACTIVE is implemented at the moment. See sys/watchdog.h for an explanation of the difference between WD_ACTIVE and WD_PASSIVE. - Remove the I_HAVE_TOTALLY_LOST_MY_SENSE_OF_HUMOR define. If you've lost your sense of humor, than don't add a define. i80321_wdog.c Don't roll your own passive watchdog tickle as this would defeat the purpose of an active (userland) watchdog tickle. ichwd.c / ipmi.c: WD_ACTIVE means userland pats of the watchdog, not whether the watchdog is active. See sys/watchdog.h. kern_clock.c: Remove a check for WD_ACTIVE as this does not make sense here. This reverts r1.181, as that commit doesn't make sense. Index: sys/arm/xscale/i80321/i80321_wdog.c =================================================================== RCS file: /home/ncvs/src/sys/arm/xscale/i80321/i80321_wdog.c,v retrieving revision 1.2 diff -u -r1.2 i80321_wdog.c --- sys/arm/xscale/i80321/i80321_wdog.c 15 Jan 2005 18:38:10 -0000 1.2 +++ sys/arm/xscale/i80321/i80321_wdog.c 9 Dec 2006 12:40:40 -0000 @@ -62,7 +62,6 @@ device_t dev; int armed; int wdog_period; - struct callout_handle wdog_callout; }; static __inline void @@ -83,8 +82,6 @@ return; wdtcr_write(WDTCR_ENABLE1); wdtcr_write(WDTCR_ENABLE2); - sc->wdog_callout = timeout(iopwdog_tickle, sc, - hz * (sc->wdog_period - 1)); } static int @@ -112,14 +109,19 @@ { struct iopwdog_softc *sc = private; - if (cmd == 0) - return; - if ((((uint64_t)1 << (cmd & WD_INTERVAL))) > - (uint64_t)sc->wdog_period * 1000000000) - return; - sc->armed = 1; - iopwdog_tickle(sc); - *error = 0; + cmd &= WD_INTERVAL; + if (cmd > 0 && cmd <= 63 + && (uint64_t)1 << (cmd & WD_INTERVAL) <= + (uint64_t)sc->wdog_period * 1000000000) { + /* Valid value -> Enable watchdog */ + iopwdog_tickle(sc); + sc->armed = 1; + *error = 0; + } else { + /* XXX Can't disable this watchdog? */ + if (sc->armed) + *error = EOPNOTSUPP; + } } static int Index: sys/dev/ichwd/ichwd.c =================================================================== RCS file: /home/ncvs/src/sys/dev/ichwd/ichwd.c,v retrieving revision 1.5.2.1 diff -u -r1.5.2.1 ichwd.c --- sys/dev/ichwd/ichwd.c 15 Jun 2006 15:15:07 -0000 1.5.2.1 +++ sys/dev/ichwd/ichwd.c 9 Dec 2006 12:40:46 -0000 @@ -178,38 +178,20 @@ struct ichwd_softc *sc = arg; unsigned int timeout; + /* convert from power-of-two-ns to WDT ticks */ + cmd &= WD_INTERVAL; + timeout = ((uint64_t)1 << cmd) / ICHWD_TICK; + if (cmd > 0 && cmd <= 63 + && timeout >= ICHWD_MIN_TIMEOUT && timeout <= ICHWD_MAX_TIMEOUT) { + if (timeout != sc->timeout) + ichwd_tmr_set(sc, timeout); - /* disable / enable */ - if (!(cmd & WD_ACTIVE)) { + ichwd_tmr_reload(sc); + *error = 0; + } else { if (sc->active) ichwd_tmr_disable(sc); - *error = 0; - return; } - if (!sc->active) - ichwd_tmr_enable(sc); - - cmd &= WD_INTERVAL; - /* convert from power-of-to-ns to WDT ticks */ - if (cmd >= 64) { - *error = EINVAL; - return; - } - timeout = ((uint64_t)1 << cmd) / ICHWD_TICK; - if (timeout < ICHWD_MIN_TIMEOUT || timeout > ICHWD_MAX_TIMEOUT) { - *error = EINVAL; - return; - } - - /* set new initial value */ - if (timeout != sc->timeout) - ichwd_tmr_set(sc, timeout); - - /* reload */ - ichwd_tmr_reload(sc); - - *error = 0; - return; } static unsigned int pmbase = 0; Index: sys/dev/ipmi/ipmi.c =================================================================== RCS file: /home/ncvs/src/sys/dev/ipmi/ipmi.c,v retrieving revision 1.3.2.3 diff -u -r1.3.2.3 ipmi.c --- sys/dev/ipmi/ipmi.c 19 Oct 2006 14:50:48 -0000 1.3.2.3 +++ sys/dev/ipmi/ipmi.c 9 Dec 2006 12:40:47 -0000 @@ -649,25 +649,14 @@ struct ipmi_softc *sc = arg; unsigned int timeout; - /* disable / enable */ - if (!(cmd & WD_ACTIVE)) { - ipmi_set_watchdog(sc, 0); - *error = 0; - return; - } - cmd &= WD_INTERVAL; - /* convert from power-of-to-ns to WDT ticks */ - if (cmd >= 64) { - *error = EINVAL; - return; + if (cmd > 0 && cmd <= 63) { + timeout = ((uint64_t)1 << cmd) / 1800000000; + ipmi_set_watchdog(sc, timeout); + *error = 0; + } else { + ipmi_set_watchdog(sc, 0); } - timeout = ((uint64_t)1 << cmd) / 1800000000; - - /* reload */ - ipmi_set_watchdog(sc, timeout); - - *error = 0; } #ifdef CLONING Index: sys/dev/watchdog/watchdog.c =================================================================== RCS file: /home/ncvs/src/sys/dev/watchdog/watchdog.c,v retrieving revision 1.2 diff -u -r1.2 watchdog.c --- sys/dev/watchdog/watchdog.c 16 Jun 2004 09:47:02 -0000 1.2 +++ sys/dev/watchdog/watchdog.c 9 Dec 2006 12:40:51 -0000 @@ -55,9 +55,16 @@ return (EINVAL); if ((u & (WD_ACTIVE | WD_PASSIVE)) == (WD_ACTIVE | WD_PASSIVE)) return (EINVAL); - if ((u & WD_INTERVAL) == WD_TO_NEVER) + if (u & WD_PASSIVE) + return (ENOSYS); /* XXX Not implemented yet */ + if ((u & WD_INTERVAL) == WD_TO_NEVER) { u = 0; - error = EOPNOTSUPP; + /* Assume all is well; watchdog signals failure. */ + error = 0; + } else { + /* Assume no watchdog available; watchdog flags success */ + error = EOPNOTSUPP; + } EVENTHANDLER_INVOKE(watchdog_list, u, &error); return (error); } Index: sys/i386/i386/elan-mmcr.c =================================================================== RCS file: /home/ncvs/src/sys/i386/i386/elan-mmcr.c,v retrieving revision 1.31.2.1 diff -u -r1.31.2.1 elan-mmcr.c --- sys/i386/i386/elan-mmcr.c 16 Aug 2005 22:47:14 -0000 1.31.2.1 +++ sys/i386/i386/elan-mmcr.c 9 Dec 2006 12:40:53 -0000 @@ -367,11 +367,11 @@ static void elan_watchdog(void *foo __unused, u_int spec, int *error) { - u_int u, v; + u_int u, v, w; static u_int cur; u = spec & WD_INTERVAL; - if (spec && u <= 35) { + if (spec && u > 0 && u <= 35) { u = imax(u - 5, 24); v = 2 << (u - 24); v |= 0xc000; @@ -383,7 +383,7 @@ * for other reasons. Save and restore the GP echo mode * around our hardware tom-foolery. */ - u = elan_mmcr->GPECHO; + w = elan_mmcr->GPECHO; elan_mmcr->GPECHO = 0; if (v != cur) { /* Clear the ENB bit */ @@ -401,17 +401,17 @@ elan_mmcr->WDTMRCTL = 0xaaaa; elan_mmcr->WDTMRCTL = 0x5555; } - elan_mmcr->GPECHO = u; + elan_mmcr->GPECHO = w; *error = 0; return; } else { - u = elan_mmcr->GPECHO; + w = elan_mmcr->GPECHO; elan_mmcr->GPECHO = 0; elan_mmcr->WDTMRCTL = 0x3333; elan_mmcr->WDTMRCTL = 0xcccc; elan_mmcr->WDTMRCTL = 0x4080; - elan_mmcr->WDTMRCTL = u; - elan_mmcr->GPECHO = u; + elan_mmcr->WDTMRCTL = w; /* XXX What does this statement do? */ + elan_mmcr->GPECHO = w; cur = 0; return; } Index: sys/kern/kern_clock.c =================================================================== RCS file: /home/ncvs/src/sys/kern/kern_clock.c,v retrieving revision 1.178.2.3 diff -u -r1.178.2.3 kern_clock.c --- sys/kern/kern_clock.c 10 Mar 2006 19:37:33 -0000 1.178.2.3 +++ sys/kern/kern_clock.c 9 Dec 2006 12:40:55 -0000 @@ -541,7 +541,7 @@ u_int u; u = cmd & WD_INTERVAL; - if ((cmd & WD_ACTIVE) && u >= WD_TO_1SEC) { + if (u >= WD_TO_1SEC) { watchdog_ticks = (1 << (u - WD_TO_1SEC)) * hz; watchdog_enabled = 1; *err = 0; Index: sys/sys/watchdog.h =================================================================== RCS file: /home/ncvs/src/sys/sys/watchdog.h,v retrieving revision 1.3 diff -u -r1.3 watchdog.h --- sys/sys/watchdog.h 28 Feb 2004 20:06:58 -0000 1.3 +++ sys/sys/watchdog.h 9 Dec 2006 12:41:08 -0000 @@ -30,11 +30,7 @@ #include -#ifdef I_HAVE_TOTALLY_LOST_MY_SENSE_OF_HUMOUR -#define _PATH_WATCHDOG "watchdog" -#else #define _PATH_WATCHDOG "fido" -#endif #define WDIOCPATPAT _IOW('W', 42, u_int) Index: share/man/man4/Makefile =================================================================== RCS file: /home/ncvs/src/share/man/man4/Makefile,v retrieving revision 1.320.2.21 diff -u -r1.320.2.21 Makefile --- share/man/man4/Makefile 23 Nov 2006 09:21:24 -0000 1.320.2.21 +++ share/man/man4/Makefile 9 Dec 2006 12:41:08 -0000 @@ -500,6 +500,7 @@ MLINKS+=wi.4 if_wi.4 MLINKS+=xe.4 if_xe.4 MLINKS+=xl.4 if_xl.4 +MLINKS+=watchdog.4 SW_WATCHDOG.4 .if ${MACHINE_ARCH} == "amd64" || ${MACHINE_ARCH} == "i386" _amdsmb.4= amdsmb.4 Index: share/man/man4/watchdog.4 =================================================================== RCS file: /home/ncvs/src/share/man/man4/watchdog.4,v retrieving revision 1.6.8.1 diff -u -r1.6.8.1 watchdog.4 --- share/man/man4/watchdog.4 1 May 2006 19:42:45 -0000 1.6.8.1 +++ share/man/man4/watchdog.4 9 Dec 2006 12:41:08 -0000 @@ -32,54 +32,84 @@ .Nm watchdog .Nd "hardware and software watchdog" .Sh SYNOPSIS -.Cd "options CPU_ELAN" -.Cd "options CPU_GEODE" -.Cd "options SW_WATCHDOG" -.Pp .In sys/watchdog.h .Sh DESCRIPTION The .Nm facility is used for controlling hardware and software watchdogs. .Pp -The interface is through a device .Pa /dev/fido -which responds to a single +responds to a single .Xr ioctl 2 call, .Dv WDIOCPATPAT . +It takes a single argument which represents a timeout value specified as a +power of two nanoseconds, or-ed with a flag selecting active or passive control +of the watchdog. .Pp -The call takes a single argument which represents a timeout value -specified as an integer power of two nanoseconds. -.Pp -The .Dv WD_ACTIVE -flag signals that the +indicates that the .Nm -will be kept from -timing out from userland, for instance by the +will be kept from timing out from userland, for instance by the .Xr watchdogd 8 daemon. -.Pp -To disable the watchdogs, an argument of zero should be used. +.Dv WD_PASSIVE +indicates that the +.Nm +will be kept from timing out from the kernel. .Pp The .Xr ioctl 2 call will return success if just one of the available .Xr watchdog 9 -implementations support the request. -If the call fails, for instance if none of +implementations supports setting the timeout to the specified timeout. This +means that at least one watchdog is armed. If the call fails, for instance if +none of .Xr watchdog 9 -implementations support the timeout -length, all watchdogs are disabled and must be explicitly re-enabled. +implementations support the timeout length, all watchdogs are disabled and must +be explicitly re-enabled. +.Pp +To disable the watchdogs pass +.Dv WD_TO_NEVER . +If disarming the watchdog(s) failed an error is returned. The watchdog might +still be armed! .Sh EXAMPLES -.\" XXX insert some descriptive text here .Bd -literal -offset indent -u_int u = WD_ACTIVE | WD_TO_8SEC; -int fd = open("/dev/fido", O_RDWR); +#include +#include + +#define WDPATH "/dev/" _PATH_WATCHDOG +int wdfd = -1; -ioctl(fd, WDIOCPATPAT, &u); +static void +wd_init(void) +{ + wdfd = open(WDPATH, O_RDWR); + if (wdfd == -1) + err(1, WDPATH); +} +static void +wd_reset(u_int timeout) +{ + if (ioctl(wdfd, WDIOCPATPAT, &timeout) == -1) + err(1, "WDIOCPATPAT"); +} + +/* in main() */ +wd_init(); +wd_reset(WD_ACTIVE|WD_TO_8SEC); +/* potential freeze point */ +wd_reset(WD_TO_NEVER); .Ed +.Pp +Enables a watchdog to recover from a potentially freezing piece of code. +.Pp +.Bd -literal -offset indent +options SW_WATCHDOG +.Ed +.Pp +in your kernel config adds a software watchdog in the kernel, dropping to KDB +or panic-ing when firing. .Sh SEE ALSO .Xr watchdogd 8 , .Xr watchdog 9 @@ -88,14 +118,17 @@ .Nm code first appeared in .Fx 5.1 . +.Sh BUGS +The +.Dv WD_PASSIVE +option has not yet been implemented. .Sh AUTHORS .An -nosplit The .Nm facility was written by .An Poul-Henning Kamp Aq phk@FreeBSD.org . -The software watchdog code -and this manual page were written by +The software watchdog code and this manual page were written by .An Sean Kelly Aq smkelly@FreeBSD.org . Some contributions were made by .An Jeff Roberson Aq jeff@FreeBSD.org . Index: watchdog.9 =================================================================== RCS file: /home/ncvs/src/share/man/man9/watchdog.9,v retrieving revision 1.3 diff -u -r1.3 watchdog.9 --- watchdog.9 6 Jul 2004 07:39:50 -0000 1.3 +++ watchdog.9 9 Dec 2006 13:14:54 -0000 @@ -35,6 +35,7 @@ .Ft void .Fn watchdog_fn "void *private" "u_int cmd" "int *error" .Fn EVENTHANDLER_REGISTER watchdog_list watchdog_fn private 0 +.Fn EVENTHANDLER_DEREGISTER watchdog_list eventhandler_tag .Sh DESCRIPTION To implement a watchdog in software or hardware, only a single function needs to be written and registered on the global @@ -60,7 +61,9 @@ If the watchdog cannot be configured to the proposed timeout, it must be disabled and the .Fa error -argument left untouched. +argument left untouched. If the watchdog cannot be disabled, the +.Fa error +argument must be set to indicate the error. .Pp There is no specification of what the watchdog should do when it times out, but a hardware reset or similar