watchdog: iTCO_wdt: Backport patches to enable handle_boot_enabled=0

These patches are needed in order to make iTCO_wdt respect handle_boot_enabled=0, i.e. to step back from a running watchdog until userspace picks it up. Up to 5.16, the driver simply stopped the watchdog which could have created a non-monitored window during boot-up in the handover from the firmware starting it to userspace (typically sytemd) safely driving. E.g., any infinite loops in the initramfs would could have created a locked-up system.

These patches were proposed to the watchdog maintainers for official stable integration [1] but received no reaction so far. As the issue is critical for CIP's OTA software update integration, let's use the CIP path instead.

I don't have any test setup for 4.19, so I'm only proposing this for 5.10 here.

Jan

[1] https://lore.kernel.org/linux-watchdog/[email protected]/T/#t

Mika Westerberg (2):
watchdog: iTCO_wdt: No need to stop the timer in probe
watchdog: iTCO_wdt: Set NO_REBOOT if the watchdog is not already
running

 drivers/watchdog/iTCO_wdt.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

2.35.3

From: Mika Westerberg <[email protected]>

Commit 1ae3e78c08209ac657c59f6f7ea21bbbd7f6a1d4 upstream.

The watchdog core can handle pinging of the watchdog before userspace opens the device. For this reason instead of stopping the timer, just mark it as running and let the watchdog core take care of it.

---
 drivers/watchdog/iTCO_wdt.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

index a370a185a41c..9048fa44897f 100644
@@ -426,6 +426,16 @@ static unsigned int iTCO_wdt_get_timeleft(struct watchdog_device *wd_dev)
 	return time_left;
 }
 
+static void iTCO_wdt_set_running(struct iTCO_wdt_private *p)
+{
+	u16 val;
+
+	/* Bit 11: TCO Timer Halt -> 0 = The TCO timer is * enabled */
+	val = inw(TCO1_CNT(p));
+	if (!(val & BIT(11)))
+		set_bit(WDOG_HW_RUNNING, &p->wddev.status);
+}
+
 /*
  *	Kernel Interfaces
  */

@@ -568,8 +578,7 @@ static int iTCO_wdt_probe(struct platform_device *pdev)
 	watchdog_set_drvdata(&p->wddev, p);
 	platform_set_drvdata(pdev, p);
 
-	/* Make sure the watchdog is not running */
-	iTCO_wdt_stop(&p->wddev);
+	iTCO_wdt_set_running(p);
 
 	/* Check that the heartbeat value is within it's range;
 	   if not reset to the default */

2.35.3

From: Mika Westerberg <[email protected]>

Commit ef9b7bf52c2f47f0a9bf988543c577b92c92d15e upstream.

Daniel reported that the commit 1ae3e78c0820 ("watchdog: iTCO_wdt: No need to stop the timer in probe") makes QEMU implementation of the iTCO watchdog not to trigger reboot anymore when NO_REBOOT flag is initially cleared using this option (in QEMU command line):

-global ICH9-LPC.noreboot=false

The problem with the commit is that it left the unconditional setting of NO_REBOOT that is not cleared anymore when the kernel keeps pinging the watchdog (as opposed to the previous code that called iTCO_wdt_stop() that cleared it).

Fix this so that we only set NO_REBOOT if the watchdog was not initially running.

---
 drivers/watchdog/iTCO_wdt.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

index 9048fa44897f..50c874d48860 100644
@@ -426,14 +426,18 @@ static unsigned int iTCO_wdt_get_timeleft(struct watchdog_device *wd_dev)
 	return time_left;
 }
 
-static void iTCO_wdt_set_running(struct iTCO_wdt_private *p)
+/* Returns true if the watchdog was running */
+static bool iTCO_wdt_set_running(struct iTCO_wdt_private *p)
 {
 	u16 val;
 
-	/* Bit 11: TCO Timer Halt -> 0 = The TCO timer is * enabled */
+	/* Bit 11: TCO Timer Halt -> 0 = The TCO timer is enabled */
 	val = inw(TCO1_CNT(p));
-	if (!(val & BIT(11)))
+	if (!(val & BIT(11))) {
 		set_bit(WDOG_HW_RUNNING, &p->wddev.status);
+		return true;
+	}
+	return false;
 }
 
 /*

@@ -524,9 +528,6 @@ static int iTCO_wdt_probe(struct platform_device *pdev)
 		return -ENODEV;	/* Cannot reset NO_REBOOT bit */
 	}
 
-	/* Set the NO_REBOOT bit to prevent later reboots, just for sure */
-	p->update_no_reboot_bit(p->no_reboot_priv, true);
-
 	if (turn_SMI_watchdog_clear_off >= p->iTCO_version) {
 		/*
 		 * Bit 13: TCO_EN -> 0

@@ -578,7 +579,13 @@ static int iTCO_wdt_probe(struct platform_device *pdev)
 	watchdog_set_drvdata(&p->wddev, p);
 	platform_set_drvdata(pdev, p);
 
-	iTCO_wdt_set_running(p);
+	if (!iTCO_wdt_set_running(p)) {
+		/*
+		 * If the watchdog was not running set NO_REBOOT now to
+		 * prevent later reboots.
+		 */
+		p->update_no_reboot_bit(p->no_reboot_priv, true);
+	}
 
 	/* Check that the heartbeat value is within it's range;
 	   if not reset to the default */

2.35.3

Hi Jan,

Feedback from the watchdog maintainers is reassuring, but you can also submit backported patches directly to stable. How about sending a patch to stable first?

Best regard,
Nobuhiro

Hi!

So... these are mainline, but -stable failed to react. Rules say we can take them.

Both patches are in 6.1, so we don't need to port it there.

And I don't see anything horribly wrong; I propose we apply them if they pass testing.

Best regards,
Pavel

DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany