[PATCH 1/6 v14] ARM: Handle a device tree in lowmem

[Linus Walleij]

Hi Ard,

thanks for helping out, finally we're getting somewhere with this!

On Mon, Oct 5, 2020 at 9:15 AM Ard Biesheuvel <ardb at kernel.org> wrote:
> On Sun, 4 Oct 2020 at 22:50, Linus Walleij <linus.walleij at linaro.org> wrote:

> OK, so from your explanation, I gathered that:
> - the 'appended DTB' is not appended anymore when the core kernel
> boots, and the problem is caused by the fact that the decompressor
> blindly chooses a location for it, whereas the firmware makes a more
> informed decision when it places the DT in memory
> - the memory holding the DT may be unmapped inadvertently
> - the memory holding the DT may be wiped inadvertently.

Correct.

> > The kernel actually fits in the first memblock, but then it clears
> > the lowmem right below itself and by that point the MMU
> > has figured out that there is another memblock below it
> > that it will happily use for lowmem and just goes ahead and
> > wipes that. The code has no awareness
> > that there might be a DTB there.
>
> Where in the code does this happen?

This happens in mm/mmu.c, prepare_page_table(), line1283:

/*
* Clear out all the kernel space mappings, except for the first
* memory bank, up to the vmalloc region.
*/
for (addr = __phys_to_virt(end); addr < VMALLOC_START; addr += PMD_SIZE)
        pmd_clear(pmd_off_k(addr));

Here end is augmented roofed at arm_lowmem_limit which seems
common but that incidentally does not happen on my platform
because of the small size of the first memblock.

> It seems to me that at this point,
> the DT memory should have been memblock_reserve()d, and the code in
> question should disregard it from wiping.

This code seems pretty ignorant about anything like that,
This  happens as the first call in paging_init() after
arm_memblock_init() but before we
have called memblock_set_current_limit().

We need to keep these mappings alive until we
have called unflatten_and_copy_device_tree()
which transfers the device tree into memory the kernel
controls. Then we should free/unmap that memory
so that the kernel can use it. What happens to the memory
where the DTB is stored right now is kind of "undefined"
which doesn't feel good to me at all.

> I think putting the mapping of the DT outside of the linear region is
> reasonable, tbh, and this is what we do on arm64.

Yes that is much better. Arm64 also relies on the bootloader
to do the decompression and some other simplifications I really
like.

> One option would be to create a virtual mapping for the DT at the base
> of the modules region. This takes up 1 MB in the typical (non-LPAE)
> case, and 4 MB in the worst case, making it more likely that you will
> run out of module space, but we already have infrastructure to deal
> with that.

2MB (two section mappings) is what head.S always maps
and that would be 4MB on LPAE indeed.

I tried to use the earlymaps for this but I couldn't get it to
work, besides it only supports some 16 pages so not good
approach since the early boot maps a whopping 2MB.

Yours,
Linus Walleij