Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210104130111.1269694-1-geert+renesas@glider.be> <CACRpkdZyV-tye0b6Pxf6s_SSEy1sq=Hqr_xXUopJrCkXsu9m9g@mail.gmail.com>
In-Reply-To: <CACRpkdZyV-tye0b6Pxf6s_SSEy1sq=Hqr_xXUopJrCkXsu9m9g@mail.gmail.com>
From:   Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Mon, 11 Jan 2021 17:19:24 +0100
Message-ID: <CAMuHMdVK+iYu-mEPfcNK0OwpFMs8re2uC7YAYzt5_CYvo_8BQg@mail.gmail.com>
Subject: Re: [PATCH v12] ARM: uncompress: Validate start of physical memory
 against passed DTB
To:     Linus Walleij <linus.walleij@linaro.org>
Cc:     Russell King <linux@armlinux.org.uk>,
        Ard Biesheuvel <ardb@kernel.org>,
        Nicolas Pitre <nico@fluxnic.net>,
        Marek Szyprowski <m.szyprowski@samsung.com>,
        Dmitry Osipenko <digetx@gmail.com>,
        Arnd Bergmann <arnd@arndb.de>,
        Eric Miao <eric.miao@nvidia.com>,
        =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= 
        <u.kleine-koenig@pengutronix.de>,
        Lukasz Stelmach <l.stelmach@samsung.com>,
        Stephen Boyd <sboyd@kernel.org>,
        Chris Brandt <chris.brandt@renesas.com>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Grant Likely <grant.likely@linaro.org>,
        Michael Ellerman <mpe@ellerman.id.au>,
        Heinrich Schuchardt <xypron.glpk@gmx.de>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Linus,

CC Rob, Grant, Michael, Heinrich, DT

On Sat, Jan 9, 2021 at 12:57 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Jan 4, 2021 at 2:01 PM Geert Uytterhoeven
> <geert+renesas@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 validating the masked address against the memory
> > information in the passed DTB.  Only use the start address
> > from DTB when masking would yield an out-of-range address, prefer the
> > traditional method in all other cases.  Note that this applies only to the
> > explicitly passed DTB on modern systems, and not to a DTB appended to
> > the kernel, or to ATAGS.  The appended DTB may need to be augmented by
> > information from ATAGS, which may need to rely on knowledge of the start
> > address of physical memory itself.
> >
> > 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@fluxnic.net>
> > Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> > Acked-by: Nicolas Pitre <nico@fluxnic.net>
>
> Sorry for the long delay in reading the patch :(
>
> I really like the looks of this now, moreover it is very useful.
> I suppose it is already in the patch tracker, but for the record:
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Thanks a lot!

> > +               reg = fdt_getprop(fdt, offset, "linux,usable-memory", &len);
>
> I suppose we already had a discussion of why this property
> is undocumented? Or should we document it? Obviously
> it is already in widespread use.

This comes from commit 51975db0b7333cf3 ("of/flattree: merge
early_init_dt_scan_memory() common code"), which combined existing
practises on Microblaze (commit 12e8414263f47352 ("microblaze_v8: Open
firmware files")) and PowerPC (ba7594852f4e7121 ("[PATCH] powerpc: Add
support for "linux,usable-memory" on memory nodes")), with the former
obviously just copying the latter.
Unfortunately none of this is documented in The DeviceTree
Specification, ePAPR, or P1275.

Heinrich tried to document it, but his patch was ignored:
[PATCH] Documentation: devicetree: "linux,usable-memory" property
https://lore.kernel.org/linux-devicetree/20161223161747.9986-1-xypron.glpk@gmx.de/
https://lkml.org/lkml/2016/12/23/175
https://lore.kernel.org/patchwork/patch/745784/
Note that Heinrichs address is mangled in lore (imported from gmane?,
but lkml and patchwork have it right.


Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds