[PATCH 1/2] powerpc: reserve memory for capture kernel after hugepages init

[PATCH 1/2] powerpc: reserve memory for capture kernel after hugepages init

[Hari Bathini]

Sometimes, memory reservation for KDump/FADump can overlap with memory
marked for hugepages. This overlap leads to error, hang in KDump case
and copy error reported by f/w in case of FADump, while trying to
capture dump. Report error while setting up memory for the capture
kernel instead of running into issues while capturing dump, by moving
KDump/FADump reservation below MMU early init and failing gracefully
when hugepages memory overlaps with capture kernel memory.

Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
 arch/powerpc/kernel/prom.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 7159e79..454e19cf 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -731,14 +731,6 @@ void __init early_init_devtree(void *params)
 	if (PHYSICAL_START > MEMORY_START)
 		memblock_reserve(MEMORY_START, 0x8000);
 	reserve_kdump_trampoline();
-#ifdef CONFIG_FA_DUMP
-	/*
-	 * If we fail to reserve memory for firmware-assisted dump then
-	 * fallback to kexec based kdump.
-	 */
-	if (fadump_reserve_mem() == 0)
-#endif
-		reserve_crashkernel();
 	early_reserve_mem();
 
 	/* Ensure that total memory size is page-aligned. */
@@ -777,6 +769,14 @@ void __init early_init_devtree(void *params)
 #endif
 
 	mmu_early_init_devtree();
+#ifdef CONFIG_FA_DUMP
+	/*
+	 * If we fail to reserve memory for firmware-assisted dump then
+	 * fallback to kexec based kdump.
+	 */
+	if (fadump_reserve_mem() == 0)
+#endif
+		reserve_crashkernel();
 
 #ifdef CONFIG_PPC_POWERNV
 	/* Scan and build the list of machine check recoverable ranges */

[PATCH 2/2] powerpc: avoid adjusting memory_limit for capture kernel memory reservation

[Hari Bathini]

Currently, if memory_limit is specified and it overlaps with memory to
be reserved for capture kernel, memory_limit is adjusted to accommodate
capture kernel. With memory reservation for capture kernel moved later
(after enforcing memory limit), this adjustment no longer holds water.
So, avoid adjusting memory_limit and error out instead.

Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
 arch/powerpc/kernel/fadump.c        |   16 ----------------
 arch/powerpc/kernel/machine_kexec.c |   22 +++++++++++-----------
 2 files changed, 11 insertions(+), 27 deletions(-)

diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 4eab972..a784695 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -476,22 +476,6 @@ int __init fadump_reserve_mem(void)
 #endif
 	}
 
-	/*
-	 * Calculate the memory boundary.
-	 * If memory_limit is less than actual memory boundary then reserve
-	 * the memory for fadump beyond the memory_limit and adjust the
-	 * memory_limit accordingly, so that the running kernel can run with
-	 * specified memory_limit.
-	 */
-	if (memory_limit && memory_limit < memblock_end_of_DRAM()) {
-		size = get_fadump_area_size();
-		if ((memory_limit + size) < memblock_end_of_DRAM())
-			memory_limit += size;
-		else
-			memory_limit = memblock_end_of_DRAM();
-		printk(KERN_INFO "Adjusted memory_limit for firmware-assisted"
-				" dump, now %#016llx\n", memory_limit);
-	}
 	if (memory_limit)
 		memory_boundary = memory_limit;
 	else
diff --git a/arch/powerpc/kernel/machine_kexec.c b/arch/powerpc/kernel/machine_kexec.c
index c4ed328..fc5533b 100644
--- a/arch/powerpc/kernel/machine_kexec.c
+++ b/arch/powerpc/kernel/machine_kexec.c
@@ -125,10 +125,8 @@ void __init reserve_crashkernel(void)
 		crashk_res.end = crash_base + crash_size - 1;
 	}
 
-	if (crashk_res.end == crashk_res.start) {
-		crashk_res.start = crashk_res.end = 0;
-		return;
-	}
+	if (crashk_res.end == crashk_res.start)
+		goto error_out;
 
 	/* We might have got these values via the command line or the
 	 * device tree, either way sanitise them now. */
@@ -170,15 +168,13 @@ void __init reserve_crashkernel(void)
 	if (overlaps_crashkernel(__pa(_stext), _end - _stext)) {
 		printk(KERN_WARNING
 			"Crash kernel can not overlap current kernel\n");
-		crashk_res.start = crashk_res.end = 0;
-		return;
+		goto error_out;
 	}
 
 	/* Crash kernel trumps memory limit */
 	if (memory_limit && memory_limit <= crashk_res.end) {
-		memory_limit = crashk_res.end + 1;
-		printk("Adjusted memory limit for crashkernel, now 0x%llx\n",
-		       memory_limit);
+		pr_err("Crash kernel size can't exceed memory_limit\n");
+		goto error_out;
 	}
 
 	printk(KERN_INFO "Reserving %ldMB of memory at %ldMB "
@@ -190,9 +186,13 @@ void __init reserve_crashkernel(void)
 	if (!memblock_is_region_memory(crashk_res.start, crash_size) ||
 	    memblock_reserve(crashk_res.start, crash_size)) {
 		pr_err("Failed to reserve memory for crashkernel!\n");
-		crashk_res.start = crashk_res.end = 0;
-		return;
+		goto error_out;
 	}
+
+	return;
+error_out:
+	crashk_res.start = crashk_res.end = 0;
+	return;
 }
 
 int overlaps_crashkernel(unsigned long start, unsigned long size)

Re: [PATCH 2/2] powerpc: avoid adjusting memory_limit for capture kernel memory reservation

[Michal Suchánek]

On Fri, 28 Jun 2019 00:51:19 +0530
Hari Bathini <hbathini@linux.ibm.com> wrote:

> Currently, if memory_limit is specified and it overlaps with memory to
> be reserved for capture kernel, memory_limit is adjusted to accommodate
> capture kernel. With memory reservation for capture kernel moved later
> (after enforcing memory limit), this adjustment no longer holds water.
> So, avoid adjusting memory_limit and error out instead.

Can you split out the memory limit adjustment out of memory reservation
so it can still be adjusted?

Thanks

Michal
> 
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
>  arch/powerpc/kernel/fadump.c        |   16 ----------------
>  arch/powerpc/kernel/machine_kexec.c |   22 +++++++++++-----------
>  2 files changed, 11 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 4eab972..a784695 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -476,22 +476,6 @@ int __init fadump_reserve_mem(void)
>  #endif
>  	}
>  
> -	/*
> -	 * Calculate the memory boundary.
> -	 * If memory_limit is less than actual memory boundary then reserve
> -	 * the memory for fadump beyond the memory_limit and adjust the
> -	 * memory_limit accordingly, so that the running kernel can run with
> -	 * specified memory_limit.
> -	 */
> -	if (memory_limit && memory_limit < memblock_end_of_DRAM()) {
> -		size = get_fadump_area_size();
> -		if ((memory_limit + size) < memblock_end_of_DRAM())
> -			memory_limit += size;
> -		else
> -			memory_limit = memblock_end_of_DRAM();
> -		printk(KERN_INFO "Adjusted memory_limit for firmware-assisted"
> -				" dump, now %#016llx\n", memory_limit);
> -	}
>  	if (memory_limit)
>  		memory_boundary = memory_limit;
>  	else
> diff --git a/arch/powerpc/kernel/machine_kexec.c b/arch/powerpc/kernel/machine_kexec.c
> index c4ed328..fc5533b 100644
> --- a/arch/powerpc/kernel/machine_kexec.c
> +++ b/arch/powerpc/kernel/machine_kexec.c
> @@ -125,10 +125,8 @@ void __init reserve_crashkernel(void)
>  		crashk_res.end = crash_base + crash_size - 1;
>  	}
>  
> -	if (crashk_res.end == crashk_res.start) {
> -		crashk_res.start = crashk_res.end = 0;
> -		return;
> -	}
> +	if (crashk_res.end == crashk_res.start)
> +		goto error_out;
>  
>  	/* We might have got these values via the command line or the
>  	 * device tree, either way sanitise them now. */
> @@ -170,15 +168,13 @@ void __init reserve_crashkernel(void)
>  	if (overlaps_crashkernel(__pa(_stext), _end - _stext)) {
>  		printk(KERN_WARNING
>  			"Crash kernel can not overlap current kernel\n");
> -		crashk_res.start = crashk_res.end = 0;
> -		return;
> +		goto error_out;
>  	}
>  
>  	/* Crash kernel trumps memory limit */
>  	if (memory_limit && memory_limit <= crashk_res.end) {
> -		memory_limit = crashk_res.end + 1;
> -		printk("Adjusted memory limit for crashkernel, now 0x%llx\n",
> -		       memory_limit);
> +		pr_err("Crash kernel size can't exceed memory_limit\n");
> +		goto error_out;
>  	}
>  
>  	printk(KERN_INFO "Reserving %ldMB of memory at %ldMB "
> @@ -190,9 +186,13 @@ void __init reserve_crashkernel(void)
>  	if (!memblock_is_region_memory(crashk_res.start, crash_size) ||
>  	    memblock_reserve(crashk_res.start, crash_size)) {
>  		pr_err("Failed to reserve memory for crashkernel!\n");
> -		crashk_res.start = crashk_res.end = 0;
> -		return;
> +		goto error_out;
>  	}
> +
> +	return;
> +error_out:
> +	crashk_res.start = crashk_res.end = 0;
> +	return;
>  }
>  
>  int overlaps_crashkernel(unsigned long start, unsigned long size)
> 

Re: [PATCH 2/2] powerpc: avoid adjusting memory_limit for capture kernel memory reservation

[Mahesh Jagannath Salgaonkar]

On 7/22/19 11:19 PM, Michal Suchánek wrote:
> On Fri, 28 Jun 2019 00:51:19 +0530
> Hari Bathini <hbathini@linux.ibm.com> wrote:
> 
>> Currently, if memory_limit is specified and it overlaps with memory to
>> be reserved for capture kernel, memory_limit is adjusted to accommodate
>> capture kernel. With memory reservation for capture kernel moved later
>> (after enforcing memory limit), this adjustment no longer holds water.
>> So, avoid adjusting memory_limit and error out instead.
> 
> Can you split out the memory limit adjustment out of memory reservation
> so it can still be adjusted?

Do you mean adjust the memory limit before we do the actual reservation ?

> 
> Thanks
> 
> Michal
>>
>> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
>> ---
>>  arch/powerpc/kernel/fadump.c        |   16 ----------------
>>  arch/powerpc/kernel/machine_kexec.c |   22 +++++++++++-----------
>>  2 files changed, 11 insertions(+), 27 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
>> index 4eab972..a784695 100644
>> --- a/arch/powerpc/kernel/fadump.c
>> +++ b/arch/powerpc/kernel/fadump.c
>> @@ -476,22 +476,6 @@ int __init fadump_reserve_mem(void)
>>  #endif
>>  	}
>>  
>> -	/*
>> -	 * Calculate the memory boundary.
>> -	 * If memory_limit is less than actual memory boundary then reserve
>> -	 * the memory for fadump beyond the memory_limit and adjust the
>> -	 * memory_limit accordingly, so that the running kernel can run with
>> -	 * specified memory_limit.
>> -	 */
>> -	if (memory_limit && memory_limit < memblock_end_of_DRAM()) {
>> -		size = get_fadump_area_size();
>> -		if ((memory_limit + size) < memblock_end_of_DRAM())
>> -			memory_limit += size;
>> -		else
>> -			memory_limit = memblock_end_of_DRAM();
>> -		printk(KERN_INFO "Adjusted memory_limit for firmware-assisted"
>> -				" dump, now %#016llx\n", memory_limit);
>> -	}
>>  	if (memory_limit)
>>  		memory_boundary = memory_limit;
>>  	else
>> diff --git a/arch/powerpc/kernel/machine_kexec.c b/arch/powerpc/kernel/machine_kexec.c
>> index c4ed328..fc5533b 100644
>> --- a/arch/powerpc/kernel/machine_kexec.c
>> +++ b/arch/powerpc/kernel/machine_kexec.c
>> @@ -125,10 +125,8 @@ void __init reserve_crashkernel(void)
>>  		crashk_res.end = crash_base + crash_size - 1;
>>  	}
>>  
>> -	if (crashk_res.end == crashk_res.start) {
>> -		crashk_res.start = crashk_res.end = 0;
>> -		return;
>> -	}
>> +	if (crashk_res.end == crashk_res.start)
>> +		goto error_out;
>>  
>>  	/* We might have got these values via the command line or the
>>  	 * device tree, either way sanitise them now. */
>> @@ -170,15 +168,13 @@ void __init reserve_crashkernel(void)
>>  	if (overlaps_crashkernel(__pa(_stext), _end - _stext)) {
>>  		printk(KERN_WARNING
>>  			"Crash kernel can not overlap current kernel\n");
>> -		crashk_res.start = crashk_res.end = 0;
>> -		return;
>> +		goto error_out;
>>  	}
>>  
>>  	/* Crash kernel trumps memory limit */
>>  	if (memory_limit && memory_limit <= crashk_res.end) {
>> -		memory_limit = crashk_res.end + 1;
>> -		printk("Adjusted memory limit for crashkernel, now 0x%llx\n",
>> -		       memory_limit);
>> +		pr_err("Crash kernel size can't exceed memory_limit\n");
>> +		goto error_out;
>>  	}
>>  
>>  	printk(KERN_INFO "Reserving %ldMB of memory at %ldMB "
>> @@ -190,9 +186,13 @@ void __init reserve_crashkernel(void)
>>  	if (!memblock_is_region_memory(crashk_res.start, crash_size) ||
>>  	    memblock_reserve(crashk_res.start, crash_size)) {
>>  		pr_err("Failed to reserve memory for crashkernel!\n");
>> -		crashk_res.start = crashk_res.end = 0;
>> -		return;
>> +		goto error_out;
>>  	}
>> +
>> +	return;
>> +error_out:
>> +	crashk_res.start = crashk_res.end = 0;
>> +	return;
>>  }
>>  
>>  int overlaps_crashkernel(unsigned long start, unsigned long size)
>>
> 

Re: [PATCH 2/2] powerpc: avoid adjusting memory_limit for capture kernel memory reservation

[Michal Suchánek]

On Wed, Jul 24, 2019 at 11:26:59AM +0530, Mahesh Jagannath Salgaonkar wrote:
> On 7/22/19 11:19 PM, Michal Suchánek wrote:
> > On Fri, 28 Jun 2019 00:51:19 +0530
> > Hari Bathini <hbathini@linux.ibm.com> wrote:
> > 
> >> Currently, if memory_limit is specified and it overlaps with memory to
> >> be reserved for capture kernel, memory_limit is adjusted to accommodate
> >> capture kernel. With memory reservation for capture kernel moved later
> >> (after enforcing memory limit), this adjustment no longer holds water.
> >> So, avoid adjusting memory_limit and error out instead.
> > 
> > Can you split out the memory limit adjustment out of memory reservation
> > so it can still be adjusted?
> 
> Do you mean adjust the memory limit before we do the actual reservation ?

Yes, without that you get a regression in ability to enable fadump with
limited memory - something like the below patch should fix it. Then
again, there is no code to un-move the memory_limit in case the allocation
fails, and we now have cma allocation which is dubious to allocate
beyond memory_limit. So maybe removing the memory_limit adjustment is a
bugfix removing 'feature' that has bitrotten over time.

Thanks

Michal

From: Michal Suchanek <msuchanek@suse.de>
Date: Mon, 6 Jan 2020 14:55:40 +0100
Subject: [PATCH 2/2] powerpc/fadump: adjust memlimit before MMU early init

Moving the farump memory reservation before early MMU init makes the
memlimit adjustment to make room for fadump ineffective.

Move the adjustment back before early MMU init.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/include/asm/fadump.h |  3 +-
 arch/powerpc/kernel/fadump.c      | 80 +++++++++++++++++++++++--------
 arch/powerpc/kernel/prom.c        |  3 ++
 3 files changed, 66 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h
index 526a6a647312..76d3cbe1379c 100644
--- a/arch/powerpc/include/asm/fadump.h
+++ b/arch/powerpc/include/asm/fadump.h
@@ -30,6 +30,7 @@ static inline void fadump_cleanup(void) { }
 #if defined(CONFIG_FA_DUMP) || defined(CONFIG_PRESERVE_FA_DUMP)
 extern int early_init_dt_scan_fw_dump(unsigned long node, const char *uname,
 				      int depth, void *data);
-extern int fadump_reserve_mem(void);
+int fadump_adjust_memlimit(void);
+int fadump_reserve_mem(void);
 #endif
 #endif /* _ASM_POWERPC_FADUMP_H */
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 8ad6d8d1cdbe..4d76452dcb3d 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -431,19 +431,22 @@ static int __init fadump_get_boot_mem_regions(void)
 	return ret;
 }
 
-int __init fadump_reserve_mem(void)
+static inline u64 fadump_get_reserve_alignment(void)
 {
-	u64 base, size, mem_boundary, bootmem_min, align = PAGE_SIZE;
-	bool is_memblock_bottom_up = memblock_bottom_up();
-	int ret = 1;
+	u64 align = PAGE_SIZE;
 
-	if (!fw_dump.fadump_enabled)
-		return 0;
+#ifdef CONFIG_CMA
+	if (!fw_dump.nocma)
+		align = FADUMP_CMA_ALIGNMENT;
+#endif
 
-	if (!fw_dump.fadump_supported) {
-		pr_info("Firmware-Assisted Dump is not supported on this hardware\n");
-		goto error_out;
-	}
+	return align;
+}
+
+static inline u64 fadump_get_bootmem_min(void)
+{
+	u64 bootmem_min = 0;
+	u64 align = fadump_get_reserve_alignment();
 
 	/*
 	 * Initialize boot memory size
@@ -455,7 +458,6 @@ int __init fadump_reserve_mem(void)
 			PAGE_ALIGN(fadump_calculate_reserve_size());
 #ifdef CONFIG_CMA
 		if (!fw_dump.nocma) {
-			align = FADUMP_CMA_ALIGNMENT;
 			fw_dump.boot_memory_size =
 				ALIGN(fw_dump.boot_memory_size, align);
 		}
@@ -472,8 +474,43 @@ int __init fadump_reserve_mem(void)
 			pr_err("Too many holes in boot memory area to enable fadump\n");
 			goto error_out;
 		}
+
+	}
+
+	return bootmem_min;
+error_out:
+	fw_dump.fadump_enabled = 0;
+	return 0;
+}
+
+int __init fadump_adjust_memlimit(void)
+{
+	u64 size, bootmem_min;
+
+	if (!fw_dump.fadump_enabled)
+		return 0;
+
+	if (!fw_dump.fadump_supported) {
+		pr_info("Firmware-Assisted Dump is not supported on this hardware\n");
+		fw_dump.fadump_enabled = 0;
+		return 0;
 	}
 
+#ifdef CONFIG_HUGETLB_PAGE
+	if (fw_dump.dump_active) {
+		/*
+		 * FADump capture kernel doesn't care much about hugepages.
+		 * In fact, handling hugepages in capture kernel is asking for
+		 * trouble. So, disable HugeTLB support when fadump is active.
+		 */
+		hugetlb_disabled = true;
+	}
+#endif
+
+	bootmem_min = fadump_get_bootmem_min();
+	if (!bootmem_min)
+		return 0;
+
 	/*
 	 * Calculate the memory boundary.
 	 * If memory_limit is less than actual memory boundary then reserve
@@ -490,6 +527,19 @@ int __init fadump_reserve_mem(void)
 		printk(KERN_INFO "Adjusted memory_limit for firmware-assisted"
 				" dump, now %#016llx\n", memory_limit);
 	}
+
+	return 0;
+}
+
+int __init fadump_reserve_mem(void)
+{
+	u64 base, size, mem_boundary, align = fadump_get_reserve_alignment();
+	bool is_memblock_bottom_up = memblock_bottom_up();
+	int ret = 1;
+
+	if (!fw_dump.fadump_enabled)
+		return 0;
+
 	if (memory_limit)
 		mem_boundary = memory_limit;
 	else
@@ -501,14 +551,6 @@ int __init fadump_reserve_mem(void)
 	if (fw_dump.dump_active) {
 		pr_info("Firmware-assisted dump is active.\n");
 
-#ifdef CONFIG_HUGETLB_PAGE
-		/*
-		 * FADump capture kernel doesn't care much about hugepages.
-		 * In fact, handling hugepages in capture kernel is asking for
-		 * trouble. So, disable HugeTLB support when fadump is active.
-		 */
-		hugetlb_disabled = true;
-#endif
 		/*
 		 * If last boot has crashed then reserve all the memory
 		 * above boot memory size so that we don't touch it until
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 7b6cdd9bf78d..4fff8c2222b1 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -731,6 +731,9 @@ void __init early_init_devtree(void *params)
 	if (PHYSICAL_START > MEMORY_START)
 		memblock_reserve(MEMORY_START, 0x8000);
 	reserve_kdump_trampoline();
+#if defined(CONFIG_FA_DUMP) || defined(CONFIG_PRESERVE_FA_DUMP)
+	fadump_adjust_memlimit();
+#endif
 	early_reserve_mem();
 
 	/* Ensure that total memory size is page-aligned. */
-- 
2.23.0

[PATCH rebased 1/2] powerpc: reserve memory for capture kernel after hugepages init

[Michal Suchanek]

From: Hari Bathini <hbathini@linux.ibm.com>

Sometimes, memory reservation for KDump/FADump can overlap with memory
marked for hugepages. This overlap leads to error, hang in KDump case
and copy error reported by f/w in case of FADump, while trying to
capture dump. Report error while setting up memory for the capture
kernel instead of running into issues while capturing dump, by moving
KDump/FADump reservation below MMU early init and failing gracefully
when hugepages memory overlaps with capture kernel memory.

Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
 arch/powerpc/kernel/prom.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 6620f37abe73..0f14dc9c4dab 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -735,14 +735,6 @@ void __init early_init_devtree(void *params)
 	if (PHYSICAL_START > MEMORY_START)
 		memblock_reserve(MEMORY_START, 0x8000);
 	reserve_kdump_trampoline();
-#if defined(CONFIG_FA_DUMP) || defined(CONFIG_PRESERVE_FA_DUMP)
-	/*
-	 * If we fail to reserve memory for firmware-assisted dump then
-	 * fallback to kexec based kdump.
-	 */
-	if (fadump_reserve_mem() == 0)
-#endif
-		reserve_crashkernel();
 	early_reserve_mem();
 
 	/* Ensure that total memory size is page-aligned. */
@@ -781,6 +773,14 @@ void __init early_init_devtree(void *params)
 #endif
 
 	mmu_early_init_devtree();
+#if defined(CONFIG_FA_DUMP) || defined(CONFIG_PRESERVE_FA_DUMP)
+	/*
+	 * If we fail to reserve memory for firmware-assisted dump then
+	 * fallback to kexec based kdump.
+	 */
+	if (fadump_reserve_mem() == 0)
+#endif
+		reserve_crashkernel();
 
 #ifdef CONFIG_PPC_POWERNV
 	/* Scan and build the list of machine check recoverable ranges */
-- 
2.23.0

[PATCH rebased 2/2] powerpc: avoid adjusting memory_limit for capture kernel memory reservation

[Michal Suchanek]

From: Hari Bathini <hbathini@linux.ibm.com>

Currently, if memory_limit is specified and it overlaps with memory to
be reserved for capture kernel, memory_limit is adjusted to accommodate
capture kernel. With memory reservation for capture kernel moved later
(after enforcing memory limit), this adjustment no longer holds water.
So, avoid adjusting memory_limit and error out instead.

Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
Reviewed-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/kernel/fadump.c | 16 ----------------
 arch/powerpc/kexec/core.c    | 22 +++++++++++-----------
 2 files changed, 11 insertions(+), 27 deletions(-)

diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index ff0114aeba9b..54d56ecadf1a 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -472,22 +472,6 @@ int __init fadump_reserve_mem(void)
 		}
 	}
 
-	/*
-	 * Calculate the memory boundary.
-	 * If memory_limit is less than actual memory boundary then reserve
-	 * the memory for fadump beyond the memory_limit and adjust the
-	 * memory_limit accordingly, so that the running kernel can run with
-	 * specified memory_limit.
-	 */
-	if (memory_limit && memory_limit < memblock_end_of_DRAM()) {
-		size = get_fadump_area_size();
-		if ((memory_limit + size) < memblock_end_of_DRAM())
-			memory_limit += size;
-		else
-			memory_limit = memblock_end_of_DRAM();
-		printk(KERN_INFO "Adjusted memory_limit for firmware-assisted"
-				" dump, now %#016llx\n", memory_limit);
-	}
 	if (memory_limit)
 		mem_boundary = memory_limit;
 	else
diff --git a/arch/powerpc/kexec/core.c b/arch/powerpc/kexec/core.c
index 078fe3d76feb..491e08d98c1b 100644
--- a/arch/powerpc/kexec/core.c
+++ b/arch/powerpc/kexec/core.c
@@ -126,10 +126,8 @@ void __init reserve_crashkernel(void)
 		crashk_res.end = crash_base + crash_size - 1;
 	}
 
-	if (crashk_res.end == crashk_res.start) {
-		crashk_res.start = crashk_res.end = 0;
-		return;
-	}
+	if (crashk_res.end == crashk_res.start)
+		goto error_out;
 
 	/* We might have got these values via the command line or the
 	 * device tree, either way sanitise them now. */
@@ -171,15 +169,13 @@ void __init reserve_crashkernel(void)
 	if (overlaps_crashkernel(__pa(_stext), _end - _stext)) {
 		printk(KERN_WARNING
 			"Crash kernel can not overlap current kernel\n");
-		crashk_res.start = crashk_res.end = 0;
-		return;
+		goto error_out;
 	}
 
 	/* Crash kernel trumps memory limit */
 	if (memory_limit && memory_limit <= crashk_res.end) {
-		memory_limit = crashk_res.end + 1;
-		printk("Adjusted memory limit for crashkernel, now 0x%llx\n",
-		       memory_limit);
+		pr_err("Crash kernel size can't exceed memory_limit\n");
+		goto error_out;
 	}
 
 	printk(KERN_INFO "Reserving %ldMB of memory at %ldMB "
@@ -191,9 +187,13 @@ void __init reserve_crashkernel(void)
 	if (!memblock_is_region_memory(crashk_res.start, crash_size) ||
 	    memblock_reserve(crashk_res.start, crash_size)) {
 		pr_err("Failed to reserve memory for crashkernel!\n");
-		crashk_res.start = crashk_res.end = 0;
-		return;
+		goto error_out;
 	}
+
+	return;
+error_out:
+	crashk_res.start = crashk_res.end = 0;
+	return;
 }
 
 int overlaps_crashkernel(unsigned long start, unsigned long size)
-- 
2.23.0

Re: [PATCH 2/2] powerpc: avoid adjusting memory_limit for capture kernel memory reservation

[Michal Suchánek]

On Fri, Jun 28, 2019 at 12:51:19AM +0530, Hari Bathini wrote:
> Currently, if memory_limit is specified and it overlaps with memory to
> be reserved for capture kernel, memory_limit is adjusted to accommodate
> capture kernel. With memory reservation for capture kernel moved later
> (after enforcing memory limit), this adjustment no longer holds water.
> So, avoid adjusting memory_limit and error out instead.

The adjustment of memory limit does not look quite sound
 - There is no code to undo the adjustment in case reservation fails
 - I don't think reservation is still forced to the end of memory
   causing the kernel to use memory it was supposed not to
 - The CMA reservation again causes teh reserved memory to be used
 - Finally the CMA reservation makes this obsolete because the reserved
   memory is can be used by the system

> 
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
Reviewed-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  arch/powerpc/kernel/fadump.c        |   16 ----------------
>  arch/powerpc/kernel/machine_kexec.c |   22 +++++++++++-----------
>  2 files changed, 11 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 4eab972..a784695 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -476,22 +476,6 @@ int __init fadump_reserve_mem(void)
>  #endif
>  	}
>  
> -	/*
> -	 * Calculate the memory boundary.
> -	 * If memory_limit is less than actual memory boundary then reserve
> -	 * the memory for fadump beyond the memory_limit and adjust the
> -	 * memory_limit accordingly, so that the running kernel can run with
> -	 * specified memory_limit.
> -	 */
> -	if (memory_limit && memory_limit < memblock_end_of_DRAM()) {
> -		size = get_fadump_area_size();
> -		if ((memory_limit + size) < memblock_end_of_DRAM())
> -			memory_limit += size;
> -		else
> -			memory_limit = memblock_end_of_DRAM();
> -		printk(KERN_INFO "Adjusted memory_limit for firmware-assisted"
> -				" dump, now %#016llx\n", memory_limit);
> -	}
>  	if (memory_limit)
>  		memory_boundary = memory_limit;
>  	else
> diff --git a/arch/powerpc/kernel/machine_kexec.c b/arch/powerpc/kernel/machine_kexec.c
> index c4ed328..fc5533b 100644
> --- a/arch/powerpc/kernel/machine_kexec.c
> +++ b/arch/powerpc/kernel/machine_kexec.c
> @@ -125,10 +125,8 @@ void __init reserve_crashkernel(void)
>  		crashk_res.end = crash_base + crash_size - 1;
>  	}
>  
> -	if (crashk_res.end == crashk_res.start) {
> -		crashk_res.start = crashk_res.end = 0;
> -		return;
> -	}
> +	if (crashk_res.end == crashk_res.start)
> +		goto error_out;
>  
>  	/* We might have got these values via the command line or the
>  	 * device tree, either way sanitise them now. */
> @@ -170,15 +168,13 @@ void __init reserve_crashkernel(void)
>  	if (overlaps_crashkernel(__pa(_stext), _end - _stext)) {
>  		printk(KERN_WARNING
>  			"Crash kernel can not overlap current kernel\n");
> -		crashk_res.start = crashk_res.end = 0;
> -		return;
> +		goto error_out;
>  	}
>  
>  	/* Crash kernel trumps memory limit */
>  	if (memory_limit && memory_limit <= crashk_res.end) {
> -		memory_limit = crashk_res.end + 1;
> -		printk("Adjusted memory limit for crashkernel, now 0x%llx\n",
> -		       memory_limit);
> +		pr_err("Crash kernel size can't exceed memory_limit\n");
> +		goto error_out;
>  	}
>  
>  	printk(KERN_INFO "Reserving %ldMB of memory at %ldMB "
> @@ -190,9 +186,13 @@ void __init reserve_crashkernel(void)
>  	if (!memblock_is_region_memory(crashk_res.start, crash_size) ||
>  	    memblock_reserve(crashk_res.start, crash_size)) {
>  		pr_err("Failed to reserve memory for crashkernel!\n");
> -		crashk_res.start = crashk_res.end = 0;
> -		return;
> +		goto error_out;
>  	}
> +
> +	return;
> +error_out:
> +	crashk_res.start = crashk_res.end = 0;
> +	return;
>  }
>  
>  int overlaps_crashkernel(unsigned long start, unsigned long size)
> 

