diff mbox

[1/5] of/fdt: allow FDT virtual address outside of linear direct mapping

Message ID 1425380630-3684-2-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show

Commit Message

Ard Biesheuvel March 3, 2015, 11:03 a.m. UTC
The early FDT code reserves the physical region that contains the
FDT, and uses __pa() to calculate the FDT's physical address.
However, if the FDT is mapped outside of the linear direct mapping,
__pa() cannot be used.

So create a __weak default wrapper called fdt_virt_to_phys() around
__pa(), and use that instead. This allows architectures to drop in
their own virt/phys mapping for the FDT blob.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---

I am aware that __weak functions are generally frowned upon, but in this
case, I wonder if it is worth the trouble to add arch specific header files
so we can include them here.

 drivers/of/fdt.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

Ard Biesheuvel March 11, 2015, 8:34 a.m. UTC | #1
On 10 March 2015 at 22:47, Rob Herring <robh@kernel.org> wrote:
> On Tue, Mar 3, 2015 at 5:03 AM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>> The early FDT code reserves the physical region that contains the
>> FDT, and uses __pa() to calculate the FDT's physical address.
>> However, if the FDT is mapped outside of the linear direct mapping,
>> __pa() cannot be used.
>>
>> So create a __weak default wrapper called fdt_virt_to_phys() around
>> __pa(), and use that instead. This allows architectures to drop in
>> their own virt/phys mapping for the FDT blob.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>
>> I am aware that __weak functions are generally frowned upon, but in this
>> case, I wonder if it is worth the trouble to add arch specific header files
>> so we can include them here.
>>
>>  drivers/of/fdt.c | 14 +++++++++++++-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index 3a896c9aeb74..b10ce880000b 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -547,6 +547,18 @@ static int __init __fdt_scan_reserved_mem(unsigned long node, const char *uname,
>>  }
>>
>>  /**
>> + * fdt_virt_to_phys - translate a virtual address inside the FDT image
>> + *                    to its corresponding physical address
>> + *
>> + * This needs to be overridden by the architecture if the virtual mapping
>> + * of the FDT is separate from the linear direct mapping of system RAM
>> + */
>> +phys_addr_t __weak __init fdt_virt_to_phys(void *virt)
>> +{
>> +       return __pa(virt);
>> +}
>> +
>> +/**
>>   * early_init_fdt_scan_reserved_mem() - create reserved memory regions
>>   *
>>   * This function grabs memory from early allocator for device exclusive use
>> @@ -562,7 +574,7 @@ void __init early_init_fdt_scan_reserved_mem(void)
>>                 return;
>>
>>         /* Reserve the dtb region */
>> -       early_init_dt_reserve_memory_arch(__pa(initial_boot_params),
>> +       early_init_dt_reserve_memory_arch(fdt_virt_to_phys(initial_boot_params),
>
> This is already a weak function call, so I'd rather change
> early_init_dt_reserve_memory_arch to take the virt address and do the
> conversion inside it.
> Or we could just pass both addresses from the
> arch code to the core code.
>

Alternatively, could we just have a way to tell the core FDT code
/not/ to do the reservation at all? It would make sense in the
non-linear mapped FDT case to round up the reservation to the mapped
size so that the remainder is not mapped elsewhere with different
attributes, so it is not just the physical address but also the size
that is different, and it is trivial to add the memblock_reserve() to
the remapping code.
Ard Biesheuvel March 11, 2015, 11:48 a.m. UTC | #2
On 11 March 2015 at 09:34, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 10 March 2015 at 22:47, Rob Herring <robh@kernel.org> wrote:
>> On Tue, Mar 3, 2015 at 5:03 AM, Ard Biesheuvel
>> <ard.biesheuvel@linaro.org> wrote:
>>> The early FDT code reserves the physical region that contains the
>>> FDT, and uses __pa() to calculate the FDT's physical address.
>>> However, if the FDT is mapped outside of the linear direct mapping,
>>> __pa() cannot be used.
>>>
>>> So create a __weak default wrapper called fdt_virt_to_phys() around
>>> __pa(), and use that instead. This allows architectures to drop in
>>> their own virt/phys mapping for the FDT blob.
>>>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>>
>>> I am aware that __weak functions are generally frowned upon, but in this
>>> case, I wonder if it is worth the trouble to add arch specific header files
>>> so we can include them here.
>>>
>>>  drivers/of/fdt.c | 14 +++++++++++++-
>>>  1 file changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>>> index 3a896c9aeb74..b10ce880000b 100644
>>> --- a/drivers/of/fdt.c
>>> +++ b/drivers/of/fdt.c
>>> @@ -547,6 +547,18 @@ static int __init __fdt_scan_reserved_mem(unsigned long node, const char *uname,
>>>  }
>>>
>>>  /**
>>> + * fdt_virt_to_phys - translate a virtual address inside the FDT image
>>> + *                    to its corresponding physical address
>>> + *
>>> + * This needs to be overridden by the architecture if the virtual mapping
>>> + * of the FDT is separate from the linear direct mapping of system RAM
>>> + */
>>> +phys_addr_t __weak __init fdt_virt_to_phys(void *virt)
>>> +{
>>> +       return __pa(virt);
>>> +}
>>> +
>>> +/**
>>>   * early_init_fdt_scan_reserved_mem() - create reserved memory regions
>>>   *
>>>   * This function grabs memory from early allocator for device exclusive use
>>> @@ -562,7 +574,7 @@ void __init early_init_fdt_scan_reserved_mem(void)
>>>                 return;
>>>
>>>         /* Reserve the dtb region */
>>> -       early_init_dt_reserve_memory_arch(__pa(initial_boot_params),
>>> +       early_init_dt_reserve_memory_arch(fdt_virt_to_phys(initial_boot_params),
>>
>> This is already a weak function call, so I'd rather change
>> early_init_dt_reserve_memory_arch to take the virt address and do the
>> conversion inside it.
>> Or we could just pass both addresses from the
>> arch code to the core code.
>>
>
> Alternatively, could we just have a way to tell the core FDT code
> /not/ to do the reservation at all? It would make sense in the
> non-linear mapped FDT case to round up the reservation to the mapped
> size so that the remainder is not mapped elsewhere with different
> attributes, so it is not just the physical address but also the size
> that is different, and it is trivial to add the memblock_reserve() to
> the remapping code.

Actually, having the FDT region retain duplicate mappings of physical
pages that are not covered by the FDT memory reservation itself is not
recognized as a big deal, so memblock_reserve()'ing the actual size of
the FDT is fine.

But still, what you are proposing (I think) is update the prototype of
early_init_dt_scan() to take an additional physical address, which
would also involve updating a handful of architectures to modify the
call sites (yay). And the alternative to change
early_init_dt_reserve_memory_arch() to take a virtual address won't
work for high memory.

So what about just doing this instead?

@@ -574,7 +562,8 @@ void __init early_init_fdt_scan_reserved_mem(void)
                return;

        /* Reserve the dtb region */
-       early_init_dt_reserve_memory_arch(fdt_virt_to_phys(initial_boot_params),
+       if (!IS_ENABLED(CONFIG_OF_HAVE_VIRT_REMAPPED_FDT))
+               early_init_dt_reserve_memory_arch(__pa(initial_boot_params),
                                          fdt_totalsize(initial_boot_params),
                                          0);

This way, it is left up to the architecture to decide which memory
(and how much) to reserve.
diff mbox

Patch

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 3a896c9aeb74..b10ce880000b 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -547,6 +547,18 @@  static int __init __fdt_scan_reserved_mem(unsigned long node, const char *uname,
 }
 
 /**
+ * fdt_virt_to_phys - translate a virtual address inside the FDT image
+ *                    to its corresponding physical address
+ *
+ * This needs to be overridden by the architecture if the virtual mapping
+ * of the FDT is separate from the linear direct mapping of system RAM
+ */
+phys_addr_t __weak __init fdt_virt_to_phys(void *virt)
+{
+	return __pa(virt);
+}
+
+/**
  * early_init_fdt_scan_reserved_mem() - create reserved memory regions
  *
  * This function grabs memory from early allocator for device exclusive use
@@ -562,7 +574,7 @@  void __init early_init_fdt_scan_reserved_mem(void)
 		return;
 
 	/* Reserve the dtb region */
-	early_init_dt_reserve_memory_arch(__pa(initial_boot_params),
+	early_init_dt_reserve_memory_arch(fdt_virt_to_phys(initial_boot_params),
 					  fdt_totalsize(initial_boot_params),
 					  0);