linux-kernel - Re: [PATCH] watchdog: qcom: fine tune the max timeout value calculation

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: <cd223912-ac76-4b5d-93c3-92b1f89b9820@quicinc.com>
Date: Thu, 11 Jan 2024 20:37:03 +0530
From: Kathiravan Thirumoorthy <quic_kathirav@...cinc.com>
To: Guenter Roeck <linux@...ck-us.net>,
        Bjorn Andersson
	<andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Wim Van
 Sebroeck <wim@...ux-watchdog.org>
CC: <linux-arm-msm@...r.kernel.org>, <linux-watchdog@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] watchdog: qcom: fine tune the max timeout value
 calculation



On 1/11/2024 8:21 PM, Guenter Roeck wrote:
> On 1/11/24 01:49, Kathiravan Thirumoorthy wrote:
>> To determine the max_timeout value, the below calculation is used.
>>
>>     max_timeout = 0x10000000 / clk_rate
>>
>> cat 
>> /sys/devices/platform/soc@...017000.watchdog/watchdog/watchdog0/max_timeout
>> 8388
>>
>> However, this is not valid for all the platforms. IPQ SoCs starting from
>> IPQ40xx and recent Snapdragron SoCs also has the bark and bite time field
>> length of 20bits, which can hold max up to 32 seconds if the clk_rate is
>> 32KHz.
>>
>> If the user tries to configure the timeout more than 32s, then the value
>> will be truncated and the actual value will not be reflected in the HW.
>>
>> To avoid this, lets add a variable called max_tick_count in the device 
>> data,
>> which defines max counter value of the WDT controller. Using this, 
>> max-timeout
>> will be calculated in runtime for various WDT contorllers.
>>
>> With this change, we get the proper max_timeout as below and restricts
>> the user from configuring the timeout higher than this.
>>
>> cat 
>> /sys/devices/platform/soc@...017000.watchdog/watchdog/watchdog0/max_timeout
>> 32
>>
>> Signed-off-by: Kathiravan Thirumoorthy <quic_kathirav@...cinc.com>
> 
> [ ... ]
> 
>> @@ -268,11 +271,11 @@ static int qcom_wdt_probe(struct platform_device 
>> *pdev)
>>           wdt->wdd.bootstatus = WDIOF_CARDRESET;
>>       /*
>> -     * If 'timeout-sec' unspecified in devicetree, assume a 30 second
>> -     * default, unless the max timeout is less than 30 seconds, then use
>> +     * If 'timeout-sec' unspecified in devicetree, assume a 32 second
>> +     * default, unless the max timeout is less than 32 seconds, then use
>>        * the max instead.
>>        */
>> -    wdt->wdd.timeout = min(wdt->wdd.max_timeout, 30U);
>> +    wdt->wdd.timeout = min(wdt->wdd.max_timeout, 32U);
> 
> This is an unrelated change which would have to be discussed separately,
> in a separate patch. But I really don't see the point of it.


Ack, will revert this change in V2.


> 
> Guenter
>