[PATCH v2 0/5] Allow the trampoline to use EFI boot services RAM

[PATCH v2 0/5] 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()

Changes from v1:
 - Fix comment in the last patch (hpa)
 - Add missing first patch (I can't count...)
 - Rebase to v4.8-rc1, uneventfully

Andy Lutomirski (5):
  x86/boot: Run reserve_bios_regions() after we initialize the memory
    map
  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/head32.c        |  2 --
 arch/x86/kernel/head64.c        |  1 -
 arch/x86/kernel/setup.c         | 19 ++++++++++-------
 arch/x86/platform/efi/quirks.c  | 21 ++++++++++++++++++
 arch/x86/realmode/init.c        | 47 ++++++++++++++++++++++++++++++-----------
 6 files changed, 76 insertions(+), 24 deletions(-)

-- 
2.7.4

[PATCH v2 1/5] x86/boot: Run reserve_bios_regions() after we initialize the memory map

[Andy Lutomirski]

reserve_bios_regions() is a quirk that reserves memory that we might
otherwise think is available.  There's no need to run it so early,
and running it before we have the memory map initialized with its
non-quirky inputs makes it hard to make reserve_bios_regions() more
intelligent.

Move it right after we populate the memblock state.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/kernel/head32.c | 2 --
 arch/x86/kernel/head64.c | 1 -
 arch/x86/kernel/setup.c  | 2 ++
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c
index 2dda0bc4576e..f16c55bfc090 100644
--- a/arch/x86/kernel/head32.c
+++ b/arch/x86/kernel/head32.c
@@ -25,8 +25,6 @@ static void __init i386_default_early_setup(void)
 	/* Initialize 32bit specific setup functions */
 	x86_init.resources.reserve_resources = i386_reserve_resources;
 	x86_init.mpparse.setup_ioapic_ids = setup_ioapic_ids_from_mpc;
-
-	reserve_bios_regions();
 }
 
 asmlinkage __visible void __init i386_start_kernel(void)
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 99d48e7d2974..54a2372f5dbb 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -183,7 +183,6 @@ void __init x86_64_start_reservations(char *real_mode_data)
 		copy_bootdata(__va(real_mode_data));
 
 	x86_early_init_platform_quirks();
-	reserve_bios_regions();
 
 	switch (boot_params.hdr.hardware_subarch) {
 	case X86_SUBARCH_INTEL_MID:
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 991b77986d57..082ec5171ddc 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1097,6 +1097,8 @@ void __init setup_arch(char **cmdline_p)
 		efi_find_mirror();
 	}
 
+	reserve_bios_regions();
+
 	/*
 	 * The EFI specification says that boot service code won't be called
 	 * after ExitBootServices(). This is, in fact, a lie.
-- 
2.7.4

[tip:x86/urgent] x86/boot: Run reserve_bios_regions() after we initialize the memory map

[tip-bot for Andy Lutomirski]

Commit-ID:  007b756053386af079ba963a8f5817ac651c7c59
Gitweb:     http://git.kernel.org/tip/007b756053386af079ba963a8f5817ac651c7c59
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Wed, 10 Aug 2016 02:29:13 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 11 Aug 2016 11:14:59 +0200

x86/boot: Run reserve_bios_regions() after we initialize the memory map

reserve_bios_regions() is a quirk that reserves memory that we might
otherwise think is available.  There's no need to run it so early,
and running it before we have the memory map initialized with its
non-quirky inputs makes it hard to make reserve_bios_regions() more
intelligent.

Move it right after we populate the memblock state.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mario Limonciello <mario_limonciello@dell.com>
Cc: Matt Fleming <mfleming@suse.de>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/59f58618911005c799c6c9979ce6ae4881d907c2.1470821230.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/head32.c | 2 --
 arch/x86/kernel/head64.c | 1 -
 arch/x86/kernel/setup.c  | 2 ++
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c
index 2dda0bc..f16c55b 100644
--- a/arch/x86/kernel/head32.c
+++ b/arch/x86/kernel/head32.c
@@ -25,8 +25,6 @@ static void __init i386_default_early_setup(void)
 	/* Initialize 32bit specific setup functions */
 	x86_init.resources.reserve_resources = i386_reserve_resources;
 	x86_init.mpparse.setup_ioapic_ids = setup_ioapic_ids_from_mpc;
-
-	reserve_bios_regions();
 }
 
 asmlinkage __visible void __init i386_start_kernel(void)
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 99d48e7..54a2372 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -183,7 +183,6 @@ void __init x86_64_start_reservations(char *real_mode_data)
 		copy_bootdata(__va(real_mode_data));
 
 	x86_early_init_platform_quirks();
-	reserve_bios_regions();
 
 	switch (boot_params.hdr.hardware_subarch) {
 	case X86_SUBARCH_INTEL_MID:
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 95cf31c..bf780e0 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1101,6 +1101,8 @@ void __init setup_arch(char **cmdline_p)
 		efi_find_mirror();
 	}
 
+	reserve_bios_regions();
+
 	/*
 	 * The EFI specification says that boot service code won't be called
 	 * after ExitBootServices(). This is, in fact, a lie.

[PATCH v2 2/5] 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 082ec5171ddc..47006b28cac4 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1127,6 +1127,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());
@@ -1176,13 +1186,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

[tip:x86/urgent] x86/boot: Synchronize trampoline_cr4_features and mmu_cr4_features directly

[tip-bot for Andy Lutomirski]

Commit-ID:  18bc7bd523e0fc5be8d76bf84bde733a97a8c375
Gitweb:     http://git.kernel.org/tip/18bc7bd523e0fc5be8d76bf84bde733a97a8c375
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Wed, 10 Aug 2016 02:29:14 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 11 Aug 2016 11:15:00 +0200

x86/boot: Synchronize trampoline_cr4_features and mmu_cr4_features directly

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>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mario Limonciello <mario_limonciello@dell.com>
Cc: Matt Fleming <mfleming@suse.de>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/d48a263f9912389b957dd495a7127b009259ffe0.1470821230.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@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 bf780e0..95c8c9c 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1131,6 +1131,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());
@@ -1180,13 +1190,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 705e3ff..c5bdc4e 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;

[PATCH v2 3/5] 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 47006b28cac4..c39a4a28fea1 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1137,8 +1137,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

[tip:x86/urgent] x86/boot: Defer setup_real_mode() to early_initcall time

[tip-bot for Andy Lutomirski]

Commit-ID:  d0de0f685db7faf2ae4597a39a59996dd84e18c7
Gitweb:     http://git.kernel.org/tip/d0de0f685db7faf2ae4597a39a59996dd84e18c7
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Wed, 10 Aug 2016 02:29:15 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 11 Aug 2016 11:15:00 +0200

x86/boot: Defer setup_real_mode() to early_initcall time

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>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mario Limonciello <mario_limonciello@dell.com>
Cc: Matt Fleming <mfleming@suse.de>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/fd62f0da4f79357695e9bf3e365623736b05f119.1470821230.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@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 9c6b890..8d67777 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 95c8c9c..0fa60f5 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1141,8 +1141,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 c5bdc4e..747b71e 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);

[PATCH v2 4/5] 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

[tip:x86/urgent] x86/boot: Rework reserve_real_mode() to allow multiple tries

[tip-bot for Andy Lutomirski]

Commit-ID:  5ff3e2c3c3eebe13967d81ad1f23b9468fefea81
Gitweb:     http://git.kernel.org/tip/5ff3e2c3c3eebe13967d81ad1f23b9468fefea81
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Wed, 10 Aug 2016 02:29:16 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 11 Aug 2016 11:15:01 +0200

x86/boot: Rework reserve_real_mode() to allow multiple tries

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>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mario Limonciello <mario_limonciello@dell.com>
Cc: Matt Fleming <mfleming@suse.de>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/876e383038f3e9971aa72fd20a4f5da05f9d193d.1470821230.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@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 8d67777..b2988c0 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 747b71e..5db706f1 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)

[PATCH v2 5/5] 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..89d1146f5a6f 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 manually allocate the trampoline 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

[tip:x86/urgent] x86/efi: Allocate a trampoline if needed in efi_free_boot_services()

[tip-bot for Andy Lutomirski]

Commit-ID:  5bc653b7318217c54244a14f248f1f07abe0a865
Gitweb:     http://git.kernel.org/tip/5bc653b7318217c54244a14f248f1f07abe0a865
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Wed, 10 Aug 2016 02:29:17 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 11 Aug 2016 13:53:07 +0200

x86/efi: Allocate a trampoline if needed in efi_free_boot_services()

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>
Reviewed-by: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mario Limonciello <mario_limonciello@dell.com>
Cc: Matt Fleming <mfleming@suse.de>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/998c77b3bf709f3dfed85cb30701ed1a5d8a438b.1470821230.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@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 4480c06..89d1146 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 manually allocate the trampoline 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);
 	}
 

Re: [PATCH v2 0/5] Allow the trampoline to use EFI boot services RAM

[Ingo Molnar]

* Andy Lutomirski <luto@kernel.org> 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.

Ok, so I like this series - if Matt acks it I can apply it.

How urgent is it? The 'laptop does not boot' aspect worries me - how frequently 
are systems hit by this?

The approach you chose looks sufficiently robust and straightforward to me, so it 
ought to work fine even for x86/urgent - but we can phase it into efi/core as well 
if Matt prefers that.

Thanks,

	Ingo

Re: [PATCH v2 0/5] Allow the trampoline to use EFI boot services RAM

[Andy Lutomirski]

On Wed, Aug 10, 2016 at 5:28 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Andy Lutomirski <luto@kernel.org> 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.
>
> Ok, so I like this series - if Matt acks it I can apply it.
>
> How urgent is it? The 'laptop does not boot' aspect worries me - how frequently
> are systems hit by this?

As far as I know, I'm the only affected user unless Mario has heard
otherwise.  I pinged the Fedora kernel maintainers and no one else has
reported this issue.  I think it needs a fully up-to-date BIOS on a
particular laptop with a non-default BIOS setting enabled that runs
Fedora (or maybe RHEL or CentOS) -- my best guess is that this is only
triggered when using Red Hat / Fedora's patched GRUB, and I have no
clue why.  I've checked, and the reported EFI memory map is different
if I boot using Fedora's GRUB and if I boot exactly the same system
off a live USB stick.

>
> The approach you chose looks sufficiently robust and straightforward to me, so it
> ought to work fine even for x86/urgent - but we can phase it into efi/core as well
> if Matt prefers that.
>

I like x86/urgent because I like to be able to boot stock kernels :)

--Andy

RE: [PATCH v2 0/5] Allow the trampoline to use EFI boot services RAM

[Mario_Limonciello]

> -----Original Message-----
> From: Andy Lutomirski [mailto:luto@amacapital.net]
> Sent: Wednesday, August 10, 2016 8:22 AM
> To: Ingo Molnar <mingo@kernel.org>
> Cc: Andy Lutomirski <luto@kernel.org>; H. Peter Anvin <hpa@zytor.com>;
> X86 ML <x86@kernel.org>; Limonciello, Mario
> <Mario_Limonciello@Dell.com>; Matthew Garrett <mjg59@srcf.ucam.org>;
> Borislav Petkov <bp@alien8.de>; Matt Fleming <mfleming@suse.de>; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v2 0/5] Allow the trampoline to use EFI boot services
> RAM
> 
> On Wed, Aug 10, 2016 at 5:28 AM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * Andy Lutomirski <luto@kernel.org> 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.
> >
> > Ok, so I like this series - if Matt acks it I can apply it.
> >
> > How urgent is it? The 'laptop does not boot' aspect worries me - how
> frequently
> > are systems hit by this?
> 
> As far as I know, I'm the only affected user unless Mario has heard
> otherwise.  I pinged the Fedora kernel maintainers and no one else has
> reported this issue.  I think it needs a fully up-to-date BIOS on a
> particular laptop with a non-default BIOS setting enabled that runs
> Fedora (or maybe RHEL or CentOS) -- my best guess is that this is only
> triggered when using Red Hat / Fedora's patched GRUB, and I have no
> clue why.  I've checked, and the reported EFI memory map is different
> if I boot using Fedora's GRUB and if I boot exactly the same system
> off a live USB stick.
> 

I haven't heard otherwise.  I've asked around for reports of problems
like this, but nothing has come up.  
Keep in mind two it's just recently SGX is getting availability in Linux; it's 
not upstream yet.  It's also not default to on for any BIOS Dell ships today.

So admittedly, the particular configuration that caused failures for Andy 
isn't tested by anyone in Dell or our partners with Linux to my knowledge.


> >
> > The approach you chose looks sufficiently robust and straightforward to
> me, so it
> > ought to work fine even for x86/urgent - but we can phase it into efi/core
> as well
> > if Matt prefers that.
> >
> 
> I like x86/urgent because I like to be able to boot stock kernels :)
> 

I'd agree if possible, if this does turn out to be something systemic with the
perfect storm that Andy created on his box but eventually can reproduce in 
other places I would prefer that the kernel was on top of it.

Re: [PATCH v2 0/5] Allow the trampoline to use EFI boot services RAM

[Ingo Molnar]

One side note:

* Andy Lutomirski <luto@kernel.org> wrote:

> 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.

>   x86/efi: Allocate a trampoline if needed in efi_free_boot_services()

Btw., this means that we first try to allocate the trampoline the old fashioned 
way, and in the rare cases this fails we allocate it from the EFI data area, 
right?

This is problematic from the probability management POV: we are creating a rare 
piece of code that will run only on a select few systems.

I think it would be much better to allocate the trampoline from the EFI area on 
all EFI systems by default. Is there any reason why that would not work?

Thanks,

	Ingo

Re: [PATCH v2 0/5] Allow the trampoline to use EFI boot services RAM

[Andy Lutomirski]

On Aug 10, 2016 3:31 PM, "Ingo Molnar" <mingo@kernel.org> wrote:
>
>
> One side note:
>
> * Andy Lutomirski <luto@kernel.org> wrote:
>
> > 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.
>
> >   x86/efi: Allocate a trampoline if needed in efi_free_boot_services()
>
> Btw., this means that we first try to allocate the trampoline the old fashioned
> way, and in the rare cases this fails we allocate it from the EFI data area,
> right?

Yes, exactly.

>
> This is problematic from the probability management POV: we are creating a rare
> piece of code that will run only on a select few systems.
>
> I think it would be much better to allocate the trampoline from the EFI area on
> all EFI systems by default. Is there any reason why that would not work?

I think most EFI systems don't have any boot services below 1MB, so
that wouldn't work.

We could try allocating from EFI more generically, but that sounds
much scarier.  The EFI memory map code is tangled with the e820 code
and the memblock code, and I'd be nervous about confusing the e820
code or accidentally allocating blacklisted RAM (EBDA,
Sandybridge-quirked, etc.)  The code I wrote should only allocate the
trampoline at a different address than current kernels in cases where
current kernels would panic.

I don't like it either, but after scratching my head for a while I
didn't come up with anything better.  At least the actual special case
is only a couple lines of code.

--Andy

Re: [PATCH v2 0/5] Allow the trampoline to use EFI boot services RAM

[Ingo Molnar]

* Andy Lutomirski <luto@amacapital.net> wrote:

> On Aug 10, 2016 3:31 PM, "Ingo Molnar" <mingo@kernel.org> wrote:
> >
> >
> > One side note:
> >
> > * Andy Lutomirski <luto@kernel.org> wrote:
> >
> > > 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.
> >
> > >   x86/efi: Allocate a trampoline if needed in efi_free_boot_services()
> >
> > Btw., this means that we first try to allocate the trampoline the old fashioned
> > way, and in the rare cases this fails we allocate it from the EFI data area,
> > right?
> 
> Yes, exactly.
> 
> >
> > This is problematic from the probability management POV: we are creating a rare
> > piece of code that will run only on a select few systems.
> >
> > I think it would be much better to allocate the trampoline from the EFI area on
> > all EFI systems by default. Is there any reason why that would not work?
> 
> I think most EFI systems don't have any boot services below 1MB, so
> that wouldn't work.
> 
> We could try allocating from EFI more generically, but that sounds
> much scarier.  The EFI memory map code is tangled with the e820 code
> and the memblock code, and I'd be nervous about confusing the e820
> code or accidentally allocating blacklisted RAM (EBDA,
> Sandybridge-quirked, etc.)  The code I wrote should only allocate the
> trampoline at a different address than current kernels in cases where
> current kernels would panic.
> 
> I don't like it either, but after scratching my head for a while I
> didn't come up with anything better.  At least the actual special case
> is only a couple lines of code.

Ok, fine enough to me!

Matt, is patch #5:

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

looking good to you?

Thanks,

	Ingo

Re: [PATCH v2 0/5] Allow the trampoline to use EFI boot services RAM

[Matt Fleming]

On Thu, 11 Aug, at 10:52:25AM, Ingo Molnar wrote:
> 
> Ok, fine enough to me!
> 
> Matt, is patch #5:
> 
>    [PATCH v2 5/5] x86/efi: Allocate a trampoline if needed in efi_free_boot_services()
> 
> looking good to you?

Yep, looks good.

Reviewed-by: Matt Fleming <matt@codeblueprint.co.uk>