Re: [PATCH rebased 1/2] powerpc: reserve memory for capture kernel after hugepages init

[Michal Suchánek]

Hello,

This seems to cause crash with kdump reservation 1GB quite reliably.

Thanks

Michal

On Tue, Feb 18, 2020 at 05:28:34PM +0100, Michal Suchanek wrote:
> From: Hari Bathini <hbathini@linux.ibm.com>
> 
> Sometimes, memory reservation for KDump/FADump can overlap with memory
> marked for hugepages. This overlap leads to error, hang in KDump case
> and copy error reported by f/w in case of FADump, while trying to
> capture dump. Report error while setting up memory for the capture
> kernel instead of running into issues while capturing dump, by moving
> KDump/FADump reservation below MMU early init and failing gracefully
> when hugepages memory overlaps with capture kernel memory.
> 
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
>  arch/powerpc/kernel/prom.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 6620f37abe73..0f14dc9c4dab 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -735,14 +735,6 @@ void __init early_init_devtree(void *params)
>  	if (PHYSICAL_START > MEMORY_START)
>  		memblock_reserve(MEMORY_START, 0x8000);
>  	reserve_kdump_trampoline();
> -#if defined(CONFIG_FA_DUMP) || defined(CONFIG_PRESERVE_FA_DUMP)
> -	/*
> -	 * If we fail to reserve memory for firmware-assisted dump then
> -	 * fallback to kexec based kdump.
> -	 */
> -	if (fadump_reserve_mem() == 0)
> -#endif
> -		reserve_crashkernel();
>  	early_reserve_mem();
>  
>  	/* Ensure that total memory size is page-aligned. */
> @@ -781,6 +773,14 @@ void __init early_init_devtree(void *params)
>  #endif
>  
>  	mmu_early_init_devtree();
> +#if defined(CONFIG_FA_DUMP) || defined(CONFIG_PRESERVE_FA_DUMP)
> +	/*
> +	 * If we fail to reserve memory for firmware-assisted dump then
> +	 * fallback to kexec based kdump.
> +	 */
> +	if (fadump_reserve_mem() == 0)
> +#endif
> +		reserve_crashkernel();
>  
>  #ifdef CONFIG_PPC_POWERNV
>  	/* Scan and build the list of machine check recoverable ranges */
> -- 
> 2.23.0
> 

Re: [PATCH 1/2] powerpc: reserve memory for capture kernel after hugepages init

[Christophe Leroy]

Le 27/06/2019 à 21:21, Hari Bathini a écrit :
> Sometimes, memory reservation for KDump/FADump can overlap with memory
> marked for hugepages. This overlap leads to error, hang in KDump case
> and copy error reported by f/w in case of FADump, while trying to
> capture dump. Report error while setting up memory for the capture
> kernel instead of running into issues while capturing dump, by moving
> KDump/FADump reservation below MMU early init and failing gracefully
> when hugepages memory overlaps with capture kernel memory.

This patch doesn't apply, if it's still needed can you please rebase ?

Christophe

> 
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
>   arch/powerpc/kernel/prom.c |   16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 7159e79..454e19cf 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -731,14 +731,6 @@ void __init early_init_devtree(void *params)
>   	if (PHYSICAL_START > MEMORY_START)
>   		memblock_reserve(MEMORY_START, 0x8000);
>   	reserve_kdump_trampoline();
> -#ifdef CONFIG_FA_DUMP
> -	/*
> -	 * If we fail to reserve memory for firmware-assisted dump then
> -	 * fallback to kexec based kdump.
> -	 */
> -	if (fadump_reserve_mem() == 0)
> -#endif
> -		reserve_crashkernel();
>   	early_reserve_mem();
>   
>   	/* Ensure that total memory size is page-aligned. */
> @@ -777,6 +769,14 @@ void __init early_init_devtree(void *params)
>   #endif
>   
>   	mmu_early_init_devtree();
> +#ifdef CONFIG_FA_DUMP
> +	/*
> +	 * If we fail to reserve memory for firmware-assisted dump then
> +	 * fallback to kexec based kdump.
> +	 */
> +	if (fadump_reserve_mem() == 0)
> +#endif
> +		reserve_crashkernel();
>   
>   #ifdef CONFIG_PPC_POWERNV
>   	/* Scan and build the list of machine check recoverable ranges */