On Fri, 11 Sep 2020 at 21:45, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > On Fri, 11 Sep 2020 at 13:27, Maxim Uvarov <maxim.uvarov@xxxxxxxxxx> 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@xxxxxxxxxx Maxim. > > > On Fri, 11 Sep 2020 at 10:56, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > > > > > On Fri, 11 Sep 2020 at 05:16, Palmer Dabbelt <palmer@xxxxxxxxxxx> wrote: > > > > > > > > On Thu, 10 Sep 2020 07:08:07 PDT (-0700), ardb@xxxxxxxxxx wrote: > > > > > On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > > > >> > > > > >> On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@xxxxxxxxxxxxxx> wrote: > > > > >> > > > > > >> > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@xxxxxxxxxxxxxx> wrote: > > > > >> > > > > > > >> > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@xxxxxxxxxxx> wrote: > > > > >> > > > > > > > >> > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@xxxxxxxxxx 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@xxxxxxxxxx> > > > > >> > > > > Cc: Heinrich Schuchardt <xypron.glpk@xxxxxx> > > > > >> > > > > Cc: Atish Patra <atish.patra@xxxxxxx> > > > > >> > > > > Cc: Palmer Dabbelt <palmer@xxxxxxxxxxx> > > > > >> > > > > Cc: Jens Wiklander <jens.wiklander@xxxxxxxxxx> > > > > >> > > > > Cc: Francois Ozog <francois.ozog@xxxxxxxxxx> > > > > >> > > > > Cc: Etienne CARRIERE <etienne.carriere@xxxxxx> > > > > >> > > > > Cc: Takahiro Akashi <takahiro.akashi@xxxxxxxxxx> > > > > >> > > > > Cc: Patrice CHOTARD <patrice.chotard@xxxxxx> > > > > >> > > > > Cc: Sumit Garg <sumit.garg@xxxxxxxxxx> > > > > >> > > > > Cc: Grant Likely <Grant.Likely@xxxxxxx> > > > > >> > > > > Cc: Ilias Apalodimas <ilias.apalodimas@xxxxxxxxxx> > > > > >> > > > > Cc: Christophe Priouzeau <christophe.priouzeau@xxxxxxxxxx> > > > > >> > > > > Cc: Rouven Czerwinski <r.czerwinski@xxxxxxxxxxxxxx> > > > > >> > > > > Cc: Patrick DELAUNAY <patrick.delaunay@xxxxxx> > > > > >> > > > > > > > > >> > > > > 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@xxxxxxx> > > > > >> > > > > >> 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
