On Mon, Aug 23, 2021 at 09:44:55AM -0500, Rob Herring wrote: > On Mon, Aug 23, 2021 at 8:10 AM Mike Rapoport <rppt@xxxxxxxxxx> wrote: > > > > On Mon, Aug 23, 2021 at 12:17:50PM +0200, Geert Uytterhoeven wrote: > > > Hi Mike, > > > > > > On Mon, Aug 16, 2021 at 7:52 AM Mike Rapoport <rppt@xxxxxxxxxx> wrote: > > > > On Wed, Aug 11, 2021 at 10:50:59AM +0200, Geert Uytterhoeven wrote: > > > > > Prepare for early_init_fdt_scan_reserved_mem() reserving the memory > > > > > occupied by an elf core header described in the device tree. > > > > > As arch_mem_init() calls early_init_fdt_scan_reserved_mem() before > > > > > mips_reserve_vmcore(), the latter needs to check if the memory has > > > > > already been reserved before. > > > > > > > > Doing memblock_reserve() for the same region is usually fine, did you > > > > encounter any issues without this patch? > > > > > > Does it also work if the same region is part of an earlier larger > > > reservation? I am no memblock expert, so I don't know. > > > I didn't run into any issues, as my MIPS platform is non-DT, but I > > > assume arch/arm64/mm/init.c:reserve_elfcorehdr() had the check for > > > a reason. > > > > The memory will be reserved regardless of the earlier reservation, the > > issue may appear when the reservations are made for different purpose. E.g. > > if there was crash kernel allocation before the reservation of elfcorehdr. > > > > The check in such case will prevent the second reservation, but, at least > > in arch/arm64/mm/init.c:reserve_elfcorehdr() it does not seem to prevent > > different users of the overlapping regions to step on each others toes. > > If the kernel has been passed in overlapping regions, is there > anything you can do other than hope to get a message out? Nothing really. I've been thinking about adding flags to memblock.reserved to at least distinguish firmware regions from the kernel allocations, but I never got to that. > > Moreover, arm64::reserve_elfcorehdr() seems buggy to me, because of there > > is only a partial overlap of the elfcorehdr with the previous reservation, > > the non-overlapping part of elfcorehdr won't get reserved at all. > > What do you suggest as the arm64 version is not the common version? I'm not really familiar with crash dump internals, so I don't know if resetting elfcorehdr_addr to ELFCORE_ADDR_ERR is a good idea. I think at least arm64::reserve_elfcorehdr() should reserve the entire elfcorehdr area regardless of the overlap. Otherwise it might get overwritten by a random memblock_alloc(). > Rob -- Sincerely yours, Mike.
