efi/libstub: arm32: Remove dependency on dram_base

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 <[email protected]>

Cc Heinrich Schuchardt <[email protected]>

Cc Atish Patra <[email protected]>

Cc Palmer Dabbelt <[email protected]>

Cc Jens Wiklander <[email protected]>

Cc Francois Ozog <[email protected]>

Cc Etienne CARRIERE <[email protected]>

Cc Takahiro Akashi <[email protected]>

Cc Patrice CHOTARD <[email protected]>

Cc Sumit Garg <[email protected]>

Cc Grant Likely <[email protected]>

Cc Ilias Apalodimas <[email protected]>

Cc Christophe Priouzeau <[email protected]>

Cc Rouven Czerwinski <[email protected]>

Cc Patrick DELAUNAY <[email protected]>

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(-)

Quoted 10 lines · 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)

Quoted 3 lines

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?

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 <[email protected]>

Cc Heinrich Schuchardt <[email protected]>

Cc Atish Patra <[email protected]>

Cc Palmer Dabbelt <[email protected]>

Cc Jens Wiklander <[email protected]>

Cc Francois Ozog <[email protected]>

Cc Etienne CARRIERE <[email protected]>

Cc Takahiro Akashi <[email protected]>

Cc Patrice CHOTARD <[email protected]>

Cc Sumit Garg <[email protected]>

Cc Grant Likely <[email protected]>

Cc Ilias Apalodimas <[email protected]>

Cc Christophe Priouzeau <[email protected]>

Cc Rouven Czerwinski <[email protected]>

Cc Patrick DELAUNAY <[email protected]>

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(-)

Quoted 10 lines · On Wed, 09 Sep 2020 08:16:20 PDT (-0700), [email protected] 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)

Quoted 3 lines

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.

Quoted 29 lines

Cc Maxim Uvarov <[email protected]>

Cc Heinrich Schuchardt <[email protected]>

Cc Atish Patra <[email protected]>

Cc Palmer Dabbelt <[email protected]>

Cc Jens Wiklander <[email protected]>

Cc Francois Ozog <[email protected]>

Cc Etienne CARRIERE <[email protected]>

Cc Takahiro Akashi <[email protected]>

Cc Patrice CHOTARD <[email protected]>

Cc Sumit Garg <[email protected]>

Cc Grant Likely <[email protected]>

Cc Ilias Apalodimas <[email protected]>

Cc Christophe Priouzeau <[email protected]>

Cc Rouven Czerwinski <[email protected]>

Cc Patrick DELAUNAY <[email protected]>

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(-)

Quoted 16 lines · Palmer Dabbelt

Quoted 10 lines · On Wed, 09 Sep 2020 08:16:20 PDT (-0700), [email protected] 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)

Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 29 lines

Quoted 29 lines

Cc Maxim Uvarov <[email protected]>

Cc Heinrich Schuchardt <[email protected]>

Cc Atish Patra <[email protected]>

Cc Palmer Dabbelt <[email protected]>

Cc Jens Wiklander <[email protected]>

Cc Francois Ozog <[email protected]>

Cc Etienne CARRIERE <[email protected]>

Cc Takahiro Akashi <[email protected]>

Cc Patrice CHOTARD <[email protected]>

Cc Sumit Garg <[email protected]>

Cc Grant Likely <[email protected]>

Cc Ilias Apalodimas <[email protected]>

Cc Christophe Priouzeau <[email protected]>

Cc Rouven Czerwinski <[email protected]>

Cc Patrick DELAUNAY <[email protected]>

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

Quoted 32 lines · Atish Patra

Quoted 16 lines · Palmer Dabbelt

Quoted 10 lines · On Wed, 09 Sep 2020 08:16:20 PDT (-0700), [email protected] 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)

Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 33 lines

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

Quoted 29 lines

Quoted 29 lines

Cc Maxim Uvarov <[email protected]>

Cc Heinrich Schuchardt <[email protected]>

Cc Atish Patra <[email protected]>

Cc Palmer Dabbelt <[email protected]>

Cc Jens Wiklander <[email protected]>

Cc Francois Ozog <[email protected]>

Cc Etienne CARRIERE <[email protected]>

Cc Takahiro Akashi <[email protected]>

Cc Patrice CHOTARD <[email protected]>

Cc Sumit Garg <[email protected]>

