[PATCH v2 0/3] powerpc/crash: protect kdump from active watchdogs

[PATCH v2 0/3] powerpc/crash: protect kdump from active watchdogs

[Sourabh Jain]

On pseries LPAR systems in a high-availability environment using the
SBD[1][2] service, I observed that the system abruptly rebooted before
dump capture could complete.

Further investigation showed that SBD had configured a watchdog with
a 30-second timeout. Since the kernel crashes directly into the
kdump kernel without shutting down userspace services, the watchdog
remained active during dump capture. Once the watchdog timeout
expired, PHYP reset the LPAR, causing dump capture to fail.

The issue was reproducible only when the watchdog was active. Dump
capture completed successfully after disabling the watchdog,
stopping the SBD service, or increasing the watchdog timeout value.

This patch fixes the issue by stopping all active watchdogs on the
crash shutdown path before booting the kdump kernel.

Driver that export the hardware watchdog device is:
drivers/watchdog/pseries-wdt.c

[1] https://github.com/clusterlabs/sbd/blob/main/man/sbd.8.pod.in
[2] https://documentation.suse.com/sle-ha/15-SP4/html/SLE-HA-all/cha-ha-storage-protect.html

Changelog:
==========

v2:
 - Move H_WATCHDOG definitions to a common header for shared use
   across pseries code. 1/3
 - Added a new patch to handle pseries watchdog device registration
   failure. 2/3
 - Stop active watchdogs in crash hanlder. 3/3 Ritesh
 - Add suggested-by tag 1/3 & 3/3

v1:
https://lore.kernel.org/all/20260603070217.483696-1-sourabhjain@linux.ibm.com/


This issue can be reproduce using below program:
------------------------------------------------

#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>
#include <unistd.h>
#include <signal.h>
#include <errno.h>
#include <sys/ioctl.h>
#include <linux/watchdog.h>

#define WATCHDOG_DEV    "/dev/watchdog"
#define TIMEOUT         10
#define PET_INTERVAL    1

static int wdt_fd = -1;

static void watchdog_close(int disarm)
{
    int flags;

    if (wdt_fd < 0)
        return;

    if (disarm) {
        flags = WDIOS_DISABLECARD;
        if (ioctl(wdt_fd, WDIOC_SETOPTIONS, &flags) < 0)
            printf("WDIOS_DISABLECARD failed: %m (nowayout may be set)\n");
        else
            printf("Watchdog disabled via WDIOS_DISABLECARD\n");

        if (write(wdt_fd, "V", 1) < 0)
            printf("Magic 'V' write failed: %m\n");
        else
            printf("Magic 'V' written\n");
    } else {
        printf("Closing WITHOUT disarming - watchdog keeps running!\n");
    }

    close(wdt_fd);
    wdt_fd = -1;
    printf("Watchdog fd closed\n");
}

static void safe_exit(int sig)
{
    printf("\nSignal %d received - disarming watchdog...\n", sig);
    watchdog_close(1);
    exit(0);
}

static int watchdog_init(void)
{
    int flags, timeout = TIMEOUT;
    struct watchdog_info ident;

    printf("Opening %s...\n", WATCHDOG_DEV);
    wdt_fd = open(WATCHDOG_DEV, O_WRONLY);
    if (wdt_fd < 0) {
        printf("Failed to open %s: %m\n", WATCHDOG_DEV);
        return -1;
    }
    printf("Watchdog opened and ARMED\n");

    flags = WDIOS_ENABLECARD;
    if (ioctl(wdt_fd, WDIOC_SETOPTIONS, &flags) < 0)
        /* ENOTTY = driver always enabled, that's fine */
        printf("WDIOS_ENABLECARD: %m (ok if ENOTTY)\n");
    else
        printf("Watchdog enabled via WDIOS_ENABLECARD\n");

    if (ioctl(wdt_fd, WDIOC_SETTIMEOUT, &timeout) < 0)
        printf("WDIOC_SETTIMEOUT failed: %m\n");
    else
        printf("Timeout set to %d seconds\n", timeout);

    /* verify what the driver actually set */
    if (ioctl(wdt_fd, WDIOC_GETTIMEOUT, &timeout) == 0)
        printf("Actual timeout  : %d seconds\n", timeout);

    if (ioctl(wdt_fd, WDIOC_GETSUPPORT, &ident) == 0)
        printf("Identity        : %s\n", ident.identity);

    return 0;
}

static void watchdog_tickle(void)
{
    int timeleft = 0;

    if (ioctl(wdt_fd, WDIOC_KEEPALIVE, 0) < 0) {
        printf("WDIOC_KEEPALIVE failed: %m - falling back to write\n");
        write(wdt_fd, "1", 1);
    }

    if (ioctl(wdt_fd, WDIOC_GETTIMELEFT, &timeleft) == 0)
        printf("Petted watchdog. Timeleft: %d sec\n", timeleft);
    else
        printf("Petted watchdog.\n");
}

int main(void)
{
    signal(SIGINT,  safe_exit);
    signal(SIGTERM, safe_exit);

    if (watchdog_init() < 0)
        return 1;

    printf("\nPetting every %d seconds. Ctrl+C to safely stop.\n\n",
           PET_INTERVAL);

    while (1) {
        watchdog_tickle();
        sleep(PET_INTERVAL);
    }

    return 0;
}

Steps to reproduce the issue:
-----------------------------
1. Load kdump kernel
2. Insert pseries-wdt driver
3. Compile the above proram and run the binary
4. Crash the kernel (echo c > /proc/sysrq-trigger)

Sourabh Jain (3):
  powerpc/pseries: Move H_WATCHDOG definitions to a common header
  powerpc/pseries: Handle and log pseries-wdt registration failures
  powerpc/crash: stop watchdogs before booting kdump kernel

 arch/powerpc/include/asm/papr-watchdog.h | 60 ++++++++++++++++++++++++
 arch/powerpc/platforms/pseries/setup.c   | 32 ++++++++++++-
 drivers/watchdog/pseries-wdt.c           | 53 +--------------------
 3 files changed, 91 insertions(+), 54 deletions(-)
 create mode 100644 arch/powerpc/include/asm/papr-watchdog.h

-- 
2.52.0

[PATCH v2 1/3] powerpc/pseries: Move H_WATCHDOG definitions to a common header

[Sourabh Jain]

The H_WATCHDOG input and output definitions are currently local to the
pseries watchdog driver. The next patch in this series also needs these
definitions to issue H_WATCHDOG hypercalls outside the watchdog driver.

Move the H_WATCHDOG definitions to a new common header,
asm/papr-watchdog.h, so they can be shared without duplicating the
PAPR watchdog definitions.

No functional changes.

Suggested-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
 arch/powerpc/include/asm/papr-watchdog.h | 58 ++++++++++++++++++++++++
 drivers/watchdog/pseries-wdt.c           | 53 +---------------------
 2 files changed, 59 insertions(+), 52 deletions(-)
 create mode 100644 arch/powerpc/include/asm/papr-watchdog.h

diff --git a/arch/powerpc/include/asm/papr-watchdog.h b/arch/powerpc/include/asm/papr-watchdog.h
new file mode 100644
index 000000000000..fb3a511aa861
--- /dev/null
+++ b/arch/powerpc/include/asm/papr-watchdog.h
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _ASM_POWERPC_CRASHDUMP_PPC64_H
+#define _ASM_POWERPC_CRASHDUMP_PPC64_H
+
+/*
+ * H_WATCHDOG Input
+ *
+ * R4: "flags":
+ *
+ *         Bits 48-55: "operation"
+ */
+#define PSERIES_WDTF_OP_START	0x100UL		/* start timer */
+#define PSERIES_WDTF_OP_STOP	0x200UL		/* stop timer */
+#define PSERIES_WDTF_OP_QUERY	0x300UL		/* query timer capabilities */
+
+/*
+ *         Bits 56-63: "timeoutAction" (for "Start Watchdog" only)
+ */
+#define PSERIES_WDTF_ACTION_HARD_POWEROFF	0x1UL	/* poweroff */
+#define PSERIES_WDTF_ACTION_HARD_RESTART	0x2UL	/* restart */
+#define PSERIES_WDTF_ACTION_DUMP_RESTART	0x3UL	/* dump + restart */
+
+/*
+ * H_WATCHDOG Output
+ *
+ * R3: Return code
+ *
+ *     H_SUCCESS    The operation completed.
+ *
+ *     H_BUSY	    The hypervisor is too busy; retry the operation.
+ *
+ *     H_PARAMETER  The given "flags" are somehow invalid.  Either the
+ *                  "operation" or "timeoutAction" is invalid, or a
+ *                  reserved bit is set.
+ *
+ *     H_P2         The given "watchdogNumber" is zero or exceeds the
+ *                  supported maximum value.
+ *
+ *     H_P3         The given "timeoutInMs" is below the supported
+ *                  minimum value.
+ *
+ *     H_NOOP       The given "watchdogNumber" is already stopped.
+ *
+ *     H_HARDWARE   The operation failed for ineffable reasons.
+ *
+ *     H_FUNCTION   The H_WATCHDOG hypercall is not supported by this
+ *                  hypervisor.
+ *
+ * R4:
+ *
+ * - For the "Query Watchdog Capabilities" operation, a 64-bit
+ *   structure:
+ */
+#define PSERIES_WDTQ_MIN_TIMEOUT(cap)	(((cap) >> 48) & 0xffff)
+#define PSERIES_WDTQ_MAX_NUMBER(cap)	(((cap) >> 32) & 0xffff)
+
+#endif /* _ASM_POWERPC_CRASHDUMP_PPC64_H */
diff --git a/drivers/watchdog/pseries-wdt.c b/drivers/watchdog/pseries-wdt.c
index 48d67f7c972a..e97b943e1d3c 100644
--- a/drivers/watchdog/pseries-wdt.c
+++ b/drivers/watchdog/pseries-wdt.c
@@ -12,61 +12,10 @@
 #include <linux/platform_device.h>
 #include <linux/time64.h>
 #include <linux/watchdog.h>
+#include <asm/papr-watchdog.h>
 
 #define DRV_NAME "pseries-wdt"
 
-/*
- * H_WATCHDOG Input
- *
- * R4: "flags":
- *
- *         Bits 48-55: "operation"
- */
-#define PSERIES_WDTF_OP_START	0x100UL		/* start timer */
-#define PSERIES_WDTF_OP_STOP	0x200UL		/* stop timer */
-#define PSERIES_WDTF_OP_QUERY	0x300UL		/* query timer capabilities */
-
-/*
- *         Bits 56-63: "timeoutAction" (for "Start Watchdog" only)
- */
-#define PSERIES_WDTF_ACTION_HARD_POWEROFF	0x1UL	/* poweroff */
-#define PSERIES_WDTF_ACTION_HARD_RESTART	0x2UL	/* restart */
-#define PSERIES_WDTF_ACTION_DUMP_RESTART	0x3UL	/* dump + restart */
-
-/*
- * H_WATCHDOG Output
- *
- * R3: Return code
- *
- *     H_SUCCESS    The operation completed.
- *
- *     H_BUSY	    The hypervisor is too busy; retry the operation.
- *
- *     H_PARAMETER  The given "flags" are somehow invalid.  Either the
- *                  "operation" or "timeoutAction" is invalid, or a
- *                  reserved bit is set.
- *
- *     H_P2         The given "watchdogNumber" is zero or exceeds the
- *                  supported maximum value.
- *
- *     H_P3         The given "timeoutInMs" is below the supported
- *                  minimum value.
- *
- *     H_NOOP       The given "watchdogNumber" is already stopped.
- *
- *     H_HARDWARE   The operation failed for ineffable reasons.
- *
- *     H_FUNCTION   The H_WATCHDOG hypercall is not supported by this
- *                  hypervisor.
- *
- * R4:
- *
- * - For the "Query Watchdog Capabilities" operation, a 64-bit
- *   structure:
- */
-#define PSERIES_WDTQ_MIN_TIMEOUT(cap)	(((cap) >> 48) & 0xffff)
-#define PSERIES_WDTQ_MAX_NUMBER(cap)	(((cap) >> 32) & 0xffff)
-
 static const unsigned long pseries_wdt_action[] = {
 	[0] = PSERIES_WDTF_ACTION_HARD_POWEROFF,
 	[1] = PSERIES_WDTF_ACTION_HARD_RESTART,
-- 
2.52.0

[PATCH v2 2/3] powerpc/pseries: Handle and log pseries-wdt registration failures

[Sourabh Jain]

The pseries watchdog initialization registers the pseries-wdt platform
device using platform_device_register_simple(), but currently ignores
its return value.

Check the returned pointer for errors, log a descriptive error message
when registration fails, and propagate the failure code to the caller.
This avoids silently ignoring platform device registration failures.

Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/setup.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 1223dc961242..bbb2813f8ede 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -191,8 +191,18 @@ static void __init fwnmi_init(void)
  */
 static __init int pseries_wdt_init(void)
 {
-	if (firmware_has_feature(FW_FEATURE_WATCHDOG))
-		platform_device_register_simple("pseries-wdt", 0, NULL, 0);
+	struct platform_device *pseries_wdt_dev;
+
+	if (!firmware_has_feature(FW_FEATURE_WATCHDOG))
+		return 0;
+
+	pseries_wdt_dev = platform_device_register_simple("pseries-wdt", 0, NULL, 0);
+
+	if (IS_ERR(pseries_wdt_dev)) {
+		pr_err("Failed to register pseries-wdt platform device\n");
+		return PTR_ERR(pseries_wdt_dev);
+	}
+
 	return 0;
 }
 machine_subsys_initcall(pseries, pseries_wdt_init);
-- 
2.52.0

[PATCH v2 3/3] powerpc/crash: stop watchdogs before booting kdump kernel

[Sourabh Jain]

On pseries LPAR systems, watchdog timers configured from userspace can
remain active after a kernel panic. When a panic triggers kdump, the
crashing kernel jumps directly to the kdump kernel without stopping
active watchdogs. As a result, the watchdogs remain active after the
kdump kernel starts.

If dump capture takes longer than the watchdog timeout, PHYP resets the
LPAR before the dump is fully captured, causing dump capture to fail.

Fix this by issuing the `H_WATCHDOG` hcall during the crash shutdown
sequence to stop all active watchdogs before booting the kdump kernel.

Fixes: 69472ffa6575 ("watchdog/pseries-wdt: initial support for H_WATCHDOG-based watchdog timers")
Reported-by: Mahesh Kumar G <mahe657@linux.ibm.com>
Suggested-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
 arch/powerpc/include/asm/papr-watchdog.h |  2 ++
 arch/powerpc/platforms/pseries/setup.c   | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/arch/powerpc/include/asm/papr-watchdog.h b/arch/powerpc/include/asm/papr-watchdog.h
index fb3a511aa861..84bbe1ddd56f 100644
--- a/arch/powerpc/include/asm/papr-watchdog.h
+++ b/arch/powerpc/include/asm/papr-watchdog.h
@@ -55,4 +55,6 @@
 #define PSERIES_WDTQ_MIN_TIMEOUT(cap)	(((cap) >> 48) & 0xffff)
 #define PSERIES_WDTQ_MAX_NUMBER(cap)	(((cap) >> 32) & 0xffff)
 
+#define PSERIES_WDT_NUM_ALL	((unsigned long)-1)
+
 #endif /* _ASM_POWERPC_CRASHDUMP_PPC64_H */
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index bbb2813f8ede..2e40a9dba637 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -77,6 +77,7 @@
 #include <asm/dtl.h>
 #include <asm/hvconsole.h>
 #include <asm/setup.h>
+#include <asm/papr-watchdog.h>
 
 #include "pseries.h"
 
@@ -185,6 +186,18 @@ static void __init fwnmi_init(void)
 #endif
 }
 
+#ifdef CONFIG_CRASH_DUMP
+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);
+}
+#endif /* CONFIG_CRASH_DUMP */
+
 /*
  * Affix a device for the first timer to the platform bus if
  * we have firmware support for the H_WATCHDOG hypercall.
@@ -203,6 +216,11 @@ static __init int pseries_wdt_init(void)
 		return PTR_ERR(pseries_wdt_dev);
 	}
 
+#ifdef CONFIG_CRASH_DUMP
+	if (crash_shutdown_register(pseries_crash_stop_watchdogs))
+		pr_warn("Could not register watchdog crash shutdown handler\n");
+#endif
+
 	return 0;
 }
 machine_subsys_initcall(pseries, pseries_wdt_init);
-- 
2.52.0

Re: [PATCH v2 1/3] powerpc/pseries: Move H_WATCHDOG definitions to a common header

[Ritesh Harjani]

Sourabh Jain <sourabhjain@linux.ibm.com> writes:

> The H_WATCHDOG input and output definitions are currently local to the
> pseries watchdog driver. The next patch in this series also needs these
> definitions to issue H_WATCHDOG hypercalls outside the watchdog driver.
>
> Move the H_WATCHDOG definitions to a new common header,
> asm/papr-watchdog.h, so they can be shared without duplicating the
> PAPR watchdog definitions.
>
> No functional changes.
>
> Suggested-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/papr-watchdog.h | 58 ++++++++++++++++++++++++
>  drivers/watchdog/pseries-wdt.c           | 53 +---------------------
>  2 files changed, 59 insertions(+), 52 deletions(-)
>  create mode 100644 arch/powerpc/include/asm/papr-watchdog.h
>
> diff --git a/arch/powerpc/include/asm/papr-watchdog.h b/arch/powerpc/include/asm/papr-watchdog.h
> new file mode 100644
> index 000000000000..fb3a511aa861
> --- /dev/null
> +++ b/arch/powerpc/include/asm/papr-watchdog.h
> @@ -0,0 +1,58 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef _ASM_POWERPC_CRASHDUMP_PPC64_H
> +#define _ASM_POWERPC_CRASHDUMP_PPC64_H

should be _ASM_POWERPC_PAPR_WATCHDOG_H

> +
> +/*
> + * H_WATCHDOG Input
> + *
> + * R4: "flags":
> + *
> + *         Bits 48-55: "operation"
> + */
> +#define PSERIES_WDTF_OP_START	0x100UL		/* start timer */
> +#define PSERIES_WDTF_OP_STOP	0x200UL		/* stop timer */
> +#define PSERIES_WDTF_OP_QUERY	0x300UL		/* query timer capabilities */
> +
> +/*
> + *         Bits 56-63: "timeoutAction" (for "Start Watchdog" only)
> + */
> +#define PSERIES_WDTF_ACTION_HARD_POWEROFF	0x1UL	/* poweroff */
> +#define PSERIES_WDTF_ACTION_HARD_RESTART	0x2UL	/* restart */
> +#define PSERIES_WDTF_ACTION_DUMP_RESTART	0x3UL	/* dump + restart */
> +
> +/*
> + * H_WATCHDOG Output
> + *
> + * R3: Return code
> + *
> + *     H_SUCCESS    The operation completed.
> + *
> + *     H_BUSY	    The hypervisor is too busy; retry the operation.
> + *
> + *     H_PARAMETER  The given "flags" are somehow invalid.  Either the
> + *                  "operation" or "timeoutAction" is invalid, or a
> + *                  reserved bit is set.
> + *
> + *     H_P2         The given "watchdogNumber" is zero or exceeds the
> + *                  supported maximum value.
> + *
> + *     H_P3         The given "timeoutInMs" is below the supported
> + *                  minimum value.
> + *
> + *     H_NOOP       The given "watchdogNumber" is already stopped.
> + *
> + *     H_HARDWARE   The operation failed for ineffable reasons.
> + *
> + *     H_FUNCTION   The H_WATCHDOG hypercall is not supported by this
> + *                  hypervisor.
> + *
> + * R4:
> + *
> + * - For the "Query Watchdog Capabilities" operation, a 64-bit
> + *   structure:
> + */
> +#define PSERIES_WDTQ_MIN_TIMEOUT(cap)	(((cap) >> 48) & 0xffff)
> +#define PSERIES_WDTQ_MAX_NUMBER(cap)	(((cap) >> 32) & 0xffff)
> +
> +#endif /* _ASM_POWERPC_CRASHDUMP_PPC64_H */

ditto

-ritesh

Re: [PATCH v2 1/3] powerpc/pseries: Move H_WATCHDOG definitions to a common header

[Sourabh Jain]

On 13/07/26 09:48, Ritesh Harjani (IBM) wrote:
> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>
>> The H_WATCHDOG input and output definitions are currently local to the
>> pseries watchdog driver. The next patch in this series also needs these
>> definitions to issue H_WATCHDOG hypercalls outside the watchdog driver.
>>
>> Move the H_WATCHDOG definitions to a new common header,
>> asm/papr-watchdog.h, so they can be shared without duplicating the
>> PAPR watchdog definitions.
>>
>> No functional changes.
>>
>> Suggested-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> ---
>>   arch/powerpc/include/asm/papr-watchdog.h | 58 ++++++++++++++++++++++++
>>   drivers/watchdog/pseries-wdt.c           | 53 +---------------------
>>   2 files changed, 59 insertions(+), 52 deletions(-)
>>   create mode 100644 arch/powerpc/include/asm/papr-watchdog.h
>>
>> diff --git a/arch/powerpc/include/asm/papr-watchdog.h b/arch/powerpc/include/asm/papr-watchdog.h
>> new file mode 100644
>> index 000000000000..fb3a511aa861
>> --- /dev/null
>> +++ b/arch/powerpc/include/asm/papr-watchdog.h
>> @@ -0,0 +1,58 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +
>> +#ifndef _ASM_POWERPC_CRASHDUMP_PPC64_H
>> +#define _ASM_POWERPC_CRASHDUMP_PPC64_H
> should be _ASM_POWERPC_PAPR_WATCHDOG_H

oops my bad. I will fix it in v3.


>
>> +
>> +/*
>> + * H_WATCHDOG Input
>> + *
>> + * R4: "flags":
>> + *
>> + *         Bits 48-55: "operation"
>> + */
>> +#define PSERIES_WDTF_OP_START	0x100UL		/* start timer */
>> +#define PSERIES_WDTF_OP_STOP	0x200UL		/* stop timer */
>> +#define PSERIES_WDTF_OP_QUERY	0x300UL		/* query timer capabilities */
>> +
>> +/*
>> + *         Bits 56-63: "timeoutAction" (for "Start Watchdog" only)
>> + */
>> +#define PSERIES_WDTF_ACTION_HARD_POWEROFF	0x1UL	/* poweroff */
>> +#define PSERIES_WDTF_ACTION_HARD_RESTART	0x2UL	/* restart */
>> +#define PSERIES_WDTF_ACTION_DUMP_RESTART	0x3UL	/* dump + restart */
>> +
>> +/*
>> + * H_WATCHDOG Output
>> + *
>> + * R3: Return code
>> + *
>> + *     H_SUCCESS    The operation completed.
>> + *
>> + *     H_BUSY	    The hypervisor is too busy; retry the operation.
>> + *
>> + *     H_PARAMETER  The given "flags" are somehow invalid.  Either the
>> + *                  "operation" or "timeoutAction" is invalid, or a
>> + *                  reserved bit is set.
>> + *
>> + *     H_P2         The given "watchdogNumber" is zero or exceeds the
>> + *                  supported maximum value.
>> + *
>> + *     H_P3         The given "timeoutInMs" is below the supported
>> + *                  minimum value.
>> + *
>> + *     H_NOOP       The given "watchdogNumber" is already stopped.
>> + *
>> + *     H_HARDWARE   The operation failed for ineffable reasons.
>> + *
>> + *     H_FUNCTION   The H_WATCHDOG hypercall is not supported by this
>> + *                  hypervisor.
>> + *
>> + * R4:
>> + *
>> + * - For the "Query Watchdog Capabilities" operation, a 64-bit
>> + *   structure:
>> + */
>> +#define PSERIES_WDTQ_MIN_TIMEOUT(cap)	(((cap) >> 48) & 0xffff)
>> +#define PSERIES_WDTQ_MAX_NUMBER(cap)	(((cap) >> 32) & 0xffff)
>> +
>> +#endif /* _ASM_POWERPC_CRASHDUMP_PPC64_H */
> ditto

Re: [PATCH v2 2/3] powerpc/pseries: Handle and log pseries-wdt registration failures

[Ritesh Harjani]

Sourabh Jain <sourabhjain@linux.ibm.com> writes:

> The pseries watchdog initialization registers the pseries-wdt platform
> device using platform_device_register_simple(), but currently ignores
> its return value.
>
> Check the returned pointer for errors, log a descriptive error message
> when registration fails, and propagate the failure code to the caller.
> This avoids silently ignoring platform device registration failures.
>

Fair enough.

> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/setup.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
> index 1223dc961242..bbb2813f8ede 100644
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -191,8 +191,18 @@ static void __init fwnmi_init(void)
>   */
>  static __init int pseries_wdt_init(void)
>  {
> -	if (firmware_has_feature(FW_FEATURE_WATCHDOG))
> -		platform_device_register_simple("pseries-wdt", 0, NULL, 0);
> +	struct platform_device *pseries_wdt_dev;

minor nit: we should rename this to pdev, since it is already under
pseries_wdt_init(). That is generally how all platform drivers use it
unless it requires more than one platform device.

But either ways the patch looks good to me:

Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>

> +
> +	if (!firmware_has_feature(FW_FEATURE_WATCHDOG))
> +		return 0;
> +
> +	pseries_wdt_dev = platform_device_register_simple("pseries-wdt", 0, NULL, 0);
> +
> +	if (IS_ERR(pseries_wdt_dev)) {
> +		pr_err("Failed to register pseries-wdt platform device\n");
> +		return PTR_ERR(pseries_wdt_dev);
> +	}
> +
>  	return 0;
>  }
>  machine_subsys_initcall(pseries, pseries_wdt_init);
> -- 
> 2.52.0

Re: [PATCH v2 2/3] powerpc/pseries: Handle and log pseries-wdt registration failures

[Sourabh Jain]

On 13/07/26 09:59, Ritesh Harjani (IBM) wrote:
> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>
>> The pseries watchdog initialization registers the pseries-wdt platform
>> device using platform_device_register_simple(), but currently ignores
>> its return value.
>>
>> Check the returned pointer for errors, log a descriptive error message
>> when registration fails, and propagate the failure code to the caller.
>> This avoids silently ignoring platform device registration failures.
>>
> Fair enough.
>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> ---
>>   arch/powerpc/platforms/pseries/setup.c | 14 ++++++++++++--
>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
>> index 1223dc961242..bbb2813f8ede 100644
>> --- a/arch/powerpc/platforms/pseries/setup.c
>> +++ b/arch/powerpc/platforms/pseries/setup.c
>> @@ -191,8 +191,18 @@ static void __init fwnmi_init(void)
>>    */
>>   static __init int pseries_wdt_init(void)
>>   {
>> -	if (firmware_has_feature(FW_FEATURE_WATCHDOG))
>> -		platform_device_register_simple("pseries-wdt", 0, NULL, 0);
>> +	struct platform_device *pseries_wdt_dev;
> minor nit: we should rename this to pdev, since it is already under
> pseries_wdt_init(). That is generally how all platform drivers use it
> unless it requires more than one platform device.
>
> But either ways the patch looks good to me:
>
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>

Thanks for the review. I will rename the variable in next version.

- Sourabh Jain

>
>> +
>> +	if (!firmware_has_feature(FW_FEATURE_WATCHDOG))
>> +		return 0;
>> +
>> +	pseries_wdt_dev = platform_device_register_simple("pseries-wdt", 0, NULL, 0);
>> +
>> +	if (IS_ERR(pseries_wdt_dev)) {
>> +		pr_err("Failed to register pseries-wdt platform device\n");
>> +		return PTR_ERR(pseries_wdt_dev);
>> +	}
>> +
>>   	return 0;
>>   }
>>   machine_subsys_initcall(pseries, pseries_wdt_init);
>> -- 
>> 2.52.0

Re: [PATCH v2 3/3] powerpc/crash: stop watchdogs before booting kdump kernel

[Ritesh Harjani]

Sourabh Jain <sourabhjain@linux.ibm.com> writes:

