[PATCH v4 11/25] powernv/fadump: register kernel metadata address with opal

[Mahesh J Salgaonkar]

On 2019-07-16 17:03:15 Tue, Hari Bathini wrote:
> OPAL allows registering address with it in the first kernel and
> retrieving it after MPIPL. Setup kernel metadata and register its
> address with OPAL to use it for processing the crash dump.
> 
> Signed-off-by: Hari Bathini <hbathini at linux.ibm.com>
> ---
>  arch/powerpc/kernel/fadump-common.h          |    4 +
>  arch/powerpc/kernel/fadump.c                 |   65 ++++++++++++++---------
>  arch/powerpc/platforms/powernv/opal-fadump.c |   73 ++++++++++++++++++++++++++
>  arch/powerpc/platforms/powernv/opal-fadump.h |   37 +++++++++++++
>  arch/powerpc/platforms/pseries/rtas-fadump.c |   32 +++++++++--
>  5 files changed, 177 insertions(+), 34 deletions(-)
>  create mode 100644 arch/powerpc/platforms/powernv/opal-fadump.h
> 
[...]
> @@ -346,30 +349,42 @@ int __init fadump_reserve_mem(void)
>  		 * use memblock_find_in_range() here since it doesn't allocate
>  		 * from bottom to top.
>  		 */
> -		for (base = fw_dump.boot_memory_size;
> -		     base <= (memory_boundary - size);
> -		     base += size) {
> +		while (base <= (memory_boundary - size)) {
>  			if (memblock_is_region_memory(base, size) &&
>  			    !memblock_is_region_reserved(base, size))
>  				break;
> +
> +			base += size;
>  		}
> -		if ((base > (memory_boundary - size)) ||
> -		    memblock_reserve(base, size)) {
> +
> +		if (base > (memory_boundary - size)) {
> +			pr_err("Failed to find memory chunk for reservation\n");
> +			goto error_out;
> +		}
> +		fw_dump.reserve_dump_area_start = base;
> +
> +		/*
> +		 * Calculate the kernel metadata address and register it with
> +		 * f/w if the platform supports.
> +		 */
> +		if (fw_dump.ops->setup_kernel_metadata(&fw_dump) < 0)
> +			goto error_out;

I see setup_kernel_metadata() registers the metadata address with opal without
having any minimum data initialized in it. Secondaly, why can't this wait until
registration ? I think we should defer this until fadump registration.
What if kernel crashes before metadata area is initialized ?

> +
> +		if (memblock_reserve(base, size)) {
>  			pr_err("Failed to reserve memory\n");
> -			return 0;
> +			goto error_out;
>  		}
[...]
> -
>  static struct fadump_ops rtas_fadump_ops = {
> -	.init_fadump_mem_struct	= rtas_fadump_init_mem_struct,
> -	.register_fadump	= rtas_fadump_register_fadump,
> -	.unregister_fadump	= rtas_fadump_unregister_fadump,
> -	.invalidate_fadump	= rtas_fadump_invalidate_fadump,
> -	.process_fadump		= rtas_fadump_process_fadump,
> -	.fadump_region_show	= rtas_fadump_region_show,
> -	.fadump_trigger		= rtas_fadump_trigger,
> +	.init_fadump_mem_struct		= rtas_fadump_init_mem_struct,
> +	.get_kernel_metadata_size	= rtas_fadump_get_kernel_metadata_size,
> +	.setup_kernel_metadata		= rtas_fadump_setup_kernel_metadata,
> +	.register_fadump		= rtas_fadump_register_fadump,
> +	.unregister_fadump		= rtas_fadump_unregister_fadump,
> +	.invalidate_fadump		= rtas_fadump_invalidate_fadump,
> +	.process_fadump			= rtas_fadump_process_fadump,
> +	.fadump_region_show		= rtas_fadump_region_show,
> +	.fadump_trigger			= rtas_fadump_trigger,

Can you make the tab space changes in your previous patch where these
were initially introduced ? So that this patch can only show new members
that are added.

Thanks,
-Mahesh.