Skip to contentSkip to conversation

More EFI updates for v5.6

GIT PULL New Ard Biesheuvel 3 participants 24 messages Jan 3, 2020 – Jan 13, 2020

Subsystems (5)
Documentation x86 x86 MM EFI +1 more subsystems
lore
Share & tools
lore

Ard Biesheuvel

author maintainer
#0 lore raw

Ingo, Thomas,

This is the second batch of EFI updates for v5.6. Two things are still under discussion, so I'll probably have a few more changes for this cycle in a week or so.

The following changes since commit 0679715e714345d273c0e1eb78078535ffc4b2a1:

efi/libstub/x86: Avoid globals to store context during mixed mode calls (2019-12-25 10:49:26 +0100)

are available in the Git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git tags/efi-next

for you to fetch changes up to d95e4feae5368a91775c4597a8f298ba84f31535:

efi/x86: avoid RWX mappings for all of DRAM (2020-01-03 11:46:15 +0100)

----------------------------------------------------------------
Second batch of EFI updates for v5.6:
- Some followup fixes for the EFI stub changes that have been queued up already.
- Overhaul of the x86 EFI boot/runtime code, to peel off layers of pointer casting and type mangling via variadic macros and asm wrappers that made the code fragile and ugly.
- Increase robustness for mixed mode code, by using argmaps to annotate and translate function prototypes that are not mixed mode safe. (Arvind)
- Add the ability to disable DMA at the root port level in the EFI stub, to avoid booting into the kernel proper with IOMMUs in pass through and DMA enabled (suggested by Matthew)
- Get rid of RWX mappings in the EFI memory map, where possible.

----------------------------------------------------------------
Ard Biesheuvel (17):
efi/libstub: fix boot argument handling in mixed mode entry code efi/libstub/x86: force 'hidden' visibility for extern declarations efi/x86: re-disable RT services for 32-bit kernels running on 64-bit EFI efi/x86: map the entire EFI vendor string before copying it efi/x86: avoid redundant cast of EFI firmware service pointer efi/x86: split off some old memmap handling into separate routines efi/x86: split SetVirtualAddresMap() wrappers into 32 and 64 bit versions efi/x86: simplify i386 efi_call_phys() firmware call wrapper efi/x86: simplify 64-bit EFI firmware call wrapper
efi/x86: simplify mixed mode call wrapper
efi/x86: drop two near identical versions of efi_runtime_init() efi/x86: clean up efi_systab_init() routine for legibility efi/x86: don't panic or BUG() on non-critical error conditions efi/x86: remove unreachable code in kexec_enter_virtual_mode() x86/mm: fix NX bit clearing issue in kernel_map_pages_in_pgd efi/x86: don't map the entire kernel text RW for mixed mode efi/x86: avoid RWX mappings for all of DRAM

Arvind Sankar (2):
efi/x86: Check number of arguments to variadic functions efi/x86: Allow translating 64-bit arguments for mixed mode calls

Matthew Garrett (1):
efi: Allow disabling PCI busmastering on bridges during boot

 Documentation/admin-guide/kernel-parameters.txt |   7 +-
 arch/x86/boot/compressed/eboot.c                |  18 +-
 arch/x86/boot/compressed/efi_thunk_64.S         |   4 +-
 arch/x86/boot/compressed/head_64.S              |  17 +-
 arch/x86/include/asm/efi.h                      | 169 ++++++++---
 arch/x86/mm/pageattr.c                          |   8 +-
 arch/x86/platform/efi/Makefile                  |   1 -
 arch/x86/platform/efi/efi.c                     | 354 ++++++++----------------
 arch/x86/platform/efi/efi_32.c                  |  22 +-
 arch/x86/platform/efi/efi_64.c                  | 157 +++++++----
 arch/x86/platform/efi/efi_stub_32.S             | 109 ++------
 arch/x86/platform/efi/efi_stub_64.S             |  43 +--
 arch/x86/platform/efi/efi_thunk_64.S            | 121 ++------
 arch/x86/platform/uv/bios_uv.c                  |   7 +-
 drivers/firmware/efi/Kconfig                    |  22 ++
 drivers/firmware/efi/libstub/Makefile           |   2 +-
 drivers/firmware/efi/libstub/efi-stub-helper.c  |  20 +-
 drivers/firmware/efi/libstub/pci.c              | 114 ++++++++
 include/linux/efi.h                             |  29 +-
 19 files changed, 597 insertions(+), 627 deletions(-)
 create mode 100644 drivers/firmware/efi/libstub/pci.c
author maintainer
#1 lore raw

PATCH 1/20 efi/libstub: fix boot argument handling in mixed mode entry code

The mixed mode refactor actually broke mixed mode by failing to pass the bootparam structure to startup_32(). This went unnoticed because it apparently has a high tolerance for being passed random junk, and still boots fine in some cases. So let's fix this by populating %esi as required when entering via efi32_stub_entry, and while at it, preserve the arguments themselves instead of their address in memory (via the stack pointer) since that memory could be clobbered before we get to it.

---
 arch/x86/boot/compressed/head_64.S | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)
arch/x86/boot/compressed/head_64.S +10 -7
index a6f3ee9ca61d..44a6bb6964b5 100644
@@ -208,13 +208,12 @@ SYM_FUNC_START(startup_32)
 	pushl	$__KERNEL_CS
 	leal	startup_64(%ebp), %eax
 #ifdef CONFIG_EFI_MIXED
-	movl	efi32_boot_args(%ebp), %ebx
-	cmp	$0, %ebx
+	movl	efi32_boot_args(%ebp), %edi
+	cmp	$0, %edi
 	jz	1f
 	leal	handover_entry(%ebp), %eax
-	movl	0(%ebx), %edi
-	movl	4(%ebx), %esi
-	movl	8(%ebx), %edx
+	movl	%esi, %edx
+	movl	efi32_boot_args+4(%ebp), %esi
 	movl	$0x0, %ecx
 1:
 #endif
@@ -232,12 +231,16 @@ SYM_FUNC_END(startup_32)
 	.org 0x190
 SYM_FUNC_START(efi32_stub_entry)
 	add	$0x4, %esp		/* Discard return address */
+	popl	%ecx
+	popl	%edx
+	popl	%esi
 
 	call	1f
 1:	pop	%ebp
 	subl	$1b, %ebp
 
-	movl	%esp, efi32_boot_args(%ebp)
+	movl	%ecx, efi32_boot_args(%ebp)
+	movl	%edx, efi32_boot_args+4(%ebp)
 	sgdtl	efi32_boot_gdt(%ebp)
 
 	/* Disable paging */
@@ -628,7 +631,7 @@ SYM_DATA_START_LOCAL(gdt)
 SYM_DATA_END_LABEL(gdt, SYM_L_LOCAL, gdt_end)
 
 #ifdef CONFIG_EFI_MIXED
-SYM_DATA_LOCAL(efi32_boot_args, .long 0)
+SYM_DATA_LOCAL(efi32_boot_args, .long 0, 0)
 #endif
 
 /*
Signature · 1 line
2.20.1
author maintainer
#2 lore raw

PATCH 2/20 efi/libstub/x86: force 'hidden' visibility for extern declarations

In reply to Ard Biesheuvel

Commit c3710de5065d ("efi/libstub/x86: Drop __efi_early() export and efi_config struct") introduced a reference from C code in eboot.c to the startup_32 symbol defined in the .S startup code. This results in a GOT based reference to startup_32, and since GOT entries carry absolute addresses, they need to be fixed up before they can be used.

On modern toolchains (binutils 2.26 or later), this reference is relaxed into a R_386_GOTOFF relocation (or the analogous X86_64 one) which never uses the absolute address in the entry, and so we get away with not fixing up the GOT table before calling the EFI entry point. However, GCC 4.6 combined with a binutils of the era (2.24) will produce a true GOT indirected reference, resulting in a wrong value to be returned for the address of startup_32() if the boot code is not running at the address it was linked at.

Fortunately, we can easily override this behavior, and force GCC to emit the GOTOFF relocations explicitly, by setting the visibility pragma 'hidden'.

---
 arch/x86/boot/compressed/eboot.c | 2 ++
 1 file changed, 2 insertions(+)
arch/x86/boot/compressed/eboot.c +2 -0
index da04948d75ed..565ee4733579 100644
@@ -6,6 +6,8 @@
  *
  * ----------------------------------------------------------------------- */
 
+#pragma GCC visibility push(hidden)
+
 #include <linux/efi.h>
 #include <linux/pci.h>
 
Signature · 1 line
2.20.1
author maintainer
#3 lore raw

PATCH 3/20 efi/x86: re-disable RT services for 32-bit kernels running on 64-bit EFI

In reply to Ard Biesheuvel

Commit a8147dba75b1 ("efi/x86: Rename efi_is_native() to efi_is_mixed()") renamed and refactored efi_is_native() into efi_is_mixed(), but failed to take into account that these are not diametrical opposites.

Mixed mode is a construct that permits 64-bit kernels to boot on 32-bit firmware, but there is another non-native combination which is supported, i.e., 32-bit kernels booting on 64-bit firmware, but only for boot and not for runtime services. Also, mixed mode can be disabled in Kconfig, in which case the 64-bit kernel can still be booted from 32-bit firmware, but without access to runtime services.

Due to this oversight, efi_runtime_supported() now incorrectly returns true for such configurations, resulting in crashes at boot. So fix this by making efi_runtime_supported() aware of this.

As a side effect, some efi_thunk_xxx() stubs have become obsolete, so remove them as well.

---
 arch/x86/include/asm/efi.h | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)
arch/x86/include/asm/efi.h +2 -16
index 2d1378f19b74..b35b5d423e9d 100644
@@ -163,10 +163,10 @@ static inline bool efi_is_mixed(void)
 
 static inline bool efi_runtime_supported(void)
 {
-	if (!efi_is_mixed())
+	if (IS_ENABLED(CONFIG_X86_64) == efi_enabled(EFI_64BIT))
 		return true;
 
-	if (!efi_enabled(EFI_OLD_MEMMAP))
+	if (IS_ENABLED(CONFIG_EFI_MIXED) && !efi_enabled(EFI_OLD_MEMMAP))
 		return true;
 
 	return false;
@@ -176,7 +176,6 @@ extern void parse_efi_setup(u64 phys_addr, u32 data_len);
 
 extern void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
 
-#ifdef CONFIG_EFI_MIXED
 extern void efi_thunk_runtime_setup(void);
 extern efi_status_t efi_thunk_set_virtual_address_map(
 	void *phys_set_virtual_address_map,
@@ -184,19 +183,6 @@ extern efi_status_t efi_thunk_set_virtual_address_map(
 	unsigned long descriptor_size,
 	u32 descriptor_version,
 	efi_memory_desc_t *virtual_map);
-#else
-static inline void efi_thunk_runtime_setup(void) {}
-static inline efi_status_t efi_thunk_set_virtual_address_map(
-	void *phys_set_virtual_address_map,
-	unsigned long memory_map_size,
-	unsigned long descriptor_size,
-	u32 descriptor_version,
-	efi_memory_desc_t *virtual_map)
-{
-	return EFI_SUCCESS;
-}
-#endif /* CONFIG_EFI_MIXED */
-
 
 /* arch specific definitions used by the stub code */
 
Signature · 1 line
2.20.1
author maintainer
#4 lore raw

PATCH 4/20 efi/x86: map the entire EFI vendor string before copying it

In reply to Ard Biesheuvel

Fix a couple of issues with the way we map and copy the vendor string:
- we map only 2 bytes, which usually works since you get at least a page, but if the vendor string happens to cross a page boundary, a crash will result
- only call early_memunmap() if early_memremap() succeeded, or we will call it with a NULL address which it doesn't like,
- while at it, switch to early_memremap_ro(), and array indexing rather than pointer dereferencing to read the CHAR16 characters.

Fixes 5b83683f32b1 ("x86: EFI runtime service support")
---
 arch/x86/platform/efi/efi.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
arch/x86/platform/efi/efi.c +7 -6
index d96953d9d4e7..3ce32c31bb61 100644
@@ -541,7 +541,6 @@ void __init efi_init(void)
 	efi_char16_t *c16;
 	char vendor[100] = "unknown";
 	int i = 0;
-	void *tmp;
 
 #ifdef CONFIG_X86_32
 	if (boot_params.efi_info.efi_systab_hi ||
@@ -566,14 +565,16 @@ void __init efi_init(void)
 	/*
 	 * Show what we know for posterity
 	 */
-	c16 = tmp = early_memremap(efi.systab->fw_vendor, 2);
+	c16 = early_memremap_ro(efi.systab->fw_vendor,
+				sizeof(vendor) * sizeof(efi_char16_t));
 	if (c16) {
-		for (i = 0; i < sizeof(vendor) - 1 && *c16; ++i)
-			vendor[i] = *c16++;
+		for (i = 0; i < sizeof(vendor) - 1 && c16[i]; ++i)
+			vendor[i] = c16[i];
 		vendor[i] = '\0';
-	} else
+		early_memunmap(c16, sizeof(vendor) * sizeof(efi_char16_t));
+	} else {
 		pr_err("Could not map the firmware vendor!\n");
-	early_memunmap(tmp, 2);
+	}
 
 	pr_info("EFI v%u.%.02u by %s\n",
 		efi.systab->hdr.revision >> 16,
Signature · 1 line
2.20.1
author maintainer
#5 lore raw

PATCH 5/20 efi/x86: avoid redundant cast of EFI firmware service pointer

In reply to Ard Biesheuvel

All EFI firmware call prototypes have been annotated as __efiapi, permitting us to attach attributes regarding the calling convention by overriding __efiapi to an architecture specific value.

On 32-bit x86, EFI firmware calls use the plain calling convention where all arguments are passed via the stack, and cleaned up by the caller. Let's add this to the __efiapi definition so we no longer need to cast the function pointers before invoking them.

---
 arch/x86/include/asm/efi.h | 8 +-------
 include/linux/efi.h        | 4 +++-
 2 files changed, 4 insertions(+), 8 deletions(-)
arch/x86/include/asm/efi.h +1 -7
index b35b5d423e9d..09c3fc468793 100644
@@ -51,13 +51,7 @@ extern asmlinkage unsigned long efi_call_phys(void *, ...);
 })
 
 
-/*
- * Wrap all the virtual calls in a way that forces the parameters on the stack.
- */
-#define arch_efi_call_virt(p, f, args...)				\
-({									\
-	((efi_##f##_t __attribute__((regparm(0)))*) p->f)(args);	\
-})
+#define arch_efi_call_virt(p, f, args...)	p->f(args)
 
 #define efi_ioremap(addr, size, type, attr)	ioremap_cache(addr, size)
 
include/linux/efi.h +3 -1
index 726673e98990..952c1659dfd9 100644
@@ -48,8 +48,10 @@ typedef u16 efi_char16_t;		/* UNICODE character */
 typedef u64 efi_physical_addr_t;
 typedef void *efi_handle_t;
 
-#ifdef CONFIG_X86_64
+#if defined(CONFIG_X86_64)
 #define __efiapi __attribute__((ms_abi))
+#elif defined(CONFIG_X86_32)
+#define __efiapi __attribute__((regparm(0)))
 #else
 #define __efiapi
 #endif
Signature · 1 line
2.20.1
author maintainer
#6 lore raw

PATCH 6/20 efi/x86: split off some old memmap handling into separate routines

In reply to Ard Biesheuvel

In a subsequent patch, we will fold the prolog/epilog routines that are part of the support code to call SetVirtualAddressMap() with a 1:1 mapping into the callers. However, the 64-bit version mostly consists of ugly mapping code that is only used when efi=old_map is in effect, which is extremely rare. So let's move this code out of the way so it does not clutter the common code.

---
 arch/x86/platform/efi/efi_64.c | 35 +++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)
arch/x86/platform/efi/efi_64.c +22 -13
index 03c2ed3c645c..a72bbabbc595 100644
@@ -72,7 +72,9 @@ static void __init early_code_mapping_set_exec(int executable)
 	}
 }
 
-pgd_t * __init efi_call_phys_prolog(void)
+void __init efi_old_memmap_phys_epilog(pgd_t *save_pgd);
+
+pgd_t * __init efi_old_memmap_phys_prolog(void)
 {
 	unsigned long vaddr, addr_pgd, addr_p4d, addr_pud;
 	pgd_t *save_pgd, *pgd_k, *pgd_efi;
@@ -82,11 +84,6 @@ pgd_t * __init efi_call_phys_prolog(void)
 	int pgd;
 	int n_pgds, i, j;
 
-	if (!efi_enabled(EFI_OLD_MEMMAP)) {
-		efi_switch_mm(&efi_mm);
-		return efi_mm.pgd;
-	}
-
 	early_code_mapping_set_exec(1);
 
 	n_pgds = DIV_ROUND_UP((max_pfn << PAGE_SHIFT), PGDIR_SIZE);
@@ -143,11 +140,11 @@ pgd_t * __init efi_call_phys_prolog(void)
 	__flush_tlb_all();
 	return save_pgd;
 out:
-	efi_call_phys_epilog(save_pgd);
+	efi_old_memmap_phys_epilog(save_pgd);
 	return NULL;
 }
 
