[PATCH/RFC v7] ARM: boot: Obtain start of physical memory from DTB

[Ard Biesheuvel]

On Tue, 7 Jul 2020 at 10:58, Geert Uytterhoeven <geert at linux-m68k.org> wrote:
>
> Hi Ard,
>
> On Tue, Jul 7, 2020 at 9:45 AM Ard Biesheuvel <ardb at kernel.org> wrote:
> > On Tue, 7 Jul 2020 at 10:39, Geert Uytterhoeven <geert at linux-m68k.org> wrote:
> > > On Tue, Jul 7, 2020 at 8:50 AM Ard Biesheuvel <ardb at kernel.org> wrote:
> > > > On Mon, 6 Jul 2020 at 18:02, Geert Uytterhoeven <geert+renesas at glider.be> wrote:
> > > > > Currently, the start address of physical memory is obtained by masking
> > > > > the program counter with a fixed mask of 0xf8000000.  This mask value
> > > > > was chosen as a balance between the requirements of different platforms.
> > > > > However, this does require that the start address of physical memory is
> > > > > a multiple of 128 MiB, precluding booting Linux on platforms where this
> > > > > requirement is not fulfilled.
> > > > >
> > > > > Fix this limitation by obtaining the start address from the DTB instead,
> > > > > if available (either explicitly passed, or appended to the kernel).
> > > > > Fall back to the traditional method when needed.
> > > > >
> > > > > This allows to boot Linux on r7s9210/rza2mevb using the 64 MiB of SDRAM
> > > > > on the RZA2MEVB sub board, which is located at 0x0C000000 (CS3 space),
> > > > > i.e. not at a multiple of 128 MiB.
> > > > >
> > > > > Suggested-by: Nicolas Pitre <nico at fluxnic.net>
> > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas at glider.be>
> > > > > Reviewed-by: Nicolas Pitre <nico at fluxnic.net>
> > > > > Reviewed-by: Ard Biesheuvel <ardb at kernel.org>
> > > > > Tested-by: Marek Szyprowski <m.szyprowski at samsung.com>
> > > > > Tested-by: Dmitry Osipenko <digetx at gmail.com>
> > > > > Cc: Lukasz Stelmach <l.stelmach at samsung.com>
> > > > > ---
> > > > > Marked as RFC, because:
> > > > >   1. This is known to break crashkernel support, as the memory used by
> > > > >      the crashkernel is not marked reserved in DT (yet),
> > > > >   2. Russell won't apply this for v5.9 anyway,
> > > > >
> > > >
> > > > Would it help if we make this behavior dependent on a simple heuristic, e.g.,
> > > >
> > > > if (round_up(load_address, 128M) >= dram_end)
> > > >   use dram_start from DT
> > > > else
> > > >   use round_up(load_address, 128M)
> > > >
> > > > That way, the fix is guaranteed to only take effect for systems that
> > > > cannot even boot otherwise, which fixes the crashkernel case, as well
> > > > as other potential regressions due to the load address of the core
> > > > kernel changing for existing boards.
> > >
> > > Thanks for your suggestion!
> > >   1. Shouldn't the calculation use round_down() instead of round_up()?
> > >   2. Likewise, "round_down(load_address, 128M) < dram_start from DT"?
> > >
> >
> > No.
> >
> > What the code does today is round *up* to a multiple of 128 MB, and
> > only when that leads to a problem, we should use the DT provided
> > memory regions.
>
>                 mov     r4, pc
>                 and     r4, r4, #0xf8000000
>
> Surely this is rounding down, isn't it?
>

Yes you are right.

>                 add     r4, r4, #TEXT_OFFSET
>
> Followed by adding a small number (typically 0x00008000).
>
> On RZA2MEVB with 64 MiB of RAM, the result lies below dram_start.

Yes, but in the general case, this is not true. Platforms that manage
to boot using the current arrangement will do so by putting the
decompressor above the first 128 MB aligned boundary covered by DRAM
(and lose access to any memory below it via the linear mapping, but
this memory could still be used via a no-map reserved-memory node
AFAIK.)

> BTW, how to obtain dram_end? From DT again? Do we trust it, as we
> apparently cannot trust dram_start in some configurations.
>
> Do I need more coffee?
>

Maybe we both do :-)

AIUI, the reason we cannot trust dram_start is because of the
crashkernel case, i.e., the kernel may have deliberately been put high
up in memory, and the expectation is that the load address is derived
by rounding down the load address of the decompressor.

Hence my suggestion to round *up* and compare with dram_end: if
round_up(load_address, 128M) >= dram_end holds, it is guaranteed that
no address exists in memory from which we could round down and arrive
at a valid DRAM address. This would mean that your change will only
affect platforms that were unable to boot to begin with, and not
affect any other weird configurations including crashkernels etc