[PATCH v2 0/8] efi/libstub: simplify arm64 kernel image loading

[PATCH v2 0/8] efi/libstub: simplify arm64 kernel image loading

[Ard Biesheuvel]

On arm64, the kernel image used to be virtually mapped via the linear
region, making the two mappings correlated in a way that required the
kernel to be located at the start of the linear region, or the memory
below would not be accessible. For this reason, the EFI stub loader
code for arm64 has the notion of a 'preferred offset' for the physical
placement of the kernel image, and tries to put the kernel there, or
at least as low as possible in physical memory (unless KASLR is active,
in which case the placement is randomized)

When KASLR was introduced, the virtual mapping of the kernel was moved
into the vmalloc region, and now, regardless of whether KASLR support
is built in or active, the kernel can be placed anywhere in physical
memory without any detrimental side effects on the linear region.

This means that we can drop the notion of 'preferred offset' entirely,
and invoke the kernel in place if the PE/COFF loader loaded it at the
right offset. If not, we can invoke the ordinary UEFI top down page
allocator to reallocate it elsewhere in memory. By updating the PE/COFF
metadata, we can inform the PE/COFF loader about the desired alignment,
making it less likely that we need to move the kernel image in the first
place.

Ard Biesheuvel (8):
  efi/libstub/random: align allocate size to EFI_ALLOC_ALIGN
  efi/libstub/random: increase random alloc granularity
  efi/libstub/arm64: replace 'preferred' offset with alignment check
  efi/libstub/arm64: simplify randomized loading of kernel image
  efi/libstub/arm64: align PE/COFF sections to segment alignment
  efi/libstub: add API function to allocate aligned memory
  efi/libstub/arm64: switch to ordinary page allocator for kernel image
  efi/libstub: move efi_relocate_kernel() into separate source file

 arch/arm64/kernel/efi-header.S             |   2 +-
 arch/arm64/kernel/vmlinux.lds.S            |   3 +-
 drivers/firmware/efi/libstub/Makefile      |   3 +-
 drivers/firmware/efi/libstub/alignedmem.c  |  57 ++++++
 drivers/firmware/efi/libstub/arm64-stub.c  |  92 +++-------
 drivers/firmware/efi/libstub/efistub.h     |  18 +-
 drivers/firmware/efi/libstub/mem.c         | 191 +-------------------
 drivers/firmware/efi/libstub/randomalloc.c |   6 +-
 drivers/firmware/efi/libstub/relocate.c    | 174 ++++++++++++++++++
 9 files changed, 280 insertions(+), 266 deletions(-)
 create mode 100644 drivers/firmware/efi/libstub/alignedmem.c
 create mode 100644 drivers/firmware/efi/libstub/relocate.c

-- 
2.17.1

[PATCH v2 0/8] efi/libstub: simplify arm64 kernel image loading

[Ard Biesheuvel]

On arm64, the kernel image used to be virtually mapped via the linear
region, making the two mappings correlated in a way that required the
kernel to be located at the start of the linear region, or the memory
below would not be accessible. For this reason, the EFI stub loader
code for arm64 has the notion of a 'preferred offset' for the physical
placement of the kernel image, and tries to put the kernel there, or
at least as low as possible in physical memory (unless KASLR is active,
in which case the placement is randomized)

When KASLR was introduced, the virtual mapping of the kernel was moved
into the vmalloc region, and now, regardless of whether KASLR support
is built in or active, the kernel can be placed anywhere in physical
memory without any detrimental side effects on the linear region.

This means that we can drop the notion of 'preferred offset' entirely,
and invoke the kernel in place if the PE/COFF loader loaded it at the
right offset. If not, we can invoke the ordinary UEFI top down page
allocator to reallocate it elsewhere in memory. By updating the PE/COFF
metadata, we can inform the PE/COFF loader about the desired alignment,
making it less likely that we need to move the kernel image in the first
place.

Ard Biesheuvel (8):
  efi/libstub/random: align allocate size to EFI_ALLOC_ALIGN
  efi/libstub/random: increase random alloc granularity
  efi/libstub/arm64: replace 'preferred' offset with alignment check
  efi/libstub/arm64: simplify randomized loading of kernel image
  efi/libstub/arm64: align PE/COFF sections to segment alignment
  efi/libstub: add API function to allocate aligned memory
  efi/libstub/arm64: switch to ordinary page allocator for kernel image
  efi/libstub: move efi_relocate_kernel() into separate source file

 arch/arm64/kernel/efi-header.S             |   2 +-
 arch/arm64/kernel/vmlinux.lds.S            |   3 +-
 drivers/firmware/efi/libstub/Makefile      |   3 +-
 drivers/firmware/efi/libstub/alignedmem.c  |  57 ++++++
 drivers/firmware/efi/libstub/arm64-stub.c  |  92 +++-------
 drivers/firmware/efi/libstub/efistub.h     |  18 +-
 drivers/firmware/efi/libstub/mem.c         | 191 +-------------------
 drivers/firmware/efi/libstub/randomalloc.c |   6 +-
 drivers/firmware/efi/libstub/relocate.c    | 174 ++++++++++++++++++
 9 files changed, 280 insertions(+), 266 deletions(-)
 create mode 100644 drivers/firmware/efi/libstub/alignedmem.c
 create mode 100644 drivers/firmware/efi/libstub/relocate.c

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/8] efi/libstub/random: align allocate size to EFI_ALLOC_ALIGN

[Ard Biesheuvel]

The EFI stub uses a per-architecture #define for the minimum base
and size alignment of page allocations, which is set to 4 KB for
all architecures except arm64, which uses 64 KB, to ensure that
allocations can always be (un)mapped efficiently, regardless of
the page size used by the kernel proper, which could be a kexec'ee

The API wrappers around page based allocations assume that this
alignment is always taken into account, and so efi_free() will
also round up its size argument to EFI_ALLOC_ALIGN.

Currently, efi_random_alloc() does not honour this alignment for
the allocated size, and so freeing such an allocation may result
in unrelated memory to be freed, potentially leading to issues
after boot. So let's round up size in efi_random_alloc() as well.

Fixes: 2ddbfc81eac84a29 ("efi: stub: add implementation of efi_random_alloc()")
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/randomalloc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/randomalloc.c b/drivers/firmware/efi/libstub/randomalloc.c
index 4578f59e160c..6200dfa650f5 100644
--- a/drivers/firmware/efi/libstub/randomalloc.c
+++ b/drivers/firmware/efi/libstub/randomalloc.c
@@ -74,6 +74,8 @@ efi_status_t efi_random_alloc(unsigned long size,
 	if (align < EFI_ALLOC_ALIGN)
 		align = EFI_ALLOC_ALIGN;
 
+	size = round_up(size, EFI_ALLOC_ALIGN);
+
 	/* count the suitable slots in each memory map entry */
 	for (map_offset = 0; map_offset < map_size; map_offset += desc_size) {
 		efi_memory_desc_t *md = (void *)memory_map + map_offset;
@@ -109,7 +111,7 @@ efi_status_t efi_random_alloc(unsigned long size,
 		}
 
 		target = round_up(md->phys_addr, align) + target_slot * align;
-		pages = round_up(size, EFI_PAGE_SIZE) / EFI_PAGE_SIZE;
+		pages = size / EFI_PAGE_SIZE;
 
 		status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
 				     EFI_LOADER_DATA, pages, &target);
-- 
2.17.1

[PATCH v2 1/8] efi/libstub/random: align allocate size to EFI_ALLOC_ALIGN

[Ard Biesheuvel]

The EFI stub uses a per-architecture #define for the minimum base
and size alignment of page allocations, which is set to 4 KB for
all architecures except arm64, which uses 64 KB, to ensure that
allocations can always be (un)mapped efficiently, regardless of
the page size used by the kernel proper, which could be a kexec'ee

The API wrappers around page based allocations assume that this
alignment is always taken into account, and so efi_free() will
also round up its size argument to EFI_ALLOC_ALIGN.

Currently, efi_random_alloc() does not honour this alignment for
the allocated size, and so freeing such an allocation may result
in unrelated memory to be freed, potentially leading to issues
after boot. So let's round up size in efi_random_alloc() as well.

Fixes: 2ddbfc81eac84a29 ("efi: stub: add implementation of efi_random_alloc()")
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/randomalloc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/randomalloc.c b/drivers/firmware/efi/libstub/randomalloc.c
index 4578f59e160c..6200dfa650f5 100644
--- a/drivers/firmware/efi/libstub/randomalloc.c
+++ b/drivers/firmware/efi/libstub/randomalloc.c
@@ -74,6 +74,8 @@ efi_status_t efi_random_alloc(unsigned long size,
 	if (align < EFI_ALLOC_ALIGN)
 		align = EFI_ALLOC_ALIGN;
 
+	size = round_up(size, EFI_ALLOC_ALIGN);
+
 	/* count the suitable slots in each memory map entry */
 	for (map_offset = 0; map_offset < map_size; map_offset += desc_size) {
 		efi_memory_desc_t *md = (void *)memory_map + map_offset;
@@ -109,7 +111,7 @@ efi_status_t efi_random_alloc(unsigned long size,
 		}
 
 		target = round_up(md->phys_addr, align) + target_slot * align;
-		pages = round_up(size, EFI_PAGE_SIZE) / EFI_PAGE_SIZE;
+		pages = size / EFI_PAGE_SIZE;
 
 		status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
 				     EFI_LOADER_DATA, pages, &target);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/8] efi/libstub/random: increase random alloc granularity

[Ard Biesheuvel]

The implementation of efi_random_alloc() arbitrarily truncates the
provided random seed to 16 bits, which limits the granularity of the
randomly chosen allocation offset in memory. This is currently only
an issue if the size of physical memory exceeds 128 GB, but going
forward, we will reduce the allocation alignment to 64 KB, and this
means we need to increase the granularity to ensure that the random
memory allocations are distributed evenly.

We will need to switch to 64-bit arithmetic for the multiplication,
but this does not result in 64-bit integer intrinsic calls on ARM or
on i386.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/randomalloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/randomalloc.c b/drivers/firmware/efi/libstub/randomalloc.c
index 6200dfa650f5..a408df474d83 100644
--- a/drivers/firmware/efi/libstub/randomalloc.c
+++ b/drivers/firmware/efi/libstub/randomalloc.c
@@ -87,7 +87,7 @@ efi_status_t efi_random_alloc(unsigned long size,
 	}
 
 	/* find a random number between 0 and total_slots */
-	target_slot = (total_slots * (u16)random_seed) >> 16;
+	target_slot = (total_slots * (u64)(random_seed & U32_MAX)) >> 32;
 
 	/*
 	 * target_slot is now a value in the range [0, total_slots), and so
-- 
2.17.1

[PATCH v2 2/8] efi/libstub/random: increase random alloc granularity

[Ard Biesheuvel]

The implementation of efi_random_alloc() arbitrarily truncates the
provided random seed to 16 bits, which limits the granularity of the
randomly chosen allocation offset in memory. This is currently only
an issue if the size of physical memory exceeds 128 GB, but going
forward, we will reduce the allocation alignment to 64 KB, and this
means we need to increase the granularity to ensure that the random
memory allocations are distributed evenly.

We will need to switch to 64-bit arithmetic for the multiplication,
but this does not result in 64-bit integer intrinsic calls on ARM or
on i386.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/randomalloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/randomalloc.c b/drivers/firmware/efi/libstub/randomalloc.c
index 6200dfa650f5..a408df474d83 100644
--- a/drivers/firmware/efi/libstub/randomalloc.c
+++ b/drivers/firmware/efi/libstub/randomalloc.c
@@ -87,7 +87,7 @@ efi_status_t efi_random_alloc(unsigned long size,
 	}
 
 	/* find a random number between 0 and total_slots */
-	target_slot = (total_slots * (u16)random_seed) >> 16;
+	target_slot = (total_slots * (u64)(random_seed & U32_MAX)) >> 32;
 
 	/*
 	 * target_slot is now a value in the range [0, total_slots), and so
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 3/8] efi/libstub/arm64: replace 'preferred' offset with alignment check

[Ard Biesheuvel]

The notion of a 'preferred' load offset for the kernel dates back to the
times when the kernel's primary mapping overlapped with the linear region,
and memory below it could not be used at all.

Today, the arm64 kernel does not really care where it is loaded in physical
memory, as long as the alignment requirements are met, and so there is no
point in unconditionally moving the kernel to a new location in memory at
boot. Instead, we can
- check for a KASLR seed, and randomly reallocate the kernel if one is
  provided
- otherwise, check whether the alignment requirements are met for the
  current placement of the kernel, and just run it in place if they are
- finally, do an ordinary page allocation and reallocate the kernel to a
  suitably aligned buffer anywhere in memory.

By the same reasoning, there is no need to take TEXT_OFFSET into account
if it is a round multiple of the minimum alignment, which is the usual
case for relocatable kernels with TEXT_OFFSET randomization disabled.
Otherwise, it suffices to use the relative misaligment of TEXT_OFFSET
when reallocating the kernel.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/arm64-stub.c | 62 ++++++++------------
 1 file changed, 25 insertions(+), 37 deletions(-)

diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index fc9f8ab533a7..cfd535c13242 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -34,6 +34,15 @@ efi_status_t check_platform_features(void)
 	return EFI_SUCCESS;
 }
 
+/*
+ * Relocatable kernels can fix up the misalignment with respect to
+ * MIN_KIMG_ALIGN, so they only require a minimum alignment of EFI_KIMG_ALIGN
+ * (which accounts for the alignment of statically allocated objects such as
+ * the swapper stack.)
+ */
+static const u64 min_kimg_align = IS_ENABLED(CONFIG_RELOCATABLE) ? EFI_KIMG_ALIGN
+								 : MIN_KIMG_ALIGN;
+
 efi_status_t handle_kernel_image(unsigned long *image_addr,
 				 unsigned long *image_size,
 				 unsigned long *reserve_addr,
@@ -43,7 +52,6 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 {
 	efi_status_t status;
 	unsigned long kernel_size, kernel_memsize = 0;
-	unsigned long preferred_offset;
 	u64 phys_seed = 0;
 
 	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
@@ -61,14 +69,8 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 		}
 	}
 
-	/*
-	 * 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, MIN_KIMG_ALIGN) + TEXT_OFFSET;
-	if (preferred_offset < dram_base)
-		preferred_offset += MIN_KIMG_ALIGN;
+	if (image->image_base != _text)
+		pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n");
 
 	kernel_size = _edata - _text;
 	kernel_memsize = kernel_size + (_end - _edata);
@@ -103,46 +105,32 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 
 		*image_addr = *reserve_addr + offset;
 	} else {
-		/*
-		 * Else, 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 = (unsigned long)_text;
-		if (*image_addr == preferred_offset)
-			return EFI_SUCCESS;
-
-		*image_addr = *reserve_addr = preferred_offset;
-		*reserve_size = round_up(kernel_memsize, EFI_ALLOC_ALIGN);
-
-		status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
-				     EFI_LOADER_DATA,
-				     *reserve_size / EFI_PAGE_SIZE,
-				     (efi_physical_addr_t *)reserve_addr);
+		status = EFI_OUT_OF_RESOURCES;
 	}
 
 	if (status != EFI_SUCCESS) {
-		*reserve_size = kernel_memsize + TEXT_OFFSET;
+		if (IS_ALIGNED((u64)_text - TEXT_OFFSET, min_kimg_align)) {
+			/*
+			 * Just execute from wherever we were loaded by the
+			 * UEFI PE/COFF loader if the alignment is suitable.
+			 */
+			*image_addr = (u64)_text;
+			*reserve_size = 0;
+			return EFI_SUCCESS;
+		}
+
+		*reserve_size = kernel_memsize + TEXT_OFFSET % min_kimg_align;
 		status = efi_low_alloc(*reserve_size,
-				       MIN_KIMG_ALIGN, reserve_addr);
+				       min_kimg_align, reserve_addr);
 
 		if (status != EFI_SUCCESS) {
 			pr_efi_err("Failed to relocate kernel\n");
 			*reserve_size = 0;
 			return status;
 		}
-		*image_addr = *reserve_addr + TEXT_OFFSET;
+		*image_addr = *reserve_addr + TEXT_OFFSET % min_kimg_align;
 	}
 
-	if (image->image_base != _text)
-		pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n");
-
 	memcpy((void *)*image_addr, _text, kernel_size);
 
 	return EFI_SUCCESS;
-- 
2.17.1

[PATCH v2 3/8] efi/libstub/arm64: replace 'preferred' offset with alignment check

[Ard Biesheuvel]

The notion of a 'preferred' load offset for the kernel dates back to the
times when the kernel's primary mapping overlapped with the linear region,
and memory below it could not be used at all.

Today, the arm64 kernel does not really care where it is loaded in physical
memory, as long as the alignment requirements are met, and so there is no
point in unconditionally moving the kernel to a new location in memory at
boot. Instead, we can
- check for a KASLR seed, and randomly reallocate the kernel if one is
  provided
- otherwise, check whether the alignment requirements are met for the
  current placement of the kernel, and just run it in place if they are
- finally, do an ordinary page allocation and reallocate the kernel to a
  suitably aligned buffer anywhere in memory.

By the same reasoning, there is no need to take TEXT_OFFSET into account
if it is a round multiple of the minimum alignment, which is the usual
case for relocatable kernels with TEXT_OFFSET randomization disabled.
Otherwise, it suffices to use the relative misaligment of TEXT_OFFSET
when reallocating the kernel.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/arm64-stub.c | 62 ++++++++------------
 1 file changed, 25 insertions(+), 37 deletions(-)

diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index fc9f8ab533a7..cfd535c13242 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -34,6 +34,15 @@ efi_status_t check_platform_features(void)
 	return EFI_SUCCESS;
 }
 
+/*
+ * Relocatable kernels can fix up the misalignment with respect to
+ * MIN_KIMG_ALIGN, so they only require a minimum alignment of EFI_KIMG_ALIGN
+ * (which accounts for the alignment of statically allocated objects such as
+ * the swapper stack.)
+ */
+static const u64 min_kimg_align = IS_ENABLED(CONFIG_RELOCATABLE) ? EFI_KIMG_ALIGN
+								 : MIN_KIMG_ALIGN;
+
 efi_status_t handle_kernel_image(unsigned long *image_addr,
 				 unsigned long *image_size,
 				 unsigned long *reserve_addr,
@@ -43,7 +52,6 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 {
 	efi_status_t status;
 	unsigned long kernel_size, kernel_memsize = 0;
-	unsigned long preferred_offset;
 	u64 phys_seed = 0;
 
 	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
@@ -61,14 +69,8 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 		}
 	}
 
-	/*
-	 * 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, MIN_KIMG_ALIGN) + TEXT_OFFSET;
-	if (preferred_offset < dram_base)
-		preferred_offset += MIN_KIMG_ALIGN;
+	if (image->image_base != _text)
+		pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n");
 
 	kernel_size = _edata - _text;
 	kernel_memsize = kernel_size + (_end - _edata);
@@ -103,46 +105,32 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 
 		*image_addr = *reserve_addr + offset;
 	} else {
-		/*
-		 * Else, 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 = (unsigned long)_text;
-		if (*image_addr == preferred_offset)
-			return EFI_SUCCESS;
-
-		*image_addr = *reserve_addr = preferred_offset;
-		*reserve_size = round_up(kernel_memsize, EFI_ALLOC_ALIGN);
-
-		status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
-				     EFI_LOADER_DATA,
-				     *reserve_size / EFI_PAGE_SIZE,
-				     (efi_physical_addr_t *)reserve_addr);
+		status = EFI_OUT_OF_RESOURCES;
 	}
 
 	if (status != EFI_SUCCESS) {
-		*reserve_size = kernel_memsize + TEXT_OFFSET;
+		if (IS_ALIGNED((u64)_text - TEXT_OFFSET, min_kimg_align)) {
+			/*
+			 * Just execute from wherever we were loaded by the
+			 * UEFI PE/COFF loader if the alignment is suitable.
+			 */
+			*image_addr = (u64)_text;
+			*reserve_size = 0;
+			return EFI_SUCCESS;
+		}
+
+		*reserve_size = kernel_memsize + TEXT_OFFSET % min_kimg_align;
 		status = efi_low_alloc(*reserve_size,
-				       MIN_KIMG_ALIGN, reserve_addr);
+				       min_kimg_align, reserve_addr);
 
 		if (status != EFI_SUCCESS) {
 			pr_efi_err("Failed to relocate kernel\n");
 			*reserve_size = 0;
 			return status;
 		}
-		*image_addr = *reserve_addr + TEXT_OFFSET;
+		*image_addr = *reserve_addr + TEXT_OFFSET % min_kimg_align;
 	}
 
-	if (image->image_base != _text)
-		pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n");
-
 	memcpy((void *)*image_addr, _text, kernel_size);
 
 	return EFI_SUCCESS;
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 4/8] efi/libstub/arm64: simplify randomized loading of kernel image

[Ard Biesheuvel]

The KASLR code path in the arm64 version of the EFI stub incorporates
some overly complicated logic to randomly allocate a region of the right
alignment: there is no need to randomize the placement of the kernel
modulo 2 MiB separately from the placement of the 2 MiB aligned allocation
itself - we can simply follow the same logic used by the non-randomized
placement, which is to allocate at the correct alignment, and only take
TEXT_OFFSET into account if it is not a round multiple of the alignment.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/arm64-stub.c | 32 +++-----------------
 1 file changed, 5 insertions(+), 27 deletions(-)

diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index cfd535c13242..6fc3bd9a56db 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -52,7 +52,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 {
 	efi_status_t status;
 	unsigned long kernel_size, kernel_memsize = 0;
-	u64 phys_seed = 0;
+	u32 phys_seed = 0;
 
 	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
 		if (!nokaslr()) {
@@ -74,36 +74,15 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 
 	kernel_size = _edata - _text;
 	kernel_memsize = kernel_size + (_end - _edata);
+	*reserve_size = kernel_memsize + TEXT_OFFSET % min_kimg_align;
 
 	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE) && phys_seed != 0) {
-		/*
-		 * Produce a displacement in the interval [0, MIN_KIMG_ALIGN)
-		 * that doesn't violate this kernel's de-facto alignment
-		 * constraints.
-		 */
-		u32 mask = (MIN_KIMG_ALIGN - 1) & ~(EFI_KIMG_ALIGN - 1);
-		u32 offset = (phys_seed >> 32) & mask;
-
-		/*
-		 * With CONFIG_RANDOMIZE_TEXT_OFFSET=y, TEXT_OFFSET may not
-		 * be a multiple of EFI_KIMG_ALIGN, and we must ensure that
-		 * we preserve the misalignment of 'offset' relative to
-		 * EFI_KIMG_ALIGN so that statically allocated objects whose
-		 * alignment exceeds PAGE_SIZE appear correctly aligned in
-		 * memory.
-		 */
-		offset |= TEXT_OFFSET % EFI_KIMG_ALIGN;
-
 		/*
 		 * If KASLR is enabled, and we have some randomness available,
 		 * locate the kernel at a randomized offset in physical memory.
 		 */
-		*reserve_size = kernel_memsize + offset;
-		status = efi_random_alloc(*reserve_size,
-					  MIN_KIMG_ALIGN, reserve_addr,
-					  (u32)phys_seed);
-
-		*image_addr = *reserve_addr + offset;
+		status = efi_random_alloc(*reserve_size, min_kimg_align,
+					  reserve_addr, phys_seed);
 	} else {
 		status = EFI_OUT_OF_RESOURCES;
 	}
@@ -119,7 +98,6 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 			return EFI_SUCCESS;
 		}
 
-		*reserve_size = kernel_memsize + TEXT_OFFSET % min_kimg_align;
 		status = efi_low_alloc(*reserve_size,
 				       min_kimg_align, reserve_addr);
 
@@ -128,9 +106,9 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 			*reserve_size = 0;
 			return status;
 		}
-		*image_addr = *reserve_addr + TEXT_OFFSET % min_kimg_align;
 	}
 
+	*image_addr = *reserve_addr + TEXT_OFFSET % min_kimg_align;
 	memcpy((void *)*image_addr, _text, kernel_size);
 
 	return EFI_SUCCESS;
-- 
2.17.1

[PATCH v2 4/8] efi/libstub/arm64: simplify randomized loading of kernel image

[Ard Biesheuvel]

The KASLR code path in the arm64 version of the EFI stub incorporates
some overly complicated logic to randomly allocate a region of the right
alignment: there is no need to randomize the placement of the kernel
modulo 2 MiB separately from the placement of the 2 MiB aligned allocation
itself - we can simply follow the same logic used by the non-randomized
placement, which is to allocate at the correct alignment, and only take
TEXT_OFFSET into account if it is not a round multiple of the alignment.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/arm64-stub.c | 32 +++-----------------
 1 file changed, 5 insertions(+), 27 deletions(-)

diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index cfd535c13242..6fc3bd9a56db 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -52,7 +52,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 {
 	efi_status_t status;
 	unsigned long kernel_size, kernel_memsize = 0;
-	u64 phys_seed = 0;
+	u32 phys_seed = 0;
 
 	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
 		if (!nokaslr()) {
@@ -74,36 +74,15 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 
 	kernel_size = _edata - _text;
 	kernel_memsize = kernel_size + (_end - _edata);
+	*reserve_size = kernel_memsize + TEXT_OFFSET % min_kimg_align;
 
 	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE) && phys_seed != 0) {
-		/*
-		 * Produce a displacement in the interval [0, MIN_KIMG_ALIGN)
-		 * that doesn't violate this kernel's de-facto alignment
-		 * constraints.
-		 */
-		u32 mask = (MIN_KIMG_ALIGN - 1) & ~(EFI_KIMG_ALIGN - 1);
-		u32 offset = (phys_seed >> 32) & mask;
-
-		/*
-		 * With CONFIG_RANDOMIZE_TEXT_OFFSET=y, TEXT_OFFSET may not
-		 * be a multiple of EFI_KIMG_ALIGN, and we must ensure that
-		 * we preserve the misalignment of 'offset' relative to
-		 * EFI_KIMG_ALIGN so that statically allocated objects whose
-		 * alignment exceeds PAGE_SIZE appear correctly aligned in
-		 * memory.
-		 */
-		offset |= TEXT_OFFSET % EFI_KIMG_ALIGN;
-
 		/*
 		 * If KASLR is enabled, and we have some randomness available,
 		 * locate the kernel at a randomized offset in physical memory.
 		 */
-		*reserve_size = kernel_memsize + offset;
-		status = efi_random_alloc(*reserve_size,
-					  MIN_KIMG_ALIGN, reserve_addr,
-					  (u32)phys_seed);
-
-		*image_addr = *reserve_addr + offset;
+		status = efi_random_alloc(*reserve_size, min_kimg_align,
+					  reserve_addr, phys_seed);
 	} else {
 		status = EFI_OUT_OF_RESOURCES;
 	}
@@ -119,7 +98,6 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 			return EFI_SUCCESS;
 		}
 
-		*reserve_size = kernel_memsize + TEXT_OFFSET % min_kimg_align;
 		status = efi_low_alloc(*reserve_size,
 				       min_kimg_align, reserve_addr);
 
@@ -128,9 +106,9 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 			*reserve_size = 0;
 			return status;
 		}
-		*image_addr = *reserve_addr + TEXT_OFFSET % min_kimg_align;
 	}
 
+	*image_addr = *reserve_addr + TEXT_OFFSET % min_kimg_align;
 	memcpy((void *)*image_addr, _text, kernel_size);
 
 	return EFI_SUCCESS;
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 5/8] efi/libstub/arm64: align PE/COFF sections to segment alignment

[Ard Biesheuvel]

The arm64 kernel's segment alignment is fixed at 64 KB for any page
size, and relocatable kernels are able to fix up any misalignment of
the kernel image with respect to the 2 MB section alignment that is
mandated by the arm64 boot protocol.

Let's increase the PE/COFF section alignment to the same value, so that
kernels loaded by the UEFI PE/COFF loader are guaranteed to end up at
an address that doesn't require any reallocation to be done if the
kernel is relocatable.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/kernel/efi-header.S  | 2 +-
 arch/arm64/kernel/vmlinux.lds.S | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/efi-header.S b/arch/arm64/kernel/efi-header.S
index 914999ccaf8a..6f58998ef647 100644
--- a/arch/arm64/kernel/efi-header.S
+++ b/arch/arm64/kernel/efi-header.S
@@ -32,7 +32,7 @@ optional_header:
 
 extra_header_fields:
 	.quad	0					// ImageBase
-	.long	SZ_4K					// SectionAlignment
+	.long	SEGMENT_ALIGN				// SectionAlignment
 	.long	PECOFF_FILE_ALIGNMENT			// FileAlignment
 	.short	0					// MajorOperatingSystemVersion
 	.short	0					// MinorOperatingSystemVersion
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index 497f9675071d..1d399db0644f 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -175,7 +175,7 @@ SECTIONS
 		*(.altinstr_replacement)
 	}
 
-	. = ALIGN(PAGE_SIZE);
+	. = ALIGN(SEGMENT_ALIGN);
 	__inittext_end = .;
 	__initdata_begin = .;
 
@@ -246,6 +246,7 @@ SECTIONS
 	. += INIT_DIR_SIZE;
 	init_pg_end = .;
 
+	. = ALIGN(SEGMENT_ALIGN);
 	__pecoff_data_size = ABSOLUTE(. - __initdata_begin);
 	_end = .;
 
-- 
2.17.1

[PATCH v2 5/8] efi/libstub/arm64: align PE/COFF sections to segment alignment

[Ard Biesheuvel]

The arm64 kernel's segment alignment is fixed at 64 KB for any page
size, and relocatable kernels are able to fix up any misalignment of
the kernel image with respect to the 2 MB section alignment that is
mandated by the arm64 boot protocol.

Let's increase the PE/COFF section alignment to the same value, so that
kernels loaded by the UEFI PE/COFF loader are guaranteed to end up at
an address that doesn't require any reallocation to be done if the
kernel is relocatable.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/kernel/efi-header.S  | 2 +-
 arch/arm64/kernel/vmlinux.lds.S | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/efi-header.S b/arch/arm64/kernel/efi-header.S
index 914999ccaf8a..6f58998ef647 100644
--- a/arch/arm64/kernel/efi-header.S
+++ b/arch/arm64/kernel/efi-header.S
@@ -32,7 +32,7 @@ optional_header:
 
 extra_header_fields:
 	.quad	0					// ImageBase
-	.long	SZ_4K					// SectionAlignment
+	.long	SEGMENT_ALIGN				// SectionAlignment
 	.long	PECOFF_FILE_ALIGNMENT			// FileAlignment
 	.short	0					// MajorOperatingSystemVersion
 	.short	0					// MinorOperatingSystemVersion
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index 497f9675071d..1d399db0644f 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -175,7 +175,7 @@ SECTIONS
 		*(.altinstr_replacement)
 	}
 
-	. = ALIGN(PAGE_SIZE);
+	. = ALIGN(SEGMENT_ALIGN);
 	__inittext_end = .;
 	__initdata_begin = .;
 
@@ -246,6 +246,7 @@ SECTIONS
 	. += INIT_DIR_SIZE;
 	init_pg_end = .;
 
+	. = ALIGN(SEGMENT_ALIGN);
 	__pecoff_data_size = ABSOLUTE(. - __initdata_begin);
 	_end = .;
 
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 6/8] efi/libstub: add API function to allocate aligned memory

[Ard Biesheuvel]

Break out the code to create an aligned page allocation from mem.c
and move it into a function efi_allocate_pages_aligned() in alignedmem.c.
Update efi_allocate_pages() to invoke it unless the minimum alignment
equals the EFI page size (4 KB), in which case the ordinary page
allocator is sufficient. This way, efi_allocate_pages_aligned() will
only be pulled into the build if it is actually being used (which will
be on arm64 only in the immediate future)

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/Makefile     |  3 +-
 drivers/firmware/efi/libstub/alignedmem.c | 57 ++++++++++++++++++++
 drivers/firmware/efi/libstub/efistub.h    |  3 ++
 drivers/firmware/efi/libstub/mem.c        | 25 ++++-----
 4 files changed, 71 insertions(+), 17 deletions(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 094eabdecfe6..1a09b9445394 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -42,7 +42,8 @@ KCOV_INSTRUMENT			:= n
 
 lib-y				:= efi-stub-helper.o gop.o secureboot.o tpm.o \
 				   file.o mem.o random.o randomalloc.o pci.o \
-				   skip_spaces.o lib-cmdline.o lib-ctype.o
+				   skip_spaces.o lib-cmdline.o lib-ctype.o \
+				   alignedmem.o
 
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
 arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
diff --git a/drivers/firmware/efi/libstub/alignedmem.c b/drivers/firmware/efi/libstub/alignedmem.c
new file mode 100644
index 000000000000..cc89c4d6196f
--- /dev/null
+++ b/drivers/firmware/efi/libstub/alignedmem.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/efi.h>
+#include <asm/efi.h>
+
+#include "efistub.h"
+
+/**
+ * efi_allocate_pages_aligned() - Allocate memory pages
+ * @size:	minimum number of bytes to allocate
+ * @addr:	On return the address of the first allocated page. The first
+ *		allocated page has alignment EFI_ALLOC_ALIGN which is an
+ *		architecture dependent multiple of the page size.
+ * @max:	the address that the last allocated memory page shall not
+ *		exceed
+ * @align:	minimum alignment of the base of the allocation
+ *
+ * Allocate pages as EFI_LOADER_DATA. The allocated pages are aligned according
+ * to @align, which should be >= EFI_ALLOC_ALIGN. The last allocated page will
+ * not exceed the address given by @max.
+ *
+ * Return:	status code
+ */
+efi_status_t efi_allocate_pages_aligned(unsigned long size, unsigned long *addr,
+					unsigned long max, unsigned long align)
+{
+	efi_physical_addr_t alloc_addr;
+	efi_status_t status;
+	int slack;
+
+	if (align < EFI_ALLOC_ALIGN)
+		align = EFI_ALLOC_ALIGN;
+
+	alloc_addr = ALIGN_DOWN(max + 1, align) - 1;
+	size = round_up(size, EFI_ALLOC_ALIGN);
+	slack = align / EFI_PAGE_SIZE - 1;
+
+	status = efi_bs_call(allocate_pages, EFI_ALLOCATE_MAX_ADDRESS,
+			     EFI_LOADER_DATA, size / EFI_PAGE_SIZE + slack,
+			     &alloc_addr);
+	if (status != EFI_SUCCESS)
+		return status;
+
+	*addr = ALIGN((unsigned long)alloc_addr, align);
+
+	if (slack > 0) {
+		int l = (alloc_addr % align) / EFI_PAGE_SIZE;
+
+		if (l) {
+			efi_bs_call(free_pages, alloc_addr, slack - l + 1);
+			slack = l - 1;
+		}
+		if (slack)
+			efi_bs_call(free_pages, *addr + size, slack);
+	}
+	return EFI_SUCCESS;
+}
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 4844c3bd40df..5dcfadcf2bc1 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -641,6 +641,9 @@ efi_status_t efi_low_alloc(unsigned long size, unsigned long align,
 efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
 				unsigned long max);
 
+efi_status_t efi_allocate_pages_aligned(unsigned long size, unsigned long *addr,
+					unsigned long max, unsigned long align);
+
 efi_status_t efi_relocate_kernel(unsigned long *image_addr,
 				 unsigned long image_size,
 				 unsigned long alloc_size,
diff --git a/drivers/firmware/efi/libstub/mem.c b/drivers/firmware/efi/libstub/mem.c
index 869a79c8946f..0020b0fa9587 100644
--- a/drivers/firmware/efi/libstub/mem.c
+++ b/drivers/firmware/efi/libstub/mem.c
@@ -93,31 +93,24 @@ efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
 efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
 				unsigned long max)
 {
-	efi_physical_addr_t alloc_addr = ALIGN_DOWN(max + 1, EFI_ALLOC_ALIGN) - 1;
-	int slack = EFI_ALLOC_ALIGN / EFI_PAGE_SIZE - 1;
+	efi_physical_addr_t alloc_addr;
 	efi_status_t status;
 
-	size = round_up(size, EFI_ALLOC_ALIGN);
+	if (EFI_ALLOC_ALIGN > EFI_PAGE_SIZE)
+		return efi_allocate_pages_aligned(size, addr, max,
+						  EFI_ALLOC_ALIGN);
+
+	alloc_addr = ALIGN_DOWN(max + 1, EFI_ALLOC_ALIGN) - 1;
 	status = efi_bs_call(allocate_pages, EFI_ALLOCATE_MAX_ADDRESS,
-			     EFI_LOADER_DATA, size / EFI_PAGE_SIZE + slack,
+			     EFI_LOADER_DATA, DIV_ROUND_UP(size, EFI_PAGE_SIZE),
 			     &alloc_addr);
 	if (status != EFI_SUCCESS)
 		return status;
 
-	*addr = ALIGN((unsigned long)alloc_addr, EFI_ALLOC_ALIGN);
-
-	if (slack > 0) {
-		int l = (alloc_addr % EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE;
-
-		if (l) {
-			efi_bs_call(free_pages, alloc_addr, slack - l + 1);
-			slack = l - 1;
-		}
-		if (slack)
-			efi_bs_call(free_pages, *addr + size, slack);
-	}
+	*addr = alloc_addr;
 	return EFI_SUCCESS;
 }
+
 /**
  * efi_low_alloc_above() - allocate pages at or above given address
  * @size:	size of the memory area to allocate
-- 
2.17.1

[PATCH v2 6/8] efi/libstub: add API function to allocate aligned memory

[Ard Biesheuvel]

Break out the code to create an aligned page allocation from mem.c
and move it into a function efi_allocate_pages_aligned() in alignedmem.c.
Update efi_allocate_pages() to invoke it unless the minimum alignment
equals the EFI page size (4 KB), in which case the ordinary page
allocator is sufficient. This way, efi_allocate_pages_aligned() will
only be pulled into the build if it is actually being used (which will
be on arm64 only in the immediate future)

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/Makefile     |  3 +-
 drivers/firmware/efi/libstub/alignedmem.c | 57 ++++++++++++++++++++
 drivers/firmware/efi/libstub/efistub.h    |  3 ++
 drivers/firmware/efi/libstub/mem.c        | 25 ++++-----
 4 files changed, 71 insertions(+), 17 deletions(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 094eabdecfe6..1a09b9445394 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -42,7 +42,8 @@ KCOV_INSTRUMENT			:= n
 
 lib-y				:= efi-stub-helper.o gop.o secureboot.o tpm.o \
 				   file.o mem.o random.o randomalloc.o pci.o \
-				   skip_spaces.o lib-cmdline.o lib-ctype.o
+				   skip_spaces.o lib-cmdline.o lib-ctype.o \
+				   alignedmem.o
 
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
 arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
diff --git a/drivers/firmware/efi/libstub/alignedmem.c b/drivers/firmware/efi/libstub/alignedmem.c
new file mode 100644
index 000000000000..cc89c4d6196f
--- /dev/null
+++ b/drivers/firmware/efi/libstub/alignedmem.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/efi.h>
+#include <asm/efi.h>
+
+#include "efistub.h"
+
+/**
+ * efi_allocate_pages_aligned() - Allocate memory pages
+ * @size:	minimum number of bytes to allocate
+ * @addr:	On return the address of the first allocated page. The first
+ *		allocated page has alignment EFI_ALLOC_ALIGN which is an
+ *		architecture dependent multiple of the page size.
+ * @max:	the address that the last allocated memory page shall not
+ *		exceed
+ * @align:	minimum alignment of the base of the allocation
+ *
+ * Allocate pages as EFI_LOADER_DATA. The allocated pages are aligned according
+ * to @align, which should be >= EFI_ALLOC_ALIGN. The last allocated page will
+ * not exceed the address given by @max.
+ *
+ * Return:	status code
+ */
+efi_status_t efi_allocate_pages_aligned(unsigned long size, unsigned long *addr,
+					unsigned long max, unsigned long align)
+{
+	efi_physical_addr_t alloc_addr;
+	efi_status_t status;
+	int slack;
+
+	if (align < EFI_ALLOC_ALIGN)
+		align = EFI_ALLOC_ALIGN;
+
+	alloc_addr = ALIGN_DOWN(max + 1, align) - 1;
+	size = round_up(size, EFI_ALLOC_ALIGN);
+	slack = align / EFI_PAGE_SIZE - 1;
+
+	status = efi_bs_call(allocate_pages, EFI_ALLOCATE_MAX_ADDRESS,
+			     EFI_LOADER_DATA, size / EFI_PAGE_SIZE + slack,
+			     &alloc_addr);
+	if (status != EFI_SUCCESS)
+		return status;
+
+	*addr = ALIGN((unsigned long)alloc_addr, align);
+
+	if (slack > 0) {
+		int l = (alloc_addr % align) / EFI_PAGE_SIZE;
+
+		if (l) {
+			efi_bs_call(free_pages, alloc_addr, slack - l + 1);
+			slack = l - 1;
+		}
+		if (slack)
+			efi_bs_call(free_pages, *addr + size, slack);
+	}
+	return EFI_SUCCESS;
+}
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 4844c3bd40df..5dcfadcf2bc1 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -641,6 +641,9 @@ efi_status_t efi_low_alloc(unsigned long size, unsigned long align,
 efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
 				unsigned long max);
 
+efi_status_t efi_allocate_pages_aligned(unsigned long size, unsigned long *addr,
+					unsigned long max, unsigned long align);
+
 efi_status_t efi_relocate_kernel(unsigned long *image_addr,
 				 unsigned long image_size,
 				 unsigned long alloc_size,
diff --git a/drivers/firmware/efi/libstub/mem.c b/drivers/firmware/efi/libstub/mem.c
index 869a79c8946f..0020b0fa9587 100644
--- a/drivers/firmware/efi/libstub/mem.c
+++ b/drivers/firmware/efi/libstub/mem.c
@@ -93,31 +93,24 @@ efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
 efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
 				unsigned long max)
 {
-	efi_physical_addr_t alloc_addr = ALIGN_DOWN(max + 1, EFI_ALLOC_ALIGN) - 1;
-	int slack = EFI_ALLOC_ALIGN / EFI_PAGE_SIZE - 1;
+	efi_physical_addr_t alloc_addr;
 	efi_status_t status;
 
-	size = round_up(size, EFI_ALLOC_ALIGN);
+	if (EFI_ALLOC_ALIGN > EFI_PAGE_SIZE)
+		return efi_allocate_pages_aligned(size, addr, max,
+						  EFI_ALLOC_ALIGN);
+
+	alloc_addr = ALIGN_DOWN(max + 1, EFI_ALLOC_ALIGN) - 1;
 	status = efi_bs_call(allocate_pages, EFI_ALLOCATE_MAX_ADDRESS,
-			     EFI_LOADER_DATA, size / EFI_PAGE_SIZE + slack,
+			     EFI_LOADER_DATA, DIV_ROUND_UP(size, EFI_PAGE_SIZE),
 			     &alloc_addr);
 	if (status != EFI_SUCCESS)
 		return status;
 
-	*addr = ALIGN((unsigned long)alloc_addr, EFI_ALLOC_ALIGN);
-
-	if (slack > 0) {
-		int l = (alloc_addr % EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE;
-
-		if (l) {
-			efi_bs_call(free_pages, alloc_addr, slack - l + 1);
-			slack = l - 1;
-		}
-		if (slack)
-			efi_bs_call(free_pages, *addr + size, slack);
-	}
+	*addr = alloc_addr;
 	return EFI_SUCCESS;
 }
+
 /**
  * efi_low_alloc_above() - allocate pages at or above given address
  * @size:	size of the memory area to allocate
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 7/8] efi/libstub/arm64: switch to ordinary page allocator for kernel image

[Ard Biesheuvel]

It is no longer necessary to locate the kernel as low as possible in
physical memory, and so we can switch from efi_low_alloc() [which is
a rather nasty concoction on top of GetMemoryMap()] to a new helper
called efi_allocate_pages_aligned(), which simply rounds up the size
to account for the alignment, and frees the misaligned pages again.

So considering that the kernel can live anywhere in the physical
address space, as long as its alignment requirements are met, let's
switch to efi_allocate_pages_aligned() to allocate the pages.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/arm64-stub.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index 6fc3bd9a56db..99b67e88a33b 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -98,8 +98,8 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 			return EFI_SUCCESS;
 		}
 
-		status = efi_low_alloc(*reserve_size,
-				       min_kimg_align, reserve_addr);
+		status = efi_allocate_pages_aligned(*reserve_size, reserve_addr,
+						    ULONG_MAX, min_kimg_align);
 
 		if (status != EFI_SUCCESS) {
 			pr_efi_err("Failed to relocate kernel\n");
-- 
2.17.1

[PATCH v2 7/8] efi/libstub/arm64: switch to ordinary page allocator for kernel image

[Ard Biesheuvel]

It is no longer necessary to locate the kernel as low as possible in
physical memory, and so we can switch from efi_low_alloc() [which is
a rather nasty concoction on top of GetMemoryMap()] to a new helper
called efi_allocate_pages_aligned(), which simply rounds up the size
to account for the alignment, and frees the misaligned pages again.

So considering that the kernel can live anywhere in the physical
address space, as long as its alignment requirements are met, let's
switch to efi_allocate_pages_aligned() to allocate the pages.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/arm64-stub.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index 6fc3bd9a56db..99b67e88a33b 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -98,8 +98,8 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 			return EFI_SUCCESS;
 		}
 
-		status = efi_low_alloc(*reserve_size,
-				       min_kimg_align, reserve_addr);
+		status = efi_allocate_pages_aligned(*reserve_size, reserve_addr,
+						    ULONG_MAX, min_kimg_align);
 
 		if (status != EFI_SUCCESS) {
 			pr_efi_err("Failed to relocate kernel\n");
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 8/8] efi/libstub: move efi_relocate_kernel() into separate source file

[Ard Biesheuvel]

Move efi_relocate_kernel() into a separate source file, so that it
only gets pulled into builds for architectures that use it. Since
efi_relocate_kernel() is the only user of efi_low_alloc(), let's
move that over as well.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/Makefile   |   2 +-
 drivers/firmware/efi/libstub/efistub.h  |  15 --
 drivers/firmware/efi/libstub/mem.c      | 168 -------------------
 drivers/firmware/efi/libstub/relocate.c | 174 ++++++++++++++++++++
 4 files changed, 175 insertions(+), 184 deletions(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 1a09b9445394..2dbe4394f818 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -43,7 +43,7 @@ KCOV_INSTRUMENT			:= n
 lib-y				:= efi-stub-helper.o gop.o secureboot.o tpm.o \
 				   file.o mem.o random.o randomalloc.o pci.o \
 				   skip_spaces.o lib-cmdline.o lib-ctype.o \
-				   alignedmem.o
+				   alignedmem.o relocate.o
 
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
 arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 5dcfadcf2bc1..eafb409278fa 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -623,21 +623,6 @@ char *efi_convert_cmdline(efi_loaded_image_t *image, int *cmd_line_len,
 
 efi_status_t efi_get_memory_map(struct efi_boot_memmap *map);
 
-efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
-				 unsigned long *addr, unsigned long min);
-
-static inline
-efi_status_t efi_low_alloc(unsigned long size, unsigned long align,
-			   unsigned long *addr)
-{
-	/*
-	 * Don't allocate at 0x0. It will confuse code that
-	 * checks pointers against NULL. Skip the first 8
-	 * bytes so we start at a nice even number.
-	 */
-	return efi_low_alloc_above(size, align, addr, 0x8);
-}
-
 efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
 				unsigned long max);
 
diff --git a/drivers/firmware/efi/libstub/mem.c b/drivers/firmware/efi/libstub/mem.c
index 0020b0fa9587..6e0ee6b3d897 100644
--- a/drivers/firmware/efi/libstub/mem.c
+++ b/drivers/firmware/efi/libstub/mem.c
@@ -111,96 +111,6 @@ efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
 	return EFI_SUCCESS;
 }
 
-/**
- * efi_low_alloc_above() - allocate pages at or above given address
- * @size:	size of the memory area to allocate
- * @align:	minimum alignment of the allocated memory area. It should
- *		a power of two.
- * @addr:	on exit the address of the allocated memory
- * @min:	minimum address to used for the memory allocation
- *
- * Allocate at the lowest possible address that is not below @min as
- * EFI_LOADER_DATA. The allocated pages are aligned according to @align but at
- * least EFI_ALLOC_ALIGN. The first allocated page will not below the address
- * given by @min.
- *
- * Return:	status code
- */
-efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
-				 unsigned long *addr, unsigned long min)
-{
-	unsigned long map_size, desc_size, buff_size;
-	efi_memory_desc_t *map;
-	efi_status_t status;
-	unsigned long nr_pages;
-	int i;
-	struct efi_boot_memmap boot_map;
-
-	boot_map.map		= &map;
-	boot_map.map_size	= &map_size;
-	boot_map.desc_size	= &desc_size;
-	boot_map.desc_ver	= NULL;
-	boot_map.key_ptr	= NULL;
-	boot_map.buff_size	= &buff_size;
-
-	status = efi_get_memory_map(&boot_map);
-	if (status != EFI_SUCCESS)
-		goto fail;
-
-	/*
-	 * Enforce minimum alignment that EFI or Linux requires when
-	 * requesting a specific address.  We are doing page-based (or
-	 * larger) allocations, and both the address and size must meet
-	 * alignment constraints.
-	 */
-	if (align < EFI_ALLOC_ALIGN)
-		align = EFI_ALLOC_ALIGN;
-
-	size = round_up(size, EFI_ALLOC_ALIGN);
-	nr_pages = size / EFI_PAGE_SIZE;
-	for (i = 0; i < map_size / desc_size; i++) {
-		efi_memory_desc_t *desc;
-		unsigned long m = (unsigned long)map;
-		u64 start, end;
-
-		desc = efi_early_memdesc_ptr(m, desc_size, i);
-
-		if (desc->type != EFI_CONVENTIONAL_MEMORY)
-			continue;
-
-		if (efi_soft_reserve_enabled() &&
-		    (desc->attribute & EFI_MEMORY_SP))
-			continue;
-
-		if (desc->num_pages < nr_pages)
-			continue;
-
-		start = desc->phys_addr;
-		end = start + desc->num_pages * EFI_PAGE_SIZE;
-
-		if (start < min)
-			start = min;
-
-		start = round_up(start, align);
-		if ((start + size) > end)
-			continue;
-
-		status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
-				     EFI_LOADER_DATA, nr_pages, &start);
-		if (status == EFI_SUCCESS) {
-			*addr = start;
-			break;
-		}
-	}
-
-	if (i == map_size / desc_size)
-		status = EFI_NOT_FOUND;
-
-	efi_bs_call(free_pool, map);
-fail:
-	return status;
-}
-
 /**
  * efi_free() - free memory pages
  * @size:	size of the memory area to free in bytes
@@ -222,81 +132,3 @@ void efi_free(unsigned long size, unsigned long addr)
 	nr_pages = round_up(size, EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE;
 	efi_bs_call(free_pages, addr, nr_pages);
 }
-
-/**
- * efi_relocate_kernel() - copy memory area
- * @image_addr:		pointer to address of memory area to copy
- * @image_size:		size of memory area to copy
- * @alloc_size:		minimum size of memory to allocate, must be greater or
- *			equal to image_size
- * @preferred_addr:	preferred target address
- * @alignment:		minimum alignment of the allocated memory area. It
- *			should be a power of two.
- * @min_addr:		minimum target address
- *
- * Copy a memory area to a newly allocated memory area aligned according
- * to @alignment but at least EFI_ALLOC_ALIGN. If the preferred address
- * is not available, the allocated address will not be below @min_addr.
- * On exit, @image_addr is updated to the target copy address that was used.
- *
- * This function is used to copy the Linux kernel verbatim. It does not apply
- * any relocation changes.
- *
- * Return:		status code
- */
-efi_status_t efi_relocate_kernel(unsigned long *image_addr,
-				 unsigned long image_size,
-				 unsigned long alloc_size,
-				 unsigned long preferred_addr,
-				 unsigned long alignment,
-				 unsigned long min_addr)
-{
-	unsigned long cur_image_addr;
-	unsigned long new_addr = 0;
-	efi_status_t status;
-	unsigned long nr_pages;
-	efi_physical_addr_t efi_addr = preferred_addr;
-
-	if (!image_addr || !image_size || !alloc_size)
-		return EFI_INVALID_PARAMETER;
-	if (alloc_size < image_size)
-		return EFI_INVALID_PARAMETER;
-
-	cur_image_addr = *image_addr;
-
-	/*
-	 * The EFI firmware loader could have placed the kernel image
-	 * anywhere in memory, but the kernel has restrictions on the
-	 * max physical address it can run at.  Some architectures
-	 * also have a prefered address, so first try to relocate
-	 * to the preferred address.  If that fails, allocate as low
-	 * as possible while respecting the required alignment.
-	 */
-	nr_pages = round_up(alloc_size, EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE;
-	status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
-			     EFI_LOADER_DATA, nr_pages, &efi_addr);
-	new_addr = efi_addr;
-	/*
-	 * If preferred address allocation failed allocate as low as
-	 * possible.
-	 */
-	if (status != EFI_SUCCESS) {
-		status = efi_low_alloc_above(alloc_size, alignment, &new_addr,
-					     min_addr);
-	}
-	if (status != EFI_SUCCESS) {
-		pr_efi_err("Failed to allocate usable memory for kernel.\n");
-		return status;
-	}
-
-	/*
-	 * We know source/dest won't overlap since both memory ranges
-	 * have been allocated by UEFI, so we can safely use memcpy.
-	 */
-	memcpy((void *)new_addr, (void *)cur_image_addr, image_size);
-
-	/* Return the new address of the relocated image. */
-	*image_addr = new_addr;
-
-	return status;
-}
diff --git a/drivers/firmware/efi/libstub/relocate.c b/drivers/firmware/efi/libstub/relocate.c
new file mode 100644
index 000000000000..1507d3c82884
--- /dev/null
+++ b/drivers/firmware/efi/libstub/relocate.c
@@ -0,0 +1,174 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/efi.h>
+#include <asm/efi.h>
+
+#include "efistub.h"
+
+/**
+ * efi_low_alloc_above() - allocate pages at or above given address
+ * @size:	size of the memory area to allocate
+ * @align:	minimum alignment of the allocated memory area. It should
+ *		a power of two.
+ * @addr:	on exit the address of the allocated memory
+ * @min:	minimum address to used for the memory allocation
+ *
+ * Allocate at the lowest possible address that is not below @min as
+ * EFI_LOADER_DATA. The allocated pages are aligned according to @align but at
+ * least EFI_ALLOC_ALIGN. The first allocated page will not below the address
+ * given by @min.
+ *
+ * Return:	status code
+ */
+static efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
+					unsigned long *addr, unsigned long min)
+{
+	unsigned long map_size, desc_size, buff_size;
+	efi_memory_desc_t *map;
+	efi_status_t status;
+	unsigned long nr_pages;
+	int i;
+	struct efi_boot_memmap boot_map;
+
+	boot_map.map		= &map;
+	boot_map.map_size	= &map_size;
+	boot_map.desc_size	= &desc_size;
+	boot_map.desc_ver	= NULL;
+	boot_map.key_ptr	= NULL;
+	boot_map.buff_size	= &buff_size;
+
+	status = efi_get_memory_map(&boot_map);
+	if (status != EFI_SUCCESS)
+		goto fail;
+
+	/*
+	 * Enforce minimum alignment that EFI or Linux requires when
+	 * requesting a specific address.  We are doing page-based (or
+	 * larger) allocations, and both the address and size must meet
+	 * alignment constraints.
+	 */
+	if (align < EFI_ALLOC_ALIGN)
+		align = EFI_ALLOC_ALIGN;
+
+	size = round_up(size, EFI_ALLOC_ALIGN);
+	nr_pages = size / EFI_PAGE_SIZE;
+	for (i = 0; i < map_size / desc_size; i++) {
+		efi_memory_desc_t *desc;
+		unsigned long m = (unsigned long)map;
+		u64 start, end;
+
+		desc = efi_early_memdesc_ptr(m, desc_size, i);
+
+		if (desc->type != EFI_CONVENTIONAL_MEMORY)
+			continue;
+
+		if (efi_soft_reserve_enabled() &&
+		    (desc->attribute & EFI_MEMORY_SP))
+			continue;
+
+		if (desc->num_pages < nr_pages)
+			continue;
+
+		start = desc->phys_addr;
+		end = start + desc->num_pages * EFI_PAGE_SIZE;
+
+		if (start < min)
+			start = min;
+
+		start = round_up(start, align);
+		if ((start + size) > end)
+			continue;
+
+		status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
+				     EFI_LOADER_DATA, nr_pages, &start);
+		if (status == EFI_SUCCESS) {
+			*addr = start;
+			break;
+		}
+	}
+
+	if (i == map_size / desc_size)
+		status = EFI_NOT_FOUND;
+
+	efi_bs_call(free_pool, map);
+fail:
+	return status;
+}
+
+/**
+ * efi_relocate_kernel() - copy memory area
+ * @image_addr:		pointer to address of memory area to copy
+ * @image_size:		size of memory area to copy
+ * @alloc_size:		minimum size of memory to allocate, must be greater or
+ *			equal to image_size
+ * @preferred_addr:	preferred target address
+ * @alignment:		minimum alignment of the allocated memory area. It
+ *			should be a power of two.
+ * @min_addr:		minimum target address
+ *
+ * Copy a memory area to a newly allocated memory area aligned according
+ * to @alignment but at least EFI_ALLOC_ALIGN. If the preferred address
+ * is not available, the allocated address will not be below @min_addr.
+ * On exit, @image_addr is updated to the target copy address that was used.
+ *
+ * This function is used to copy the Linux kernel verbatim. It does not apply
+ * any relocation changes.
+ *
+ * Return:		status code
+ */
+efi_status_t efi_relocate_kernel(unsigned long *image_addr,
+				 unsigned long image_size,
+				 unsigned long alloc_size,
+				 unsigned long preferred_addr,
+				 unsigned long alignment,
+				 unsigned long min_addr)
+{
+	unsigned long cur_image_addr;
+	unsigned long new_addr = 0;
+	efi_status_t status;
+	unsigned long nr_pages;
+	efi_physical_addr_t efi_addr = preferred_addr;
+
+	if (!image_addr || !image_size || !alloc_size)
+		return EFI_INVALID_PARAMETER;
+	if (alloc_size < image_size)
+		return EFI_INVALID_PARAMETER;
+
+	cur_image_addr = *image_addr;
+
+	/*
+	 * The EFI firmware loader could have placed the kernel image
+	 * anywhere in memory, but the kernel has restrictions on the
+	 * max physical address it can run at.  Some architectures
+	 * also have a prefered address, so first try to relocate
+	 * to the preferred address.  If that fails, allocate as low
+	 * as possible while respecting the required alignment.
+	 */
+	nr_pages = round_up(alloc_size, EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE;
+	status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
+			     EFI_LOADER_DATA, nr_pages, &efi_addr);
+	new_addr = efi_addr;
+	/*
+	 * If preferred address allocation failed allocate as low as
+	 * possible.
+	 */
+	if (status != EFI_SUCCESS) {
+		status = efi_low_alloc_above(alloc_size, alignment, &new_addr,
+					     min_addr);
+	}
+	if (status != EFI_SUCCESS) {
+		pr_efi_err("Failed to allocate usable memory for kernel.\n");
+		return status;
+	}
+
+	/*
+	 * We know source/dest won't overlap since both memory ranges
+	 * have been allocated by UEFI, so we can safely use memcpy.
+	 */
+	memcpy((void *)new_addr, (void *)cur_image_addr, image_size);
+
+	/* Return the new address of the relocated image. */
+	*image_addr = new_addr;
+
+	return status;
+}
-- 
2.17.1

[PATCH v2 8/8] efi/libstub: move efi_relocate_kernel() into separate source file

[Ard Biesheuvel]

Move efi_relocate_kernel() into a separate source file, so that it
only gets pulled into builds for architectures that use it. Since
efi_relocate_kernel() is the only user of efi_low_alloc(), let's
move that over as well.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/Makefile   |   2 +-
 drivers/firmware/efi/libstub/efistub.h  |  15 --
 drivers/firmware/efi/libstub/mem.c      | 168 -------------------
 drivers/firmware/efi/libstub/relocate.c | 174 ++++++++++++++++++++
 4 files changed, 175 insertions(+), 184 deletions(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 1a09b9445394..2dbe4394f818 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -43,7 +43,7 @@ KCOV_INSTRUMENT			:= n
 lib-y				:= efi-stub-helper.o gop.o secureboot.o tpm.o \
 				   file.o mem.o random.o randomalloc.o pci.o \
 				   skip_spaces.o lib-cmdline.o lib-ctype.o \
-				   alignedmem.o
+				   alignedmem.o relocate.o
 
 # include the stub's generic dependencies from lib/ when building for ARM/arm64
 arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 5dcfadcf2bc1..eafb409278fa 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -623,21 +623,6 @@ char *efi_convert_cmdline(efi_loaded_image_t *image, int *cmd_line_len,
 
 efi_status_t efi_get_memory_map(struct efi_boot_memmap *map);
 
-efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
-				 unsigned long *addr, unsigned long min);
-
-static inline
-efi_status_t efi_low_alloc(unsigned long size, unsigned long align,
-			   unsigned long *addr)
-{
-	/*
-	 * Don't allocate at 0x0. It will confuse code that
-	 * checks pointers against NULL. Skip the first 8
-	 * bytes so we start at a nice even number.
-	 */
-	return efi_low_alloc_above(size, align, addr, 0x8);
-}
-
 efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
 				unsigned long max);
 
diff --git a/drivers/firmware/efi/libstub/mem.c b/drivers/firmware/efi/libstub/mem.c
index 0020b0fa9587..6e0ee6b3d897 100644
--- a/drivers/firmware/efi/libstub/mem.c
+++ b/drivers/firmware/efi/libstub/mem.c
@@ -111,96 +111,6 @@ efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
 	return EFI_SUCCESS;
 }
 
-/**
- * efi_low_alloc_above() - allocate pages at or above given address
- * @size:	size of the memory area to allocate
- * @align:	minimum alignment of the allocated memory area. It should
- *		a power of two.
- * @addr:	on exit the address of the allocated memory
- * @min:	minimum address to used for the memory allocation
- *
- * Allocate at the lowest possible address that is not below @min as
- * EFI_LOADER_DATA. The allocated pages are aligned according to @align but at
- * least EFI_ALLOC_ALIGN. The first allocated page will not below the address
- * given by @min.
- *
- * Return:	status code
- */
-efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
-				 unsigned long *addr, unsigned long min)
-{
-	unsigned long map_size, desc_size, buff_size;
-	efi_memory_desc_t *map;
-	efi_status_t status;
-	unsigned long nr_pages;
-	int i;
-	struct efi_boot_memmap boot_map;
-
-	boot_map.map		= &map;
-	boot_map.map_size	= &map_size;
-	boot_map.desc_size	= &desc_size;
-	boot_map.desc_ver	= NULL;
-	boot_map.key_ptr	= NULL;
-	boot_map.buff_size	= &buff_size;
-
-	status = efi_get_memory_map(&boot_map);
-	if (status != EFI_SUCCESS)
-		goto fail;
-
-	/*
-	 * Enforce minimum alignment that EFI or Linux requires when
-	 * requesting a specific address.  We are doing page-based (or
-	 * larger) allocations, and both the address and size must meet
-	 * alignment constraints.
-	 */
-	if (align < EFI_ALLOC_ALIGN)
-		align = EFI_ALLOC_ALIGN;
-
-	size = round_up(size, EFI_ALLOC_ALIGN);
-	nr_pages = size / EFI_PAGE_SIZE;
-	for (i = 0; i < map_size / desc_size; i++) {
-		efi_memory_desc_t *desc;
-		unsigned long m = (unsigned long)map;
-		u64 start, end;
-
-		desc = efi_early_memdesc_ptr(m, desc_size, i);
-
-		if (desc->type != EFI_CONVENTIONAL_MEMORY)
-			continue;
-
-		if (efi_soft_reserve_enabled() &&
-		    (desc->attribute & EFI_MEMORY_SP))
-			continue;
-
-		if (desc->num_pages < nr_pages)
-			continue;
-
-		start = desc->phys_addr;
-		end = start + desc->num_pages * EFI_PAGE_SIZE;
-
-		if (start < min)
-			start = min;
-
-		start = round_up(start, align);
-		if ((start + size) > end)
-			continue;
-
-		status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
-				     EFI_LOADER_DATA, nr_pages, &start);
-		if (status == EFI_SUCCESS) {
-			*addr = start;
-			break;
-		}
-	}
-
-	if (i == map_size / desc_size)
-		status = EFI_NOT_FOUND;
-
-	efi_bs_call(free_pool, map);
-fail:
-	return status;
-}
-
 /**
  * efi_free() - free memory pages
  * @size:	size of the memory area to free in bytes
@@ -222,81 +132,3 @@ void efi_free(unsigned long size, unsigned long addr)
 	nr_pages = round_up(size, EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE;
 	efi_bs_call(free_pages, addr, nr_pages);
 }
-
-/**
- * efi_relocate_kernel() - copy memory area
- * @image_addr:		pointer to address of memory area to copy
- * @image_size:		size of memory area to copy
- * @alloc_size:		minimum size of memory to allocate, must be greater or
- *			equal to image_size
- * @preferred_addr:	preferred target address
- * @alignment:		minimum alignment of the allocated memory area. It
- *			should be a power of two.
- * @min_addr:		minimum target address
- *
- * Copy a memory area to a newly allocated memory area aligned according
- * to @alignment but at least EFI_ALLOC_ALIGN. If the preferred address
- * is not available, the allocated address will not be below @min_addr.
- * On exit, @image_addr is updated to the target copy address that was used.
- *
- * This function is used to copy the Linux kernel verbatim. It does not apply
- * any relocation changes.
- *
- * Return:		status code
- */
-efi_status_t efi_relocate_kernel(unsigned long *image_addr,
-				 unsigned long image_size,
-				 unsigned long alloc_size,
-				 unsigned long preferred_addr,
-				 unsigned long alignment,
-				 unsigned long min_addr)
-{
-	unsigned long cur_image_addr;
-	unsigned long new_addr = 0;
-	efi_status_t status;
-	unsigned long nr_pages;
-	efi_physical_addr_t efi_addr = preferred_addr;
-
-	if (!image_addr || !image_size || !alloc_size)
-		return EFI_INVALID_PARAMETER;
-	if (alloc_size < image_size)
-		return EFI_INVALID_PARAMETER;
-
-	cur_image_addr = *image_addr;
-
-	/*
-	 * The EFI firmware loader could have placed the kernel image
-	 * anywhere in memory, but the kernel has restrictions on the
-	 * max physical address it can run at.  Some architectures
-	 * also have a prefered address, so first try to relocate
-	 * to the preferred address.  If that fails, allocate as low
-	 * as possible while respecting the required alignment.
-	 */
-	nr_pages = round_up(alloc_size, EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE;
-	status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
-			     EFI_LOADER_DATA, nr_pages, &efi_addr);
-	new_addr = efi_addr;
-	/*
-	 * If preferred address allocation failed allocate as low as
-	 * possible.
-	 */
-	if (status != EFI_SUCCESS) {
-		status = efi_low_alloc_above(alloc_size, alignment, &new_addr,
-					     min_addr);
-	}
-	if (status != EFI_SUCCESS) {
-		pr_efi_err("Failed to allocate usable memory for kernel.\n");
-		return status;
-	}
-
-	/*
-	 * We know source/dest won't overlap since both memory ranges
-	 * have been allocated by UEFI, so we can safely use memcpy.
-	 */
-	memcpy((void *)new_addr, (void *)cur_image_addr, image_size);
-
-	/* Return the new address of the relocated image. */
-	*image_addr = new_addr;
-
-	return status;
-}
diff --git a/drivers/firmware/efi/libstub/relocate.c b/drivers/firmware/efi/libstub/relocate.c
new file mode 100644
index 000000000000..1507d3c82884
--- /dev/null
+++ b/drivers/firmware/efi/libstub/relocate.c
@@ -0,0 +1,174 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/efi.h>
+#include <asm/efi.h>
+
+#include "efistub.h"
+
+/**
+ * efi_low_alloc_above() - allocate pages at or above given address
+ * @size:	size of the memory area to allocate
+ * @align:	minimum alignment of the allocated memory area. It should
+ *		a power of two.
+ * @addr:	on exit the address of the allocated memory
+ * @min:	minimum address to used for the memory allocation
+ *
+ * Allocate at the lowest possible address that is not below @min as
+ * EFI_LOADER_DATA. The allocated pages are aligned according to @align but at
+ * least EFI_ALLOC_ALIGN. The first allocated page will not below the address
+ * given by @min.
+ *
+ * Return:	status code
+ */
+static efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
+					unsigned long *addr, unsigned long min)
+{
+	unsigned long map_size, desc_size, buff_size;
+	efi_memory_desc_t *map;
+	efi_status_t status;
+	unsigned long nr_pages;
+	int i;
+	struct efi_boot_memmap boot_map;
+
+	boot_map.map		= &map;
+	boot_map.map_size	= &map_size;
+	boot_map.desc_size	= &desc_size;
+	boot_map.desc_ver	= NULL;
+	boot_map.key_ptr	= NULL;
+	boot_map.buff_size	= &buff_size;
+
+	status = efi_get_memory_map(&boot_map);
+	if (status != EFI_SUCCESS)
+		goto fail;
+
+	/*
+	 * Enforce minimum alignment that EFI or Linux requires when
+	 * requesting a specific address.  We are doing page-based (or
+	 * larger) allocations, and both the address and size must meet
+	 * alignment constraints.
+	 */
+	if (align < EFI_ALLOC_ALIGN)
+		align = EFI_ALLOC_ALIGN;
+
+	size = round_up(size, EFI_ALLOC_ALIGN);
+	nr_pages = size / EFI_PAGE_SIZE;
+	for (i = 0; i < map_size / desc_size; i++) {
+		efi_memory_desc_t *desc;
+		unsigned long m = (unsigned long)map;
+		u64 start, end;
+
+		desc = efi_early_memdesc_ptr(m, desc_size, i);
+
+		if (desc->type != EFI_CONVENTIONAL_MEMORY)
+			continue;
+
+		if (efi_soft_reserve_enabled() &&
+		    (desc->attribute & EFI_MEMORY_SP))
+			continue;
+
+		if (desc->num_pages < nr_pages)
+			continue;
+
+		start = desc->phys_addr;
+		end = start + desc->num_pages * EFI_PAGE_SIZE;
+
+		if (start < min)
+			start = min;
+
+		start = round_up(start, align);
+		if ((start + size) > end)
+			continue;
+
+		status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
+				     EFI_LOADER_DATA, nr_pages, &start);
+		if (status == EFI_SUCCESS) {
+			*addr = start;
+			break;
+		}
+	}
+
+	if (i == map_size / desc_size)
+		status = EFI_NOT_FOUND;
+
+	efi_bs_call(free_pool, map);
+fail:
+	return status;
+}
+
+/**
+ * efi_relocate_kernel() - copy memory area
+ * @image_addr:		pointer to address of memory area to copy
+ * @image_size:		size of memory area to copy
+ * @alloc_size:		minimum size of memory to allocate, must be greater or
+ *			equal to image_size
+ * @preferred_addr:	preferred target address
+ * @alignment:		minimum alignment of the allocated memory area. It
+ *			should be a power of two.
+ * @min_addr:		minimum target address
+ *
+ * Copy a memory area to a newly allocated memory area aligned according
+ * to @alignment but at least EFI_ALLOC_ALIGN. If the preferred address
+ * is not available, the allocated address will not be below @min_addr.
+ * On exit, @image_addr is updated to the target copy address that was used.
+ *
+ * This function is used to copy the Linux kernel verbatim. It does not apply
+ * any relocation changes.
+ *
+ * Return:		status code
+ */
+efi_status_t efi_relocate_kernel(unsigned long *image_addr,
+				 unsigned long image_size,
+				 unsigned long alloc_size,
+				 unsigned long preferred_addr,
+				 unsigned long alignment,
+				 unsigned long min_addr)
+{
+	unsigned long cur_image_addr;
+	unsigned long new_addr = 0;
+	efi_status_t status;
+	unsigned long nr_pages;
+	efi_physical_addr_t efi_addr = preferred_addr;
+
+	if (!image_addr || !image_size || !alloc_size)
+		return EFI_INVALID_PARAMETER;
+	if (alloc_size < image_size)
+		return EFI_INVALID_PARAMETER;
+
+	cur_image_addr = *image_addr;
+
+	/*
+	 * The EFI firmware loader could have placed the kernel image
+	 * anywhere in memory, but the kernel has restrictions on the
+	 * max physical address it can run at.  Some architectures
+	 * also have a prefered address, so first try to relocate
+	 * to the preferred address.  If that fails, allocate as low
+	 * as possible while respecting the required alignment.
+	 */
+	nr_pages = round_up(alloc_size, EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE;
+	status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
+			     EFI_LOADER_DATA, nr_pages, &efi_addr);
+	new_addr = efi_addr;
+	/*
+	 * If preferred address allocation failed allocate as low as
+	 * possible.
+	 */
+	if (status != EFI_SUCCESS) {
+		status = efi_low_alloc_above(alloc_size, alignment, &new_addr,
+					     min_addr);
+	}
+	if (status != EFI_SUCCESS) {
+		pr_efi_err("Failed to allocate usable memory for kernel.\n");
+		return status;
+	}
+
+	/*
+	 * We know source/dest won't overlap since both memory ranges
+	 * have been allocated by UEFI, so we can safely use memcpy.
+	 */
+	memcpy((void *)new_addr, (void *)cur_image_addr, image_size);
+
+	/* Return the new address of the relocated image. */
+	*image_addr = new_addr;
+
+	return status;
+}
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 0/8] efi/libstub: simplify arm64 kernel image loading

[Atish Patra]

On Mon, Apr 13, 2020 at 8:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On arm64, the kernel image used to be virtually mapped via the linear
> region, making the two mappings correlated in a way that required the
> kernel to be located at the start of the linear region, or the memory
> below would not be accessible. For this reason, the EFI stub loader
> code for arm64 has the notion of a 'preferred offset' for the physical
> placement of the kernel image, and tries to put the kernel there, or
> at least as low as possible in physical memory (unless KASLR is active,
> in which case the placement is randomized)
>
> When KASLR was introduced, the virtual mapping of the kernel was moved
> into the vmalloc region, and now, regardless of whether KASLR support
> is built in or active, the kernel can be placed anywhere in physical
> memory without any detrimental side effects on the linear region.
>
> This means that we can drop the notion of 'preferred offset' entirely,
> and invoke the kernel in place if the PE/COFF loader loaded it at the
> right offset. If not, we can invoke the ordinary UEFI top down page
> allocator to reallocate it elsewhere in memory. By updating the PE/COFF
> metadata, we can inform the PE/COFF loader about the desired alignment,
> making it less likely that we need to move the kernel image in the first
> place.
>
> Ard Biesheuvel (8):
>   efi/libstub/random: align allocate size to EFI_ALLOC_ALIGN
>   efi/libstub/random: increase random alloc granularity
>   efi/libstub/arm64: replace 'preferred' offset with alignment check
>   efi/libstub/arm64: simplify randomized loading of kernel image
>   efi/libstub/arm64: align PE/COFF sections to segment alignment
>   efi/libstub: add API function to allocate aligned memory
>   efi/libstub/arm64: switch to ordinary page allocator for kernel image
>   efi/libstub: move efi_relocate_kernel() into separate source file
>
>  arch/arm64/kernel/efi-header.S             |   2 +-
>  arch/arm64/kernel/vmlinux.lds.S            |   3 +-
>  drivers/firmware/efi/libstub/Makefile      |   3 +-
>  drivers/firmware/efi/libstub/alignedmem.c  |  57 ++++++
>  drivers/firmware/efi/libstub/arm64-stub.c  |  92 +++-------
>  drivers/firmware/efi/libstub/efistub.h     |  18 +-
>  drivers/firmware/efi/libstub/mem.c         | 191 +-------------------
>  drivers/firmware/efi/libstub/randomalloc.c |   6 +-
>  drivers/firmware/efi/libstub/relocate.c    | 174 ++++++++++++++++++
>  9 files changed, 280 insertions(+), 266 deletions(-)
>  create mode 100644 drivers/firmware/efi/libstub/alignedmem.c
>  create mode 100644 drivers/firmware/efi/libstub/relocate.c
>
> --
> 2.17.1
>

Oops. I just noticed this series after I sent out a v2.
I see that efi_low_alloc is removed now and the handle_kernel_image is
simplified for arm64.
I will update the risc-v uefi series accordingly. Sorry for the noise.

--
Regards,
Atish

Re: [PATCH v2 0/8] efi/libstub: simplify arm64 kernel image loading

[Atish Patra]

On Mon, Apr 13, 2020 at 8:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On arm64, the kernel image used to be virtually mapped via the linear
> region, making the two mappings correlated in a way that required the
> kernel to be located at the start of the linear region, or the memory
> below would not be accessible. For this reason, the EFI stub loader
> code for arm64 has the notion of a 'preferred offset' for the physical
> placement of the kernel image, and tries to put the kernel there, or
> at least as low as possible in physical memory (unless KASLR is active,
> in which case the placement is randomized)
>
> When KASLR was introduced, the virtual mapping of the kernel was moved
> into the vmalloc region, and now, regardless of whether KASLR support
> is built in or active, the kernel can be placed anywhere in physical
> memory without any detrimental side effects on the linear region.
>
> This means that we can drop the notion of 'preferred offset' entirely,
> and invoke the kernel in place if the PE/COFF loader loaded it at the
> right offset. If not, we can invoke the ordinary UEFI top down page
> allocator to reallocate it elsewhere in memory. By updating the PE/COFF
> metadata, we can inform the PE/COFF loader about the desired alignment,
> making it less likely that we need to move the kernel image in the first
> place.
>
> Ard Biesheuvel (8):
>   efi/libstub/random: align allocate size to EFI_ALLOC_ALIGN
>   efi/libstub/random: increase random alloc granularity
>   efi/libstub/arm64: replace 'preferred' offset with alignment check
>   efi/libstub/arm64: simplify randomized loading of kernel image
>   efi/libstub/arm64: align PE/COFF sections to segment alignment
>   efi/libstub: add API function to allocate aligned memory
>   efi/libstub/arm64: switch to ordinary page allocator for kernel image
>   efi/libstub: move efi_relocate_kernel() into separate source file
>
>  arch/arm64/kernel/efi-header.S             |   2 +-
>  arch/arm64/kernel/vmlinux.lds.S            |   3 +-
>  drivers/firmware/efi/libstub/Makefile      |   3 +-
>  drivers/firmware/efi/libstub/alignedmem.c  |  57 ++++++
>  drivers/firmware/efi/libstub/arm64-stub.c  |  92 +++-------
>  drivers/firmware/efi/libstub/efistub.h     |  18 +-
>  drivers/firmware/efi/libstub/mem.c         | 191 +-------------------
>  drivers/firmware/efi/libstub/randomalloc.c |   6 +-
>  drivers/firmware/efi/libstub/relocate.c    | 174 ++++++++++++++++++
>  9 files changed, 280 insertions(+), 266 deletions(-)
>  create mode 100644 drivers/firmware/efi/libstub/alignedmem.c
>  create mode 100644 drivers/firmware/efi/libstub/relocate.c
>
> --
> 2.17.1
>

Oops. I just noticed this series after I sent out a v2.
I see that efi_low_alloc is removed now and the handle_kernel_image is
simplified for arm64.
I will update the risc-v uefi series accordingly. Sorry for the noise.

--
Regards,
Atish

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 0/8] efi/libstub: simplify arm64 kernel image loading

[Ard Biesheuvel]

On Mon, 13 Apr 2020 at 23:54, Atish Patra <atishp@atishpatra.org> wrote:
>
> On Mon, Apr 13, 2020 at 8:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On arm64, the kernel image used to be virtually mapped via the linear
> > region, making the two mappings correlated in a way that required the
> > kernel to be located at the start of the linear region, or the memory
> > below would not be accessible. For this reason, the EFI stub loader
> > code for arm64 has the notion of a 'preferred offset' for the physical
> > placement of the kernel image, and tries to put the kernel there, or
> > at least as low as possible in physical memory (unless KASLR is active,
> > in which case the placement is randomized)
> >
> > When KASLR was introduced, the virtual mapping of the kernel was moved
> > into the vmalloc region, and now, regardless of whether KASLR support
> > is built in or active, the kernel can be placed anywhere in physical
> > memory without any detrimental side effects on the linear region.
> >
> > This means that we can drop the notion of 'preferred offset' entirely,
> > and invoke the kernel in place if the PE/COFF loader loaded it at the
> > right offset. If not, we can invoke the ordinary UEFI top down page
> > allocator to reallocate it elsewhere in memory. By updating the PE/COFF
> > metadata, we can inform the PE/COFF loader about the desired alignment,
> > making it less likely that we need to move the kernel image in the first
> > place.
> >
> > Ard Biesheuvel (8):
> >   efi/libstub/random: align allocate size to EFI_ALLOC_ALIGN
> >   efi/libstub/random: increase random alloc granularity
> >   efi/libstub/arm64: replace 'preferred' offset with alignment check
> >   efi/libstub/arm64: simplify randomized loading of kernel image
> >   efi/libstub/arm64: align PE/COFF sections to segment alignment
> >   efi/libstub: add API function to allocate aligned memory
> >   efi/libstub/arm64: switch to ordinary page allocator for kernel image
> >   efi/libstub: move efi_relocate_kernel() into separate source file
> >
> >  arch/arm64/kernel/efi-header.S             |   2 +-
> >  arch/arm64/kernel/vmlinux.lds.S            |   3 +-
> >  drivers/firmware/efi/libstub/Makefile      |   3 +-
> >  drivers/firmware/efi/libstub/alignedmem.c  |  57 ++++++
> >  drivers/firmware/efi/libstub/arm64-stub.c  |  92 +++-------
> >  drivers/firmware/efi/libstub/efistub.h     |  18 +-
> >  drivers/firmware/efi/libstub/mem.c         | 191 +-------------------
> >  drivers/firmware/efi/libstub/randomalloc.c |   6 +-
> >  drivers/firmware/efi/libstub/relocate.c    | 174 ++++++++++++++++++
> >  9 files changed, 280 insertions(+), 266 deletions(-)
> >  create mode 100644 drivers/firmware/efi/libstub/alignedmem.c
> >  create mode 100644 drivers/firmware/efi/libstub/relocate.c
> >
> > --
> > 2.17.1
> >
>
> Oops. I just noticed this series after I sent out a v2.
> I see that efi_low_alloc is removed now and the handle_kernel_image is
> simplified for arm64.
> I will update the risc-v uefi series accordingly. Sorry for the noise.
>

No worries. And apologies for making this a moving target :-)

I realised you will probably need to bring back efi_low_alloc() as a
global symbol, as I don't think you will be able to switch to
efi_relocate_kernel().

In any case, this series is just set of patches on the list, so feel
free to propose changes if they are making your life too difficult.

Re: [PATCH v2 0/8] efi/libstub: simplify arm64 kernel image loading

[Ard Biesheuvel]

On Mon, 13 Apr 2020 at 23:54, Atish Patra <atishp@atishpatra.org> wrote:
>
> On Mon, Apr 13, 2020 at 8:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On arm64, the kernel image used to be virtually mapped via the linear
> > region, making the two mappings correlated in a way that required the
> > kernel to be located at the start of the linear region, or the memory
> > below would not be accessible. For this reason, the EFI stub loader
> > code for arm64 has the notion of a 'preferred offset' for the physical
> > placement of the kernel image, and tries to put the kernel there, or
> > at least as low as possible in physical memory (unless KASLR is active,
> > in which case the placement is randomized)
> >
> > When KASLR was introduced, the virtual mapping of the kernel was moved
> > into the vmalloc region, and now, regardless of whether KASLR support
> > is built in or active, the kernel can be placed anywhere in physical
> > memory without any detrimental side effects on the linear region.
> >
> > This means that we can drop the notion of 'preferred offset' entirely,
> > and invoke the kernel in place if the PE/COFF loader loaded it at the
> > right offset. If not, we can invoke the ordinary UEFI top down page
> > allocator to reallocate it elsewhere in memory. By updating the PE/COFF
> > metadata, we can inform the PE/COFF loader about the desired alignment,
> > making it less likely that we need to move the kernel image in the first
> > place.
> >
> > Ard Biesheuvel (8):
> >   efi/libstub/random: align allocate size to EFI_ALLOC_ALIGN
> >   efi/libstub/random: increase random alloc granularity
> >   efi/libstub/arm64: replace 'preferred' offset with alignment check
> >   efi/libstub/arm64: simplify randomized loading of kernel image
> >   efi/libstub/arm64: align PE/COFF sections to segment alignment
> >   efi/libstub: add API function to allocate aligned memory
> >   efi/libstub/arm64: switch to ordinary page allocator for kernel image
> >   efi/libstub: move efi_relocate_kernel() into separate source file
> >
> >  arch/arm64/kernel/efi-header.S             |   2 +-
> >  arch/arm64/kernel/vmlinux.lds.S            |   3 +-
> >  drivers/firmware/efi/libstub/Makefile      |   3 +-
> >  drivers/firmware/efi/libstub/alignedmem.c  |  57 ++++++
> >  drivers/firmware/efi/libstub/arm64-stub.c  |  92 +++-------
> >  drivers/firmware/efi/libstub/efistub.h     |  18 +-
> >  drivers/firmware/efi/libstub/mem.c         | 191 +-------------------
> >  drivers/firmware/efi/libstub/randomalloc.c |   6 +-
> >  drivers/firmware/efi/libstub/relocate.c    | 174 ++++++++++++++++++
> >  9 files changed, 280 insertions(+), 266 deletions(-)
> >  create mode 100644 drivers/firmware/efi/libstub/alignedmem.c
> >  create mode 100644 drivers/firmware/efi/libstub/relocate.c
> >
> > --
> > 2.17.1
> >
>
> Oops. I just noticed this series after I sent out a v2.
> I see that efi_low_alloc is removed now and the handle_kernel_image is
> simplified for arm64.
> I will update the risc-v uefi series accordingly. Sorry for the noise.
>

No worries. And apologies for making this a moving target :-)

I realised you will probably need to bring back efi_low_alloc() as a
global symbol, as I don't think you will be able to switch to
efi_relocate_kernel().

In any case, this series is just set of patches on the list, so feel
free to propose changes if they are making your life too difficult.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/8] efi/libstub/arm64: replace 'preferred' offset with alignment check

[Atish Patra]

On Mon, Apr 13, 2020 at 8:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> The notion of a 'preferred' load offset for the kernel dates back to the
> times when the kernel's primary mapping overlapped with the linear region,
> and memory below it could not be used at all.
>
> Today, the arm64 kernel does not really care where it is loaded in physical
> memory, as long as the alignment requirements are met, and so there is no

Just for my understanding, why do we need a TEXT_OFFSET in that case ?
Is it just to provide an option for SoC vendors ?

> point in unconditionally moving the kernel to a new location in memory at
> boot. Instead, we can
> - check for a KASLR seed, and randomly reallocate the kernel if one is
>   provided
> - otherwise, check whether the alignment requirements are met for the
>   current placement of the kernel, and just run it in place if they are
> - finally, do an ordinary page allocation and reallocate the kernel to a
>   suitably aligned buffer anywhere in memory.
>
> By the same reasoning, there is no need to take TEXT_OFFSET into account
> if it is a round multiple of the minimum alignment, which is the usual
> case for relocatable kernels with TEXT_OFFSET randomization disabled.
> Otherwise, it suffices to use the relative misaligment of TEXT_OFFSET
> when reallocating the kernel.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/firmware/efi/libstub/arm64-stub.c | 62 ++++++++------------
>  1 file changed, 25 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> index fc9f8ab533a7..cfd535c13242 100644
> --- a/drivers/firmware/efi/libstub/arm64-stub.c
> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> @@ -34,6 +34,15 @@ efi_status_t check_platform_features(void)
>         return EFI_SUCCESS;
>  }
>
> +/*
> + * Relocatable kernels can fix up the misalignment with respect to
> + * MIN_KIMG_ALIGN, so they only require a minimum alignment of EFI_KIMG_ALIGN
> + * (which accounts for the alignment of statically allocated objects such as
> + * the swapper stack.)
> + */
> +static const u64 min_kimg_align = IS_ENABLED(CONFIG_RELOCATABLE) ? EFI_KIMG_ALIGN
> +                                                                : MIN_KIMG_ALIGN;
> +
>  efi_status_t handle_kernel_image(unsigned long *image_addr,
>                                  unsigned long *image_size,
>                                  unsigned long *reserve_addr,
> @@ -43,7 +52,6 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>  {
>         efi_status_t status;
>         unsigned long kernel_size, kernel_memsize = 0;
> -       unsigned long preferred_offset;
>         u64 phys_seed = 0;
>
>         if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
> @@ -61,14 +69,8 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>                 }
>         }
>
> -       /*
> -        * 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, MIN_KIMG_ALIGN) + TEXT_OFFSET;
> -       if (preferred_offset < dram_base)
> -               preferred_offset += MIN_KIMG_ALIGN;
> +       if (image->image_base != _text)
> +               pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n");
>
>         kernel_size = _edata - _text;
>         kernel_memsize = kernel_size + (_end - _edata);
> @@ -103,46 +105,32 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>
>                 *image_addr = *reserve_addr + offset;
>         } else {
> -               /*
> -                * Else, 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 = (unsigned long)_text;
> -               if (*image_addr == preferred_offset)
> -                       return EFI_SUCCESS;
> -
> -               *image_addr = *reserve_addr = preferred_offset;
> -               *reserve_size = round_up(kernel_memsize, EFI_ALLOC_ALIGN);
> -
> -               status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
> -                                    EFI_LOADER_DATA,
> -                                    *reserve_size / EFI_PAGE_SIZE,
> -                                    (efi_physical_addr_t *)reserve_addr);
> +               status = EFI_OUT_OF_RESOURCES;
>         }
>
>         if (status != EFI_SUCCESS) {
> -               *reserve_size = kernel_memsize + TEXT_OFFSET;
> +               if (IS_ALIGNED((u64)_text - TEXT_OFFSET, min_kimg_align)) {
> +                       /*
> +                        * Just execute from wherever we were loaded by the
> +                        * UEFI PE/COFF loader if the alignment is suitable.
> +                        */
> +                       *image_addr = (u64)_text;
> +                       *reserve_size = 0;
> +                       return EFI_SUCCESS;
> +               }
> +
> +               *reserve_size = kernel_memsize + TEXT_OFFSET % min_kimg_align;
>                 status = efi_low_alloc(*reserve_size,
> -                                      MIN_KIMG_ALIGN, reserve_addr);
> +                                      min_kimg_align, reserve_addr);
>
>                 if (status != EFI_SUCCESS) {
>                         pr_efi_err("Failed to relocate kernel\n");
>                         *reserve_size = 0;
>                         return status;
>                 }
> -               *image_addr = *reserve_addr + TEXT_OFFSET;
> +               *image_addr = *reserve_addr + TEXT_OFFSET % min_kimg_align;
>         }
>
> -       if (image->image_base != _text)
> -               pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n");
> -
>         memcpy((void *)*image_addr, _text, kernel_size);
>
>         return EFI_SUCCESS;
> --
> 2.17.1
>

Looks good to me. FWIW,

Reviewed-by: Atish Patra <atish.patra@wdc.com>

-- 
Regards,
Atish

Re: [PATCH v2 3/8] efi/libstub/arm64: replace 'preferred' offset with alignment check

[Atish Patra]

On Mon, Apr 13, 2020 at 8:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> The notion of a 'preferred' load offset for the kernel dates back to the
> times when the kernel's primary mapping overlapped with the linear region,
> and memory below it could not be used at all.
>
> Today, the arm64 kernel does not really care where it is loaded in physical
> memory, as long as the alignment requirements are met, and so there is no

Just for my understanding, why do we need a TEXT_OFFSET in that case ?
Is it just to provide an option for SoC vendors ?

> point in unconditionally moving the kernel to a new location in memory at
> boot. Instead, we can
> - check for a KASLR seed, and randomly reallocate the kernel if one is
>   provided
> - otherwise, check whether the alignment requirements are met for the
>   current placement of the kernel, and just run it in place if they are
> - finally, do an ordinary page allocation and reallocate the kernel to a
>   suitably aligned buffer anywhere in memory.
>
> By the same reasoning, there is no need to take TEXT_OFFSET into account
> if it is a round multiple of the minimum alignment, which is the usual
> case for relocatable kernels with TEXT_OFFSET randomization disabled.
> Otherwise, it suffices to use the relative misaligment of TEXT_OFFSET
> when reallocating the kernel.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/firmware/efi/libstub/arm64-stub.c | 62 ++++++++------------
>  1 file changed, 25 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> index fc9f8ab533a7..cfd535c13242 100644
> --- a/drivers/firmware/efi/libstub/arm64-stub.c
> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> @@ -34,6 +34,15 @@ efi_status_t check_platform_features(void)
>         return EFI_SUCCESS;
>  }
>
> +/*
> + * Relocatable kernels can fix up the misalignment with respect to
> + * MIN_KIMG_ALIGN, so they only require a minimum alignment of EFI_KIMG_ALIGN
> + * (which accounts for the alignment of statically allocated objects such as
> + * the swapper stack.)
> + */
> +static const u64 min_kimg_align = IS_ENABLED(CONFIG_RELOCATABLE) ? EFI_KIMG_ALIGN
> +                                                                : MIN_KIMG_ALIGN;
> +
>  efi_status_t handle_kernel_image(unsigned long *image_addr,
>                                  unsigned long *image_size,
>                                  unsigned long *reserve_addr,
> @@ -43,7 +52,6 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>  {
>         efi_status_t status;
>         unsigned long kernel_size, kernel_memsize = 0;
> -       unsigned long preferred_offset;
>         u64 phys_seed = 0;
>
>         if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
> @@ -61,14 +69,8 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>                 }
>         }
>
> -       /*
> -        * 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, MIN_KIMG_ALIGN) + TEXT_OFFSET;
> -       if (preferred_offset < dram_base)
> -               preferred_offset += MIN_KIMG_ALIGN;
> +       if (image->image_base != _text)
> +               pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n");
>
>         kernel_size = _edata - _text;
>         kernel_memsize = kernel_size + (_end - _edata);
> @@ -103,46 +105,32 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>
>                 *image_addr = *reserve_addr + offset;
>         } else {
> -               /*
> -                * Else, 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 = (unsigned long)_text;
> -               if (*image_addr == preferred_offset)
> -                       return EFI_SUCCESS;
> -
> -               *image_addr = *reserve_addr = preferred_offset;
> -               *reserve_size = round_up(kernel_memsize, EFI_ALLOC_ALIGN);
> -
> -               status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
> -                                    EFI_LOADER_DATA,
> -                                    *reserve_size / EFI_PAGE_SIZE,
> -                                    (efi_physical_addr_t *)reserve_addr);
> +               status = EFI_OUT_OF_RESOURCES;
>         }
>
>         if (status != EFI_SUCCESS) {
> -               *reserve_size = kernel_memsize + TEXT_OFFSET;
> +               if (IS_ALIGNED((u64)_text - TEXT_OFFSET, min_kimg_align)) {
> +                       /*
> +                        * Just execute from wherever we were loaded by the
> +                        * UEFI PE/COFF loader if the alignment is suitable.
> +                        */
> +                       *image_addr = (u64)_text;
> +                       *reserve_size = 0;
> +                       return EFI_SUCCESS;
> +               }
> +
> +               *reserve_size = kernel_memsize + TEXT_OFFSET % min_kimg_align;
>                 status = efi_low_alloc(*reserve_size,
> -                                      MIN_KIMG_ALIGN, reserve_addr);
> +                                      min_kimg_align, reserve_addr);
>
>                 if (status != EFI_SUCCESS) {
>                         pr_efi_err("Failed to relocate kernel\n");
>                         *reserve_size = 0;
>                         return status;
>                 }
> -               *image_addr = *reserve_addr + TEXT_OFFSET;
> +               *image_addr = *reserve_addr + TEXT_OFFSET % min_kimg_align;
>         }
>
> -       if (image->image_base != _text)
> -               pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n");
> -
>         memcpy((void *)*image_addr, _text, kernel_size);
>
>         return EFI_SUCCESS;
> --
> 2.17.1
>

Looks good to me. FWIW,

Reviewed-by: Atish Patra <atish.patra@wdc.com>

-- 
Regards,
Atish

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 7/8] efi/libstub/arm64: switch to ordinary page allocator for kernel image

[Atish Patra]

