[U-Boot] [PATCH] libfdt: Add option to disable arch_fixup_fdt() calls

Alexander Graf
2016-06-10T12:34:47Z

On 10.06.16 14:31, Michal Simek wrote:
> On 10.6.2016 14:12, Alexander Graf wrote:
>>
>>
>> On 10.06.16 13:51, Michal Simek wrote:
>>> On 10.6.2016 13:13, Alexander Graf wrote:
>>>>
>>>>
>>>> On 10.06.16 13:07, Michal Simek wrote:
>>>>> On 9.6.2016 16:40, Alexander Graf wrote:
>>>>>> On 06/09/2016 04:32 PM, Michal Simek wrote:
>>>>>>> On 9.6.2016 16:29, Alexander Graf wrote:
>>>>>>>> On 06/09/2016 04:23 PM, Michal Simek wrote:
>>>>>>>>> Disable arch_fixup_fdt() calls for cases where U-Boot shouldn't update
>>>>>>>>> memory setup in DTB file.
>>>>>>>>> One example of usage of this option is to boot OS with different memory
>>>>>>>>> setup than U-Boot use.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>>>>> Could we instead just have the board file provide a fixup? It could then
>>>>>>>> also fix up the efi memory map.
>>>>>>> Not sure what exactly you are asking for.
>>>>>>> Do you mean to add fixup function to board file and overwrite default
>>>>>>> one?
>>>>>>
>>>>>> You have to touch some other code anyway to make this work with a
>>>>>> particular board, right? In that case, you can as well add a function to
>>>>>> the board file that explicitly provides a different, known good memory map.
>>>>>
>>>>> I cant' see the reason to touch particular board. In past AMP solution
>>>>> where one core use the part of memory and second another part was the
>>>>> reason I needed this.
>>>>> For ARM64 case as you know from arm IRC I was trying to boot Linux from
>>>>> memory above 32bit space and for these tests I need to convince u-boot
>>>>> not to touch dtb memory setup.
>>>>>
>>>>> But for the same board I want to use standard behavior but for some case
>>>>> this needs to be enabled.
>>>>
>>>> For that particular case, can you try to see whether you can convince
>>>> U-Boot to just run in high memory instead? That would make things much
>>>> more consistent IMHO.
>>>>
>>>> Giving U-Boot and Linux different views of the memory map is really just
>>>> asking for trouble. You'd have to make sure that you flush your caches
>>>> for example so that Linux doesn't suddenly get memory overwritten from
>>>> stale cache entries on the low memory even though Linux is already
>>>> accessing the high addresses.
>>>
>>> For systems where you have one main memory and standard usage model with
>>> OS I agree that having this option enabled is bring more problems.
>>> But for our use cases you can add memory controller to PL and it will be
>>> ready when bitstream is loaded which can be done after u-boot is loaded.
>>> It means u-boot have to work with different memory setup then Linux
>>> later on.
>>
>> Oh, I see. So U-Boot actually uses completely different memory? But that
>> means that the original memory is also still there, doesn't it? Wouldn't
>> that mean we'd merely have to extend the bdinfo?
>
> It can be completely different memory or just part can be shared.
> Depends on use case.
> Also you can run just SW on particular core/cores.
>
> I don't think that there is need for bootloader to know all stuff about
> system. For me bootloader is here to help me to bring my app (or better
> OS with APP) how I like.
> And doing smart stuff is good but not for complicated cases that's why I
> would like to have a button to disable it.
>
> I can imagine for zcu102 which you also have to read SPD from memory and
> based on that setup memory sizes and push this setting to OS as very
> useful but running this standard OS style is just one particular use case.
> R5s on the chip + the whole PL with possible cpus there requires
> different behavior.
>
>>
>>> Caches should be flushed before u-boot pass control to Linux that's why
>>> there shouldn't be any stale entries.
>>>
>>> For ZynqMP you can partitioned memory that part of it goes to A53s and
>>> part to R5s and u-boot is capable to boot both cores.
>>
>> Hm, maybe I'm not fully grasping the exact scenario you're envisioning.
>
> :-)
>
>>
>>>> For testing, sure, but to actually make use of it I'd rather see a clean
>>>> solution.
>>>
>>> Do we have any experimental Kconfig entry which I can use for it?
>>
>> Uh, the patch you posted I guess. Just make sure people don't set it if
>> they don't know *exactly* what they're getting themselves into.
>
> It is not enabled by default but I am happy to add any other more
> advance dependency.

I don't think there's a need for an advanced dependency. Just a more
explicit warning in the help text should be enough.

Alex