Cc Grant Likely <[email protected]>

Cc Ilias Apalodimas <[email protected]>

Cc Christophe Priouzeau <[email protected]>

Cc Rouven Czerwinski <[email protected]>

Cc Patrick DELAUNAY <[email protected]>

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.

arch/riscv/include/asm/efi.h +0 -0

index 93c305a638f4..dd6ceea9d548 100644

drivers/firmware/efi/libstub/riscv-stub.c +1 -1

@@ -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 <[email protected]>

Quoted 75 lines · Atish Patra

Quoted 32 lines · Atish Patra

Quoted 16 lines · Palmer Dabbelt

Quoted 10 lines · On Wed, 09 Sep 2020 08:16:20 PDT (-0700), [email protected] 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)

Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 33 lines

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

Quoted 29 lines

Quoted 29 lines

Cc Maxim Uvarov <[email protected]>

Cc Heinrich Schuchardt <[email protected]>

Cc Atish Patra <[email protected]>

Cc Palmer Dabbelt <[email protected]>

Cc Jens Wiklander <[email protected]>

Cc Francois Ozog <[email protected]>

Cc Etienne CARRIERE <[email protected]>

Cc Takahiro Akashi <[email protected]>

Cc Patrice CHOTARD <[email protected]>

Cc Sumit Garg <[email protected]>

Cc Grant Likely <[email protected]>

Cc Ilias Apalodimas <[email protected]>

Cc Christophe Priouzeau <[email protected]>

Cc Rouven Czerwinski <[email protected]>

Cc Patrick DELAUNAY <[email protected]>

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.

arch/riscv/include/asm/efi.h +0 -0

index 93c305a638f4..dd6ceea9d548 100644

arch/riscv/include/asm/efi.h static inline unsigned long · line 43

Quoted 8 lines

arch/riscv/include/asm/efi.h +1 -1

@@ -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.

Quoted 9 lines

drivers/firmware/efi/libstub/riscv-stub.c +1 -1

@@ -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);
 

Quoted 1 line

FWIW: Tested-by: Atish Patra <[email protected]>

Thanks for confirming.

Quoted 98 lines · Ard Biesheuvel

Quoted 75 lines · Atish Patra

Quoted 32 lines · Atish Patra

Quoted 16 lines · Palmer Dabbelt

Quoted 10 lines · On Wed, 09 Sep 2020 08:16:20 PDT (-0700), [email protected] 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)

Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 33 lines

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

Quoted 29 lines

Quoted 29 lines

Cc Maxim Uvarov <[email protected]>

Cc Heinrich Schuchardt <[email protected]>

Cc Atish Patra <[email protected]>

Cc Palmer Dabbelt <[email protected]>

Cc Jens Wiklander <[email protected]>

Cc Francois Ozog <[email protected]>

Cc Etienne CARRIERE <[email protected]>

Cc Takahiro Akashi <[email protected]>

Cc Patrice CHOTARD <[email protected]>

Cc Sumit Garg <[email protected]>

Cc Grant Likely <[email protected]>

Cc Ilias Apalodimas <[email protected]>

Cc Christophe Priouzeau <[email protected]>

Cc Rouven Czerwinski <[email protected]>

Cc Patrick DELAUNAY <[email protected]>

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.

arch/riscv/include/asm/efi.h +0 -0

index 93c305a638f4..dd6ceea9d548 100644

arch/riscv/include/asm/efi.h static inline unsigned long · line 43

Quoted 8 lines

arch/riscv/include/asm/efi.h +1 -1

@@ -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.

Quoted 9 lines

drivers/firmware/efi/libstub/riscv-stub.c +1 -1

@@ -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);
 

Quoted 3 lines

Quoted 1 line

FWIW: Tested-by: Atish Patra <[email protected]>

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.

Quoted 98 lines · Ard Biesheuvel

Quoted 75 lines · Atish Patra

Quoted 32 lines · Atish Patra

Quoted 16 lines · Palmer Dabbelt

Quoted 10 lines · On Wed, 09 Sep 2020 08:16:20 PDT (-0700), [email protected] 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)

Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 33 lines

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

Quoted 29 lines

Quoted 29 lines

Cc Maxim Uvarov <[email protected]>

Cc Heinrich Schuchardt <[email protected]>

Cc Atish Patra <[email protected]>

Cc Palmer Dabbelt <[email protected]>

Cc Jens Wiklander <[email protected]>

Cc Francois Ozog <[email protected]>

Cc Etienne CARRIERE <[email protected]>

Cc Takahiro Akashi <[email protected]>

Cc Patrice CHOTARD <[email protected]>

Cc Sumit Garg <[email protected]>

Cc Grant Likely <[email protected]>

Cc Ilias Apalodimas <[email protected]>

Cc Christophe Priouzeau <[email protected]>

Cc Rouven Czerwinski <[email protected]>

Cc Patrick DELAUNAY <[email protected]>

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.

arch/riscv/include/asm/efi.h +0 -0

index 93c305a638f4..dd6ceea9d548 100644

arch/riscv/include/asm/efi.h static inline unsigned long · line 43

Quoted 8 lines

arch/riscv/include/asm/efi.h +1 -1

@@ -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.

Quoted 9 lines

drivers/firmware/efi/libstub/riscv-stub.c +1 -1

@@ -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);
 

Quoted 3 lines

Quoted 1 line

FWIW: Tested-by: Atish Patra <[email protected]>

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.

Quoted 120 lines · On Thu, 10 Sep 2020 07:08:07 PDT (-0700), [email protected] wrote:

Quoted 98 lines · Ard Biesheuvel

Quoted 75 lines · Atish Patra

Quoted 32 lines · Atish Patra

Quoted 16 lines · Palmer Dabbelt

Quoted 10 lines · On Wed, 09 Sep 2020 08:16:20 PDT (-0700), [email protected] 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)

Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 33 lines

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

Quoted 29 lines

Quoted 29 lines

Cc Maxim Uvarov <[email protected]>

Cc Heinrich Schuchardt <[email protected]>

Cc Atish Patra <[email protected]>

Cc Palmer Dabbelt <[email protected]>

Cc Jens Wiklander <[email protected]>

Cc Francois Ozog <[email protected]>

Cc Etienne CARRIERE <[email protected]>

Cc Takahiro Akashi <[email protected]>

Cc Patrice CHOTARD <[email protected]>

Cc Sumit Garg <[email protected]>

Cc Grant Likely <[email protected]>

Cc Ilias Apalodimas <[email protected]>

Cc Christophe Priouzeau <[email protected]>

Cc Rouven Czerwinski <[email protected]>

Cc Patrick DELAUNAY <[email protected]>

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.

arch/riscv/include/asm/efi.h +0 -0

index 93c305a638f4..dd6ceea9d548 100644

arch/riscv/include/asm/efi.h static inline unsigned long · line 43

Quoted 8 lines

arch/riscv/include/asm/efi.h +1 -1

@@ -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.

Quoted 9 lines

drivers/firmware/efi/libstub/riscv-stub.c +1 -1

@@ -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);
 

Quoted 3 lines

Quoted 1 line

FWIW: Tested-by: Atish Patra <[email protected]>

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.

Quoted 3 lines

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.

Quoted 122 lines · Palmer Dabbelt

Quoted 120 lines · On Thu, 10 Sep 2020 07:08:07 PDT (-0700), [email protected] wrote:

Quoted 98 lines · Ard Biesheuvel

Quoted 75 lines · Atish Patra

Quoted 32 lines · Atish Patra

Quoted 16 lines · Palmer Dabbelt

Quoted 10 lines · On Wed, 09 Sep 2020 08:16:20 PDT (-0700), [email protected] 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)

Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 33 lines

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

Quoted 29 lines

Quoted 29 lines

Cc Maxim Uvarov <[email protected]>

Cc Heinrich Schuchardt <[email protected]>

Cc Atish Patra <[email protected]>

Cc Palmer Dabbelt <[email protected]>

Cc Jens Wiklander <[email protected]>

Cc Francois Ozog <[email protected]>

Cc Etienne CARRIERE <[email protected]>

Cc Takahiro Akashi <[email protected]>

Cc Patrice CHOTARD <[email protected]>

Cc Sumit Garg <[email protected]>

Cc Grant Likely <[email protected]>

Cc Ilias Apalodimas <[email protected]>

Cc Christophe Priouzeau <[email protected]>

Cc Rouven Czerwinski <[email protected]>

Cc Patrick DELAUNAY <[email protected]>

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.

arch/riscv/include/asm/efi.h +0 -0

index 93c305a638f4..dd6ceea9d548 100644

arch/riscv/include/asm/efi.h static inline unsigned long · line 43

Quoted 8 lines

arch/riscv/include/asm/efi.h +1 -1

@@ -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.

Quoted 9 lines

drivers/firmware/efi/libstub/riscv-stub.c +1 -1

@@ -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);
 

Quoted 3 lines

Quoted 1 line

FWIW: Tested-by: Atish Patra <[email protected]>

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.

Quoted 5 lines

Quoted 3 lines

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

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

Quoted 153 lines · Palmer Dabbelt

Quoted 122 lines · On Thu, 10 Sep 2020 07:08:07 PDT (-0700), [email protected] wrote:

Quoted 120 lines · Ard Biesheuvel

Quoted 98 lines · Atish Patra

Quoted 75 lines · Atish Patra

Quoted 32 lines · Palmer Dabbelt

Quoted 16 lines · On Wed, 09 Sep 2020 08:16:20 PDT (-0700), [email protected] wrote:

Quoted 10 lines

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)

Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 33 lines

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

Quoted 29 lines

Quoted 29 lines

Cc Maxim Uvarov <[email protected]>

Cc Heinrich Schuchardt <[email protected]>

Cc Atish Patra <[email protected]>

Cc Palmer Dabbelt <[email protected]>

Cc Jens Wiklander <[email protected]>

Cc Francois Ozog <[email protected]>

Cc Etienne CARRIERE <[email protected]>

Cc Takahiro Akashi <[email protected]>

Cc Patrice CHOTARD <[email protected]>

Cc Sumit Garg <[email protected]>

Cc Grant Likely <[email protected]>

Cc Ilias Apalodimas <[email protected]>

Cc Christophe Priouzeau <[email protected]>

Cc Rouven Czerwinski <[email protected]>

Cc Patrick DELAUNAY <[email protected]>

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.

arch/riscv/include/asm/efi.h +0 -0

index 93c305a638f4..dd6ceea9d548 100644

arch/riscv/include/asm/efi.h static inline unsigned long · line 43

Quoted 8 lines

arch/riscv/include/asm/efi.h +1 -1

@@ -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.

Quoted 9 lines

drivers/firmware/efi/libstub/riscv-stub.c +1 -1

@@ -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);
 

Quoted 3 lines

Quoted 1 line

FWIW: Tested-by: Atish Patra <[email protected]>

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.

Quoted 5 lines

Quoted 3 lines

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

Quoted 4 lines · Maxim Uvarov

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

Quoted 148 lines · Ard Biesheuvel

Quoted 146 lines · Palmer Dabbelt

Quoted 122 lines · On Thu, 10 Sep 2020 07:08:07 PDT (-0700), [email protected] wrote:

Quoted 120 lines · Ard Biesheuvel

Quoted 98 lines · Atish Patra

Quoted 75 lines · Atish Patra

Quoted 32 lines · Palmer Dabbelt

Quoted 16 lines · On Wed, 09 Sep 2020 08:16:20 PDT (-0700), [email protected] wrote:

Quoted 10 lines

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)

Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 33 lines

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

Quoted 29 lines

Quoted 29 lines

Cc Maxim Uvarov <[email protected]>

Cc Heinrich Schuchardt <[email protected]>

Cc Atish Patra <[email protected]>

Cc Palmer Dabbelt <[email protected]>

Cc Jens Wiklander <[email protected]>

Cc Francois Ozog <[email protected]>

Cc Etienne CARRIERE <[email protected]>

Cc Takahiro Akashi <[email protected]>

Cc Patrice CHOTARD <[email protected]>

Cc Sumit Garg <[email protected]>

Cc Grant Likely <[email protected]>

Cc Ilias Apalodimas <[email protected]>

Cc Christophe Priouzeau <[email protected]>

Cc Rouven Czerwinski <[email protected]>

Cc Patrick DELAUNAY <[email protected]>

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.

arch/riscv/include/asm/efi.h +0 -0

index 93c305a638f4..dd6ceea9d548 100644

arch/riscv/include/asm/efi.h static inline unsigned long · line 43

Quoted 8 lines

arch/riscv/include/asm/efi.h +1 -1

@@ -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.

Quoted 9 lines

drivers/firmware/efi/libstub/riscv-stub.c +1 -1

@@ -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);
 

Quoted 3 lines

Quoted 1 line

FWIW: Tested-by: Atish Patra <[email protected]>

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.

Quoted 5 lines

Quoted 3 lines

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.

Quoted 7 lines

Quoted 7 lines

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

Quoted 12 lines · Ard Biesheuvel

Quoted 4 lines · Maxim Uvarov

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 <[email protected]

Maxim.

Quoted 126 lines · Palmer Dabbelt

Quoted 123 lines · On Thu, 10 Sep 2020 07:08:07 PDT (-0700), [email protected] wrote:

Quoted 98 lines · Ard Biesheuvel

Quoted 75 lines · Atish Patra

Quoted 32 lines · Atish Patra

Quoted 16 lines · Palmer Dabbelt

Quoted 10 lines · On Wed, 09 Sep 2020 08:16:20 PDT (-0700), [email protected] 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)

Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 33 lines

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

Quoted 29 lines

Quoted 29 lines

Cc Maxim Uvarov <[email protected]>

Cc Heinrich Schuchardt <[email protected]>

Cc Atish Patra <[email protected]>

Cc Palmer Dabbelt <[email protected]>

Cc Jens Wiklander <[email protected]>

Cc Francois Ozog <[email protected]>

Cc Etienne CARRIERE <[email protected]>

Cc Takahiro Akashi <[email protected]>

Cc Patrice CHOTARD <[email protected]>

Cc Sumit Garg <[email protected]>

Cc Grant Likely <[email protected]>

Cc Ilias Apalodimas <[email protected]>

Cc Christophe Priouzeau <[email protected]>

Cc Rouven Czerwinski <[email protected]>

Cc Patrick DELAUNAY <[email protected]>

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.

arch/riscv/include/asm/efi.h +0 -0

index 93c305a638f4..dd6ceea9d548 100644

arch/riscv/include/asm/efi.h static inline unsigned long · line 43

Quoted 8 lines

arch/riscv/include/asm/efi.h +1 -1

@@ -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.

Quoted 9 lines

drivers/firmware/efi/libstub/riscv-stub.c +1 -1

@@ -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);
 

Quoted 3 lines

Quoted 1 line

FWIW: Tested-by: Atish Patra <[email protected]>

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.

Quoted 1 line

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

Quoted 128 lines · Ard Biesheuvel

Quoted 126 lines · Palmer Dabbelt

Quoted 123 lines · On Thu, 10 Sep 2020 07:08:07 PDT (-0700), [email protected] wrote:

Quoted 98 lines · Ard Biesheuvel

Quoted 75 lines · Atish Patra

Quoted 32 lines · Atish Patra

Quoted 16 lines · Palmer Dabbelt

Quoted 10 lines · On Wed, 09 Sep 2020 08:16:20 PDT (-0700), [email protected] 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)

Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 33 lines

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

Quoted 29 lines

Quoted 29 lines

Cc Maxim Uvarov <[email protected]>

Cc Heinrich Schuchardt <[email protected]>

Cc Atish Patra <[email protected]>

Cc Palmer Dabbelt <[email protected]>

Cc Jens Wiklander <[email protected]>

Cc Francois Ozog <[email protected]>

Cc Etienne CARRIERE <[email protected]>

Cc Takahiro Akashi <[email protected]>

Cc Patrice CHOTARD <[email protected]>

Cc Sumit Garg <[email protected]>

Cc Grant Likely <[email protected]>

Cc Ilias Apalodimas <[email protected]>

Cc Christophe Priouzeau <[email protected]>

Cc Rouven Czerwinski <[email protected]>

Cc Patrick DELAUNAY <[email protected]>

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.

arch/riscv/include/asm/efi.h +0 -0

index 93c305a638f4..dd6ceea9d548 100644

arch/riscv/include/asm/efi.h static inline unsigned long · line 43

Quoted 8 lines

arch/riscv/include/asm/efi.h +1 -1

@@ -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.

Quoted 9 lines

drivers/firmware/efi/libstub/riscv-stub.c +1 -1

@@ -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);
 

Quoted 3 lines

Quoted 1 line

FWIW: Tested-by: Atish Patra <[email protected]>

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.

Quoted 9 lines

Quoted 1 line

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.

Quoted 16 lines

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