Hi Ard,

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

On Mon, Oct 5, 2020 at 9:15 AM Ard Biesheuvel [off-list ref] wrote:

On Sun, 4 Oct 2020 at 22:50, Linus Walleij [off-list ref] 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.

quoted

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel