| Message ID | 1455201681-29972-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 11 February 2016 at 16:24, Laurentiu Tudor <laurentiu.tudor@nxp.com> wrote: > Hi Ard, > > On 02/11/2016 04:41 PM, Ard Biesheuvel wrote: >> Commit 66f51676a095 ("arm64: allow kernel Image to be loaded anywhere in >> physical memory") defers the assignment of memstart_addr to the point where >> all memory has been discovered and possibly clipped based on the size of >> the linear region and the presence of a mem= command line parameter. >> >> However, this results in __va() translations that have been performed up >> to this point to have been carried out with a preliminary, incorrect value >> of memstart_addr, and these values need to be fixed up. >> >> To make this more transparent, change the initial value of memstart_addr to >> PAGE_OFFSET so that __va() translations are effectively identity functions, >> and fix up the values of initrd_start and initrd_end before use. >> >> Tested-by: Laurentiu Tudor <laurentiu.tudor@nxp.com> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> >> This is the least ugly way to address this issue in the short term (since the >> offending patch is already in -next). The preferred way of dealing with this >> would be to parametrise early_init_dt_check_for_initrd() in such a way that >> we can decide per-arch whether it should perform the __va() translation or >> not. If this is the preferred approach also for the short term, I am happy >> to code that up as well. >> >> arch/arm64/mm/init.c | 20 +++++++++++++++----- >> 1 file changed, 15 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c >> index 3a9fc46cbf80..bb53236e54d9 100644 >> --- a/arch/arm64/mm/init.c >> +++ b/arch/arm64/mm/init.c >> @@ -48,7 +48,13 @@ >> >> #include "mm.h" >> >> -phys_addr_t memstart_addr __read_mostly = 0; >> +/* >> + * Initialize memstart_addr to PAGE_OFFSET so that __va() translations >> + * performed before arm64_memblock_init() executes (i.e., assignments >> + * of initrd_start and initrd_end in the early FDT parsing code) are >> + * effectively identity functions. >> + */ >> +phys_addr_t memstart_addr __read_mostly = PAGE_OFFSET; >> phys_addr_t arm64_dma_phys_limit __read_mostly; >> >> #ifdef CONFIG_BLK_DEV_INITRD >> @@ -61,8 +67,8 @@ static int __init early_initrd(char *p) >> if (*endp == ',') { >> size = memparse(endp + 1, NULL); >> >> - initrd_start = (unsigned long)__va(start); >> - initrd_end = (unsigned long)__va(start + size); >> + initrd_start = start; >> + initrd_end = start + size; >> } >> return 0; >> } >> @@ -213,8 +219,12 @@ void __init arm64_memblock_init(void) >> */ >> memblock_reserve(__pa(_text), _end - _text); >> #ifdef CONFIG_BLK_DEV_INITRD >> - if (initrd_start) >> - memblock_reserve(__virt_to_phys(initrd_start), initrd_end - initrd_start); >> + if (initrd_start) { >> + memblock_reserve(initrd_start, initrd_end - initrd_start); >> + >> + initrd_start = phys_to_virt(initrd_start); >> + initrd_end = phys_to_virt(initrd_end); > > There are some warnings here so I think you need to cast these to 'unsigned long'. > > arch/arm64/mm/init.c: In function 'arm64_memblock_init': > arch/arm64/mm/init.c:225:16: warning: assignment makes integer from pointer without a cast [enabled by default] > initrd_start = phys_to_virt(initrd_start); > ^ > arch/arm64/mm/init.c:226:14: warning: assignment makes integer from pointer without a cast [enabled by default] > initrd_end = phys_to_virt(initrd_end); > ^ Thanks for catching that. I should have used the __phys_to_virt() variant instead. Thanks, Ard. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index 3a9fc46cbf80..bb53236e54d9 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -48,7 +48,13 @@ #include "mm.h" -phys_addr_t memstart_addr __read_mostly = 0; +/* + * Initialize memstart_addr to PAGE_OFFSET so that __va() translations + * performed before arm64_memblock_init() executes (i.e., assignments + * of initrd_start and initrd_end in the early FDT parsing code) are + * effectively identity functions. + */ +phys_addr_t memstart_addr __read_mostly = PAGE_OFFSET; phys_addr_t arm64_dma_phys_limit __read_mostly; #ifdef CONFIG_BLK_DEV_INITRD @@ -61,8 +67,8 @@ static int __init early_initrd(char *p) if (*endp == ',') { size = memparse(endp + 1, NULL); - initrd_start = (unsigned long)__va(start); - initrd_end = (unsigned long)__va(start + size); + initrd_start = start; + initrd_end = start + size; } return 0; } @@ -213,8 +219,12 @@ void __init arm64_memblock_init(void) */ memblock_reserve(__pa(_text), _end - _text); #ifdef CONFIG_BLK_DEV_INITRD - if (initrd_start) - memblock_reserve(__virt_to_phys(initrd_start), initrd_end - initrd_start); + if (initrd_start) { + memblock_reserve(initrd_start, initrd_end - initrd_start); + + initrd_start = phys_to_virt(initrd_start); + initrd_end = phys_to_virt(initrd_end); + } #endif early_init_fdt_scan_reserved_mem();
Commit 66f51676a095 ("arm64: allow kernel Image to be loaded anywhere in physical memory") defers the assignment of memstart_addr to the point where all memory has been discovered and possibly clipped based on the size of the linear region and the presence of a mem= command line parameter. However, this results in __va() translations that have been performed up to this point to have been carried out with a preliminary, incorrect value of memstart_addr, and these values need to be fixed up. To make this more transparent, change the initial value of memstart_addr to PAGE_OFFSET so that __va() translations are effectively identity functions, and fix up the values of initrd_start and initrd_end before use. Tested-by: Laurentiu Tudor <laurentiu.tudor@nxp.com> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- This is the least ugly way to address this issue in the short term (since the offending patch is already in -next). The preferred way of dealing with this would be to parametrise early_init_dt_check_for_initrd() in such a way that we can decide per-arch whether it should perform the __va() translation or not. If this is the preferred approach also for the short term, I am happy to code that up as well. arch/arm64/mm/init.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) -- 2.5.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel