[PATCH] arm64: defer __va() translations of initrd_start and initrd_end

[Ard Biesheuvel]

On 11 February 2016 at 16:24, Laurentiu Tudor <laurentiu.tudor at 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 at nxp.com>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at 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.