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

[Christoph Müllner]

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?

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
> 
> 
>