[U-Boot] [PATCH] rockchip: make_fit_atf.py: fix loadables property set error【请注意，邮件由mark.kettenis at sibelius.xs4all.nl代发】

[Andy Yan]

Hi Christoph:

On 2019/7/7 上午1:32, Christoph Müllner wrote:
> On 7/6/19 5:02 PM, Kever Yang wrote:
>> Hi Mark,
>>
>>
>> On 07/05/2019 08:03 PM, Mark Kettenis wrote:
>>>> From: Kever Yang<kever.yang at rock-chips.com>
>>>> Date: Fri, 5 Jul 2019 19:38:42 +0800
>>>>
>>>> Hi Christoph,
>>>>
>>>>
>>>> On 07/05/2019 05:15 PM, Christoph Müllner wrote:
>>>>> On 04.07.19 11:44, Andy Yan wrote:
>>>>>> Commit b238e4b00ced ("rockchip: Cleanup of make_fit_atf.py.") set
>>>>>> firmware = "atf_1";
>>>>>> loadables = "uboot","atf_1","atf_2";
>>>>>>
>>>>>> Actually it should be:
>>>>>> firmware = "atf_1";
>>>>>> loadables = "uboot","atf_2","atf_3";
>>>>> Does "atf_1" not need to be among loadables as well?
>>>> The "atf_1" not need to be among loadables, or else it will be loaded twice.
>>>> The loadables suppose to be those firmware not have a entry but need to
>>>> load by SPL.
>>>>> My version of the script produces:
>>>>>
>>>>> loadables = "uboot","atf_1","atf_2","atf_3";
>>>>>
>>>>> And with that I was able to boot mainline ATF.
>>>> The mainline ATF now have 4 segments instead of 3, I don't know why
>>>> there is a stand alone 'GNU_STACK' section, so it's able to boot there is
>>>> one extra segment help, it loads all atf_1, atf_2 and atf_3. Rockchip elf
>>>> and older mainline ATF do not have the extra 'GNU_STACK' section, and
>>>> the last atf_3 is not loaded, which lead to the ATF not able to run.
>>>> Program Headers:
>>>>    Type           Offset             VirtAddr           PhysAddr
>>>>                   FileSiz            MemSiz              Flags Align
>>>>    LOAD           0x0000000000000000 0x0000000000000000 0x0000000000000000
>>>>                   0x0000000000014058 0x0000000000044000  RWE 10000
>>>>    LOAD           0x0000000000020000 0x00000000ff3b0000 0x00000000ff3b0000
>>>>                   0x0000000000001f58 0x0000000000001f58  RWE 10000
>>>>    LOAD           0x0000000000030000 0x00000000ff8c0000 0x00000000ff8c0000
>>>>                   0x0000000000002000 0x0000000000003000  RWE 10000
>>>>    GNU_STACK      0x0000000000000000 0x0000000000000000 0x0000000000000000
>>>>                   0x0000000000000000 0x0000000000000000  RW     10
>>>>
>>>> I will apply this patch as a fix.
>>> That GNU_STACK segment is produced by your toolchain.  It doesn't
>>> really make sense to have it for standalone binaries like ATF, and
>>> there is no reason to load it.  Not all toolchains produce a GNU_STACK
>>> section.
>>>
>>> But the make_fit_atf.py script already checks that the segment type is
>>> PT_LOAD, so I don't understand what you're trying to fix here.  For me
>>> things seem to work fine with mainline u-boot and ATF 2.1 on both the
>>> rock64e and rockpro64.
>> Looks work fine does not means every thing is correct.
>>
>> The ATF2.1 works find because of the extra GNU_STACK, but the logical
>> is not correct.
>> For mainline ATF 2.1, there are for segments, and SPL loads:
>> - atf_1 as firmware;
>> - atf_1, atf_2, atf_3 as loadables;
>> atf_1 load twice loads twice which is not correct.
>>
>> For other ATF which not have extra GNU_STACK, eg, rockchip vendor atf at
>> [0],
>> the SPL loads:
>> - atf_1 as firmware;
>> - atf_1 and atf_2 as loadables;
>> atf_1 load twice and atf_3 is missing which the ATF is not able to boot.
> So with this patch applied, we require the GNU_STACK section to be
> present in the mainline ATF ELF?
>
> In other words:
> If I use a toolchain, which does not create the GNU_STACK section,
> or if I have this information stripped away, then the Python script
> does not work correctly anymore?
>
> Is this summary correct?


No. This is the situation with your patch(b238e4b00ced): After your 
patch applied, the script won't work correctly when there is no extra 
GNU_STACK segment.

When my fix applied, the script can work well with or without the extra 
GNU_STACK segment again.

> Thanks,
> Christoph
>
>> This patch fix this logic.
>>
>> Thanks,
>> - Kever
>>   
>> [0]https://github.com/rockchip-linux/rkbin/blob/master/bin/rk33/rk3399_bl31_v1.28.elf
>>
>>>>>> Signed-off-by: Andy Yan<andy.yan at rock-chips.com>
>>>>>> ---
>>>>>>
>>>>>>   arch/arm/mach-rockchip/make_fit_atf.py | 2 +-
>>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/arch/arm/mach-rockchip/make_fit_atf.py b/arch/arm/mach-rockchip/make_fit_atf.py
>>>>>> index 45ec105887..db0ae96ca8 100755
>>>>>> --- a/arch/arm/mach-rockchip/make_fit_atf.py
>>>>>> +++ b/arch/arm/mach-rockchip/make_fit_atf.py
>>>>>> @@ -94,7 +94,7 @@ def append_conf_section(file, cnt, dtname, segments):
>>>>>>       if segments != 0:
>>>>>>           file.write(',')
>>>>>>       for i in range(1, segments):
>>>>>> -        file.write('"atf_%d"' % (i))
>>>>>> +        file.write('"atf_%d"' % (i + 1))
>>>>>>           if i != (segments - 1):
>>>>>>               file.write(',')
>>>>>>           else:
>>>>>>
>>>> _______________________________________________
>>>> U-Boot mailing list
>>>> U-Boot at lists.denx.de
>>>> https://lists.denx.de/listinfo/u-boot
>>
>