[U-Boot] [PATCH v2] efi_loader: Align runtime section to 64kb

Discussion:

Alexander Graf

2018-09-17 11:54:33 UTC

The UEFI spec mandates that runtime sections are 64kb aligned to enable
support for 64kb page size OSs.

This patch ensures that we extend the runtime section to 64kb to be spec
compliant.

Signed-off-by: Alexander Graf <***@suse.de>

---

v1 -> v2:

- rename kb to KiB in accordance to the spec
- add reference to spec section for alignment requirement
- only use 64KiB alignment on aarch64
---
lib/efi_loader/efi_memory.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index 5bd4f4d7fc..b4cb543275 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -11,6 +11,7 @@
#include <mapmem.h>
#include <watchdog.h>
#include <linux/list_sort.h>
+#include <linux/sizes.h>

DECLARE_GLOBAL_DATA_PTR;

@@ -563,6 +564,7 @@ __weak void efi_add_known_memory(void)
static void add_u_boot_and_runtime(void)
{
unsigned long runtime_start, runtime_end, runtime_pages;
+ unsigned long runtime_mask = EFI_PAGE_MASK;
unsigned long uboot_start, uboot_pages;
unsigned long uboot_stack_size = 16 * 1024 * 1024;

@@ -571,10 +573,22 @@ static void add_u_boot_and_runtime(void)
uboot_pages = (gd->ram_top - uboot_start) >> EFI_PAGE_SHIFT;
efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA, false);

- /* Add Runtime Services */
- runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
+#if defined(__aarch64__)
+ /*
+ * Runtime Services must be 64KiB aligned according to the
+ * "AArch64 Platforms" section in the UEFI spec (2.7+).
+ */
+
+ runtime_mask = SZ_64K - 1;
+#endif
+
+ /*
+ * Add Runtime Services. We mark surrounding boottime code as runtime as
+ * well to fulfill the runtime alignment constraints but avoid padding.
+ */
+ runtime_start = (ulong)&__efi_runtime_start & ~runtime_mask;
runtime_end = (ulong)&__efi_runtime_stop;
- runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
+ runtime_end = (runtime_end + runtime_mask) & ~runtime_mask;
runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
efi_add_memory_map(runtime_start, runtime_pages,
EFI_RUNTIME_SERVICES_CODE, false);

--
2.12.3

Alexander Graf

2018-12-02 20:59:45 UTC

Permalink

Thanks, applied to efi-next

Alex

Heinrich Schuchardt

2018-12-08 13:16:13 UTC

Permalink

Hello Alex,

the patch 7a82c3051c8f ("efi_loader: Align runtime section to 64kb")
currently breaks booting Linux on the Odroid-C2.

Output stops for kernel 4.18 after earlycon is replaced:
[ 0.012518] console [tty0] enabled
[ 0.015923] bootconsole [meson0] disabled
Without your patch it continues.

The calculation introduced by the patch does what is expected:
ram_start = 0x0000000000000000
ram_end = 0x0000000080000000
__efi_runtime_start = 0x000000007ff67118
~runtime_mask = 0xffffffffffff0000
runtime_start = 0x000000007ff60000
runtime_end = 0x000000007ff70000

These two memory reservation do not conflict with addresses in question:
arch/arm/mach-meson/board-common.c(60) meson_board_add_reserved_memory:
0x0000000000000000-0x0000000001000000
arch/arm/mach-meson/board-common.c(60) meson_board_add_reserved_memory:
0x0000000010000000-0x0000000010200000

Regards

Heinrich

Post by Alexander Graf
The UEFI spec mandates that runtime sections are 64kb aligned to enable
support for 64kb page size OSs.
This patch ensures that we extend the runtime section to 64kb to be spec
compliant.
---
- rename kb to KiB in accordance to the spec
- add reference to spec section for alignment requirement
- only use 64KiB alignment on aarch64
---
lib/efi_loader/efi_memory.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index 5bd4f4d7fc..b4cb543275 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -11,6 +11,7 @@
#include <mapmem.h>
#include <watchdog.h>
#include <linux/list_sort.h>
+#include <linux/sizes.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -563,6 +564,7 @@ __weak void efi_add_known_memory(void)
static void add_u_boot_and_runtime(void)
{
unsigned long runtime_start, runtime_end, runtime_pages;
+ unsigned long runtime_mask = EFI_PAGE_MASK;
unsigned long uboot_start, uboot_pages;
unsigned long uboot_stack_size = 16 * 1024 * 1024;
@@ -571,10 +573,22 @@ static void add_u_boot_and_runtime(void)
uboot_pages = (gd->ram_top - uboot_start) >> EFI_PAGE_SHIFT;
efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA, false);
- /* Add Runtime Services */
- runtime_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
+#if defined(__aarch64__)
+ /*
+ * Runtime Services must be 64KiB aligned according to the
+ * "AArch64 Platforms" section in the UEFI spec (2.7+).
+ */
+
+ runtime_mask = SZ_64K - 1;
+#endif
+
+ /*
+ * Add Runtime Services. We mark surrounding boottime code as runtime as
+ * well to fulfill the runtime alignment constraints but avoid padding.
+ */
+ runtime_start = (ulong)&__efi_runtime_start & ~runtime_mask;
runtime_end = (ulong)&__efi_runtime_stop;
- runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
+ runtime_end = (runtime_end + runtime_mask) & ~runtime_mask;
runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
efi_add_memory_map(runtime_start, runtime_pages,
EFI_RUNTIME_SERVICES_CODE, false);

Alexander Graf

2018-12-08 21:59:44 UTC

Permalink

Post by Heinrich Schuchardt
Hello Alex,
the patch 7a82c3051c8f ("efi_loader: Align runtime section to 64kb")
currently breaks booting Linux on the Odroid-C2.
[ 0.012518] console [tty0] enabled
[ 0.015923] bootconsole [meson0] disabled
Without your patch it continues.
ram_start = 0x0000000000000000
ram_end = 0x0000000080000000
__efi_runtime_start = 0x000000007ff67118
~runtime_mask = 0xffffffffffff0000
runtime_start = 0x000000007ff60000
runtime_end = 0x000000007ff70000
0x0000000000000000-0x0000000001000000
0x0000000010000000-0x0000000010200000

We had an unaligned runtime start before already, so I don't quite
understand yet why increasing that to 64kb would break things.

I do fully understand that this patch *is* the culprit. I just would
like to understand why, so we can fix it appropriately.

Loic, this is probably also the failing patch for you?

Alex

Heinrich Schuchardt

2018-12-09 12:55:48 UTC

Permalink

I'll write up a nice patch description after some sleep, but here's at
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index 95844efdb0..4f2aa9deda 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -436,8 +436,13 @@ static efi_status_t EFIAPI efi_set_virtual_address_map(
uint32_t descriptor_version,
struct efi_mem_desc *virtmap)
{
+#if defined(__aarch64__)
+ unsigned long runtime_mask = SZ_64K - 1;
+#else
+ unsigned long runtime_mask = EFI_PAGE_MASK;
+#endif
ulong runtime_start = (ulong)&__efi_runtime_start &
- ~(ulong)EFI_PAGE_MASK;
+ ~runtime_mask;
int n = memory_map_size / descriptor_size;
int i;
Alex

Hello Alex,

the patch is lacking the following lines:

diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index 4f2aa9deda..7523f350eb 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -11,6 +11,7 @@
#include <elf.h>
#include <efi_loader.h>
#include <rtc.h>
+#include <linux/sizes.h>

/* For manual relocation support */

With your patch and the reference to the include on top of U-Boot HEAD
my Odroid-C2 is able to boot via GRUB EFI into Linux 4.18.20.

So, please, go ahead with you patch.

Your current patches only handle 64KiB alignment for the U-Boot runtime.
Won't we need further patches to align runtime drivers and runtime data
along 64KiB boundaries to comply with the UEFI spec?

Best regards

Heinrich