arm64/efi: do not assume DRAM base is aligned to 2 MB

PATCH New Ard Biesheuvel 4 participants 6 messages Oct 29, 2015 arm64

Share & tools

Ard Biesheuvel

author

2015-10-29 14:07 UTC #0 lore raw

PATCH 1/1 arm64/efi: do not assume DRAM base is aligned to 2 MB

The current arm64 Image relocation code in the UEFI stub assumes that the dram_base argument it receives is always a multiple of 2 MB. In reality, it is simply the lowest start address of all RAM entries in the UEFI memory map, which means it could be any multiple of 4 KB.

Since the arm64 kernel Image needs to reside TEXT_OFFSET bytes beyond a 2 MB aligned base, or it will fail to boot, make sure we round dram_base to 2 MB before using it to calculate the relocation address.

Reported-by Timur Tabi <[email protected]>

Acked-by Mark Rutland <[email protected]>

Signed-off-by Ard Biesheuvel <[email protected]>

---

If deemed acceptable, could we either get this into v4.3 or tag it for stable (v4.3 only) please?

 arch/arm64/kernel/efi-stub.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

arch/arm64/kernel/efi-stub.c +12 -2

index 816120ece6bc..78dfbd34b6bf 100644
@@ -25,10 +25,20 @@ efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table_arg,
 	unsigned long kernel_size, kernel_memsize = 0;
 	unsigned long nr_pages;
 	void *old_image_addr = (void *)*image_addr;
+	unsigned long preferred_offset;
+
+	/*
+	 * The preferred offset of the kernel Image is TEXT_OFFSET bytes beyond
+	 * a 2 MB aligned base, which itself may be lower than dram_base, as
+	 * long as the resulting offset equals or exceeds it.
+	 */
+	preferred_offset = round_down(dram_base, SZ_2M) + TEXT_OFFSET;
+	if (preferred_offset < dram_base)
+		preferred_offset += SZ_2M;
 
 	/* Relocate the image, if required. */
 	kernel_size = _edata - _text;
