hi Will:
>
> [+Ard in case I'm missing something]
>
> On Mon, Apr 01, 2019 at 10:59:53PM +0800, pierre kuo wrote:
> > > > With CONFIG_RANDOMIZE_BASE we can get a further change to memstart_addr
> > > > after the place where you moved the initrd_{start,end} setting, which
> > > > would result in a different value for __phys_to_virt(phys_initrd_start).
> > >
> > > I found what you mean, since __phys_to_virt will use PHYS_OFFSET
> > > (memstart_addr) for calculating.
> > > How about moving CONFIG_RANDOMIZE_BASE part of code ahead of
> > > CONFIG_BLK_DEV_INITRD checking?
> > >
> > > That means below (d) moving ahead of (c)
> > > prvious:
> > > if (memstart_addr + linear_region_size < memblock_end_of_DRAM()) {} ---(a)
> > > if (memory_limit != PHYS_ADDR_MAX) {}            ---(b)
> > > if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) && phys_initrd_size) {} ---(c)
> > > if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {} ---(d)
> > >
> > > now:
> > > if (memstart_addr + linear_region_size < memblock_end_of_DRAM()) { ---(a)
> > > if (memory_limit != PHYS_ADDR_MAX) {}              ----------------(b)
> > > if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {}  --------------(d)
> > > if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) && phys_initrd_size) {}  ---(c)
> > >
> >
> > After tracing the kernel code,
> > is it even possible to move CONFIG_RANDOMIZE_BASE part of code ahead
> > of memory_limit?
> >
> > that mean the flow may look like below:
> > now2:
> > if (memstart_addr + linear_region_size < memblock_end_of_DRAM()) {} ---(a)
> > if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {} ---(d)
> > if (memory_limit != PHYS_ADDR_MAX) {}            ---(b)
> > if (IS_ENABLED(CONFIG_BLK_DEV_INITRD) && phys_initrd_size) {} ---(c)
> >
> > in (b), the result of __pa_symbol(_text), memory_limit,
> > memblock_mem_limit_remove_map and  memblock_add
> > are not depended on memsart_addr.
> > So the now2 flow can grouping modification of memstart_address, put
> > (a) and (d) together.
>
> I'm afraid that you've lost me with this.
welcome for ur kind suggestion ^^

>Why isn't it just as simple as
> the diff below?
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index c29b17b520cd..ec3487c94b10 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -377,7 +377,7 @@ void __init arm64_memblock_init(void)
>                          base + size > memblock_start_of_DRAM() +
>                                        linear_region_size,
>                         "initrd not fully accessible via the linear mapping 
> -- please check your bootloader ...\n")) {
> -                       initrd_start = 0;
> +                       phys_initrd_size = 0;
>                 } else {
>                         memblock_remove(base, size); /* clear MEMBLOCK_ flags 
> */
>                         memblock_add(base, size);

This patch will also fix the issue, but it still needs 2 "if
comparisions" for getting initrd_start/initrd_end.
By possible grouping modification of memstart_address, and put
initrd_start/initrd_end to be calculated only when linear mapping check
pass. Maybe (just if) can let the code be more concise.

Sincerely appreciate all of yours great comment.

Reply via email to