[PATCH 4.8? 0/4] Allow the trampoline to use EFI boot services RAM

[PATCH 4.8? 0/4] Allow the trampoline to use EFI boot services RAM

[Andy Lutomirski]

As currently configured, my laptop cannot boot any existing kernel
because the real mode trampoline can't be reserved.  The ranges in
which it could live are rejected by the kernel: one is EFI boot
services data and the other is above the EBDA.

Allowing use of RAM between the EBDA and 640k is scary: there are
probably many quirky BIOSes out there, and, as currently structured,
it would be awkward to allow it just on EFI boots because we
currently reserve that range before we figure out whether we're
using EFI.

This series fixes it the other way: it allow the trampoline to live
in boot services memory.  It achieves this by deferring the panic
due to failure to reserve a trampoline until early_initcall time
and then adjusting the EFI boot services quirk to reserve space
for the trampoline if we haven't already found it a home.

I'm hoping this is okay for 4.8 even though it's late: it fixes
a boot failure and it's fairly conservative -- the only significant
changes in behavior should be on systems that currently fail to boot.

I'm not currently proposing it for stable because AFAIK I'm the
only person to have seen this issue.  If it survives in Linus'
tree for a while, though, I might propose it for -stable later
on.

Andy Lutomirski (4):
  x86/boot: Synchronize trampoline_cr4_features and mmu_cr4_features
    directly
  x86/boot: Defer setup_real_mode() to early_initcall time
  x86/boot: Rework reserve_real_mode() to allow multiple tries
  x86/efi: Allocate a trampoline if needed in efi_free_boot_services()

 arch/x86/include/asm/realmode.h | 10 ++++++++-
 arch/x86/kernel/setup.c         | 17 ++++++++-------
 arch/x86/platform/efi/quirks.c  | 21 ++++++++++++++++++
 arch/x86/realmode/init.c        | 47 ++++++++++++++++++++++++++++++-----------
 4 files changed, 74 insertions(+), 21 deletions(-)

-- 
2.7.4

[PATCH 1/4] x86/boot: Synchronize trampoline_cr4_features and mmu_cr4_features directly

[Andy Lutomirski]

The initialization process for trampoline_cr4_features and
mmu_cr4_features was confusing.  The intent is for mmu_cr4_features
and *trampoline_cr4_features to stay in sync, but
trampoline_cr4_features is NULL until setup_real_mode() runs.  The
old code synchronized *trampoline_cr4_features *twice*, once in
setup_real_mode() and once in setup_arch().  It also initialized
mmu_cr4_features in setup_real_mode(), which causes the actual value
of mmu_cr4_features to potentially depend on when setup_real_mode()
is called.

With this patch, mmu_cr4_features is initialized directly in
setup_arch(), and *trampoline_cr4_features is synchronized to
mmu_cr4_features when the trampoline is set up.

After this patch, it should be safe to defer setup_real_mode().

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/kernel/setup.c  | 17 ++++++++++-------
 arch/x86/realmode/init.c |  3 ++-
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 0825c493f475..f292a58356fb 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1134,6 +1134,16 @@ void __init setup_arch(char **cmdline_p)
 
 	early_trap_pf_init();
 
+	/*
+	 * Update mmu_cr4_features (and, indirectly, trampoline_cr4_features)
+	 * with the current CR4 value.  This may not be necessary, but
+	 * auditing all the early-boot CR4 manipulation would be needed to
+	 * rule it out.
+	 */
+	if (boot_cpu_data.cpuid_level >= 0)
+		/* A CPU has %cr4 if and only if it has CPUID. */
+		mmu_cr4_features = __read_cr4();
+
 	setup_real_mode();
 
 	memblock_set_current_limit(get_max_mapped());
@@ -1183,13 +1193,6 @@ void __init setup_arch(char **cmdline_p)
 
 	kasan_init();
 
-	if (boot_cpu_data.cpuid_level >= 0) {
-		/* A CPU has %cr4 if and only if it has CPUID */
-		mmu_cr4_features = __read_cr4();
-		if (trampoline_cr4_features)
-			*trampoline_cr4_features = mmu_cr4_features;
-	}
-
 #ifdef CONFIG_X86_32
 	/* sync back kernel address range */
 	clone_pgd_range(initial_page_table + KERNEL_PGD_BOUNDARY,
diff --git a/arch/x86/realmode/init.c b/arch/x86/realmode/init.c
index 705e3fffb4a1..c5bdc4e473e7 100644
--- a/arch/x86/realmode/init.c
+++ b/arch/x86/realmode/init.c
@@ -4,6 +4,7 @@
 #include <asm/cacheflush.h>
 #include <asm/pgtable.h>
 #include <asm/realmode.h>
+#include <asm/tlbflush.h>
 
 struct real_mode_header *real_mode_header;
 u32 *trampoline_cr4_features;
@@ -84,7 +85,7 @@ void __init setup_real_mode(void)
 
 	trampoline_header->start = (u64) secondary_startup_64;
 	trampoline_cr4_features = &trampoline_header->cr4;
-	*trampoline_cr4_features = __read_cr4();
+	*trampoline_cr4_features = mmu_cr4_features;
 
 	trampoline_pgd = (u64 *) __va(real_mode_header->trampoline_pgd);
 	trampoline_pgd[0] = trampoline_pgd_entry.pgd;
-- 
2.7.4

[PATCH 2/4] x86/boot: Defer setup_real_mode() to early_initcall time

[Andy Lutomirski]

There's no need to run setup_real_mode() as early as we run it.
Defer it to the same early_initcall that sets up the page
permissions for the real mode code.

This should be a code size reduction.  More importantly, it give us
a longer window in which we can allocate the real mode trampoline.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/realmode.h |  1 -
 arch/x86/kernel/setup.c         |  2 --
 arch/x86/realmode/init.c        | 15 ++++++++++++---
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/realmode.h b/arch/x86/include/asm/realmode.h
index 9c6b890d5e7a..8d6777724ba4 100644
--- a/arch/x86/include/asm/realmode.h
+++ b/arch/x86/include/asm/realmode.h
@@ -59,6 +59,5 @@ extern unsigned char secondary_startup_64[];
 #endif
 
 void reserve_real_mode(void);
-void setup_real_mode(void);
 
 #endif /* _ARCH_X86_REALMODE_H */
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index f292a58356fb..3bccfa174201 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1144,8 +1144,6 @@ void __init setup_arch(char **cmdline_p)
 		/* A CPU has %cr4 if and only if it has CPUID. */
 		mmu_cr4_features = __read_cr4();
 
-	setup_real_mode();
-
 	memblock_set_current_limit(get_max_mapped());
 
 	/*
diff --git a/arch/x86/realmode/init.c b/arch/x86/realmode/init.c
index c5bdc4e473e7..747b71e8f547 100644
--- a/arch/x86/realmode/init.c
+++ b/arch/x86/realmode/init.c
@@ -30,7 +30,7 @@ void __init reserve_real_mode(void)
 	       base, (unsigned long long)mem, size);
 }
 
-void __init setup_real_mode(void)
+static void __init setup_real_mode(void)
 {
 	u16 real_mode_seg;
 	const u32 *rel;
@@ -101,7 +101,7 @@ void __init setup_real_mode(void)
  * need to mark it executable at do_pre_smp_initcalls() at least,
  * thus run it as a early_initcall().
  */
-static int __init set_real_mode_permissions(void)
+static void __init set_real_mode_permissions(void)
 {
 	unsigned char *base = (unsigned char *) real_mode_header;
 	size_t size = PAGE_ALIGN(real_mode_blob_end - real_mode_blob);
@@ -120,7 +120,16 @@ static int __init set_real_mode_permissions(void)
 	set_memory_nx((unsigned long) base, size >> PAGE_SHIFT);
 	set_memory_ro((unsigned long) base, ro_size >> PAGE_SHIFT);
 	set_memory_x((unsigned long) text_start, text_size >> PAGE_SHIFT);
+}
+
+static int __init init_real_mode(void)
+{
+	if (!real_mode_header)
+		panic("Real mode trampoline was not allocated");
+
+	setup_real_mode();
+	set_real_mode_permissions();
 
 	return 0;
 }
-early_initcall(set_real_mode_permissions);
+early_initcall(init_real_mode);
-- 
2.7.4

[PATCH 3/4] x86/boot: Rework reserve_real_mode() to allow multiple tries

[Andy Lutomirski]

If reserve_real_mode() fails, panicing immediately means we're
doomed.  Make it safe to try more than once to allocate the
trampoline:

 - Degrade a failure from panic() to pr_info().  (If we make it to
   setup_real_mode() without reserving the trampoline, we'll panic
   them.)

 - Factor out helpers so that platform code can supply a specific
   address to try.

 - Warn if reserve_real_mode() is called after we're done with the
   memblock allocator.  If that were to happen, we would behave
   unpredictably.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/realmode.h |  9 +++++++++
 arch/x86/realmode/init.c        | 29 +++++++++++++++++++++--------
 2 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/realmode.h b/arch/x86/include/asm/realmode.h
index 8d6777724ba4..b2988c0ed829 100644
--- a/arch/x86/include/asm/realmode.h
+++ b/arch/x86/include/asm/realmode.h
@@ -58,6 +58,15 @@ extern unsigned char boot_gdt[];
 extern unsigned char secondary_startup_64[];
 #endif
 
+static inline size_t real_mode_size_needed(void)
+{
+	if (real_mode_header)
+		return 0;	/* already allocated. */
+
+	return ALIGN(real_mode_blob_end - real_mode_blob, PAGE_SIZE);
+}
+
+void set_real_mode_mem(phys_addr_t mem, size_t size);
 void reserve_real_mode(void);
 
 #endif /* _ARCH_X86_REALMODE_H */
diff --git a/arch/x86/realmode/init.c b/arch/x86/realmode/init.c
index 747b71e8f547..5db706f14111 100644
--- a/arch/x86/realmode/init.c
+++ b/arch/x86/realmode/init.c
@@ -1,4 +1,5 @@
 #include <linux/io.h>
+#include <linux/slab.h>
 #include <linux/memblock.h>
 
 #include <asm/cacheflush.h>
@@ -12,22 +13,34 @@ u32 *trampoline_cr4_features;
 /* Hold the pgd entry used on booting additional CPUs */
 pgd_t trampoline_pgd_entry;
 
+void __init set_real_mode_mem(phys_addr_t mem, size_t size)
+{
+	void *base = __va(mem);
+
+	real_mode_header = (struct real_mode_header *) base;
+	printk(KERN_DEBUG "Base memory trampoline at [%p] %llx size %zu\n",
+	       base, (unsigned long long)mem, size);
+}
+
 void __init reserve_real_mode(void)
 {
 	phys_addr_t mem;
-	unsigned char *base;
-	size_t size = PAGE_ALIGN(real_mode_blob_end - real_mode_blob);
+	size_t size = real_mode_size_needed();
+
+	if (!size)
+		return;
+
+	WARN_ON(slab_is_available());
 
 	/* Has to be under 1M so we can execute real-mode AP code. */
 	mem = memblock_find_in_range(0, 1<<20, size, PAGE_SIZE);
-	if (!mem)
-		panic("Cannot allocate trampoline\n");
+	if (!mem) {
+		pr_info("No sub-1M memory is available for the trampoline\n");
+		return;
+	}
 
-	base = __va(mem);
 	memblock_reserve(mem, size);
-	real_mode_header = (struct real_mode_header *) base;
-	printk(KERN_DEBUG "Base memory trampoline at [%p] %llx size %zu\n",
-	       base, (unsigned long long)mem, size);
+	set_real_mode_mem(mem, size);
 }
 
 static void __init setup_real_mode(void)
-- 
2.7.4

[PATCH 4/4] x86/efi: Allocate a trampoline if needed in efi_free_boot_services()

[Andy Lutomirski]

On my Dell XPS 13 9350 with firmware 1.4.4 and SGX on, if I boot
Fedora 24's grub2-efi off a hard disk, my first 1MB of RAM looks
like:

efi: mem00: [Runtime Data       |RUN|  |  |  |  |  |  |   |WB|WT|WC|UC] range=[0x0000000000000000-0x0000000000000fff] (0MB)
efi: mem01: [Boot Data          |   |  |  |  |  |  |  |   |WB|WT|WC|UC] range=[0x0000000000001000-0x0000000000027fff] (0MB)
efi: mem02: [Loader Data        |   |  |  |  |  |  |  |   |WB|WT|WC|UC] range=[0x0000000000028000-0x0000000000029fff] (0MB)
efi: mem03: [Reserved           |   |  |  |  |  |  |  |   |WB|WT|WC|UC] range=[0x000000000002a000-0x000000000002bfff] (0MB)
efi: mem04: [Runtime Data       |RUN|  |  |  |  |  |  |   |WB|WT|WC|UC] range=[0x000000000002c000-0x000000000002cfff] (0MB)
efi: mem05: [Loader Data        |   |  |  |  |  |  |  |   |WB|WT|WC|UC] range=[0x000000000002d000-0x000000000002dfff] (0MB)
efi: mem06: [Conventional Memory|   |  |  |  |  |  |  |   |WB|WT|WC|UC] range=[0x000000000002e000-0x0000000000057fff] (0MB)
efi: mem07: [Reserved           |   |  |  |  |  |  |  |   |WB|WT|WC|UC] range=[0x0000000000058000-0x0000000000058fff] (0MB)
efi: mem08: [Conventional Memory|   |  |  |  |  |  |  |   |WB|WT|WC|UC] range=[0x0000000000059000-0x000000000009ffff] (0MB)

My EBDA is at 0x2c000, which blocks off everything from 0x2c000 and
up, and my trampoline is 0x6000 bytes (6 pages), so it doesn't fit
in the loader data range at 0x28000.

Without this patch, it panics due to a failure to allocate the
trampoline.  With this patch, it works:

[  +0.001744] Base memory trampoline at [ffff880000001000] 1000 size 24576

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/platform/efi/quirks.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 4480c06cade7..b9c0eee649ef 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -254,6 +254,7 @@ void __init efi_free_boot_services(void)
 	for_each_efi_memory_desc(md) {
 		unsigned long long start = md->phys_addr;
 		unsigned long long size = md->num_pages << EFI_PAGE_SHIFT;
+		size_t rm_size;
 
 		if (md->type != EFI_BOOT_SERVICES_CODE &&
 		    md->type != EFI_BOOT_SERVICES_DATA)
@@ -263,6 +264,26 @@ void __init efi_free_boot_services(void)
 		if (md->attribute & EFI_MEMORY_RUNTIME)
 			continue;
 
+		/*
+		 * Nasty quirk: if all sub-1MB memory is used for boot
+		 * services, we can get here without having allocated the
+		 * real mode trampoline.  It's too late to hand boot services
+		 * memory back to the memblock allocator, so instead
+		 * try to reclaim a single page if needed.
+		 *
+		 * I've seen this on a Dell XPS 13 9350 with firmware
+		 * 1.4.4 with SGX enabled booting Linux via Fedora 24's
+		 * grub2-efi on a hard disk.  (And no, I don't know why
+		 * this happened, but Linux should still try to boot rather
+		 * panicing early.)
+		 */
+		rm_size = real_mode_size_needed();
+		if (rm_size && (start + rm_size) < (1<<20) && size >= rm_size) {
+			set_real_mode_mem(start, rm_size);
+			start += rm_size;
+			size -= rm_size;
+		}
+
 		free_bootmem_late(start, size);
 	}
 
-- 
2.7.4

Re: [PATCH 4.8? 0/4] Allow the trampoline to use EFI boot services RAM

[Matt Fleming]

On Tue, 26 Jul, at 03:55:24PM, Andy Lutomirski wrote:
> As currently configured, my laptop cannot boot any existing kernel
> because the real mode trampoline can't be reserved.  The ranges in
> which it could live are rejected by the kernel: one is EFI boot
> services data and the other is above the EBDA.
> 
> Allowing use of RAM between the EBDA and 640k is scary: there are
> probably many quirky BIOSes out there, and, as currently structured,
> it would be awkward to allow it just on EFI boots because we
> currently reserve that range before we figure out whether we're
> using EFI.
> 
> This series fixes it the other way: it allow the trampoline to live
> in boot services memory.  It achieves this by deferring the panic
> due to failure to reserve a trampoline until early_initcall time
> and then adjusting the EFI boot services quirk to reserve space
> for the trampoline if we haven't already found it a home.
> 
> I'm hoping this is okay for 4.8 even though it's late: it fixes
> a boot failure and it's fairly conservative -- the only significant
> changes in behavior should be on systems that currently fail to boot.
> 
> I'm not currently proposing it for stable because AFAIK I'm the
> only person to have seen this issue.  If it survives in Linus'
> tree for a while, though, I might propose it for -stable later
> on.

I took a very, very quick look over this series and nothing jumped out
as being wrong. I'll take a much closer look this week.

Re: [PATCH 4/4] x86/efi: Allocate a trampoline if needed in efi_free_boot_services()

[H. Peter Anvin]

On 07/26/16 15:55, Andy Lutomirski wrote:
> +		/*
> +		 * Nasty quirk: if all sub-1MB memory is used for boot
> +		 * services, we can get here without having allocated the
> +		 * real mode trampoline.  It's too late to hand boot services
> +		 * memory back to the memblock allocator, so instead
> +		 * try to reclaim a single page if needed.
> +		 *

This comment makes no sense.  Nowhere is there a dependency on a single
page, and this refers to allocation, not reclaim...

	-hpa

Re: [PATCH 4/4] x86/efi: Allocate a trampoline if needed in efi_free_boot_services()

[Andy Lutomirski]

On Sun, Jul 31, 2016 at 10:07 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 07/26/16 15:55, Andy Lutomirski wrote:
>> +             /*
>> +              * Nasty quirk: if all sub-1MB memory is used for boot
>> +              * services, we can get here without having allocated the
>> +              * real mode trampoline.  It's too late to hand boot services
>> +              * memory back to the memblock allocator, so instead
>> +              * try to reclaim a single page if needed.
>> +              *
>
> This comment makes no sense.  Nowhere is there a dependency on a single
> page, and this refers to allocation, not reclaim...

I wrote that while I had the misapprehension that it was a page and I
forgot to fix it.  How about:

Nasty quirk: if all sub-1MB memory is used for boot
services, we can get here without having allocated the
real mode trampoline.  It's too late to hand boot services
memory back to the memblock allocator, so instead
try to manually allocate the trampoline if needed.

Re: [PATCH 4.8? 0/4] Allow the trampoline to use EFI boot services RAM

[Matt Fleming]

On Sun, 31 Jul, at 10:38:17PM, Matt Fleming wrote:
> 
> I took a very, very quick look over this series and nothing jumped out
> as being wrong. I'll take a much closer look this week.

FWIW, this all looks fine to me and passes my EFI boot tests.