| Message ID | 20190704094440.30896-1-andy.yan@rock-chips.com |
|---|---|
| State | Accepted |
| Commit | 619f002db864ef8caa30863bde62df5c651a7312 |
| Delegated to: | Kever Yang |
| Headers | show |
| Series | [U-Boot] rockchip: make_fit_atf.py: fix loadables property set error | expand |
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:
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: >
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: >> > >
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: >>> >> >> > >
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: >>
> 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@lists.denx.de > https://lists.denx.de/listinfo/u-boot
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@lists.denx.de >> https://lists.denx.de/listinfo/u-boot
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@lists.denx.de >>> https://lists.denx.de/listinfo/u-boot > > >
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@lists.denx.de >>>> https://lists.denx.de/listinfo/u-boot >> >
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:
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(-)