diff mbox series

[v3,1/2] powerpc/fadump: allocate memory for additional parameters early

Message ID 20241113070618.75744-1-sourabhjain@linux.ibm.com (mailing list archive)
State Superseded
Headers show
Series [v3,1/2] powerpc/fadump: allocate memory for additional parameters early | expand

Commit Message

Sourabh Jain Nov. 13, 2024, 7:06 a.m. UTC 
From: Hari Bathini <hbathini@linux.ibm.com>

Memory for passing additional parameters to fadump capture kernel
is allocated during subsys_initcall level, using memblock. But
as slab is already available by this time, allocation happens via
the buddy allocator. This may work for radix MMU but is likely to
fail in most cases for hash MMU as hash MMU needs this memory in
the first memory block for it to be accessible in real mode in the
capture kernel (second boot). So, allocate memory for additional
parameters area as soon as MMU mode is obvious.

Fixes: 683eab94da75 ("powerpc/fadump: setup additional parameters for dump capture kernel")
Reported-by: Venkat Rao Bagalkote <venkat88@linux.vnet.ibm.com>
Closes: https://lore.kernel.org/lkml/a70e4064-a040-447b-8556-1fd02f19383d@linux.vnet.ibm.com/T/#u
Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---

Changelog:

Since v1: https://lore.kernel.org/all/20241104083528.99520-1-sourabhjain@linux.ibm.com/
  - Drop extern keyword from fadump_setup_param_area function declaration
  - Don't use __va() while resetting param memory area

Since v2: https://lore.kernel.org/all/20241107055817.489795-1-sourabhjain@linux.ibm.com/
 - Update comment around fadump_setup_param_area() to indicate its
   dependency on MMU type
 - Add Reviewed-by tag
 - No functional change

---
 arch/powerpc/include/asm/fadump.h |  2 ++
 arch/powerpc/kernel/fadump.c      | 15 ++++++++++-----
 arch/powerpc/kernel/prom.c        |  7 +++++++
 3 files changed, 19 insertions(+), 5 deletions(-)

Comments

Christophe Leroy Jan. 17, 2025, 12:13 p.m. UTC | #1 
Le 13/11/2024 à 08:06, Sourabh Jain a écrit :
> From: Hari Bathini <hbathini@linux.ibm.com>
> 
> Memory for passing additional parameters to fadump capture kernel
> is allocated during subsys_initcall level, using memblock. But
> as slab is already available by this time, allocation happens via
> the buddy allocator. This may work for radix MMU but is likely to
> fail in most cases for hash MMU as hash MMU needs this memory in
> the first memory block for it to be accessible in real mode in the
> capture kernel (second boot). So, allocate memory for additional
> parameters area as soon as MMU mode is obvious.
> 
> Fixes: 683eab94da75 ("powerpc/fadump: setup additional parameters for dump capture kernel")
> Reported-by: Venkat Rao Bagalkote <venkat88@linux.vnet.ibm.com>
> Closes: https://lore.kernel.org/lkml/a70e4064-a040-447b-8556-1fd02f19383d@linux.vnet.ibm.com/T/#u
> Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>

Version v2 of this series was applied.

If needed, can you rebase this patch ?
Sourabh Jain Jan. 17, 2025, 12:32 p.m. UTC | #2 
Hello Christophe,