> On pseries LPAR systems, watchdog timers configured from userspace can
> remain active after a kernel panic. When a panic triggers kdump, the
> crashing kernel jumps directly to the kdump kernel without stopping
> active watchdogs. As a result, the watchdogs remain active after the
> kdump kernel starts.
>
> If dump capture takes longer than the watchdog timeout, PHYP resets the
> LPAR before the dump is fully captured, causing dump capture to fail.
>
> Fix this by issuing the `H_WATCHDOG` hcall during the crash shutdown
> sequence to stop all active watchdogs before booting the kdump kernel.
>
> Fixes: 69472ffa6575 ("watchdog/pseries-wdt: initial support for H_WATCHDOG-based watchdog timers")
> Reported-by: Mahesh Kumar G <mahe657@linux.ibm.com>
> Suggested-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/papr-watchdog.h |  2 ++
>  arch/powerpc/platforms/pseries/setup.c   | 18 ++++++++++++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/papr-watchdog.h b/arch/powerpc/include/asm/papr-watchdog.h
> index fb3a511aa861..84bbe1ddd56f 100644
> --- a/arch/powerpc/include/asm/papr-watchdog.h
> +++ b/arch/powerpc/include/asm/papr-watchdog.h
> @@ -55,4 +55,6 @@
>  #define PSERIES_WDTQ_MIN_TIMEOUT(cap)	(((cap) >> 48) & 0xffff)
>  #define PSERIES_WDTQ_MAX_NUMBER(cap)	(((cap) >> 32) & 0xffff)
>  
> +#define PSERIES_WDT_NUM_ALL	((unsigned long)-1)
> +

minor nit:

This should be defined at the end of the H_WATCHDOG Input section.
/*
 * H_WATCHDOG Input
 *

<...>

Something like this maybe?

/*
 * R5: "watchdogNumber":
 *       PAPR says use -1 (all ones) to stop all watchdogs.
 */
#define PSERIES_WDT_NUM_ALL	((unsigned long)-1)

