From: Jean Delvare <jdelvare@suse.de>

Subject: watchdog: wdat_wdt: Set the min and max timeout values properly

References: bsc#1194023

Patch-mainline: submitted, 2022-08-23 https://lore.kernel.org/linux-watchdog/20220823154713.023ee771@endymion.delvare/

The wdat_wdt driver is misusing the min_hw_heartbeat_ms field. This

field should only be used when the hardware watchdog device should not

be pinged more frequently than a specific period. The ACPI WDAT

"Minimum Count" field, on the other hand, specifies the minimum

timeout value that can be set. This corresponds to the min_timeout

field in Linux's watchdog infrastructure.

Setting min_hw_heartbeat_ms instead can cause pings to the hardware

to be delayed when there is no reason for that, eventually leading to

unexpected firing of the watchdog timer (and thus unexpected reboot).

Since commit 6d72c7ac9fbe ("watchdog: wdat_wdt: Using the existing

function to check parameter timeout"), min_timeout is being set too,

but to the arbitrary value of 1 second, which doesn't make sense and

allows setting timeout values lower that the ACPI WDAT "Minimum

Count" field.

I'm also changing max_hw_heartbeat_ms to max_timeout for symmetry,

although the use of this one isn't fundamentally wrong, but there is

also no reason to enable the software-driven ping mechanism for the

wdat_wdt driver.

Signed-off-by: Jean Delvare <jdelvare@suse.de>

Fixes: 058dfc767008 ("ACPI / watchdog: Add support for WDAT hardware watchdog")

Fixes: 6d72c7ac9fbe ("watchdog: wdat_wdt: Using the existing function to check parameter timeout")

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

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

Cc: Wim Van Sebroeck <wim@linux-watchdog.org>

Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Cc: Liu Xinpeng <liuxp11@chinatelecom.cn>

---

 drivers/watchdog/wdat_wdt.c |    8 ++++----

 1 file changed, 4 insertions(+), 4 deletions(-)

--- a/drivers/watchdog/wdat_wdt.c

+++ b/drivers/watchdog/wdat_wdt.c

@@ -342,8 +342,8 @@ static int wdat_wdt_probe(struct platfor

 		return -EINVAL;

 	wdat->period = tbl->timer_period;

-	wdat->wdd.min_hw_heartbeat_ms = wdat->period * tbl->min_count;

-	wdat->wdd.max_hw_heartbeat_ms = wdat->period * tbl->max_count;

+	wdat->wdd.min_timeout = DIV_ROUND_UP(wdat->period * tbl->min_count, 1000);

+	wdat->wdd.max_timeout = wdat->period * tbl->max_count / 1000;

 	wdat->stopped_in_sleep = tbl->flags & ACPI_WDAT_STOPPED;

 	wdat->wdd.info = &wdat_wdt_info;

 	wdat->wdd.ops = &wdat_wdt_ops;

@@ -450,8 +450,8 @@ static int wdat_wdt_probe(struct platfor

 	 * watchdog properly after it has opened the device. In some cases

 	 * the BIOS default is too short and causes immediate reboot.

 	 */

-	if (timeout * 1000 < wdat->wdd.min_hw_heartbeat_ms ||

-	    timeout * 1000 > wdat->wdd.max_hw_heartbeat_ms) {

+	if (timeout < wdat->wdd.min_timeout ||

+	    timeout > wdat->wdd.max_timeout) {

 		dev_warn(dev, "Invalid timeout %d given, using %d\n",

 			 timeout, WDAT_DEFAULT_TIMEOUT);

 		timeout = WDAT_DEFAULT_TIMEOUT;