linux-kernel - Re: [PATCH v2] iTCO_wdt: all versions count down twice

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <3a2a4b95-ef9c-25d8-db48-ebf2b55e89b8@roeck-us.net>
Date:   Thu, 6 Apr 2017 04:46:18 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Paolo Bonzini <pbonzini@...hat.com>, linux-kernel@...r.kernel.org
Cc:     Andy Shevchenko <andy.shevchenko@...il.com>,
        Wim Van Sebroeck <wim@...ana.be>,
        linux-watchdog@...r.kernel.org
Subject: Re: [PATCH v2] iTCO_wdt: all versions count down twice

On 04/05/2017 04:41 AM, Paolo Bonzini wrote:
> The ICH9 is listed as having TCO v2, and indeed the behavior in the
> datasheet corresponds to v2 (for example the NO_REBOOT flag is
> accessible via the 16KiB-aligned Root Complex Base Address).
>
> However, the TCO counts twice just like in v1; the documentation
> of the SECOND_TO_STS bit says: "ICH9 sets this bit to 1 to indicate
> that the TIMEOUT bit had been (or is currently) set and a second
> timeout occurred before the TCO_RLD register was written. If this
> bit is set and the NO_REBOOT config bit is 0, then the ICH9 will
> reboot the system after the second timeout.  The same can be found
> in the BayTrail (Atom E3800) datasheet, and even HOWTOs around
> the Internet say that it will reboot after _twice_ the specified
> heartbeat.
>
> I did not find the Apollo Lake datasheet, but because v4/v5 has
> a SECOND_TO_STS bit just like the previous version I'm enabling
> this for Apollo Lake as well.
>
> Cc: Wim Van Sebroeck <wim@...ana.be>
> Cc: Guenter Roeck <linux@...ck-us.net>
> Cc: linux-watchdog@...r.kernel.org
> Reviewed-by: Andy Shevchenko <andy.shevchenko@...il.com>
> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>

Reviewed-by: Guenter Roeck <linux@...ck-us.net>

> ---
>  Documentation/watchdog/watchdog-parameters.txt |  2 +-
>  drivers/watchdog/iTCO_wdt.c                    | 22 ++++++++++------------
>  2 files changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
> index 4f7d86dd0a5d..914518aeb972 100644
> --- a/Documentation/watchdog/watchdog-parameters.txt
> +++ b/Documentation/watchdog/watchdog-parameters.txt
> @@ -117,7 +117,7 @@ nowayout: Watchdog cannot be stopped once started
>  -------------------------------------------------
>  iTCO_wdt:
>  heartbeat: Watchdog heartbeat in seconds.
> -	(2<heartbeat<39 (TCO v1) or 613 (TCO v2), default=30)
> +	(5<=heartbeat<=74 (TCO v1) or 1226 (TCO v2), default=30)
>  nowayout: Watchdog cannot be stopped once started
>  	(default=kernel config parameter)
>  -------------------------------------------------
> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> index 3d0abc0d59b4..d1d0446011c4 100644
> --- a/drivers/watchdog/iTCO_wdt.c
> +++ b/drivers/watchdog/iTCO_wdt.c
> @@ -280,16 +280,15 @@ static int iTCO_wdt_ping(struct watchdog_device *wd_dev)
>
>  	iTCO_vendor_pre_keepalive(p->smi_res, wd_dev->timeout);
>
> +	/* Reset the timeout status bit so that the timer
> +	 * needs to count down twice again before rebooting */
> +	outw(0x0008, TCO1_STS(p));	/* write 1 to clear bit */
> +
>  	/* Reload the timer by writing to the TCO Timer Counter register */
> -	if (p->iTCO_version >= 2) {
> +	if (p->iTCO_version >= 2)
>  		outw(0x01, TCO_RLD(p));
> -	} else if (p->iTCO_version == 1) {
> -		/* Reset the timeout status bit so that the timer
> -		 * needs to count down twice again before rebooting */
> -		outw(0x0008, TCO1_STS(p));	/* write 1 to clear bit */
> -
> +	else if (p->iTCO_version == 1)
>  		outb(0x01, TCO_RLD(p));
> -	}
>
>  	spin_unlock(&p->io_lock);
>  	return 0;
> @@ -302,11 +301,8 @@ static int iTCO_wdt_set_timeout(struct watchdog_device *wd_dev, unsigned int t)
>  	unsigned char val8;
>  	unsigned int tmrval;
>
> -	tmrval = seconds_to_ticks(p, t);
> -
> -	/* For TCO v1 the timer counts down twice before rebooting */
> -	if (p->iTCO_version == 1)
> -		tmrval /= 2;
> +	/* The timer counts down twice before rebooting */
> +	tmrval = seconds_to_ticks(p, t) / 2;
>
>  	/* from the specs: */
>  	/* "Values of 0h-3h are ignored and should not be attempted" */
> @@ -359,6 +355,8 @@ static unsigned int iTCO_wdt_get_timeleft(struct watchdog_device *wd_dev)
>  		spin_lock(&p->io_lock);
>  		val16 = inw(TCO_RLD(p));
>  		val16 &= 0x3ff;
> +		if (!(inw(TCO1_STS(p)) & 0x0008))
> +			val16 += (inw(TCOv2_TMR(p)) & 0x3ff);
>  		spin_unlock(&p->io_lock);
>
>  		time_left = ticks_to_seconds(p, val16);
>