linux-kernel - Re: [PATCH] Watchdog: sp5100_tco: Lower verbosity of disabled watchdog hardware

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:   Mon, 9 May 2022 19:37:33 -0500
From:   Mario Limonciello <mario.limonciello@....com>
To:     Jerry.Hoemann@....com, Guenter Roeck <linux@...ck-us.net>
Cc:     Wim Van Sebroeck <wim@...ux-watchdog.org>,
        "open list:WATCHDOG DEVICE DRIVERS" <linux-watchdog@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>, ionut_n2001@...oo.com
Subject: Re: [PATCH] Watchdog: sp5100_tco: Lower verbosity of disabled
 watchdog hardware

On 5/9/22 19:33, Jerry Hoemann wrote:
> On Mon, May 09, 2022 at 03:55:54PM -0700, Guenter Roeck wrote:
>> On 5/9/22 09:33, Mario Limonciello wrote:
>>> If watchdog hardware has been disabled, currently the kernel driver
>>> will show at err level during probe:
>>>
>>> "Watchdog hardware is disabled"
>>>
>>> This is unnecessarily verbose as there is already a -ENODEV returned.
>>> Lower the level to debug.
>>
>> Is it ? Without this message, a user may try to load the driver,
>> get an error message, and have no idea why the driver was not
>> enabled even though the hardware exists. If anything , -ENODEV
>> is less than perfect. Unfortunately there does not seem to be
>> a better error code, or at least I don't see one.
>>
>> Guenter
> 
> Coincidentally, I was looking at this code on Friday.
> 
> Some HPE Proliant servers are disabling the AMD WDT in BIOS.  However,
> sp5100_tco was still getting configured.  It was the lack of
> "Watchdog hardware is disabled" message that helped clue us into
> what was going on (Linux is enabling the WDT anyway.)
> 
> So, I liked that this message exists.
> 
> I'll send an RFC patch for this other issue as it orthogonal.
> But just wanted to point out the message is useful.

I personally don't have a problem blacklisting on a system I encounter 
this. I take anything at "err" level as there is a firmware problem or a 
hardware problem that should be looked at.

As the message is genuinely useful as Jerry points out how about meeting 
in the middle at info or notice?

> 
> 
>>
>>>
>>> Reported-by: ionut_n2001@...oo.com
>>> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D215762&amp;data=05%7C01%7Cmario.limonciello%40amd.com%7C5eb5f65caaf241d1515308da321cbd10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637877396427792688%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=tqQYyfwbGK%2BTvTxlkef0I2iKyLe8WsOjxskA0SQPlcw%3D&amp;reserved=0
>>> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
>>> ---
>>>    drivers/watchdog/sp5100_tco.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/watchdog/sp5100_tco.c b/drivers/watchdog/sp5100_tco.c
>>> index 86ffb58fbc85..e51ecbd5c8b7 100644
>>> --- a/drivers/watchdog/sp5100_tco.c
>>> +++ b/drivers/watchdog/sp5100_tco.c
>>> @@ -286,7 +286,7 @@ static int sp5100_tco_timer_init(struct sp5100_tco *tco)
>>>    	val = readl(SP5100_WDT_CONTROL(tco->tcobase));
>>>    	if (val & SP5100_WDT_DISABLED) {
>>> -		dev_err(dev, "Watchdog hardware is disabled\n");
>>> +		dev_dbg(dev, "Watchdog hardware is disabled\n");
>>>    		return -ENODEV;
>>>    	}
>