[PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Ard Biesheuvel]

Maxim reports boot failures on platforms that describe reserved memory
regions in DT that are disjoint from system DRAM, and which are converted
to EfiReservedMemory regions by the EFI subsystem in u-boot.

As it turns out, the whole notion of discovering the base of DRAM is
problematic, and it would be better to simply rely on the EFI memory
allocation routines instead, and derive the FDT and initrd allocation
limits from the actual placement of the kernel (which is what defines
the start of the linear region anyway)

Finally, we should be able to get rid of get_dram_base() entirely.
However, as RISC-V only just started using it, we will need to address
that at a later time.

Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Atish Patra <atish.patra@wdc.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Jens Wiklander <jens.wiklander@linaro.org>
Cc: Francois Ozog <francois.ozog@linaro.org>
Cc: Etienne CARRIERE <etienne.carriere@st.com>
Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
Cc: Patrice CHOTARD <patrice.chotard@st.com>
Cc: Sumit Garg <sumit.garg@linaro.org>
Cc: Grant Likely <Grant.Likely@arm.com>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
Cc: Patrick DELAUNAY <patrick.delaunay@st.com>

Ard Biesheuvel (3):
  efi/libstub: Export efi_low_alloc_above() to other units
  efi/libstub: Use low allocation for the uncompressed kernel
  efi/libstub: base FDT and initrd placement on image address not DRAM
    base

 arch/arm/include/asm/efi.h                |   6 +-
 arch/arm64/include/asm/efi.h              |   2 +-
 drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
 drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
 drivers/firmware/efi/libstub/efistub.h    |   3 +
 drivers/firmware/efi/libstub/relocate.c   |   4 +-
 6 files changed, 47 insertions(+), 147 deletions(-)

-- 
2.17.1

[PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Ard Biesheuvel]

Maxim reports boot failures on platforms that describe reserved memory
regions in DT that are disjoint from system DRAM, and which are converted
to EfiReservedMemory regions by the EFI subsystem in u-boot.

As it turns out, the whole notion of discovering the base of DRAM is
problematic, and it would be better to simply rely on the EFI memory
allocation routines instead, and derive the FDT and initrd allocation
limits from the actual placement of the kernel (which is what defines
the start of the linear region anyway)

Finally, we should be able to get rid of get_dram_base() entirely.
However, as RISC-V only just started using it, we will need to address
that at a later time.

Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Atish Patra <atish.patra@wdc.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Jens Wiklander <jens.wiklander@linaro.org>
Cc: Francois Ozog <francois.ozog@linaro.org>
Cc: Etienne CARRIERE <etienne.carriere@st.com>
Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
Cc: Patrice CHOTARD <patrice.chotard@st.com>
Cc: Sumit Garg <sumit.garg@linaro.org>
Cc: Grant Likely <Grant.Likely@arm.com>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
Cc: Patrick DELAUNAY <patrick.delaunay@st.com>

Ard Biesheuvel (3):
  efi/libstub: Export efi_low_alloc_above() to other units
  efi/libstub: Use low allocation for the uncompressed kernel
  efi/libstub: base FDT and initrd placement on image address not DRAM
    base

 arch/arm/include/asm/efi.h                |   6 +-
 arch/arm64/include/asm/efi.h              |   2 +-
 drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
 drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
 drivers/firmware/efi/libstub/efistub.h    |   3 +
 drivers/firmware/efi/libstub/relocate.c   |   4 +-
 6 files changed, 47 insertions(+), 147 deletions(-)

-- 
2.17.1


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

[PATCH RFC/RFT 1/3] efi/libstub: Export efi_low_alloc_above() to other units

[Ard Biesheuvel]

Permit arm32-stub.c to access efi_low_alloc_above() in a subsequent
patch by giving it external linkage and declaring it in efistub.h.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/efistub.h  | 3 +++
 drivers/firmware/efi/libstub/relocate.c | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 85050f5a1b28..158f86f1f9fc 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -740,6 +740,9 @@ efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
 efi_status_t efi_allocate_pages_aligned(unsigned long size, unsigned long *addr,
 					unsigned long max, unsigned long align);
 
+efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
+				 unsigned long *addr, unsigned long min);
+
 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/relocate.c b/drivers/firmware/efi/libstub/relocate.c
index 9b1aaf8b123f..8ee9eb2b9039 100644
--- a/drivers/firmware/efi/libstub/relocate.c
+++ b/drivers/firmware/efi/libstub/relocate.c
@@ -20,8 +20,8 @@
  *
  * Return:	status code
  */
-static efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
-					unsigned long *addr, unsigned long min)
+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;
-- 
2.17.1

[PATCH RFC/RFT 1/3] efi/libstub: Export efi_low_alloc_above() to other units

[Ard Biesheuvel]

Permit arm32-stub.c to access efi_low_alloc_above() in a subsequent
patch by giving it external linkage and declaring it in efistub.h.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/efistub.h  | 3 +++
 drivers/firmware/efi/libstub/relocate.c | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 85050f5a1b28..158f86f1f9fc 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -740,6 +740,9 @@ efi_status_t efi_allocate_pages(unsigned long size, unsigned long *addr,
 efi_status_t efi_allocate_pages_aligned(unsigned long size, unsigned long *addr,
 					unsigned long max, unsigned long align);
 
+efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
+				 unsigned long *addr, unsigned long min);
+
 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/relocate.c b/drivers/firmware/efi/libstub/relocate.c
index 9b1aaf8b123f..8ee9eb2b9039 100644
--- a/drivers/firmware/efi/libstub/relocate.c
+++ b/drivers/firmware/efi/libstub/relocate.c
@@ -20,8 +20,8 @@
  *
  * Return:	status code
  */
-static efi_status_t efi_low_alloc_above(unsigned long size, unsigned long align,
-					unsigned long *addr, unsigned long min)
+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;
-- 
2.17.1


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

[PATCH RFC/RFT 2/3] efi/libstub: Use low allocation for the uncompressed kernel

[Ard Biesheuvel]

Before commit

  d0f9ca9be11f25ef ("ARM: decompressor: run decompressor in place if loaded via UEFI")

we were rather limited in the choice of base address for the uncompressed
kernel, as we were relying on the logic in the decompressor that blindly
rounds down the decompressor execution address to the next multiple of 128
MiB, and decompresses the kernel there. For this reason, we have a lot of
complicated memory region handling code, to ensure that this memory window
is available, even though it could be occupied by reserved regions or
other allocations that may or may not collide with the uncompressed image.

Today, we simply pass the target address for the decompressed image to the
decompressor directly, and so we can choose a suitable window just by
finding a 16 MiB aligned region, while taking TEXT_OFFSET and the region
for the swapper page tables into account.

So let's get rid of the complicated logic, and instead, use the existing
bottom up allocation routine to allocate a suitable window as low as
possible, and carve out a memory region that has the right properties.

Note that this removes any dependencies on the 'dram_base' argument to
handle_kernel_image(), which we will hopefully be able to remove entirely
in a future patch.

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

diff --git a/drivers/firmware/efi/libstub/arm32-stub.c b/drivers/firmware/efi/libstub/arm32-stub.c
index d08e5d55838c..a9a4da98c7a0 100644
--- a/drivers/firmware/efi/libstub/arm32-stub.c
+++ b/drivers/firmware/efi/libstub/arm32-stub.c
@@ -113,134 +113,6 @@ void free_screen_info(struct screen_info *si)
 	efi_bs_call(free_pool, si);
 }
 
-static efi_status_t reserve_kernel_base(unsigned long dram_base,
-					unsigned long *reserve_addr,
-					unsigned long *reserve_size)
-{
-	efi_physical_addr_t alloc_addr;
-	efi_memory_desc_t *memory_map;
-	unsigned long nr_pages, map_size, desc_size, buff_size;
-	efi_status_t status;
-	unsigned long l;
-
-	struct efi_boot_memmap map = {
-		.map		= &memory_map,
-		.map_size	= &map_size,
-		.desc_size	= &desc_size,
-		.desc_ver	= NULL,
-		.key_ptr	= NULL,
-		.buff_size	= &buff_size,
-	};
-
-	/*
-	 * Reserve memory for the uncompressed kernel image. This is
-	 * all that prevents any future allocations from conflicting
-	 * with the kernel. Since we can't tell from the compressed
-	 * image how much DRAM the kernel actually uses (due to BSS
-	 * size uncertainty) we allocate the maximum possible size.
-	 * Do this very early, as prints can cause memory allocations
-	 * that may conflict with this.
-	 */
-	alloc_addr = dram_base + MAX_UNCOMP_KERNEL_SIZE;
-	nr_pages = MAX_UNCOMP_KERNEL_SIZE / EFI_PAGE_SIZE;
-	status = efi_bs_call(allocate_pages, EFI_ALLOCATE_MAX_ADDRESS,
-			     EFI_BOOT_SERVICES_DATA, nr_pages, &alloc_addr);
-	if (status == EFI_SUCCESS) {
-		if (alloc_addr == dram_base) {
-			*reserve_addr = alloc_addr;
-			*reserve_size = MAX_UNCOMP_KERNEL_SIZE;
-			return EFI_SUCCESS;
-		}
-		/*
-		 * If we end up here, the allocation succeeded but starts below
-		 * dram_base. This can only occur if the real base of DRAM is
-		 * not a multiple of 128 MB, in which case dram_base will have
-		 * been rounded up. Since this implies that a part of the region
-		 * was already occupied, we need to fall through to the code
-		 * below to ensure that the existing allocations don't conflict.
-		 * For this reason, we use EFI_BOOT_SERVICES_DATA above and not
-		 * EFI_LOADER_DATA, which we wouldn't able to distinguish from
-		 * allocations that we want to disallow.
-		 */
-	}
-
-	/*
-	 * If the allocation above failed, we may still be able to proceed:
-	 * if the only allocations in the region are of types that will be
-	 * released to the OS after ExitBootServices(), the decompressor can
-	 * safely overwrite them.
-	 */
-	status = efi_get_memory_map(&map);
-	if (status != EFI_SUCCESS) {
-		efi_err("reserve_kernel_base(): Unable to retrieve memory map.\n");
-		return status;
-	}
-
-	for (l = 0; l < map_size; l += desc_size) {
-		efi_memory_desc_t *desc;
-		u64 start, end;
-
-		desc = (void *)memory_map + l;
-		start = desc->phys_addr;
-		end = start + desc->num_pages * EFI_PAGE_SIZE;
-
-		/* Skip if entry does not intersect with region */
-		if (start >= dram_base + MAX_UNCOMP_KERNEL_SIZE ||
-		    end <= dram_base)
-			continue;
-
-		switch (desc->type) {
-		case EFI_BOOT_SERVICES_CODE:
-		case EFI_BOOT_SERVICES_DATA:
-			/* Ignore types that are released to the OS anyway */
-			continue;
-
-		case EFI_CONVENTIONAL_MEMORY:
-			/* Skip soft reserved conventional memory */
-			if (efi_soft_reserve_enabled() &&
-			    (desc->attribute & EFI_MEMORY_SP))
-				continue;
-
-			/*
-			 * Reserve the intersection between this entry and the
-			 * region.
-			 */
-			start = max(start, (u64)dram_base);
-			end = min(end, (u64)dram_base + MAX_UNCOMP_KERNEL_SIZE);
-
-			status = efi_bs_call(allocate_pages,
-					     EFI_ALLOCATE_ADDRESS,
-					     EFI_LOADER_DATA,
-					     (end - start) / EFI_PAGE_SIZE,
-					     &start);
-			if (status != EFI_SUCCESS) {
-				efi_err("reserve_kernel_base(): alloc failed.\n");
-				goto out;
-			}
-			break;
-
-		case EFI_LOADER_CODE:
-		case EFI_LOADER_DATA:
-			/*
-			 * These regions may be released and reallocated for
-			 * another purpose (including EFI_RUNTIME_SERVICE_DATA)
-			 * at any time during the execution of the OS loader,
-			 * so we cannot consider them as safe.
-			 */
-		default:
-			/*
-			 * Treat any other allocation in the region as unsafe */
-			status = EFI_OUT_OF_RESOURCES;
-			goto out;
-		}
-	}
-
-	status = EFI_SUCCESS;
-out:
-	efi_bs_call(free_pool, memory_map);
-	return status;
-}
-
 efi_status_t handle_kernel_image(unsigned long *image_addr,
 				 unsigned long *image_size,
 				 unsigned long *reserve_addr,
@@ -248,27 +120,52 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 				 unsigned long dram_base,
 				 efi_loaded_image_t *image)
 {
-	unsigned long kernel_base;
+	const int slack = TEXT_OFFSET - 5 * PAGE_SIZE;
+	int alloc_size = MAX_UNCOMP_KERNEL_SIZE + SZ_16M + slack;
+	unsigned long alloc_base, kernel_base;
 	efi_status_t status;
 
-	/* use a 16 MiB aligned base for the decompressed kernel */
-	kernel_base = round_up(dram_base, SZ_16M) + TEXT_OFFSET;
-
 	/*
-	 * Note that some platforms (notably, the Raspberry Pi 2) put
-	 * spin-tables and other pieces of firmware at the base of RAM,
-	 * abusing the fact that the window of TEXT_OFFSET bytes at the
-	 * base of the kernel image is only partially used at the moment.
-	 * (Up to 5 pages are used for the swapper page tables)
+	 * Allocate space for the decompressed kernel as low as possible.
+	 * The region should be 16 MiB aligned, but the first 'slack' bytes
+	 * are not used by Linux, so we allow those to be occupied by the
+	 * firmware.
 	 */
-	status = reserve_kernel_base(kernel_base - 5 * PAGE_SIZE, reserve_addr,
-				     reserve_size);
+	status = efi_low_alloc_above(alloc_size, EFI_PAGE_SIZE, &alloc_base, 0x0);
 	if (status != EFI_SUCCESS) {
 		efi_err("Unable to allocate memory for uncompressed kernel.\n");
 		return status;
 	}
 
-	*image_addr = kernel_base;
+	if ((alloc_base % SZ_16M) > slack) {
+		/*
+		 * More than 'slack' bytes are already occupied at the base of
+		 * the allocation, so we need to advance to the next 16 MiB block.
+		 */
+		kernel_base = round_up(alloc_base, SZ_16M);
+		efi_info("Free memory starts at 0x%lx, setting kernel_base to 0x%lx\n",
+			 alloc_base, kernel_base);
+	} else {
+		kernel_base = round_down(alloc_base, SZ_16M);
+	}
+
+	*reserve_addr = kernel_base + slack;
+	*reserve_size = MAX_UNCOMP_KERNEL_SIZE;
+
+	/* now free the parts that we will not use */
+	if (*reserve_addr > alloc_base) {
+		efi_bs_call(free_pages, alloc_base,
+			    (*reserve_addr - alloc_base) / EFI_PAGE_SIZE);
+		alloc_size -= *reserve_addr - alloc_base;
+	}
+	efi_bs_call(free_pages, *reserve_addr + MAX_UNCOMP_KERNEL_SIZE,
+		    (alloc_size - MAX_UNCOMP_KERNEL_SIZE) / EFI_PAGE_SIZE);
+
+	*image_addr = kernel_base + TEXT_OFFSET;
 	*image_size = 0;
+
+	efi_debug("image addr == 0x%lx, reserve_addr == 0x%lx\n",
+		  *image_addr, *reserve_addr);
+
 	return EFI_SUCCESS;
 }
-- 
2.17.1

[PATCH RFC/RFT 2/3] efi/libstub: Use low allocation for the uncompressed kernel

[Ard Biesheuvel]

Before commit

  d0f9ca9be11f25ef ("ARM: decompressor: run decompressor in place if loaded via UEFI")

we were rather limited in the choice of base address for the uncompressed
kernel, as we were relying on the logic in the decompressor that blindly
rounds down the decompressor execution address to the next multiple of 128
MiB, and decompresses the kernel there. For this reason, we have a lot of
complicated memory region handling code, to ensure that this memory window
is available, even though it could be occupied by reserved regions or
other allocations that may or may not collide with the uncompressed image.

Today, we simply pass the target address for the decompressed image to the
decompressor directly, and so we can choose a suitable window just by
finding a 16 MiB aligned region, while taking TEXT_OFFSET and the region
for the swapper page tables into account.

So let's get rid of the complicated logic, and instead, use the existing
bottom up allocation routine to allocate a suitable window as low as
possible, and carve out a memory region that has the right properties.

Note that this removes any dependencies on the 'dram_base' argument to
handle_kernel_image(), which we will hopefully be able to remove entirely
in a future patch.

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

diff --git a/drivers/firmware/efi/libstub/arm32-stub.c b/drivers/firmware/efi/libstub/arm32-stub.c
index d08e5d55838c..a9a4da98c7a0 100644
--- a/drivers/firmware/efi/libstub/arm32-stub.c
+++ b/drivers/firmware/efi/libstub/arm32-stub.c
@@ -113,134 +113,6 @@ void free_screen_info(struct screen_info *si)
 	efi_bs_call(free_pool, si);
 }
 