-void __init efi_call_phys_epilog(pgd_t *save_pgd)
+void __init efi_old_memmap_phys_epilog(pgd_t *save_pgd)
 {
 	/*
 	 * After the lock is released, the original page table is restored.
@@ -158,11 +155,6 @@ void __init efi_call_phys_epilog(pgd_t *save_pgd)
 	p4d_t *p4d;
 	pud_t *pud;
 
-	if (!efi_enabled(EFI_OLD_MEMMAP)) {
-		efi_switch_mm(efi_scratch.prev_mm);
-		return;
-	}
-
 	nr_pgds = DIV_ROUND_UP((max_pfn << PAGE_SHIFT) , PGDIR_SIZE);
 
 	for (pgd_idx = 0; pgd_idx < nr_pgds; pgd_idx++) {
@@ -193,6 +185,23 @@ void __init efi_call_phys_epilog(pgd_t *save_pgd)
 	early_code_mapping_set_exec(0);
 }
 
+pgd_t * __init efi_call_phys_prolog(void)
+{
+	if (efi_enabled(EFI_OLD_MEMMAP))
+		return efi_old_memmap_phys_prolog();
+
+	efi_switch_mm(&efi_mm);
+	return efi_mm.pgd;
+}
+
+void __init efi_call_phys_epilog(pgd_t *save_pgd)
+{
+	if (efi_enabled(EFI_OLD_MEMMAP))
+		efi_old_memmap_phys_epilog(save_pgd);
+	else
+		efi_switch_mm(efi_scratch.prev_mm);
+}
+
 EXPORT_SYMBOL_GPL(efi_mm);
 
 /*
Signature · 1 line
2.20.1
author maintainer
#7 lore raw

PATCH 7/20 efi/x86: split SetVirtualAddresMap() wrappers into 32 and 64 bit versions

In reply to Ard Biesheuvel

Split the phys_efi_set_virtual_address_map() routine into 32 and 64 bit versions, so we can simplify them individually in subsequent patches.

There is very little overlap between the logic anyway, and this has already been factored out in prolog/epilog routines which are completely different between 32 bit and 64 bit. So let's take it one step further, and get rid of the overlap completely.

---
 arch/x86/include/asm/efi.h     |  8 ++---
 arch/x86/platform/efi/efi.c    | 30 ++----------------
 arch/x86/platform/efi/efi_32.c | 21 ++++++++++---
 arch/x86/platform/efi/efi_64.c | 56 ++++++++++++++++++++++------------
 4 files changed, 58 insertions(+), 57 deletions(-)
arch/x86/include/asm/efi.h +4 -4
index 09c3fc468793..e29e5dc0b750 100644
@@ -61,8 +61,6 @@ extern asmlinkage unsigned long efi_call_phys(void *, ...);
 
 extern asmlinkage u64 efi_call(void *fp, ...);
 
-#define efi_call_phys(f, args...)		efi_call((f), args)
-
 /*
  * struct efi_scratch - Scratch space used while switching to/from efi_mm
  * @phys_stack: stack used during EFI Mixed Mode
@@ -115,8 +113,6 @@ extern void __iomem *__init efi_ioremap(unsigned long addr, unsigned long size,
 extern struct efi_scratch efi_scratch;
 extern void __init efi_set_executable(efi_memory_desc_t *md, bool executable);
 extern int __init efi_memblock_x86_reserve_range(void);
-extern pgd_t * __init efi_call_phys_prolog(void);
-extern void __init efi_call_phys_epilog(pgd_t *save_pgd);
 extern void __init efi_print_memmap(void);
 extern void __init efi_memory_uc(u64 addr, unsigned long size);
 extern void __init efi_map_region(efi_memory_desc_t *md);
@@ -177,6 +173,10 @@ extern efi_status_t efi_thunk_set_virtual_address_map(
 	unsigned long descriptor_size,
 	u32 descriptor_version,
 	efi_memory_desc_t *virtual_map);
+efi_status_t efi_set_virtual_address_map(unsigned long memory_map_size,
+					 unsigned long descriptor_size,
+					 u32 descriptor_version,
+					 efi_memory_desc_t *virtual_map);
 
 /* arch specific definitions used by the stub code */
 
arch/x86/platform/efi/efi.c +2 -28
index 3ce32c31bb61..50f8123e658a 100644
@@ -54,7 +54,7 @@
 #include <asm/x86_init.h>
 #include <asm/uv/uv.h>
 
-static struct efi efi_phys __initdata;
+struct efi efi_phys __initdata;
 static efi_system_table_t efi_systab __initdata;
 
 static efi_config_table_type_t arch_tables[] __initdata = {
@@ -97,32 +97,6 @@ static int __init setup_add_efi_memmap(char *arg)
 }
 early_param("add_efi_memmap", setup_add_efi_memmap);
 
-static efi_status_t __init phys_efi_set_virtual_address_map(
-	unsigned long memory_map_size,
-	unsigned long descriptor_size,
-	u32 descriptor_version,
-	efi_memory_desc_t *virtual_map)
-{
-	efi_status_t status;
-	unsigned long flags;
-	pgd_t *save_pgd;
-
-	save_pgd = efi_call_phys_prolog();
-	if (!save_pgd)
-		return EFI_ABORTED;
-
-	/* Disable interrupts around EFI calls: */
-	local_irq_save(flags);
-	status = efi_call_phys(efi_phys.set_virtual_address_map,
-			       memory_map_size, descriptor_size,
-			       descriptor_version, virtual_map);
-	local_irq_restore(flags);
-
-	efi_call_phys_epilog(save_pgd);
-
-	return status;
-}
-
 void __init efi_find_mirror(void)
 {
 	efi_memory_desc_t *md;
@@ -1042,7 +1016,7 @@ static void __init __efi_enter_virtual_mode(void)
 	efi_sync_low_kernel_mappings();
 
 	if (!efi_is_mixed()) {
-		status = phys_efi_set_virtual_address_map(
+		status = efi_set_virtual_address_map(
 				efi.memmap.desc_size * count,
 				efi.memmap.desc_size,
 				efi.memmap.desc_version,
arch/x86/platform/efi/efi_32.c +16 -5
index 9959657127f4..185950ade0e9 100644
@@ -66,9 +66,16 @@ void __init efi_map_region(efi_memory_desc_t *md)
 void __init efi_map_region_fixed(efi_memory_desc_t *md) {}
 void __init parse_efi_setup(u64 phys_addr, u32 data_len) {}
 
-pgd_t * __init efi_call_phys_prolog(void)
+extern struct efi efi_phys;
+
+efi_status_t __init efi_set_virtual_address_map(unsigned long memory_map_size,
+						unsigned long descriptor_size,
+						u32 descriptor_version,
+						efi_memory_desc_t *virtual_map)
 {
 	struct desc_ptr gdt_descr;
+	efi_status_t status;
+	unsigned long flags;
 	pgd_t *save_pgd;
 
 	/* Current pgd is swapper_pg_dir, we'll restore it later: */
@@ -80,14 +87,18 @@ pgd_t * __init efi_call_phys_prolog(void)
 	gdt_descr.size = GDT_SIZE - 1;
 	load_gdt(&gdt_descr);
 
-	return save_pgd;
-}
+	/* Disable interrupts around EFI calls: */
+	local_irq_save(flags);
+	status = efi_call_phys(efi_phys.set_virtual_address_map,
+			       memory_map_size, descriptor_size,
+			       descriptor_version, virtual_map);
+	local_irq_restore(flags);
 
-void __init efi_call_phys_epilog(pgd_t *save_pgd)
-{
 	load_fixmap_gdt(0);
 	load_cr3(save_pgd);
 	__flush_tlb_all();
+
+	return status;
 }
 
 void __init efi_runtime_update_mappings(void)
arch/x86/platform/efi/efi_64.c +36 -20

78 lines not on this page · raw message

Signature · 1 line
2.20.1
author maintainer
#8 lore raw

PATCH 8/20 efi/x86: simplify i386 efi_call_phys() firmware call wrapper

In reply to Ard Biesheuvel

The variadic efi_call_phys() wrapper that exists on i386 was originally created to call into any EFI firmware runtime service, but in practice, we only use it once, to call SetVirtualAddressMap() during early boot.
The flexibility provided by the variadic nature also makes it type unsafe, and makes the assembler code more complicated than needed, since it has to deal with an unknown number of arguments living on the stack.

So clean this up, by renaming the helper to efi_call_svam(), and dropping the unneeded complexity. Let's also drop the reference to the efi_phys struct and grab the address from the EFI system table directly.

---
 arch/x86/include/asm/efi.h          |   3 -
 arch/x86/platform/efi/efi_32.c      |   5 +-
 arch/x86/platform/efi/efi_stub_32.S | 109 +++++-----------------------
 3 files changed, 20 insertions(+), 97 deletions(-)
arch/x86/include/asm/efi.h +0 -3
index e29e5dc0b750..cb08035b89a0 100644
@@ -35,9 +35,6 @@
 #define ARCH_EFI_IRQ_FLAGS_MASK	X86_EFLAGS_IF
 
 #ifdef CONFIG_X86_32
-
-extern asmlinkage unsigned long efi_call_phys(void *, ...);
-
 #define arch_efi_call_virt_setup()					\
 ({									\
 	kernel_fpu_begin();						\
arch/x86/platform/efi/efi_32.c +3 -2
index 185950ade0e9..71dddd1620f9 100644
@@ -66,7 +66,8 @@ void __init efi_map_region(efi_memory_desc_t *md)
 void __init efi_map_region_fixed(efi_memory_desc_t *md) {}
 void __init parse_efi_setup(u64 phys_addr, u32 data_len) {}
 
-extern struct efi efi_phys;
+efi_status_t efi_call_svam(efi_set_virtual_address_map_t *__efiapi *,
+			   u32, u32, u32, void *);
 
 efi_status_t __init efi_set_virtual_address_map(unsigned long memory_map_size,
 						unsigned long descriptor_size,
@@ -89,7 +90,7 @@ efi_status_t __init efi_set_virtual_address_map(unsigned long memory_map_size,
 
 	/* Disable interrupts around EFI calls: */
 	local_irq_save(flags);
-	status = efi_call_phys(efi_phys.set_virtual_address_map,
+	status = efi_call_svam(&efi.systab->runtime->set_virtual_address_map,
 			       memory_map_size, descriptor_size,
 			       descriptor_version, virtual_map);
 	local_irq_restore(flags);
arch/x86/platform/efi/efi_stub_32.S +17 -92

135 lines not on this page · raw message

Signature · 1 line
2.20.1
author maintainer
#9 lore raw

PATCH 9/20 efi/x86: simplify 64-bit EFI firmware call wrapper

In reply to Ard Biesheuvel

The efi_call() wrapper used to invoke EFI runtime services serves a number of purposes:
- realign the stack to 16 bytes
- preserve FP and CR0 register state
- translate from SysV to MS calling convention.

Preserving CR0.TS is no longer necessary in Linux, and preserving the FP register state is also redundant in most cases, since efi_call() is almost always used from within the scope of a pair of kernel_fpu_begin()/ kernel_fpu_end() calls, with the exception of the early call to SetVirtualAddressMap() and the SGI UV support code.

So let's add a pair of kernel_fpu_begin()/_end() calls there as well, and remove the unnecessary code from the assembly implementation of efi_call(), and only keep the pieces that deal with the stack alignment and the ABI translation.

---
 arch/x86/platform/efi/Makefile      |  1 -
 arch/x86/platform/efi/efi_64.c      |  3 +++
 arch/x86/platform/efi/efi_stub_64.S | 39 +++--------------------------
 arch/x86/platform/uv/bios_uv.c      |  7 ++++--
 4 files changed, 12 insertions(+), 38 deletions(-)
arch/x86/platform/efi/Makefile +0 -1
index fe29f3f5d384..7ec3a8b31f8b 100644
@@ -1,6 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 OBJECT_FILES_NON_STANDARD_efi_thunk_$(BITS).o := y
-OBJECT_FILES_NON_STANDARD_efi_stub_$(BITS).o := y
 
 obj-$(CONFIG_EFI) 		+= quirks.o efi.o efi_$(BITS).o efi_stub_$(BITS).o
 obj-$(CONFIG_EFI_MIXED)		+= efi_thunk_$(BITS).o
arch/x86/platform/efi/efi_64.c +3 -0
index a7f11d1ff7c4..03565dad0c4b 100644
@@ -1019,6 +1019,8 @@ efi_status_t __init efi_set_virtual_address_map(unsigned long memory_map_size,
 		efi_switch_mm(&efi_mm);
 	}
 
+	kernel_fpu_begin();
+
 	/* Disable interrupts around EFI calls: */
 	local_irq_save(flags);
 	status = efi_call(efi.systab->runtime->set_virtual_address_map,
@@ -1026,6 +1028,7 @@ efi_status_t __init efi_set_virtual_address_map(unsigned long memory_map_size,
 			  descriptor_version, virtual_map);
 	local_irq_restore(flags);
 
+	kernel_fpu_end();
 
 	if (save_pgd)
 		efi_old_memmap_phys_epilog(save_pgd);
arch/x86/platform/efi/efi_stub_64.S +4 -35
index b1d2313fe3bf..e7e1020f4ccb 100644
@@ -8,41 +8,12 @@
  */
 
 #include <linux/linkage.h>
-#include <asm/segment.h>
-#include <asm/msr.h>
-#include <asm/processor-flags.h>
-#include <asm/page_types.h>
-
-#define SAVE_XMM			\
-	mov %rsp, %rax;			\
-	subq $0x70, %rsp;		\
-	and $~0xf, %rsp;		\
-	mov %rax, (%rsp);		\
-	mov %cr0, %rax;			\
-	clts;				\
-	mov %rax, 0x8(%rsp);		\
-	movaps %xmm0, 0x60(%rsp);	\
-	movaps %xmm1, 0x50(%rsp);	\
-	movaps %xmm2, 0x40(%rsp);	\
-	movaps %xmm3, 0x30(%rsp);	\
-	movaps %xmm4, 0x20(%rsp);	\
-	movaps %xmm5, 0x10(%rsp)
-
-#define RESTORE_XMM			\
-	movaps 0x60(%rsp), %xmm0;	\
-	movaps 0x50(%rsp), %xmm1;	\
-	movaps 0x40(%rsp), %xmm2;	\
-	movaps 0x30(%rsp), %xmm3;	\
-	movaps 0x20(%rsp), %xmm4;	\
-	movaps 0x10(%rsp), %xmm5;	\
-	mov 0x8(%rsp), %rsi;		\
-	mov %rsi, %cr0;			\
-	mov (%rsp), %rsp
+#include <asm/nospec-branch.h>
 
 SYM_FUNC_START(efi_call)
 	pushq %rbp
 	movq %rsp, %rbp
-	SAVE_XMM
+	and $~0xf, %rsp
 	mov 16(%rbp), %rax
 	subq $48, %rsp
 	mov %r9, 32(%rsp)
@@ -50,9 +21,7 @@ SYM_FUNC_START(efi_call)
 	mov %r8, %r9
 	mov %rcx, %r8
 	mov %rsi, %rcx
-	call *%rdi
-	addq $48, %rsp
-	RESTORE_XMM
-	popq %rbp
+	CALL_NOSPEC %rdi
+	leave
 	ret
 SYM_FUNC_END(efi_call)
arch/x86/platform/uv/bios_uv.c +5 -2
index ece9cb9c1189..5c0e2eb5d87c 100644
@@ -34,10 +34,13 @@ static s64 __uv_bios_call(enum uv_bios_cmd which, u64 a1, u64 a2, u64 a3,
 	 * If EFI_OLD_MEMMAP is set, we need to fall back to using our old EFI
 	 * callback method, which uses efi_call() directly, with the kernel page tables:
 	 */
-	if (unlikely(efi_enabled(EFI_OLD_MEMMAP)))
+	if (unlikely(efi_enabled(EFI_OLD_MEMMAP))) {
+		kernel_fpu_begin();
 		ret = efi_call((void *)__va(tab->function), (u64)which, a1, a2, a3, a4, a5);
-	else
+		kernel_fpu_end();
+	} else {
 		ret = efi_call_virt_pointer(tab, function, (u64)which, a1, a2, a3, a4, a5);
+	}
 
 	return ret;
 }
Signature · 1 line
2.20.1
author maintainer
#10 lore raw

PATCH 10/20 efi/x86: simplify mixed mode call wrapper

In reply to Ard Biesheuvel

Calling 32-bit EFI runtime services from a 64-bit OS involves switching back to the flat mapping with a stack carved out of memory that is 32-bit addressable.

There is no need to actually execute the 64-bit part of this routine from the flat mapping as well, as long as the entry and return address fit in 32 bits. There is also no need to preserve part of the calling context in global variables: we can simply push the old stack pointer value to the new stack, and keep the return address from the code32 section in EBX.

While at it, move the conditional check whether to invoke the mixed mode version of SetVirtualAddressMap() into the 64-bit implementation of the wrapper routine.

---
 arch/x86/include/asm/efi.h           |   6 --
 arch/x86/platform/efi/efi.c          |  19 +----
 arch/x86/platform/efi/efi_64.c       |  73 ++++++++++------
 arch/x86/platform/efi/efi_thunk_64.S | 121 +++++----------------------
 4 files changed, 71 insertions(+), 148 deletions(-)
arch/x86/include/asm/efi.h +0 -6
index cb08035b89a0..e7e9c6e057f9 100644
@@ -164,12 +164,6 @@ extern void parse_efi_setup(u64 phys_addr, u32 data_len);
 extern void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
 
 extern void efi_thunk_runtime_setup(void);
-extern efi_status_t efi_thunk_set_virtual_address_map(
-	void *phys_set_virtual_address_map,
-	unsigned long memory_map_size,
-	unsigned long descriptor_size,
-	u32 descriptor_version,
-	efi_memory_desc_t *virtual_map);
 efi_status_t efi_set_virtual_address_map(unsigned long memory_map_size,
 					 unsigned long descriptor_size,
 					 u32 descriptor_version,
arch/x86/platform/efi/efi.c +4 -15
index 50f8123e658a..e4d3afac7be3 100644
@@ -1015,21 +1015,10 @@ static void __init __efi_enter_virtual_mode(void)
 
 	efi_sync_low_kernel_mappings();
 
-	if (!efi_is_mixed()) {
-		status = efi_set_virtual_address_map(
-				efi.memmap.desc_size * count,
-				efi.memmap.desc_size,
-				efi.memmap.desc_version,
-				(efi_memory_desc_t *)pa);
-	} else {
-		status = efi_thunk_set_virtual_address_map(
-				efi_phys.set_virtual_address_map,
-				efi.memmap.desc_size * count,
-				efi.memmap.desc_size,
-				efi.memmap.desc_version,
-				(efi_memory_desc_t *)pa);
-	}
-
+	status = efi_set_virtual_address_map(efi.memmap.desc_size * count,
+					     efi.memmap.desc_size,
+					     efi.memmap.desc_version,
+					     (efi_memory_desc_t *)pa);
 	if (status != EFI_SUCCESS) {
 		pr_alert("Unable to switch EFI into virtual mode (status=%lx)!\n",
 			 status);
arch/x86/platform/efi/efi_64.c +47 -26

129 lines not on this page · raw message

arch/x86/platform/efi/efi_thunk_64.S +20 -101

148 lines not on this page · raw message

Signature · 1 line
2.20.1
author maintainer
#11 lore raw

PATCH 11/20 efi/x86: drop two near identical versions of efi_runtime_init()

In reply to Ard Biesheuvel

The routines efi_runtime_init32() and efi_runtime_init64() are almost indistinguishable, and the only relevant difference is the offset in the runtime struct from where to obtain the physical address of the SetVirtualAddressMap() routine.

However, this address is only used once, when installing the virtual address map that the OS will use to invoke EFI runtime services, and at the time of the call, we will necessarily be running with a 1:1 mapping, and so there is no need to do the map/unmap dance here to retrieve the address. In fact, in the preceding changes to these users, we stopped using the address recorded here entirely.

So let's just get rid of all this code since it no longer serves a purpose. While at it, tweak the logic so that we handle unsupported and disable EFI runtime services in the same way, and unmap the EFI memory map in both cases.

---
 arch/x86/platform/efi/efi.c | 95 ++-----------------------------------
 include/linux/efi.h         | 19 --------
 2 files changed, 5 insertions(+), 109 deletions(-)
arch/x86/platform/efi/efi.c +5 -90
index e4d3afac7be3..67cb0fd18777 100644
@@ -429,87 +429,6 @@ static int __init efi_systab_init(void *phys)
 	return 0;
 }
 
-static int __init efi_runtime_init32(void)
-{
-	efi_runtime_services_32_t *runtime;
-
-	runtime = early_memremap((unsigned long)efi.systab->runtime,
-			sizeof(efi_runtime_services_32_t));
-	if (!runtime) {
-		pr_err("Could not map the runtime service table!\n");
-		return -ENOMEM;
-	}
-
-	/*
-	 * We will only need *early* access to the SetVirtualAddressMap
-	 * EFI runtime service. All other runtime services will be called
-	 * via the virtual mapping.
-	 */
-	efi_phys.set_virtual_address_map =
-			(efi_set_virtual_address_map_t *)
-			(unsigned long)runtime->set_virtual_address_map;
-	early_memunmap(runtime, sizeof(efi_runtime_services_32_t));
-
-	return 0;
-}
-
-static int __init efi_runtime_init64(void)
-{
-	efi_runtime_services_64_t *runtime;
-
-	runtime = early_memremap((unsigned long)efi.systab->runtime,
-			sizeof(efi_runtime_services_64_t));
-	if (!runtime) {
-		pr_err("Could not map the runtime service table!\n");
-		return -ENOMEM;
-	}
-
-	/*
-	 * We will only need *early* access to the SetVirtualAddressMap
-	 * EFI runtime service. All other runtime services will be called
-	 * via the virtual mapping.
-	 */
-	efi_phys.set_virtual_address_map =
-			(efi_set_virtual_address_map_t *)
-			(unsigned long)runtime->set_virtual_address_map;
-	early_memunmap(runtime, sizeof(efi_runtime_services_64_t));
-
-	return 0;
-}
-
-static int __init efi_runtime_init(void)
-{
-	int rv;
-
-	/*
-	 * Check out the runtime services table. We need to map
-	 * the runtime services table so that we can grab the physical
-	 * address of several of the EFI runtime functions, needed to
-	 * set the firmware into virtual mode.
-	 *
-	 * When EFI_PARAVIRT is in force then we could not map runtime
-	 * service memory region because we do not have direct access to it.
-	 * However, runtime services are available through proxy functions
-	 * (e.g. in case of Xen dom0 EFI implementation they call special
-	 * hypercall which executes relevant EFI functions) and that is why
-	 * they are always enabled.
-	 */
-
-	if (!efi_enabled(EFI_PARAVIRT)) {
-		if (efi_enabled(EFI_64BIT))
-			rv = efi_runtime_init64();
-		else
-			rv = efi_runtime_init32();
-
-		if (rv)
-			return rv;
-	}
-
-	set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
-
-	return 0;
-}
-
 void __init efi_init(void)
 {
 	efi_char16_t *c16;
@@ -567,13 +486,13 @@ void __init efi_init(void)
 
 	if (!efi_runtime_supported())
 		pr_info("No EFI runtime due to 32/64-bit mismatch with kernel\n");
-	else {
-		if (efi_runtime_disabled() || efi_runtime_init()) {
-			efi_memmap_unmap();
-			return;
-		}
+
+	if (!efi_runtime_supported() || efi_runtime_disabled()) {
+		efi_memmap_unmap();
+		return;
 	}
 
+	set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
 	efi_clean_memmap();
 
 	if (efi_enabled(EFI_DBG))
@@ -934,8 +853,6 @@ static void __init kexec_enter_virtual_mode(void)
 
 	efi_native_runtime_setup();
 
-	efi.set_virtual_address_map = NULL;
-
 	if (efi_enabled(EFI_OLD_MEMMAP) && (__supported_pte_mask & _PAGE_NX))
 		runtime_code_page_mkexec();
 #endif
@@ -1040,8 +957,6 @@ static void __init __efi_enter_virtual_mode(void)
 	else
 		efi_thunk_runtime_setup();
 
-	efi.set_virtual_address_map = NULL;
-
 	/*
 	 * Apply more restrictive page table mapping attributes now that
 	 * SVAM() has been called and the firmware has performed all
include/linux/efi.h +0 -19
index 952c1659dfd9..ee68ea6f85ff 100644
@@ -539,24 +539,6 @@ typedef struct {
 	u32 query_variable_info;
 } efi_runtime_services_32_t;
 
-typedef struct {
-	efi_table_hdr_t hdr;
-	u64 get_time;
-	u64 set_time;
-	u64 get_wakeup_time;
-	u64 set_wakeup_time;
-	u64 set_virtual_address_map;
-	u64 convert_pointer;
-	u64 get_variable;
-	u64 get_next_variable;
-	u64 set_variable;
-	u64 get_next_high_mono_count;
-	u64 reset_system;
-	u64 update_capsule;
-	u64 query_capsule_caps;
-	u64 query_variable_info;
-} efi_runtime_services_64_t;
-
 typedef efi_status_t efi_get_time_t (efi_time_t *tm, efi_time_cap_t *tc);
 typedef efi_status_t efi_set_time_t (efi_time_t *tm);
 typedef efi_status_t efi_get_wakeup_time_t (efi_bool_t *enabled, efi_bool_t *pending,
@@ -946,7 +928,6 @@ extern struct efi {
 	efi_query_capsule_caps_t *query_capsule_caps;
 	efi_get_next_high_mono_count_t *get_next_high_mono_count;
 	efi_reset_system_t *reset_system;
-	efi_set_virtual_address_map_t *set_virtual_address_map;
 	struct efi_memory_map memmap;
 	unsigned long flags;
 } efi;
Signature · 1 line
2.20.1
author maintainer
#12 lore raw

PATCH 12/20 efi/x86: clean up efi_systab_init() routine for legibility

In reply to Ard Biesheuvel

Clean up the efi_systab_init() routine which maps the EFI system table and copies the relevant pieces of data out of it.

The current routine is very difficult to read, so let's clean that up. Also, switch to a R/O mapping of the system table since that is all we need.

Finally, use a plain u64 variable to record the physical address of the system table instead of pointlessly stashing it in a struct efi that is never used for anything else.

---
 arch/x86/platform/efi/efi.c | 166 ++++++++++++++++++------------------
 1 file changed, 82 insertions(+), 84 deletions(-)
arch/x86/platform/efi/efi.c +82 -84
index 67cb0fd18777..53ed0b123641 100644
@@ -54,8 +54,8 @@
 #include <asm/x86_init.h>
 #include <asm/uv/uv.h>
 
-struct efi efi_phys __initdata;
 static efi_system_table_t efi_systab __initdata;
+static u64 efi_systab_phys __initdata;
 
 static efi_config_table_type_t arch_tables[] __initdata = {
 #ifdef CONFIG_X86_UV
@@ -327,89 +327,90 @@ void __init efi_print_memmap(void)
 	}
 }
 
-static int __init efi_systab_init(void *phys)
+static int __init efi_systab_init(u64 phys)
 {
+	int size = efi_enabled(EFI_64BIT) ? sizeof(efi_system_table_64_t)
+					  : sizeof(efi_system_table_32_t);
+	bool over4g = false;
+	void *p;
+
+	p = early_memremap_ro(phys, size);
+	if (p == NULL) {
+		pr_err("Couldn't map the system table!\n");
+		return -ENOMEM;
+	}
+
 	if (efi_enabled(EFI_64BIT)) {
-		efi_system_table_64_t *systab64;
-		struct efi_setup_data *data = NULL;
-		u64 tmp = 0;
+		const efi_system_table_64_t *systab64 = p;
+
+		efi_systab.hdr			= systab64->hdr;
+		efi_systab.fw_vendor		= systab64->fw_vendor;
+		efi_systab.fw_revision		= systab64->fw_revision;
+		efi_systab.con_in_handle	= systab64->con_in_handle;
+		efi_systab.con_in		= systab64->con_in;
+		efi_systab.con_out_handle	= systab64->con_out_handle;
+		efi_systab.con_out		= (void *)(unsigned long)systab64->con_out;
+		efi_systab.stderr_handle	= systab64->stderr_handle;
+		efi_systab.stderr		= systab64->stderr;
+		efi_systab.runtime		= (void *)(unsigned long)systab64->runtime;
+		efi_systab.boottime		= (void *)(unsigned long)systab64->boottime;
+		efi_systab.nr_tables		= systab64->nr_tables;
+		efi_systab.tables		= systab64->tables;
+
+		over4g = systab64->con_in_handle	> U32_MAX ||
+			 systab64->con_in		> U32_MAX ||
+			 systab64->con_out_handle	> U32_MAX ||
+			 systab64->con_out		> U32_MAX ||
+			 systab64->stderr_handle	> U32_MAX ||
+			 systab64->stderr		> U32_MAX ||
+			 systab64->boottime		> U32_MAX;
 
 		if (efi_setup) {
-			data = early_memremap(efi_setup, sizeof(*data));
-			if (!data)
+			struct efi_setup_data *data;
+
+			data = early_memremap_ro(efi_setup, sizeof(*data));
+			if (!data) {
+				early_memunmap(p, size);
 				return -ENOMEM;
-		}
-		systab64 = early_memremap((unsigned long)phys,
-					 sizeof(*systab64));
-		if (systab64 == NULL) {
-			pr_err("Couldn't map the system table!\n");
-			if (data)
-				early_memunmap(data, sizeof(*data));
-			return -ENOMEM;
-		}
+			}
+
+			efi_systab.fw_vendor	= (unsigned long)data->fw_vendor;
+			efi_systab.runtime	= (void *)(unsigned long)data->runtime;
+			efi_systab.tables	= (unsigned long)data->tables;
+
+			over4g |= data->fw_vendor	> U32_MAX ||
+				  data->runtime		> U32_MAX ||
+				  data->tables		> U32_MAX;
 
-		efi_systab.hdr = systab64->hdr;
-		efi_systab.fw_vendor = data ? (unsigned long)data->fw_vendor :
-					      systab64->fw_vendor;
-		tmp |= data ? data->fw_vendor : systab64->fw_vendor;
-		efi_systab.fw_revision = systab64->fw_revision;
-		efi_systab.con_in_handle = systab64->con_in_handle;
-		tmp |= systab64->con_in_handle;
-		efi_systab.con_in = systab64->con_in;
-		tmp |= systab64->con_in;
-		efi_systab.con_out_handle = systab64->con_out_handle;
-		tmp |= systab64->con_out_handle;
-		efi_systab.con_out = (void *)(unsigned long)systab64->con_out;
-		tmp |= systab64->con_out;
-		efi_systab.stderr_handle = systab64->stderr_handle;
-		tmp |= systab64->stderr_handle;
-		efi_systab.stderr = systab64->stderr;
-		tmp |= systab64->stderr;
-		efi_systab.runtime = data ?
-				     (void *)(unsigned long)data->runtime :
-				     (void *)(unsigned long)systab64->runtime;
-		tmp |= data ? data->runtime : systab64->runtime;
-		efi_systab.boottime = (void *)(unsigned long)systab64->boottime;
-		tmp |= systab64->boottime;
-		efi_systab.nr_tables = systab64->nr_tables;
-		efi_systab.tables = data ? (unsigned long)data->tables :
-					   systab64->tables;
-		tmp |= data ? data->tables : systab64->tables;
-
-		early_memunmap(systab64, sizeof(*systab64));
-		if (data)
 			early_memunmap(data, sizeof(*data));
-#ifdef CONFIG_X86_32
-		if (tmp >> 32) {
-			pr_err("EFI data located above 4GB, disabling EFI.\n");
-			return -EINVAL;
+		} else {
+			over4g |= systab64->fw_vendor	> U32_MAX ||
+				  systab64->runtime	> U32_MAX ||
+				  systab64->tables	> U32_MAX;
 		}
-#endif
 	} else {
-		efi_system_table_32_t *systab32;
-
-		systab32 = early_memremap((unsigned long)phys,
-					 sizeof(*systab32));
-		if (systab32 == NULL) {
-			pr_err("Couldn't map the system table!\n");
-			return -ENOMEM;
-		}
+		const efi_system_table_32_t *systab32 = p;
+
+		efi_systab.hdr			= systab32->hdr;
+		efi_systab.fw_vendor		= systab32->fw_vendor;
+		efi_systab.fw_revision		= systab32->fw_revision;
+		efi_systab.con_in_handle	= systab32->con_in_handle;
+		efi_systab.con_in		= systab32->con_in;
+		efi_systab.con_out_handle	= systab32->con_out_handle;
+		efi_systab.con_out		= (void *)(unsigned long)systab32->con_out;
+		efi_systab.stderr_handle	= systab32->stderr_handle;
+		efi_systab.stderr		= systab32->stderr;
+		efi_systab.runtime		= (void *)(unsigned long)systab32->runtime;
+		efi_systab.boottime		= (void *)(unsigned long)systab32->boottime;
+		efi_systab.nr_tables		= systab32->nr_tables;
+		efi_systab.tables		= systab32->tables;
+	}
 
-		efi_systab.hdr = systab32->hdr;
-		efi_systab.fw_vendor = systab32->fw_vendor;
-		efi_systab.fw_revision = systab32->fw_revision;
-		efi_systab.con_in_handle = systab32->con_in_handle;
-		efi_systab.con_in = systab32->con_in;
-		efi_systab.con_out_handle = systab32->con_out_handle;
-		efi_systab.con_out = (void *)(unsigned long)systab32->con_out;
-		efi_systab.stderr_handle = systab32->stderr_handle;
-		efi_systab.stderr = systab32->stderr;
-		efi_systab.runtime = (void *)(unsigned long)systab32->runtime;
-		efi_systab.boottime = (void *)(unsigned long)systab32->boottime;
-		efi_systab.nr_tables = systab32->nr_tables;
-		efi_systab.tables = systab32->tables;
+	early_memunmap(p, size);
 
-		early_memunmap(systab32, sizeof(*systab32));
+	if (IS_ENABLED(CONFIG_X86_32) && over4g) {
+		pr_err("EFI data located above 4GB, disabling EFI.\n");
+		return -EINVAL;
 	}
 
 	efi.systab = &efi_systab;
@@ -435,20 +436,17 @@ void __init efi_init(void)
 	char vendor[100] = "unknown";
 	int i = 0;
 
-#ifdef CONFIG_X86_32
-	if (boot_params.efi_info.efi_systab_hi ||
-	    boot_params.efi_info.efi_memmap_hi) {
+	if (IS_ENABLED(CONFIG_X86_32) &&
+	    (boot_params.efi_info.efi_systab_hi ||
+	     boot_params.efi_info.efi_memmap_hi)) {
 		pr_info("Table located above 4GB, disabling EFI.\n");
 		return;
 	}
-	efi_phys.systab = (efi_system_table_t *)boot_params.efi_info.efi_systab;
-#else
-	efi_phys.systab = (efi_system_table_t *)
-			  (boot_params.efi_info.efi_systab |
-			  ((__u64)boot_params.efi_info.efi_systab_hi<<32));
-#endif
 
-	if (efi_systab_init(efi_phys.systab))
+	efi_systab_phys = boot_params.efi_info.efi_systab |
+			  ((__u64)boot_params.efi_info.efi_systab_hi << 32);
+
+	if (efi_systab_init(efi_systab_phys))
 		return;
 
 	efi.config_table = (unsigned long)efi.systab->tables;
@@ -601,7 +599,7 @@ static void __init get_systab_virt_addr(efi_memory_desc_t *md)
 
 	size = md->num_pages << EFI_PAGE_SHIFT;
 	end = md->phys_addr + size;
-	systab = (u64)(unsigned long)efi_phys.systab;
+	systab = efi_systab_phys;
 	if (md->phys_addr <= systab && systab < end) {
 		systab += md->virt_addr - md->phys_addr;
 		efi.systab = (efi_system_table_t *)(unsigned long)systab;
Signature · 1 line
2.20.1
author maintainer
#13 lore raw

PATCH 13/20 efi/x86: don't panic or BUG() on non-critical error conditions

In reply to Ard Biesheuvel

The logic in __efi_enter_virtual_mode() does a number of steps in sequence, all of which may fail in one way or the other. In most cases, we simply print an error and disable EFI runtime services support, but in some cases, we BUG() or panic() and bring down the system when encountering conditions that we could easily handle in the same way.

While at it, replace a pointless page-to-virt-phys conversion with one that goes straight from struct page to physical.

---
 arch/x86/platform/efi/efi.c    | 28 ++++++++++++++--------------
 arch/x86/platform/efi/efi_64.c |  9 +++++----
 2 files changed, 19 insertions(+), 18 deletions(-)
arch/x86/platform/efi/efi.c +14 -14
index 53ed0b123641..4f539bfdc051 100644
@@ -889,16 +889,14 @@ static void __init __efi_enter_virtual_mode(void)
 
 	if (efi_alloc_page_tables()) {
 		pr_err("Failed to allocate EFI page tables\n");
-		clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
-		return;
+		goto err;
 	}
 
 	efi_merge_regions();
 	new_memmap = efi_map_regions(&count, &pg_shift);
 	if (!new_memmap) {
 		pr_err("Error reallocating memory, EFI runtime non-functional!\n");
-		clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
-		return;
+		goto err;
 	}
 
 	pa = __pa(new_memmap);
@@ -912,8 +910,7 @@ static void __init __efi_enter_virtual_mode(void)
 
 	if (efi_memmap_init_late(pa, efi.memmap.desc_size * count)) {
 		pr_err("Failed to remap late EFI memory map\n");
-		clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
-		return;
+		goto err;
 	}
 
 	if (efi_enabled(EFI_DBG)) {
@@ -921,12 +918,11 @@ static void __init __efi_enter_virtual_mode(void)
 		efi_print_memmap();
 	}
 
-	BUG_ON(!efi.systab);
+	if (WARN_ON(!efi.systab))
+		goto err;
 
-	if (efi_setup_page_tables(pa, 1 << pg_shift)) {
-		clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
-		return;
-	}
+	if (efi_setup_page_tables(pa, 1 << pg_shift))
+		goto err;
 
 	efi_sync_low_kernel_mappings();
 
@@ -935,9 +931,9 @@ static void __init __efi_enter_virtual_mode(void)
 					     efi.memmap.desc_version,
 					     (efi_memory_desc_t *)pa);
 	if (status != EFI_SUCCESS) {
-		pr_alert("Unable to switch EFI into virtual mode (status=%lx)!\n",
-			 status);
-		panic("EFI call to SetVirtualAddressMap() failed!");
+		pr_err("Unable to switch EFI into virtual mode (status=%lx)!\n",
+		       status);
+		goto err;
 	}
 
 	efi_free_boot_services();
@@ -964,6 +960,10 @@ static void __init __efi_enter_virtual_mode(void)
 
 	/* clean DUMMY object */
 	efi_delete_dummy_variable();
+	return;
+
+err:
+	clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
 }
 
 void __init efi_enter_virtual_mode(void)
arch/x86/platform/efi/efi_64.c +5 -4
index 910e9ec03b09..c13fa2150976 100644
@@ -384,11 +384,12 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 		return 0;
 
 	page = alloc_page(GFP_KERNEL|__GFP_DMA32);
-	if (!page)
-		panic("Unable to allocate EFI runtime stack < 4GB\n");
+	if (!page) {
+		pr_err("Unable to allocate EFI runtime stack < 4GB\n");
+		return 1;
+	}
 
-	efi_scratch.phys_stack = virt_to_phys(page_address(page));
-	efi_scratch.phys_stack += PAGE_SIZE; /* stack grows down */
+	efi_scratch.phys_stack = page_to_phys(page + 1); /* stack grows down */
 
 	npages = (_etext - _text) >> PAGE_SHIFT;
 	text = __pa(_text);
Signature · 1 line
2.20.1
author maintainer
#14 lore raw

PATCH 14/20 efi/x86: remove unreachable code in kexec_enter_virtual_mode()

In reply to Ard Biesheuvel

Remove some code that is guaranteed to be unreachable, given that we have already bailed by this time if EFI_OLD_MEMMAP is set.

---
 arch/x86/platform/efi/efi.c | 3 ---
 1 file changed, 3 deletions(-)
arch/x86/platform/efi/efi.c +0 -3
index 4f539bfdc051..b931c4bea284 100644
@@ -850,9 +850,6 @@ static void __init kexec_enter_virtual_mode(void)
 	efi.runtime_version = efi_systab.hdr.revision;
 
 	efi_native_runtime_setup();
-
-	if (efi_enabled(EFI_OLD_MEMMAP) && (__supported_pte_mask & _PAGE_NX))
-		runtime_code_page_mkexec();
 #endif
 }
 
Signature · 1 line
2.20.1
author maintainer
#15 lore raw

PATCH 15/20 efi/x86: Check number of arguments to variadic functions

In reply to Ard Biesheuvel

From: Arvind Sankar <[email protected]>

On x86 we need to thunk through assembler stubs to call the EFI services for mixed mode, and for runtime services in 64-bit mode. The assembler stubs have limits on how many arguments it handles. Introduce a few macros to check that we do not try to pass too many arguments to the stubs.

---
 arch/x86/boot/compressed/efi_thunk_64.S |  4 +-
 arch/x86/include/asm/efi.h              | 54 ++++++++++++++++++++++++-
 arch/x86/platform/efi/efi_stub_64.S     |  4 +-
 arch/x86/platform/efi/efi_thunk_64.S    |  4 +-
 4 files changed, 58 insertions(+), 8 deletions(-)
arch/x86/boot/compressed/efi_thunk_64.S +2 -2
index 6d95eb6b8912..d040ff5458e5 100644
@@ -23,7 +23,7 @@
 
 	.code64
 	.text
-SYM_FUNC_START(efi64_thunk)
+SYM_FUNC_START(__efi64_thunk)
 	push	%rbp
 	push	%rbx
 
@@ -95,7 +95,7 @@ SYM_FUNC_START(efi64_thunk)
 	pop	%rbx
 	pop	%rbp
 	ret
-SYM_FUNC_END(efi64_thunk)
+SYM_FUNC_END(__efi64_thunk)
 
 	.code32
 /*
arch/x86/include/asm/efi.h +52 -2

78 lines not on this page · raw message

arch/x86/platform/efi/efi_stub_64.S +2 -2
index e7e1020f4ccb..15da118f04f0 100644
@@ -10,7 +10,7 @@
 #include <linux/linkage.h>
 #include <asm/nospec-branch.h>
 
-SYM_FUNC_START(efi_call)
+SYM_FUNC_START(__efi_call)
 	pushq %rbp
 	movq %rsp, %rbp
 	and $~0xf, %rsp
@@ -24,4 +24,4 @@ SYM_FUNC_START(efi_call)
 	CALL_NOSPEC %rdi
 	leave
 	ret
-SYM_FUNC_END(efi_call)
+SYM_FUNC_END(__efi_call)
arch/x86/platform/efi/efi_thunk_64.S +2 -2
index 162b35729633..26f0da238c1c 100644
@@ -25,7 +25,7 @@
 
 	.text
 	.code64
-SYM_CODE_START(efi64_thunk)
+SYM_CODE_START(__efi64_thunk)
 	push	%rbp
 	push	%rbx
 
@@ -69,4 +69,4 @@ SYM_CODE_START(efi64_thunk)
 2:	pushl	$__KERNEL_CS
 	pushl	%ebp
 	lret
-SYM_CODE_END(efi64_thunk)
+SYM_CODE_END(__efi64_thunk)
Signature · 1 line
2.20.1
author maintainer
#16 lore raw

PATCH 16/20 efi/x86: Allow translating 64-bit arguments for mixed mode calls

In reply to Ard Biesheuvel

From: Arvind Sankar <[email protected]>

Introduce the ability to define macros to perform argument translation for the calls that need it, and define them for the boot services that we currently use.

When calling 32-bit firmware methods in mixed mode, all output parameters that are 32-bit according to the firmware, but 64-bit in the kernel (ie OUT UINTN * or OUT VOID **) must be initialized in the kernel, or the upper 32 bits may contain garbage. Define macros that zero out the upper 32 bits of the output before invoking the firmware method.

When a 32-bit EFI call takes 64-bit arguments, the mixed-mode call must push the two 32-bit halves as separate arguments onto the stack. This can be achieved by splitting the argument into its two halves when calling the assembler thunk. Define a macro to do this for the free_pages boot service.

---
 arch/x86/boot/compressed/eboot.c              | 16 -----
 arch/x86/include/asm/efi.h                    | 71 +++++++++++++++++--
 .../firmware/efi/libstub/efi-stub-helper.c    |  5 +-
 3 files changed, 67 insertions(+), 25 deletions(-)
arch/x86/boot/compressed/eboot.c +0 -16
index 565ee4733579..4afd29eb5b34 100644
@@ -891,19 +891,3 @@ struct boot_params *efi_main(efi_handle_t handle,
 	for (;;)
 		asm("hlt");
 }
-
-#ifdef CONFIG_EFI_MIXED
-void efi_free_native(unsigned long size, unsigned long addr);
-
-void efi_free(unsigned long size, unsigned long addr)
-{
-	if (!size)
-		return;
-
-	if (efi_is_native())
-		efi_free_native(size, addr);
-	else
-		efi64_thunk(efi_system_table()->boottime->mixed_mode.free_pages,
-			    addr, 0, DIV_ROUND_UP(size, EFI_PAGE_SIZE));
-}
-#endif
arch/x86/include/asm/efi.h +66 -5

88 lines not on this page · raw message

drivers/firmware/efi/libstub/efi-stub-helper.c +1 -4
index f1b9c36934e9..fcc45ee94e02 100644
@@ -344,9 +344,6 @@ efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
 }
 
 void efi_free(unsigned long size, unsigned long addr)
-	__weak __alias(efi_free_native);
-
-void efi_free_native(unsigned long size, unsigned long addr)
 {
 	unsigned long nr_pages;
 
@@ -354,7 +351,7 @@ void efi_free_native(unsigned long size, unsigned long addr)
 		return;
 
 	nr_pages = round_up(size, EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE;
-	efi_system_table()->boottime->free_pages(addr, nr_pages);
+	efi_bs_call(free_pages, addr, nr_pages);
 }
 
 static efi_status_t efi_file_size(void *__fh, efi_char16_t *filename_16,
Signature · 1 line
2.20.1
author maintainer
#17 lore raw

PATCH 17/20 efi: Allow disabling PCI busmastering on bridges during boot

In reply to Ard Biesheuvel

From: Matthew Garrett <[email protected]>

Add an option to disable the busmaster bit in the control register on all PCI bridges before calling ExitBootServices() and passing control to the runtime kernel. System firmware may configure the IOMMU to prevent malicious PCI devices from being able to attack the OS via DMA. However, since firmware can't guarantee that the OS is IOMMU-aware, it will tear down IOMMU configuration when ExitBootServices() is called. This leaves a window between where a hostile device could still cause damage before Linux configures the IOMMU again.

If CONFIG_EFI_DISABLE_PCI_DMA is enabled or "efi=disable_early_pci_dma" is passed on the command line, the EFI stub will clear the busmaster bit on all PCI bridges before ExitBootServices() is called. This will prevent any malicious PCI devices from being able to perform DMA until the kernel reenables busmastering after configuring the IOMMU.

This option may cause failures with some poorly behaved hardware and should not be enabled without testing. The kernel commandline options "efi=disable_early_pci_dma" or "efi=no_disable_early_pci_dma" may be used to override the default. Note that PCI devices downstream from PCI bridges are disconnected from their drivers first, using the UEFI driver model API, so that DMA can be disabled safely at the bridge level.

Cc Andy Lutomirski <[email protected]>
Cc Arvind Sankar <[email protected]>
[ardb: disconnect PCI I/O handles first, as suggested by Arvind]
---
 .../admin-guide/kernel-parameters.txt         |   7 +-
 arch/x86/include/asm/efi.h                    |   5 +
 drivers/firmware/efi/Kconfig                  |  22 ++++
 drivers/firmware/efi/libstub/Makefile         |   2 +-
 .../firmware/efi/libstub/efi-stub-helper.c    |  15 +++
 drivers/firmware/efi/libstub/pci.c            | 114 ++++++++++++++++++
 include/linux/efi.h                           |   6 +-
 7 files changed, 168 insertions(+), 3 deletions(-)
 create mode 100644 drivers/firmware/efi/libstub/pci.c
Documentation/admin-guide/kernel-parameters.txt +6 -1
index ade4e6ec23e0..994632ae48de 100644
@@ -1165,7 +1165,8 @@
 
 	efi=		[EFI]
 			Format: { "old_map", "nochunk", "noruntime", "debug",
-				  "nosoftreserve" }
+				  "nosoftreserve", "disable_early_pci_dma",
+				  "no_disable_early_pci_dma" }
 			old_map [X86-64]: switch to the old ioremap-based EFI
 			runtime services mapping. 32-bit still uses this one by
 			default.
@@ -1180,6 +1181,10 @@
 			claim. Specify efi=nosoftreserve to disable this
 			reservation and treat the memory by its base type
 			(i.e. EFI_CONVENTIONAL_MEMORY / "System RAM").
+			disable_early_pci_dma: Disable the busmaster bit on all
+			PCI bridges while in the EFI boot stub
+			no_disable_early_pci_dma: Leave the busmaster bit set
+			on all PCI bridges while in the EFI boot stub
 
 	efi_no_storage_paranoia [EFI; X86]
 			Using this parameter you can use more than 50% of
arch/x86/include/asm/efi.h +5 -0
index 166f0386719e..383f7a0fc170 100644
@@ -283,6 +283,11 @@ static inline void *efi64_zero_upper(void *p)
 #define __efi64_argmap_locate_protocol(protocol, reg, interface)	\
 	((protocol), (reg), efi64_zero_upper(interface))
 
+/* PCI I/O */
+#define __efi64_argmap_get_location(protocol, seg, bus, dev, func)	\
+	((protocol), efi64_zero_upper(seg), efi64_zero_upper(bus),	\
+	 efi64_zero_upper(dev), efi64_zero_upper(func))
+
 /*
  * The macros below handle the plumbing for the argument mapping. To add a
  * mapping for a specific EFI method, simply define a macro
drivers/firmware/efi/Kconfig +22 -0
index bcc378c19ebe..ecc83e2f032c 100644
@@ -215,6 +215,28 @@ config EFI_RCI2_TABLE
 
 	  Say Y here for Dell EMC PowerEdge systems.
 
+config EFI_DISABLE_PCI_DMA
+       bool "Clear Busmaster bit on PCI bridges during ExitBootServices()"
+       help
+	  Disable the busmaster bit in the control register on all PCI bridges
+	  while calling ExitBootServices() and passing control to the runtime
+	  kernel. System firmware may configure the IOMMU to prevent malicious
+	  PCI devices from being able to attack the OS via DMA. However, since
+	  firmware can't guarantee that the OS is IOMMU-aware, it will tear
+	  down IOMMU configuration when ExitBootServices() is called. This
+	  leaves a window between where a hostile device could still cause
+	  damage before Linux configures the IOMMU again.
+
+	  If you say Y here, the EFI stub will clear the busmaster bit on all
+	  PCI bridges before ExitBootServices() is called. This will prevent
+	  any malicious PCI devices from being able to perform DMA until the
+	  kernel reenables busmastering after configuring the IOMMU.
+
+	  This option will cause failures with some poorly behaved hardware
+	  and should not be enabled without testing. The kernel commandline
+	  options "efi=disable_early_pci_dma" or "efi=no_disable_early_pci_dma"
+	  may be used to override this option.
+
 endmenu
 
 config UEFI_CPER
drivers/firmware/efi/libstub/Makefile +1 -1
index c35f893897e1..98a81576213d 100644
@@ -39,7 +39,7 @@ OBJECT_FILES_NON_STANDARD	:= y
 KCOV_INSTRUMENT			:= n
 
 lib-y				:= efi-stub-helper.o gop.o secureboot.o tpm.o \
-				   random.o
+				   random.o pci.o
 
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
 arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
drivers/firmware/efi/libstub/efi-stub-helper.c +15 -0
index fcc45ee94e02..74ddfb496140 100644
@@ -33,6 +33,8 @@ static bool __efistub_global efi_nokaslr;
 static bool __efistub_global efi_quiet;
 static bool __efistub_global efi_novamap;
 static bool __efistub_global efi_nosoftreserve;
+static bool __efistub_global efi_disable_pci_dma =
+					IS_ENABLED(CONFIG_EFI_DISABLE_PCI_DMA);
 
 bool __pure nokaslr(void)
 {
@@ -490,6 +492,16 @@ efi_status_t efi_parse_options(char const *cmdline)
 			efi_nosoftreserve = true;
 		}
 
+		if (!strncmp(str, "disable_early_pci_dma", 21)) {
+			str += strlen("disable_early_pci_dma");
+			efi_disable_pci_dma = true;
+		}
+
+		if (!strncmp(str, "no_disable_early_pci_dma", 24)) {
+			str += strlen("no_disable_early_pci_dma");
+			efi_disable_pci_dma = false;
+		}
+
 		/* Group words together, delimited by "," */
 		while (*str && *str != ' ' && *str != ',')
 			str++;
@@ -876,6 +888,9 @@ efi_status_t efi_exit_boot_services(void *handle,
 	if (status != EFI_SUCCESS)
 		goto free_map;
 
+	if (efi_disable_pci_dma)
+		efi_pci_disable_bridge_busmaster();
+
 	status = efi_bs_call(exit_boot_services, handle, *map->key_ptr);
 
 	if (status == EFI_INVALID_PARAMETER) {
drivers/firmware/efi/libstub/pci.c new file +114 -0

114 lines not on this page · raw message

include/linux/efi.h +5 -1
index ee68ea6f85ff..7e8e25b1d11c 100644
@@ -319,7 +319,9 @@ typedef union {
 		void *stall;
 		void *set_watchdog_timer;
 		void *connect_controller;
-		void *disconnect_controller;
+		efi_status_t (__efiapi *disconnect_controller)(efi_handle_t,
+							       efi_handle_t,
+							       efi_handle_t);
 		void *open_protocol;
 		void *close_protocol;
 		void *open_protocol_information;
@@ -1692,4 +1694,6 @@ struct linux_efi_memreserve {
 #define EFI_MEMRESERVE_COUNT(size) (((size) - sizeof(struct linux_efi_memreserve)) \
 	/ sizeof(((struct linux_efi_memreserve *)0)->entry[0]))
 
+void efi_pci_disable_bridge_busmaster(void);
+
 #endif /* _LINUX_EFI_H */
Signature · 1 line
2.20.1
author maintainer
#18 lore raw

PATCH 18/20 x86/mm: fix NX bit clearing issue in kernel_map_pages_in_pgd

In reply to Ard Biesheuvel

Commit 15f003d20782 ("x86/mm/pat: Don't implicitly allow _PAGE_RW in kernel_map_pages_in_pgd()") modified kernel_map_pages_in_pgd() to manage writable permissions of memory mappings in the EFI page table in a different way, but in the process, it removed the ability to clear NX attributes from read-only mappings, by clobbering the clear mask if _PAGE_RW is not being requested.

Failure to remove the NX attribute from read-only mappings is unlikely to be a security issue, but it does prevent us from tightening the permissions in the EFI page tables going forward, so let's fix it now.

Fixes 15f003d20782 ("x86/mm/pat: Don't implicitly allow _PAGE_RW in kernel_map_pages_in_pgd()
---
 arch/x86/mm/pageattr.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
arch/x86/mm/pageattr.c +1 -7
index 1b99ad05b117..f42780ba0893 100644
@@ -2215,7 +2215,7 @@ int __init kernel_map_pages_in_pgd(pgd_t *pgd, u64 pfn, unsigned long address,
 		.pgd = pgd,
 		.numpages = numpages,
 		.mask_set = __pgprot(0),
-		.mask_clr = __pgprot(0),
+		.mask_clr = __pgprot(~page_flags & (_PAGE_NX|_PAGE_RW)),
 		.flags = 0,
 	};
 
@@ -2224,12 +2224,6 @@ int __init kernel_map_pages_in_pgd(pgd_t *pgd, u64 pfn, unsigned long address,
 	if (!(__supported_pte_mask & _PAGE_NX))
 		goto out;
 
-	if (!(page_flags & _PAGE_NX))
-		cpa.mask_clr = __pgprot(_PAGE_NX);
-
-	if (!(page_flags & _PAGE_RW))
-		cpa.mask_clr = __pgprot(_PAGE_RW);
-
 	if (!(page_flags & _PAGE_ENC))
 		cpa.mask_clr = pgprot_encrypted(cpa.mask_clr);
 
Signature · 1 line
2.20.1
author maintainer
#19 lore raw

PATCH 19/20 efi/x86: don't map the entire kernel text RW for mixed mode

In reply to Ard Biesheuvel

The mixed mode thunking routine requires a part of it to be mapped 1:1, and for this reason, we currently map the entire kernel .text read/write in the EFI page tables, which is bad.

In fact, the kernel_map_pages_in_pgd() invocation that installs this mapping is entirely redundant, since all of DRAM is already 1:1 mapped read/write in the EFI page tables when we reach this point, which means that .rodata is mapped read-write as well.

So let's remap both .text and .rodata read-only in the EFI page tables.

---
 arch/x86/platform/efi/efi_64.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/platform/efi/efi_64.c +2 -2
index c13fa2150976..6ec58ff60b56 100644
@@ -391,11 +391,11 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 
 	efi_scratch.phys_stack = page_to_phys(page + 1); /* stack grows down */
 
-	npages = (_etext - _text) >> PAGE_SHIFT;
+	npages = (__end_rodata_aligned - _text) >> PAGE_SHIFT;
 	text = __pa(_text);
 	pfn = text >> PAGE_SHIFT;
 
-	pf = _PAGE_RW | _PAGE_ENC;
+	pf = _PAGE_ENC;
 	if (kernel_map_pages_in_pgd(pgd, pfn, text, npages, pf)) {
 		pr_err("Failed to map kernel text 1:1\n");
 		return 1;
Signature · 1 line
2.20.1
author maintainer
#20 lore raw

PATCH 20/20 efi/x86: avoid RWX mappings for all of DRAM

In reply to Ard Biesheuvel

The EFI code creates RWX mappings for all memory regions that are occupied after the stub completes, and in the mixed mode case, it even creates RWX mappings for all of the remaining DRAM as well.

Let's try to avoid this, by setting the NX bit for all memory regions except the ones that are marked as EFI runtime services code [which means text+rodata+data in practice, so we cannot mark them read-only right away]

---
 arch/x86/platform/efi/efi_64.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)
arch/x86/platform/efi/efi_64.c +13 -4
index 6ec58ff60b56..a974f8a17f73 100644
@@ -365,10 +365,6 @@ int __init efi_setup_page_tables(unsigned long pa_memmap, unsigned num_pages)
 	 * as trim_bios_range() will reserve the first page and isolate it away
 	 * from memory allocators anyway.
 	 */
-	pf = _PAGE_RW;
-	if (sev_active())
-		pf |= _PAGE_ENC;
-
 	if (kernel_map_pages_in_pgd(pgd, 0x0, 0x0, 1, pf)) {
 		pr_err("Failed to create 1:1 mapping for the first page!\n");
 		return 1;
@@ -410,6 +406,19 @@ static void __init __map_region(efi_memory_desc_t *md, u64 va)
 	unsigned long pfn;
 	pgd_t *pgd = efi_mm.pgd;
 
+	/*
+	 * EFI_RUNTIME_SERVICES_CODE regions typically cover PE/COFF
+	 * executable images in memory that consist of both R-X and
+	 * RW- sections, so we cannot apply read-only or non-exec
+	 * permissions just yet. However, modern EFI systems provide
+	 * a memory attributes table that describes those sections
+	 * with the appropriate restricted permissions, which are
+	 * applied in efi_runtime_update_mappings() below. All other
+	 * regions can be mapped non-executable at this point.
+	 */
+	if (md->type != EFI_RUNTIME_SERVICES_CODE)
+		flags |= _PAGE_NX;
+
 	if (!(md->attribute & EFI_MEMORY_WB))
 		flags |= _PAGE_PCD;
 
Signature · 1 line
2.20.1
#21 lore raw
In reply to Ard Biesheuvel
Quoted 11 lines · Ard Biesheuvel

Ingo, Thomas,

This is the second batch of EFI updates for v5.6. Two things are still under discussion, so I'll probably have a few more changes for this cycle in a week or so.

The following changes since commit 0679715e714345d273c0e1eb78078535ffc4b2a1:

efi/libstub/x86: Avoid globals to store context during mixed mode calls (2019-12-25 10:49:26 +0100)

Quoted 8 lines

are available in the Git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git tags/efi-next

for you to fetch changes up to d95e4feae5368a91775c4597a8f298ba84f31535:

efi/x86: avoid RWX mappings for all of DRAM (2020-01-03 11:46:15 +0100)

Ingo, Thomas,

I'll be submitting another PR later today or tomorrow that goes on top of these changes. Please let me know if you would like a v2 of this PR with the new content included, or rather keep them separate.

Thanks,
Ard.

Quoted 62 lines

----------------------------------------------------------------
Second batch of EFI updates for v5.6:
- Some followup fixes for the EFI stub changes that have been queued up already.
- Overhaul of the x86 EFI boot/runtime code, to peel off layers of pointer casting and type mangling via variadic macros and asm wrappers that made the code fragile and ugly.
- Increase robustness for mixed mode code, by using argmaps to annotate and translate function prototypes that are not mixed mode safe. (Arvind)
- Add the ability to disable DMA at the root port level in the EFI stub, to avoid booting into the kernel proper with IOMMUs in pass through and DMA enabled (suggested by Matthew)
- Get rid of RWX mappings in the EFI memory map, where possible.

----------------------------------------------------------------
Ard Biesheuvel (17):
efi/libstub: fix boot argument handling in mixed mode entry code efi/libstub/x86: force 'hidden' visibility for extern declarations efi/x86: re-disable RT services for 32-bit kernels running on 64-bit EFI efi/x86: map the entire EFI vendor string before copying it efi/x86: avoid redundant cast of EFI firmware service pointer efi/x86: split off some old memmap handling into separate routines efi/x86: split SetVirtualAddresMap() wrappers into 32 and 64 bit versions efi/x86: simplify i386 efi_call_phys() firmware call wrapper efi/x86: simplify 64-bit EFI firmware call wrapper
efi/x86: simplify mixed mode call wrapper
efi/x86: drop two near identical versions of efi_runtime_init() efi/x86: clean up efi_systab_init() routine for legibility efi/x86: don't panic or BUG() on non-critical error conditions efi/x86: remove unreachable code in kexec_enter_virtual_mode() x86/mm: fix NX bit clearing issue in kernel_map_pages_in_pgd efi/x86: don't map the entire kernel text RW for mixed mode efi/x86: avoid RWX mappings for all of DRAM

Arvind Sankar (2):
efi/x86: Check number of arguments to variadic functions efi/x86: Allow translating 64-bit arguments for mixed mode calls

Matthew Garrett (1):
efi: Allow disabling PCI busmastering on bridges during boot

 Documentation/admin-guide/kernel-parameters.txt |   7 +-
 arch/x86/boot/compressed/eboot.c                |  18 +-
 arch/x86/boot/compressed/efi_thunk_64.S         |   4 +-
 arch/x86/boot/compressed/head_64.S              |  17 +-
 arch/x86/include/asm/efi.h                      | 169 ++++++++---
 arch/x86/mm/pageattr.c                          |   8 +-
 arch/x86/platform/efi/Makefile                  |   1 -
 arch/x86/platform/efi/efi.c                     | 354 ++++++++----------------
 arch/x86/platform/efi/efi_32.c                  |  22 +-
 arch/x86/platform/efi/efi_64.c                  | 157 +++++++----
 arch/x86/platform/efi/efi_stub_32.S             | 109 ++------
 arch/x86/platform/efi/efi_stub_64.S             |  43 +--
 arch/x86/platform/efi/efi_thunk_64.S            | 121 ++------
 arch/x86/platform/uv/bios_uv.c                  |   7 +-
 drivers/firmware/efi/Kconfig                    |  22 ++
 drivers/firmware/efi/libstub/Makefile           |   2 +-
 drivers/firmware/efi/libstub/efi-stub-helper.c  |  20 +-
 drivers/firmware/efi/libstub/pci.c              | 114 ++++++++
 include/linux/efi.h                             |  29 +-
 19 files changed, 597 insertions(+), 627 deletions(-)
 create mode 100644 drivers/firmware/efi/libstub/pci.c
#22 lore raw
Quoted 18 lines · Ard Biesheuvel
Quoted 17 lines · Ard Biesheuvel

Ingo, Thomas,

This is the second batch of EFI updates for v5.6. Two things are still under discussion, so I'll probably have a few more changes for this cycle in a week or so.

The following changes since commit 0679715e714345d273c0e1eb78078535ffc4b2a1:

efi/libstub/x86: Avoid globals to store context during mixed mode calls (2019-12-25 10:49:26 +0100)

are available in the Git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git tags/efi-next

for you to fetch changes up to d95e4feae5368a91775c4597a8f298ba84f31535:

Quoted 8 lines
Quoted 2 lines

efi/x86: avoid RWX mappings for all of DRAM (2020-01-03 11:46:15 +0100)

Ingo, Thomas,

I'll be submitting another PR later today or tomorrow that goes on top of these changes. Please let me know if you would like a v2 of this PR with the new content included, or rather keep them separate.

So there's one complication I noticed, there's conflicts with ongoing x86/mm work. I've merged x86/mm into efi/core (and will send the branches in that order in the merge window), and the final three patches conflict.

Mind sending those three patches and your other patches on top of the latest efi/core (4444f8541dad)?

Thanks,

Ingo

#23 lore raw
Quoted 31 lines · Ingo Molnar
Quoted 18 lines · Ard Biesheuvel
Quoted 17 lines · Ard Biesheuvel

Ingo, Thomas,

This is the second batch of EFI updates for v5.6. Two things are still under discussion, so I'll probably have a few more changes for this cycle in a week or so.

The following changes since commit 0679715e714345d273c0e1eb78078535ffc4b2a1:

efi/libstub/x86: Avoid globals to store context during mixed mode calls (2019-12-25 10:49:26 +0100)

are available in the Git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git tags/efi-next

for you to fetch changes up to d95e4feae5368a91775c4597a8f298ba84f31535:

Quoted 8 lines
Quoted 2 lines

efi/x86: avoid RWX mappings for all of DRAM (2020-01-03 11:46:15 +0100)

Ingo, Thomas,

I'll be submitting another PR later today or tomorrow that goes on top of these changes. Please let me know if you would like a v2 of this PR with the new content included, or rather keep them separate.

Quoted 7 lines

So there's one complication I noticed, there's conflicts with ongoing x86/mm work. I've merged x86/mm into efi/core (and will send the branches in that order in the merge window), and the final three patches conflict.

Mind sending those three patches and your other patches on top of the latest efi/core (4444f8541dad)?

No problem. I'll rebase and retest, and send out the result end of today or tomorrow.

Thanks,
Ard.