Am 2020-05-14 20:27, schrieb Heinrich Schuchardt:
On 5/14/20 2:38 PM, Michael Walle wrote:
Commit 7a82c3051c8f ("efi_loader: Align runtime section to 64kb")
already aligned the memory region to 64kb, but it does not align the
actual efi runtime code. Thus it is likely, that efi_add_memory_map()
actually adds a larger memory region than the efi runtime code really
is, which is no error I guess. But what actually leads to an error is
that there might be other efi_add_memory_map() calls with regions
overlapping with the already registered efi runtime code section.
Do you relate to this sentence:
"If a 64KiB physical page contains any 4KiB page with any of the
following types listed below, then all 4KiB pages in the 64KiB page
must
use identical ARM Memory Page Attributes"?
I don't think this is what I want to fix here.
Commit 7a82c3051c8f ("efi_loader: Align runtime section to 64kb")
reserves the memory region for runtime services and align the start
(and size) to 64kb boundaries. But the actual runtime services are
not 64kb aligned. And at least on my board, another memory region
right next to it is reserved as well. But of course as another type.
Align the actual runtime code to 64kb instead.
Fixes: 7a82c3051c8f ("efi_loader: Align runtime section to 64kb")
Signed-off-by: Michael Walle <mich...@walle.cc>
---
arch/arm/cpu/armv8/u-boot.lds | 9 ++++++++-
lib/efi_loader/efi_memory.c | 15 ++-------------
2 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/arch/arm/cpu/armv8/u-boot.lds
b/arch/arm/cpu/armv8/u-boot.lds
index 2554980595..3bc4675586 100644
--- a/arch/arm/cpu/armv8/u-boot.lds
+++ b/arch/arm/cpu/armv8/u-boot.lds
@@ -27,7 +27,14 @@ SECTIONS
CPUDIR/start.o (.text*)
}
- /* This needs to come before *(.text*) */
+ /*
+ * Runtime Services must be 64KiB aligned according to the
+ * "AArch64 Platforms" section in the UEFI spec (2.7+).
This is not the exact requirement of the spec. Please, use a
description
that matches the spec.
well I just moved this exact sentence. I'm not familiar with the UEFI
spec.
The requirement that 64KiB areas should have the same attributes was
already presen in UEFI spec 2.4. Please, drop the version reference.
As mentioned above, its about the alignment of the runtime section.
-michael
+ *
+ * This needs to come before *(.text*)
+ */
+
+ . = ALIGN(65536);
Isn't this an alignment before relocation that is irrelevant with
regards to the UEFI spec?
.efi_runtime : {
__efi_runtime_start = .;
*(.text.efi_runtime*)
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index 97d90f069a..fd79178da9 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -12,7 +12,6 @@
#include <mapmem.h>
#include <watchdog.h>
#include <linux/list_sort.h>
-#include <linux/sizes.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -734,7 +733,6 @@ __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;
@@ -745,22 +743,13 @@ static void add_u_boot_and_runtime(void)
uboot_start + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT;
efi_add_memory_map(uboot_start, uboot_pages, EFI_LOADER_DATA,
false);
-#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_start = (ulong)&__efi_runtime_start & ~EFI_PAGE_MASK;
runtime_end = (ulong)&__efi_runtime_stop;
- runtime_end = (runtime_end + runtime_mask) & ~runtime_mask;
+ runtime_end = (runtime_end + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
I cannot see that after these changes you match the requirements of the
UEFI spec.
Best regards
Heinrich
runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
efi_add_memory_map(runtime_start, runtime_pages,
EFI_RUNTIME_SERVICES_CODE, false);
--
-michael