-static efi_status_t reserve_kernel_base(unsigned long dram_base,
-					unsigned long *reserve_addr,
-					unsigned long *reserve_size)
-{
-	efi_physical_addr_t alloc_addr;
-	efi_memory_desc_t *memory_map;
-	unsigned long nr_pages, map_size, desc_size, buff_size;
-	efi_status_t status;
-	unsigned long l;
-
-	struct efi_boot_memmap map = {
-		.map		= &memory_map,
-		.map_size	= &map_size,
-		.desc_size	= &desc_size,
-		.desc_ver	= NULL,
-		.key_ptr	= NULL,
-		.buff_size	= &buff_size,
-	};
-
-	/*
-	 * Reserve memory for the uncompressed kernel image. This is
-	 * all that prevents any future allocations from conflicting
-	 * with the kernel. Since we can't tell from the compressed
-	 * image how much DRAM the kernel actually uses (due to BSS
-	 * size uncertainty) we allocate the maximum possible size.
-	 * Do this very early, as prints can cause memory allocations
-	 * that may conflict with this.
-	 */
-	alloc_addr = dram_base + MAX_UNCOMP_KERNEL_SIZE;
-	nr_pages = MAX_UNCOMP_KERNEL_SIZE / EFI_PAGE_SIZE;
-	status = efi_bs_call(allocate_pages, EFI_ALLOCATE_MAX_ADDRESS,
-			     EFI_BOOT_SERVICES_DATA, nr_pages, &alloc_addr);
-	if (status == EFI_SUCCESS) {
-		if (alloc_addr == dram_base) {
-			*reserve_addr = alloc_addr;
-			*reserve_size = MAX_UNCOMP_KERNEL_SIZE;
-			return EFI_SUCCESS;
-		}
-		/*
-		 * If we end up here, the allocation succeeded but starts below
-		 * dram_base. This can only occur if the real base of DRAM is
-		 * not a multiple of 128 MB, in which case dram_base will have
-		 * been rounded up. Since this implies that a part of the region
-		 * was already occupied, we need to fall through to the code
-		 * below to ensure that the existing allocations don't conflict.
-		 * For this reason, we use EFI_BOOT_SERVICES_DATA above and not
-		 * EFI_LOADER_DATA, which we wouldn't able to distinguish from
-		 * allocations that we want to disallow.
-		 */
-	}
-
-	/*
-	 * If the allocation above failed, we may still be able to proceed:
-	 * if the only allocations in the region are of types that will be
-	 * released to the OS after ExitBootServices(), the decompressor can
-	 * safely overwrite them.
-	 */
-	status = efi_get_memory_map(&map);
-	if (status != EFI_SUCCESS) {
-		efi_err("reserve_kernel_base(): Unable to retrieve memory map.\n");
-		return status;
-	}
-
-	for (l = 0; l < map_size; l += desc_size) {
-		efi_memory_desc_t *desc;
-		u64 start, end;
-
-		desc = (void *)memory_map + l;
-		start = desc->phys_addr;
-		end = start + desc->num_pages * EFI_PAGE_SIZE;
-
-		/* Skip if entry does not intersect with region */
-		if (start >= dram_base + MAX_UNCOMP_KERNEL_SIZE ||
-		    end <= dram_base)
-			continue;
-
-		switch (desc->type) {
-		case EFI_BOOT_SERVICES_CODE:
-		case EFI_BOOT_SERVICES_DATA:
-			/* Ignore types that are released to the OS anyway */
-			continue;
-
-		case EFI_CONVENTIONAL_MEMORY:
-			/* Skip soft reserved conventional memory */
-			if (efi_soft_reserve_enabled() &&
-			    (desc->attribute & EFI_MEMORY_SP))
-				continue;
-
-			/*
-			 * Reserve the intersection between this entry and the
-			 * region.
-			 */
-			start = max(start, (u64)dram_base);
-			end = min(end, (u64)dram_base + MAX_UNCOMP_KERNEL_SIZE);
-
-			status = efi_bs_call(allocate_pages,
-					     EFI_ALLOCATE_ADDRESS,
-					     EFI_LOADER_DATA,
-					     (end - start) / EFI_PAGE_SIZE,
-					     &start);
-			if (status != EFI_SUCCESS) {
-				efi_err("reserve_kernel_base(): alloc failed.\n");
-				goto out;
-			}
-			break;
-
-		case EFI_LOADER_CODE:
-		case EFI_LOADER_DATA:
-			/*
-			 * These regions may be released and reallocated for
-			 * another purpose (including EFI_RUNTIME_SERVICE_DATA)
-			 * at any time during the execution of the OS loader,
-			 * so we cannot consider them as safe.
-			 */
-		default:
-			/*
-			 * Treat any other allocation in the region as unsafe */
-			status = EFI_OUT_OF_RESOURCES;
-			goto out;
-		}
-	}
-
-	status = EFI_SUCCESS;
-out:
-	efi_bs_call(free_pool, memory_map);
-	return status;
-}
-
 efi_status_t handle_kernel_image(unsigned long *image_addr,
 				 unsigned long *image_size,
 				 unsigned long *reserve_addr,
@@ -248,27 +120,52 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 				 unsigned long dram_base,
 				 efi_loaded_image_t *image)
 {
-	unsigned long kernel_base;
+	const int slack = TEXT_OFFSET - 5 * PAGE_SIZE;
+	int alloc_size = MAX_UNCOMP_KERNEL_SIZE + SZ_16M + slack;
+	unsigned long alloc_base, kernel_base;
 	efi_status_t status;
 
-	/* use a 16 MiB aligned base for the decompressed kernel */
-	kernel_base = round_up(dram_base, SZ_16M) + TEXT_OFFSET;
-
 	/*
-	 * Note that some platforms (notably, the Raspberry Pi 2) put
-	 * spin-tables and other pieces of firmware at the base of RAM,
-	 * abusing the fact that the window of TEXT_OFFSET bytes at the
-	 * base of the kernel image is only partially used at the moment.
-	 * (Up to 5 pages are used for the swapper page tables)
+	 * Allocate space for the decompressed kernel as low as possible.
+	 * The region should be 16 MiB aligned, but the first 'slack' bytes
+	 * are not used by Linux, so we allow those to be occupied by the
+	 * firmware.
 	 */
-	status = reserve_kernel_base(kernel_base - 5 * PAGE_SIZE, reserve_addr,
-				     reserve_size);
+	status = efi_low_alloc_above(alloc_size, EFI_PAGE_SIZE, &alloc_base, 0x0);
 	if (status != EFI_SUCCESS) {
 		efi_err("Unable to allocate memory for uncompressed kernel.\n");
 		return status;
 	}
 
-	*image_addr = kernel_base;
+	if ((alloc_base % SZ_16M) > slack) {
+		/*
+		 * More than 'slack' bytes are already occupied at the base of
+		 * the allocation, so we need to advance to the next 16 MiB block.
+		 */
+		kernel_base = round_up(alloc_base, SZ_16M);
+		efi_info("Free memory starts at 0x%lx, setting kernel_base to 0x%lx\n",
+			 alloc_base, kernel_base);
+	} else {
+		kernel_base = round_down(alloc_base, SZ_16M);
+	}
+
+	*reserve_addr = kernel_base + slack;
+	*reserve_size = MAX_UNCOMP_KERNEL_SIZE;
+
+	/* now free the parts that we will not use */
+	if (*reserve_addr > alloc_base) {
+		efi_bs_call(free_pages, alloc_base,
+			    (*reserve_addr - alloc_base) / EFI_PAGE_SIZE);
+		alloc_size -= *reserve_addr - alloc_base;
+	}
+	efi_bs_call(free_pages, *reserve_addr + MAX_UNCOMP_KERNEL_SIZE,
+		    (alloc_size - MAX_UNCOMP_KERNEL_SIZE) / EFI_PAGE_SIZE);
+
+	*image_addr = kernel_base + TEXT_OFFSET;
 	*image_size = 0;
+
+	efi_debug("image addr == 0x%lx, reserve_addr == 0x%lx\n",
+		  *image_addr, *reserve_addr);
+
 	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 RFC/RFT 3/3] efi/libstub: base FDT and initrd placement on image address not DRAM base

[Ard Biesheuvel]

The way we use the base of DRAM in the EFI stub is problematic as it
is ill defined what the base of DRAM actually means. There are some
restrictions on the placement of FDT and initrd which are defined in
terms of dram_base, but given that the placement of the kernel in
memory is what defines these boundaries, it is better to use the image
address in these cases, and disregard dram_base altogether.

In a future patch, we should be able to get rid of dram_base entirely,
but at the moment, RISC-V support is in flight in another tree, so we
keep it around for now.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm/include/asm/efi.h              | 6 +++---
 arch/arm64/include/asm/efi.h            | 2 +-
 drivers/firmware/efi/libstub/efi-stub.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h
index 5dcf3c6011b7..9e481b362227 100644
--- a/arch/arm/include/asm/efi.h
+++ b/arch/arm/include/asm/efi.h
@@ -75,16 +75,16 @@ static inline void efifb_setup_from_dmi(struct screen_info *si, const char *opt)
 #define MIN_ZIMAGE_OFFSET	MAX_UNCOMP_KERNEL_SIZE
 
 /* on ARM, the FDT should be located in the first 128 MB of RAM */
-static inline unsigned long efi_get_max_fdt_addr(unsigned long dram_base)
+static inline unsigned long efi_get_max_fdt_addr(unsigned long image_addr)
 {
-	return dram_base + ZIMAGE_OFFSET_LIMIT;
+	return image_addr + ZIMAGE_OFFSET_LIMIT;
 }
 
 /* on ARM, the initrd should be loaded in a lowmem region */
 static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
 						    unsigned long image_addr)
 {
-	return dram_base + SZ_512M;
+	return image_addr + SZ_512M;
 }
 
 struct efi_arm_entry_state {
diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index d4ab3f73e7a3..27c2e8959ab6 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -65,7 +65,7 @@ efi_status_t __efi_rt_asm_wrapper(void *, const char *, ...);
 	(SEGMENT_ALIGN > THREAD_ALIGN ? SEGMENT_ALIGN : THREAD_ALIGN)
 
 /* on arm64, the FDT may be located anywhere in system RAM */
-static inline unsigned long efi_get_max_fdt_addr(unsigned long dram_base)
+static inline unsigned long efi_get_max_fdt_addr(unsigned long image_addr)
 {
 	return ULONG_MAX;
 }
diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index a5a405d8ab44..76ce60065f10 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -306,7 +306,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	install_memreserve_table();
 
 	status = allocate_new_fdt_and_exit_boot(handle, &fdt_addr,
-						efi_get_max_fdt_addr(dram_base),
+						efi_get_max_fdt_addr(image_addr),
 						initrd_addr, initrd_size,
 						cmdline_ptr, fdt_addr, fdt_size);
 	if (status != EFI_SUCCESS)
-- 
2.17.1

[PATCH RFC/RFT 3/3] efi/libstub: base FDT and initrd placement on image address not DRAM base

[Ard Biesheuvel]

The way we use the base of DRAM in the EFI stub is problematic as it
is ill defined what the base of DRAM actually means. There are some
restrictions on the placement of FDT and initrd which are defined in
terms of dram_base, but given that the placement of the kernel in
memory is what defines these boundaries, it is better to use the image
address in these cases, and disregard dram_base altogether.

In a future patch, we should be able to get rid of dram_base entirely,
but at the moment, RISC-V support is in flight in another tree, so we
keep it around for now.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm/include/asm/efi.h              | 6 +++---
 arch/arm64/include/asm/efi.h            | 2 +-
 drivers/firmware/efi/libstub/efi-stub.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h
index 5dcf3c6011b7..9e481b362227 100644
--- a/arch/arm/include/asm/efi.h
+++ b/arch/arm/include/asm/efi.h
@@ -75,16 +75,16 @@ static inline void efifb_setup_from_dmi(struct screen_info *si, const char *opt)
 #define MIN_ZIMAGE_OFFSET	MAX_UNCOMP_KERNEL_SIZE
 
 /* on ARM, the FDT should be located in the first 128 MB of RAM */
-static inline unsigned long efi_get_max_fdt_addr(unsigned long dram_base)
+static inline unsigned long efi_get_max_fdt_addr(unsigned long image_addr)
 {
-	return dram_base + ZIMAGE_OFFSET_LIMIT;
+	return image_addr + ZIMAGE_OFFSET_LIMIT;
 }
 
 /* on ARM, the initrd should be loaded in a lowmem region */
 static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
 						    unsigned long image_addr)
 {
-	return dram_base + SZ_512M;
+	return image_addr + SZ_512M;
 }
 
 struct efi_arm_entry_state {
diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index d4ab3f73e7a3..27c2e8959ab6 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -65,7 +65,7 @@ efi_status_t __efi_rt_asm_wrapper(void *, const char *, ...);
 	(SEGMENT_ALIGN > THREAD_ALIGN ? SEGMENT_ALIGN : THREAD_ALIGN)
 
 /* on arm64, the FDT may be located anywhere in system RAM */
-static inline unsigned long efi_get_max_fdt_addr(unsigned long dram_base)
+static inline unsigned long efi_get_max_fdt_addr(unsigned long image_addr)
 {
 	return ULONG_MAX;
 }
diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index a5a405d8ab44..76ce60065f10 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -306,7 +306,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	install_memreserve_table();
 
 	status = allocate_new_fdt_and_exit_boot(handle, &fdt_addr,
-						efi_get_max_fdt_addr(dram_base),
+						efi_get_max_fdt_addr(image_addr),
 						initrd_addr, initrd_size,
 						cmdline_ptr, fdt_addr, fdt_size);
 	if (status != 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

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Grant Likely]

On 09/09/2020 16:16, Ard Biesheuvel wrote:
> Maxim reports boot failures on platforms that describe reserved memory
> regions in DT that are disjoint from system DRAM, and which are converted
> to EfiReservedMemory regions by the EFI subsystem in u-boot.
> 
> As it turns out, the whole notion of discovering the base of DRAM is
> problematic, and it would be better to simply rely on the EFI memory
> allocation routines instead, and derive the FDT and initrd allocation
> limits from the actual placement of the kernel (which is what defines
> the start of the linear region anyway)
> 
> Finally, we should be able to get rid of get_dram_base() entirely.
> However, as RISC-V only just started using it, we will need to address
> that at a later time.

Looks reasonable to me. Presumably all special cases (platform specific 
spin tables, etc) are covered as reserved in the UEFI memory map, correct?

g.

> 
> Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Atish Patra <atish.patra@wdc.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Jens Wiklander <jens.wiklander@linaro.org>
> Cc: Francois Ozog <francois.ozog@linaro.org>
> Cc: Etienne CARRIERE <etienne.carriere@st.com>
> Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> Cc: Patrice CHOTARD <patrice.chotard@st.com>
> Cc: Sumit Garg <sumit.garg@linaro.org>
> Cc: Grant Likely <Grant.Likely@arm.com>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> 
> Ard Biesheuvel (3):
>    efi/libstub: Export efi_low_alloc_above() to other units
>    efi/libstub: Use low allocation for the uncompressed kernel
>    efi/libstub: base FDT and initrd placement on image address not DRAM
>      base
> 
>   arch/arm/include/asm/efi.h                |   6 +-
>   arch/arm64/include/asm/efi.h              |   2 +-
>   drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
>   drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
>   drivers/firmware/efi/libstub/efistub.h    |   3 +
>   drivers/firmware/efi/libstub/relocate.c   |   4 +-
>   6 files changed, 47 insertions(+), 147 deletions(-)
> 

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Grant Likely]

On 09/09/2020 16:16, Ard Biesheuvel wrote:
> Maxim reports boot failures on platforms that describe reserved memory
> regions in DT that are disjoint from system DRAM, and which are converted
> to EfiReservedMemory regions by the EFI subsystem in u-boot.
> 
> As it turns out, the whole notion of discovering the base of DRAM is
> problematic, and it would be better to simply rely on the EFI memory
> allocation routines instead, and derive the FDT and initrd allocation
> limits from the actual placement of the kernel (which is what defines
> the start of the linear region anyway)
> 
> Finally, we should be able to get rid of get_dram_base() entirely.
> However, as RISC-V only just started using it, we will need to address
> that at a later time.

Looks reasonable to me. Presumably all special cases (platform specific 
spin tables, etc) are covered as reserved in the UEFI memory map, correct?

g.

> 
> Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Atish Patra <atish.patra@wdc.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Jens Wiklander <jens.wiklander@linaro.org>
> Cc: Francois Ozog <francois.ozog@linaro.org>
> Cc: Etienne CARRIERE <etienne.carriere@st.com>
> Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> Cc: Patrice CHOTARD <patrice.chotard@st.com>
> Cc: Sumit Garg <sumit.garg@linaro.org>
> Cc: Grant Likely <Grant.Likely@arm.com>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> 
> Ard Biesheuvel (3):
>    efi/libstub: Export efi_low_alloc_above() to other units
>    efi/libstub: Use low allocation for the uncompressed kernel
>    efi/libstub: base FDT and initrd placement on image address not DRAM
>      base
> 
>   arch/arm/include/asm/efi.h                |   6 +-
>   arch/arm64/include/asm/efi.h              |   2 +-
>   drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
>   drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
>   drivers/firmware/efi/libstub/efistub.h    |   3 +
>   drivers/firmware/efi/libstub/relocate.c   |   4 +-
>   6 files changed, 47 insertions(+), 147 deletions(-)
> 

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

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Ard Biesheuvel]

On Wed, 9 Sep 2020 at 18:26, Grant Likely <grant.likely@arm.com> wrote:
>
>
>
> On 09/09/2020 16:16, Ard Biesheuvel wrote:
> > Maxim reports boot failures on platforms that describe reserved memory
> > regions in DT that are disjoint from system DRAM, and which are converted
> > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> >
> > As it turns out, the whole notion of discovering the base of DRAM is
> > problematic, and it would be better to simply rely on the EFI memory
> > allocation routines instead, and derive the FDT and initrd allocation
> > limits from the actual placement of the kernel (which is what defines
> > the start of the linear region anyway)
> >
> > Finally, we should be able to get rid of get_dram_base() entirely.
> > However, as RISC-V only just started using it, we will need to address
> > that at a later time.
>
> Looks reasonable to me. Presumably all special cases (platform specific
> spin tables, etc) are covered as reserved in the UEFI memory map, correct?
>

Yes. The only reason the code is as it is today is for a proprietary
HP Inc. platform that had a bootservicesdata allocation at the base of
DRAM of 8 KiB, but this should now be covered in the same way as any
other reserved region living in the window below TEXT_OFFSET. (Note
that the current logic is flawed in any case: even though boot
services regions are released to the OS at ExitBootServices(), there
are types of EFI configuration tables that keep their contents in
BsData regions as well, and those may get clobbered by the
decompressed image)

In summary, I am not expecting these changes to regress any platforms
we care about (famous last words), and if it works, we can start
removing the dram base handling code altogether (which currently gets
called on arm64 as well, even though we never use the result)

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Ard Biesheuvel]

On Wed, 9 Sep 2020 at 18:26, Grant Likely <grant.likely@arm.com> wrote:
>
>
>
> On 09/09/2020 16:16, Ard Biesheuvel wrote:
> > Maxim reports boot failures on platforms that describe reserved memory
> > regions in DT that are disjoint from system DRAM, and which are converted
> > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> >
> > As it turns out, the whole notion of discovering the base of DRAM is
> > problematic, and it would be better to simply rely on the EFI memory
> > allocation routines instead, and derive the FDT and initrd allocation
> > limits from the actual placement of the kernel (which is what defines
> > the start of the linear region anyway)
> >
> > Finally, we should be able to get rid of get_dram_base() entirely.
> > However, as RISC-V only just started using it, we will need to address
> > that at a later time.
>
> Looks reasonable to me. Presumably all special cases (platform specific
> spin tables, etc) are covered as reserved in the UEFI memory map, correct?
>

Yes. The only reason the code is as it is today is for a proprietary
HP Inc. platform that had a bootservicesdata allocation at the base of
DRAM of 8 KiB, but this should now be covered in the same way as any
other reserved region living in the window below TEXT_OFFSET. (Note
that the current logic is flawed in any case: even though boot
services regions are released to the OS at ExitBootServices(), there
are types of EFI configuration tables that keep their contents in
BsData regions as well, and those may get clobbered by the
decompressed image)

In summary, I am not expecting these changes to regress any platforms
we care about (famous last words), and if it works, we can start
removing the dram base handling code altogether (which currently gets
called on arm64 as well, even though we never use the result)

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

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Palmer Dabbelt]

On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> Maxim reports boot failures on platforms that describe reserved memory
> regions in DT that are disjoint from system DRAM, and which are converted
> to EfiReservedMemory regions by the EFI subsystem in u-boot.
>
> As it turns out, the whole notion of discovering the base of DRAM is
> problematic, and it would be better to simply rely on the EFI memory
> allocation routines instead, and derive the FDT and initrd allocation
> limits from the actual placement of the kernel (which is what defines
> the start of the linear region anyway)
>
> Finally, we should be able to get rid of get_dram_base() entirely.
> However, as RISC-V only just started using it, we will need to address
> that at a later time.

Looks like we're using dram_base to derive two argumets to
efi_relocate_kernel(): the preferred load address and the minimum load address.
I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
x86 uses for the minimum load address, but I don't think we have any mechanism
like "struct boot_params" so we'd need to come up with something.

> Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Atish Patra <atish.patra@wdc.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Jens Wiklander <jens.wiklander@linaro.org>
> Cc: Francois Ozog <francois.ozog@linaro.org>
> Cc: Etienne CARRIERE <etienne.carriere@st.com>
> Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> Cc: Patrice CHOTARD <patrice.chotard@st.com>
> Cc: Sumit Garg <sumit.garg@linaro.org>
> Cc: Grant Likely <Grant.Likely@arm.com>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
>
> Ard Biesheuvel (3):
>   efi/libstub: Export efi_low_alloc_above() to other units
>   efi/libstub: Use low allocation for the uncompressed kernel
>   efi/libstub: base FDT and initrd placement on image address not DRAM
>     base
>
>  arch/arm/include/asm/efi.h                |   6 +-
>  arch/arm64/include/asm/efi.h              |   2 +-
>  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
>  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
>  drivers/firmware/efi/libstub/efistub.h    |   3 +
>  drivers/firmware/efi/libstub/relocate.c   |   4 +-
>  6 files changed, 47 insertions(+), 147 deletions(-)

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Palmer Dabbelt]

On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> Maxim reports boot failures on platforms that describe reserved memory
> regions in DT that are disjoint from system DRAM, and which are converted
> to EfiReservedMemory regions by the EFI subsystem in u-boot.
>
> As it turns out, the whole notion of discovering the base of DRAM is
> problematic, and it would be better to simply rely on the EFI memory
> allocation routines instead, and derive the FDT and initrd allocation
> limits from the actual placement of the kernel (which is what defines
> the start of the linear region anyway)
>
> Finally, we should be able to get rid of get_dram_base() entirely.
> However, as RISC-V only just started using it, we will need to address
> that at a later time.

Looks like we're using dram_base to derive two argumets to
efi_relocate_kernel(): the preferred load address and the minimum load address.
I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
x86 uses for the minimum load address, but I don't think we have any mechanism
like "struct boot_params" so we'd need to come up with something.

> Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Atish Patra <atish.patra@wdc.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Jens Wiklander <jens.wiklander@linaro.org>
> Cc: Francois Ozog <francois.ozog@linaro.org>
> Cc: Etienne CARRIERE <etienne.carriere@st.com>
> Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> Cc: Patrice CHOTARD <patrice.chotard@st.com>
> Cc: Sumit Garg <sumit.garg@linaro.org>
> Cc: Grant Likely <Grant.Likely@arm.com>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
>
> Ard Biesheuvel (3):
>   efi/libstub: Export efi_low_alloc_above() to other units
>   efi/libstub: Use low allocation for the uncompressed kernel
>   efi/libstub: base FDT and initrd placement on image address not DRAM
>     base
>
>  arch/arm/include/asm/efi.h                |   6 +-
>  arch/arm64/include/asm/efi.h              |   2 +-
>  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
>  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
>  drivers/firmware/efi/libstub/efistub.h    |   3 +
>  drivers/firmware/efi/libstub/relocate.c   |   4 +-
>  6 files changed, 47 insertions(+), 147 deletions(-)

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

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Atish Patra]

On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > Maxim reports boot failures on platforms that describe reserved memory
> > regions in DT that are disjoint from system DRAM, and which are converted
> > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> >
> > As it turns out, the whole notion of discovering the base of DRAM is
> > problematic, and it would be better to simply rely on the EFI memory
> > allocation routines instead, and derive the FDT and initrd allocation
> > limits from the actual placement of the kernel (which is what defines
> > the start of the linear region anyway)
> >
> > Finally, we should be able to get rid of get_dram_base() entirely.
> > However, as RISC-V only just started using it, we will need to address
> > that at a later time.
>
> Looks like we're using dram_base to derive two argumets to
> efi_relocate_kernel(): the preferred load address and the minimum load address.
> I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> x86 uses for the minimum load address, but I don't think we have any mechanism
> like "struct boot_params" so we'd need to come up with something.
>

As discussed in the other thread
(https://www.spinics.net/lists/linux-efi/msg20262.html),
we don't need to do anything special. efi_relocate_kernel can just
take preferred address as 0
so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
allocate 2MB/4MB aligned address as per requirement.

I don't think the other changes in this series will cause any issue
for RISC-V. I will test it and update anyways.

> > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > Cc: Atish Patra <atish.patra@wdc.com>
> > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > Cc: Francois Ozog <francois.ozog@linaro.org>
> > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > Cc: Sumit Garg <sumit.garg@linaro.org>
> > Cc: Grant Likely <Grant.Likely@arm.com>
> > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> >
> > Ard Biesheuvel (3):
> >   efi/libstub: Export efi_low_alloc_above() to other units
> >   efi/libstub: Use low allocation for the uncompressed kernel
> >   efi/libstub: base FDT and initrd placement on image address not DRAM
> >     base
> >
> >  arch/arm/include/asm/efi.h                |   6 +-
> >  arch/arm64/include/asm/efi.h              |   2 +-
> >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> >  6 files changed, 47 insertions(+), 147 deletions(-)



-- 
Regards,
Atish

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Atish Patra]

On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > Maxim reports boot failures on platforms that describe reserved memory
> > regions in DT that are disjoint from system DRAM, and which are converted
> > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> >
> > As it turns out, the whole notion of discovering the base of DRAM is
> > problematic, and it would be better to simply rely on the EFI memory
> > allocation routines instead, and derive the FDT and initrd allocation
> > limits from the actual placement of the kernel (which is what defines
> > the start of the linear region anyway)
> >
> > Finally, we should be able to get rid of get_dram_base() entirely.
> > However, as RISC-V only just started using it, we will need to address
> > that at a later time.
>
> Looks like we're using dram_base to derive two argumets to
> efi_relocate_kernel(): the preferred load address and the minimum load address.
> I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> x86 uses for the minimum load address, but I don't think we have any mechanism
> like "struct boot_params" so we'd need to come up with something.
>

As discussed in the other thread
(https://www.spinics.net/lists/linux-efi/msg20262.html),
we don't need to do anything special. efi_relocate_kernel can just
take preferred address as 0
so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
allocate 2MB/4MB aligned address as per requirement.

I don't think the other changes in this series will cause any issue
for RISC-V. I will test it and update anyways.

> > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > Cc: Atish Patra <atish.patra@wdc.com>
> > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > Cc: Francois Ozog <francois.ozog@linaro.org>
> > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > Cc: Sumit Garg <sumit.garg@linaro.org>
> > Cc: Grant Likely <Grant.Likely@arm.com>
> > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> >
> > Ard Biesheuvel (3):
> >   efi/libstub: Export efi_low_alloc_above() to other units
> >   efi/libstub: Use low allocation for the uncompressed kernel
> >   efi/libstub: base FDT and initrd placement on image address not DRAM
> >     base
> >
> >  arch/arm/include/asm/efi.h                |   6 +-
> >  arch/arm64/include/asm/efi.h              |   2 +-
> >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> >  6 files changed, 47 insertions(+), 147 deletions(-)



-- 
Regards,
Atish

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

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Atish Patra]

On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >
> > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > > Maxim reports boot failures on platforms that describe reserved memory
> > > regions in DT that are disjoint from system DRAM, and which are converted
> > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > >
> > > As it turns out, the whole notion of discovering the base of DRAM is
> > > problematic, and it would be better to simply rely on the EFI memory
> > > allocation routines instead, and derive the FDT and initrd allocation
> > > limits from the actual placement of the kernel (which is what defines
> > > the start of the linear region anyway)
> > >
> > > Finally, we should be able to get rid of get_dram_base() entirely.
> > > However, as RISC-V only just started using it, we will need to address
> > > that at a later time.
> >
> > Looks like we're using dram_base to derive two argumets to
> > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > x86 uses for the minimum load address, but I don't think we have any mechanism
> > like "struct boot_params" so we'd need to come up with something.
> >
>
> As discussed in the other thread
> (https://www.spinics.net/lists/linux-efi/msg20262.html),
> we don't need to do anything special. efi_relocate_kernel can just
> take preferred address as 0
> so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> allocate 2MB/4MB aligned address as per requirement.
>
> I don't think the other changes in this series will cause any issue
> for RISC-V. I will test it and update anyways.
>
> > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > Cc: Atish Patra <atish.patra@wdc.com>
> > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > Cc: Grant Likely <Grant.Likely@arm.com>
> > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > >
> > > Ard Biesheuvel (3):
> > >   efi/libstub: Export efi_low_alloc_above() to other units
> > >   efi/libstub: Use low allocation for the uncompressed kernel
> > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> > >     base
> > >
> > >  arch/arm/include/asm/efi.h                |   6 +-
> > >  arch/arm64/include/asm/efi.h              |   2 +-
> > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> > >  6 files changed, 47 insertions(+), 147 deletions(-)
>

I verified the above patches along with the following RISC-V specific changes.

diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
index 93c305a638f4..dd6ceea9d548 100644
--- a/arch/riscv/include/asm/efi.h
+++ b/arch/riscv/include/asm/efi.h
@@ -37,7 +37,7 @@ static inline unsigned long
efi_get_max_fdt_addr(unsigned long dram_base)
 static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
                                                    unsigned long image_addr)
 {
-       return dram_base + SZ_256M;
+       return image_addr + SZ_256M;
 }

--- a/drivers/firmware/efi/libstub/riscv-stub.c
+++ b/drivers/firmware/efi/libstub/riscv-stub.c
@@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
         */
        preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
        status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
-                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
+                                    0, MIN_KIMG_ALIGN, 0);

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

Do

-- 
Regards,
Atish

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Atish Patra]

On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >
> > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > > Maxim reports boot failures on platforms that describe reserved memory
> > > regions in DT that are disjoint from system DRAM, and which are converted
> > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > >
> > > As it turns out, the whole notion of discovering the base of DRAM is
> > > problematic, and it would be better to simply rely on the EFI memory
> > > allocation routines instead, and derive the FDT and initrd allocation
> > > limits from the actual placement of the kernel (which is what defines
> > > the start of the linear region anyway)
> > >
> > > Finally, we should be able to get rid of get_dram_base() entirely.
> > > However, as RISC-V only just started using it, we will need to address
> > > that at a later time.
> >
> > Looks like we're using dram_base to derive two argumets to
> > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > x86 uses for the minimum load address, but I don't think we have any mechanism
> > like "struct boot_params" so we'd need to come up with something.
> >
>
> As discussed in the other thread
> (https://www.spinics.net/lists/linux-efi/msg20262.html),
> we don't need to do anything special. efi_relocate_kernel can just
> take preferred address as 0
> so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> allocate 2MB/4MB aligned address as per requirement.
>
> I don't think the other changes in this series will cause any issue
> for RISC-V. I will test it and update anyways.
>
> > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > Cc: Atish Patra <atish.patra@wdc.com>
> > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > Cc: Grant Likely <Grant.Likely@arm.com>
> > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > >
> > > Ard Biesheuvel (3):
> > >   efi/libstub: Export efi_low_alloc_above() to other units
> > >   efi/libstub: Use low allocation for the uncompressed kernel
> > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> > >     base
> > >
> > >  arch/arm/include/asm/efi.h                |   6 +-
> > >  arch/arm64/include/asm/efi.h              |   2 +-
> > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> > >  6 files changed, 47 insertions(+), 147 deletions(-)
>

I verified the above patches along with the following RISC-V specific changes.

diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
index 93c305a638f4..dd6ceea9d548 100644
--- a/arch/riscv/include/asm/efi.h
+++ b/arch/riscv/include/asm/efi.h
@@ -37,7 +37,7 @@ static inline unsigned long
efi_get_max_fdt_addr(unsigned long dram_base)
 static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
                                                    unsigned long image_addr)
 {
-       return dram_base + SZ_256M;
+       return image_addr + SZ_256M;
 }

--- a/drivers/firmware/efi/libstub/riscv-stub.c
+++ b/drivers/firmware/efi/libstub/riscv-stub.c
@@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
         */
        preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
        status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
-                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
+                                    0, MIN_KIMG_ALIGN, 0);

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

Do

-- 
Regards,
Atish

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

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Ard Biesheuvel]

On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
>
> On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> >
> > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > >
> > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > > > Maxim reports boot failures on platforms that describe reserved memory
> > > > regions in DT that are disjoint from system DRAM, and which are converted
> > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > > >
> > > > As it turns out, the whole notion of discovering the base of DRAM is
> > > > problematic, and it would be better to simply rely on the EFI memory
> > > > allocation routines instead, and derive the FDT and initrd allocation
> > > > limits from the actual placement of the kernel (which is what defines
> > > > the start of the linear region anyway)
> > > >
> > > > Finally, we should be able to get rid of get_dram_base() entirely.
> > > > However, as RISC-V only just started using it, we will need to address
> > > > that at a later time.
> > >
> > > Looks like we're using dram_base to derive two argumets to
> > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > > x86 uses for the minimum load address, but I don't think we have any mechanism
> > > like "struct boot_params" so we'd need to come up with something.
> > >
> >
> > As discussed in the other thread
> > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> > we don't need to do anything special. efi_relocate_kernel can just
> > take preferred address as 0
> > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> > allocate 2MB/4MB aligned address as per requirement.
> >
> > I don't think the other changes in this series will cause any issue
> > for RISC-V. I will test it and update anyways.
> >
> > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > > Cc: Grant Likely <Grant.Likely@arm.com>
> > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > > >
> > > > Ard Biesheuvel (3):
> > > >   efi/libstub: Export efi_low_alloc_above() to other units
> > > >   efi/libstub: Use low allocation for the uncompressed kernel
> > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> > > >     base
> > > >
> > > >  arch/arm/include/asm/efi.h                |   6 +-
> > > >  arch/arm64/include/asm/efi.h              |   2 +-
> > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> > > >  6 files changed, 47 insertions(+), 147 deletions(-)
> >
>
> I verified the above patches along with the following RISC-V specific changes.
>
> diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> index 93c305a638f4..dd6ceea9d548 100644
> --- a/arch/riscv/include/asm/efi.h
> +++ b/arch/riscv/include/asm/efi.h
> @@ -37,7 +37,7 @@ static inline unsigned long
> efi_get_max_fdt_addr(unsigned long dram_base)
>  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
>                                                     unsigned long image_addr)
>  {
> -       return dram_base + SZ_256M;
> +       return image_addr + SZ_256M;
>  }
>

Ah yes, we need this change as well - this is a bit unfortunate since
that creates a conflict with the RISC-V tree.

> --- a/drivers/firmware/efi/libstub/riscv-stub.c
> +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>          */
>         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
>         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
> +                                    0, MIN_KIMG_ALIGN, 0);
>
> FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>

Thanks for confirming.

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Ard Biesheuvel]

On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
>
> On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> >
> > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > >
> > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > > > Maxim reports boot failures on platforms that describe reserved memory
> > > > regions in DT that are disjoint from system DRAM, and which are converted
> > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > > >
> > > > As it turns out, the whole notion of discovering the base of DRAM is
> > > > problematic, and it would be better to simply rely on the EFI memory
> > > > allocation routines instead, and derive the FDT and initrd allocation
> > > > limits from the actual placement of the kernel (which is what defines
> > > > the start of the linear region anyway)
> > > >
> > > > Finally, we should be able to get rid of get_dram_base() entirely.
> > > > However, as RISC-V only just started using it, we will need to address
> > > > that at a later time.
> > >
> > > Looks like we're using dram_base to derive two argumets to
> > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > > x86 uses for the minimum load address, but I don't think we have any mechanism
> > > like "struct boot_params" so we'd need to come up with something.
> > >
> >
> > As discussed in the other thread
> > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> > we don't need to do anything special. efi_relocate_kernel can just
> > take preferred address as 0
> > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> > allocate 2MB/4MB aligned address as per requirement.
> >
> > I don't think the other changes in this series will cause any issue
> > for RISC-V. I will test it and update anyways.
> >
> > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > > Cc: Grant Likely <Grant.Likely@arm.com>
> > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > > >
> > > > Ard Biesheuvel (3):
> > > >   efi/libstub: Export efi_low_alloc_above() to other units
> > > >   efi/libstub: Use low allocation for the uncompressed kernel
> > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> > > >     base
> > > >
> > > >  arch/arm/include/asm/efi.h                |   6 +-
> > > >  arch/arm64/include/asm/efi.h              |   2 +-
> > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> > > >  6 files changed, 47 insertions(+), 147 deletions(-)
> >
>
> I verified the above patches along with the following RISC-V specific changes.
>
> diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> index 93c305a638f4..dd6ceea9d548 100644
> --- a/arch/riscv/include/asm/efi.h
> +++ b/arch/riscv/include/asm/efi.h
> @@ -37,7 +37,7 @@ static inline unsigned long
> efi_get_max_fdt_addr(unsigned long dram_base)
>  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
>                                                     unsigned long image_addr)
>  {
> -       return dram_base + SZ_256M;
> +       return image_addr + SZ_256M;
>  }
>

Ah yes, we need this change as well - this is a bit unfortunate since
that creates a conflict with the RISC-V tree.

> --- a/drivers/firmware/efi/libstub/riscv-stub.c
> +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>          */
>         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
>         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
> +                                    0, MIN_KIMG_ALIGN, 0);
>
> FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>

Thanks for confirming.

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

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Ard Biesheuvel]

On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
> >
> > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> > >
> > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > > >
> > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > > > > Maxim reports boot failures on platforms that describe reserved memory
> > > > > regions in DT that are disjoint from system DRAM, and which are converted
> > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > > > >
> > > > > As it turns out, the whole notion of discovering the base of DRAM is
> > > > > problematic, and it would be better to simply rely on the EFI memory
> > > > > allocation routines instead, and derive the FDT and initrd allocation
> > > > > limits from the actual placement of the kernel (which is what defines
> > > > > the start of the linear region anyway)
> > > > >
> > > > > Finally, we should be able to get rid of get_dram_base() entirely.
> > > > > However, as RISC-V only just started using it, we will need to address
> > > > > that at a later time.
> > > >
> > > > Looks like we're using dram_base to derive two argumets to
> > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > > > x86 uses for the minimum load address, but I don't think we have any mechanism
> > > > like "struct boot_params" so we'd need to come up with something.
> > > >
> > >
> > > As discussed in the other thread
> > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> > > we don't need to do anything special. efi_relocate_kernel can just
> > > take preferred address as 0
> > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> > > allocate 2MB/4MB aligned address as per requirement.
> > >
> > > I don't think the other changes in this series will cause any issue
> > > for RISC-V. I will test it and update anyways.
> > >
> > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > > > Cc: Grant Likely <Grant.Likely@arm.com>
> > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > > > >
> > > > > Ard Biesheuvel (3):
> > > > >   efi/libstub: Export efi_low_alloc_above() to other units
> > > > >   efi/libstub: Use low allocation for the uncompressed kernel
> > > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> > > > >     base
> > > > >
> > > > >  arch/arm/include/asm/efi.h                |   6 +-
> > > > >  arch/arm64/include/asm/efi.h              |   2 +-
> > > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> > > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> > > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> > > > >  6 files changed, 47 insertions(+), 147 deletions(-)
> > >
> >
> > I verified the above patches along with the following RISC-V specific changes.
> >
> > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> > index 93c305a638f4..dd6ceea9d548 100644
> > --- a/arch/riscv/include/asm/efi.h
> > +++ b/arch/riscv/include/asm/efi.h
> > @@ -37,7 +37,7 @@ static inline unsigned long
> > efi_get_max_fdt_addr(unsigned long dram_base)
> >  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
> >                                                     unsigned long image_addr)
> >  {
> > -       return dram_base + SZ_256M;
> > +       return image_addr + SZ_256M;
> >  }
> >
>
> Ah yes, we need this change as well - this is a bit unfortunate since
> that creates a conflict with the RISC-V tree.
>
> > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> >          */
> >         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
> >         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> > -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
> > +                                    0, MIN_KIMG_ALIGN, 0);
> >
> > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
>
> Thanks for confirming.

