Hi Linus, On Thu, Sep 3, 2020 at 10:49 PM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote: > On Wed, Sep 2, 2020 at 5:36 PM Geert Uytterhoeven > <geert+renesas@xxxxxxxxx> wrote: > > @@ -254,8 +254,56 @@ not_angel: > > This looks like it happens before CONFIG_ARM_ATAG_DTB_COMPAT > meaning it will not use the DTB that is augmented with ATAGs, > especially not ATAG_MEM (0x54410002) correct? > > That seems sad, because that would actually be useful to me. > > Can you see if it is possible to put this in after the ATAGs have > been merged into the DTB? I made a deep dive into arch/arm/boot/compressed/head.S, to analyze all possible combinations of the various options (your article [1] was a great help, thanks for that!). I considered all of the following: - Passed by bootloader in r2: [A] ATAGS, [B] DTB (with proper memory nodes), [C] Rubbish. - Optional appended DTB (CONFIG_ARM_APPENDED_DTB=y): [D] DTB (with proper memory nodes), [E] DTB (without memory nodes). Notes: - The appended DTB takes precedence over the DTB passed via r2 (case [B])! - This is meant as a backward compatibility convenience for systems with a bootloader that can't be upgraded to accommodate the documented boot protocol using a device tree. - ATAGS to augment appended DTB (CONFIG_ARM_ATAG_DTB_COMPAT=y): [F] Passed via r2, [G] Stored at start of memory + 0x100. Notes: - [F] is tried first; it it fails, [H] is tried next, - This is meant as another backward compatibility convenience for systems with an old bootloader, - [H] The Kdump kernel is deliberately not stored in the first 128 MiB of RAM. Of course not all combinations are possible/sensible. Note that there exists another option (handover from EFI), which is not relevant here, as it follows a different code path, and passes the image base explicitly. > It would be better if we could avoid copy/paste and > merge the code in here so we first check if there is a > DTB, else there is not much to do, and if there is, we > get the size, move the sp and do both operations: > > 1. Check for ATAGs and augment the DTB > 2. Check for memory size in the DTB > > This way the ATAG-augmented DTB can be used > for checking for the memory start. > > I understand that you need the physical address > before turning on the caches, so what I am thinking > is if it is possible to move the check for appended > DTB and ATAG augmentation business up before > the cache enablement in a separate patch and then > modify it from there. Then you could potentially > merge these two things. > > Maybe there is something conceptually wrong with > this that I have overlooked... :/ Augmenting the appended DTB (case [E]) with information passed in ATAGS via r2 (case [F]) can indeed be moved up. However, augmenting the appended DTB with information stored in ATAGS at the start of physical RAM + 0x100 (case [G]) cannot be moved up, as it relies on knowing the start of memory. Hence this can only be done on systems where the start of RAM is a multiple of 128 MiB, and masking the PC with 0xf8000000 yields a valid RAM address, thus leading to a chicken-and-egg problem... Given the appended DTB, and augmenting it with information from ATAGS, is clearly marked as a backward compatibility convenience for systems with a bootloader that cannot be upgraded, I think it is reasonable to take a step back, and limit the validation to modern systems which do pass the DTB in r2. This would simplify the code, and avoid regressions. Does that sound right? Later, we can easily add on top support for kdump adding a "linux,usable-memory-range" property to the DTB (passed in r2), cfr. [2] and [3]. Thanks for your comments! [1] https://people.kernel.org/linusw/how-the-arm32-linux-kernel-decompresses [2] "[PATCH] ARM: Parse kdump DT properties" (https://lore.kernel.org/r/20200902154538.6807-1-geert+renesas@xxxxxxxxx) [3] "PATCH] arm: kdump: Add DT properties to crash dump kernel's DTB" (https://lore.kernel.org/r/20200902154129.6358-1-geert+renesas@xxxxxxxxx). 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