On Tue, May 14, 2019 at 02:05:43PM -0700, Stephen Boyd wrote: > Quoting Hsin-Yi Wang (2019-05-13 04:14:32) > > On Mon, May 13, 2019 at 4:59 PM Mike Rapoport <rppt@xxxxxxxxxxxxx> wrote: > > > > > > > > This makes the fdt mapped without the call to meblock_reserve(fdt) which > > > makes the fdt memory available for memblock allocations. > > > > > > Chances that is will be actually allocated are small, but you know, things > > > happen. > > > > > > IMHO, instead of calling directly __fixmap_remap_fdt() it would be better > > > to add pgprot parameter to fixmap_remap_fdt(). Then here and in kaslr.c it > > > can be called with PAGE_KERNEL and below with PAGE_KERNEL_RO. > > > > > > There is no problem to call memblock_reserve() for the same area twice, > > > it's essentially a NOP. > > > > > Thanks for the suggestion. Will update fixmap_remap_fdt() in next patch. > > > > However, I tested on some arm64 platform, if we also call > > memblock_reserve() in kaslr.c, would cause warning[1] when > > memblock_reserve() is called again in setup_machine_fdt(). The warning > > comes from https://elixir.bootlin.com/linux/latest/source/mm/memblock.c#L601 > > ``` > > if (type->regions[0].size == 0) { > > WARN_ON(type->cnt != 1 || type->total_size); > > ... > > ``` > > > > Call memblock_reserve() multiple times after setup_machine_fdt() > > doesn't have such warning though. > > > > I didn't trace the real reason causing this. But in this case, maybe > > don't call memblock_reserve() in kaslr? > > > > Why not just have fixmap_remap_fdt() that maps it as RW and reserves > memblock once, and then call __fixmap_remap_fdt() with RO after > early_init_dt_scan() or unflatten_device_tree() is called? Why the > desire to call memblock_reserve() twice or even three times? There's no desire to call memblock_reserve() twice. It's just that leaving the call for it in kaslr rather than in setup_arch() may end up with unreserved FDT because kaslr was disabled or even compiled out. I've suggested to use fixmap_remap_fdt() everywhere because IMHO this improves readability and allows to un-export __fixmap_remap_fdt(). -- Sincerely yours, Mike.
