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]
Date:   Thu, 11 Apr 2019 09:03:05 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc:     Vladimir Zapolskiy <vz@...ia.com>,
        Wim Van Sebroeck <wim@...ux-watchdog.org>,
        Sylvain Lemieux <slemieux.tyco@...il.com>,
        Gregory Clement <gregory.clement@...tlin.com>,
        linux-arm-kernel@...ts.infradead.org,
        linux-watchdog@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] watchdog: pnx4008: readout watchddog state

Hi Alexandre,

On Thu, Apr 11, 2019 at 05:36:46PM +0200, Alexandre Belloni wrote:
> Readout the enabled state so it is possible to get the pre-userspace
> handler working. Also, avoid disabling the watchdog when nowayout is set to
> ensure the watchdog continues working and triggers if there is an issue
> later in the boot or if userspace fails to start.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@...tlin.com>
> ---
>  drivers/watchdog/pnx4008_wdt.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/pnx4008_wdt.c b/drivers/watchdog/pnx4008_wdt.c
> index 8e261799c84e..9e3714e3814c 100644
> --- a/drivers/watchdog/pnx4008_wdt.c
> +++ b/drivers/watchdog/pnx4008_wdt.c
> @@ -209,7 +209,11 @@ static int pnx4008_wdt_probe(struct platform_device *pdev)
>  	watchdog_set_nowayout(&pnx4008_wdd, nowayout);
>  	watchdog_set_restart_priority(&pnx4008_wdd, 128);
>  
> -	pnx4008_wdt_stop(&pnx4008_wdd);	/* disable for now */
> +	if (readl(WDTIM_CTRL(wdt_base)) & COUNT_ENAB)
> +		set_bit(WDOG_HW_RUNNING, &pnx4008_wdd.status);
> +
> +	if (!nowayout)
> +		pnx4008_wdt_stop(&pnx4008_wdd);	/* disable for now */

I don't see the point of stopping the watchdog here, even though that is the
old behavior. If it is possible to detect that the watchdog is running, we
can let the core handle pings until the watchdog device is opened. The code
above does that, but only if nowayout is set. That seems inconsistent and
unnecessary. Is there a downside to doing that unconditionally ?

Nitpick: s/watchddog/watchdog/ in subject line.

Thanks,
Guenter

>  
>  	ret = watchdog_register_device(&pnx4008_wdd);
>  	if (ret < 0) {
> -- 
> 2.20.1
> 

Powered by blists - more mailing lists