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 <hbath...@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.
