[SPAM] Re: [PATCH v2] xilinx: zynqmp: Do not use 0 as spl bss start address

[Stefan Herbrechtsmeier]

Hi,

Am 14.06.2022 um 15:03 schrieb Michal Simek:
> Hi,
> 
> On 6/13/22 11:02, Stefan Herbrechtsmeier wrote:
>> [CAUTION: External Email]
>>
>> Hi Michal,
>>
>> Am 13.06.2022 um 10:21 schrieb Stefan Herbrechtsmeier:
>>> Hi Michal,
>>>
>>> Am 13.06.2022 um 09:20 schrieb Michal Simek:
>>>> Hi,
>>>>
>>>> On 6/10/22 18:48, Xavier Drudis Ferran wrote:
>>>>> El Fri, Jun 10, 2022 at 04:42:55PM +0200, Stefan Herbrechtsmeier deia:
>>>>>> Hi Michal,
>>>>>>
>>>>>> what is the default entry address for the aft / bl31.bin?
>>>>>>
>>>>>> I have a bl31.bin with an entry address of 0x1000 and this is inside
>>>>>> the
>>>>>> BSS.
>>>>>>
>>>>>
>>>>> Me too, load address at 0x1000, but for me in SPL text, not BSS.
>>>>>
>>>>> I have a litle customized, a little old TF-A  for rk3399 / Rock pi 4
>>>>> loading at address 0 with entry at 0x1000.
>>>>>
>>>>> But include/configs/rk3399_common.h sets my
>>>>> CONFIG_SPL_BSS_START_ADDR=0x400000, away from harm.
>>>>> I had problems booting anyway.
>>>>>
>>>>> Now I can load U-Boot from MMC with these patches
>>>>> https://lists.denx.de/pipermail/u-boot/2022-June/485497.html
>>>>>
>>>>> In particular
>>>>> CONFIG_ROCKCHIP_SPL_RESERVE_IRAM=0x15000
>>>>>
>>>>> This is defined in arch/arm/mach-rockchip/Kconfig and says it's
>>>>> to avoid conflicts with SPL text area, not BSS
>>>>>
>>>>> But I found other boards with CONFIG_ROCKCHIP_SPL_RESERVE_IRAM=0x4000,
>>>>> so I thought some low addresses where normal. I don't know.
>>>>>
>>>>> I had to modify the code loading from SPI because, unlike MMC code, it
>>>>> thought address 0 meant no destination (I can send those patches when
>>>>> I have them cleaner if anyone wants them).
>>>>>
>>>>> I just realised that I have CONFIG_SPL_TEXT_BASE=0x0.  I'm not finding
>>>>> where that's defined, maybe it's simply because it's not defined
>>>>> anywhere, so maybe the solution for me would be setting
>>>>> CONFIG_SPL_TEXT_BASE
>>>>> to 0x10000000 or something. Or maybe it needs to be at 0x0 because
>>>>> it is bootrom who is loading it, and it won't look where I define it?
>>>>> I can't remember whether I tried this.
>>>>>
>>>>> Maybe you can try to look at the size of a file bl31_0x00000000.bin
>>>>> that is generated when you build U-boot with BL31 pointing at your
>>>>> bl31.elf (check u-boot.its if that's not the name for you).
>>>>>
>>>>> Then set CONFIG_SPL_BSS_START_ADDR to that size + L (L= value of load
>>>>> property
>>>>> in entry atf_1 of u-boot.its). This should leave a hole at the 
>>>>> beginning
>>>>> of U-Boot to make room for your TF-A, and leave BSS elsewhere.
>>>>>
>>>>> The sources and build scripts for TF-A are public, so maybe one could
>>>>> look at what's the criteria for putting images at different addresses?
>>>>>
>>>>
>>>> I have no idea what rockchip has to do with memory layout on zynqmp.
>>>> Anyway SPL must be placed to OCM that's why text base is at 0xfffc0000
>>>> where OCM starts and which is also default address where a53s starts.
>>>>
>>>> TF-A can be placed to any address but address which I use in SPL flow
>>>> is 0xfffe5000. PetaLinux which uses FSBL is using different address
>>>> 0xfffea000 which is default in TF-A.
>>>> In DDR I normally use end of low ddr locations. It means 0x7fxxxxxx.
>>>>
>>>> And of course you can't place TFA to location which is used by SPL or
>>>> by u-boot or other SW.
>>>
>>> Thanks for the information. I will check why our TF-A is linked to the
>>> lower DDR and move it back to the default address.
>>
>> The TF-A was build with debug flag [1]:
>>
>>  > By default, the Arm-trusted firmware builds for OCM space at address
>>  > 0xFFFEA000. But, with DEBUG flag set to 1, it can't fit in OCM, so by
>>  > default with DEBUG=1, it builds for DDR location 0x1000 with build
>>  > flag DEBUG=1 mentioned while building.
>>
>> Either the CONFIG_SPL_BSS_START_ADDR or the default location of the TF-A
>> with DEBUG flag should be changed.
>>
>> [1]
>> https://xilinx-wiki.atlassian.net/wiki/spaces/A/pages/18842107/Arm+Trusted+Firmware 
>>
> 
> It is really question what should be default address with DEBUG=1.
> 
> I tried this
> 
> diff --git a/plat/xilinx/zynqmp/include/platform_def.h 
> b/plat/xilinx/zynqmp/include/platform_def.h
> index 9c1600a7b7e3..66bbf30a65a4 100644
> --- a/plat/xilinx/zynqmp/include/platform_def.h
> +++ b/plat/xilinx/zynqmp/include/platform_def.h
> @@ -40,8 +40,8 @@
>   # define BL31_BASE                     U(0xfffea000)
>   # define BL31_LIMIT                    U(0x100000000)
>   #else
> -# define BL31_BASE                     U(0x1000)
> -# define BL31_LIMIT                    U(0x7ffff)
> +# define BL31_BASE                     U(0xfff5a000)
> +# define BL31_LIMIT                    U(0x100000000)
>   #endif
>   #else
>   # define BL31_BASE                     (ZYNQMP_ATF_MEM_BASE)
> 
> 
> And with the latest TF-A this works fine.
> No problem to change TF-A code to place it to different addresses. I 
> think it should stay in OCM if possible or allocate space at the end of 
> of low ddr address. It means any 0x7fxxxxxx address should be fine.
> With any SPD on we won't be able to fit OCM anyway.

Thanks for your work. I think it should stay in OCM and we should use 
the lower DDR address for BSS.

Regards
   Stefan