[GIT PULL 0/5] EFI fixes for v4.20

[GIT PULL 0/5] EFI fixes for v4.20

[Ard Biesheuvel]

The following changes since commit ccda4af0f4b92f7b4c308d3acc262f4a7e3affad:

  Linux 4.20-rc2 (2018-11-11 17:12:31 -0600)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git efi-urgent

for you to fetch changes up to 844d2592ed2f1903f8e38d8d395fd0d38ecdd536:

  efi: permit calling efi_mem_reserve_persistent from atomic context (2018-11-14 05:49:28 -0800)

----------------------------------------------------------------
EFI fixes for v4.20:
- fix incorrect 'on stack' annotation of efi_rts_work (all EFI arches)
- revert deferred memory map unmapping on ARM
- pack the FDT before handing over to the OS (ARM, arm64)
- fixes for new persistent memreserve feature (ARM, arm64)

----------------------------------------------------------------
Ard Biesheuvel (4):
      efi: arm: revert deferred unmap of early memmap mapping
      efi/arm: libstub: pack FDT after populating it
      efi/arm: defer persistent reservations until after paging_init()
      efi: permit calling efi_mem_reserve_persistent from atomic context

Waiman Long (1):
      efi: Fix debugobjects warning on efi_rts_work

 arch/arm64/kernel/setup.c               |  1 +
 drivers/firmware/efi/arm-init.c         |  4 ++++
 drivers/firmware/efi/arm-runtime.c      |  2 +-
 drivers/firmware/efi/efi.c              | 35 ++++++++++++++++++++++-----------
 drivers/firmware/efi/libstub/arm-stub.c |  3 +++
 drivers/firmware/efi/libstub/fdt.c      |  4 ++++
 drivers/firmware/efi/memmap.c           |  3 +++
 drivers/firmware/efi/runtime-wrappers.c |  2 +-
 include/linux/efi.h                     |  7 +++++++
 9 files changed, 47 insertions(+), 14 deletions(-)

[PATCH 1/5] efi: Fix debugobjects warning on efi_rts_work

[Ard Biesheuvel]

From: Waiman Long <longman@redhat.com>

The commit 9dbbedaa6171 ("efi: Make efi_rts_work accessible to efi
page fault handler") converted efi_rts_work from an auto variable to a
global variable. However, when submitting the work, INIT_WORK_ONSTACK()
was still used causing the following complaint from debugobjects:

ODEBUG: object 00000000ed27b500 is NOT on stack 00000000c7d38760,
but annotated.

Change the macro to just INIT_WORK() to eliminate the warning.

Fixes: 9dbbedaa6171 ("efi: Make efi_rts_work accessible to efi page fault handler")
Signed-off-by: Waiman Long <longman@redhat.com>
Acked-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/runtime-wrappers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index a19d845bdb06..8903b9ccfc2b 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -67,7 +67,7 @@ struct efi_runtime_work efi_rts_work;
 	}								\
 									\
 	init_completion(&efi_rts_work.efi_rts_comp);			\
-	INIT_WORK_ONSTACK(&efi_rts_work.work, efi_call_rts);		\
+	INIT_WORK(&efi_rts_work.work, efi_call_rts);			\
 	efi_rts_work.arg1 = _arg1;					\
 	efi_rts_work.arg2 = _arg2;					\
 	efi_rts_work.arg3 = _arg3;					\
-- 
2.17.1

[PATCH 2/5] efi: arm: revert deferred unmap of early memmap mapping

[Ard Biesheuvel]

Commit 3ea86495aef2 ("efi/arm: preserve early mapping of UEFI memory
map longer for BGRT") deferred the unmap of the early mapping of the
UEFI memory map to accommodate the ACPI BGRT code, which looks up the
memory type that backs the BGRT table to validate it against the
requirements of the UEFI spec.

Unfortunately, this causes problems on ARM, which does not permit
early mappings to persist after paging_init() is called, resulting
in a WARN() splat. Since we don't support the BGRT table on ARM anway,
let's revert ARM to the old behaviour, which is to take down the
early mapping at the end of efi_init().

Fixes: 3ea86495aef2 ("efi/arm: preserve early mapping of UEFI memory ...")
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/arm-init.c    | 4 ++++
 drivers/firmware/efi/arm-runtime.c | 2 +-
 drivers/firmware/efi/memmap.c      | 3 +++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index 388a929baf95..1a6a77df8a5e 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -265,6 +265,10 @@ void __init efi_init(void)
 				    (params.mmap & ~PAGE_MASK)));
 
 	init_screen_info();
+
+	/* ARM does not permit early mappings to persist across paging_init() */
+	if (IS_ENABLED(CONFIG_ARM))
+		efi_memmap_unmap();
 }
 
 static int __init register_gop_device(void)
diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 922cfb813109..a00934d263c5 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -110,7 +110,7 @@ static int __init arm_enable_runtime_services(void)
 {
 	u64 mapsize;
 
-	if (!efi_enabled(EFI_BOOT) || !efi_enabled(EFI_MEMMAP)) {
+	if (!efi_enabled(EFI_BOOT)) {
 		pr_info("EFI services will not be available.\n");
 		return 0;
 	}
diff --git a/drivers/firmware/efi/memmap.c b/drivers/firmware/efi/memmap.c
index fa2904fb841f..38b686c67b17 100644
--- a/drivers/firmware/efi/memmap.c
+++ b/drivers/firmware/efi/memmap.c
@@ -118,6 +118,9 @@ int __init efi_memmap_init_early(struct efi_memory_map_data *data)
 
 void __init efi_memmap_unmap(void)
 {
+	if (!efi_enabled(EFI_MEMMAP))
+		return;
+
 	if (!efi.memmap.late) {
 		unsigned long size;
 
-- 
2.17.1

[PATCH 3/5] efi/arm: libstub: pack FDT after populating it

[Ard Biesheuvel]

Commit 24d7c494ce46 ("efi/arm-stub: Round up FDT allocation to mapping
size") increased the allocation size for the FDT image created by the
stub to a fixed value of 2 MB, to simplify the former code that made
several attempts with increasing values for the size. This is reasonable
given that the allocation is of type EFI_LOADER_DATA, which is released
to the kernel unless it is explicitly memblock_reserve()d by the early
boot code.

However, this allocation size leaked into the 'size' field of the FDT
header metadata, and so the entire allocation remains occupied by the
device tree binary, even if most of it is not used to store device tree
information.

So call fdt_pack() to shrink the FDT data structure to its minimum size
after populating all the fields, so that the remaining memory is no
longer wasted.

Cc: <stable@vger.kernel.org> # v4.12+
Fixes: 24d7c494ce46 ("efi/arm-stub: Round up FDT allocation to mapping size")
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/libstub/fdt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index 8830fa601e45..0c0d2312f4a8 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -158,6 +158,10 @@ static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
 			return efi_status;
 		}
 	}
+
+	/* shrink the FDT back to its minimum size */
+	fdt_pack(fdt);
+
 	return EFI_SUCCESS;
 
 fdt_set_fail:
-- 
2.17.1

[PATCH 4/5] 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 routine
and call it after paging_init() on arm64. On ARM, because of limited
reviewing bandwidth of the maintainer, we cannot currently fix this,
so instead, disable the EFI persistent memreserve entirely on ARM so
we can fix it later.

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

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/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index 30ac0c975f8a..3d36142cf812 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -75,6 +75,9 @@ void install_memreserve_table(efi_system_table_t *sys_table_arg)
 	efi_guid_t memreserve_table_guid = LINUX_EFI_MEMRESERVE_TABLE_GUID;
 	efi_status_t status;
 
+	if (IS_ENABLED(CONFIG_ARM))
+		return;
+
 	status = efi_call_early(allocate_pool, EFI_LOADER_DATA, sizeof(*rsv),
 				(void **)&rsv);
 	if (status != EFI_SUCCESS) {
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.17.1

[PATCH 5/5] efi: permit calling efi_mem_reserve_persistent from atomic context

[Ard Biesheuvel]

Currently, efi_mem_reserve_persistent() may not be called from atomic
context, since both the kmalloc() call and the memremap() call may
sleep.

The kmalloc() call is easy enough to fix, but the memremap() call
needs to be moved into an init hook since we cannot control the
memory allocation behavior of memremap() at the call site.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/efi.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 72a4da76d274..fad7c62cfc0e 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -967,36 +967,43 @@ bool efi_is_table_address(unsigned long phys_addr)
 }
 
 static DEFINE_SPINLOCK(efi_mem_reserve_persistent_lock);
+static struct linux_efi_memreserve *efi_memreserve_root __ro_after_init;
 
 int efi_mem_reserve_persistent(phys_addr_t addr, u64 size)
 {
-	struct linux_efi_memreserve *rsv, *parent;
+	struct linux_efi_memreserve *rsv;
 
-	if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR)
+	if (!efi_memreserve_root)
 		return -ENODEV;
 
-	rsv = kmalloc(sizeof(*rsv), GFP_KERNEL);
+	rsv = kmalloc(sizeof(*rsv), GFP_ATOMIC);
 	if (!rsv)
 		return -ENOMEM;
 
-	parent = memremap(efi.mem_reserve, sizeof(*rsv), MEMREMAP_WB);
-	if (!parent) {
-		kfree(rsv);
-		return -ENOMEM;
-	}
-
 	rsv->base = addr;
 	rsv->size = size;
 
 	spin_lock(&efi_mem_reserve_persistent_lock);
-	rsv->next = parent->next;
-	parent->next = __pa(rsv);
+	rsv->next = efi_memreserve_root->next;
+	efi_memreserve_root->next = __pa(rsv);
 	spin_unlock(&efi_mem_reserve_persistent_lock);
 
-	memunmap(parent);
+	return 0;
+}
 
+static int __init efi_memreserve_root_init(void)
+{
+	if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR)
+		return -ENODEV;
+
+	efi_memreserve_root = memremap(efi.mem_reserve,
+				       sizeof(*efi_memreserve_root),
+				       MEMREMAP_WB);
+	if (!efi_memreserve_root)
+		return -ENOMEM;
 	return 0;
 }
+early_initcall(efi_memreserve_root_init);
 
 #ifdef CONFIG_KEXEC
 static int update_efi_random_seed(struct notifier_block *nb,
-- 
2.17.1

Re: [PATCH 5/5] efi: permit calling efi_mem_reserve_persistent from atomic context

[Jeremy Linton]

Hi Ard,


On 11/14/2018 11:55 AM, Ard Biesheuvel wrote:
> Currently, efi_mem_reserve_persistent() may not be called from atomic
> context, since both the kmalloc() call and the memremap() call may
> sleep.
> 
> The kmalloc() call is easy enough to fix, but the memremap() call
> needs to be moved into an init hook since we cannot control the
> memory allocation behavior of memremap() at the call site.

So, at first glance this looks correct until I noticed that 
its_cpu_init_lpis() is being called before the early_initcalls are run.

This results in the WARN_ON triggering with the following backtrace:

[    0.000000]  its_cpu_init_lpis+0x1d4/0x2e0
[    0.000000]  its_cpu_init+0x78/0x1b4
[    0.000000]  gic_init_bases+0x2c4/0x2e0
[    0.000000]  gic_acpi_init+0x158/0x270
[    0.000000]  acpi_match_madt+0x4c/0x84
[    0.000000]  acpi_table_parse_entries_array+0x140/0x218
[    0.000000]  acpi_table_parse_entries+0x70/0x98
[    0.000000]  acpi_table_parse_madt+0x40/0x50
[    0.000000]  __acpi_probe_device_table+0x88/0xe0
[    0.000000]  irqchip_init+0x38/0x40
[    0.000000]  init_IRQ+0xfc/0x130
[    0.000000]  start_kernel+0x344/0x4cc

due to the efi_memreserve_root not yet being set.



> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>   drivers/firmware/efi/efi.c | 31 +++++++++++++++++++------------
>   1 file changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 72a4da76d274..fad7c62cfc0e 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -967,36 +967,43 @@ bool efi_is_table_address(unsigned long phys_addr)
>   }
>   
>   static DEFINE_SPINLOCK(efi_mem_reserve_persistent_lock);
> +static struct linux_efi_memreserve *efi_memreserve_root __ro_after_init;
>   
>   int efi_mem_reserve_persistent(phys_addr_t addr, u64 size)
>   {
> -	struct linux_efi_memreserve *rsv, *parent;
> +	struct linux_efi_memreserve *rsv;
>   
> -	if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR)
> +	if (!efi_memreserve_root)
>   		return -ENODEV;
>   
> -	rsv = kmalloc(sizeof(*rsv), GFP_KERNEL);
> +	rsv = kmalloc(sizeof(*rsv), GFP_ATOMIC);
>   	if (!rsv)
>   		return -ENOMEM;
>   
> -	parent = memremap(efi.mem_reserve, sizeof(*rsv), MEMREMAP_WB);
> -	if (!parent) {
> -		kfree(rsv);
> -		return -ENOMEM;
> -	}
> -
>   	rsv->base = addr;
>   	rsv->size = size;
>   
>   	spin_lock(&efi_mem_reserve_persistent_lock);
> -	rsv->next = parent->next;
> -	parent->next = __pa(rsv);
> +	rsv->next = efi_memreserve_root->next;
> +	efi_memreserve_root->next = __pa(rsv);
>   	spin_unlock(&efi_mem_reserve_persistent_lock);
>   
> -	memunmap(parent);
> +	return 0;
> +}
>   
> +static int __init efi_memreserve_root_init(void)
> +{
> +	if (efi.mem_reserve == EFI_INVALID_TABLE_ADDR)
> +		return -ENODEV;
> +
> +	efi_memreserve_root = memremap(efi.mem_reserve,
> +				       sizeof(*efi_memreserve_root),
> +				       MEMREMAP_WB);
> +	if (!efi_memreserve_root)
> +		return -ENOMEM;
>   	return 0;
>   }
> +early_initcall(efi_memreserve_root_init);
>   
>   #ifdef CONFIG_KEXEC
>   static int update_efi_random_seed(struct notifier_block *nb,
> 

Re: [PATCH 5/5] efi: permit calling efi_mem_reserve_persistent from atomic context

[Ard Biesheuvel]

On Tue, 27 Nov 2018 at 19:59, Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> Hi Ard,
>
>
> On 11/14/2018 11:55 AM, Ard Biesheuvel wrote:
> > Currently, efi_mem_reserve_persistent() may not be called from atomic
> > context, since both the kmalloc() call and the memremap() call may
> > sleep.
> >
> > The kmalloc() call is easy enough to fix, but the memremap() call
> > needs to be moved into an init hook since we cannot control the
> > memory allocation behavior of memremap() at the call site.
>
> So, at first glance this looks correct until I noticed that
> its_cpu_init_lpis() is being called before the early_initcalls are run.
>
> This results in the WARN_ON triggering with the following backtrace:
>
> [    0.000000]  its_cpu_init_lpis+0x1d4/0x2e0
> [    0.000000]  its_cpu_init+0x78/0x1b4
> [    0.000000]  gic_init_bases+0x2c4/0x2e0
> [    0.000000]  gic_acpi_init+0x158/0x270
> [    0.000000]  acpi_match_madt+0x4c/0x84
> [    0.000000]  acpi_table_parse_entries_array+0x140/0x218
> [    0.000000]  acpi_table_parse_entries+0x70/0x98
> [    0.000000]  acpi_table_parse_madt+0x40/0x50
> [    0.000000]  __acpi_probe_device_table+0x88/0xe0
> [    0.000000]  irqchip_init+0x38/0x40
> [    0.000000]  init_IRQ+0xfc/0x130
> [    0.000000]  start_kernel+0x344/0x4cc
>
> due to the efi_memreserve_root not yet being set.
>

Yup

https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=efi/urgent&id=976b489120cdab2b1b3a41ffa14661db43d58190

That should fix it. Please let me know if it doesn't work for you