Sourabh Jain [off-list ref] writes:

On pseries LPAR systems, watchdog timers configured from userspace
can remain active after a kernel panic. During panic triggered crash
dump capture, the crashing kernel jumps directly to the kdump kernel
without shutting down userspace services. As a result, active
watchdogs are not stopped before entering the kdump kernel.

If dump capture takes longer than the watchdog timeout, PHYP resets
the LPAR before dump collection completes, resulting in dump capture
failure.

Fix this by issuing the H_WATCHDOG hcall on the crash shutdown path
to stop all active watchdogs before booting the kdump kernel.

Nice catch!

would be nice, if we could avoid papr specific header into common crash.c

ditto.
Also I guess this could be FW_FEATURE_WATCHDOG

+
+		rc = plpar_hcall_norets_notrace(H_WATCHDOG, 0x200UL, -1);

- 0x200 is hardcoded.
- -1 is hardcoded.
- I think it's return value is long.

+		if (rc != H_SUCCESS && rc != H_NOOP)
+			pr_warn("crash: failed to stop watchdogs\n");

Let's print rc as well.

+	}
+}
+

Looking at the code, we already have a mechanism to register a crash
shutdown handler which anyways is getting called from
default_machine_crash_shutdown(). So, I think we could use this generic
crash handler register mechanism and keep the wdt specific calls within
pseries/setup.c file...

...How about something like this? 

diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 50b26ed8432d..4e557694d724 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c

@@ -59,6 +59,7 @@
 #include <asm/xics.h>
 #include <asm/xive.h>
 #include <asm/papr-sysparm.h>
+#include <asm/papr-watchdog.h>
 #include <asm/ppc-pci.h>
 #include <asm/i8259.h>
 #include <asm/udbg.h>

@@ -185,14 +186,42 @@ static void __init fwnmi_init(void)
 #endif
 }

<...>

+static void pseries_crash_stop_watchdogs(void)
+{
+       long rc;
+
+       rc = plpar_hcall_norets_notrace(H_WATCHDOG, PSERIES_WDTF_OP_STOP,
+                                       PSERIES_WDT_NUM_ALL);
+       if (rc != H_SUCCESS && rc != H_NOOP)
+               pr_warn("Could not stop watchdogs before kdump rc=%ld\n", rc);
+}
+
 /*
  * Affix a device for the first timer to the platform bus if
  * we have firmware support for the H_WATCHDOG hypercall.
  */
 static __init int pseries_wdt_init(void)
 {
-       if (firmware_has_feature(FW_FEATURE_WATCHDOG))
-               platform_device_register_simple("pseries-wdt", 0, NULL, 0);
+       if (!firmware_has_feature(FW_FEATURE_WATCHDOG))
+               return 0;
+
+       platform_device_register_simple("pseries-wdt", 0, NULL, 0);
+
+       if (crash_shutdown_register(pseries_crash_stop_watchdogs))
+               pr_warn("Could not register watchdog crash shutdown handler\n");
+
        return 0;
 }
 machine_subsys_initcall(pseries, pseries_wdt_init);


Note that I added papr-watchdog.h header file in above. I am guessing we
can move some definitions from drivers/watchdog/pseries-wdt.c to
arch/powerpc/include/asm/papr-watchdog.h in a separate patch before this
change.

I think you get the idea. Can you try this way and let me know if this works?

-ritesh