OK,

So, just to annoy Palmer and you more than I already have up to this
point: any chance we could do a final respin of the RISC-V code on top
of these changes? They are important for ARM, and I would prefer these
to be merged in a way that makes it easy to backport them to -stable
if needed.

So what I would suggest is:
- I will create a new 'shared-efi' tag/stable branch containing the
existing two patches, as well as these changes (in a slightly updated
form)
- Palmer creates a new topic branch in the riscv repo based on this
shared tag, and applies the [updated] RISC-V patches on top
- Palmer drops the current version of the riscv patches from
riscv/for-next, and merges the topic branch into it instead.

Again, sorry to be a pain, but I think this is the cleanest way to get
these changes queued up for v5.10 without painting ourselves into a
corner too much when it comes to future follow-up changes.

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Ard Biesheuvel]

On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
> >
> > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> > >
> > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > > >
> > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > > > > Maxim reports boot failures on platforms that describe reserved memory
> > > > > regions in DT that are disjoint from system DRAM, and which are converted
> > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > > > >
> > > > > As it turns out, the whole notion of discovering the base of DRAM is
> > > > > problematic, and it would be better to simply rely on the EFI memory
> > > > > allocation routines instead, and derive the FDT and initrd allocation
> > > > > limits from the actual placement of the kernel (which is what defines
> > > > > the start of the linear region anyway)
> > > > >
> > > > > Finally, we should be able to get rid of get_dram_base() entirely.
> > > > > However, as RISC-V only just started using it, we will need to address
> > > > > that at a later time.
> > > >
> > > > Looks like we're using dram_base to derive two argumets to
> > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > > > x86 uses for the minimum load address, but I don't think we have any mechanism
> > > > like "struct boot_params" so we'd need to come up with something.
> > > >
> > >
> > > As discussed in the other thread
> > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> > > we don't need to do anything special. efi_relocate_kernel can just
> > > take preferred address as 0
> > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> > > allocate 2MB/4MB aligned address as per requirement.
> > >
> > > I don't think the other changes in this series will cause any issue
> > > for RISC-V. I will test it and update anyways.
> > >
> > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > > > Cc: Grant Likely <Grant.Likely@arm.com>
> > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > > > >
> > > > > Ard Biesheuvel (3):
> > > > >   efi/libstub: Export efi_low_alloc_above() to other units
> > > > >   efi/libstub: Use low allocation for the uncompressed kernel
> > > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> > > > >     base
> > > > >
> > > > >  arch/arm/include/asm/efi.h                |   6 +-
> > > > >  arch/arm64/include/asm/efi.h              |   2 +-
> > > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> > > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> > > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> > > > >  6 files changed, 47 insertions(+), 147 deletions(-)
> > >
> >
> > I verified the above patches along with the following RISC-V specific changes.
> >
> > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> > index 93c305a638f4..dd6ceea9d548 100644
> > --- a/arch/riscv/include/asm/efi.h
> > +++ b/arch/riscv/include/asm/efi.h
> > @@ -37,7 +37,7 @@ static inline unsigned long
> > efi_get_max_fdt_addr(unsigned long dram_base)
> >  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
> >                                                     unsigned long image_addr)
> >  {
> > -       return dram_base + SZ_256M;
> > +       return image_addr + SZ_256M;
> >  }
> >
>
> Ah yes, we need this change as well - this is a bit unfortunate since
> that creates a conflict with the RISC-V tree.
>
> > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> >          */
> >         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
> >         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> > -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
> > +                                    0, MIN_KIMG_ALIGN, 0);
> >
> > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
>
> Thanks for confirming.

OK,

So, just to annoy Palmer and you more than I already have up to this
point: any chance we could do a final respin of the RISC-V code on top
of these changes? They are important for ARM, and I would prefer these
to be merged in a way that makes it easy to backport them to -stable
if needed.

So what I would suggest is:
- I will create a new 'shared-efi' tag/stable branch containing the
existing two patches, as well as these changes (in a slightly updated
form)
- Palmer creates a new topic branch in the riscv repo based on this
shared tag, and applies the [updated] RISC-V patches on top
- Palmer drops the current version of the riscv patches from
riscv/for-next, and merges the topic branch into it instead.

Again, sorry to be a pain, but I think this is the cleanest way to get
these changes queued up for v5.10 without painting ourselves into a
corner too much when it comes to future follow-up changes.

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

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Atish Patra]

On Thu, Sep 10, 2020 at 7:08 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
> > >
> > > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> > > >
> > > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > > > >
> > > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > > > > > Maxim reports boot failures on platforms that describe reserved memory
> > > > > > regions in DT that are disjoint from system DRAM, and which are converted
> > > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > > > > >
> > > > > > As it turns out, the whole notion of discovering the base of DRAM is
> > > > > > problematic, and it would be better to simply rely on the EFI memory
> > > > > > allocation routines instead, and derive the FDT and initrd allocation
> > > > > > limits from the actual placement of the kernel (which is what defines
> > > > > > the start of the linear region anyway)
> > > > > >
> > > > > > Finally, we should be able to get rid of get_dram_base() entirely.
> > > > > > However, as RISC-V only just started using it, we will need to address
> > > > > > that at a later time.
> > > > >
> > > > > Looks like we're using dram_base to derive two argumets to
> > > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > > > > x86 uses for the minimum load address, but I don't think we have any mechanism
> > > > > like "struct boot_params" so we'd need to come up with something.
> > > > >
> > > >
> > > > As discussed in the other thread
> > > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> > > > we don't need to do anything special. efi_relocate_kernel can just
> > > > take preferred address as 0
> > > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> > > > allocate 2MB/4MB aligned address as per requirement.
> > > >
> > > > I don't think the other changes in this series will cause any issue
> > > > for RISC-V. I will test it and update anyways.
> > > >
> > > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > > > > Cc: Grant Likely <Grant.Likely@arm.com>
> > > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > > > > >
> > > > > > Ard Biesheuvel (3):
> > > > > >   efi/libstub: Export efi_low_alloc_above() to other units
> > > > > >   efi/libstub: Use low allocation for the uncompressed kernel
> > > > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> > > > > >     base
> > > > > >
> > > > > >  arch/arm/include/asm/efi.h                |   6 +-
> > > > > >  arch/arm64/include/asm/efi.h              |   2 +-
> > > > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > > > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> > > > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> > > > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> > > > > >  6 files changed, 47 insertions(+), 147 deletions(-)
> > > >
> > >
> > > I verified the above patches along with the following RISC-V specific changes.
> > >
> > > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> > > index 93c305a638f4..dd6ceea9d548 100644
> > > --- a/arch/riscv/include/asm/efi.h
> > > +++ b/arch/riscv/include/asm/efi.h
> > > @@ -37,7 +37,7 @@ static inline unsigned long
> > > efi_get_max_fdt_addr(unsigned long dram_base)
> > >  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
> > >                                                     unsigned long image_addr)
> > >  {
> > > -       return dram_base + SZ_256M;
> > > +       return image_addr + SZ_256M;
> > >  }
> > >
> >
> > Ah yes, we need this change as well - this is a bit unfortunate since
> > that creates a conflict with the RISC-V tree.
> >
> > > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> > > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> > > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> > >          */
> > >         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
> > >         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> > > -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
> > > +                                    0, MIN_KIMG_ALIGN, 0);
> > >
> > > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
> >
> > Thanks for confirming.
>
> OK,
>
> So, just to annoy Palmer and you more than I already have up to this
> point: any chance we could do a final respin of the RISC-V code on top
> of these changes? They are important for ARM, and I would prefer these
> to be merged in a way that makes it easy to backport them to -stable
> if needed.
>

No worries. It's better to address these issues now rather than
patching it after the code is merged.
I will rebase and update the RISC-V patch series on top of this series
as per above discussion.

Should I also add a patch to remove get_dram_base() completely or are
you planning to do that ?

> So what I would suggest is:
> - I will create a new 'shared-efi' tag/stable branch containing the
> existing two patches, as well as these changes (in a slightly updated
> form)
> - Palmer creates a new topic branch in the riscv repo based on this
> shared tag, and applies the [updated] RISC-V patches on top
> - Palmer drops the current version of the riscv patches from
> riscv/for-next, and merges the topic branch into it instead.
>
> Again, sorry to be a pain, but I think this is the cleanest way to get
> these changes queued up for v5.10 without painting ourselves into a
> corner too much when it comes to future follow-up changes.

Sounds good to me. I will try to send a v8 early next week and let
palmer decide how he wants to proceed.

-- 
Regards,
Atish

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Atish Patra]

On Thu, Sep 10, 2020 at 7:08 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
> > >
> > > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> > > >
> > > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > > > >
> > > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > > > > > Maxim reports boot failures on platforms that describe reserved memory
> > > > > > regions in DT that are disjoint from system DRAM, and which are converted
> > > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > > > > >
> > > > > > As it turns out, the whole notion of discovering the base of DRAM is
> > > > > > problematic, and it would be better to simply rely on the EFI memory
> > > > > > allocation routines instead, and derive the FDT and initrd allocation
> > > > > > limits from the actual placement of the kernel (which is what defines
> > > > > > the start of the linear region anyway)
> > > > > >
> > > > > > Finally, we should be able to get rid of get_dram_base() entirely.
> > > > > > However, as RISC-V only just started using it, we will need to address
> > > > > > that at a later time.
> > > > >
> > > > > Looks like we're using dram_base to derive two argumets to
> > > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > > > > x86 uses for the minimum load address, but I don't think we have any mechanism
> > > > > like "struct boot_params" so we'd need to come up with something.
> > > > >
> > > >
> > > > As discussed in the other thread
> > > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> > > > we don't need to do anything special. efi_relocate_kernel can just
> > > > take preferred address as 0
> > > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> > > > allocate 2MB/4MB aligned address as per requirement.
> > > >
> > > > I don't think the other changes in this series will cause any issue
> > > > for RISC-V. I will test it and update anyways.
> > > >
> > > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > > > > Cc: Grant Likely <Grant.Likely@arm.com>
> > > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > > > > >
> > > > > > Ard Biesheuvel (3):
> > > > > >   efi/libstub: Export efi_low_alloc_above() to other units
> > > > > >   efi/libstub: Use low allocation for the uncompressed kernel
> > > > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> > > > > >     base
> > > > > >
> > > > > >  arch/arm/include/asm/efi.h                |   6 +-
> > > > > >  arch/arm64/include/asm/efi.h              |   2 +-
> > > > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > > > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> > > > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> > > > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> > > > > >  6 files changed, 47 insertions(+), 147 deletions(-)
> > > >
> > >
> > > I verified the above patches along with the following RISC-V specific changes.
> > >
> > > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> > > index 93c305a638f4..dd6ceea9d548 100644
> > > --- a/arch/riscv/include/asm/efi.h
> > > +++ b/arch/riscv/include/asm/efi.h
> > > @@ -37,7 +37,7 @@ static inline unsigned long
> > > efi_get_max_fdt_addr(unsigned long dram_base)
> > >  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
> > >                                                     unsigned long image_addr)
> > >  {
> > > -       return dram_base + SZ_256M;
> > > +       return image_addr + SZ_256M;
> > >  }
> > >
> >
> > Ah yes, we need this change as well - this is a bit unfortunate since
> > that creates a conflict with the RISC-V tree.
> >
> > > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> > > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> > > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> > >          */
> > >         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
> > >         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> > > -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
> > > +                                    0, MIN_KIMG_ALIGN, 0);
> > >
> > > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
> >
> > Thanks for confirming.
>
> OK,
>
> So, just to annoy Palmer and you more than I already have up to this
> point: any chance we could do a final respin of the RISC-V code on top
> of these changes? They are important for ARM, and I would prefer these
> to be merged in a way that makes it easy to backport them to -stable
> if needed.
>

No worries. It's better to address these issues now rather than
patching it after the code is merged.
I will rebase and update the RISC-V patch series on top of this series
as per above discussion.

Should I also add a patch to remove get_dram_base() completely or are
you planning to do that ?

> So what I would suggest is:
> - I will create a new 'shared-efi' tag/stable branch containing the
> existing two patches, as well as these changes (in a slightly updated
> form)
> - Palmer creates a new topic branch in the riscv repo based on this
> shared tag, and applies the [updated] RISC-V patches on top
> - Palmer drops the current version of the riscv patches from
> riscv/for-next, and merges the topic branch into it instead.
>
> Again, sorry to be a pain, but I think this is the cleanest way to get
> these changes queued up for v5.10 without painting ourselves into a
> corner too much when it comes to future follow-up changes.

Sounds good to me. I will try to send a v8 early next week and let
palmer decide how he wants to proceed.

-- 
Regards,
Atish

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

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Palmer Dabbelt]

On Thu, 10 Sep 2020 07:08:07 PDT (-0700), ardb@kernel.org wrote:
> On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
>>
>> On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
>> >
>> > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
>> > >
>> > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> > > >
>> > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
>> > > > > Maxim reports boot failures on platforms that describe reserved memory
>> > > > > regions in DT that are disjoint from system DRAM, and which are converted
>> > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
>> > > > >
>> > > > > As it turns out, the whole notion of discovering the base of DRAM is
>> > > > > problematic, and it would be better to simply rely on the EFI memory
>> > > > > allocation routines instead, and derive the FDT and initrd allocation
>> > > > > limits from the actual placement of the kernel (which is what defines
>> > > > > the start of the linear region anyway)
>> > > > >
>> > > > > Finally, we should be able to get rid of get_dram_base() entirely.
>> > > > > However, as RISC-V only just started using it, we will need to address
>> > > > > that at a later time.
>> > > >
>> > > > Looks like we're using dram_base to derive two argumets to
>> > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
>> > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
>> > > > x86 uses for the minimum load address, but I don't think we have any mechanism
>> > > > like "struct boot_params" so we'd need to come up with something.
>> > > >
>> > >
>> > > As discussed in the other thread
>> > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
>> > > we don't need to do anything special. efi_relocate_kernel can just
>> > > take preferred address as 0
>> > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
>> > > allocate 2MB/4MB aligned address as per requirement.
>> > >
>> > > I don't think the other changes in this series will cause any issue
>> > > for RISC-V. I will test it and update anyways.
>> > >
>> > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
>> > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> > > > > Cc: Atish Patra <atish.patra@wdc.com>
>> > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
>> > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
>> > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
>> > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
>> > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
>> > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
>> > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
>> > > > > Cc: Grant Likely <Grant.Likely@arm.com>
>> > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>> > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
>> > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
>> > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
>> > > > >
>> > > > > Ard Biesheuvel (3):
>> > > > >   efi/libstub: Export efi_low_alloc_above() to other units
>> > > > >   efi/libstub: Use low allocation for the uncompressed kernel
>> > > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
>> > > > >     base
>> > > > >
>> > > > >  arch/arm/include/asm/efi.h                |   6 +-
>> > > > >  arch/arm64/include/asm/efi.h              |   2 +-
>> > > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
>> > > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
>> > > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
>> > > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
>> > > > >  6 files changed, 47 insertions(+), 147 deletions(-)
>> > >
>> >
>> > I verified the above patches along with the following RISC-V specific changes.
>> >
>> > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
>> > index 93c305a638f4..dd6ceea9d548 100644
>> > --- a/arch/riscv/include/asm/efi.h
>> > +++ b/arch/riscv/include/asm/efi.h
>> > @@ -37,7 +37,7 @@ static inline unsigned long
>> > efi_get_max_fdt_addr(unsigned long dram_base)
>> >  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
>> >                                                     unsigned long image_addr)
>> >  {
>> > -       return dram_base + SZ_256M;
>> > +       return image_addr + SZ_256M;
>> >  }
>> >
>>
>> Ah yes, we need this change as well - this is a bit unfortunate since
>> that creates a conflict with the RISC-V tree.
>>
>> > --- a/drivers/firmware/efi/libstub/riscv-stub.c
>> > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
>> > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>> >          */
>> >         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
>> >         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
>> > -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
>> > +                                    0, MIN_KIMG_ALIGN, 0);
>> >
>> > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
>>
>> Thanks for confirming.
>
> OK,
>
> So, just to annoy Palmer and you more than I already have up to this
> point: any chance we could do a final respin of the RISC-V code on top
> of these changes? They are important for ARM, and I would prefer these
> to be merged in a way that makes it easy to backport them to -stable
> if needed.
>
> So what I would suggest is:
> - I will create a new 'shared-efi' tag/stable branch containing the
> existing two patches, as well as these changes (in a slightly updated
> form)
> - Palmer creates a new topic branch in the riscv repo based on this
> shared tag, and applies the [updated] RISC-V patches on top
> - Palmer drops the current version of the riscv patches from
> riscv/for-next, and merges the topic branch into it instead.
>
> Again, sorry to be a pain, but I think this is the cleanest way to get
> these changes queued up for v5.10 without painting ourselves into a
> corner too much when it comes to future follow-up changes.

That's fine for me.

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Palmer Dabbelt]

On Thu, 10 Sep 2020 07:08:07 PDT (-0700), ardb@kernel.org wrote:
> On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
>>
>> On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
>> >
>> > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
>> > >
>> > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>> > > >
>> > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
>> > > > > Maxim reports boot failures on platforms that describe reserved memory
>> > > > > regions in DT that are disjoint from system DRAM, and which are converted
>> > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
>> > > > >
>> > > > > As it turns out, the whole notion of discovering the base of DRAM is
>> > > > > problematic, and it would be better to simply rely on the EFI memory
>> > > > > allocation routines instead, and derive the FDT and initrd allocation
>> > > > > limits from the actual placement of the kernel (which is what defines
>> > > > > the start of the linear region anyway)
>> > > > >
>> > > > > Finally, we should be able to get rid of get_dram_base() entirely.
>> > > > > However, as RISC-V only just started using it, we will need to address
>> > > > > that at a later time.
>> > > >
>> > > > Looks like we're using dram_base to derive two argumets to
>> > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
>> > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
>> > > > x86 uses for the minimum load address, but I don't think we have any mechanism
>> > > > like "struct boot_params" so we'd need to come up with something.
>> > > >
>> > >
>> > > As discussed in the other thread
>> > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
>> > > we don't need to do anything special. efi_relocate_kernel can just
>> > > take preferred address as 0
>> > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
>> > > allocate 2MB/4MB aligned address as per requirement.
>> > >
>> > > I don't think the other changes in this series will cause any issue
>> > > for RISC-V. I will test it and update anyways.
>> > >
>> > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
>> > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> > > > > Cc: Atish Patra <atish.patra@wdc.com>
>> > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
>> > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
>> > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
>> > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
>> > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
>> > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
>> > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
>> > > > > Cc: Grant Likely <Grant.Likely@arm.com>
>> > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>> > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
>> > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
>> > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
>> > > > >
>> > > > > Ard Biesheuvel (3):
>> > > > >   efi/libstub: Export efi_low_alloc_above() to other units
>> > > > >   efi/libstub: Use low allocation for the uncompressed kernel
>> > > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
>> > > > >     base
>> > > > >
>> > > > >  arch/arm/include/asm/efi.h                |   6 +-
>> > > > >  arch/arm64/include/asm/efi.h              |   2 +-
>> > > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
>> > > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
>> > > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
>> > > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
>> > > > >  6 files changed, 47 insertions(+), 147 deletions(-)
>> > >
>> >
>> > I verified the above patches along with the following RISC-V specific changes.
>> >
>> > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
>> > index 93c305a638f4..dd6ceea9d548 100644
>> > --- a/arch/riscv/include/asm/efi.h
>> > +++ b/arch/riscv/include/asm/efi.h
>> > @@ -37,7 +37,7 @@ static inline unsigned long
>> > efi_get_max_fdt_addr(unsigned long dram_base)
>> >  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
>> >                                                     unsigned long image_addr)
>> >  {
>> > -       return dram_base + SZ_256M;
>> > +       return image_addr + SZ_256M;
>> >  }
>> >
>>
>> Ah yes, we need this change as well - this is a bit unfortunate since
>> that creates a conflict with the RISC-V tree.
>>
>> > --- a/drivers/firmware/efi/libstub/riscv-stub.c
>> > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
>> > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>> >          */
>> >         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
>> >         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
>> > -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
>> > +                                    0, MIN_KIMG_ALIGN, 0);
>> >
>> > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
>>
>> Thanks for confirming.
>
> OK,
>
> So, just to annoy Palmer and you more than I already have up to this
> point: any chance we could do a final respin of the RISC-V code on top
> of these changes? They are important for ARM, and I would prefer these
> to be merged in a way that makes it easy to backport them to -stable
> if needed.
>
> So what I would suggest is:
> - I will create a new 'shared-efi' tag/stable branch containing the
> existing two patches, as well as these changes (in a slightly updated
> form)
> - Palmer creates a new topic branch in the riscv repo based on this
> shared tag, and applies the [updated] RISC-V patches on top
> - Palmer drops the current version of the riscv patches from
> riscv/for-next, and merges the topic branch into it instead.
>
> Again, sorry to be a pain, but I think this is the cleanest way to get
> these changes queued up for v5.10 without painting ourselves into a
> corner too much when it comes to future follow-up changes.

That's fine for me.

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

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Ard Biesheuvel]

On Fri, 11 Sep 2020 at 05:16, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Thu, 10 Sep 2020 07:08:07 PDT (-0700), ardb@kernel.org wrote:
> > On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
> >>
> >> On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
> >> >
> >> > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> >> > >
> >> > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >> > > >
> >> > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> >> > > > > Maxim reports boot failures on platforms that describe reserved memory
> >> > > > > regions in DT that are disjoint from system DRAM, and which are converted
> >> > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> >> > > > >
> >> > > > > As it turns out, the whole notion of discovering the base of DRAM is
> >> > > > > problematic, and it would be better to simply rely on the EFI memory
> >> > > > > allocation routines instead, and derive the FDT and initrd allocation
> >> > > > > limits from the actual placement of the kernel (which is what defines
> >> > > > > the start of the linear region anyway)
> >> > > > >
> >> > > > > Finally, we should be able to get rid of get_dram_base() entirely.
> >> > > > > However, as RISC-V only just started using it, we will need to address
> >> > > > > that at a later time.
> >> > > >
> >> > > > Looks like we're using dram_base to derive two argumets to
> >> > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> >> > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> >> > > > x86 uses for the minimum load address, but I don't think we have any mechanism
> >> > > > like "struct boot_params" so we'd need to come up with something.
> >> > > >
> >> > >
> >> > > As discussed in the other thread
> >> > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> >> > > we don't need to do anything special. efi_relocate_kernel can just
> >> > > take preferred address as 0
> >> > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> >> > > allocate 2MB/4MB aligned address as per requirement.
> >> > >
> >> > > I don't think the other changes in this series will cause any issue
> >> > > for RISC-V. I will test it and update anyways.
> >> > >
> >> > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> >> > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >> > > > > Cc: Atish Patra <atish.patra@wdc.com>
> >> > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> >> > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> >> > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> >> > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> >> > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> >> > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> >> > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> >> > > > > Cc: Grant Likely <Grant.Likely@arm.com>
> >> > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> >> > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> >> > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> >> > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> >> > > > >
> >> > > > > Ard Biesheuvel (3):
> >> > > > >   efi/libstub: Export efi_low_alloc_above() to other units
> >> > > > >   efi/libstub: Use low allocation for the uncompressed kernel
> >> > > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> >> > > > >     base
> >> > > > >
> >> > > > >  arch/arm/include/asm/efi.h                |   6 +-
> >> > > > >  arch/arm64/include/asm/efi.h              |   2 +-
> >> > > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> >> > > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> >> > > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> >> > > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> >> > > > >  6 files changed, 47 insertions(+), 147 deletions(-)
> >> > >
> >> >
> >> > I verified the above patches along with the following RISC-V specific changes.
> >> >
> >> > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> >> > index 93c305a638f4..dd6ceea9d548 100644
> >> > --- a/arch/riscv/include/asm/efi.h
> >> > +++ b/arch/riscv/include/asm/efi.h
> >> > @@ -37,7 +37,7 @@ static inline unsigned long
> >> > efi_get_max_fdt_addr(unsigned long dram_base)
> >> >  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
> >> >                                                     unsigned long image_addr)
> >> >  {
> >> > -       return dram_base + SZ_256M;
> >> > +       return image_addr + SZ_256M;
> >> >  }
> >> >
> >>
> >> Ah yes, we need this change as well - this is a bit unfortunate since
> >> that creates a conflict with the RISC-V tree.
> >>
> >> > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> >> > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> >> > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> >> >          */
> >> >         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
> >> >         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> >> > -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
> >> > +                                    0, MIN_KIMG_ALIGN, 0);
> >> >
> >> > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
> >>
> >> Thanks for confirming.
> >
> > OK,
> >
> > So, just to annoy Palmer and you more than I already have up to this
> > point: any chance we could do a final respin of the RISC-V code on top
> > of these changes? They are important for ARM, and I would prefer these
> > to be merged in a way that makes it easy to backport them to -stable
> > if needed.
> >
> > So what I would suggest is:
> > - I will create a new 'shared-efi' tag/stable branch containing the
> > existing two patches, as well as these changes (in a slightly updated
> > form)
> > - Palmer creates a new topic branch in the riscv repo based on this
> > shared tag, and applies the [updated] RISC-V patches on top
> > - Palmer drops the current version of the riscv patches from
> > riscv/for-next, and merges the topic branch into it instead.
> >
> > Again, sorry to be a pain, but I think this is the cleanest way to get
> > these changes queued up for v5.10 without painting ourselves into a
> > corner too much when it comes to future follow-up changes.
>
> That's fine for me.

Excellent.

I have created a signed tag efi-riscv-shared-for-v5.10 in the EFI repo
[0], which is based on v5.9-rc1. Please merge that at the start of
your for-next/efi topic branch, and apply the reworked EFI for RISC-V
series on top.

I have created a preliminary version of that branch as 'riscv-tmp' on
[1], incorporating some changes that are needed for the rebase. Note
that I applied some other tweaks as well - one is in a separate patch
on top, the other is that I omitted the Makefile rule for .stub.o
objects under arch/riscv/Makefile, as it is not actually used.

Atish - please pick whatever seems useful to you from that branch when
you do the respin.

Thanks,
Ard.



[0] git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
[1] git://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Ard Biesheuvel]

On Fri, 11 Sep 2020 at 05:16, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Thu, 10 Sep 2020 07:08:07 PDT (-0700), ardb@kernel.org wrote:
> > On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
> >>
> >> On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
> >> >
> >> > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> >> > >
> >> > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >> > > >
> >> > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> >> > > > > Maxim reports boot failures on platforms that describe reserved memory
> >> > > > > regions in DT that are disjoint from system DRAM, and which are converted
> >> > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> >> > > > >
> >> > > > > As it turns out, the whole notion of discovering the base of DRAM is
> >> > > > > problematic, and it would be better to simply rely on the EFI memory
> >> > > > > allocation routines instead, and derive the FDT and initrd allocation
> >> > > > > limits from the actual placement of the kernel (which is what defines
> >> > > > > the start of the linear region anyway)
> >> > > > >
> >> > > > > Finally, we should be able to get rid of get_dram_base() entirely.
> >> > > > > However, as RISC-V only just started using it, we will need to address
> >> > > > > that at a later time.
> >> > > >
> >> > > > Looks like we're using dram_base to derive two argumets to
> >> > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> >> > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> >> > > > x86 uses for the minimum load address, but I don't think we have any mechanism
> >> > > > like "struct boot_params" so we'd need to come up with something.
> >> > > >
> >> > >
> >> > > As discussed in the other thread
> >> > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> >> > > we don't need to do anything special. efi_relocate_kernel can just
> >> > > take preferred address as 0
> >> > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> >> > > allocate 2MB/4MB aligned address as per requirement.
> >> > >
> >> > > I don't think the other changes in this series will cause any issue
> >> > > for RISC-V. I will test it and update anyways.
> >> > >
> >> > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> >> > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >> > > > > Cc: Atish Patra <atish.patra@wdc.com>
> >> > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> >> > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> >> > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> >> > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> >> > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> >> > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> >> > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> >> > > > > Cc: Grant Likely <Grant.Likely@arm.com>
> >> > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> >> > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> >> > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> >> > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> >> > > > >
> >> > > > > Ard Biesheuvel (3):
> >> > > > >   efi/libstub: Export efi_low_alloc_above() to other units
> >> > > > >   efi/libstub: Use low allocation for the uncompressed kernel
> >> > > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> >> > > > >     base
> >> > > > >
> >> > > > >  arch/arm/include/asm/efi.h                |   6 +-
> >> > > > >  arch/arm64/include/asm/efi.h              |   2 +-
> >> > > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> >> > > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> >> > > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> >> > > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> >> > > > >  6 files changed, 47 insertions(+), 147 deletions(-)
> >> > >
> >> >
> >> > I verified the above patches along with the following RISC-V specific changes.
> >> >
> >> > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> >> > index 93c305a638f4..dd6ceea9d548 100644
> >> > --- a/arch/riscv/include/asm/efi.h
> >> > +++ b/arch/riscv/include/asm/efi.h
> >> > @@ -37,7 +37,7 @@ static inline unsigned long
> >> > efi_get_max_fdt_addr(unsigned long dram_base)
> >> >  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
> >> >                                                     unsigned long image_addr)
> >> >  {
> >> > -       return dram_base + SZ_256M;
> >> > +       return image_addr + SZ_256M;
> >> >  }
> >> >
> >>
> >> Ah yes, we need this change as well - this is a bit unfortunate since
> >> that creates a conflict with the RISC-V tree.
> >>
> >> > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> >> > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> >> > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> >> >          */
> >> >         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
> >> >         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> >> > -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
> >> > +                                    0, MIN_KIMG_ALIGN, 0);
> >> >
> >> > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
> >>
> >> Thanks for confirming.
> >
> > OK,
> >
> > So, just to annoy Palmer and you more than I already have up to this
> > point: any chance we could do a final respin of the RISC-V code on top
> > of these changes? They are important for ARM, and I would prefer these
> > to be merged in a way that makes it easy to backport them to -stable
> > if needed.
> >
> > So what I would suggest is:
> > - I will create a new 'shared-efi' tag/stable branch containing the
> > existing two patches, as well as these changes (in a slightly updated
> > form)
> > - Palmer creates a new topic branch in the riscv repo based on this
> > shared tag, and applies the [updated] RISC-V patches on top
> > - Palmer drops the current version of the riscv patches from
> > riscv/for-next, and merges the topic branch into it instead.
> >
> > Again, sorry to be a pain, but I think this is the cleanest way to get
> > these changes queued up for v5.10 without painting ourselves into a
> > corner too much when it comes to future follow-up changes.
>
> That's fine for me.

Excellent.

I have created a signed tag efi-riscv-shared-for-v5.10 in the EFI repo
[0], which is based on v5.9-rc1. Please merge that at the start of
your for-next/efi topic branch, and apply the reworked EFI for RISC-V
series on top.

I have created a preliminary version of that branch as 'riscv-tmp' on
[1], incorporating some changes that are needed for the rebase. Note
that I applied some other tweaks as well - one is in a separate patch
on top, the other is that I omitted the Makefile rule for .stub.o
objects under arch/riscv/Makefile, as it is not actually used.

Atish - please pick whatever seems useful to you from that branch when
you do the respin.

Thanks,
Ard.



[0] git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
[1] git://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git

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

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Maxim Uvarov]

Changes look fine for and should fix booting, while I can test them in
my environment next week.

Thanks,
Maxim.

On Fri, 11 Sep 2020 at 10:56, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 11 Sep 2020 at 05:16, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >
> > On Thu, 10 Sep 2020 07:08:07 PDT (-0700), ardb@kernel.org wrote:
> > > On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >>
> > >> On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
> > >> >
> > >> > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> > >> > >
> > >> > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > >> > > >
> > >> > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > >> > > > > Maxim reports boot failures on platforms that describe reserved memory
> > >> > > > > regions in DT that are disjoint from system DRAM, and which are converted
> > >> > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > >> > > > >
> > >> > > > > As it turns out, the whole notion of discovering the base of DRAM is
> > >> > > > > problematic, and it would be better to simply rely on the EFI memory
> > >> > > > > allocation routines instead, and derive the FDT and initrd allocation
> > >> > > > > limits from the actual placement of the kernel (which is what defines
> > >> > > > > the start of the linear region anyway)
> > >> > > > >
> > >> > > > > Finally, we should be able to get rid of get_dram_base() entirely.
> > >> > > > > However, as RISC-V only just started using it, we will need to address
> > >> > > > > that at a later time.
> > >> > > >
> > >> > > > Looks like we're using dram_base to derive two argumets to
> > >> > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > >> > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > >> > > > x86 uses for the minimum load address, but I don't think we have any mechanism
> > >> > > > like "struct boot_params" so we'd need to come up with something.
> > >> > > >
> > >> > >
> > >> > > As discussed in the other thread
> > >> > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> > >> > > we don't need to do anything special. efi_relocate_kernel can just
> > >> > > take preferred address as 0
> > >> > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> > >> > > allocate 2MB/4MB aligned address as per requirement.
> > >> > >
> > >> > > I don't think the other changes in this series will cause any issue
> > >> > > for RISC-V. I will test it and update anyways.
> > >> > >
> > >> > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > >> > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > >> > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > >> > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > >> > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > >> > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > >> > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > >> > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > >> > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > >> > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > >> > > > > Cc: Grant Likely <Grant.Likely@arm.com>
> > >> > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > >> > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > >> > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > >> > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > >> > > > >
> > >> > > > > Ard Biesheuvel (3):
> > >> > > > >   efi/libstub: Export efi_low_alloc_above() to other units
> > >> > > > >   efi/libstub: Use low allocation for the uncompressed kernel
> > >> > > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> > >> > > > >     base
> > >> > > > >
> > >> > > > >  arch/arm/include/asm/efi.h                |   6 +-
> > >> > > > >  arch/arm64/include/asm/efi.h              |   2 +-
> > >> > > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > >> > > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> > >> > > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> > >> > > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> > >> > > > >  6 files changed, 47 insertions(+), 147 deletions(-)
> > >> > >
> > >> >
> > >> > I verified the above patches along with the following RISC-V specific changes.
> > >> >
> > >> > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> > >> > index 93c305a638f4..dd6ceea9d548 100644
> > >> > --- a/arch/riscv/include/asm/efi.h
> > >> > +++ b/arch/riscv/include/asm/efi.h
> > >> > @@ -37,7 +37,7 @@ static inline unsigned long
> > >> > efi_get_max_fdt_addr(unsigned long dram_base)
> > >> >  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
> > >> >                                                     unsigned long image_addr)
> > >> >  {
> > >> > -       return dram_base + SZ_256M;
> > >> > +       return image_addr + SZ_256M;
> > >> >  }
> > >> >
> > >>
> > >> Ah yes, we need this change as well - this is a bit unfortunate since
> > >> that creates a conflict with the RISC-V tree.
> > >>
> > >> > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> > >> > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> > >> > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> > >> >          */
> > >> >         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
> > >> >         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> > >> > -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
> > >> > +                                    0, MIN_KIMG_ALIGN, 0);
> > >> >
> > >> > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
> > >>
> > >> Thanks for confirming.
> > >
> > > OK,
> > >
> > > So, just to annoy Palmer and you more than I already have up to this
> > > point: any chance we could do a final respin of the RISC-V code on top
> > > of these changes? They are important for ARM, and I would prefer these
> > > to be merged in a way that makes it easy to backport them to -stable
> > > if needed.
> > >
> > > So what I would suggest is:
> > > - I will create a new 'shared-efi' tag/stable branch containing the
> > > existing two patches, as well as these changes (in a slightly updated
> > > form)
> > > - Palmer creates a new topic branch in the riscv repo based on this
> > > shared tag, and applies the [updated] RISC-V patches on top
> > > - Palmer drops the current version of the riscv patches from
> > > riscv/for-next, and merges the topic branch into it instead.
> > >
> > > Again, sorry to be a pain, but I think this is the cleanest way to get
> > > these changes queued up for v5.10 without painting ourselves into a
> > > corner too much when it comes to future follow-up changes.
> >
> > That's fine for me.
>
> Excellent.
>
> I have created a signed tag efi-riscv-shared-for-v5.10 in the EFI repo
> [0], which is based on v5.9-rc1. Please merge that at the start of
> your for-next/efi topic branch, and apply the reworked EFI for RISC-V
> series on top.
>
> I have created a preliminary version of that branch as 'riscv-tmp' on
> [1], incorporating some changes that are needed for the rebase. Note
> that I applied some other tweaks as well - one is in a separate patch
> on top, the other is that I omitted the Makefile rule for .stub.o
> objects under arch/riscv/Makefile, as it is not actually used.
>
> Atish - please pick whatever seems useful to you from that branch when
> you do the respin.
>
> Thanks,
> Ard.
>
>
>
> [0] git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Maxim Uvarov]

Changes look fine for and should fix booting, while I can test them in
my environment next week.

Thanks,
Maxim.

On Fri, 11 Sep 2020 at 10:56, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 11 Sep 2020 at 05:16, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >
> > On Thu, 10 Sep 2020 07:08:07 PDT (-0700), ardb@kernel.org wrote:
> > > On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >>
> > >> On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
> > >> >
> > >> > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> > >> > >
> > >> > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > >> > > >
> > >> > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > >> > > > > Maxim reports boot failures on platforms that describe reserved memory
> > >> > > > > regions in DT that are disjoint from system DRAM, and which are converted
> > >> > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > >> > > > >
> > >> > > > > As it turns out, the whole notion of discovering the base of DRAM is
> > >> > > > > problematic, and it would be better to simply rely on the EFI memory
> > >> > > > > allocation routines instead, and derive the FDT and initrd allocation
> > >> > > > > limits from the actual placement of the kernel (which is what defines
> > >> > > > > the start of the linear region anyway)
> > >> > > > >
> > >> > > > > Finally, we should be able to get rid of get_dram_base() entirely.
> > >> > > > > However, as RISC-V only just started using it, we will need to address
> > >> > > > > that at a later time.
> > >> > > >
> > >> > > > Looks like we're using dram_base to derive two argumets to
> > >> > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > >> > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > >> > > > x86 uses for the minimum load address, but I don't think we have any mechanism
> > >> > > > like "struct boot_params" so we'd need to come up with something.
> > >> > > >
> > >> > >
> > >> > > As discussed in the other thread
> > >> > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> > >> > > we don't need to do anything special. efi_relocate_kernel can just
> > >> > > take preferred address as 0
> > >> > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> > >> > > allocate 2MB/4MB aligned address as per requirement.
> > >> > >
> > >> > > I don't think the other changes in this series will cause any issue
> > >> > > for RISC-V. I will test it and update anyways.
> > >> > >
> > >> > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > >> > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > >> > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > >> > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > >> > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > >> > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > >> > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > >> > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > >> > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > >> > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > >> > > > > Cc: Grant Likely <Grant.Likely@arm.com>
> > >> > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > >> > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > >> > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > >> > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > >> > > > >
> > >> > > > > Ard Biesheuvel (3):
> > >> > > > >   efi/libstub: Export efi_low_alloc_above() to other units
> > >> > > > >   efi/libstub: Use low allocation for the uncompressed kernel
> > >> > > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> > >> > > > >     base
> > >> > > > >
> > >> > > > >  arch/arm/include/asm/efi.h                |   6 +-
> > >> > > > >  arch/arm64/include/asm/efi.h              |   2 +-
> > >> > > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > >> > > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> > >> > > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> > >> > > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> > >> > > > >  6 files changed, 47 insertions(+), 147 deletions(-)
> > >> > >
> > >> >
> > >> > I verified the above patches along with the following RISC-V specific changes.
> > >> >
> > >> > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> > >> > index 93c305a638f4..dd6ceea9d548 100644
> > >> > --- a/arch/riscv/include/asm/efi.h
> > >> > +++ b/arch/riscv/include/asm/efi.h
> > >> > @@ -37,7 +37,7 @@ static inline unsigned long
> > >> > efi_get_max_fdt_addr(unsigned long dram_base)
> > >> >  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
> > >> >                                                     unsigned long image_addr)
> > >> >  {
> > >> > -       return dram_base + SZ_256M;
> > >> > +       return image_addr + SZ_256M;
> > >> >  }
> > >> >
> > >>
> > >> Ah yes, we need this change as well - this is a bit unfortunate since
> > >> that creates a conflict with the RISC-V tree.
> > >>
> > >> > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> > >> > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> > >> > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> > >> >          */
> > >> >         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
> > >> >         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> > >> > -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
> > >> > +                                    0, MIN_KIMG_ALIGN, 0);
> > >> >
> > >> > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
> > >>
> > >> Thanks for confirming.
> > >
> > > OK,
> > >
> > > So, just to annoy Palmer and you more than I already have up to this
> > > point: any chance we could do a final respin of the RISC-V code on top
> > > of these changes? They are important for ARM, and I would prefer these
> > > to be merged in a way that makes it easy to backport them to -stable
> > > if needed.
> > >
> > > So what I would suggest is:
> > > - I will create a new 'shared-efi' tag/stable branch containing the
> > > existing two patches, as well as these changes (in a slightly updated
> > > form)
> > > - Palmer creates a new topic branch in the riscv repo based on this
> > > shared tag, and applies the [updated] RISC-V patches on top
> > > - Palmer drops the current version of the riscv patches from
> > > riscv/for-next, and merges the topic branch into it instead.
> > >
> > > Again, sorry to be a pain, but I think this is the cleanest way to get
> > > these changes queued up for v5.10 without painting ourselves into a
> > > corner too much when it comes to future follow-up changes.
> >
> > That's fine for me.
>
> Excellent.
>
> I have created a signed tag efi-riscv-shared-for-v5.10 in the EFI repo
> [0], which is based on v5.9-rc1. Please merge that at the start of
> your for-next/efi topic branch, and apply the reworked EFI for RISC-V
> series on top.
>
> I have created a preliminary version of that branch as 'riscv-tmp' on
> [1], incorporating some changes that are needed for the rebase. Note
> that I applied some other tweaks as well - one is in a separate patch
> on top, the other is that I omitted the Makefile rule for .stub.o
> objects under arch/riscv/Makefile, as it is not actually used.
>
> Atish - please pick whatever seems useful to you from that branch when
> you do the respin.
>
> Thanks,
> Ard.
>
>
>
> [0] git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git

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

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Ard Biesheuvel]

On Fri, 11 Sep 2020 at 13:27, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>
> Changes look fine for and should fix booting, while I can test them in
> my environment next week.
>

Thanks

Please use the version below for testing:
https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/tag/?h=efi-riscv-shared-for-v5.10


> On Fri, 11 Sep 2020 at 10:56, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Fri, 11 Sep 2020 at 05:16, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > >
> > > On Thu, 10 Sep 2020 07:08:07 PDT (-0700), ardb@kernel.org wrote:
> > > > On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >>
> > > >> On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
> > > >> >
> > > >> > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> > > >> > >
> > > >> > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > > >> > > >
> > > >> > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > > >> > > > > Maxim reports boot failures on platforms that describe reserved memory
> > > >> > > > > regions in DT that are disjoint from system DRAM, and which are converted
> > > >> > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > > >> > > > >
> > > >> > > > > As it turns out, the whole notion of discovering the base of DRAM is
> > > >> > > > > problematic, and it would be better to simply rely on the EFI memory
> > > >> > > > > allocation routines instead, and derive the FDT and initrd allocation
> > > >> > > > > limits from the actual placement of the kernel (which is what defines
> > > >> > > > > the start of the linear region anyway)
> > > >> > > > >
> > > >> > > > > Finally, we should be able to get rid of get_dram_base() entirely.
> > > >> > > > > However, as RISC-V only just started using it, we will need to address
> > > >> > > > > that at a later time.
> > > >> > > >
> > > >> > > > Looks like we're using dram_base to derive two argumets to
> > > >> > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > > >> > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > > >> > > > x86 uses for the minimum load address, but I don't think we have any mechanism
> > > >> > > > like "struct boot_params" so we'd need to come up with something.
> > > >> > > >
> > > >> > >
> > > >> > > As discussed in the other thread
> > > >> > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> > > >> > > we don't need to do anything special. efi_relocate_kernel can just
> > > >> > > take preferred address as 0
> > > >> > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> > > >> > > allocate 2MB/4MB aligned address as per requirement.
> > > >> > >
> > > >> > > I don't think the other changes in this series will cause any issue
> > > >> > > for RISC-V. I will test it and update anyways.
> > > >> > >
> > > >> > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > > >> > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > >> > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > >> > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > >> > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > > >> > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > > >> > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > > >> > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > > >> > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > > >> > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > >> > > > > Cc: Grant Likely <Grant.Likely@arm.com>
> > > >> > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > >> > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > > >> > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > >> > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > > >> > > > >
> > > >> > > > > Ard Biesheuvel (3):
> > > >> > > > >   efi/libstub: Export efi_low_alloc_above() to other units
> > > >> > > > >   efi/libstub: Use low allocation for the uncompressed kernel
> > > >> > > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> > > >> > > > >     base
> > > >> > > > >
> > > >> > > > >  arch/arm/include/asm/efi.h                |   6 +-
> > > >> > > > >  arch/arm64/include/asm/efi.h              |   2 +-
> > > >> > > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > > >> > > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> > > >> > > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> > > >> > > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> > > >> > > > >  6 files changed, 47 insertions(+), 147 deletions(-)
> > > >> > >
> > > >> >
> > > >> > I verified the above patches along with the following RISC-V specific changes.
> > > >> >
> > > >> > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> > > >> > index 93c305a638f4..dd6ceea9d548 100644
> > > >> > --- a/arch/riscv/include/asm/efi.h
> > > >> > +++ b/arch/riscv/include/asm/efi.h
> > > >> > @@ -37,7 +37,7 @@ static inline unsigned long
> > > >> > efi_get_max_fdt_addr(unsigned long dram_base)
> > > >> >  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
> > > >> >                                                     unsigned long image_addr)
> > > >> >  {
> > > >> > -       return dram_base + SZ_256M;
> > > >> > +       return image_addr + SZ_256M;
> > > >> >  }
> > > >> >
> > > >>
> > > >> Ah yes, we need this change as well - this is a bit unfortunate since
> > > >> that creates a conflict with the RISC-V tree.
> > > >>
> > > >> > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> > > >> > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> > > >> > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> > > >> >          */
> > > >> >         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
> > > >> >         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> > > >> > -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
> > > >> > +                                    0, MIN_KIMG_ALIGN, 0);
> > > >> >
> > > >> > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
> > > >>
> > > >> Thanks for confirming.
> > > >
> > > > OK,
> > > >
> > > > So, just to annoy Palmer and you more than I already have up to this
> > > > point: any chance we could do a final respin of the RISC-V code on top
> > > > of these changes? They are important for ARM, and I would prefer these
> > > > to be merged in a way that makes it easy to backport them to -stable
> > > > if needed.
> > > >
> > > > So what I would suggest is:
> > > > - I will create a new 'shared-efi' tag/stable branch containing the
> > > > existing two patches, as well as these changes (in a slightly updated
> > > > form)
> > > > - Palmer creates a new topic branch in the riscv repo based on this
> > > > shared tag, and applies the [updated] RISC-V patches on top
> > > > - Palmer drops the current version of the riscv patches from
> > > > riscv/for-next, and merges the topic branch into it instead.
> > > >
> > > > Again, sorry to be a pain, but I think this is the cleanest way to get
> > > > these changes queued up for v5.10 without painting ourselves into a
> > > > corner too much when it comes to future follow-up changes.
> > >
> > > That's fine for me.
> >
> > Excellent.
> >
> > I have created a signed tag efi-riscv-shared-for-v5.10 in the EFI repo
> > [0], which is based on v5.9-rc1. Please merge that at the start of
> > your for-next/efi topic branch, and apply the reworked EFI for RISC-V
> > series on top.
> >
> > I have created a preliminary version of that branch as 'riscv-tmp' on
> > [1], incorporating some changes that are needed for the rebase. Note
> > that I applied some other tweaks as well - one is in a separate patch
> > on top, the other is that I omitted the Makefile rule for .stub.o
> > objects under arch/riscv/Makefile, as it is not actually used.
> >
> > Atish - please pick whatever seems useful to you from that branch when
> > you do the respin.
> >
> > Thanks,
> > Ard.
> >
> >
> >
> > [0] git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
> > [1] git://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Ard Biesheuvel]

On Fri, 11 Sep 2020 at 13:27, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>
> Changes look fine for and should fix booting, while I can test them in
> my environment next week.
>

Thanks

Please use the version below for testing:
https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/tag/?h=efi-riscv-shared-for-v5.10


> On Fri, 11 Sep 2020 at 10:56, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Fri, 11 Sep 2020 at 05:16, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > >
> > > On Thu, 10 Sep 2020 07:08:07 PDT (-0700), ardb@kernel.org wrote:
> > > > On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >>
> > > >> On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
> > > >> >
> > > >> > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> > > >> > >
> > > >> > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > > >> > > >
> > > >> > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > > >> > > > > Maxim reports boot failures on platforms that describe reserved memory
> > > >> > > > > regions in DT that are disjoint from system DRAM, and which are converted
> > > >> > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > > >> > > > >
> > > >> > > > > As it turns out, the whole notion of discovering the base of DRAM is
> > > >> > > > > problematic, and it would be better to simply rely on the EFI memory
> > > >> > > > > allocation routines instead, and derive the FDT and initrd allocation
> > > >> > > > > limits from the actual placement of the kernel (which is what defines
> > > >> > > > > the start of the linear region anyway)
> > > >> > > > >
> > > >> > > > > Finally, we should be able to get rid of get_dram_base() entirely.
> > > >> > > > > However, as RISC-V only just started using it, we will need to address
> > > >> > > > > that at a later time.
> > > >> > > >
> > > >> > > > Looks like we're using dram_base to derive two argumets to
> > > >> > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > > >> > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > > >> > > > x86 uses for the minimum load address, but I don't think we have any mechanism
> > > >> > > > like "struct boot_params" so we'd need to come up with something.
> > > >> > > >
> > > >> > >
> > > >> > > As discussed in the other thread
> > > >> > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> > > >> > > we don't need to do anything special. efi_relocate_kernel can just
> > > >> > > take preferred address as 0
> > > >> > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> > > >> > > allocate 2MB/4MB aligned address as per requirement.
> > > >> > >
> > > >> > > I don't think the other changes in this series will cause any issue
> > > >> > > for RISC-V. I will test it and update anyways.
> > > >> > >
> > > >> > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > > >> > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > >> > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > >> > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > >> > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > > >> > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > > >> > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > > >> > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > > >> > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > > >> > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > >> > > > > Cc: Grant Likely <Grant.Likely@arm.com>
> > > >> > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > >> > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > > >> > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > >> > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > > >> > > > >
> > > >> > > > > Ard Biesheuvel (3):
> > > >> > > > >   efi/libstub: Export efi_low_alloc_above() to other units
> > > >> > > > >   efi/libstub: Use low allocation for the uncompressed kernel
> > > >> > > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> > > >> > > > >     base
> > > >> > > > >
> > > >> > > > >  arch/arm/include/asm/efi.h                |   6 +-
> > > >> > > > >  arch/arm64/include/asm/efi.h              |   2 +-
> > > >> > > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > > >> > > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> > > >> > > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> > > >> > > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> > > >> > > > >  6 files changed, 47 insertions(+), 147 deletions(-)
> > > >> > >
> > > >> >
> > > >> > I verified the above patches along with the following RISC-V specific changes.
> > > >> >
> > > >> > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> > > >> > index 93c305a638f4..dd6ceea9d548 100644
> > > >> > --- a/arch/riscv/include/asm/efi.h
> > > >> > +++ b/arch/riscv/include/asm/efi.h
> > > >> > @@ -37,7 +37,7 @@ static inline unsigned long
> > > >> > efi_get_max_fdt_addr(unsigned long dram_base)
> > > >> >  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
> > > >> >                                                     unsigned long image_addr)
> > > >> >  {
> > > >> > -       return dram_base + SZ_256M;
> > > >> > +       return image_addr + SZ_256M;
> > > >> >  }
> > > >> >
> > > >>
> > > >> Ah yes, we need this change as well - this is a bit unfortunate since
> > > >> that creates a conflict with the RISC-V tree.
> > > >>
> > > >> > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> > > >> > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> > > >> > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> > > >> >          */
> > > >> >         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
> > > >> >         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> > > >> > -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
> > > >> > +                                    0, MIN_KIMG_ALIGN, 0);
> > > >> >
> > > >> > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
> > > >>
> > > >> Thanks for confirming.
> > > >
> > > > OK,
> > > >
> > > > So, just to annoy Palmer and you more than I already have up to this
> > > > point: any chance we could do a final respin of the RISC-V code on top
> > > > of these changes? They are important for ARM, and I would prefer these
> > > > to be merged in a way that makes it easy to backport them to -stable
> > > > if needed.
> > > >
> > > > So what I would suggest is:
> > > > - I will create a new 'shared-efi' tag/stable branch containing the
> > > > existing two patches, as well as these changes (in a slightly updated
> > > > form)
> > > > - Palmer creates a new topic branch in the riscv repo based on this
> > > > shared tag, and applies the [updated] RISC-V patches on top
> > > > - Palmer drops the current version of the riscv patches from
> > > > riscv/for-next, and merges the topic branch into it instead.
> > > >
> > > > Again, sorry to be a pain, but I think this is the cleanest way to get
> > > > these changes queued up for v5.10 without painting ourselves into a
> > > > corner too much when it comes to future follow-up changes.
> > >
> > > That's fine for me.
> >
> > Excellent.
> >
> > I have created a signed tag efi-riscv-shared-for-v5.10 in the EFI repo
> > [0], which is based on v5.9-rc1. Please merge that at the start of
> > your for-next/efi topic branch, and apply the reworked EFI for RISC-V
> > series on top.
> >
> > I have created a preliminary version of that branch as 'riscv-tmp' on
> > [1], incorporating some changes that are needed for the rebase. Note
> > that I applied some other tweaks as well - one is in a separate patch
> > on top, the other is that I omitted the Makefile rule for .stub.o
> > objects under arch/riscv/Makefile, as it is not actually used.
> >
> > Atish - please pick whatever seems useful to you from that branch when
> > you do the respin.
> >
> > Thanks,
> > Ard.
> >
> >
> >
> > [0] git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
> > [1] git://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git

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

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Maxim Uvarov]

On Fri, 11 Sep 2020 at 21:45, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 11 Sep 2020 at 13:27, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> >
> > Changes look fine for and should fix booting, while I can test them in
> > my environment next week.
> >
>
> Thanks
>
> Please use the version below for testing:
> https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/tag/?h=efi-riscv-shared-for-v5.10
>
tested this one, qemu 5.0 ( qemu-system-arm -machine virt,secure=on
-cpu cortex-a15 -m 2048 ..)
atf - fc721f83085
optee - d1c635434
uboot - 9f04a634ef
All components are almost the latest versions for the current moment.
Works fine for me.

Reviewed-and-Tested-by: Maxim Uvarov <maxim.uvarov@linaro.org

Maxim.

>
> > On Fri, 11 Sep 2020 at 10:56, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Fri, 11 Sep 2020 at 05:16, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > > >
> > > > On Thu, 10 Sep 2020 07:08:07 PDT (-0700), ardb@kernel.org wrote:
> > > > > On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > >>
> > > > >> On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
> > > > >> >
> > > > >> > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> > > > >> > >
> > > > >> > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > > > >> > > >
> > > > >> > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > > > >> > > > > Maxim reports boot failures on platforms that describe reserved memory
> > > > >> > > > > regions in DT that are disjoint from system DRAM, and which are converted
> > > > >> > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > > > >> > > > >
> > > > >> > > > > As it turns out, the whole notion of discovering the base of DRAM is
> > > > >> > > > > problematic, and it would be better to simply rely on the EFI memory
> > > > >> > > > > allocation routines instead, and derive the FDT and initrd allocation
> > > > >> > > > > limits from the actual placement of the kernel (which is what defines
> > > > >> > > > > the start of the linear region anyway)
> > > > >> > > > >
> > > > >> > > > > Finally, we should be able to get rid of get_dram_base() entirely.
> > > > >> > > > > However, as RISC-V only just started using it, we will need to address
> > > > >> > > > > that at a later time.
> > > > >> > > >
> > > > >> > > > Looks like we're using dram_base to derive two argumets to
> > > > >> > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > > > >> > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > > > >> > > > x86 uses for the minimum load address, but I don't think we have any mechanism
> > > > >> > > > like "struct boot_params" so we'd need to come up with something.
> > > > >> > > >
> > > > >> > >
> > > > >> > > As discussed in the other thread
> > > > >> > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> > > > >> > > we don't need to do anything special. efi_relocate_kernel can just
> > > > >> > > take preferred address as 0
> > > > >> > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> > > > >> > > allocate 2MB/4MB aligned address as per requirement.
> > > > >> > >
> > > > >> > > I don't think the other changes in this series will cause any issue
> > > > >> > > for RISC-V. I will test it and update anyways.
> > > > >> > >
> > > > >> > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > > > >> > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > > >> > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > >> > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > > >> > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > > > >> > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > > > >> > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > > > >> > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > > > >> > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > > > >> > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > > >> > > > > Cc: Grant Likely <Grant.Likely@arm.com>
> > > > >> > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > >> > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > > > >> > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > > >> > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > > > >> > > > >
> > > > >> > > > > Ard Biesheuvel (3):
> > > > >> > > > >   efi/libstub: Export efi_low_alloc_above() to other units
> > > > >> > > > >   efi/libstub: Use low allocation for the uncompressed kernel
> > > > >> > > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> > > > >> > > > >     base
> > > > >> > > > >
> > > > >> > > > >  arch/arm/include/asm/efi.h                |   6 +-
> > > > >> > > > >  arch/arm64/include/asm/efi.h              |   2 +-
> > > > >> > > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > > > >> > > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> > > > >> > > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> > > > >> > > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> > > > >> > > > >  6 files changed, 47 insertions(+), 147 deletions(-)
> > > > >> > >
> > > > >> >
> > > > >> > I verified the above patches along with the following RISC-V specific changes.
> > > > >> >
> > > > >> > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> > > > >> > index 93c305a638f4..dd6ceea9d548 100644
> > > > >> > --- a/arch/riscv/include/asm/efi.h
> > > > >> > +++ b/arch/riscv/include/asm/efi.h
> > > > >> > @@ -37,7 +37,7 @@ static inline unsigned long
> > > > >> > efi_get_max_fdt_addr(unsigned long dram_base)
> > > > >> >  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
> > > > >> >                                                     unsigned long image_addr)
> > > > >> >  {
> > > > >> > -       return dram_base + SZ_256M;
> > > > >> > +       return image_addr + SZ_256M;
> > > > >> >  }
> > > > >> >
> > > > >>
> > > > >> Ah yes, we need this change as well - this is a bit unfortunate since
> > > > >> that creates a conflict with the RISC-V tree.
> > > > >>
> > > > >> > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> > > > >> > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> > > > >> > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> > > > >> >          */
> > > > >> >         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
> > > > >> >         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> > > > >> > -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
> > > > >> > +                                    0, MIN_KIMG_ALIGN, 0);
> > > > >> >
> > > > >> > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
> > > > >>
> > > > >> Thanks for confirming.
> > > > >
> > > > > OK,
> > > > >
> > > > > So, just to annoy Palmer and you more than I already have up to this
> > > > > point: any chance we could do a final respin of the RISC-V code on top
> > > > > of these changes? They are important for ARM, and I would prefer these
> > > > > to be merged in a way that makes it easy to backport them to -stable
> > > > > if needed.
> > > > >
> > > > > So what I would suggest is:
> > > > > - I will create a new 'shared-efi' tag/stable branch containing the
> > > > > existing two patches, as well as these changes (in a slightly updated
> > > > > form)
> > > > > - Palmer creates a new topic branch in the riscv repo based on this
> > > > > shared tag, and applies the [updated] RISC-V patches on top
> > > > > - Palmer drops the current version of the riscv patches from
> > > > > riscv/for-next, and merges the topic branch into it instead.
> > > > >
> > > > > Again, sorry to be a pain, but I think this is the cleanest way to get
> > > > > these changes queued up for v5.10 without painting ourselves into a
> > > > > corner too much when it comes to future follow-up changes.
> > > >
> > > > That's fine for me.
> > >
> > > Excellent.
> > >
> > > I have created a signed tag efi-riscv-shared-for-v5.10 in the EFI repo
> > > [0], which is based on v5.9-rc1. Please merge that at the start of
> > > your for-next/efi topic branch, and apply the reworked EFI for RISC-V
> > > series on top.
> > >
> > > I have created a preliminary version of that branch as 'riscv-tmp' on
> > > [1], incorporating some changes that are needed for the rebase. Note
> > > that I applied some other tweaks as well - one is in a separate patch
> > > on top, the other is that I omitted the Makefile rule for .stub.o
> > > objects under arch/riscv/Makefile, as it is not actually used.
> > >
> > > Atish - please pick whatever seems useful to you from that branch when
> > > you do the respin.
> > >
> > > Thanks,
> > > Ard.
> > >
> > >
> > >
> > > [0] git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
> > > [1] git://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Maxim Uvarov]

On Fri, 11 Sep 2020 at 21:45, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 11 Sep 2020 at 13:27, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> >
> > Changes look fine for and should fix booting, while I can test them in
> > my environment next week.
> >
>
> Thanks
>
> Please use the version below for testing:
> https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/tag/?h=efi-riscv-shared-for-v5.10
>
tested this one, qemu 5.0 ( qemu-system-arm -machine virt,secure=on
-cpu cortex-a15 -m 2048 ..)
atf - fc721f83085
optee - d1c635434
uboot - 9f04a634ef
All components are almost the latest versions for the current moment.
Works fine for me.

Reviewed-and-Tested-by: Maxim Uvarov <maxim.uvarov@linaro.org

Maxim.

>
> > On Fri, 11 Sep 2020 at 10:56, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Fri, 11 Sep 2020 at 05:16, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > > >
> > > > On Thu, 10 Sep 2020 07:08:07 PDT (-0700), ardb@kernel.org wrote:
> > > > > On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > >>
> > > > >> On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
> > > > >> >
> > > > >> > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> > > > >> > >
> > > > >> > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > > > >> > > >
> > > > >> > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > > > >> > > > > Maxim reports boot failures on platforms that describe reserved memory
> > > > >> > > > > regions in DT that are disjoint from system DRAM, and which are converted
> > > > >> > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > > > >> > > > >
> > > > >> > > > > As it turns out, the whole notion of discovering the base of DRAM is
> > > > >> > > > > problematic, and it would be better to simply rely on the EFI memory
> > > > >> > > > > allocation routines instead, and derive the FDT and initrd allocation
> > > > >> > > > > limits from the actual placement of the kernel (which is what defines
> > > > >> > > > > the start of the linear region anyway)
> > > > >> > > > >
> > > > >> > > > > Finally, we should be able to get rid of get_dram_base() entirely.
> > > > >> > > > > However, as RISC-V only just started using it, we will need to address
> > > > >> > > > > that at a later time.
> > > > >> > > >
> > > > >> > > > Looks like we're using dram_base to derive two argumets to
> > > > >> > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > > > >> > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > > > >> > > > x86 uses for the minimum load address, but I don't think we have any mechanism
> > > > >> > > > like "struct boot_params" so we'd need to come up with something.
> > > > >> > > >
> > > > >> > >
> > > > >> > > As discussed in the other thread
> > > > >> > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> > > > >> > > we don't need to do anything special. efi_relocate_kernel can just
> > > > >> > > take preferred address as 0
> > > > >> > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> > > > >> > > allocate 2MB/4MB aligned address as per requirement.
> > > > >> > >
> > > > >> > > I don't think the other changes in this series will cause any issue
> > > > >> > > for RISC-V. I will test it and update anyways.
> > > > >> > >
> > > > >> > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > > > >> > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > > >> > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > >> > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > > >> > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > > > >> > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > > > >> > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > > > >> > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > > > >> > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > > > >> > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > > >> > > > > Cc: Grant Likely <Grant.Likely@arm.com>
> > > > >> > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > >> > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > > > >> > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > > >> > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > > > >> > > > >
> > > > >> > > > > Ard Biesheuvel (3):
> > > > >> > > > >   efi/libstub: Export efi_low_alloc_above() to other units
> > > > >> > > > >   efi/libstub: Use low allocation for the uncompressed kernel
> > > > >> > > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> > > > >> > > > >     base
> > > > >> > > > >
> > > > >> > > > >  arch/arm/include/asm/efi.h                |   6 +-
> > > > >> > > > >  arch/arm64/include/asm/efi.h              |   2 +-
> > > > >> > > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > > > >> > > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> > > > >> > > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> > > > >> > > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> > > > >> > > > >  6 files changed, 47 insertions(+), 147 deletions(-)
> > > > >> > >
> > > > >> >
> > > > >> > I verified the above patches along with the following RISC-V specific changes.
> > > > >> >
> > > > >> > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> > > > >> > index 93c305a638f4..dd6ceea9d548 100644
> > > > >> > --- a/arch/riscv/include/asm/efi.h
> > > > >> > +++ b/arch/riscv/include/asm/efi.h
> > > > >> > @@ -37,7 +37,7 @@ static inline unsigned long
> > > > >> > efi_get_max_fdt_addr(unsigned long dram_base)
> > > > >> >  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
> > > > >> >                                                     unsigned long image_addr)
> > > > >> >  {
> > > > >> > -       return dram_base + SZ_256M;
> > > > >> > +       return image_addr + SZ_256M;
> > > > >> >  }
> > > > >> >
> > > > >>
> > > > >> Ah yes, we need this change as well - this is a bit unfortunate since
> > > > >> that creates a conflict with the RISC-V tree.
> > > > >>
> > > > >> > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> > > > >> > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> > > > >> > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> > > > >> >          */
> > > > >> >         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
> > > > >> >         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> > > > >> > -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
> > > > >> > +                                    0, MIN_KIMG_ALIGN, 0);
> > > > >> >
> > > > >> > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
> > > > >>
> > > > >> Thanks for confirming.
> > > > >
> > > > > OK,
> > > > >
> > > > > So, just to annoy Palmer and you more than I already have up to this
> > > > > point: any chance we could do a final respin of the RISC-V code on top
> > > > > of these changes? They are important for ARM, and I would prefer these
> > > > > to be merged in a way that makes it easy to backport them to -stable
> > > > > if needed.
> > > > >
> > > > > So what I would suggest is:
> > > > > - I will create a new 'shared-efi' tag/stable branch containing the
> > > > > existing two patches, as well as these changes (in a slightly updated
> > > > > form)
> > > > > - Palmer creates a new topic branch in the riscv repo based on this
> > > > > shared tag, and applies the [updated] RISC-V patches on top
> > > > > - Palmer drops the current version of the riscv patches from
> > > > > riscv/for-next, and merges the topic branch into it instead.
> > > > >
> > > > > Again, sorry to be a pain, but I think this is the cleanest way to get
> > > > > these changes queued up for v5.10 without painting ourselves into a
> > > > > corner too much when it comes to future follow-up changes.
> > > >
> > > > That's fine for me.
> > >
> > > Excellent.
> > >
> > > I have created a signed tag efi-riscv-shared-for-v5.10 in the EFI repo
> > > [0], which is based on v5.9-rc1. Please merge that at the start of
> > > your for-next/efi topic branch, and apply the reworked EFI for RISC-V
> > > series on top.
> > >
> > > I have created a preliminary version of that branch as 'riscv-tmp' on
> > > [1], incorporating some changes that are needed for the rebase. Note
> > > that I applied some other tweaks as well - one is in a separate patch
> > > on top, the other is that I omitted the Makefile rule for .stub.o
> > > objects under arch/riscv/Makefile, as it is not actually used.
> > >
> > > Atish - please pick whatever seems useful to you from that branch when
> > > you do the respin.
> > >
> > > Thanks,
> > > Ard.
> > >
> > >
> > >
> > > [0] git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
> > > [1] git://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git

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

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Ard Biesheuvel]

On Wed, 16 Sep 2020 at 18:43, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>
> On Fri, 11 Sep 2020 at 21:45, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Fri, 11 Sep 2020 at 13:27, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > >
> > > Changes look fine for and should fix booting, while I can test them in
> > > my environment next week.
> > >
> >
> > Thanks
> >
> > Please use the version below for testing:
> > https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/tag/?h=efi-riscv-shared-for-v5.10
> >
> tested this one, qemu 5.0 ( qemu-system-arm -machine virt,secure=on
> -cpu cortex-a15 -m 2048 ..)
> atf - fc721f83085
> optee - d1c635434
> uboot - 9f04a634ef
> All components are almost the latest versions for the current moment.
> Works fine for me.
>
> Reviewed-and-Tested-by: Maxim Uvarov <maxim.uvarov@linaro.org
>

Thanks!


> Maxim.
>

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Ard Biesheuvel]

On Wed, 16 Sep 2020 at 18:43, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>
> On Fri, 11 Sep 2020 at 21:45, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Fri, 11 Sep 2020 at 13:27, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > >
> > > Changes look fine for and should fix booting, while I can test them in
> > > my environment next week.
> > >
> >
> > Thanks
> >
> > Please use the version below for testing:
> > https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/tag/?h=efi-riscv-shared-for-v5.10
> >
> tested this one, qemu 5.0 ( qemu-system-arm -machine virt,secure=on
> -cpu cortex-a15 -m 2048 ..)
> atf - fc721f83085
> optee - d1c635434
> uboot - 9f04a634ef
> All components are almost the latest versions for the current moment.
> Works fine for me.
>
> Reviewed-and-Tested-by: Maxim Uvarov <maxim.uvarov@linaro.org
>

Thanks!


> Maxim.
>

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

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Ard Biesheuvel]

On Fri, 11 Sep 2020 at 10:56, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 11 Sep 2020 at 05:16, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >
> > On Thu, 10 Sep 2020 07:08:07 PDT (-0700), ardb@kernel.org wrote:
> > > On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >>
> > >> On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
> > >> >
> > >> > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> > >> > >
> > >> > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > >> > > >
> > >> > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > >> > > > > Maxim reports boot failures on platforms that describe reserved memory
> > >> > > > > regions in DT that are disjoint from system DRAM, and which are converted
> > >> > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > >> > > > >
> > >> > > > > As it turns out, the whole notion of discovering the base of DRAM is
> > >> > > > > problematic, and it would be better to simply rely on the EFI memory
> > >> > > > > allocation routines instead, and derive the FDT and initrd allocation
> > >> > > > > limits from the actual placement of the kernel (which is what defines
> > >> > > > > the start of the linear region anyway)
> > >> > > > >
> > >> > > > > Finally, we should be able to get rid of get_dram_base() entirely.
> > >> > > > > However, as RISC-V only just started using it, we will need to address
> > >> > > > > that at a later time.
> > >> > > >
> > >> > > > Looks like we're using dram_base to derive two argumets to
> > >> > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > >> > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > >> > > > x86 uses for the minimum load address, but I don't think we have any mechanism
> > >> > > > like "struct boot_params" so we'd need to come up with something.
> > >> > > >
> > >> > >
> > >> > > As discussed in the other thread
> > >> > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> > >> > > we don't need to do anything special. efi_relocate_kernel can just
> > >> > > take preferred address as 0
> > >> > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> > >> > > allocate 2MB/4MB aligned address as per requirement.
> > >> > >
> > >> > > I don't think the other changes in this series will cause any issue
> > >> > > for RISC-V. I will test it and update anyways.
> > >> > >
> > >> > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > >> > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > >> > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > >> > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > >> > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > >> > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > >> > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > >> > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > >> > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > >> > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > >> > > > > Cc: Grant Likely <Grant.Likely@arm.com>
> > >> > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > >> > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > >> > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > >> > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > >> > > > >
> > >> > > > > Ard Biesheuvel (3):
> > >> > > > >   efi/libstub: Export efi_low_alloc_above() to other units
> > >> > > > >   efi/libstub: Use low allocation for the uncompressed kernel
> > >> > > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> > >> > > > >     base
> > >> > > > >
> > >> > > > >  arch/arm/include/asm/efi.h                |   6 +-
> > >> > > > >  arch/arm64/include/asm/efi.h              |   2 +-
> > >> > > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > >> > > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> > >> > > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> > >> > > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> > >> > > > >  6 files changed, 47 insertions(+), 147 deletions(-)
> > >> > >
> > >> >
> > >> > I verified the above patches along with the following RISC-V specific changes.
> > >> >
> > >> > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> > >> > index 93c305a638f4..dd6ceea9d548 100644
> > >> > --- a/arch/riscv/include/asm/efi.h
> > >> > +++ b/arch/riscv/include/asm/efi.h
> > >> > @@ -37,7 +37,7 @@ static inline unsigned long
> > >> > efi_get_max_fdt_addr(unsigned long dram_base)
> > >> >  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
> > >> >                                                     unsigned long image_addr)
> > >> >  {
> > >> > -       return dram_base + SZ_256M;
> > >> > +       return image_addr + SZ_256M;
> > >> >  }
> > >> >
> > >>
> > >> Ah yes, we need this change as well - this is a bit unfortunate since
> > >> that creates a conflict with the RISC-V tree.
> > >>
> > >> > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> > >> > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> > >> > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> > >> >          */
> > >> >         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
> > >> >         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> > >> > -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
> > >> > +                                    0, MIN_KIMG_ALIGN, 0);
> > >> >
> > >> > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
> > >>
> > >> Thanks for confirming.
> > >
> > > OK,
> > >
> > > So, just to annoy Palmer and you more than I already have up to this
> > > point: any chance we could do a final respin of the RISC-V code on top
> > > of these changes? They are important for ARM, and I would prefer these
> > > to be merged in a way that makes it easy to backport them to -stable
> > > if needed.
> > >
> > > So what I would suggest is:
> > > - I will create a new 'shared-efi' tag/stable branch containing the
> > > existing two patches, as well as these changes (in a slightly updated
> > > form)
> > > - Palmer creates a new topic branch in the riscv repo based on this
> > > shared tag, and applies the [updated] RISC-V patches on top
> > > - Palmer drops the current version of the riscv patches from
> > > riscv/for-next, and merges the topic branch into it instead.
> > >
> > > Again, sorry to be a pain, but I think this is the cleanest way to get
> > > these changes queued up for v5.10 without painting ourselves into a
> > > corner too much when it comes to future follow-up changes.
> >
> > That's fine for me.
>
> Excellent.
>
> I have created a signed tag efi-riscv-shared-for-v5.10 in the EFI repo
> [0], which is based on v5.9-rc1. Please merge that at the start of
> your for-next/efi topic branch, and apply the reworked EFI for RISC-V
> series on top.
>

Since this tag has not appeared in the riscv repo yet, and Atish's
rebase was not sent to the list yet either, I have pushed a new
version of this tag just now.

If you want, I can send you a formal PR separately.


> I have created a preliminary version of that branch as 'riscv-tmp' on
> [1], incorporating some changes that are needed for the rebase. Note
> that I applied some other tweaks as well - one is in a separate patch
> on top, the other is that I omitted the Makefile rule for .stub.o
> objects under arch/riscv/Makefile, as it is not actually used.
>
> Atish - please pick whatever seems useful to you from that branch when
> you do the respin.
>
> Thanks,
> Ard.
>
>
>
> [0] git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Ard Biesheuvel]

On Fri, 11 Sep 2020 at 10:56, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 11 Sep 2020 at 05:16, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >
> > On Thu, 10 Sep 2020 07:08:07 PDT (-0700), ardb@kernel.org wrote:
> > > On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >>
> > >> On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
> > >> >
> > >> > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> > >> > >
> > >> > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > >> > > >
> > >> > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > >> > > > > Maxim reports boot failures on platforms that describe reserved memory
> > >> > > > > regions in DT that are disjoint from system DRAM, and which are converted
> > >> > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > >> > > > >
> > >> > > > > As it turns out, the whole notion of discovering the base of DRAM is
> > >> > > > > problematic, and it would be better to simply rely on the EFI memory
> > >> > > > > allocation routines instead, and derive the FDT and initrd allocation
> > >> > > > > limits from the actual placement of the kernel (which is what defines
> > >> > > > > the start of the linear region anyway)
> > >> > > > >
> > >> > > > > Finally, we should be able to get rid of get_dram_base() entirely.
> > >> > > > > However, as RISC-V only just started using it, we will need to address
> > >> > > > > that at a later time.
> > >> > > >
> > >> > > > Looks like we're using dram_base to derive two argumets to
> > >> > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > >> > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > >> > > > x86 uses for the minimum load address, but I don't think we have any mechanism
> > >> > > > like "struct boot_params" so we'd need to come up with something.
> > >> > > >
> > >> > >
> > >> > > As discussed in the other thread
> > >> > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> > >> > > we don't need to do anything special. efi_relocate_kernel can just
> > >> > > take preferred address as 0
> > >> > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> > >> > > allocate 2MB/4MB aligned address as per requirement.
> > >> > >
> > >> > > I don't think the other changes in this series will cause any issue
> > >> > > for RISC-V. I will test it and update anyways.
> > >> > >
> > >> > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > >> > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > >> > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > >> > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > >> > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > >> > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > >> > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > >> > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > >> > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > >> > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > >> > > > > Cc: Grant Likely <Grant.Likely@arm.com>
> > >> > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > >> > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > >> > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > >> > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > >> > > > >
> > >> > > > > Ard Biesheuvel (3):
> > >> > > > >   efi/libstub: Export efi_low_alloc_above() to other units
> > >> > > > >   efi/libstub: Use low allocation for the uncompressed kernel
> > >> > > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> > >> > > > >     base
> > >> > > > >
> > >> > > > >  arch/arm/include/asm/efi.h                |   6 +-
> > >> > > > >  arch/arm64/include/asm/efi.h              |   2 +-
> > >> > > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > >> > > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> > >> > > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> > >> > > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> > >> > > > >  6 files changed, 47 insertions(+), 147 deletions(-)
> > >> > >
> > >> >
> > >> > I verified the above patches along with the following RISC-V specific changes.
> > >> >
> > >> > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> > >> > index 93c305a638f4..dd6ceea9d548 100644
> > >> > --- a/arch/riscv/include/asm/efi.h
> > >> > +++ b/arch/riscv/include/asm/efi.h
> > >> > @@ -37,7 +37,7 @@ static inline unsigned long
> > >> > efi_get_max_fdt_addr(unsigned long dram_base)
> > >> >  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
> > >> >                                                     unsigned long image_addr)
> > >> >  {
> > >> > -       return dram_base + SZ_256M;
> > >> > +       return image_addr + SZ_256M;
> > >> >  }
> > >> >
> > >>
> > >> Ah yes, we need this change as well - this is a bit unfortunate since
> > >> that creates a conflict with the RISC-V tree.
> > >>
> > >> > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> > >> > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> > >> > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> > >> >          */
> > >> >         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
> > >> >         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> > >> > -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
> > >> > +                                    0, MIN_KIMG_ALIGN, 0);
> > >> >
> > >> > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
> > >>
> > >> Thanks for confirming.
> > >
> > > OK,
> > >
> > > So, just to annoy Palmer and you more than I already have up to this
> > > point: any chance we could do a final respin of the RISC-V code on top
> > > of these changes? They are important for ARM, and I would prefer these
> > > to be merged in a way that makes it easy to backport them to -stable
> > > if needed.
> > >
> > > So what I would suggest is:
> > > - I will create a new 'shared-efi' tag/stable branch containing the
> > > existing two patches, as well as these changes (in a slightly updated
> > > form)
> > > - Palmer creates a new topic branch in the riscv repo based on this
> > > shared tag, and applies the [updated] RISC-V patches on top
> > > - Palmer drops the current version of the riscv patches from
> > > riscv/for-next, and merges the topic branch into it instead.
> > >
> > > Again, sorry to be a pain, but I think this is the cleanest way to get
> > > these changes queued up for v5.10 without painting ourselves into a
> > > corner too much when it comes to future follow-up changes.
> >
> > That's fine for me.
>
> Excellent.
>
> I have created a signed tag efi-riscv-shared-for-v5.10 in the EFI repo
> [0], which is based on v5.9-rc1. Please merge that at the start of
> your for-next/efi topic branch, and apply the reworked EFI for RISC-V
> series on top.
>

Since this tag has not appeared in the riscv repo yet, and Atish's
rebase was not sent to the list yet either, I have pushed a new
version of this tag just now.

If you want, I can send you a formal PR separately.


> I have created a preliminary version of that branch as 'riscv-tmp' on
> [1], incorporating some changes that are needed for the rebase. Note
> that I applied some other tweaks as well - one is in a separate patch
> on top, the other is that I omitted the Makefile rule for .stub.o
> objects under arch/riscv/Makefile, as it is not actually used.
>
> Atish - please pick whatever seems useful to you from that branch when
> you do the respin.
>
> Thanks,
> Ard.
>
>
>
> [0] git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git

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

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Atish Patra]

On Wed, Sep 16, 2020 at 9:02 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 11 Sep 2020 at 10:56, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Fri, 11 Sep 2020 at 05:16, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > >
> > > On Thu, 10 Sep 2020 07:08:07 PDT (-0700), ardb@kernel.org wrote:
> > > > On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >>
> > > >> On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
> > > >> >
> > > >> > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> > > >> > >
> > > >> > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > > >> > > >
> > > >> > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > > >> > > > > Maxim reports boot failures on platforms that describe reserved memory
> > > >> > > > > regions in DT that are disjoint from system DRAM, and which are converted
> > > >> > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > > >> > > > >
> > > >> > > > > As it turns out, the whole notion of discovering the base of DRAM is
> > > >> > > > > problematic, and it would be better to simply rely on the EFI memory
> > > >> > > > > allocation routines instead, and derive the FDT and initrd allocation
> > > >> > > > > limits from the actual placement of the kernel (which is what defines
> > > >> > > > > the start of the linear region anyway)
> > > >> > > > >
> > > >> > > > > Finally, we should be able to get rid of get_dram_base() entirely.
> > > >> > > > > However, as RISC-V only just started using it, we will need to address
> > > >> > > > > that at a later time.
> > > >> > > >
> > > >> > > > Looks like we're using dram_base to derive two argumets to
> > > >> > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > > >> > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > > >> > > > x86 uses for the minimum load address, but I don't think we have any mechanism
> > > >> > > > like "struct boot_params" so we'd need to come up with something.
> > > >> > > >
> > > >> > >
> > > >> > > As discussed in the other thread
> > > >> > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> > > >> > > we don't need to do anything special. efi_relocate_kernel can just
> > > >> > > take preferred address as 0
> > > >> > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> > > >> > > allocate 2MB/4MB aligned address as per requirement.
> > > >> > >
> > > >> > > I don't think the other changes in this series will cause any issue
> > > >> > > for RISC-V. I will test it and update anyways.
> > > >> > >
> > > >> > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > > >> > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > >> > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > >> > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > >> > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > > >> > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > > >> > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > > >> > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > > >> > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > > >> > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > >> > > > > Cc: Grant Likely <Grant.Likely@arm.com>
> > > >> > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > >> > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > > >> > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > >> > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > > >> > > > >
> > > >> > > > > Ard Biesheuvel (3):
> > > >> > > > >   efi/libstub: Export efi_low_alloc_above() to other units
> > > >> > > > >   efi/libstub: Use low allocation for the uncompressed kernel
> > > >> > > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> > > >> > > > >     base
> > > >> > > > >
> > > >> > > > >  arch/arm/include/asm/efi.h                |   6 +-
> > > >> > > > >  arch/arm64/include/asm/efi.h              |   2 +-
> > > >> > > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > > >> > > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> > > >> > > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> > > >> > > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> > > >> > > > >  6 files changed, 47 insertions(+), 147 deletions(-)
> > > >> > >
> > > >> >
> > > >> > I verified the above patches along with the following RISC-V specific changes.
> > > >> >
> > > >> > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> > > >> > index 93c305a638f4..dd6ceea9d548 100644
> > > >> > --- a/arch/riscv/include/asm/efi.h
> > > >> > +++ b/arch/riscv/include/asm/efi.h
> > > >> > @@ -37,7 +37,7 @@ static inline unsigned long
> > > >> > efi_get_max_fdt_addr(unsigned long dram_base)
> > > >> >  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
> > > >> >                                                     unsigned long image_addr)
> > > >> >  {
> > > >> > -       return dram_base + SZ_256M;
> > > >> > +       return image_addr + SZ_256M;
> > > >> >  }
> > > >> >
> > > >>
> > > >> Ah yes, we need this change as well - this is a bit unfortunate since
> > > >> that creates a conflict with the RISC-V tree.
> > > >>
> > > >> > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> > > >> > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> > > >> > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> > > >> >          */
> > > >> >         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
> > > >> >         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> > > >> > -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
> > > >> > +                                    0, MIN_KIMG_ALIGN, 0);
> > > >> >
> > > >> > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
> > > >>
> > > >> Thanks for confirming.
> > > >
> > > > OK,
> > > >
> > > > So, just to annoy Palmer and you more than I already have up to this
> > > > point: any chance we could do a final respin of the RISC-V code on top
> > > > of these changes? They are important for ARM, and I would prefer these
> > > > to be merged in a way that makes it easy to backport them to -stable
> > > > if needed.
> > > >
> > > > So what I would suggest is:
> > > > - I will create a new 'shared-efi' tag/stable branch containing the
> > > > existing two patches, as well as these changes (in a slightly updated
> > > > form)
> > > > - Palmer creates a new topic branch in the riscv repo based on this
> > > > shared tag, and applies the [updated] RISC-V patches on top
> > > > - Palmer drops the current version of the riscv patches from
> > > > riscv/for-next, and merges the topic branch into it instead.
> > > >
> > > > Again, sorry to be a pain, but I think this is the cleanest way to get
> > > > these changes queued up for v5.10 without painting ourselves into a
> > > > corner too much when it comes to future follow-up changes.
> > >
> > > That's fine for me.
> >
> > Excellent.
> >
> > I have created a signed tag efi-riscv-shared-for-v5.10 in the EFI repo
> > [0], which is based on v5.9-rc1. Please merge that at the start of
> > your for-next/efi topic branch, and apply the reworked EFI for RISC-V
> > series on top.
> >
>
> Since this tag has not appeared in the riscv repo yet, and Atish's
> rebase was not sent to the list yet either, I have pushed a new
> version of this tag just now.
>
Thanks. I rebased my series on top of it and addressed all the concerns.
The patches are hosted in github[1]. I am just waiting for 0-day bot results
before posting the series to the mailing list to avoid any more churns.

[1] https://github.com/atishp04/linux/pull/new/uefi_riscv_5.10_v8

> If you want, I can send you a formal PR separately.
>
>
> > I have created a preliminary version of that branch as 'riscv-tmp' on
> > [1], incorporating some changes that are needed for the rebase. Note
> > that I applied some other tweaks as well - one is in a separate patch
> > on top, the other is that I omitted the Makefile rule for .stub.o
> > objects under arch/riscv/Makefile, as it is not actually used.
> >
> > Atish - please pick whatever seems useful to you from that branch when
> > you do the respin.
> >
> > Thanks,
> > Ard.
> >
> >
> >
> > [0] git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
> > [1] git://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git



-- 
Regards,
Atish

Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base

[Atish Patra]

On Wed, Sep 16, 2020 at 9:02 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 11 Sep 2020 at 10:56, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Fri, 11 Sep 2020 at 05:16, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > >
> > > On Thu, 10 Sep 2020 07:08:07 PDT (-0700), ardb@kernel.org wrote:
> > > > On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >>
> > > >> On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
> > > >> >
> > > >> > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> > > >> > >
> > > >> > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > > >> > > >
> > > >> > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > > >> > > > > Maxim reports boot failures on platforms that describe reserved memory
> > > >> > > > > regions in DT that are disjoint from system DRAM, and which are converted
> > > >> > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > > >> > > > >
> > > >> > > > > As it turns out, the whole notion of discovering the base of DRAM is
> > > >> > > > > problematic, and it would be better to simply rely on the EFI memory
> > > >> > > > > allocation routines instead, and derive the FDT and initrd allocation
> > > >> > > > > limits from the actual placement of the kernel (which is what defines
> > > >> > > > > the start of the linear region anyway)
> > > >> > > > >
> > > >> > > > > Finally, we should be able to get rid of get_dram_base() entirely.
> > > >> > > > > However, as RISC-V only just started using it, we will need to address
> > > >> > > > > that at a later time.
> > > >> > > >
> > > >> > > > Looks like we're using dram_base to derive two argumets to
> > > >> > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > > >> > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > > >> > > > x86 uses for the minimum load address, but I don't think we have any mechanism
> > > >> > > > like "struct boot_params" so we'd need to come up with something.
> > > >> > > >
> > > >> > >
> > > >> > > As discussed in the other thread
> > > >> > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> > > >> > > we don't need to do anything special. efi_relocate_kernel can just
> > > >> > > take preferred address as 0
> > > >> > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> > > >> > > allocate 2MB/4MB aligned address as per requirement.
> > > >> > >
> > > >> > > I don't think the other changes in this series will cause any issue
> > > >> > > for RISC-V. I will test it and update anyways.
> > > >> > >
> > > >> > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > > >> > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > >> > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > >> > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > >> > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > > >> > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > > >> > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > > >> > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > > >> > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > > >> > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > >> > > > > Cc: Grant Likely <Grant.Likely@arm.com>
> > > >> > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > >> > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > > >> > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > >> > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > > >> > > > >
> > > >> > > > > Ard Biesheuvel (3):
> > > >> > > > >   efi/libstub: Export efi_low_alloc_above() to other units
> > > >> > > > >   efi/libstub: Use low allocation for the uncompressed kernel
> > > >> > > > >   efi/libstub: base FDT and initrd placement on image address not DRAM
> > > >> > > > >     base
> > > >> > > > >
> > > >> > > > >  arch/arm/include/asm/efi.h                |   6 +-
> > > >> > > > >  arch/arm64/include/asm/efi.h              |   2 +-
> > > >> > > > >  drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > > >> > > > >  drivers/firmware/efi/libstub/efi-stub.c   |   2 +-
> > > >> > > > >  drivers/firmware/efi/libstub/efistub.h    |   3 +
> > > >> > > > >  drivers/firmware/efi/libstub/relocate.c   |   4 +-
> > > >> > > > >  6 files changed, 47 insertions(+), 147 deletions(-)
> > > >> > >
> > > >> >
> > > >> > I verified the above patches along with the following RISC-V specific changes.
> > > >> >
> > > >> > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> > > >> > index 93c305a638f4..dd6ceea9d548 100644
> > > >> > --- a/arch/riscv/include/asm/efi.h
> > > >> > +++ b/arch/riscv/include/asm/efi.h
> > > >> > @@ -37,7 +37,7 @@ static inline unsigned long
> > > >> > efi_get_max_fdt_addr(unsigned long dram_base)
> > > >> >  static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
> > > >> >                                                     unsigned long image_addr)
> > > >> >  {
> > > >> > -       return dram_base + SZ_256M;
> > > >> > +       return image_addr + SZ_256M;
> > > >> >  }
> > > >> >
> > > >>
> > > >> Ah yes, we need this change as well - this is a bit unfortunate since
> > > >> that creates a conflict with the RISC-V tree.
> > > >>
> > > >> > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> > > >> > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> > > >> > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> > > >> >          */
> > > >> >         preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
> > > >> >         status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> > > >> > -                                    preferred_addr, MIN_KIMG_ALIGN, dram_base);
> > > >> > +                                    0, MIN_KIMG_ALIGN, 0);
> > > >> >
> > > >> > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
> > > >>
> > > >> Thanks for confirming.
> > > >
> > > > OK,
> > > >
> > > > So, just to annoy Palmer and you more than I already have up to this
> > > > point: any chance we could do a final respin of the RISC-V code on top
> > > > of these changes? They are important for ARM, and I would prefer these
> > > > to be merged in a way that makes it easy to backport them to -stable
> > > > if needed.
> > > >
> > > > So what I would suggest is:
> > > > - I will create a new 'shared-efi' tag/stable branch containing the
> > > > existing two patches, as well as these changes (in a slightly updated
> > > > form)
> > > > - Palmer creates a new topic branch in the riscv repo based on this
> > > > shared tag, and applies the [updated] RISC-V patches on top
> > > > - Palmer drops the current version of the riscv patches from
> > > > riscv/for-next, and merges the topic branch into it instead.
> > > >
> > > > Again, sorry to be a pain, but I think this is the cleanest way to get
> > > > these changes queued up for v5.10 without painting ourselves into a
> > > > corner too much when it comes to future follow-up changes.
> > >
> > > That's fine for me.
> >
> > Excellent.
> >
> > I have created a signed tag efi-riscv-shared-for-v5.10 in the EFI repo
> > [0], which is based on v5.9-rc1. Please merge that at the start of
> > your for-next/efi topic branch, and apply the reworked EFI for RISC-V
> > series on top.
> >
>
> Since this tag has not appeared in the riscv repo yet, and Atish's
> rebase was not sent to the list yet either, I have pushed a new
> version of this tag just now.
>
Thanks. I rebased my series on top of it and addressed all the concerns.
The patches are hosted in github[1]. I am just waiting for 0-day bot results
before posting the series to the mailing list to avoid any more churns.

[1] https://github.com/atishp04/linux/pull/new/uefi_riscv_5.10_v8

> If you want, I can send you a formal PR separately.
>
>
> > I have created a preliminary version of that branch as 'riscv-tmp' on
> > [1], incorporating some changes that are needed for the rebase. Note
> > that I applied some other tweaks as well - one is in a separate patch
> > on top, the other is that I omitted the Makefile rule for .stub.o
> > objects under arch/riscv/Makefile, as it is not actually used.
> >
> > Atish - please pick whatever seems useful to you from that branch when
> > you do the respin.
> >
> > Thanks,
> > Ard.
> >
> >
> >
> > [0] git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
> > [1] git://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git



-- 
Regards,
Atish

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