Re: [PATCH v4 11/25] powernv/fadump: register kernel metadata address with opal
From: Mahesh J Salgaonkar <hidden>
Date: 2019-08-13 10:43:47
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@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
[...]
quoted hunk ↗ jump to hunk
@@ -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.