-	if (*image_addr != (dram_base + TEXT_OFFSET)) {
+	if (*image_addr != preferred_offset) {
 		kernel_memsize = kernel_size + (_end - _edata);
 
 		/*

@@ -42,7 +52,7 @@ efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table_arg,
 		 * Mustang), we can still place the kernel@the address
 		 * 'dram_base + TEXT_OFFSET'.
 		 */
-		*image_addr = *reserve_addr = dram_base + TEXT_OFFSET;
+		*image_addr = *reserve_addr = preferred_offset;
 		nr_pages = round_up(kernel_memsize, EFI_ALLOC_ALIGN) /
 			   EFI_PAGE_SIZE;
 		status = efi_call_early(allocate_pages, EFI_ALLOCATE_ADDRESS,

Signature · 1 line

2.1.4

Timur Tabi

2015-10-29 14:52 UTC #1 lore raw

PATCH 1/1 arm64/efi: do not assume DRAM base is aligned to 2 MB

Quoted 5 lines · Ard Biesheuvel

The current arm64 Image relocation code in the UEFI stub assumes that the dram_base argument it receives is always a multiple of 2 MB. In reality, it is simply the lowest start address of all RAM entries in the UEFI memory map, which means it could be any multiple of 4 KB.

Quoted 7 lines

Since the arm64 kernel Image needs to reside TEXT_OFFSET bytes beyond a 2 MB aligned base, or it will fail to boot, make sure we round dram_base to 2 MB before using it to calculate the relocation address.
Reported-by Timur Tabi<[email protected]>
Acked-by Mark Rutland<[email protected]>
Signed-off-by Ard Biesheuvel<[email protected]>

This works for me

Tested-by Timur Tabi<[email protected]>

On my system, dram_base is 0x4000820000, and *image_addr is 0x43956db000, preferred_offset is set to 0x4000880000.

Signature · 3 lines

Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Will Deacon

2015-10-29 14:57 UTC #2 lore raw

PATCH 1/1 arm64/efi: do not assume DRAM base is aligned to 2 MB

In reply to Ard Biesheuvel

Quoted 9 lines · Ard Biesheuvel

The current arm64 Image relocation code in the UEFI stub assumes that the dram_base argument it receives is always a multiple of 2 MB. In reality, it is simply the lowest start address of all RAM entries in the UEFI memory map, which means it could be any multiple of 4 KB.
Since the arm64 kernel Image needs to reside TEXT_OFFSET bytes beyond a 2 MB aligned base, or it will fail to boot, make sure we round dram_base to 2 MB before using it to calculate the relocation address.

Quoted 7 lines

Reported-by Timur Tabi <[email protected]>
Acked-by Mark Rutland <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>
---
If deemed acceptable, could we either get this into v4.3 or tag it for stable (v4.3 only) please?

So is this fixing a regression introduced in 4.3? If so, can I have a
Fixes: tag too, please? (I was under the impression that this was a longstanding issue, but the thread hasn't been easy to follow).

Will

Timur Tabi

2015-10-29 15:00 UTC #3 lore raw

PATCH 1/1 arm64/efi: do not assume DRAM base is aligned to 2 MB

Quoted 3 lines · Will Deacon

So is this fixing a regression introduced in 4.3? If so, can I have a
Fixes: tag too, please? (I was under the impression that this was a longstanding issue, but the thread hasn't been easy to follow).

It fixes a bug introduced by:

Author: Ard Biesheuvel <[email protected]> 2015-07-24 06:38:27
Committer: Will Deacon <[email protected]> 2015-07-27 05:08:41
Parent: ddeeefe2dfbe1fa6b116b9362b1bec465b64c873 (arm64/efi: efistub:
Apply __init annotation)
Child: 739fa11938bec53a849b77b2042237a0e17c70ae (arm64/efi: do not assume DRAM base is aligned to 2 MB)
Branches: dcc, master, qserver, qserver2, remotes/origin/master, tty
Follows: v4.2-rc4
Precedes: v4.3-rc1

arm64: efi: prefer AllocatePages() over efi_low_alloc() for vmlinux

Signature · 3 lines

Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Mark Rutland

reviewer

2015-10-29 15:21 UTC #4 lore raw

PATCH 1/1 arm64/efi: do not assume DRAM base is aligned to 2 MB

Quoted 17 lines · Timur Tabi

Quoted 3 lines · Will Deacon
So is this fixing a regression introduced in 4.3? If so, can I have a
Fixes: tag too, please? (I was under the impression that this was a longstanding issue, but the thread hasn't been easy to follow).
It fixes a bug introduced by:
Author: Ard Biesheuvel <[email protected]> 2015-07-24 06:38:27
Committer: Will Deacon <[email protected]> 2015-07-27 05:08:41
Parent: ddeeefe2dfbe1fa6b116b9362b1bec465b64c873 (arm64/efi:
efistub: Apply __init annotation)
Child: 739fa11938bec53a849b77b2042237a0e17c70ae (arm64/efi: do not assume DRAM base is aligned to 2 MB)
Branches: dcc, master, qserver, qserver2, remotes/origin/master, tty
Follows: v4.2-rc4
Precedes: v4.3-rc1

Quoted 1 line

arm64: efi: prefer AllocatePages() over efi_low_alloc() for vmlinux

As it wasn't in the context, that's commit ID e38457c361b30c5a, which was introduced in v4.3-rc1.

However, the check on the base address was always buggy since its introduction in v3.16 in commit 3c7f255039a2ad6e ("arm64: efi: add EFI stub"), so that portion needs to be backported further.

Thanks,
Mark.

Ard Biesheuvel

author

2015-10-29 15:23 UTC #5 lore raw

PATCH 1/1 arm64/efi: do not assume DRAM base is aligned to 2 MB

In reply to Timur Tabi

Quoted 18 lines · Timur Tabi

Quoted 5 lines · Will Deacon
So is this fixing a regression introduced in 4.3? If so, can I have a
Fixes: tag too, please? (I was under the impression that this was a longstanding issue, but the thread hasn't been easy to follow).
It fixes a bug introduced by:
Author: Ard Biesheuvel <[email protected]> 2015-07-24 06:38:27
Committer: Will Deacon <[email protected]> 2015-07-27 05:08:41
Parent: ddeeefe2dfbe1fa6b116b9362b1bec465b64c873 (arm64/efi: efistub: Apply __init annotation)
Child: 739fa11938bec53a849b77b2042237a0e17c70ae (arm64/efi: do not assume DRAM base is aligned to 2 MB)
Branches: dcc, master, qserver, qserver2, remotes/origin/master, tty
Follows: v4.2-rc4
Precedes: v4.3-rc1

Quoted 3 lines

arm64: efi: prefer AllocatePages() over efi_low_alloc() for vmlinux

The code that existed before that also assumes incorrectly that dram_base is a multiple of 2MB, but since the patch won't apply cleanly, and nobody reported any problems, I think we only need to worry about 4.3

Ard.