| Message ID | 20180917115433.84505-1-agraf@suse.de |
|---|---|
| State | Accepted |
| Commit | 7a82c3051c8fdc5c6a91db1f50303ad18c36621f |
| Headers | show |
| Series | [v2] efi_loader: Align runtime section to 64kb | expand |
> 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 <agraf@suse.de> Thanks, applied to efi-next Alex
On 08.12.18 14:16, Heinrich Schuchardt wrote: > 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 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
On 08.12.18 14:16, Heinrich Schuchardt wrote: > 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 I'll write up a nice patch description after some sleep, but here's at least something that makes it work again for me: 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
On 12/9/18 1:16 AM, Alexander Graf wrote: > > > On 08.12.18 14:16, Heinrich Schuchardt wrote: >> 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 > > I'll write up a nice patch description after some sleep, but here's at > least something that makes it work again for me: > > 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
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);
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 <agraf@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(-)