From: Palmer Dabbelt <palmer@dabbelt.com>
To: ardb@kernel.org
Cc: etienne.carriere@st.com, francois.ozog@linaro.org,
maxim.uvarov@linaro.org, Grant.Likely@arm.com,
takahiro.akashi@linaro.org, r.czerwinski@pengutronix.de,
xypron.glpk@gmx.de, ilias.apalodimas@linaro.org,
patrice.chotard@st.com, patrick.delaunay@st.com,
Atish Patra <Atish.Patra@wdc.com>,
linux-efi@vger.kernel.org, atishp@atishpatra.org,
christophe.priouzeau@linaro.org, jens.wiklander@linaro.org,
linux-arm-kernel@lists.infradead.org, sumit.garg@linaro.org
Subject: Re: [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base
Date: Thu, 10 Sep 2020 19:16:38 -0700 (PDT) [thread overview]
Message-ID: <mhng-f3ca14fc-58f7-423e-8abe-a85de1a2d55b@palmerdabbelt-glaptop1> (raw)
In-Reply-To: <CAMj1kXG6VRXOPRhHmeyiPP06BVByEYsqYE001BZYyRDqtvyDBg@mail.gmail.com>
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
next prev parent reply other threads:[~2020-09-11 2:18 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-09 15:16 [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base Ard Biesheuvel
2020-09-09 15:16 ` [PATCH RFC/RFT 1/3] efi/libstub: Export efi_low_alloc_above() to other units Ard Biesheuvel
2020-09-09 15:16 ` [PATCH RFC/RFT 2/3] efi/libstub: Use low allocation for the uncompressed kernel Ard Biesheuvel
2020-09-09 15:16 ` [PATCH RFC/RFT 3/3] efi/libstub: base FDT and initrd placement on image address not DRAM base Ard Biesheuvel
2020-09-09 15:25 ` [PATCH RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base Grant Likely
2020-09-09 15:30 ` Ard Biesheuvel
2020-09-09 20:52 ` Palmer Dabbelt
2020-09-09 21:44 ` Atish Patra
2020-09-10 1:34 ` Atish Patra
2020-09-10 10:04 ` Ard Biesheuvel
2020-09-10 14:08 ` Ard Biesheuvel
2020-09-10 23:32 ` Atish Patra
2020-09-11 2:16 ` Palmer Dabbelt [this message]
2020-09-11 7:56 ` Ard Biesheuvel
2020-09-11 10:27 ` Maxim Uvarov
2020-09-11 18:45 ` Ard Biesheuvel
2020-09-16 15:43 ` Maxim Uvarov
2020-09-16 15:58 ` Ard Biesheuvel
2020-09-16 16:02 ` Ard Biesheuvel
2020-09-16 22:01 ` Atish Patra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=mhng-f3ca14fc-58f7-423e-8abe-a85de1a2d55b@palmerdabbelt-glaptop1 \
--to=palmer@dabbelt.com \
--cc=Atish.Patra@wdc.com \
--cc=Grant.Likely@arm.com \
--cc=ardb@kernel.org \
--cc=atishp@atishpatra.org \
--cc=christophe.priouzeau@linaro.org \
--cc=etienne.carriere@st.com \
--cc=francois.ozog@linaro.org \
--cc=ilias.apalodimas@linaro.org \
--cc=jens.wiklander@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-efi@vger.kernel.org \
--cc=maxim.uvarov@linaro.org \
--cc=patrice.chotard@st.com \
--cc=patrick.delaunay@st.com \
--cc=r.czerwinski@pengutronix.de \
--cc=sumit.garg@linaro.org \
--cc=takahiro.akashi@linaro.org \
--cc=xypron.glpk@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).