[26.04_linux-nvidia-bos] NVIDIA: SAUCE: watchdog: sbsa_gwdt: park the watchdog around system sleep on MediaTek implementations

[dcemin-nv]

Intermittent resets around system suspend (S3 and s2idle) on the MT8901 ACPI laptop platform with the SBSA generic watchdog enabled.

Root cause, measured at register level on the reference laptop: the MediaTek watchdog implementation keeps comparing against the last reload after WCS.EN is cleared, latches a WOR write into the countdown only while WCS.EN is set (the register reads back the written value either way), and commits the two WOR halves on a slow clock. The driver disables the watchdog at suspend prepare, but the core keeps refreshing it until the CPUs go offline, so the platform enters the sleep with a fresh 5 s deadline that nobody can feed; the firmware re-enable after WFI then latches the short period onto the pre-sleep reload and produces the resume-side resets.

Fix: park instead of disable. On stop, program WOR to the 48-bit maximum (high half first) while enabled, wait for the commit and refresh twice, and leave WCS.EN set; on start, enable, refresh, restore the timeout (low half first) and refresh again. Version 0 watchdogs keep the architectural disable.

Validated on two laptops of the platform, including the unit that failed every earlier variant: repeated S3 and s2idle cycles (5 + 5 on that unit) resume without a reset, WOR reads the configured value after resume and WS0 asserts at the configured period, so the watchdog is live after the sleep. Same change under review as the downstream distribution MR.

Internal tracking: NVBugs 6813557 and 6783303.

bos twin of #627.

BugLink: https://bugs.launchpad.net/bugs/2169002

[nirmoy]

BaseOS Kernel Review

Warning

⚠️ Review needs attention

Two comments misdescribe watchdog behavior: version 0 MediaTek devices veto sleep when armed, and hardware stop is also reachable during probe rollback.

Findings: Critical 0 · High 0 · Medium 0 · Low 2

🔍 Review artifacts

• 🟠 Boro review: 2 findings
• 🔵 Test plan: ready for validation
• 🔴 KTS Report: 2 regressions

📦 Kernel deb builds — 🟢 2/2 passed

• 🟢 amd64: build report and 4 debs
• 🟢 arm64: build report and 4 debs

Note

Build reports and debs are retained for 10 days after the PR closes.

Review metadata

• Reviewed head: b86b42888d6a
• Overall status: attention needed
• Architectures: 2/2 successful

This comment is maintained by BaseOS Reviewer and updated when the GitHub watcher publishes a newer review.

[github-actions]

PR Validation Report

Patchscan ✅ No Missing Fixes

All cherry-picked commits checked — no missing upstream fixes found.

PR Lint ✅ All checks passed

Details

Checking 1 commits...

Cherry-pick digest:
┌──────────────┬──────────────────────────────────────────────────────────────────┬────────────┬─────────┬───────────────────────────┐
│ Local        │ Referenced upstream / Patch subject                              │ Patch-ID   │ Subject │ SoB chain                 │
├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤
│ b86b42888d6a │ [SAUCE] watchdog: sbsa_gwdt: park the watchdog around system sle │ N/A        │ N/A     │ kbutala, dcemin           │
└──────────────┴──────────────────────────────────────────────────────────────────┴────────────┴─────────┴───────────────────────────┘

Lint: all checks passed.

[nirmoy]

Codex found these issues

Codex confirmed two concerns also raised by Boro—parking on unaffected implementations and timeout updates interfering with parking—and identified an additional timeout regression. Please take a look at the inline comments and let me know what you think.

Here is the requested Launchpad bug for this pair: LP #2169002.

Review covered the driver changes and watchdog-core interactions. No build or hardware tests were run.

[nirmoy]

[P1] Preserve the requested timeout

This now stores the clamped hardware timeout in wdd->timeout. Previously, that field retained the requested timeout so the watchdog core could support longer timeouts through periodic keepalives. For example, with an 8-second hardware limit, requesting 60 seconds now records 8 seconds and disables the worker that previously extended the timeout. This also affects v0 devices. Please preserve the requested value in wdd->timeout and clamp only the hardware programming.

[nirmoy]

[P1] Keep timeout updates from undoing parking

After WDIOS_DISABLECARD, userspace can still issue WDIOC_SETTIMEOUT on the same descriptor. Stop now leaves the hardware enabled, so this unconditional WOR write replaces the parked period with a short timeout. The core has cleared WDOG_HW_RUNNING and stopped feeding the watchdog, allowing an unexpected reset. Please preserve the parked value while stopped or sleeping, save the requested timeout for the next start, and synchronize timeout programming with the parking path.

[nirmoy]

[P2] Limit parking to affected implementations

Checking only the version applies this workaround to every v1 watchdog, including non-MediaTek implementations. It also changes ordinary stop and magic-close behavior: hardware remains enabled while the core considers it stopped and stops supplying keepalives. The parked period is finite, so it can eventually expire and reset the system. Please gate parking on the affected implementation and retain architectural disable for unaffected hardware.

[jamieNguyenNVIDIA]

This applies to both #627 and #628, which carry the same commit. The points below are in addition to the Codex/Boro comments already posted.

1. Probe undoes the park, the same way WDIOC_SETTIMEOUT does. When the driver probes during a sleep transition with a firmware-enabled watchdog, sbsa_gwdt_hw_stop() parks it (sbsa_gwdt.c:625). Later, sbsa_gwdt_set_timeout(wdd, wdd->timeout) (:699) writes the short WOR while WCS.EN is still set. The core's keepalives then reload with that short period until the CPUs go offline, which is the original failure. One change covers this case and both Codex P1s. set_timeout should record only the requested (unclamped) wdd->timeout and min_hw_heartbeat_ms. It should program WOR only under sbsa_gwdt_lock when hw_armed && !sbsa_gwdt_sleeping. sbsa_gwdt_hw_start() already reprograms WOR on every start.

2. Implementer gating is more than P2. On a non-MediaTek v1 watchdog, the parked state is also reached from watchdog_stop_on_reboot() (halt, kexec), not only from magic close. 26.04_linux-nvidia runs on servers with such watchdogs. At a 1 GHz CNTFRQ, parked means a panic at about 78 h with action=1, or a WS1 reset at about 6.5 days with the default action=0. The gate needs its own impl == SBSA_GWDT_IMPL_MEDIATEK flag, because need_ws0_race_workaround also depends on action.

3. Long sleeps on the affected platform. A parked watchdog stays enabled through the sleep. An s2idle longer than about 78 h asserts WS0. With action=1, that IRQ is neither a wake IRQ nor IRQF_NO_SUSPEND, so it would fire panic() when device IRQs resume, before PM_POST_SUSPEND re-arms. With action=0, the system resets at about 6.5 days. This is likely inherent to the hardware, but please state it in the commit message as a known limitation. Was a multi-day sleep tested?

4. Upstream plan. The PR, commit and MR 28 don't mention one, and nothing has been posted to linux-watchdog. This builds on 9c77d37, which carries the v1 at https://lore.kernel.org/r/20260912182107.1156221-1-dcemin@nvidia.com. Guenter answered that posting on 2026-09-17: "this is the wrong solution". He said any fix belongs in the watchdog core and should "leave the watchdog running but ping it from the kernel while the suspend operation is going on". There has been no reply on the list since. The MediaTek property found here is a concrete implementer quirk, which makes a stronger upstream case than the generic notifier. Parking is also closer to what Guenter described. Is the plan to reply on that thread and post the park as an implementer quirk? Guenter's 2026-09-29 core series may also overlap, especially "watchdog: core: Prevent ping worker from re-arming timer on suspend" (20260929134635.2567137-4-linux@roeck-us.net).

Commit hygiene

• The author is kbutala <kbutala@nvidia.com> but the SoB reads Kaushal Rajeev Butala (checkpatch warning). Fix with git commit --amend --author="Kaushal Rajeev Butala <kbutala@nvidia.com>".
• The commit bundles unrelated changes: v%d in the probe dev_info, a dev_dbg register dump at resume, and a stray blank line before the closing brace of sbsa_gwdt_sleep_restart().
• Both PR bodies still need the LP #2169002 link. Its absence is what trips PR lint.

For the record, Boro's "WS0 workaround missed after IRQ fallback" is pre-existing. The base already calls sbsa_gwdt_get_version() before the action = 0 fallback, so it is not introduced here.

drivers/watchdog/sbsa_gwdt.o builds cleanly (arm64 defconfig, W=1). No hardware testing.

[dcemin-nv]

Thanks both, all points taken; new heads pushed to both PRs (607d5a2 / 328f3bd).

• set_timeout keeps the requested value in wdd->timeout and clamps only the hardware programming; it now programs WOR only while the watchdog is armed and no sleep transition is in progress, under the driver lock, so WDIOC_SETTIMEOUT after WDIOS_DISABLECARD and the probe-during-sleep case leave the parked period alone. hw_start programs the timeout on every start.
• Parking is gated on a new park_on_stop flag: version 1 and the MediaTek implementer id. Other implementations, and version 0, keep the architectural disable on stop.
• The finite parked period (about 78 h at 1 GHz, WS0 then panic with action=1 or reset with action=0) is stated in the commit message as a known limitation; multi-day sleeps were not tested.
• Author fixed to match the SoB; the probe banner change, the debug register dump and the stray blank line are gone. BugLink 2169002 added to the commit and both PR bodies.
• Upstream: the park will be proposed upstream as a MediaTek implementer quirk, with the measured hardware properties (compare independent of WCS.EN, WOR latched only while enabled, slow WOR commit) as the justification; the patch goes through our internal IP review today, and I will reply on the earlier linux-watchdog thread pointing at those measurements rather than the generic notifier. I will check Guenter's 2026-09-29 core series for overlap before posting.

[clsotog]

Can you check the high finding from Boro review.

[dcemin-nv]

@clsotog on the Boro High: it describes the design, and there is no software alternative on this implementation. We measured that clearing WCS.EN does not stop the compare on the MediaTek block (EN cleared, one refresh, reset two periods later, no sleep involved), so the architectural disable never actually disabled anything here; what looked like a stopped watchdog was one whose last reload happened to be stale. The park replaces "fires within seconds of a stop" with "fires after about 78 hours if nothing restores it", which only a sleep longer than that can reach; the commit message states it as a known limitation, and every other implementation keeps the architectural disable (park_on_stop is MediaTek-only). If a shorter window is preferred we can restore the timeout from the resume path earlier, but no ordering of register writes removes the finite period on this hardware.

The Low (start-path comment overstating when a parked period exists) is fixed in the new heads (81c08ea / c2a35f2).

On the GPU/DOCA compile status on 627: both GPU driver versions (615.71.09 and 580.178.04) compiled successfully against the PR kernel; the only failure is the DOCA 3.3.0 out-of-tree build (OFED 26.01.1.0.0.1), whose configure step exits 2 against 7.0.14. That is independent of a change in drivers/watchdog.

[clsotog]

@clsotog on the Boro High: it describes the design, and there is no software alternative on this implementation. We measured that clearing WCS.EN does not stop the compare on the MediaTek block (EN cleared, one refresh, reset two periods later, no sleep involved), so the architectural disable never actually disabled anything here; what looked like a stopped watchdog was one whose last reload happened to be stale. The park replaces "fires within seconds of a stop" with "fires after about 78 hours if nothing restores it", which only a sleep longer than that can reach; the commit message states it as a known limitation, and every other implementation keeps the architectural disable (park_on_stop is MediaTek-only). If a shorter window is preferred we can restore the timeout from the resume path earlier, but no ordering of register writes removes the finite period on this hardware.

thanks for the explanation but looks it will not close the finding from Boro. if MediaTek cannot be truly disabled in software, then ordinary .stop() must not report success. Parking is reasonable for sleep because there is a bounded suspend/resume transition that re-arms the watchdog. It is not a valid implementation of userspace disable/magic-close because the watchdog remains able to expire.

[dcemin-nv]

Thanks @clsotog for the thorough review, agreed, the explanation does not close it. New approach for the next heads: on the MediaTek implementation the driver will not offer a .stop op at all. The core then handles WDIOS_DISABLECARD and magic close as "hardware cannot be stopped" (WDOG_HW_RUNNING set, kernel worker keeps refreshing), which is the documented model for such hardware and the same path this driver already uses when it adopts a firmware armed watchdog at probe. No finite countdown after a userspace disable. Parking is confined to the system sleep path and to driver removal (with a warning), and other implementations keep the architectural disable on stop. Since the reboot notifier calls ops->stop directly, the MediaTek instance will not register it. We measured earlier that clearing WCS.EN with a fresh reload still resets this block, so the old stop never disabled anything here either; this change makes that explicit. Will push after board validation.

[dcemin-nv]

New heads pushed: 8be4fe9 on 627, 42f9ca3 on 628, same driver content on both. Changes since the last round:

• The MediaTek instance registers a watchdog_ops without .stop. WDIOS_DISABLECARD and magic close therefore keep the watchdog running with the core refreshing it (WDOG_HW_RUNNING), the same path the driver already takes for a watchdog found armed at probe. There is no finite countdown after a userspace disable any more.
• Parking is confined to the system sleep path and to driver removal, where it is the only option left and is logged with a warning.
• watchdog_stop_on_reboot() is not called for that instance, since the reboot notifier dereferences ops->stop. Other implementations are unchanged: architectural disable on stop, on reboot and around a sleep.
• Commit message rewritten to describe this; the known limitation now applies only to a multi-day sleep.

Validated on the reference laptop with a 10 s period (module inserted by hand, action=0): magic close and WDIOS_DISABLECARD followed by 2 and 40 minutes idle, no reset, where the old stop path reset the unit within about 10 s in the same test; four suspend cycles including two real sleeps of 1 to 2 minutes, all resumed with the period live; unbind logs "watchdog cannot be stopped, left parked" and the unit stays up, rebind restores the configured period.

[dcemin-nv]

On the new Boro High (version 0 MediaTek blocks still exposed as stoppable): new heads 63d7f3f on 627 and 7570293 on 628. The missing stop op is now gated on the implementer alone, so a version 0 MediaTek block gets no stop op either. Around a sleep it keeps the architectural disable, since its 32-bit WOR cannot hold a park period, and the removal warning reports it as armed rather than parked. No such part has been measured; the version 1 path is unchanged from the heads validated earlier today.

[clsotog]

I still get this finding:
MediaTek v0 is no longer exposed as userspace-stoppable, but suspend still treats WCS.EN clear as sufficient. Since v0 cannot be parked with a useful 32-bit WOR and MediaTek compare ignores WCS.EN, an armed v0 MediaTek watchdog should make suspend fail instead of proceeding with no refresh path.

[dcemin-nv]

New heads: 8a322fb on 627, 333499c on 628. Three changes for this round:

• @clsotog on the version 0 case: a system sleep is now refused while an armed watchdog can neither be stopped nor parked. sbsa_gwdt_sleep_stop() returns -EBUSY with a dev_err for an armed version 0 MediaTek instance, the PM notifier returns notifier_from_errno() and the device ->prepare callback propagates it, so the suspend aborts before anything is frozen instead of proceeding with no refresh path.
• Boro High on 627 (first start refreshes with a stale firmware period): set_timeout now programs WOR whenever no sleep transition is in progress, so the register holds a valid period before the first enable on every implementation, and sbsa_gwdt_hw_start() orders itself by the prior enable state. Block already enabled (restart after sleep, or a start while running): refresh, program, refresh, the order validated on hardware. Block disabled (first start): enable, program, refresh, so a zero or expired period left by firmware is never used for a reload.
• Boro High on 628 (set_timeout returns before the WOR commit, the core pings right behind it): every WOR write on the MediaTek block now reads back the control frame and waits for the slow-clock commit before returning, in both set_timeout and the start path.

Also the removal comment nit (Low on 627). Builds clean with W=1, checkpatch clean apart from the two pre-existing warnings. The restart-after-sleep sequence is unchanged apart from the commit wait; a sanity run on the reference laptop is in progress and I will report it here.

[clsotog]

thanks for the changes. I got this finding:
drivers/watchdog/sbsa_gwdt.c:739 exposes the instance to the PM notifier before wdd->timeout, max_hw_heartbeat_ms, and final timeout/action setup are initialized. The new resume path calls sbsa_gwdt_hw_start(), which now programs WOR from gwdt->wdd.timeout and clamps through max_hw_heartbeat_ms; a PM_POST_* race in that window can program WOR as 0 or with stale policy. This is new because the old hw_start() only wrote WCS.EN. Move the wdd timeout/max/action setup before list_add(), or keep the instance invisible to PM restart until it is fully initialized.

[dcemin-nv]

New heads: d2de08b on 627, b86b428 on 628.

• @clsotog on the probe ordering: agreed, the instance was visible to the PM notifier before its timeout, heartbeat limits and mode were final. The adopt and list-join block now runs after sbsa_gwdt_set_timeout(), once action, max_hw_heartbeat_ms and wdd->timeout are set, so a PM_POST_* racing the probe restarts the hardware with the configured period. Comments updated accordingly.
• Boro High on 628 (removal parks the watchdog for a finite period): the driver now sets suppress_bind_attrs. The watchdog core already holds a module reference while the hardware runs, so with the sysfs unbind path closed a running watchdog cannot lose its driver; only a probe failure can leave an armed instance behind, and that path keeps the park with its warning. This applies to every implementation: an architectural watchdog unbound while running was armed and unrefreshed as well, with a reset at the next period.
• Boro Low on 627 (comment claims core keepalives keep reloading during the park): the premise does not hold here. sbsa_gwdt_keepalive() does not look at the sleeping flag, and the core's keepalive kthread is not frozen, so pings continue until the CPUs go offline. The comment stands as written.

Builds clean with W=1, checkpatch clean apart from the two pre-existing warnings. The restart-after-sleep sequence is unchanged from the validated heads apart from the commit wait added in the previous round; the sanity run on the reference laptop for these two rounds is still pending and I will report it here.

[clsotog]

Thanks no more findings from me.
Acked-by: Carol L Soto <csoto@nvidia.com>

[nirmoy]

Acked-by: Nirmoy Das <nirmoyd@nvidia.com>

[nvmochs]

Merged, closing PR.

bf09ea9e9970 (nresolute/nvidia-bos-next) NVIDIA: SAUCE: watchdog: sbsa_gwdt: park the watchdog around system sleep on MediaTek implementations