[RFC PATCH 1/3] powerpc/pseries/fadump: add support for multiple boot memory regions
Sourabh Jain
sourabhjain at linux.ibm.com
Tue Dec 19 17:58:10 AEDT 2023
Hello Aditya,
On 17/12/23 14:11, Aditya Gupta wrote:
> Hi sourabh,
>
> On 06/12/23 01:48, Hari Bathini wrote:
>> From: Sourabh Jain <sourabhjain at linux.ibm.com>
>>
>> Currently, fadump on pseries assumes a single boot memory region even
>> though f/w supports more than one boot memory region. Add support for
>> more boot memory regions to make the implementation flexible for any
>> enhancements that introduce other region types. For this, rtas memory
>> structure for fadump is updated to have multiple boot memory regions
>> instead of just one. Additionally, methods responsible for creating
>> the fadump memory structure during both the first and second kernel
>> boot have been modified to take these multiple boot memory regions
>> into account. Also, a new callback has been added to the fadump_ops
>> structure to get the maximum boot memory regions supported by the
>> platform.
>>
>> Signed-off-by: Sourabh Jain <sourabhjain at linux.ibm.com>
>> Signed-off-by: Hari Bathini <hbathini at linux.ibm.com>
>> ---
>> arch/powerpc/include/asm/fadump-internal.h | 2 +-
>> arch/powerpc/kernel/fadump.c | 27 +-
>> arch/powerpc/platforms/powernv/opal-fadump.c | 8 +
>> arch/powerpc/platforms/pseries/rtas-fadump.c | 258 ++++++++++++-------
>> arch/powerpc/platforms/pseries/rtas-fadump.h | 26 +-
>> 5 files changed, 199 insertions(+), 122 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/fadump-internal.h
>> b/arch/powerpc/include/asm/fadump-internal.h
>> index 27f9e11eda28..b3956c400519 100644
>> --- a/arch/powerpc/include/asm/fadump-internal.h
>> +++ b/arch/powerpc/include/asm/fadump-internal.h
>> @@ -129,6 +129,7 @@ struct fadump_ops {
>> struct seq_file *m);
>> void (*fadump_trigger)(struct fadump_crash_info_header *fdh,
>> const char *msg);
>> + int (*fadump_max_boot_mem_rgns)(void);
>> };
>> /* Helper functions */
>> @@ -136,7 +137,6 @@ s32 __init fadump_setup_cpu_notes_buf(u32 num_cpus);
>> void fadump_free_cpu_notes_buf(void);
>> u32 *__init fadump_regs_to_elf_notes(u32 *buf, struct pt_regs *regs);
>> void __init fadump_update_elfcore_header(char *bufp);
>> -bool is_fadump_boot_mem_contiguous(void);
>> bool is_fadump_reserved_mem_contiguous(void);
>> #else /* !CONFIG_PRESERVE_FA_DUMP */
>> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
>> index d14eda1e8589..757681658dda 100644
>> --- a/arch/powerpc/kernel/fadump.c
>> +++ b/arch/powerpc/kernel/fadump.c
>> @@ -222,28 +222,6 @@ static bool is_fadump_mem_area_contiguous(u64
>> d_start, u64 d_end)
>> return ret;
>> }
>> -/*
>> - * Returns true, if there are no holes in boot memory area,
>> - * false otherwise.
>> - */
>> -bool is_fadump_boot_mem_contiguous(void)
>> -{
>> - unsigned long d_start, d_end;
>> - bool ret = false;
>> - int i;
>> -
>> - for (i = 0; i < fw_dump.boot_mem_regs_cnt; i++) {
>> - d_start = fw_dump.boot_mem_addr[i];
>> - d_end = d_start + fw_dump.boot_mem_sz[i];
>> -
>> - ret = is_fadump_mem_area_contiguous(d_start, d_end);
>> - if (!ret)
>> - break;
>> - }
>> -
>> - return ret;
>> -}
>> -
>> /*
>> * Returns true, if there are no holes in reserved memory area,
>> * false otherwise.
>> @@ -389,10 +367,11 @@ static unsigned long __init
>> get_fadump_area_size(void)
>> static int __init add_boot_mem_region(unsigned long rstart,
>> unsigned long rsize)
>> {
>> + int max_boot_mem_rgns = fw_dump.ops->fadump_max_boot_mem_rgns();
>> int i = fw_dump.boot_mem_regs_cnt++;
>> - if (fw_dump.boot_mem_regs_cnt > FADUMP_MAX_MEM_REGS) {
>> - fw_dump.boot_mem_regs_cnt = FADUMP_MAX_MEM_REGS;
>> + if (fw_dump.boot_mem_regs_cnt > max_boot_mem_rgns) {
>> + fw_dump.boot_mem_regs_cnt = max_boot_mem_rgns;
>> return 0;
>> }
>> diff --git a/arch/powerpc/platforms/powernv/opal-fadump.c
>> b/arch/powerpc/platforms/powernv/opal-fadump.c
>> index 964f464b1b0e..fa26c21a08d9 100644
>> --- a/arch/powerpc/platforms/powernv/opal-fadump.c
>> +++ b/arch/powerpc/platforms/powernv/opal-fadump.c
>> @@ -615,6 +615,13 @@ static void opal_fadump_trigger(struct
>> fadump_crash_info_header *fdh,
>> pr_emerg("No backend support for MPIPL!\n");
>> }
>> +/* FADUMP_MAX_MEM_REGS or lower */
>> +static int opal_fadump_max_boot_mem_rgns(void)
>> +{
>> + return FADUMP_MAX_MEM_REGS;
>> +
>> +}
>> +
>> static struct fadump_ops opal_fadump_ops = {
>> .fadump_init_mem_struct = opal_fadump_init_mem_struct,
>> .fadump_get_metadata_size = opal_fadump_get_metadata_size,
>> @@ -627,6 +634,7 @@ static struct fadump_ops opal_fadump_ops = {
>> .fadump_process = opal_fadump_process,
>> .fadump_region_show = opal_fadump_region_show,
>> .fadump_trigger = opal_fadump_trigger,
>> + .fadump_max_boot_mem_rgns = opal_fadump_max_boot_mem_rgns,
>> };
>> void __init opal_fadump_dt_scan(struct fw_dump *fadump_conf, u64
>> node)
>> diff --git a/arch/powerpc/platforms/pseries/rtas-fadump.c
>> b/arch/powerpc/platforms/pseries/rtas-fadump.c
>> index b5853e9fcc3c..1b05b4cefdfd 100644
>> --- a/arch/powerpc/platforms/pseries/rtas-fadump.c
>> +++ b/arch/powerpc/platforms/pseries/rtas-fadump.c
>> @@ -29,9 +29,6 @@ static const struct rtas_fadump_mem_struct
>> *fdm_active;
>> static void rtas_fadump_update_config(struct fw_dump *fadump_conf,
>> const struct rtas_fadump_mem_struct *fdm)
>> {
>> - fadump_conf->boot_mem_dest_addr =
>> - be64_to_cpu(fdm->rmr_region.destination_address);
>> -
>> fadump_conf->fadumphdr_addr = (fadump_conf->boot_mem_dest_addr +
>> fadump_conf->boot_memory_size);
>> }
>> @@ -43,20 +40,52 @@ static void rtas_fadump_update_config(struct
>> fw_dump *fadump_conf,
>> static void __init rtas_fadump_get_config(struct fw_dump *fadump_conf,
>> const struct rtas_fadump_mem_struct *fdm)
>> {
>> - fadump_conf->boot_mem_addr[0] =
>> - be64_to_cpu(fdm->rmr_region.source_address);
>> - fadump_conf->boot_mem_sz[0] =
>> be64_to_cpu(fdm->rmr_region.source_len);
>> - fadump_conf->boot_memory_size = fadump_conf->boot_mem_sz[0];
>> + unsigned long base, size, last_end, hole_size;
>> - fadump_conf->boot_mem_top = fadump_conf->boot_memory_size;
>> - fadump_conf->boot_mem_regs_cnt = 1;
>> + last_end = 0;
>> + hole_size = 0;
>> + fadump_conf->boot_memory_size = 0;
>> + fadump_conf->boot_mem_regs_cnt = 0;
>> + pr_debug("Boot memory regions:\n");
>> + for (int i = 0; i < be16_to_cpu(fdm->header.dump_num_sections);
>> i++) {
>> + int type = be16_to_cpu(fdm->rgn[i].source_data_type);
>> - /*
>> - * Start address of reserve dump area (permanent reservation) for
>> - * re-registering FADump after dump capture.
>> - */
>> - fadump_conf->reserve_dump_area_start =
>> - be64_to_cpu(fdm->cpu_state_data.destination_address);
>> + switch (type) {
>> + case RTAS_FADUMP_CPU_STATE_DATA:
>> + u64 addr = be64_to_cpu(fdm->rgn[i].destination_address);
>
> This caused a compiler error on my system:
>
> arch/powerpc/platforms/pseries/rtas-fadump.c: In function
> ‘rtas_fadump_get_config’:
> arch/powerpc/platforms/pseries/rtas-fadump.c:56:4: error: a label can
> only be part of a statement and a declaration is not a statement
> u64 addr = be64_to_cpu(fdm->rgn[i].destination_address);
> ^~~
>
> Probably the 'addr' local variable needs to be in it's own local scope.
>
> Adding the case block in brackets {} solved the error for me.
> Rest of the code looks good to me. I will also try to test the patch
> series later.
>
Thanks for reporting, we will fix it in next version.
- Sourabh
More information about the Linuxppc-dev
mailing list