From: pierre kuo <vichy.kuo@gmail.com> To: Will Deacon <will.deacon@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com>, Steven Price <steven.price@arm.com>, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Florian Fainelli <f.fainelli@gmail.com>, ard.biesheuvel@linaro.org Subject: Re: [PATCH v2 1/1] initrd: move initrd_start calculate within linear mapping range check Date: Thu, 4 Apr 2019 00:44:25 +0800 [thread overview] Message-ID: <CAOVJa8EcjQO2GuojwBxfFG25+T-z6BR6ytZBdcV2W2XusyW08g@mail.gmail.com> (raw) In-Reply-To: <20190401153810.GC6884@fuggles.cambridge.arm.com> 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.
WARNING: multiple messages have this Message-ID (diff)
From: pierre kuo <vichy.kuo@gmail.com> To: Will Deacon <will.deacon@arm.com> Cc: Florian Fainelli <f.fainelli@gmail.com>, ard.biesheuvel@linaro.org, Catalin Marinas <catalin.marinas@arm.com>, linux-kernel@vger.kernel.org, Steven Price <steven.price@arm.com>, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2 1/1] initrd: move initrd_start calculate within linear mapping range check Date: Thu, 4 Apr 2019 00:44:25 +0800 [thread overview] Message-ID: <CAOVJa8EcjQO2GuojwBxfFG25+T-z6BR6ytZBdcV2W2XusyW08g@mail.gmail.com> (raw) In-Reply-To: <20190401153810.GC6884@fuggles.cambridge.arm.com> 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. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-04-03 16:44 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-03-14 3:20 [PATCH v2 1/1] initrd: move initrd_start calculate within linear mapping range check pierre Kuo 2019-03-14 3:20 ` pierre Kuo 2019-03-18 3:06 ` pierre kuo 2019-03-18 3:06 ` pierre kuo 2019-03-19 15:31 ` Catalin Marinas 2019-03-19 15:31 ` Catalin Marinas 2019-03-31 15:14 ` pierre kuo 2019-03-31 15:14 ` pierre kuo 2019-04-01 14:59 ` pierre kuo 2019-04-01 14:59 ` pierre kuo 2019-04-01 15:38 ` Will Deacon 2019-04-01 15:38 ` Will Deacon 2019-04-03 16:44 ` pierre kuo [this message] 2019-04-03 16:44 ` pierre kuo 2019-04-03 17:24 ` Will Deacon 2019-04-03 17:24 ` Will Deacon 2019-04-03 17:27 ` Florian Fainelli 2019-04-03 17:27 ` Florian Fainelli 2019-04-08 16:26 ` pierre kuo 2019-04-08 16:26 ` pierre kuo
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAOVJa8EcjQO2GuojwBxfFG25+T-z6BR6ytZBdcV2W2XusyW08g@mail.gmail.com \ --to=vichy.kuo@gmail.com \ --cc=ard.biesheuvel@linaro.org \ --cc=catalin.marinas@arm.com \ --cc=f.fainelli@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=steven.price@arm.com \ --cc=will.deacon@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.