[PATCH 1/2] powerpc/fadump: avoid duplicates in crash memory ranges

[PATCH 1/2] powerpc/fadump: avoid duplicates in crash memory ranges

[Hari Bathini]

fadump sets up crash memory ranges to be used for creating PT_LOAD
program headers in elfcore header. Memory chunk RMA_START through
boot memory area size is added as the first memory range because
firmware, at the time of crash, moves this memory chunk to different
location specified during fadump registration making it necessary to
create a separate program header for it with the correct offset.
This memory chunk is skipped while setting up the remaining memory
ranges. But currently, there is possibility that some of this memory
may have duplicate entries like when it is hot-removed and added
again. Ensure that no two memory ranges represent the same memory.

When 5 lmbs are hot-removed and then hot-plugged before registering
fadump, here is how the program headers in /proc/vmcore exported by
fadump look like

without this change:

  Program Headers:
    Type           Offset             VirtAddr           PhysAddr
                   FileSiz            MemSiz              Flags  Align
    NOTE           0x0000000000010000 0x0000000000000000 0x0000000000000000
                   0x0000000000001894 0x0000000000001894         0
    LOAD           0x0000000000021020 0xc000000000000000 0x0000000000000000
                   0x0000000040000000 0x0000000040000000  RWE    0
    LOAD           0x0000000040031020 0xc000000000000000 0x0000000000000000
                   0x0000000010000000 0x0000000010000000  RWE    0
    LOAD           0x0000000050040000 0xc000000010000000 0x0000000010000000
                   0x0000000050000000 0x0000000050000000  RWE    0
    LOAD           0x00000000a0040000 0xc000000060000000 0x0000000060000000
                   0x000000019ffe0000 0x000000019ffe0000  RWE    0

