linux-kernel - Re:Re: [PATCH] watchdog: mtk: support pre-timeout when the bark irq is available

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]

Date:   Sat, 10 Apr 2021 20:19:22 +0800 (GMT+08:00)
From:   王擎 <wangqing@...o.com>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     Wim Van Sebroeck <wim@...ux-watchdog.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        linux-watchdog@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re:Re: [PATCH] watchdog: mtk: support pre-timeout when the bark irq is available


>On 4/9/21 8:11 PM, 王擎 wrote:
>> 
>>> On 4/9/21 7:42 PM, 王擎 wrote:
>>>>
>>>>> On 4/9/21 2:55 AM, Wang Qing wrote:
>>>>>> Use the bark interrupt as the pretimeout notifier if available.
>>>>>>
>>>>>> By default, the pretimeout notification shall occur one second earlier
>>>>>> than the timeout.
>>>>>>
>>>>>> Signed-off-by: Wang Qing <wangqing@...o.com>
>>>>>> ---
>>>>>>  drivers/watchdog/mtk_wdt.c | 47 +++++++++++++++++++++++++++++++++++++++++++---
>>>>>>  1 file changed, 44 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
>>>>>> index 97ca993..8b919cc
>>>>>> --- a/drivers/watchdog/mtk_wdt.c
>>>>>> +++ b/drivers/watchdog/mtk_wdt.c
>>>>>> @@ -25,6 +25,7 @@
>>>>>>  #include <linux/reset-controller.h>
>>>>>>  #include <linux/types.h>
>>>>>>  #include <linux/watchdog.h>
>>>>>> +#include <linux/interrupt.h>
>>>>>>  
>>>>>>  #define WDT_MAX_TIMEOUT		31
>>>>>>  #define WDT_MIN_TIMEOUT		1
>>>>>> @@ -234,18 +235,35 @@ static int mtk_wdt_start(struct watchdog_device *wdt_dev)
>>>>>>  	void __iomem *wdt_base = mtk_wdt->wdt_base;
>>>>>>  	int ret;
>>>>>>  
>>>>>> -	ret = mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout);
>>>>>> +	ret = mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout - wdt_dev->pretimeout);
>>>>>
>>>>> That looks suspiciously like the real watchdog won't happen at all.
>>>>> What will happen if the pretimeout governor is set to none ?
>>>>>
>>>>> Guenter
>>>>>
>>>> The pretimeout governor is panic by default. If pretimeout is enabled and the governor is
>>>> set to none, it means the timeout behavior does not need to be processed, only printing.
>>>>
>>>
>>> That was not my question. My question was if the real timeout happens in that case.
>>>
>>> Guenter
>>>
>> Yes, the real timeout will happen. After WDT timeout, IRQ is sent out instead of 
>> reset signal first. In order to ensure that CPU does not get stuck after IRQ is sent out, 
>> WDT will time again and send reset signal to reset.
>> 
>
>When will that be, or in other words how does the chip know when to time out ?
>After all, only a single timeout value is written into the chip. I don't see how
>it would know to reset the chip after wdt_dev->timeout.
>
>Guenter
>
>
Sorry I made a mistake, the IRQ is sent out first, then time again and reset only when 
WDT_MODE_IRQ_EN|WDT_MODE_DUAL_EN is enabled. WDT_MODE_DUAL_EN mode
is a special mode, depending on the specific chip design.

If WDT_MODE_IRQ_EN|~WDT_MODE_DUAL_EN as described in my patch, it just sent
out IRQ instead of reset signal, the real timeout will not happen if the pretimeout 
governor is set to none.

Also, ~(WDT_MODE_IRQ_EN | WDT_MODE_DUAL_EN) means send reset signal directly.
It does not support pretimeout processing.

Thanks
Qing Wang