From: Will Deacon <will.deacon@arm.com> To: pierre kuo <vichy.kuo@gmail.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: Mon, 1 Apr 2019 16:38:10 +0100 [thread overview] Message-ID: <20190401153810.GC6884@fuggles.cambridge.arm.com> (raw) In-Reply-To: <CAOVJa8Eefs7cEneNwV0VCrr8GLVb0AweJEb0Qo4TucgXTNuNWg@mail.gmail.com> [+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. Why isn't it just as simple as the diff below? Will --->8 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);
WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will.deacon@arm.com> To: pierre kuo <vichy.kuo@gmail.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: Mon, 1 Apr 2019 16:38:10 +0100 [thread overview] Message-ID: <20190401153810.GC6884@fuggles.cambridge.arm.com> (raw) In-Reply-To: <CAOVJa8Eefs7cEneNwV0VCrr8GLVb0AweJEb0Qo4TucgXTNuNWg@mail.gmail.com> [+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. Why isn't it just as simple as the diff below? Will --->8 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); _______________________________________________ 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-01 15:38 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 [this message] 2019-04-01 15:38 ` Will Deacon 2019-04-03 16:44 ` pierre kuo 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=20190401153810.GC6884@fuggles.cambridge.arm.com \ --to=will.deacon@arm.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=vichy.kuo@gmail.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.