On Mon, Apr 13, 2020 at 8:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> It is no longer necessary to locate the kernel as low as possible in
> physical memory, and so we can switch from efi_low_alloc() [which is
> a rather nasty concoction on top of GetMemoryMap()] to a new helper
> called efi_allocate_pages_aligned(), which simply rounds up the size
> to account for the alignment, and frees the misaligned pages again.
>
> So considering that the kernel can live anywhere in the physical
> address space, as long as its alignment requirements are met, let's
> switch to efi_allocate_pages_aligned() to allocate the pages.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/firmware/efi/libstub/arm64-stub.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> index 6fc3bd9a56db..99b67e88a33b 100644
> --- a/drivers/firmware/efi/libstub/arm64-stub.c
> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> @@ -98,8 +98,8 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>                         return EFI_SUCCESS;
>                 }
>
> -               status = efi_low_alloc(*reserve_size,
> -                                      min_kimg_align, reserve_addr);
> +               status = efi_allocate_pages_aligned(*reserve_size, reserve_addr,
> +                                                   ULONG_MAX, min_kimg_align);
>
>                 if (status != EFI_SUCCESS) {
>                         pr_efi_err("Failed to relocate kernel\n");
> --
> 2.17.1
>

Reviewed-by: Atish Patra <atish.patra@wdc.com>
-- 
Regards,
Atish

Re: [PATCH v2 6/8] efi/libstub: add API function to allocate aligned memory

[Atish Patra]

On Mon, Apr 13, 2020 at 8:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Break out the code to create an aligned page allocation from mem.c
> and move it into a function efi_allocate_pages_aligned() in alignedmem.c.
> Update efi_allocate_pages() to invoke it unless the minimum alignment
> equals the EFI page size (4 KB), in which case the ordinary page
> allocator is sufficient. This way, efi_allocate_pages_aligned() will
> only be pulled into the build if it is actually being used (which will
> be on arm64 only in the immediate future)
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/firmware/efi/libstub/Makefile     |  3 +-
>  drivers/firmware/efi/libstub/alignedmem.c | 57 ++++++++++++++++++++
>  drivers/firmware/efi/libstub/efistub.h    |  3 ++
>  drivers/firmware/efi/libstub/mem.c        | 25 ++++-----
>  4 files changed, 71 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index 094eabdecfe6..1a09b9445394 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -42,7 +42,8 @@ KCOV_INSTRUMENT                       := n
>
>  lib-y                          := efi-stub-helper.o gop.o secureboot.o tpm.o \
>                                    file.o mem.o random.o randomalloc.o pci.o \
> -                                  skip_spaces.o lib-cmdline.o lib-ctype.o
> +                                  skip_spaces.o lib-cmdline.o lib-ctype.o \
> +                                  alignedmem.o
>
>  # include the stub's generic dependencies from lib/ when building for ARM/arm64
>  arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
> diff --git a/drivers/firmware/efi/libstub/alignedmem.c b/drivers/firmware/efi/libstub/alignedmem.c
> new file mode 100644
> index 000000000000..cc89c4d6196f
> --- /dev/null
> +++ b/drivers/firmware/efi/libstub/alignedmem.c
> @@ -0,0 +1,57 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/efi.h>
> +#include <asm/efi.h>
> +
> +#include "efistub.h"
> +
> +/**
> + * efi_allocate_pages_aligned() - Allocate memory pages
> + * @size:      minimum number of bytes to allocate
> + * @addr:      On return the address of the first allocated page. The first
> + *             allocated page has alignment EFI_ALLOC_ALIGN which is an
> + *             architecture dependent multiple of the page size.
> + * @max:       the address that the last allocated memory page shall not
> + *             exceed
> + * @align:     minimum alignment of the base of the allocation
> + *
> + * Allocate pages as EFI_LOADER_DATA. The allocated pages are aligned according
> + * to @align, which should be >= EFI_ALLOC_ALIGN. The last allocated page will
> + * not exceed the address given by @max.
> + *
> + * Return:     status code
> + */
> +efi_status_t efi_allocate_pages_aligned(unsigned long size, unsigned long *addr,
> +                                       unsigned long max, unsigned long align)
> +{
> +       efi_physical_addr_t alloc_addr;
> +       efi_status_t status;
> +       int slack;
> +
> +       if (align < EFI_ALLOC_ALIGN)
> +               align = EFI_ALLOC_ALIGN;
> +
> +       alloc_addr = ALIGN_DOWN(max + 1, align) - 1;
> +       size = round_up(size, EFI_ALLOC_ALIGN);
> +       slack = align / EFI_PAGE_SIZE - 1;
> +
> +       status = efi_bs_call(allocate_pages, EFI_ALLOCATE_MAX_ADDRESS,
> +                            EFI_LOADER_DATA, size / EFI_PAGE_SIZE + slack,
> +                            &alloc_addr);
> +       if (status != EFI_SUCCESS)
> +               return status;
> +
> +       *addr = ALIGN((unsigned long)alloc_addr, align);
> +
> +       if (slack > 0) {
> +               int l = (alloc_addr % align) / EFI_PAGE_SIZE;
> +
> +               if (l) {
> +                       efi_bs_call(free_pages, alloc_addr, slack - l + 1);
> +                       slack = l - 1;
> +               }
> +               if (slack)
> +                       efi_bs_call(free_pages, *addr + size, slack);
> +       }
> +       return EFI_SUCCESS;
> +}
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index 4844c3bd40df..5dcfadcf2bc1 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -641,6 +641,9 @@ efi_status_t efi_low_alloc(unsigned long size, unsigned long align,
>  efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
>                                 unsigned long max);
>
> +efi_status_t efi_allocate_pages_aligned(unsigned long size, unsigned long *addr,
> +                                       unsigned long max, unsigned long align);
> +
>  efi_status_t efi_relocate_kernel(unsigned long *image_addr,
>                                  unsigned long image_size,
>                                  unsigned long alloc_size,
> diff --git a/drivers/firmware/efi/libstub/mem.c b/drivers/firmware/efi/libstub/mem.c
> index 869a79c8946f..0020b0fa9587 100644
> --- a/drivers/firmware/efi/libstub/mem.c
> +++ b/drivers/firmware/efi/libstub/mem.c
> @@ -93,31 +93,24 @@ efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
>  efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
>                                 unsigned long max)
>  {
> -       efi_physical_addr_t alloc_addr = ALIGN_DOWN(max + 1, EFI_ALLOC_ALIGN) - 1;
> -       int slack = EFI_ALLOC_ALIGN / EFI_PAGE_SIZE - 1;
> +       efi_physical_addr_t alloc_addr;
>         efi_status_t status;
>
> -       size = round_up(size, EFI_ALLOC_ALIGN);
> +       if (EFI_ALLOC_ALIGN > EFI_PAGE_SIZE)
> +               return efi_allocate_pages_aligned(size, addr, max,
> +                                                 EFI_ALLOC_ALIGN);
> +
> +       alloc_addr = ALIGN_DOWN(max + 1, EFI_ALLOC_ALIGN) - 1;
>         status = efi_bs_call(allocate_pages, EFI_ALLOCATE_MAX_ADDRESS,
> -                            EFI_LOADER_DATA, size / EFI_PAGE_SIZE + slack,
> +                            EFI_LOADER_DATA, DIV_ROUND_UP(size, EFI_PAGE_SIZE),
>                              &alloc_addr);
>         if (status != EFI_SUCCESS)
>                 return status;
>
> -       *addr = ALIGN((unsigned long)alloc_addr, EFI_ALLOC_ALIGN);
> -
> -       if (slack > 0) {
> -               int l = (alloc_addr % EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE;
> -
> -               if (l) {
> -                       efi_bs_call(free_pages, alloc_addr, slack - l + 1);
> -                       slack = l - 1;
> -               }
> -               if (slack)
> -                       efi_bs_call(free_pages, *addr + size, slack);
> -       }
> +       *addr = alloc_addr;
>         return EFI_SUCCESS;
>  }
> +
>  /**
>   * efi_low_alloc_above() - allocate pages at or above given address
>   * @size:      size of the memory area to allocate
> --
> 2.17.1
>

Reviewed-by: Atish Patra <atish.patra@wdc.com>
-- 
Regards,
Atish

Re: [PATCH v2 6/8] efi/libstub: add API function to allocate aligned memory

[Atish Patra]

On Mon, Apr 13, 2020 at 8:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Break out the code to create an aligned page allocation from mem.c
> and move it into a function efi_allocate_pages_aligned() in alignedmem.c.
> Update efi_allocate_pages() to invoke it unless the minimum alignment
> equals the EFI page size (4 KB), in which case the ordinary page
> allocator is sufficient. This way, efi_allocate_pages_aligned() will
> only be pulled into the build if it is actually being used (which will
> be on arm64 only in the immediate future)
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/firmware/efi/libstub/Makefile     |  3 +-
>  drivers/firmware/efi/libstub/alignedmem.c | 57 ++++++++++++++++++++
>  drivers/firmware/efi/libstub/efistub.h    |  3 ++
>  drivers/firmware/efi/libstub/mem.c        | 25 ++++-----
>  4 files changed, 71 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index 094eabdecfe6..1a09b9445394 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -42,7 +42,8 @@ KCOV_INSTRUMENT                       := n
>
>  lib-y                          := efi-stub-helper.o gop.o secureboot.o tpm.o \
>                                    file.o mem.o random.o randomalloc.o pci.o \
> -                                  skip_spaces.o lib-cmdline.o lib-ctype.o
> +                                  skip_spaces.o lib-cmdline.o lib-ctype.o \
> +                                  alignedmem.o
>
>  # include the stub's generic dependencies from lib/ when building for ARM/arm64
>  arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
> diff --git a/drivers/firmware/efi/libstub/alignedmem.c b/drivers/firmware/efi/libstub/alignedmem.c
> new file mode 100644
> index 000000000000..cc89c4d6196f
> --- /dev/null
> +++ b/drivers/firmware/efi/libstub/alignedmem.c
> @@ -0,0 +1,57 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/efi.h>
> +#include <asm/efi.h>
> +
> +#include "efistub.h"
> +
> +/**
> + * efi_allocate_pages_aligned() - Allocate memory pages
> + * @size:      minimum number of bytes to allocate
> + * @addr:      On return the address of the first allocated page. The first
> + *             allocated page has alignment EFI_ALLOC_ALIGN which is an
> + *             architecture dependent multiple of the page size.
> + * @max:       the address that the last allocated memory page shall not
> + *             exceed
> + * @align:     minimum alignment of the base of the allocation
> + *
> + * Allocate pages as EFI_LOADER_DATA. The allocated pages are aligned according
> + * to @align, which should be >= EFI_ALLOC_ALIGN. The last allocated page will
> + * not exceed the address given by @max.
> + *
> + * Return:     status code
> + */
> +efi_status_t efi_allocate_pages_aligned(unsigned long size, unsigned long *addr,
> +                                       unsigned long max, unsigned long align)
> +{
> +       efi_physical_addr_t alloc_addr;
> +       efi_status_t status;
> +       int slack;
> +
> +       if (align < EFI_ALLOC_ALIGN)
> +               align = EFI_ALLOC_ALIGN;
> +
> +       alloc_addr = ALIGN_DOWN(max + 1, align) - 1;
> +       size = round_up(size, EFI_ALLOC_ALIGN);
> +       slack = align / EFI_PAGE_SIZE - 1;
> +
> +       status = efi_bs_call(allocate_pages, EFI_ALLOCATE_MAX_ADDRESS,
> +                            EFI_LOADER_DATA, size / EFI_PAGE_SIZE + slack,
> +                            &alloc_addr);
> +       if (status != EFI_SUCCESS)
> +               return status;
> +
> +       *addr = ALIGN((unsigned long)alloc_addr, align);
> +
> +       if (slack > 0) {
> +               int l = (alloc_addr % align) / EFI_PAGE_SIZE;
> +
> +               if (l) {
> +                       efi_bs_call(free_pages, alloc_addr, slack - l + 1);
> +                       slack = l - 1;
> +               }
> +               if (slack)
> +                       efi_bs_call(free_pages, *addr + size, slack);
> +       }
> +       return EFI_SUCCESS;
> +}
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index 4844c3bd40df..5dcfadcf2bc1 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -641,6 +641,9 @@ efi_status_t efi_low_alloc(unsigned long size, unsigned long align,
>  efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
>                                 unsigned long max);
>
> +efi_status_t efi_allocate_pages_aligned(unsigned long size, unsigned long *addr,
> +                                       unsigned long max, unsigned long align);
> +
>  efi_status_t efi_relocate_kernel(unsigned long *image_addr,
>                                  unsigned long image_size,
>                                  unsigned long alloc_size,
> diff --git a/drivers/firmware/efi/libstub/mem.c b/drivers/firmware/efi/libstub/mem.c
> index 869a79c8946f..0020b0fa9587 100644
> --- a/drivers/firmware/efi/libstub/mem.c
> +++ b/drivers/firmware/efi/libstub/mem.c
> @@ -93,31 +93,24 @@ efi_status_t efi_get_memory_map(struct efi_boot_memmap *map)
>  efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
>                                 unsigned long max)
>  {
> -       efi_physical_addr_t alloc_addr = ALIGN_DOWN(max + 1, EFI_ALLOC_ALIGN) - 1;
> -       int slack = EFI_ALLOC_ALIGN / EFI_PAGE_SIZE - 1;
> +       efi_physical_addr_t alloc_addr;
>         efi_status_t status;
>
> -       size = round_up(size, EFI_ALLOC_ALIGN);
> +       if (EFI_ALLOC_ALIGN > EFI_PAGE_SIZE)
> +               return efi_allocate_pages_aligned(size, addr, max,
> +                                                 EFI_ALLOC_ALIGN);
> +
> +       alloc_addr = ALIGN_DOWN(max + 1, EFI_ALLOC_ALIGN) - 1;
>         status = efi_bs_call(allocate_pages, EFI_ALLOCATE_MAX_ADDRESS,
> -                            EFI_LOADER_DATA, size / EFI_PAGE_SIZE + slack,
> +                            EFI_LOADER_DATA, DIV_ROUND_UP(size, EFI_PAGE_SIZE),
>                              &alloc_addr);
>         if (status != EFI_SUCCESS)
>                 return status;
>
> -       *addr = ALIGN((unsigned long)alloc_addr, EFI_ALLOC_ALIGN);
> -
> -       if (slack > 0) {
> -               int l = (alloc_addr % EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE;
> -
> -               if (l) {
> -                       efi_bs_call(free_pages, alloc_addr, slack - l + 1);
> -                       slack = l - 1;
> -               }
> -               if (slack)
> -                       efi_bs_call(free_pages, *addr + size, slack);
> -       }
> +       *addr = alloc_addr;
>         return EFI_SUCCESS;
>  }
> +
>  /**
>   * efi_low_alloc_above() - allocate pages at or above given address
>   * @size:      size of the memory area to allocate
> --
> 2.17.1
>

Reviewed-by: Atish Patra <atish.patra@wdc.com>
-- 
Regards,
Atish

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/8] efi/libstub/arm64: replace 'preferred' offset with alignment check

[Ard Biesheuvel]

On Wed, 15 Apr 2020 at 01:21, Atish Patra <atishp@atishpatra.org> wrote:
>
> On Mon, Apr 13, 2020 at 8:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > The notion of a 'preferred' load offset for the kernel dates back to the
> > times when the kernel's primary mapping overlapped with the linear region,
> > and memory below it could not be used at all.
> >
> > Today, the arm64 kernel does not really care where it is loaded in physical
> > memory, as long as the alignment requirements are met, and so there is no
>
> Just for my understanding, why do we need a TEXT_OFFSET in that case ?
> Is it just to provide an option for SoC vendors ?
>

TEXT_OFFSET has become an unfortunate historical artifact, and we
would remove it if we could. The only reason we are keeping it is for
compatibility with very old bootloaders that assume TEXT_OFFSET=512k
without looking at the header.

In fact, we could simply set it to 0x0 on relocatable builds. I'll
send out a patch for that.

> > point in unconditionally moving the kernel to a new location in memory at
> > boot. Instead, we can
> > - check for a KASLR seed, and randomly reallocate the kernel if one is
> >   provided
> > - otherwise, check whether the alignment requirements are met for the
> >   current placement of the kernel, and just run it in place if they are
> > - finally, do an ordinary page allocation and reallocate the kernel to a
> >   suitably aligned buffer anywhere in memory.
> >
> > By the same reasoning, there is no need to take TEXT_OFFSET into account
> > if it is a round multiple of the minimum alignment, which is the usual
> > case for relocatable kernels with TEXT_OFFSET randomization disabled.
> > Otherwise, it suffices to use the relative misaligment of TEXT_OFFSET
> > when reallocating the kernel.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  drivers/firmware/efi/libstub/arm64-stub.c | 62 ++++++++------------
> >  1 file changed, 25 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> > index fc9f8ab533a7..cfd535c13242 100644
> > --- a/drivers/firmware/efi/libstub/arm64-stub.c
> > +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> > @@ -34,6 +34,15 @@ efi_status_t check_platform_features(void)
> >         return EFI_SUCCESS;
> >  }
> >
> > +/*
> > + * Relocatable kernels can fix up the misalignment with respect to
> > + * MIN_KIMG_ALIGN, so they only require a minimum alignment of EFI_KIMG_ALIGN
> > + * (which accounts for the alignment of statically allocated objects such as
> > + * the swapper stack.)
> > + */
> > +static const u64 min_kimg_align = IS_ENABLED(CONFIG_RELOCATABLE) ? EFI_KIMG_ALIGN
> > +                                                                : MIN_KIMG_ALIGN;
> > +
> >  efi_status_t handle_kernel_image(unsigned long *image_addr,
> >                                  unsigned long *image_size,
> >                                  unsigned long *reserve_addr,
> > @@ -43,7 +52,6 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> >  {
> >         efi_status_t status;
> >         unsigned long kernel_size, kernel_memsize = 0;
> > -       unsigned long preferred_offset;
> >         u64 phys_seed = 0;
> >
> >         if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
> > @@ -61,14 +69,8 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> >                 }
> >         }
> >
> > -       /*
> > -        * 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, MIN_KIMG_ALIGN) + TEXT_OFFSET;
> > -       if (preferred_offset < dram_base)
> > -               preferred_offset += MIN_KIMG_ALIGN;
> > +       if (image->image_base != _text)
> > +               pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n");
> >
> >         kernel_size = _edata - _text;
> >         kernel_memsize = kernel_size + (_end - _edata);
> > @@ -103,46 +105,32 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> >
> >                 *image_addr = *reserve_addr + offset;
> >         } else {
> > -               /*
> > -                * Else, 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 = (unsigned long)_text;
> > -               if (*image_addr == preferred_offset)
> > -                       return EFI_SUCCESS;
> > -
> > -               *image_addr = *reserve_addr = preferred_offset;
> > -               *reserve_size = round_up(kernel_memsize, EFI_ALLOC_ALIGN);
> > -
> > -               status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
> > -                                    EFI_LOADER_DATA,
> > -                                    *reserve_size / EFI_PAGE_SIZE,
> > -                                    (efi_physical_addr_t *)reserve_addr);
> > +               status = EFI_OUT_OF_RESOURCES;
> >         }
> >
> >         if (status != EFI_SUCCESS) {
> > -               *reserve_size = kernel_memsize + TEXT_OFFSET;
> > +               if (IS_ALIGNED((u64)_text - TEXT_OFFSET, min_kimg_align)) {
> > +                       /*
> > +                        * Just execute from wherever we were loaded by the
> > +                        * UEFI PE/COFF loader if the alignment is suitable.
> > +                        */
> > +                       *image_addr = (u64)_text;
> > +                       *reserve_size = 0;
> > +                       return EFI_SUCCESS;
> > +               }
> > +
> > +               *reserve_size = kernel_memsize + TEXT_OFFSET % min_kimg_align;
> >                 status = efi_low_alloc(*reserve_size,
> > -                                      MIN_KIMG_ALIGN, reserve_addr);
> > +                                      min_kimg_align, reserve_addr);
> >
> >                 if (status != EFI_SUCCESS) {
> >                         pr_efi_err("Failed to relocate kernel\n");
> >                         *reserve_size = 0;
> >                         return status;
> >                 }
> > -               *image_addr = *reserve_addr + TEXT_OFFSET;
> > +               *image_addr = *reserve_addr + TEXT_OFFSET % min_kimg_align;
> >         }
> >
> > -       if (image->image_base != _text)
> > -               pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n");
> > -
> >         memcpy((void *)*image_addr, _text, kernel_size);
> >
> >         return EFI_SUCCESS;
> > --
> > 2.17.1
> >
>
> Looks good to me. FWIW,
>
> Reviewed-by: Atish Patra <atish.patra@wdc.com>
>


