Skip to contentSkip to conversation

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

PATCH Superseded Ard Biesheuvel 5 participants 7 messages Jul 24, 2015 – Oct 27, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 arm64/efi: prefer AllocatePages() over efi_low_alloc() for vmlinux

When allocating memory for the kernel image, try the AllocatePages() boot service to obtain memory at the preferred offset of 'dram_base + TEXT_OFFSET', and only revert to efi_low_alloc() if that fails. This is the only way to allocate at the base of DRAM if DRAM starts at 0x0, since efi_low_alloc() refuses to allocate at 0x0.

---
v2:
- reshuffle code flow to make it more logical, and have only a single memcpy() invocation at the end of the function

---
 arch/arm64/kernel/efi-stub.c | 41 ++++++++++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 9 deletions(-)
arch/arm64/kernel/efi-stub.c +32 -9
index f5374065ad53..816120ece6bc 100644
@@ -13,7 +13,7 @@
 #include <asm/efi.h>
 #include <asm/sections.h>
 
-efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table,
+efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table_arg,
 					unsigned long *image_addr,
 					unsigned long *image_size,
 					unsigned long *reserve_addr,
@@ -23,21 +23,44 @@ efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table,
 {
 	efi_status_t status;
 	unsigned long kernel_size, kernel_memsize = 0;
+	unsigned long nr_pages;
+	void *old_image_addr = (void *)*image_addr;
 
 	/* Relocate the image, if required. */
 	kernel_size = _edata - _text;
 	if (*image_addr != (dram_base + TEXT_OFFSET)) {
 		kernel_memsize = kernel_size + (_end - _edata);
-		status = efi_low_alloc(sys_table, kernel_memsize + TEXT_OFFSET,
-				       SZ_2M, reserve_addr);
+
+		/*
+		 * First, try a straight allocation at the preferred offset.
+		 * This will work around the issue where, if dram_base == 0x0,
+		 * efi_low_alloc() refuses to allocate at 0x0 (to prevent the
+		 * address of the allocation to be mistaken for a FAIL return
+		 * value or a NULL pointer). It will also ensure that, on
+		 * platforms where the [dram_base, dram_base + TEXT_OFFSET)
+		 * interval is partially occupied by the firmware (like on APM
+		 * Mustang), we can still place the kernel at the address
+		 * 'dram_base + TEXT_OFFSET'.
+		 */
+		*image_addr = *reserve_addr = dram_base + TEXT_OFFSET;
+		nr_pages = round_up(kernel_memsize, EFI_ALLOC_ALIGN) /
+			   EFI_PAGE_SIZE;
+		status = efi_call_early(allocate_pages, EFI_ALLOCATE_ADDRESS,
+					EFI_LOADER_DATA, nr_pages,
+					(efi_physical_addr_t *)reserve_addr);
 		if (status != EFI_SUCCESS) {
-			pr_efi_err(sys_table, "Failed to relocate kernel\n");
-			return status;
+			kernel_memsize += TEXT_OFFSET;
+			status = efi_low_alloc(sys_table_arg, kernel_memsize,
+					       SZ_2M, reserve_addr);
+
+			if (status != EFI_SUCCESS) {
+				pr_efi_err(sys_table_arg, "Failed to relocate kernel\n");
+				return status;
+			}
+			*image_addr = *reserve_addr + TEXT_OFFSET;
 		}
-		memcpy((void *)*reserve_addr + TEXT_OFFSET, (void *)*image_addr,
-		       kernel_size);
-		*image_addr = *reserve_addr + TEXT_OFFSET;
-		*reserve_size = kernel_memsize + TEXT_OFFSET;
+		memcpy((void *)*image_addr, old_image_addr, kernel_size);
+		*reserve_size = kernel_memsize;
 	}
 
 
Signature · 1 line
1.9.1
reviewer
#1 lore raw

PATCH v2 1/1 arm64/efi: prefer AllocatePages() over efi_low_alloc() for vmlinux

Quoted 8 lines · Ard Biesheuvel

When allocating memory for the kernel image, try the AllocatePages() boot service to obtain memory at the preferred offset of 'dram_base + TEXT_OFFSET', and only revert to efi_low_alloc() if that fails. This is the only way to allocate at the base of DRAM if DRAM starts at 0x0, since efi_low_alloc() refuses to allocate at 0x0.

Mark.

Quoted 77 lines

---
v2:
- reshuffle code flow to make it more logical, and have only a single memcpy() invocation at the end of the function

---
 arch/arm64/kernel/efi-stub.c | 41 ++++++++++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 9 deletions(-)
arch/arm64/kernel/efi-stub.c +32 -9
index f5374065ad53..816120ece6bc 100644
@@ -13,7 +13,7 @@
 #include <asm/efi.h>
 #include <asm/sections.h>
 
-efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table,
+efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table_arg,
 					unsigned long *image_addr,
 					unsigned long *image_size,
 					unsigned long *reserve_addr,
@@ -23,21 +23,44 @@ efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table,
 {
 	efi_status_t status;
 	unsigned long kernel_size, kernel_memsize = 0;
+	unsigned long nr_pages;
+	void *old_image_addr = (void *)*image_addr;
 
 	/* Relocate the image, if required. */
 	kernel_size = _edata - _text;
 	if (*image_addr != (dram_base + TEXT_OFFSET)) {
 		kernel_memsize = kernel_size + (_end - _edata);
-		status = efi_low_alloc(sys_table, kernel_memsize + TEXT_OFFSET,
-				       SZ_2M, reserve_addr);
+
+		/*
+		 * First, try a straight allocation at the preferred offset.
+		 * This will work around the issue where, if dram_base == 0x0,
+		 * efi_low_alloc() refuses to allocate at 0x0 (to prevent the
+		 * address of the allocation to be mistaken for a FAIL return
+		 * value or a NULL pointer). It will also ensure that, on
+		 * platforms where the [dram_base, dram_base + TEXT_OFFSET)
+		 * interval is partially occupied by the firmware (like on APM
+		 * Mustang), we can still place the kernel at the address
+		 * 'dram_base + TEXT_OFFSET'.
+		 */
+		*image_addr = *reserve_addr = dram_base + TEXT_OFFSET;
+		nr_pages = round_up(kernel_memsize, EFI_ALLOC_ALIGN) /
+			   EFI_PAGE_SIZE;
+		status = efi_call_early(allocate_pages, EFI_ALLOCATE_ADDRESS,
+					EFI_LOADER_DATA, nr_pages,
+					(efi_physical_addr_t *)reserve_addr);
 		if (status != EFI_SUCCESS) {
-			pr_efi_err(sys_table, "Failed to relocate kernel\n");
-			return status;
+			kernel_memsize += TEXT_OFFSET;
+			status = efi_low_alloc(sys_table_arg, kernel_memsize,
+					       SZ_2M, reserve_addr);
+
+			if (status != EFI_SUCCESS) {
+				pr_efi_err(sys_table_arg, "Failed to relocate kernel\n");
+				return status;
+			}
+			*image_addr = *reserve_addr + TEXT_OFFSET;
 		}
-		memcpy((void *)*reserve_addr + TEXT_OFFSET, (void *)*image_addr,
-		       kernel_size);
-		*image_addr = *reserve_addr + TEXT_OFFSET;
-		*reserve_size = kernel_memsize + TEXT_OFFSET;
+		memcpy((void *)*image_addr, old_image_addr, kernel_size);
+		*reserve_size = kernel_memsize;
 	}
 
 
Signature · 1 line
1.9.1
#2 lore raw

PATCH v2 1/1 arm64/efi: prefer AllocatePages() over efi_low_alloc() for vmlinux

In reply to Ard Biesheuvel
Quoted 8 lines · Ard Biesheuvel

When allocating memory for the kernel image, try the AllocatePages() boot service to obtain memory at the preferred offset of 'dram_base + TEXT_OFFSET', and only revert to efi_low_alloc() if that fails. This is the only way to allocate at the base of DRAM if DRAM starts at 0x0, since efi_low_alloc() refuses to allocate at 0x0.

Quoted 7 lines

---
v2:
- reshuffle code flow to make it more logical, and have only a single memcpy() invocation at the end of the function

---
 arch/arm64/kernel/efi-stub.c | 41 ++++++++++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 9 deletions(-)

Would it be easier if we allow efi_low_alloc() to return 0x0 for some uses? If you don't need the preference for low allocations, probably not, but I don't want to see us working around limitations in efi_low_alloc() instead of just fixing it.

Signature · 1 line
Matt Fleming, Intel Open Source Technology Center
#3 lore raw

PATCH v2 1/1 arm64/efi: prefer AllocatePages() over efi_low_alloc() for vmlinux

Quoted 17 lines · Matt Fleming
Quoted 8 lines · Ard Biesheuvel

When allocating memory for the kernel image, try the AllocatePages() boot service to obtain memory at the preferred offset of 'dram_base + TEXT_OFFSET', and only revert to efi_low_alloc() if that fails. This is the only way to allocate at the base of DRAM if DRAM starts at 0x0, since efi_low_alloc() refuses to allocate at 0x0.

Quoted 7 lines

---
v2:
- reshuffle code flow to make it more logical, and have only a single memcpy() invocation at the end of the function

---
 arch/arm64/kernel/efi-stub.c | 41 ++++++++++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 9 deletions(-)
Quoted 5 lines

Would it be easier if we allow efi_low_alloc() to return 0x0 for some uses? If you don't need the preference for low allocations, probably not, but I don't want to see us working around limitations in efi_low_alloc() instead of just fixing it.

This workaround fixes another issue as well: the arm64 kernel needs to be loaded 512 KB above a 2MB aligned boundary, and using efi_low_alloc() as we do loses (2 MB - 512 KB) at the bottom if part of that 512 KB is occupied, since efi_low_alloc() is not aware of the fact that the first 512 KB will remain unused.

What would be most helpful is if efi_low_alloc() could take an offset param in addition to the alignment, i.e., alignment == 2MB and offset == 512 KB. The offset would default to 0, reverting to the original behavior.

If you'd be ok with such a change, I can propose it instead, and wire it up into this function.

Signature · 1 line
Ard.
#4 lore raw

PATCH v2 1/1 arm64/efi: prefer AllocatePages() over efi_low_alloc() for vmlinux

Quoted 7 lines · Ard Biesheuvel

This workaround fixes another issue as well: the arm64 kernel needs to be loaded 512 KB above a 2MB aligned boundary, and using efi_low_alloc() as we do loses (2 MB - 512 KB) at the bottom if part of that 512 KB is occupied, since efi_low_alloc() is not aware of the fact that the first 512 KB will remain unused.

Quoted 7 lines

What would be most helpful is if efi_low_alloc() could take an offset param in addition to the alignment, i.e., alignment == 2MB and offset == 512 KB. The offset would default to 0, reverting to the original behavior.

If you'd be ok with such a change, I can propose it instead, and wire it up into this function.

It's probably because it's late but I'm having trouble thinking this change through fully.

If it's not too much work, sure, please go ahead and propose a patch, even the untested, uncompiled "this is what I'm thinking" type.

Signature · 1 line
Matt Fleming, Intel Open Source Technology Center
#5 lore raw

PATCH v2 1/1 arm64/efi: prefer AllocatePages() over efi_low_alloc() for vmlinux

In reply to Ard Biesheuvel
Quoted 30 lines · Ard Biesheuvel
Quoted 17 lines · Matt Fleming
Quoted 8 lines · Ard Biesheuvel

When allocating memory for the kernel image, try the AllocatePages() boot service to obtain memory at the preferred offset of 'dram_base + TEXT_OFFSET', and only revert to efi_low_alloc() if that fails. This is the only way to allocate at the base of DRAM if DRAM starts at 0x0, since efi_low_alloc() refuses to allocate at 0x0.

Quoted 7 lines

---
v2:
- reshuffle code flow to make it more logical, and have only a single memcpy() invocation at the end of the function

---
 arch/arm64/kernel/efi-stub.c | 41 ++++++++++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 9 deletions(-)
Quoted 5 lines

Would it be easier if we allow efi_low_alloc() to return 0x0 for some uses? If you don't need the preference for low allocations, probably not, but I don't want to see us working around limitations in efi_low_alloc() instead of just fixing it.

This workaround fixes another issue as well: the arm64 kernel needs to be loaded 512 KB above a 2MB aligned boundary, and using efi_low_alloc() as we do loses (2 MB - 512 KB) at the bottom if part of that 512 KB is occupied, since efi_low_alloc() is not aware of the fact that the first 512 KB will remain unused.

Quoted 7 lines

What would be most helpful is if efi_low_alloc() could take an offset param in addition to the alignment, i.e., alignment == 2MB and offset == 512 KB. The offset would default to 0, reverting to the original behavior.

If you'd be ok with such a change, I can propose it instead, and wire it up into this function.

I already merged the original patch, so if you propose anything extra, please do it on top of that!

Will

#6 lore raw

PATCH v2 1/1 arm64/efi: prefer AllocatePages() over efi_low_alloc() for vmlinux

In reply to Ard Biesheuvel
Quoted 8 lines · Ard Biesheuvel
arch/arm64/kernel/efi-stub.c +7 -0
+               /*
+                * First, try a straight allocation at the preferred offset.
+                * This will work around the issue where, if dram_base == 0x0,
+                * efi_low_alloc() refuses to allocate at 0x0 (to prevent the
+                * address of the allocation to be mistaken for a FAIL return
+                * value or a NULL pointer). It will also ensure that, on
+                * platforms where the [dram_base, dram_base + TEXT_OFFSET)

arch/arm64/kernel/efi-stub.c efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table, · line 50

Quoted 10 lines
arch/arm64/kernel/efi-stub.c +10 -0
+                * interval is partially occupied by the firmware (like on APM
+                * Mustang), we can still place the kernel at the address
+                * 'dram_base + TEXT_OFFSET'.
+                */
+               *image_addr = *reserve_addr = dram_base + TEXT_OFFSET;
+               nr_pages = round_up(kernel_memsize, EFI_ALLOC_ALIGN) /
+                          EFI_PAGE_SIZE;
+               status = efi_call_early(allocate_pages, EFI_ALLOCATE_ADDRESS,
+                                       EFI_LOADER_DATA, nr_pages,
+                                       (efi_physical_addr_t *)reserve_addr);

This causes our kernel to crash, because on our system, dram_base is not 2MB aligned. I'll be posting a patch soon that fixes this.

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.