Hello Ritesh,
On 12/11/24 11:51, Ritesh Harjani (IBM) wrote:
Sourabh Jain <sourabhj...@linux.ibm.com> writes:
The param area is a memory region where the kernel places additional
command-line arguments for fadump kernel. Currently, the param memory
area is reserved in fadump kernel if it is above boot_mem_top. However,
it should be reserved if it is below boot_mem_top because the fadump
kernel already reserves memory from boot_mem_top to the end of DRAM.
did you mean s/reserves/preserves ?
Yeah, preserves is better.
Currently, there is no impact from not reserving param memory if it is
below boot_mem_top, as it is not used after the early boot phase of the
fadump kernel. However, if this changes in the future, it could lead to
issues in the fadump kernel.
This will only affect Hash and not radix correct? Because for radix your
param_area is somewhere within [memblock_end_of_DRAM() / 2,
memblock_end_of_DRAM()]
which is anyway above boot_mem_top so it is anyway preserved as is...
Yes.
... On second thoughts since param_area during normal kernel boot anyway
comes from memblock now. And irrespective of where it falls (above or below
boot_mem_top), we anyway append the bootargs to that. So we don't really
preserve the original contents :) right?
Sorry I didn't get it. We append strings from param_area to
boot_command_line
not the other way.
So why not just always call for
memblock_reserve() on param_area during capture kernel run?
Thoughts?
Yes, there is no harm in calling memblock_reserve regardless of whether
param_area
is below or above boot_mem_top. However, calling it when param_area is
higher than
boot_mem_top is redundant, as we know fadump preserves memory from
boot_mem_top
to the end of DRAM during early boot.
According to the memblock documentation, when reserving memory regions,
the new
regions can overlap with existing ones, but I don't see any advantage in
calling memblock_reserve
for param_area if it falls above boot_mem_top.
Regardless, I don’t have a strong opinion. If you think we should call
memblock_reserve regardless
of where param_area is placed, I can do that. Please let me know your
opinion.
Sourabh Jain
Fixes: 3416c9daa6b1 ("powerpc/fadump: pass additional parameters when fadump is
active")
Cc: Mahesh Salgaonkar <mah...@linux.ibm.com>
Cc: Michael Ellerman <m...@ellerman.id.au>
Acked-by: Hari Bathini <hbath...@linux.ibm.com>
Signed-off-by: Sourabh Jain <sourabhj...@linux.ibm.com>
---
Changelog:
Since v1:
https://lore.kernel.org/all/20241104083528.99520-1-sourabhj...@linux.ibm.com/
- Include Fixes and Acked-by tag in the commit message
- No functional changes
---
arch/powerpc/kernel/fadump.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 3a2863307863..3f3674060164 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -143,7 +143,7 @@ void __init fadump_append_bootargs(void)
if (!fw_dump.dump_active || !fw_dump.param_area_supported ||
!fw_dump.param_area)
return;
- if (fw_dump.param_area >= fw_dump.boot_mem_top) {
+ if (fw_dump.param_area < fw_dump.boot_mem_top) {
if (memblock_reserve(fw_dump.param_area, COMMAND_LINE_SIZE)) {
pr_warn("WARNING: Can't use additional parameters
area!\n");
fw_dump.param_area = 0;
--
2.46.2
