[PATCH v2 0/6] arm/efi: fix memblock reallocation crash due to persistent reservations

[PATCH v2 0/6] arm/efi: fix memblock reallocation crash due to persistent reservations

[Ard Biesheuvel]

This series addresses the kexec/kdump crash on arm64 system with many CPUs
that was reported by Bhupesh.

Patch #1 fixes the actual crash, but may result in memblock_reserve() to
fail. This is fixed in patch #4, where the point that the persistent
reservations are applied is moved to after memblock_allow_resize() has
been called.

Patches #2 and #3 contain some minor preparatory changes that are
required on ARM to ensure that efi_apply_persistent_mem_reservations()
can be called at some point (i.e., when memblock resizing is already
permitted and early memremap() is still usable)

Patches #5 and #6 optimize the EFI persistent memreserve infrastructure
so that fewer memblock reservations are required.

Changes since v1:
- Russell pointed out that switching to ordinary memremap() was not
  possible this early, and so I refactored the ARM early boot code
  slightly so that we can keep using early_memremap().

Ard Biesheuvel (6):
  arm64: memblock: don't permit memblock resizing until linear mapping
    is up
  ARM: mm: permit memblock resizing right after mapping the linear
    region
  ARM: mm: permit early_memremap() to be used in paging_init()
  efi/arm: defer persistent reservations until after paging_init()
  efi: permit multiple entries in persistent memreserve data structure
  efi: reduce the amount of memblock reservations for persistent
    allocations

 arch/arm/kernel/setup.c                 |  2 -
 arch/arm/mm/init.c                      |  1 -
 arch/arm/mm/mmu.c                       |  5 ++
 arch/arm64/kernel/setup.c               |  1 +
 arch/arm64/mm/init.c                    |  2 -
 arch/arm64/mm/mmu.c                     |  2 +
 drivers/firmware/efi/efi.c              | 59 ++++++++++++++------
 drivers/firmware/efi/libstub/arm-stub.c |  2 +-
 include/linux/efi.h                     | 23 +++++++-
 9 files changed, 72 insertions(+), 25 deletions(-)

-- 
2.19.1

[PATCH v2 1/6] arm64: memblock: don't permit memblock resizing until linear mapping is up

[Ard Biesheuvel]

Bhupesh reports that having numerous memblock reservations at early
boot may result in the following crash:

  Unable to handle kernel paging request at virtual address ffff80003ffe0000
  ...
  Call trace:
   __memcpy+0x110/0x180
   memblock_add_range+0x134/0x2e8
   memblock_reserve+0x70/0xb8
   memblock_alloc_base_nid+0x6c/0x88
   __memblock_alloc_base+0x3c/0x4c
   memblock_alloc_base+0x28/0x4c
   memblock_alloc+0x2c/0x38
   early_pgtable_alloc+0x20/0xb0
   paging_init+0x28/0x7f8

This is caused by the fact that we permit memblock resizing before the
linear mapping is up, and so the memblock_reserved() array is moved
into memory that is not mapped yet.

So let's ensure that this crash can no longer occur, by deferring to
call to memblock_allow_resize() to after the linear mapping has been
created.

Reported-by: Bhupesh Sharma <bhsharma@redhat.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Tested-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/mm/init.c | 2 --
 arch/arm64/mm/mmu.c  | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 9d9582cac6c4..9b432d9fcada 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -483,8 +483,6 @@ void __init arm64_memblock_init(void)
 	high_memory = __va(memblock_end_of_DRAM() - 1) + 1;
 
 	dma_contiguous_reserve(arm64_dma_phys_limit);
-
-	memblock_allow_resize();
 }
 
 void __init bootmem_init(void)
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index d6d05c8c5c52..e1b2d58a311a 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -671,6 +671,8 @@ void __init paging_init(void)
 
 	memblock_free(__pa_symbol(init_pg_dir),
 		      __pa_symbol(init_pg_end) - __pa_symbol(init_pg_dir));
+
+	memblock_allow_resize();
 }
 
 /*
-- 
2.19.1

[PATCH v2 2/6] ARM: mm: permit memblock resizing right after mapping the linear region

[Ard Biesheuvel]

The memblock arrays can be resized dynamically if needed, but this is
only done after memblock_allow_resize() is called, since it is up to
the architecture to decide at which point doing so is possible (i.e.,
when all the memory that memblock describes is actually mapped)

ARM grants this permission in bootmem_init(), but in order for the EFI
persistent memory reservation code (which may create memblock
reservations that trigger such a dynamic resize) to be able to be called
before shutting down early fixmap (upon which the EFI code depends due
to its use of early_memremap()), we need to do this earlier.

So let's move the call to memblock_allow_resize() to right after the
point where low memory is mapped and declared as the memory limit for
memblock allocation.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/mm/init.c | 1 -
 arch/arm/mm/mmu.c  | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 32e4845af2b6..797fad2b16ee 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -304,7 +304,6 @@ void __init bootmem_init(void)
 {
 	unsigned long min, max_low, max_high;
 
-	memblock_allow_resize();
 	max_low = max_high = 0;
 
 	find_limits(&min, &max_low, &max_high);
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index f5cc1ccfea3d..f6bf6686559d 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1626,6 +1626,7 @@ void __init paging_init(const struct machine_desc *mdesc)
 	prepare_page_table();
 	map_lowmem();
 	memblock_set_current_limit(arm_lowmem_limit);
+	memblock_allow_resize();
 	dma_contiguous_remap();
 	early_fixmap_shutdown();
 	devicemaps_init(mdesc);
-- 
2.19.1

[PATCH v2 3/6] ARM: mm: permit early_memremap() to be used in paging_init()

[Ard Biesheuvel]

early_memremap() and early_ioremap() rely on early fixmap support,
which shares its virtual address space with kmap(), and so it is
taken down in paging_init().

In order to permit the EFI persistent memory reservation code to
use early_memremap() when called from paging_init(), move the call
to early_ioremap_reset() into paging_init(), right before the call
to early_fixmap_shutdown(), creating a window where we can add the
call to efi_apply_persistent_mem_reservations() in a subsequent patch.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/kernel/setup.c | 2 --
 arch/arm/mm/mmu.c       | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index ac7e08886863..2f85cce38333 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -1114,8 +1114,6 @@ void __init setup_arch(char **cmdline_p)
 	/* Memory may have been removed so recalculate the bounds. */
 	adjust_lowmem_bounds();
 
-	early_ioremap_reset();
-
 	paging_init(mdesc);
 	request_standard_resources(mdesc);
 
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index f6bf6686559d..078f82f89fe5 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -22,6 +22,7 @@
 #include <asm/cputype.h>
 #include <asm/sections.h>
 #include <asm/cachetype.h>
+#include <asm/early_ioremap.h>
 #include <asm/fixmap.h>
 #include <asm/sections.h>
 #include <asm/setup.h>
@@ -1628,6 +1629,7 @@ void __init paging_init(const struct machine_desc *mdesc)
 	memblock_set_current_limit(arm_lowmem_limit);
 	memblock_allow_resize();
 	dma_contiguous_remap();
+	early_ioremap_reset();
 	early_fixmap_shutdown();
 	devicemaps_init(mdesc);
 	kmap_init();
-- 
2.19.1

[PATCH v2 4/6] efi/arm: defer persistent reservations until after paging_init()

[Ard Biesheuvel]

The new memory EFI reservation feature we introduced to allow memory
reservations to persist across kexec may trigger an unbounded number
of calls to memblock_reserve(). The memblock subsystem can deal with
this fine, but not before memblock resizing is enabled, which we can
only do after paging_init(), when the memory we reallocate the array
into is actually mapped.

So break out the memreserve table processing into a separate function
and call if after paging_init() on arm64, and from paging_init() on
ARM, after memblock resizing has been enabled but before the early
memremap support that we rely on has been taken down.

Tested-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/mm/mmu.c          | 2 ++
 arch/arm64/kernel/setup.c  | 1 +
 drivers/firmware/efi/efi.c | 4 ++++
 include/linux/efi.h        | 7 +++++++
 4 files changed, 14 insertions(+)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 078f82f89fe5..8ecffb8c0c0b 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -9,6 +9,7 @@
  */
 #include <linux/module.h>
 #include <linux/kernel.h>
+#include <linux/efi.h>
 #include <linux/errno.h>
 #include <linux/init.h>
 #include <linux/mman.h>
@@ -1629,6 +1630,7 @@ void __init paging_init(const struct machine_desc *mdesc)
 	memblock_set_current_limit(arm_lowmem_limit);
 	memblock_allow_resize();
 	dma_contiguous_remap();
+	efi_apply_persistent_mem_reservations();
 	early_ioremap_reset();
 	early_fixmap_shutdown();
 	devicemaps_init(mdesc);
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 953e316521fc..f4fc1e0544b7 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -313,6 +313,7 @@ void __init setup_arch(char **cmdline_p)
 	arm64_memblock_init();
 
 	paging_init();
+	efi_apply_persistent_mem_reservations();
 
 	acpi_table_upgrade();
 
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 249eb70691b0..72a4da76d274 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -592,7 +592,11 @@ int __init efi_config_parse_tables(void *config_tables, int count, int sz,
 
 		early_memunmap(tbl, sizeof(*tbl));
 	}
+	return 0;
+}
 
+int __init efi_apply_persistent_mem_reservations(void)
+{
 	if (efi.mem_reserve != EFI_INVALID_TABLE_ADDR) {
 		unsigned long prsv = efi.mem_reserve;
 
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 845174e113ce..100ce4a4aff6 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1167,6 +1167,8 @@ static inline bool efi_enabled(int feature)
 extern void efi_reboot(enum reboot_mode reboot_mode, const char *__unused);
 
 extern bool efi_is_table_address(unsigned long phys_addr);
+
+extern int efi_apply_persistent_mem_reservations(void);
 #else
 static inline bool efi_enabled(int feature)
 {
@@ -1185,6 +1187,11 @@ static inline bool efi_is_table_address(unsigned long phys_addr)
 {
 	return false;
 }
+
+static inline int efi_apply_persistent_mem_reservations(void)
+{
+	return 0;
+}
 #endif
 
 extern int efi_status_to_err(efi_status_t status);
-- 
2.19.1

[PATCH v2 5/6] efi: permit multiple entries in persistent memreserve data structure

[Ard Biesheuvel]

In preparation of updating efi_mem_reserve_persistent() to cause less
fragmentation when dealing with many persistent reservations, update
the struct definition and the code that handles it currently so it
can describe an arbitrary number of reservations using a single linked
list entry. The actual optimization will be implemented in a subsequent
patch.

Tested-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/efi.c              | 32 +++++++++++++-------
 drivers/firmware/efi/libstub/arm-stub.c |  2 +-
 include/linux/efi.h                     | 13 ++++++--
 3 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 72a4da76d274..59f8ac93c759 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -602,21 +602,28 @@ int __init efi_apply_persistent_mem_reservations(void)
 
 		while (prsv) {
 			struct linux_efi_memreserve *rsv;
+			u8 *p;
+			int i;
 
-			/* reserve the entry itself */
-			memblock_reserve(prsv, sizeof(*rsv));
-
-			rsv = early_memremap(prsv, sizeof(*rsv));
-			if (rsv == NULL) {
+			p = early_memremap(ALIGN_DOWN(prsv, PAGE_SIZE),
+					   PAGE_SIZE);
+			if (p == NULL) {
 				pr_err("Could not map UEFI memreserve entry!\n");
 				return -ENOMEM;
 			}
 
-			if (rsv->size)
-				memblock_reserve(rsv->base, rsv->size);
+			rsv = (void *)(p + prsv % PAGE_SIZE);
+
+			/* reserve the entry itself */
+			memblock_reserve(prsv, EFI_MEMRESERVE_SIZE(rsv->size));
+
+			for (i = 0; i < atomic_read(&rsv->count); i++) {
+				memblock_reserve(rsv->entry[i].base,
+						 rsv->entry[i].size);
+			}
 
 			prsv = rsv->next;
-			early_memunmap(rsv, sizeof(*rsv));
+			early_memunmap(p, PAGE_SIZE);
 		}
 	}
 
@@ -971,11 +978,12 @@ static DEFINE_SPINLOCK(efi_mem_reserve_persistent_lock);
 int efi_mem_reserve_persistent(phys_addr_t addr, u64 size)
 {
 	struct linux_efi_memreserve *rsv, *parent;
+	int rsvsize = EFI_MEMRESERVE_SIZE(1);
 
 	if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR)
 		return -ENODEV;
 
-	rsv = kmalloc(sizeof(*rsv), GFP_KERNEL);
+	rsv = kmalloc(rsvsize, GFP_KERNEL);
 	if (!rsv)
 		return -ENOMEM;
 
@@ -985,8 +993,10 @@ int efi_mem_reserve_persistent(phys_addr_t addr, u64 size)
 		return -ENOMEM;
 	}
 
-	rsv->base = addr;
-	rsv->size = size;
+	rsv->size = 1;
+	atomic_set(&rsv->count, 1);
+	rsv->entry[0].base = addr;
+	rsv->entry[0].size = size;
 
 	spin_lock(&efi_mem_reserve_persistent_lock);
 	rsv->next = parent->next;
diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index 30ac0c975f8a..5bcfa08e8bb1 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -83,8 +83,8 @@ void install_memreserve_table(efi_system_table_t *sys_table_arg)
 	}
 
 	rsv->next = 0;
-	rsv->base = 0;
 	rsv->size = 0;
+	atomic_set(&rsv->count, 0);
 
 	status = efi_call_early(install_configuration_table,
 				&memreserve_table_guid,
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 100ce4a4aff6..dfce82b2ca8a 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1715,9 +1715,16 @@ extern struct efi_runtime_work efi_rts_work;
 extern struct workqueue_struct *efi_rts_wq;
 
 struct linux_efi_memreserve {
-	phys_addr_t	next;
-	phys_addr_t	base;
-	phys_addr_t	size;
+	int		size;			// allocated size of the array
+	atomic_t	count;			// number of entries used
+	phys_addr_t	next;			// pa of next struct instance
+	struct {
+		phys_addr_t	base;
+		phys_addr_t	size;
+	} entry[0];
 };
 
+#define EFI_MEMRESERVE_SIZE(count) (sizeof(struct linux_efi_memreserve) + \
+	(count) * sizeof(((struct linux_efi_memreserve *)0)->entry[0]))
+
 #endif /* _LINUX_EFI_H */
-- 
2.19.1

[PATCH v2 6/6] efi: reduce the amount of memblock reservations for persistent allocations

[Ard Biesheuvel]

The current implementation of efi_mem_reserve_persistent() is rather
naive, in the sense that for each invocation, it creates a separate
linked list entry to describe the reservation. Since the linked list
entries themselves need to persist across subsequent kexec reboots,
every reservation created this way results in two memblock_reserve()
calls at the next boot.

On arm64 systems with 100s of CPUs, this may result in a excessive
number of memblock reservations, and needless fragmentation.

So instead, make use of the newly updated struct linux_efi_memreserve
layout to put multiple reservations into a single linked list entry.
This should get rid of the numerous tiny memblock reservations, and
effectively cut the total number of reservations in half on arm64
systems with many CPUs.

Tested-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/efi.c | 29 ++++++++++++++------
 include/linux/efi.h        |  3 ++
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 59f8ac93c759..347028cb392f 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -978,22 +978,35 @@ static DEFINE_SPINLOCK(efi_mem_reserve_persistent_lock);
 int efi_mem_reserve_persistent(phys_addr_t addr, u64 size)
 {
 	struct linux_efi_memreserve *rsv, *parent;
-	int rsvsize = EFI_MEMRESERVE_SIZE(1);
+	unsigned long prsv;
+	int index;
 
 	if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR)
 		return -ENODEV;
 
-	rsv = kmalloc(rsvsize, GFP_KERNEL);
-	if (!rsv)
-		return -ENOMEM;
-
 	parent = memremap(efi.mem_reserve, sizeof(*rsv), MEMREMAP_WB);
-	if (!parent) {
-		kfree(rsv);
+	if (!parent)
 		return -ENOMEM;
+
+	/* first try to find a slot in an existing linked list entry */
+	for (prsv = parent->next; prsv; prsv = rsv->next) {
+		rsv = __va(prsv);
+		index = atomic_fetch_add_unless(&rsv->count, 1, rsv->size);
+		if (index < rsv->size) {
+			rsv->entry[index].base = addr;
+			rsv->entry[index].size = size;
+
+			memunmap(parent);
+			return 0;
+		}
 	}
 
-	rsv->size = 1;
+	/* no slot found - allocate a new linked list entry */
+	rsv = (struct linux_efi_memreserve *)__get_free_page(GFP_KERNEL);
+	if (!rsv)
+		return -ENOMEM;
+
+	rsv->size = EFI_MEMRESERVE_COUNT(PAGE_SIZE);
 	atomic_set(&rsv->count, 1);
 	rsv->entry[0].base = addr;
 	rsv->entry[0].size = size;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index dfce82b2ca8a..1a1a081f7244 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1727,4 +1727,7 @@ struct linux_efi_memreserve {
 #define EFI_MEMRESERVE_SIZE(count) (sizeof(struct linux_efi_memreserve) + \
 	(count) * sizeof(((struct linux_efi_memreserve *)0)->entry[0]))
 
+#define EFI_MEMRESERVE_COUNT(size) (((size) - sizeof(struct linux_efi_memreserve)) \
+	/ sizeof(((struct linux_efi_memreserve *)0)->entry[0]))
+
 #endif /* _LINUX_EFI_H */
-- 
2.19.1

Re: [PATCH v2 1/6] arm64: memblock: don't permit memblock resizing until linear mapping is up

[Catalin Marinas]

On Wed, Nov 07, 2018 at 03:16:06PM +0100, Ard Biesheuvel wrote:
> Bhupesh reports that having numerous memblock reservations at early
> boot may result in the following crash:
> 
>   Unable to handle kernel paging request at virtual address ffff80003ffe0000
>   ...
>   Call trace:
>    __memcpy+0x110/0x180
>    memblock_add_range+0x134/0x2e8
>    memblock_reserve+0x70/0xb8
>    memblock_alloc_base_nid+0x6c/0x88
>    __memblock_alloc_base+0x3c/0x4c
>    memblock_alloc_base+0x28/0x4c
>    memblock_alloc+0x2c/0x38
>    early_pgtable_alloc+0x20/0xb0
>    paging_init+0x28/0x7f8
> 
> This is caused by the fact that we permit memblock resizing before the
> linear mapping is up, and so the memblock_reserved() array is moved
> into memory that is not mapped yet.
> 
> So let's ensure that this crash can no longer occur, by deferring to
> call to memblock_allow_resize() to after the linear mapping has been
> created.
> 
> Reported-by: Bhupesh Sharma <bhsharma@redhat.com>
> Acked-by: Will Deacon <will.deacon@arm.com>
> Tested-by: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

I missed this patch (wasn't cc'ed) but Will pinged me on IRC, so queued
for 4.20. Thanks.

-- 
Catalin

Re: [PATCH v2 0/6] arm/efi: fix memblock reallocation crash due to persistent reservations

[Bhupesh Sharma]

Hi Ard,

On Wed, Nov 7, 2018 at 7:46 PM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> This series addresses the kexec/kdump crash on arm64 system with many CPUs
> that was reported by Bhupesh.
>
> Patch #1 fixes the actual crash, but may result in memblock_reserve() to
> fail. This is fixed in patch #4, where the point that the persistent
> reservations are applied is moved to after memblock_allow_resize() has
> been called.
>
> Patches #2 and #3 contain some minor preparatory changes that are
> required on ARM to ensure that efi_apply_persistent_mem_reservations()
> can be called at some point (i.e., when memblock resizing is already
> permitted and early memremap() is still usable)
>
> Patches #5 and #6 optimize the EFI persistent memreserve infrastructure
> so that fewer memblock reservations are required.
>
> Changes since v1:
> - Russell pointed out that switching to ordinary memremap() was not
>   possible this early, and so I refactored the ARM early boot code
>   slightly so that we can keep using early_memremap().
>
> Ard Biesheuvel (6):
>   arm64: memblock: don't permit memblock resizing until linear mapping
>     is up
>   ARM: mm: permit memblock resizing right after mapping the linear
>     region
>   ARM: mm: permit early_memremap() to be used in paging_init()
>   efi/arm: defer persistent reservations until after paging_init()
>   efi: permit multiple entries in persistent memreserve data structure
>   efi: reduce the amount of memblock reservations for persistent
>     allocations
>
>  arch/arm/kernel/setup.c                 |  2 -
>  arch/arm/mm/init.c                      |  1 -
>  arch/arm/mm/mmu.c                       |  5 ++
>  arch/arm64/kernel/setup.c               |  1 +
>  arch/arm64/mm/init.c                    |  2 -
>  arch/arm64/mm/mmu.c                     |  2 +
>  drivers/firmware/efi/efi.c              | 59 ++++++++++++++------
>  drivers/firmware/efi/libstub/arm-stub.c |  2 +-
>  include/linux/efi.h                     | 23 +++++++-
>  9 files changed, 72 insertions(+), 25 deletions(-)
>
> --
> 2.19.1
>

I did some quick checks (as I just returned from my holidays and got
hold of the hardware just now) and I haven't had the opportunity to
look closely at the entire patch set, but it looks like a step in the
right direction especially as we try to have fewer memblock
reservations (I will have a closer look at the patchset perhaps over
the weekend).

I tested this on the hardware (with 224 CPUs) where I was seeing the
crash initially and kdump seems to work fine on the same. Here are
some kdump kernel logs with 'memblock=debug' set in bootargs:

[    0.000000] memblock_reserve:
[0x00000000e2e02e18-0x00000000e2e02e4f] efi_mem_reserve+0x3c/0x54
[    0.000000] memblock_reserve:
[0x00000000dac70000-0x00000000dac7ffff] efi_init+0xc4/0x17c
[    0.000000] memblock_remove:
[0x0001000000000000-0x0000fffffffffffe] arm64_memblock_init+0x6c/0x418
[    0.000000] memblock_remove:
[0x0000800080000000-0x000080007ffffffe] arm64_memblock_init+0xac/0x418
[    0.0001dd0000-0x00000000a2cfffff] arm64_memblock_init+0x184/0x418
[    0.000000] memblock_add: [0x00000000a1dd0000-0x00000000a2cfffff]
arm64_memblock_init+0x190/0x418
[    0.000000] memblock_reserve:
[0x00000000a1dd0000-0x00000000a2cfffff]
arm64_memblock_init+0x19c/0x418
[    0.000000] memblock_reserve:
[0x00000000a0080000-0x00000000a1dcffff]
arm64_memblock_init+0x1f8/0x418
[    0.000000] memblock_reserve:
[0x00000000a1dd0000-0x00000000a2cfb402]     0.000000]
memblock_reserve: [0x00000000bfff0000-0x00000000bfff33ff]
arm64_memblock_init+0x3b4/0x418
[    0.000000] Reserving 13KB of memory at 0xbfff0000 for elfcorehdr
[    0.000000] memblock_reserve:
[0x00000000bffe0000-0x00000000bffeffff]
memblock_alloc_base_nid+0x70/0x8c
[    0.000000] memblock_reserve:
[0x00000000bffd0000-0x00000000bffdffff]
memblock_alloc_base_nid+0x70/0x8c
[    0.000000] memblock_reserve: [0x00000000bffc0se_nid+0x70/0x8c
[    0.000000] memblock_reserve:
[0x00000000bffb0000-0x00000000bffbffff]
memblock_alloc_base_nid+0x70/0x8c
[    0.000000]    memblock_free:
[0x00000000a1d80000-0x00000000a1dcffff] paging_init+0x6d4/0x6fc
[    0.000000] memblock_reserve:
[0x00000000ddab9e18-0x00000000ddab9e27]
efi_apply_persistent_mem_reservations+0x8c/0xbc

So, things look great so far. So, please feel free to add:
Tested-by: Bhupesh Sharma <bhsharma@redhat.com>

Thanks,
Bhupesh

Re: [PATCH v2 0/6] arm/efi: fix memblock reallocation crash due to persistent reservations

[Bhupesh Sharma]

And +Cc: Catalin

On Fri, Nov 9, 2018 at 12:43 AM Bhupesh Sharma <bhsharma@redhat.com> wrote:
>
> Hi Ard,
>
> On Wed, Nov 7, 2018 at 7:46 PM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> > This series addresses the kexec/kdump crash on arm64 system with many CPUs
> > that was reported by Bhupesh.
> >
> > Patch #1 fixes the actual crash, but may result in memblock_reserve() to
> > fail. This is fixed in patch #4, where the point that the persistent
> > reservations are applied is moved to after memblock_allow_resize() has
> > been called.
> >
> > Patches #2 and #3 contain some minor preparatory changes that are
> > required on ARM to ensure that efi_apply_persistent_mem_reservations()
> > can be called at some point (i.e., when memblock resizing is already
> > permitted and early memremap() is still usable)
> >
> > Patches #5 and #6 optimize the EFI persistent memreserve infrastructure
> > so that fewer memblock reservations are required.
> >
> > Changes since v1:
> > - Russell pointed out that switching to ordinary memremap() was not
> >   possible this early, and so I refactored the ARM early boot code
> >   slightly so that we can keep using early_memremap().
> >
> > Ard Biesheuvel (6):
> >   arm64: memblock: don't permit memblock resizing until linear mapping
> >     is up
> >   ARM: mm: permit memblock resizing right after mapping the linear
> >     region
> >   ARM: mm: permit early_memremap() to be used in paging_init()
> >   efi/arm: defer persistent reservations until after paging_init()
> >   efi: permit multiple entries in persistent memreserve data structure
> >   efi: reduce the amount of memblock reservations for persistent
> >     allocations
> >
> >  arch/arm/kernel/setup.c                 |  2 -
> >  arch/arm/mm/init.c                      |  1 -
> >  arch/arm/mm/mmu.c                       |  5 ++
> >  arch/arm64/kernel/setup.c               |  1 +
> >  arch/arm64/mm/init.c                    |  2 -
> >  arch/arm64/mm/mmu.c                     |  2 +
> >  drivers/firmware/efi/efi.c              | 59 ++++++++++++++------
> >  drivers/firmware/efi/libstub/arm-stub.c |  2 +-
> >  include/linux/efi.h                     | 23 +++++++-
> >  9 files changed, 72 insertions(+), 25 deletions(-)
> >
> > --
> > 2.19.1
> >
>
> I did some quick checks (as I just returned from my holidays and got
> hold of the hardware just now) and I haven't had the opportunity to
> look closely at the entire patch set, but it looks like a step in the
> right direction especially as we try to have fewer memblock
> reservations (I will have a closer look at the patchset perhaps over
> the weekend).
>
> I tested this on the hardware (with 224 CPUs) where I was seeing the
> crash initially and kdump seems to work fine on the same. Here are
> some kdump kernel logs with 'memblock=debug' set in bootargs:
>
> [    0.000000] memblock_reserve:
> [0x00000000e2e02e18-0x00000000e2e02e4f] efi_mem_reserve+0x3c/0x54
> [    0.000000] memblock_reserve:
> [0x00000000dac70000-0x00000000dac7ffff] efi_init+0xc4/0x17c
> [    0.000000] memblock_remove:
> [0x0001000000000000-0x0000fffffffffffe] arm64_memblock_init+0x6c/0x418
> [    0.000000] memblock_remove:
> [0x0000800080000000-0x000080007ffffffe] arm64_memblock_init+0xac/0x418
> [    0.0001dd0000-0x00000000a2cfffff] arm64_memblock_init+0x184/0x418
> [    0.000000] memblock_add: [0x00000000a1dd0000-0x00000000a2cfffff]
> arm64_memblock_init+0x190/0x418
> [    0.000000] memblock_reserve:
> [0x00000000a1dd0000-0x00000000a2cfffff]
> arm64_memblock_init+0x19c/0x418
> [    0.000000] memblock_reserve:
> [0x00000000a0080000-0x00000000a1dcffff]
> arm64_memblock_init+0x1f8/0x418
> [    0.000000] memblock_reserve:
> [0x00000000a1dd0000-0x00000000a2cfb402]     0.000000]
> memblock_reserve: [0x00000000bfff0000-0x00000000bfff33ff]
> arm64_memblock_init+0x3b4/0x418
> [    0.000000] Reserving 13KB of memory at 0xbfff0000 for elfcorehdr
> [    0.000000] memblock_reserve:
> [0x00000000bffe0000-0x00000000bffeffff]
> memblock_alloc_base_nid+0x70/0x8c
> [    0.000000] memblock_reserve:
> [0x00000000bffd0000-0x00000000bffdffff]
> memblock_alloc_base_nid+0x70/0x8c
> [    0.000000] memblock_reserve: [0x00000000bffc0se_nid+0x70/0x8c
> [    0.000000] memblock_reserve:
> [0x00000000bffb0000-0x00000000bffbffff]
> memblock_alloc_base_nid+0x70/0x8c
> [    0.000000]    memblock_free:
> [0x00000000a1d80000-0x00000000a1dcffff] paging_init+0x6d4/0x6fc
> [    0.000000] memblock_reserve:
> [0x00000000ddab9e18-0x00000000ddab9e27]
> efi_apply_persistent_mem_reservations+0x8c/0xbc
>
> So, things look great so far. So, please feel free to add:
> Tested-by: Bhupesh Sharma <bhsharma@redhat.com>
>
> Thanks,
> Bhupesh

Re: [PATCH v2 0/6] arm/efi: fix memblock reallocation crash due to persistent reservations

[Ard Biesheuvel]

On 8 November 2018 at 20:13, Bhupesh Sharma <bhsharma@redhat.com> wrote:
> Hi Ard,
>
> On Wed, Nov 7, 2018 at 7:46 PM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>
>> This series addresses the kexec/kdump crash on arm64 system with many CPUs
>> that was reported by Bhupesh.
>>
>> Patch #1 fixes the actual crash, but may result in memblock_reserve() to
>> fail. This is fixed in patch #4, where the point that the persistent
>> reservations are applied is moved to after memblock_allow_resize() has
>> been called.
>>
>> Patches #2 and #3 contain some minor preparatory changes that are
>> required on ARM to ensure that efi_apply_persistent_mem_reservations()
>> can be called at some point (i.e., when memblock resizing is already
>> permitted and early memremap() is still usable)
>>
>> Patches #5 and #6 optimize the EFI persistent memreserve infrastructure
>> so that fewer memblock reservations are required.
>>
>> Changes since v1:
>> - Russell pointed out that switching to ordinary memremap() was not
>>   possible this early, and so I refactored the ARM early boot code
>>   slightly so that we can keep using early_memremap().
>>
>> Ard Biesheuvel (6):
>>   arm64: memblock: don't permit memblock resizing until linear mapping
>>     is up
>>   ARM: mm: permit memblock resizing right after mapping the linear
>>     region
>>   ARM: mm: permit early_memremap() to be used in paging_init()
>>   efi/arm: defer persistent reservations until after paging_init()
>>   efi: permit multiple entries in persistent memreserve data structure
>>   efi: reduce the amount of memblock reservations for persistent
>>     allocations
>>
>>  arch/arm/kernel/setup.c                 |  2 -
>>  arch/arm/mm/init.c                      |  1 -
>>  arch/arm/mm/mmu.c                       |  5 ++
>>  arch/arm64/kernel/setup.c               |  1 +
>>  arch/arm64/mm/init.c                    |  2 -
>>  arch/arm64/mm/mmu.c                     |  2 +
>>  drivers/firmware/efi/efi.c              | 59 ++++++++++++++------
>>  drivers/firmware/efi/libstub/arm-stub.c |  2 +-
>>  include/linux/efi.h                     | 23 +++++++-
>>  9 files changed, 72 insertions(+), 25 deletions(-)
>>
>> --
>> 2.19.1
>>
>
> I did some quick checks (as I just returned from my holidays and got
> hold of the hardware just now) and I haven't had the opportunity to
> look closely at the entire patch set, but it looks like a step in the
> right direction especially as we try to have fewer memblock
> reservations (I will have a closer look at the patchset perhaps over
> the weekend).
>
> I tested this on the hardware (with 224 CPUs) where I was seeing the
> crash initially and kdump seems to work fine on the same. Here are
> some kdump kernel logs with 'memblock=debug' set in bootargs:
>
> [    0.000000] memblock_reserve:
> [0x00000000e2e02e18-0x00000000e2e02e4f] efi_mem_reserve+0x3c/0x54
> [    0.000000] memblock_reserve:
> [0x00000000dac70000-0x00000000dac7ffff] efi_init+0xc4/0x17c
> [    0.000000] memblock_remove:
> [0x0001000000000000-0x0000fffffffffffe] arm64_memblock_init+0x6c/0x418
> [    0.000000] memblock_remove:
> [0x0000800080000000-0x000080007ffffffe] arm64_memblock_init+0xac/0x418
> [    0.0001dd0000-0x00000000a2cfffff] arm64_memblock_init+0x184/0x418
> [    0.000000] memblock_add: [0x00000000a1dd0000-0x00000000a2cfffff]
> arm64_memblock_init+0x190/0x418
> [    0.000000] memblock_reserve:
> [0x00000000a1dd0000-0x00000000a2cfffff]
> arm64_memblock_init+0x19c/0x418
> [    0.000000] memblock_reserve:
> [0x00000000a0080000-0x00000000a1dcffff]
> arm64_memblock_init+0x1f8/0x418
> [    0.000000] memblock_reserve:
> [0x00000000a1dd0000-0x00000000a2cfb402]     0.000000]
> memblock_reserve: [0x00000000bfff0000-0x00000000bfff33ff]
> arm64_memblock_init+0x3b4/0x418
> [    0.000000] Reserving 13KB of memory at 0xbfff0000 for elfcorehdr
> [    0.000000] memblock_reserve:
> [0x00000000bffe0000-0x00000000bffeffff]
> memblock_alloc_base_nid+0x70/0x8c
> [    0.000000] memblock_reserve:
> [0x00000000bffd0000-0x00000000bffdffff]
> memblock_alloc_base_nid+0x70/0x8c
> [    0.000000] memblock_reserve: [0x00000000bffc0se_nid+0x70/0x8c
> [    0.000000] memblock_reserve:
> [0x00000000bffb0000-0x00000000bffbffff]
> memblock_alloc_base_nid+0x70/0x8c
> [    0.000000]    memblock_free:
> [0x00000000a1d80000-0x00000000a1dcffff] paging_init+0x6d4/0x6fc
> [    0.000000] memblock_reserve:
> [0x00000000ddab9e18-0x00000000ddab9e27]
> efi_apply_persistent_mem_reservations+0x8c/0xbc
>
> So, things look great so far. So, please feel free to add:
> Tested-by: Bhupesh Sharma <bhsharma@redhat.com>
>

Thanks!