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

[Stefan Herbrechtsmeier]

Hi Michal,

Am 14.06.2022 um 17:49 schrieb Michal Simek:
> 
> 
> On 6/14/22 17:34, Stefan Herbrechtsmeier wrote:
>> 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.
> 
> that sort of mean that you want the change above to be upstream. Do you 
> want me to send the patch or do you want to send it patch there?

Would be nice if you could send the patch upstream.