On 17/01/25 17:43, Christophe Leroy wrote:
>
>
> Le 13/11/2024 à 08:06, Sourabh Jain a écrit :
>> From: Hari Bathini <hbathini@linux.ibm.com>
>>
>> Memory for passing additional parameters to fadump capture kernel
>> is allocated during subsys_initcall level, using memblock. But
>> as slab is already available by this time, allocation happens via
>> the buddy allocator. This may work for radix MMU but is likely to
>> fail in most cases for hash MMU as hash MMU needs this memory in
>> the first memory block for it to be accessible in real mode in the
>> capture kernel (second boot). So, allocate memory for additional
>> parameters area as soon as MMU mode is obvious.
>>
>> Fixes: 683eab94da75 ("powerpc/fadump: setup additional parameters for 
>> dump capture kernel")
>> Reported-by: Venkat Rao Bagalkote <venkat88@linux.vnet.ibm.com>
>> Closes: 
>> https://lore.kernel.org/lkml/a70e4064-a040-447b-8556-1fd02f19383d@linux.vnet.ibm.com/T/#u
>> Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
>> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>
> Version v2 of this series was applied.
>
> If needed, can you rebase this patch ?
>
Sorry, I didn't get that. Rebase on top of which tree/branch?

FYI, there was no functional change from v2 to v3. Only a
"Reviewed-by" tag was added.

Thanks,
Sourabh Jain
Christophe Leroy Jan. 17, 2025, 12:46 p.m. UTC | #3 
Le 17/01/2025 à 13:32, Sourabh Jain a écrit :
> Hello Christophe,
> 
> On 17/01/25 17:43, Christophe Leroy wrote:
>>
>>
>> Le 13/11/2024 à 08:06, Sourabh Jain a écrit :
>>> From: Hari Bathini <hbathini@linux.ibm.com>
>>>
>>> Memory for passing additional parameters to fadump capture kernel
>>> is allocated during subsys_initcall level, using memblock. But
>>> as slab is already available by this time, allocation happens via
>>> the buddy allocator. This may work for radix MMU but is likely to
>>> fail in most cases for hash MMU as hash MMU needs this memory in
>>> the first memory block for it to be accessible in real mode in the
>>> capture kernel (second boot). So, allocate memory for additional
>>> parameters area as soon as MMU mode is obvious.
>>>
>>> Fixes: 683eab94da75 ("powerpc/fadump: setup additional parameters for 
>>> dump capture kernel")
>>> Reported-by: Venkat Rao Bagalkote <venkat88@linux.vnet.ibm.com>
>>> Closes: https://eur01.safelinks.protection.outlook.com/? 
>>> url=https%3A%2F%2Flore.kernel.org%2Flkml%2Fa70e4064- 
>>> a040-447b-8556-1fd02f19383d%40linux.vnet.ibm.com%2FT%2F%23u&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7C9df078759a2c42b044cf08dd36f3183b%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638727139896068824%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=iZfkQKw4wJAvwAj%2BbGOS5kcrVtAg8xg%2FFl6ojEYZ6ls%3D&reserved=0
>>> Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
>>> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
>>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>>
>> Version v2 of this series was applied.
>>
>> If needed, can you rebase this patch ?
>>
> Sorry, I didn't get that. Rebase on top of which tree/branch?

I meant on top of any tree including the applied version.

> 
> FYI, there was no functional change from v2 to v3. Only a
> "Reviewed-by" tag was added.

Then I guess there is nothing to do.

Christophe
diff mbox series

Patch

diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h
index ef40c9b6972a..7b3473e05273 100644
--- a/arch/powerpc/include/asm/fadump.h
+++ b/arch/powerpc/include/asm/fadump.h
@@ -19,6 +19,7 @@  extern int is_fadump_active(void);
 extern int should_fadump_crash(void);
 extern void crash_fadump(struct pt_regs *, const char *);
 extern void fadump_cleanup(void);
+void fadump_setup_param_area(void);
 extern void fadump_append_bootargs(void);
 
 #else	/* CONFIG_FA_DUMP */
@@ -26,6 +27,7 @@  static inline int is_fadump_active(void) { return 0; }
 static inline int should_fadump_crash(void) { return 0; }
 static inline void crash_fadump(struct pt_regs *regs, const char *str) { }
 static inline void fadump_cleanup(void) { }
+static inline void fadump_setup_param_area(void) { }
 static inline void fadump_append_bootargs(void) { }
 #endif /* !CONFIG_FA_DUMP */
 
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index a612e7513a4f..3a2863307863 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -1586,6 +1586,12 @@  static void __init fadump_init_files(void)
 		return;
 	}
 
+	if (fw_dump.param_area) {
+		rc = sysfs_create_file(fadump_kobj, &bootargs_append_attr.attr);
+		if (rc)
+			pr_err("unable to create bootargs_append sysfs file (%d)\n", rc);
+	}
+
 	debugfs_create_file("fadump_region", 0444, arch_debugfs_dir, NULL,
 			    &fadump_region_fops);
 
@@ -1740,7 +1746,7 @@  static void __init fadump_process(void)
  * Reserve memory to store additional parameters to be passed
  * for fadump/capture kernel.
  */
-static void __init fadump_setup_param_area(void)
+void __init fadump_setup_param_area(void)
 {
 	phys_addr_t range_start, range_end;
 
@@ -1748,7 +1754,7 @@  static void __init fadump_setup_param_area(void)
 		return;
 
 	/* This memory can't be used by PFW or bootloader as it is shared across kernels */
-	if (radix_enabled()) {
+	if (early_radix_enabled()) {
 		/*
 		 * Anywhere in the upper half should be good enough as all memory
 		 * is accessible in real mode.
@@ -1776,12 +1782,12 @@  static void __init fadump_setup_param_area(void)
 						       COMMAND_LINE_SIZE,
 						       range_start,
 						       range_end);
-	if (!fw_dump.param_area || sysfs_create_file(fadump_kobj, &bootargs_append_attr.attr)) {
+	if (!fw_dump.param_area) {
 		pr_warn("WARNING: Could not setup area to pass additional parameters!\n");
 		return;
 	}
 
-	memset(phys_to_virt(fw_dump.param_area), 0, COMMAND_LINE_SIZE);
+	memset((void *)fw_dump.param_area, 0, COMMAND_LINE_SIZE);
 }
 
 /*
@@ -1807,7 +1813,6 @@  int __init setup_fadump(void)
 	}
 	/* Initialize the kernel dump memory structure and register with f/w */
 	else if (fw_dump.reserve_dump_area_size) {
-		fadump_setup_param_area();
 		fw_dump.ops->fadump_init_mem_struct(&fw_dump);
 		register_fadump();
 	}
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 0be07ed407c7..7ac03655338d 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -908,6 +908,13 @@  void __init early_init_devtree(void *params)
 
 	mmu_early_init_devtree();
 
+	/*
+	 * Setup param area for passing additional parameters to the fadump
+	 * capture kernel. Since this depends on the MMU type, the function
+	 * must be called after the MMU type is finalized.
+	 */
+	fadump_setup_param_area();
+
 #ifdef CONFIG_PPC_POWERNV
 	/* Scan and build the list of machine check recoverable ranges */
 	of_scan_flat_dt(early_init_dt_scan_recoverable_ranges, NULL);