lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <4BBCB0E7.9000701@draigBrady.com>
Date:	Wed, 07 Apr 2010 17:20:55 +0100
From:	Pádraig Brady <P@...igBrady.com>
To:	Simon Kagstrom <simon.kagstrom@...insight.net>
CC:	linux-kernel@...r.kernel.org, wim@...ana.be, seth.heasley@...el.com
Subject: Re: [PATCH] iTCO_wdt: Don't double the requested timeout

On 24/02/10 16:18, Pádraig Brady wrote:
> On 24/02/10 09:16, Simon Kagstrom wrote:
>> On Tue, 23 Feb 2010 17:09:26 +0000
>> Pádraig Brady<P@...igBrady.com>  wrote:
>>
>>> Actually looking at that code I noticed that it wasn't
>>> accounting for the timer counting down twice before reboot,
>>> which I thought was the case for ICH4 at least.
>>> The following is not even compiled, nor am I sure it
>>> applies to TCO v2. Testing/info appreciated.
>>
>> I tested your change on our TCO v1-based board, and it doubles the time
>> until the watchdog triggers. So:
> 
> Well it should be halving the timeout :)
> I amended the patch and Simon retested to verify
> that it now honors the requested timeout.
> 
> I also checked an ICH7 box here and it doesn't
> seem to need the adjustment, so I've amended the patch accordingly.
> 
> Wim, please apply, thanks...

In further testing it was seen that the "timer status" bit
needs to be cleared at each pat of the watchdog so as
to support timeouts in the 34s to 76s range.
This was done with: outb (0x08, TCO1_STS);
The updated patch is below.

cheers,
Pádraig.

iTCO_wdt: fix TCO V1 timeout values and limits

For TCO V1 devices the programmed timeout was twice too long
because the fact that the TCO V1 timer needs to count down
twice before triggering the watchdog, wasn't accounted for.
Also the timeout values in the module description and error
message were clarified.

Signed-off-by: Pádraig Brady <P@...igBrady.com>
Tested-by: Simon Kagstrom <simon.kagstrom@...insight.se>

--- a/iTCO_wdt.c	2010-04-06 15:00:41.000000000 +0000
+++ b/iTCO_wdt.c	2010-04-07 16:08:27.000000000 +0000
@@ -391,8 +391,8 @@
 #define WATCHDOG_HEARTBEAT 30	/* 30 sec default heartbeat */
 static int heartbeat = WATCHDOG_HEARTBEAT;  /* in seconds */
 module_param(heartbeat, int, 0);
-MODULE_PARM_DESC(heartbeat, "Watchdog heartbeat in seconds. "
-	"(2<heartbeat<39 (TCO v1) or 613 (TCO v2), default="
+MODULE_PARM_DESC(heartbeat, "Watchdog timeout in seconds. "
+	"5..76 (TCO v1) or 3..614 (TCO v2), default="
 				__MODULE_STRING(WATCHDOG_HEARTBEAT) ")");

 static int nowayout = WATCHDOG_NOWAYOUT;
@@ -408,8 +408,12 @@
 static inline unsigned int seconds_to_ticks(int seconds)
 {
 	/* the internal timer is stored as ticks which decrement
-	 * every 0.6 seconds */
-	return (seconds * 10) / 6;
+	 * every 0.6 seconds. For TCO v1 the timer counts down
+	 * twice before triggering the watchdog */
+	if (iTCO_wdt_private.iTCO_version == 1)
+		return (seconds * 5) / 6;
+	else
+		return (seconds * 10) / 6;
 }

 static void iTCO_wdt_set_NO_REBOOT_bit(void)
@@ -521,10 +525,15 @@
 	iTCO_vendor_pre_keepalive(iTCO_wdt_private.ACPIBASE, heartbeat);

 	/* Reload the timer by writing to the TCO Timer Counter register */
-	if (iTCO_wdt_private.iTCO_version == 2)
-		outw(0x01, TCO_RLD);
-	else if (iTCO_wdt_private.iTCO_version == 1)
+	if (iTCO_wdt_private.iTCO_version == 1) {
+		/* Reset the timeout status bit so that the timer
+		 * needs to count down twice again before rebooting */
+		outb (0x08, TCO1_STS); /* write 1 to clear bit */
+
 		outb(0x01, TCO_RLD);
+	} else if (iTCO_wdt_private.iTCO_version == 2)
+		outw(0x01, TCO_RLD);
+	}

 	spin_unlock(&iTCO_wdt_private.io_lock);
 	return 0;
@@ -843,9 +852,14 @@
 	   if not reset to the default */
 	if (iTCO_wdt_set_heartbeat(heartbeat)) {
 		iTCO_wdt_set_heartbeat(WATCHDOG_HEARTBEAT);
+		int tco_min=5; int tco_max=76; /* TCO V1 */
+		if (iTCO_wdt_private.iTCO_version == 2) {
+			tco_min=3; tco_max=614;
+		}
 		printk(KERN_INFO PFX
-			"heartbeat value must be 2 < heartbeat < 39 (TCO v1) "
-				"or 613 (TCO v2), using %d\n", heartbeat);
+			"timeout value %d is not between %d and %d"
+			" inclusive, using %d\n",
+			heartbeat, tco_min, tco_max, WATCHDOG_HEARTBEAT);
 	}

 	ret = misc_register(&iTCO_wdt_miscdev);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