Re: [PATCH v5 11/31] powernv/fadump: add fadump support on powernv

[Michael Ellerman <mpe@ellerman.id.au>]

Hari Bathini <hbathini@linux.ibm.com> writes:
> Add basic callback functions for FADump on PowerNV platform.

I assume this doesn't actually work yet?

Does something block it from appearing to work at runtime?

> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index d8dcd88..fc4ecfe 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -566,7 +566,7 @@ config CRASH_DUMP
>  
>  config FA_DUMP
>  	bool "Firmware-assisted dump"
> -	depends on PPC64 && PPC_RTAS
> +	depends on PPC64 && (PPC_RTAS || PPC_POWERNV)
>  	select CRASH_CORE
>  	select CRASH_DUMP
>  	help
> @@ -577,7 +577,8 @@ config FA_DUMP
>  	  is meant to be a kdump replacement offering robustness and
>  	  speed not possible without system firmware assistance.
>  
> -	  If unsure, say "N"
> +	  If unsure, say "y". Only special kernels like petitboot may
> +	  need to say "N" here.
>  
>  config IRQ_ALL_CPUS
>  	bool "Distribute interrupts on all CPUs by default"
> diff --git a/arch/powerpc/kernel/fadump-common.h b/arch/powerpc/kernel/fadump-common.h
> index d2c5b16..f6c52d3 100644
> --- a/arch/powerpc/kernel/fadump-common.h
> +++ b/arch/powerpc/kernel/fadump-common.h
> @@ -140,4 +140,13 @@ static inline int rtas_fadump_dt_scan(struct fw_dump *fadump_config, ulong node)
>  }
>  #endif
>  
> +#ifdef CONFIG_PPC_POWERNV
> +extern int opal_fadump_dt_scan(struct fw_dump *fadump_config, ulong node);
> +#else
> +static inline int opal_fadump_dt_scan(struct fw_dump *fadump_config, ulong node)
> +{
> +	return 1;
> +}

Extending the strange flat device tree calling convention to these
functions is not ideal.

It would be better I think if they just returned bool true/false for
"found it" / "not found", and then early_init_dt_scan_fw_dump() can
convert that into the appropriate return value.

> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index f7c8073..b8061fb9 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -114,6 +114,9 @@ int __init early_init_dt_scan_fw_dump(unsigned long node, const char *uname,
>  	if (strcmp(uname, "rtas") == 0)
>  		return rtas_fadump_dt_scan(&fw_dump, node);
>  
> +	if (strcmp(uname, "ibm,opal") == 0)
> +		return opal_fadump_dt_scan(&fw_dump, node);
> +

ie this would become:

	if (strcmp(uname, "ibm,opal") == 0 && opal_fadump_dt_scan(&fw_dump, node))
            return 1;

>  	return 0;
>  }
>  
> diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
> index da2e99e..43a6e1c 100644
> --- a/arch/powerpc/platforms/powernv/Makefile
> +++ b/arch/powerpc/platforms/powernv/Makefile
> @@ -6,6 +6,7 @@ obj-y			+= opal-msglog.o opal-hmi.o opal-power.o opal-irqchip.o
>  obj-y			+= opal-kmsg.o opal-powercap.o opal-psr.o opal-sensor-groups.o
>  
>  obj-$(CONFIG_SMP)	+= smp.o subcore.o subcore-asm.o
> +obj-$(CONFIG_FA_DUMP)	+= opal-fadump.o
>  obj-$(CONFIG_PCI)	+= pci.o pci-ioda.o npu-dma.o pci-ioda-tce.o
>  obj-$(CONFIG_CXL_BASE)	+= pci-cxl.o
>  obj-$(CONFIG_EEH)	+= eeh-powernv.o
> diff --git a/arch/powerpc/platforms/powernv/opal-fadump.c b/arch/powerpc/platforms/powernv/opal-fadump.c
> new file mode 100644
> index 0000000..e330877
> --- /dev/null
> +++ b/arch/powerpc/platforms/powernv/opal-fadump.c
> @@ -0,0 +1,97 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Firmware-Assisted Dump support on POWER platform (OPAL).
> + *
> + * Copyright 2019, IBM Corp.
> + * Author: Hari Bathini <hbathini@linux.ibm.com>
> + */
> +
> +#undef DEBUG

No undef again please.

> +#define pr_fmt(fmt) "opal fadump: " fmt
> +
> +#include <linux/string.h>
> +#include <linux/seq_file.h>
> +#include <linux/of_fdt.h>
> +#include <linux/libfdt.h>
> +
> +#include <asm/opal.h>
> +
> +#include "../../kernel/fadump-common.h"
> +
> +static ulong opal_fadump_init_mem_struct(struct fw_dump *fadump_conf)
> +{
> +	return fadump_conf->reserve_dump_area_start;
> +}
> +
> +static int opal_fadump_register(struct fw_dump *fadump_conf)
> +{
> +	return -EIO;
> +}
> +
> +static int opal_fadump_unregister(struct fw_dump *fadump_conf)
> +{
> +	return -EIO;
> +}
> +
> +static int opal_fadump_invalidate(struct fw_dump *fadump_conf)
> +{
> +	return -EIO;
> +}
> +
> +static int __init opal_fadump_process(struct fw_dump *fadump_conf)
> +{
> +	return -EINVAL;
> +}
> +
> +static void opal_fadump_region_show(struct fw_dump *fadump_conf,
> +				    struct seq_file *m)
> +{
> +}
> +
> +static void opal_fadump_trigger(struct fadump_crash_info_header *fdh,
> +				const char *msg)
> +{
> +	int rc;
> +
> +	rc = opal_cec_reboot2(OPAL_REBOOT_MPIPL, msg);
> +	if (rc == OPAL_UNSUPPORTED) {
> +		pr_emerg("Reboot type %d not supported.\n",
> +			 OPAL_REBOOT_MPIPL);
> +	} else if (rc == OPAL_HARDWARE)
> +		pr_emerg("No backend support for MPIPL!\n");
> +}
> +
> +static struct fadump_ops opal_fadump_ops = {
> +	.fadump_init_mem_struct		= opal_fadump_init_mem_struct,
> +	.fadump_register		= opal_fadump_register,
> +	.fadump_unregister		= opal_fadump_unregister,
> +	.fadump_invalidate		= opal_fadump_invalidate,
> +	.fadump_process			= opal_fadump_process,
> +	.fadump_region_show		= opal_fadump_region_show,
> +	.fadump_trigger			= opal_fadump_trigger,
> +};
> +
> +int __init opal_fadump_dt_scan(struct fw_dump *fadump_conf, ulong node)
> +{
> +	unsigned long dn;
> +
> +	/*
> +	 * Check if Firmware-Assisted Dump is supported. if yes, check
> +	 * if dump has been initiated on last reboot.
> +	 */
> +	dn = of_get_flat_dt_subnode_by_name(node, "dump");
> +	if (dn == -FDT_ERR_NOTFOUND) {
> +		pr_debug("FADump support is missing!\n");
> +		return 1;
> +	}
> +
> +	if (!of_flat_dt_is_compatible(dn, "ibm,opal-dump")) {
> +		pr_err("Support missing for this f/w version!\n");
> +		return 1;
> +	}
> +
> +	fadump_conf->ops		= &opal_fadump_ops;
> +	fadump_conf->fadump_supported	= 1;
> +
> +	return 1;
> +}

cheers