Sourabh Jain <[email protected]> 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 <[email protected]>
> Suggested-by: Ritesh Harjani (IBM) <[email protected]>
> Signed-off-by: Sourabh Jain <[email protected]>
> ---
> 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) <[email protected]>