and with this change:

  Program Headers:
    Type           Offset             VirtAddr           PhysAddr
                   FileSiz            MemSiz              Flags  Align
    NOTE           0x0000000000010000 0x0000000000000000 0x0000000000000000
                   0x0000000000001894 0x0000000000001894         0
    LOAD           0x0000000000021020 0xc000000000000000 0x0000000000000000
                   0x0000000040000000 0x0000000040000000  RWE    0
    LOAD           0x0000000040030000 0xc000000040000000 0x0000000040000000
                   0x0000000020000000 0x0000000020000000  RWE    0
    LOAD           0x0000000060030000 0xc000000060000000 0x0000000060000000
                   0x000000019ffe0000 0x000000019ffe0000  RWE    0

Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/fadump.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 8ff0dd4..03563c6 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -844,8 +844,17 @@ static void fadump_setup_crash_memory_ranges(void)
 	for_each_memblock(memory, reg) {
 		start = (unsigned long long)reg->base;
 		end = start + (unsigned long long)reg->size;
-		if (start == RMA_START && end >= fw_dump.boot_memory_size)
-			start = fw_dump.boot_memory_size;
+
+		/*
+		 * skip the first memory chunk (RMA_START through
+		 * boot_memory_size) that is already added.
+		 */
+		if (start < fw_dump.boot_memory_size && start >= RMA_START) {
+			if (end > fw_dump.boot_memory_size)
+				start = fw_dump.boot_memory_size;
+			else
+				continue;
+		}
 
 		/* add this range excluding the reserved dump area. */
 		fadump_exclude_reserved_area(start, end);

[PATCH 2/2] powerpc/fadump: avoid holes in boot memory area when fadump is registered

[Hari Bathini]

To register fadump, boot memory area - the size of low memory chunk that
is required for a kernel to boot successfully when booted with restricted
memory, is assumed to have no holes. But this memory area is currently
not protected from hot-remove operations. So, fadump could fail to
re-register after a memory hot-remove operation, if memory is removed
from boot memory area. To avoid this, ensure that memory from boot
memory area is not hot-removed when fadump is registered.

Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/fadump.h               |    1 +
 arch/powerpc/kernel/fadump.c                    |   12 ++++++++++++
 arch/powerpc/platforms/pseries/hotplug-memory.c |    7 +++++++
 3 files changed, 20 insertions(+)

diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h
index 0031806..609fccc 100644
--- a/arch/powerpc/include/asm/fadump.h
+++ b/arch/powerpc/include/asm/fadump.h
@@ -198,6 +198,7 @@ struct fad_crash_memory_ranges {
 	unsigned long long	size;
 };
 
+extern int is_fadump_boot_memory_area(u64 addr, ulong size);
 extern int early_init_dt_scan_fw_dump(unsigned long node,
 		const char *uname, int depth, void *data);
 extern int fadump_reserve_mem(void);
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 03563c6..ea7dfdc 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -114,6 +114,18 @@ int __init early_init_dt_scan_fw_dump(unsigned long node,
 	return 1;
 }
 
+/*
+ * If fadump is registered, check if the memory provided
+ * falls within boot memory area.
+ */
+int is_fadump_boot_memory_area(u64 addr, ulong size)
+{
+	if (!fw_dump.dump_registered)
+		return 0;
+
+	return (addr + size) > RMA_START && addr <= fw_dump.boot_memory_size;
+}
+
 int is_fadump_active(void)
 {
 	return fw_dump.dump_active;
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index e104c71..a186b8e 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -22,6 +22,7 @@
 #include <asm/machdep.h>
 #include <asm/prom.h>
 #include <asm/sparsemem.h>
+#include <asm/fadump.h>
 #include "pseries.h"
 
 static bool rtas_hp_event;
@@ -406,6 +407,12 @@ static bool lmb_is_removable(struct of_drconf_cell *lmb)
 	scns_per_block = block_sz / MIN_MEMORY_BLOCK_SIZE;
 	phys_addr = lmb->base_addr;
 
+#ifdef CONFIG_FA_DUMP
+	/* Don't hot-remove memory that falls in fadump boot memory area */
+	if (is_fadump_boot_memory_area(phys_addr, block_sz))
+		return false;
+#endif
+
 	for (i = 0; i < scns_per_block; i++) {
 		pfn = PFN_DOWN(phys_addr);
 		if (!pfn_present(pfn))

Re: [PATCH 2/2] powerpc/fadump: avoid holes in boot memory area when fadump is registered

[Mahesh Jagannath Salgaonkar]

On 05/04/2017 11:24 PM, Hari Bathini wrote:
> To register fadump, boot memory area - the size of low memory chunk that
> is required for a kernel to boot successfully when booted with restricted
> memory, is assumed to have no holes. But this memory area is currently
> not protected from hot-remove operations. So, fadump could fail to
> re-register after a memory hot-remove operation, if memory is removed
> from boot memory area. To avoid this, ensure that memory from boot
> memory area is not hot-removed when fadump is registered.
> 
> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>

Reviewed-by: Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com>

> ---
>  arch/powerpc/include/asm/fadump.h               |    1 +
>  arch/powerpc/kernel/fadump.c                    |   12 ++++++++++++
>  arch/powerpc/platforms/pseries/hotplug-memory.c |    7 +++++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h
> index 0031806..609fccc 100644
> --- a/arch/powerpc/include/asm/fadump.h
> +++ b/arch/powerpc/include/asm/fadump.h
> @@ -198,6 +198,7 @@ struct fad_crash_memory_ranges {
>  	unsigned long long	size;
>  };
> 
> +extern int is_fadump_boot_memory_area(u64 addr, ulong size);
>  extern int early_init_dt_scan_fw_dump(unsigned long node,
>  		const char *uname, int depth, void *data);
>  extern int fadump_reserve_mem(void);
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 03563c6..ea7dfdc 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -114,6 +114,18 @@ int __init early_init_dt_scan_fw_dump(unsigned long node,
>  	return 1;
>  }
> 
> +/*
> + * If fadump is registered, check if the memory provided
> + * falls within boot memory area.
> + */
> +int is_fadump_boot_memory_area(u64 addr, ulong size)
> +{
> +	if (!fw_dump.dump_registered)
> +		return 0;
> +
> +	return (addr + size) > RMA_START && addr <= fw_dump.boot_memory_size;
> +}
> +
>  int is_fadump_active(void)
>  {
>  	return fw_dump.dump_active;
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index e104c71..a186b8e 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -22,6 +22,7 @@
>  #include <asm/machdep.h>
>  #include <asm/prom.h>
>  #include <asm/sparsemem.h>
> +#include <asm/fadump.h>
>  #include "pseries.h"
> 
>  static bool rtas_hp_event;
> @@ -406,6 +407,12 @@ static bool lmb_is_removable(struct of_drconf_cell *lmb)
>  	scns_per_block = block_sz / MIN_MEMORY_BLOCK_SIZE;
>  	phys_addr = lmb->base_addr;
> 
> +#ifdef CONFIG_FA_DUMP
> +	/* Don't hot-remove memory that falls in fadump boot memory area */
> +	if (is_fadump_boot_memory_area(phys_addr, block_sz))
> +		return false;
> +#endif
> +
>  	for (i = 0; i < scns_per_block; i++) {
>  		pfn = PFN_DOWN(phys_addr);
>  		if (!pfn_present(pfn))
> 

Re: [PATCH 2/2] powerpc/fadump: avoid holes in boot memory area when fadump is registered

[Pingfan Liu]

----- Original Message -----
> From: "Hari Bathini" <hbathini@linux.vnet.ibm.com>
> To: "Michael Ellerman" <mpe@ellerman.id.au>
> Cc: "linuxppc-dev" <linuxppc-dev@ozlabs.org>, "Pingfan Liu" <piliu@redhat.com>, "Mahesh J Salgaonkar"
> <mahesh@linux.vnet.ibm.com>
> Sent: Friday, May 5, 2017 1:54:05 AM
> Subject: [PATCH 2/2] powerpc/fadump: avoid holes in boot memory area when fadump is registered
> 
> To register fadump, boot memory area - the size of low memory chunk that
> is required for a kernel to boot successfully when booted with restricted
> memory, is assumed to have no holes. But this memory area is currently

The continuous is required by fadump code, not by the firmware itself, right?

Thanks and regards,
Pingfan

> not protected from hot-remove operations. So, fadump could fail to
> re-register after a memory hot-remove operation, if memory is removed
> from boot memory area. To avoid this, ensure that memory from boot
> memory area is not hot-removed when fadump is registered.
> 
> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/fadump.h               |    1 +
>  arch/powerpc/kernel/fadump.c                    |   12 ++++++++++++
>  arch/powerpc/platforms/pseries/hotplug-memory.c |    7 +++++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/fadump.h
> b/arch/powerpc/include/asm/fadump.h
> index 0031806..609fccc 100644
> --- a/arch/powerpc/include/asm/fadump.h
> +++ b/arch/powerpc/include/asm/fadump.h
> @@ -198,6 +198,7 @@ struct fad_crash_memory_ranges {
>  	unsigned long long	size;
>  };
>  
> +extern int is_fadump_boot_memory_area(u64 addr, ulong size);
>  extern int early_init_dt_scan_fw_dump(unsigned long node,
>  		const char *uname, int depth, void *data);
>  extern int fadump_reserve_mem(void);
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 03563c6..ea7dfdc 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -114,6 +114,18 @@ int __init early_init_dt_scan_fw_dump(unsigned long
> node,
>  	return 1;
>  }
>  
> +/*
> + * If fadump is registered, check if the memory provided
> + * falls within boot memory area.
> + */
> +int is_fadump_boot_memory_area(u64 addr, ulong size)
> +{
> +	if (!fw_dump.dump_registered)
> +		return 0;
> +
> +	return (addr + size) > RMA_START && addr <= fw_dump.boot_memory_size;
> +}
> +
>  int is_fadump_active(void)
>  {
>  	return fw_dump.dump_active;
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c
> b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index e104c71..a186b8e 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -22,6 +22,7 @@
>  #include <asm/machdep.h>
>  #include <asm/prom.h>
>  #include <asm/sparsemem.h>
> +#include <asm/fadump.h>
>  #include "pseries.h"
>  
>  static bool rtas_hp_event;
> @@ -406,6 +407,12 @@ static bool lmb_is_removable(struct of_drconf_cell *lmb)
>  	scns_per_block = block_sz / MIN_MEMORY_BLOCK_SIZE;
>  	phys_addr = lmb->base_addr;
>  
> +#ifdef CONFIG_FA_DUMP
> +	/* Don't hot-remove memory that falls in fadump boot memory area */
> +	if (is_fadump_boot_memory_area(phys_addr, block_sz))
> +		return false;
> +#endif
> +
>  	for (i = 0; i < scns_per_block; i++) {
>  		pfn = PFN_DOWN(phys_addr);
>  		if (!pfn_present(pfn))
> 
> 

Re: [PATCH 2/2] powerpc/fadump: avoid holes in boot memory area when fadump is registered

[Hari Bathini]

On Friday 05 May 2017 05:13 PM, Pingfan Liu wrote:
>
>
>
> ----- Original Message -----
>> From: "Hari Bathini" <hbathini@linux.vnet.ibm.com>
>> To: "Michael Ellerman" <mpe@ellerman.id.au>
>> Cc: "linuxppc-dev" <linuxppc-dev@ozlabs.org>, "Pingfan Liu" <piliu@redhat.com>, "Mahesh J Salgaonkar"
>> <mahesh@linux.vnet.ibm.com>
>> Sent: Friday, May 5, 2017 1:54:05 AM
>> Subject: [PATCH 2/2] powerpc/fadump: avoid holes in boot memory area when fadump is registered
>>
>> To register fadump, boot memory area - the size of low memory chunk that
>> is required for a kernel to boot successfully when booted with restricted
>> memory, is assumed to have no holes. But this memory area is currently
> The continuous is required by fadump code, not by the firmware itself, right?
>

Hmmm... Firmware takes a memory region,what we are calling boot memory
area, as input to be backed up at the time of crash. The same memory
region is used by kernel to boot after the crash. Firmware doesn't allow
holes in this memory region at the time of registering...

Thanks
Hari

Re: [PATCH 1/2] powerpc/fadump: avoid duplicates in crash memory ranges

[Mahesh Jagannath Salgaonkar]

On 05/04/2017 11:23 PM, Hari Bathini wrote:
> fadump sets up crash memory ranges to be used for creating PT_LOAD
> program headers in elfcore header. Memory chunk RMA_START through
> boot memory area size is added as the first memory range because
> firmware, at the time of crash, moves this memory chunk to different
> location specified during fadump registration making it necessary to
> create a separate program header for it with the correct offset.
> This memory chunk is skipped while setting up the remaining memory
> ranges. But currently, there is possibility that some of this memory
> may have duplicate entries like when it is hot-removed and added
> again. Ensure that no two memory ranges represent the same memory.
> 
> When 5 lmbs are hot-removed and then hot-plugged before registering
> fadump, here is how the program headers in /proc/vmcore exported by
> fadump look like

We should also make sure that fadump registration fails with proper
error if user don't put back those lmbs creating holes below the boot
memory size.

But for this patch

Reviewed-by: Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com>

Thanks,
-Mahesh.

> 
> without this change:
> 
>   Program Headers:
>     Type           Offset             VirtAddr           PhysAddr
>                    FileSiz            MemSiz              Flags  Align
>     NOTE           0x0000000000010000 0x0000000000000000 0x0000000000000000
>                    0x0000000000001894 0x0000000000001894         0
>     LOAD           0x0000000000021020 0xc000000000000000 0x0000000000000000
>                    0x0000000040000000 0x0000000040000000  RWE    0
>     LOAD           0x0000000040031020 0xc000000000000000 0x0000000000000000
>                    0x0000000010000000 0x0000000010000000  RWE    0
>     LOAD           0x0000000050040000 0xc000000010000000 0x0000000010000000
>                    0x0000000050000000 0x0000000050000000  RWE    0
>     LOAD           0x00000000a0040000 0xc000000060000000 0x0000000060000000
>                    0x000000019ffe0000 0x000000019ffe0000  RWE    0
> 
> and with this change:
> 
>   Program Headers:
>     Type           Offset             VirtAddr           PhysAddr
>                    FileSiz            MemSiz              Flags  Align
>     NOTE           0x0000000000010000 0x0000000000000000 0x0000000000000000
>                    0x0000000000001894 0x0000000000001894         0
>     LOAD           0x0000000000021020 0xc000000000000000 0x0000000000000000
>                    0x0000000040000000 0x0000000040000000  RWE    0
>     LOAD           0x0000000040030000 0xc000000040000000 0x0000000040000000
>                    0x0000000020000000 0x0000000020000000  RWE    0
>     LOAD           0x0000000060030000 0xc000000060000000 0x0000000060000000
>                    0x000000019ffe0000 0x000000019ffe0000  RWE    0
> 
> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
> ---
>  arch/powerpc/kernel/fadump.c |   13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 8ff0dd4..03563c6 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -844,8 +844,17 @@ static void fadump_setup_crash_memory_ranges(void)
>  	for_each_memblock(memory, reg) {
>  		start = (unsigned long long)reg->base;
>  		end = start + (unsigned long long)reg->size;
> -		if (start == RMA_START && end >= fw_dump.boot_memory_size)
> -			start = fw_dump.boot_memory_size;
> +
> +		/*
> +		 * skip the first memory chunk (RMA_START through
> +		 * boot_memory_size) that is already added.
> +		 */
> +		if (start < fw_dump.boot_memory_size && start >= RMA_START) {
> +			if (end > fw_dump.boot_memory_size)
> +				start = fw_dump.boot_memory_size;
> +			else
> +				continue;
> +		}
> 
>  		/* add this range excluding the reserved dump area. */
>  		fadump_exclude_reserved_area(start, end);
>