/*
 * H_WATCHDOG Output
 *
 * R3: Return code
 *
 <...> 

>  #endif /* _ASM_POWERPC_CRASHDUMP_PPC64_H */
> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
> index bbb2813f8ede..2e40a9dba637 100644
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -77,6 +77,7 @@
>  #include <asm/dtl.h>
>  #include <asm/hvconsole.h>
>  #include <asm/setup.h>
> +#include <asm/papr-watchdog.h>
>  
>  #include "pseries.h"
>  
> @@ -185,6 +186,18 @@ static void __init fwnmi_init(void)
>  #endif
>  }
>  
> +#ifdef CONFIG_CRASH_DUMP
> +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);
> +}
> +#endif /* CONFIG_CRASH_DUMP */
> +
>  /*
>   * Affix a device for the first timer to the platform bus if
>   * we have firmware support for the H_WATCHDOG hypercall.
> @@ -203,6 +216,11 @@ static __init int pseries_wdt_init(void)
>  		return PTR_ERR(pseries_wdt_dev);
>  	}
>  
> +#ifdef CONFIG_CRASH_DUMP
> +	if (crash_shutdown_register(pseries_crash_stop_watchdogs))
> +		pr_warn("Could not register watchdog crash shutdown handler\n");
> +#endif
> +

minor nit:
I don't think we need any of the #ifdef. All definitions used inside
pseries_crash_stop_watchdogs are already available and
crash_shutdown_register() already exists for !CONFIG_CRASH_DUMP, so we
may as well drop all of the ifdefs.


Otherwise LGTM, so feel free to add:
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>

Re: [PATCH v2 0/3] powerpc/crash: protect kdump from active watchdogs

[Ritesh Harjani]

Sourabh Jain <sourabhjain@linux.ibm.com> writes:

> Changelog:
> ==========
>
> v2:
>  - Move H_WATCHDOG definitions to a common header for shared use
>    across pseries code. 1/3
>  - Added a new patch to handle pseries watchdog device registration
>    failure. 2/3
>  - Stop active watchdogs in crash hanlder. 3/3 Ritesh
>  - Add suggested-by tag 1/3 & 3/3


Reviewed the changes and mostly looks good with some minor nits added to
the individual patches.

Small request -
Could you please also update test results with v3 in your changelog
(since you mentioned we are able to reproduce the issue easily with your
test code).


aah one other thing I just noticed since you are ccing stable and you
added a Fixes tag in patch-3.
Patch-3 alone cannot be easily backported now due to patch-1 and
patch-2. There must be a way to define the dependencies if you are
looking for backporting the fix patch to stable tree, please check that
and follow that accordingly in v3.

-ritesh

Re: [PATCH v2 0/3] powerpc/crash: protect kdump from active watchdogs

[Sourabh Jain]

On 13/07/26 10:51, Ritesh Harjani (IBM) wrote:
> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>
>> Changelog:
>> ==========
>>
>> v2:
>>   - Move H_WATCHDOG definitions to a common header for shared use
>>     across pseries code. 1/3
>>   - Added a new patch to handle pseries watchdog device registration
>>     failure. 2/3
>>   - Stop active watchdogs in crash hanlder. 3/3 Ritesh
>>   - Add suggested-by tag 1/3 & 3/3
>
> Reviewed the changes and mostly looks good with some minor nits added to
> the individual patches.
>
> Small request -
> Could you please also update test results with v3 in your changelog
> (since you mentioned we are able to reproduce the issue easily with your
> test code).

I tested this fix with the program I shared in cover letter. The watchdog
was successfully stopped even when H_WATCHDOG is called form crash
handler.  I will share my test details in v3 cover letter also.

>
>
> aah one other thing I just noticed since you are ccing stable and you
> added a Fixes tag in patch-3.
> Patch-3 alone cannot be easily backported now due to patch-1 and
> patch-2. There must be a way to define the dependencies if you are
> looking for backporting the fix patch to stable tree, please check that
> and follow that accordingly in v3.

I thought about that as well, but since they are part of the same patch 
series,
I assumed they would be picked together. However, I don't think that 
will work
in all cases.

I checked the older commits and noticed that a backport note was added. 
I think
we can do the same for the fix patch. I'll add a note indicating that the
following patches should be backported first:

powerpc/pseries: Move H_WATCHDOG definitions to a common header
powerpc/pseries: Handle and log pseries-wdt registration failures

Since these patches are not upstream yet, I'll refer to them by their 
commit titles.

Does that look good to you?

- Sourabh Jain

Re: [PATCH v2 0/3] powerpc/crash: protect kdump from active watchdogs

[Ritesh Harjani]

Sourabh Jain <sourabhjain@linux.ibm.com> writes:

> On 13/07/26 10:51, Ritesh Harjani (IBM) wrote:
>> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>>
>>> Changelog:
>>> ==========
>>>
>>> v2:
>>>   - Move H_WATCHDOG definitions to a common header for shared use
>>>     across pseries code. 1/3
>>>   - Added a new patch to handle pseries watchdog device registration
>>>     failure. 2/3
>>>   - Stop active watchdogs in crash hanlder. 3/3 Ritesh
>>>   - Add suggested-by tag 1/3 & 3/3
>>
>> Reviewed the changes and mostly looks good with some minor nits added to
>> the individual patches.
>>
>> Small request -
>> Could you please also update test results with v3 in your changelog
>> (since you mentioned we are able to reproduce the issue easily with your
>> test code).
>
> I tested this fix with the program I shared in cover letter. The watchdog
> was successfully stopped even when H_WATCHDOG is called form crash
> handler.  I will share my test details in v3 cover letter also.
>
>>
>>
>> aah one other thing I just noticed since you are ccing stable and you
>> added a Fixes tag in patch-3.
>> Patch-3 alone cannot be easily backported now due to patch-1 and
>> patch-2. There must be a way to define the dependencies if you are
>> looking for backporting the fix patch to stable tree, please check that
>> and follow that accordingly in v3.
>
> I thought about that as well, but since they are part of the same patch 
> series,
> I assumed they would be picked together. However, I don't think that 
> will work
> in all cases.
>
> I checked the older commits and noticed that a backport note was added. 
> I think
> we can do the same for the fix patch. I'll add a note indicating that the
> following patches should be backported first:
>
> powerpc/pseries: Move H_WATCHDOG definitions to a common header
> powerpc/pseries: Handle and log pseries-wdt registration failures
>
> Since these patches are not upstream yet, I'll refer to them by their 
> commit titles.
>
> Does that look good to you?
>

Documentation/process/stable-kernel-rules.rst
  Note that for a patch series, you do not have to list as prerequisites the
  patches present in the series itself. For example, if you have the following
  patch series::

    patch1
    patch2

  where patch2 depends on patch1, you do not have to list patch1 as
  prerequisite of patch2 if you have already marked patch1 for stable
  inclusion.


In that case, I think, we should mark all 3 patches for stable inclusion.

  patch 1/3   Cc: stable@vger.kernel.org
  patch 2/3   Cc: stable@vger.kernel.org
  patch 3/3   Cc: stable@vger.kernel.org
              Fixes: 69472ffa6575 ("watchdog/pseries-wdt: initial support for H_WATCHDOG-based watchdog timers")

-ritesh

Re: [PATCH v2 0/3] powerpc/crash: protect kdump from active watchdogs

[Sourabh Jain]

On 13/07/26 12:10, Ritesh Harjani (IBM) wrote:
> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>
>> On 13/07/26 10:51, Ritesh Harjani (IBM) wrote:
>>> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>>>
>>>> Changelog:
>>>> ==========
>>>>
>>>> v2:
>>>>    - Move H_WATCHDOG definitions to a common header for shared use
>>>>      across pseries code. 1/3
>>>>    - Added a new patch to handle pseries watchdog device registration
>>>>      failure. 2/3
>>>>    - Stop active watchdogs in crash hanlder. 3/3 Ritesh
>>>>    - Add suggested-by tag 1/3 & 3/3
>>> Reviewed the changes and mostly looks good with some minor nits added to
>>> the individual patches.
>>>
>>> Small request -
>>> Could you please also update test results with v3 in your changelog
>>> (since you mentioned we are able to reproduce the issue easily with your
>>> test code).
>> I tested this fix with the program I shared in cover letter. The watchdog
>> was successfully stopped even when H_WATCHDOG is called form crash
>> handler.  I will share my test details in v3 cover letter also.
>>
>>>
>>> aah one other thing I just noticed since you are ccing stable and you
>>> added a Fixes tag in patch-3.
>>> Patch-3 alone cannot be easily backported now due to patch-1 and
>>> patch-2. There must be a way to define the dependencies if you are
>>> looking for backporting the fix patch to stable tree, please check that
>>> and follow that accordingly in v3.
>> I thought about that as well, but since they are part of the same patch
>> series,
>> I assumed they would be picked together. However, I don't think that
>> will work
>> in all cases.
>>
>> I checked the older commits and noticed that a backport note was added.
>> I think
>> we can do the same for the fix patch. I'll add a note indicating that the
>> following patches should be backported first:
>>
>> powerpc/pseries: Move H_WATCHDOG definitions to a common header
>> powerpc/pseries: Handle and log pseries-wdt registration failures
>>
>> Since these patches are not upstream yet, I'll refer to them by their
>> commit titles.
>>
>> Does that look good to you?
>>
> Documentation/process/stable-kernel-rules.rst
>    Note that for a patch series, you do not have to list as prerequisites the
>    patches present in the series itself. For example, if you have the following
>    patch series::
>
>      patch1
>      patch2
>
>    where patch2 depends on patch1, you do not have to list patch1 as
>    prerequisite of patch2 if you have already marked patch1 for stable
>    inclusion.
>
>
> In that case, I think, we should mark all 3 patches for stable inclusion.
>
>    patch 1/3   Cc: stable@vger.kernel.org
>    patch 2/3   Cc: stable@vger.kernel.org
>    patch 3/3   Cc: stable@vger.kernel.org
>                Fixes: 69472ffa6575 ("watchdog/pseries-wdt: initial support for H_WATCHDOG-based watchdog timers"

Okay, for stable tree inclusion, CCing is enough.

Thanks
Sourabh Jain

Re: [PATCH v2 0/3] powerpc/crash: protect kdump from active watchdogs

[Ritesh Harjani]

Sourabh Jain <sourabhjain@linux.ibm.com> writes:

>
> Okay, for stable tree inclusion, CCing is enough.

yup, by ccing here means, adding a "Cc: stable@vger.kernel.org" tag to
the commit should do.

-ritesh

Re: [PATCH v2 3/3] powerpc/crash: stop watchdogs before booting kdump kernel

[Sourabh Jain]

On 13/07/26 10:40, Ritesh Harjani (IBM) wrote:
> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>
>> On pseries LPAR systems, watchdog timers configured from userspace can
>> remain active after a kernel panic. When a panic triggers kdump, the
>> crashing kernel jumps directly to the kdump kernel without stopping
>> active watchdogs. As a result, the watchdogs remain active after the
>> kdump kernel starts.
>>
>> If dump capture takes longer than the watchdog timeout, PHYP resets the
>> LPAR before the dump is fully captured, causing dump capture to fail.
>>
>> Fix this by issuing the `H_WATCHDOG` hcall during the crash shutdown
>> sequence to stop all active watchdogs before booting the kdump kernel.
>>
>> Fixes: 69472ffa6575 ("watchdog/pseries-wdt: initial support for H_WATCHDOG-based watchdog timers")
>> Reported-by: Mahesh Kumar G <mahe657@linux.ibm.com>
>> Suggested-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> ---
>>   arch/powerpc/include/asm/papr-watchdog.h |  2 ++
>>   arch/powerpc/platforms/pseries/setup.c   | 18 ++++++++++++++++++
>>   2 files changed, 20 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/papr-watchdog.h b/arch/powerpc/include/asm/papr-watchdog.h
>> index fb3a511aa861..84bbe1ddd56f 100644
>> --- a/arch/powerpc/include/asm/papr-watchdog.h
>> +++ b/arch/powerpc/include/asm/papr-watchdog.h
>> @@ -55,4 +55,6 @@
>>   #define PSERIES_WDTQ_MIN_TIMEOUT(cap)	(((cap) >> 48) & 0xffff)
>>   #define PSERIES_WDTQ_MAX_NUMBER(cap)	(((cap) >> 32) & 0xffff)
>>   
>> +#define PSERIES_WDT_NUM_ALL	((unsigned long)-1)
>> +
> minor nit:
>
> This should be defined at the end of the H_WATCHDOG Input section.
> /*
>   * H_WATCHDOG Input
>   *
>
> <...>
>
> Something like this maybe?
>
> /*
>   * R5: "watchdogNumber":

Makes sense. Since this is the third argument to the hypercall,
R5 is the correct register to use in the comment.

>   *       PAPR says use -1 (all ones) to stop all watchdogs.
>   */
> #define PSERIES_WDT_NUM_ALL	((unsigned long)-1)
>
> /*
>   * H_WATCHDOG Output
>   *
>   * R3: Return code
>   *
>   <...>
>
>>   #endif /* _ASM_POWERPC_CRASHDUMP_PPC64_H */
>> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
>> index bbb2813f8ede..2e40a9dba637 100644
>> --- a/arch/powerpc/platforms/pseries/setup.c
>> +++ b/arch/powerpc/platforms/pseries/setup.c
>> @@ -77,6 +77,7 @@
>>   #include <asm/dtl.h>
>>   #include <asm/hvconsole.h>
>>   #include <asm/setup.h>
>> +#include <asm/papr-watchdog.h>
>>   
>>   #include "pseries.h"
>>   
>> @@ -185,6 +186,18 @@ static void __init fwnmi_init(void)
>>   #endif
>>   }
>>   
>> +#ifdef CONFIG_CRASH_DUMP
>> +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);
>> +}
>> +#endif /* CONFIG_CRASH_DUMP */
>> +
>>   /*
>>    * Affix a device for the first timer to the platform bus if
>>    * we have firmware support for the H_WATCHDOG hypercall.
>> @@ -203,6 +216,11 @@ static __init int pseries_wdt_init(void)
>>   		return PTR_ERR(pseries_wdt_dev);
>>   	}
>>   
>> +#ifdef CONFIG_CRASH_DUMP
>> +	if (crash_shutdown_register(pseries_crash_stop_watchdogs))
>> +		pr_warn("Could not register watchdog crash shutdown handler\n");
>> +#endif
>> +
> minor nit:
> I don't think we need any of the #ifdef. All definitions used inside
> pseries_crash_stop_watchdogs are already available and
> crash_shutdown_register() already exists for !CONFIG_CRASH_DUMP, so we
> may as well drop all of the ifdefs.

Yes, the #ifdef is not really needed  because crash_shutdown_register() is
always available.

I removed the #ifdef blocks and built the kernel both with and without
CONFIG_CRASH_DUMP. The kernel built successfully in both cases.

>
>
> Otherwise LGTM, so feel free to add:
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Thanks for the review.

- Sourabh Jain