Date:   Tue, 19 May 2020 10:30:05 +0100
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Lukasz Stelmach <l.stelmach@...sung.com>
Cc:     Geert Uytterhoeven <geert+renesas@...der.be>,
        Dmitry Osipenko <digetx@...il.com>,
        Nicolas Pitre <nico@...xnic.net>,
        Arnd Bergmann <arnd@...db.de>,
        Eric Miao <eric.miao@...dia.com>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Ard Biesheuvel <ardb@...nel.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Chris Brandt <chris.brandt@...esas.com>,
        linux-arm-kernel@...ts.infradead.org,
        linux-renesas-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
Subject: Re: [PATCH v6] ARM: boot: Obtain start of physical memory from DTB

On Tue, May 19, 2020 at 10:53:52AM +0200, Lukasz Stelmach wrote:
> It was <2020-04-29 śro 10:21>, when Geert Uytterhoeven 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@...xnic.net>
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@...der.be>
> > Reviewed-by: Nicolas Pitre <nico@...xnic.net>
> > Reviewed-by: Ard Biesheuvel <ardb@...nel.org>
> > Tested-by: Marek Szyprowski <m.szyprowski@...sung.com>
> > Tested-by: Dmitry Osipenko <digetx@...il.com>
> > ---
> 
> [...]
> 
> Apparently reading physical memory layout from DTB breaks crashdump
> kernels. A crashdump kernel is loaded into a region of memory, that is
> reserved in the original (i.e. to be crashed) kernel. The reserved
> region is large enough for the crashdump kernel to run completely inside
> it and don't modify anything outside it, just read and dump the remains
> of the crashed kernel. Using the information from DTB makes the
> decompressor place the kernel outside of the dedicated region.
> 
> The log below shows that a zImage and DTB are loaded at 0x18eb8000 and
> 0x193f6000 (physical). The kernel is expected to run at 0x18008000, but
> it is decompressed to 0x00008000 (see r4 reported before jumping from
> within __enter_kernel).

Right, and it's important that the kernel decompresses to 0x18008000
so it doesn't overwrite memory that was being used by the crashing
kernel, and thus can create a true coredump image of the failed
kernel.  Meanwhile, the DTB still needs to describe the full memory
layout so that we know where memory is located in order to coredump
it properly.

So, this is a flaw with this approach, and will need the commit to be
dropped yet again - this patch is fundamentally incompatible with the
way kexec's crashdump works.

Looking back at the history, we've been trying this approach since
February with four patches submitted to the patch system, and problems
eventually found with each of them.  I think this shows that the way
the decompressor works out where to decompress the kernel to today is
relied upon all over the place, and changing it is always going to
cause problems.  So, I don't think we /can/ change it without causing
a regression for someone.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up

Powered by blists - more mailing lists