Thanks.

Re: [PATCH v2 3/8] efi/libstub/arm64: replace 'preferred' offset with alignment check

[Ard Biesheuvel]

On Wed, 15 Apr 2020 at 01:21, Atish Patra <atishp@atishpatra.org> wrote:
>
> On Mon, Apr 13, 2020 at 8:55 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > The notion of a 'preferred' load offset for the kernel dates back to the
> > times when the kernel's primary mapping overlapped with the linear region,
> > and memory below it could not be used at all.
> >
> > Today, the arm64 kernel does not really care where it is loaded in physical
> > memory, as long as the alignment requirements are met, and so there is no
>
> Just for my understanding, why do we need a TEXT_OFFSET in that case ?
> Is it just to provide an option for SoC vendors ?
>

TEXT_OFFSET has become an unfortunate historical artifact, and we
would remove it if we could. The only reason we are keeping it is for
compatibility with very old bootloaders that assume TEXT_OFFSET=512k
without looking at the header.

In fact, we could simply set it to 0x0 on relocatable builds. I'll
send out a patch for that.

> > point in unconditionally moving the kernel to a new location in memory at
> > boot. Instead, we can
> > - check for a KASLR seed, and randomly reallocate the kernel if one is
> >   provided
> > - otherwise, check whether the alignment requirements are met for the
> >   current placement of the kernel, and just run it in place if they are
> > - finally, do an ordinary page allocation and reallocate the kernel to a
> >   suitably aligned buffer anywhere in memory.
> >
> > By the same reasoning, there is no need to take TEXT_OFFSET into account
> > if it is a round multiple of the minimum alignment, which is the usual
> > case for relocatable kernels with TEXT_OFFSET randomization disabled.
> > Otherwise, it suffices to use the relative misaligment of TEXT_OFFSET
> > when reallocating the kernel.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  drivers/firmware/efi/libstub/arm64-stub.c | 62 ++++++++------------
> >  1 file changed, 25 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> > index fc9f8ab533a7..cfd535c13242 100644
> > --- a/drivers/firmware/efi/libstub/arm64-stub.c
> > +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> > @@ -34,6 +34,15 @@ efi_status_t check_platform_features(void)
> >         return EFI_SUCCESS;
> >  }
> >
> > +/*
> > + * Relocatable kernels can fix up the misalignment with respect to
> > + * MIN_KIMG_ALIGN, so they only require a minimum alignment of EFI_KIMG_ALIGN
> > + * (which accounts for the alignment of statically allocated objects such as
> > + * the swapper stack.)
> > + */
> > +static const u64 min_kimg_align = IS_ENABLED(CONFIG_RELOCATABLE) ? EFI_KIMG_ALIGN
> > +                                                                : MIN_KIMG_ALIGN;
> > +
> >  efi_status_t handle_kernel_image(unsigned long *image_addr,
> >                                  unsigned long *image_size,
> >                                  unsigned long *reserve_addr,
> > @@ -43,7 +52,6 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> >  {
> >         efi_status_t status;
> >         unsigned long kernel_size, kernel_memsize = 0;
> > -       unsigned long preferred_offset;
> >         u64 phys_seed = 0;
> >
> >         if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
> > @@ -61,14 +69,8 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> >                 }
> >         }
> >
> > -       /*
> > -        * 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, MIN_KIMG_ALIGN) + TEXT_OFFSET;
> > -       if (preferred_offset < dram_base)
> > -               preferred_offset += MIN_KIMG_ALIGN;
> > +       if (image->image_base != _text)
> > +               pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n");
> >
> >         kernel_size = _edata - _text;
> >         kernel_memsize = kernel_size + (_end - _edata);
> > @@ -103,46 +105,32 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> >
> >                 *image_addr = *reserve_addr + offset;
> >         } else {
> > -               /*
> > -                * Else, 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 = (unsigned long)_text;
> > -               if (*image_addr == preferred_offset)
> > -                       return EFI_SUCCESS;
> > -
> > -               *image_addr = *reserve_addr = preferred_offset;
> > -               *reserve_size = round_up(kernel_memsize, EFI_ALLOC_ALIGN);
> > -
> > -               status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS,
> > -                                    EFI_LOADER_DATA,
> > -                                    *reserve_size / EFI_PAGE_SIZE,
> > -                                    (efi_physical_addr_t *)reserve_addr);
> > +               status = EFI_OUT_OF_RESOURCES;
> >         }
> >
> >         if (status != EFI_SUCCESS) {
> > -               *reserve_size = kernel_memsize + TEXT_OFFSET;
> > +               if (IS_ALIGNED((u64)_text - TEXT_OFFSET, min_kimg_align)) {
> > +                       /*
> > +                        * Just execute from wherever we were loaded by the
> > +                        * UEFI PE/COFF loader if the alignment is suitable.
> > +                        */
> > +                       *image_addr = (u64)_text;
> > +                       *reserve_size = 0;
> > +                       return EFI_SUCCESS;
> > +               }
> > +
> > +               *reserve_size = kernel_memsize + TEXT_OFFSET % min_kimg_align;
> >                 status = efi_low_alloc(*reserve_size,
> > -                                      MIN_KIMG_ALIGN, reserve_addr);
> > +                                      min_kimg_align, reserve_addr);
> >
> >                 if (status != EFI_SUCCESS) {
> >                         pr_efi_err("Failed to relocate kernel\n");
> >                         *reserve_size = 0;
> >                         return status;
> >                 }
> > -               *image_addr = *reserve_addr + TEXT_OFFSET;
> > +               *image_addr = *reserve_addr + TEXT_OFFSET % min_kimg_align;
> >         }
> >
> > -       if (image->image_base != _text)
> > -               pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n");
> > -
> >         memcpy((void *)*image_addr, _text, kernel_size);
> >
> >         return EFI_SUCCESS;
> > --
> > 2.17.1
> >
>
> Looks good to me. FWIW,
>
> Reviewed-by: Atish Patra <atish.patra@wdc.com>
>


Thanks.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 5/8] efi/libstub/arm64: align PE/COFF sections to segment alignment

[Ard Biesheuvel]

On Mon, 13 Apr 2020 at 17:55, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> The arm64 kernel's segment alignment is fixed at 64 KB for any page
> size, and relocatable kernels are able to fix up any misalignment of
> the kernel image with respect to the 2 MB section alignment that is
> mandated by the arm64 boot protocol.
>
> Let's increase the PE/COFF section alignment to the same value, so that
> kernels loaded by the UEFI PE/COFF loader are guaranteed to end up at
> an address that doesn't require any reallocation to be done if the
> kernel is relocatable.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/arm64/kernel/efi-header.S  | 2 +-
>  arch/arm64/kernel/vmlinux.lds.S | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
>

Catalin, Will: any thoughts whether this should go via the arm64 tree
or the EFI tree? It is part of a change on the EFI side to stop
needlessly copying the kernel around before boot, but this particular
change could go through either tree, as it is not build time
dependency for the EFI changes, nor does it have a boot time impact
beyond making it more likely that the copy can be elided (but the code
still needs to deal with the possibility that it cannot be elided in
any case)



> diff --git a/arch/arm64/kernel/efi-header.S b/arch/arm64/kernel/efi-header.S
> index 914999ccaf8a..6f58998ef647 100644
> --- a/arch/arm64/kernel/efi-header.S
> +++ b/arch/arm64/kernel/efi-header.S
> @@ -32,7 +32,7 @@ optional_header:
>
>  extra_header_fields:
>         .quad   0                                       // ImageBase
> -       .long   SZ_4K                                   // SectionAlignment
> +       .long   SEGMENT_ALIGN                           // SectionAlignment
>         .long   PECOFF_FILE_ALIGNMENT                   // FileAlignment
>         .short  0                                       // MajorOperatingSystemVersion
>         .short  0                                       // MinorOperatingSystemVersion
> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> index 497f9675071d..1d399db0644f 100644
> --- a/arch/arm64/kernel/vmlinux.lds.S
> +++ b/arch/arm64/kernel/vmlinux.lds.S
> @@ -175,7 +175,7 @@ SECTIONS
>                 *(.altinstr_replacement)
>         }
>
> -       . = ALIGN(PAGE_SIZE);
> +       . = ALIGN(SEGMENT_ALIGN);
>         __inittext_end = .;
>         __initdata_begin = .;
>
> @@ -246,6 +246,7 @@ SECTIONS
>         . += INIT_DIR_SIZE;
>         init_pg_end = .;
>
> +       . = ALIGN(SEGMENT_ALIGN);
>         __pecoff_data_size = ABSOLUTE(. - __initdata_begin);
>         _end = .;
>
> --
> 2.17.1
>

Re: [PATCH v2 5/8] efi/libstub/arm64: align PE/COFF sections to segment alignment

[Ard Biesheuvel]

On Mon, 13 Apr 2020 at 17:55, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> The arm64 kernel's segment alignment is fixed at 64 KB for any page
> size, and relocatable kernels are able to fix up any misalignment of
> the kernel image with respect to the 2 MB section alignment that is
> mandated by the arm64 boot protocol.
>
> Let's increase the PE/COFF section alignment to the same value, so that
> kernels loaded by the UEFI PE/COFF loader are guaranteed to end up at
> an address that doesn't require any reallocation to be done if the
> kernel is relocatable.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/arm64/kernel/efi-header.S  | 2 +-
>  arch/arm64/kernel/vmlinux.lds.S | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
>

Catalin, Will: any thoughts whether this should go via the arm64 tree
or the EFI tree? It is part of a change on the EFI side to stop
needlessly copying the kernel around before boot, but this particular
change could go through either tree, as it is not build time
dependency for the EFI changes, nor does it have a boot time impact
beyond making it more likely that the copy can be elided (but the code
still needs to deal with the possibility that it cannot be elided in
any case)



> diff --git a/arch/arm64/kernel/efi-header.S b/arch/arm64/kernel/efi-header.S
> index 914999ccaf8a..6f58998ef647 100644
> --- a/arch/arm64/kernel/efi-header.S
> +++ b/arch/arm64/kernel/efi-header.S
> @@ -32,7 +32,7 @@ optional_header:
>
>  extra_header_fields:
>         .quad   0                                       // ImageBase
> -       .long   SZ_4K                                   // SectionAlignment
> +       .long   SEGMENT_ALIGN                           // SectionAlignment
>         .long   PECOFF_FILE_ALIGNMENT                   // FileAlignment
>         .short  0                                       // MajorOperatingSystemVersion
>         .short  0                                       // MinorOperatingSystemVersion
> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> index 497f9675071d..1d399db0644f 100644
> --- a/arch/arm64/kernel/vmlinux.lds.S
> +++ b/arch/arm64/kernel/vmlinux.lds.S
> @@ -175,7 +175,7 @@ SECTIONS
>                 *(.altinstr_replacement)
>         }
>
> -       . = ALIGN(PAGE_SIZE);
> +       . = ALIGN(SEGMENT_ALIGN);
>         __inittext_end = .;
>         __initdata_begin = .;
>
> @@ -246,6 +246,7 @@ SECTIONS
>         . += INIT_DIR_SIZE;
>         init_pg_end = .;
>
> +       . = ALIGN(SEGMENT_ALIGN);
>         __pecoff_data_size = ABSOLUTE(. - __initdata_begin);
>         _end = .;
>
> --
> 2.17.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 5/8] efi/libstub/arm64: align PE/COFF sections to segment alignment

[Will Deacon]

On Wed, Apr 22, 2020 at 11:39:39AM +0200, Ard Biesheuvel wrote:
> On Mon, 13 Apr 2020 at 17:55, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > The arm64 kernel's segment alignment is fixed at 64 KB for any page
> > size, and relocatable kernels are able to fix up any misalignment of
> > the kernel image with respect to the 2 MB section alignment that is
> > mandated by the arm64 boot protocol.
> >
> > Let's increase the PE/COFF section alignment to the same value, so that
> > kernels loaded by the UEFI PE/COFF loader are guaranteed to end up at
> > an address that doesn't require any reallocation to be done if the
> > kernel is relocatable.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  arch/arm64/kernel/efi-header.S  | 2 +-
> >  arch/arm64/kernel/vmlinux.lds.S | 3 ++-
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> >
> 
> Catalin, Will: any thoughts whether this should go via the arm64 tree
> or the EFI tree? It is part of a change on the EFI side to stop
> needlessly copying the kernel around before boot, but this particular
> change could go through either tree, as it is not build time
> dependency for the EFI changes, nor does it have a boot time impact
> beyond making it more likely that the copy can be elided (but the code
> still needs to deal with the possibility that it cannot be elided in
> any case)

I've queued this one in the arm64 tree for 5.8. Should appear on
for-next/misc in the next day or so.

Thanks,

Will

Re: [PATCH v2 5/8] efi/libstub/arm64: align PE/COFF sections to segment alignment

[Will Deacon]

On Wed, Apr 22, 2020 at 11:39:39AM +0200, Ard Biesheuvel wrote:
> On Mon, 13 Apr 2020 at 17:55, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > The arm64 kernel's segment alignment is fixed at 64 KB for any page
> > size, and relocatable kernels are able to fix up any misalignment of
> > the kernel image with respect to the 2 MB section alignment that is
> > mandated by the arm64 boot protocol.
> >
> > Let's increase the PE/COFF section alignment to the same value, so that
> > kernels loaded by the UEFI PE/COFF loader are guaranteed to end up at
> > an address that doesn't require any reallocation to be done if the
> > kernel is relocatable.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  arch/arm64/kernel/efi-header.S  | 2 +-
> >  arch/arm64/kernel/vmlinux.lds.S | 3 ++-
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> >
> 
> Catalin, Will: any thoughts whether this should go via the arm64 tree
> or the EFI tree? It is part of a change on the EFI side to stop
> needlessly copying the kernel around before boot, but this particular
> change could go through either tree, as it is not build time
> dependency for the EFI changes, nor does it have a boot time impact
> beyond making it more likely that the copy can be elided (but the code
> still needs to deal with the possibility that it cannot be elided in
> any case)

I've queued this one in the arm64 tree for 5.8. Should appear on
for-next/misc in the next day or so.

Thanks,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel