[v2] x86: adjust initial setting of watchdog kind

Commit Message

Jan Beulich April 4, 2024, 12:09 p.m. UTC

"watchdog_timeout=0" is documented to disable the watchdog, which wasn't
really the case; the watchdog was rather ran with an, in practice,
infinite timeout. Fold that command line option with "watchdog",
resetting to defaults whenever encountering a new instance.

While touching opt_watchdog_timeout, also move it into .data.ro_after_init.

Signed-off-by: Jan Beulich <[e-mail]>
---
Really I think the comment in watchdog_setup() is wrong, and the
function would hence better go away. The CPU notifier registration can
surely be done in a pre-SMP initcall, which would have the benefit of
boot-time AP bringup then working the same as runtime CPU-onlining. (In
particular the set_timer() out of CPU_UP_PREPARE is a little suspicious,
as the timer can't possibly be run right away when a CPU isn't online
yet.) Which would leave __start_xen() to call watchdog_enable() in the
place it's calling watchdog_setup() now.
---
v2: Re-do largely from scratch.

Patch

--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,8 @@  The format is based on [Keep a Changelog
  - On x86:
    - HVM PIRQs are disabled by default.
    - Reduce IOMMU setup time for hardware domain.
+   - The "watchdog_timeout=" command line option is subsumed by the "watchdog="
+     one.
 
 ### Added
  - On x86:
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -2790,22 +2790,19 @@  suboptimal scheduling decisions, but onl
 oversubscribed (i.e., in total there are more vCPUs than pCPUs).
 
 ### watchdog (x86)
-> `= force | <boolean>`
+> `= <boolean> | List of [ force | <integer>s ]`
 
 > Default: `false`
 
 Run an NMI watchdog on each processor.  If a processor is stuck for
-longer than the **watchdog_timeout**, a panic occurs.  When `force` is
-specified, in addition to running an NMI watchdog on each processor,
-unknown NMIs will still be processed.
+longer than the specified or default timeout, a panic occurs.
 
-### watchdog_timeout (x86)
-> `= <integer>`
+*   When `force` is specified, in addition to running an NMI watchdog on each
+    processor, unknown NMIs will still be processed.
 
-> Default: `5`
-
-Set the NMI watchdog timeout in seconds.  Specifying `0` will turn off
-the watchdog.
+*   The <integer>s value allows to set the NMI watchdog timeout in seconds
+    (default: `5s`).  Specifying `0` will turn off the watchdog.  Specifying
+    a non-zero value enables the watchdog.
 
 ### x2apic (x86)
 > `= <boolean>`
--- a/xen/arch/x86/nmi.c
+++ b/xen/arch/x86/nmi.c
@@ -48,8 +48,20 @@  bool __initdata opt_watchdog;
 /* watchdog_force: If true, process unknown NMIs when running the watchdog. */
 bool watchdog_force;
 
+#define WATCHDOG_TIMEOUT_DEFAULT 5 /* seconds */
+
+/* opt_watchdog_timeout: Number of seconds to wait before panic. */
+static unsigned int __ro_after_init opt_watchdog_timeout = WATCHDOG_TIMEOUT_DEFAULT;
+
 static int __init cf_check parse_watchdog(const char *s)
 {
+    const char *ss;
+    int rc = 0;
+
+    /* Reset to defaults. */
+    watchdog_force = false;
+    opt_watchdog_timeout = WATCHDOG_TIMEOUT_DEFAULT;
+
     if ( !*s )
     {
         opt_watchdog = true;
@@ -66,28 +78,42 @@  static int __init cf_check parse_watchdo
         return 0;
     }
 
-    if ( !strcmp(s, "force") )
-        watchdog_force = opt_watchdog = true;
-    else
-        return -EINVAL;
+    do {
+        ss = strchr(s, ',');
+        if ( !ss )
+            ss = strchr(s, '\0');
+
+        if ( !cmdline_strcmp(s, "force") )
+        {
+            watchdog_force = opt_watchdog = true;
+
+            if ( !opt_watchdog_timeout )
+                opt_watchdog_timeout = WATCHDOG_TIMEOUT_DEFAULT;
+        }
+        else if ( isdigit(*s) )
+        {
+            unsigned long value = simple_strtoul(s, &s, 0);
+
+            if ( *s == 's' && s + 1 == ss )
+            {
+                opt_watchdog_timeout = value;
+                if ( opt_watchdog_timeout != value )
+                    opt_watchdog_timeout = WATCHDOG_TIMEOUT_DEFAULT;
+
+                opt_watchdog = !!opt_watchdog_timeout;
+            }
+            else
+                rc = -EINVAL;
+        }
+        else
+            rc = -EINVAL;
 
-    return 0;
-}
-custom_param("watchdog", parse_watchdog);
+        s = ss + 1;
+    } while ( *ss );
 
-/* opt_watchdog_timeout: Number of seconds to wait before panic. */
-static unsigned int opt_watchdog_timeout = 5;
-
-static int __init cf_check parse_watchdog_timeout(const char *s)
-{
-    const char *q;
-
-    opt_watchdog_timeout = simple_strtoull(s, &q, 0);
-    opt_watchdog = !!opt_watchdog_timeout;
-
-    return *q ? -EINVAL : 0;
+    return rc;
 }
-custom_param("watchdog_timeout", parse_watchdog_timeout);
+custom_param("watchdog", parse_watchdog);
 
 /*
  * lapic_nmi_owner tracks the ownership of the lapic NMI hardware: