Re: [PATCH v4 19/25] powernv/fadump: add support to preserve crash data on FADUMP disabled kernel

[Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com>]

On 2019-07-16 17:04:16 Tue, Hari Bathini wrote:
> Add a new kernel config option, CONFIG_PRESERVE_FA_DUMP that ensures
> that crash data, from previously crash'ed kernel, is preserved. This
> helps in cases where FADump is not enabled but the subsequent memory
> preserving kernel boot is likely to process this crash data. One
> typical usecase for this config option is petitboot kernel.
> 
> As OPAL allows registering address with it in the first kernel and
> retrieving it after MPIPL, use it to store the top of boot memory.
> A kernel that intends to preserve crash data retrieves it and avoids
> using memory beyond this address.
> 
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
>  arch/powerpc/Kconfig                         |    9 ++
>  arch/powerpc/include/asm/fadump.h            |    9 +-
>  arch/powerpc/kernel/Makefile                 |    6 +
>  arch/powerpc/kernel/fadump-common.h          |   13 ++-
>  arch/powerpc/kernel/fadump.c                 |  128 ++++++++++++++++----------
>  arch/powerpc/kernel/prom.c                   |    4 -
>  arch/powerpc/platforms/powernv/Makefile      |    1 
>  arch/powerpc/platforms/powernv/opal-fadump.c |   59 ++++++++++++
>  arch/powerpc/platforms/powernv/opal-fadump.h |    3 +
>  9 files changed, 176 insertions(+), 56 deletions(-)
> 
[...]
>  #include "../../kernel/fadump-common.h"
>  #include "opal-fadump.h"
>  
> +
> +#ifdef CONFIG_PRESERVE_FA_DUMP
> +/*
> + * When dump is active but PRESERVE_FA_DUMP is enabled on the kernel,
> + * ensure crash data is preserved in hope that the subsequent memory
> + * preserving kernel boot is going to process this crash data.
> + */
> +int __init opal_fadump_dt_scan(struct fw_dump *fadump_conf, ulong node)
> +{
> +	unsigned long dn;
> +	const __be32 *prop;
> +
> +	dn = of_get_flat_dt_subnode_by_name(node, "dump");
> +	if (dn == -FDT_ERR_NOTFOUND)
> +		return 1;
> +
> +	/*
> +	 * Check if dump has been initiated on last reboot.
> +	 */
> +	prop = of_get_flat_dt_prop(dn, "mpipl-boot", NULL);
> +	if (prop) {
> +		u64 addr = 0;
> +		s64 ret;
> +
> +		ret = opal_mpipl_query_tag(OPAL_MPIPL_TAG_BOOT_MEM, &addr);
> +		if ((ret != OPAL_SUCCESS) || !addr) {
> +			pr_err("Failed to get boot memory tag (%lld)\n", ret);
> +			return 1;
> +		}
> +
> +		/*
> +		 * Anything below this address can be used for booting a
> +		 * capture kernel or petitboot kernel. Preserve everything
> +		 * above this address for processing crashdump.
> +		 */
> +		fadump_conf->boot_mem_top = be64_to_cpu(addr);
> +		pr_debug("Preserve everything above %lx\n",
> +			 fadump_conf->boot_mem_top);
> +
> +		pr_info("Firmware-assisted dump is active.\n");
> +		fadump_conf->dump_active = 1;
> +	}
> +
> +	return 1;
> +}
> +
> +#else /* CONFIG_PRESERVE_FA_DUMP */
>  static const struct opal_fadump_mem_struct *opal_fdm_active;
>  static const struct opal_mpipl_fadump *opal_cpu_metadata;
>  static struct opal_fadump_mem_struct *opal_fdm;
> @@ -155,6 +202,17 @@ static int opal_fadump_setup_kernel_metadata(struct fw_dump *fadump_conf)
>  		err = -EPERM;
>  	}
>  
> +	/*
> +	 * Register boot memory top address with f/w. Should be retrieved
> +	 * by a kernel that intends to preserve crash'ed kernel's memory.
> +	 */
> +	ret = opal_mpipl_register_tag(OPAL_MPIPL_TAG_BOOT_MEM,
> +				      fadump_conf->boot_mem_top);

Looks like we only register tag but never de-register ot set them to
NULL when we don't need it. Same for kernel TAG. i.e if we kexec into
new kernel which may not do fadump and if opal crashes it will present
stale tags to next kernel. I think we should set bootmem/kernel tag to
NULL in fadump_cleanup() path so that kexec path can be taken care of.

Thanks,
-Mahesh.