[U-Boot] [PATCH] rockchip: make_fit_atf.py: fix loadables property set error

[U-Boot] [PATCH] rockchip: make_fit_atf.py: fix loadables property set error

[Andy Yan]

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

Signed-off-by: Andy Yan <andy.yan@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:
-- 
2.17.1

[U-Boot] [PATCH] rockchip: make_fit_atf.py: fix loadables property set error

[Kever Yang]

Hi Christoph,

    Could you check this issue on your side?

    This is pretty important for rk3399, this will break the support for ATF

for rk3399 boards other than puma.


On 07/04/2019 05:44 PM, 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";
>
> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>
>  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] [PATCH] rockchip: make_fit_atf.py: fix loadables property set error

[Christoph Müllner]

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?
My version of the script produces:

loadables = "uboot","atf_1","atf_2","atf_3";

And with that I was able to boot mainline ATF.

> 
> Signed-off-by: Andy Yan <andy.yan@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] [PATCH] rockchip: make_fit_atf.py: fix loadables property set error

[Andy Yan]

Hi  Christoph:

On 2019/7/5 下午5:15, 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?
> My version of the script produces:
>
> loadables = "uboot","atf_1","atf_2","atf_3";

Will you please provide your ble31.elf ?

The current u-boot mainline(your version) produces :

loadables = "uboot","atf_1","atf_2";

from [0]
[0]https://github.com/rockchip-linux/rkbin/blob/master/bin/rk33/rk3399_bl31_v1.28.elf

>
> And with that I was able to boot mainline ATF.
>
>> Signed-off-by: Andy Yan <andy.yan@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] [PATCH] rockchip: make_fit_atf.py: fix loadables property set error

[Christoph Müllner]

Hi Andy,

On 05.07.19 12:06, Andy Yan wrote:
> Hi  Christoph:
> 
> On 2019/7/5 下午5:15, 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?
>> My version of the script produces:
>>
>> loadables = "uboot","atf_1","atf_2","atf_3";
> 
> Will you please provide your ble31.elf ?

I use mainline ATF (https://github.com/ARM-software/arm-trusted-firmware)
without any additional changes.

The bl31.elf/ATF can be built with the following instructions:

  make CROSS_COMPILE=aarch64-linux-gnu- PLAT=rk3399 bl31

Last time I used commit 8917380a for testing:
https://github.com/ARM-software/arm-trusted-firmware/commit/8917380a

I will provide a download link for the ELF file
in a follow-up off-list email to you.

Thanks,
Christoph

> 
> The current u-boot mainline(your version) produces :
> 
> loadables = "uboot","atf_1","atf_2";
> 
> from [0]
> [0]https://github.com/rockchip-linux/rkbin/blob/master/bin/rk33/rk3399_bl31_v1.28.elf
> 
>>
>> And with that I was able to boot mainline ATF.
>>
>>> Signed-off-by: Andy Yan <andy.yan@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] [PATCH] rockchip: make_fit_atf.py: fix loadables property set error

[Kever Yang]

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.

Thanks,
- Kever
>
>> Signed-off-by: Andy Yan <andy.yan@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] [PATCH] rockchip: make_fit_atf.py: fix loadables property set error

[Mark Kettenis]

> From: Kever Yang <kever.yang@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.

> >> Signed-off-by: Andy Yan <andy.yan@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

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

[Kever Yang]

Hi Mark,


On 07/05/2019 08:03 PM, Mark Kettenis wrote:
>> From: Kever Yang <kever.yang@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.

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

[U-Boot] [PATCH] rockchip: make_fit_atf.py: fix loadables property set error【请注意，邮件由mark.kettenis@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@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@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
> 
> 
> 

[U-Boot] [PATCH] rockchip: make_fit_atf.py: fix loadables property set error【请注意，邮件由mark.kettenis@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@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@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
>>
>