Re: [PATCH v12] ARM: uncompress: Validate start of physical memory against passed DTB

[Geert Uytterhoeven]

Hi Linus,

CC Rob, Grant, Michael, Heinrich, DT

On Sat, Jan 9, 2021 at 12:57 AM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
> On Mon, Jan 4, 2021 at 2:01 PM Geert Uytterhoeven
> <geert+renesas@xxxxxxxxx> 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@xxxxxxxxxxx>
> > Suggested-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > Reviewed-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > Acked-by: Nicolas Pitre <nico@xxxxxxxxxxx>
>
> 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@xxxxxxxxxx>

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@xxxxxx/
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@xxxxxxxxxxxxxx

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