[PATCH u-boot-marvell 00/16] tools: kwbimage: Load address fixes

[Stefan Roese]

On 1/12/22 12:34, Pali Rohár wrote:
> On Wednesday 12 January 2022 12:06:22 Stefan Roese wrote:
>> Hi Pali,
>>
>> On 1/12/22 11:55, Stefan Roese wrote:
>>> On 1/12/22 11:41, Pali Rohár wrote:
>>>> On Wednesday 12 January 2022 08:26:10 Stefan Roese wrote:
>>>>> Hi Pali,
>>>>>
>>>>> while testing with this patchset (amongst others), I get this error
>>>>> while building for "theadorable_debug":
>>>>>
>>>>> $ make theadorable_debug_defconfig
>>>>> $ make -s -j20
>>>>> Invalid LOAD_ADDRESS 0x40004030 for BINARY spl/u-boot-spl.bin
>>>>> with 0 args.
>>>>> Address must be 4-byte aligned and in range 0x40000028-0x40000424
>>>>> .make: *** [Makefile:1448: u-boot-spl.kwb] Error 1
>>>>> make: *** Deleting file 'u-boot-spl.kwb'
>>>>>
>>>>> Could you please take a look on whats going wrong here? Do I need to
>>>>> change CONFIG_SPL_TEXT_BASE now? And if yes, why?
>>>>
>>>> Hello!
>>>>
>>>> If everything is working fine without this patch series then address
>>>> must not be changed.
>>>
>>> Yes, everything works just fine without out this series.
>>>
>>>> Now I realized that some boards have text base address 0x40004030 and
>>>> some have 0x40000030. When I was looking it during writing this patch
>>>> series, I have not spotted that there is different digit "4" in the
>>>> middle... And therefore I was in impression that every 32-bit Armada has
>>>> base address 0x40000000 (increased by magic constant 0x30 which is
>>>> explained in one of the patches).
>>>
>>> I see.
>>>
>>>> So now I need to figure out, why some boards have base address
>>>> 0x40004000 and some have 0x40000000. It it somewhere documented this
>>>> magic address? Or do you know source of this address for your board?
>>>
>>> This is so loooong ago that I worked on this. I can only guess, that the
>>> are up to offset 0x4000 was reserved by/for the BootROM.
>>
>> This info is still in some of the config headers:
>>
>> /*
>>   * Memory layout while starting into the bin_hdr via the
>>   * BootROM:
>>   *
>>   * 0x4000.4000 - 0x4003.4000	headers space (192KiB)
>>   * 0x4000.4030			bin_hdr start address
>>   * 0x4003.4000 - 0x4004.7c00	BootROM memory allocations (15KiB)
>>   * 0x4007.fffc			BootROM stack top
>>   *
>>   * The address space between 0x4007.fffc and 0x400f.fff is not locked in
>>   * L2 cache thus cannot be used.
>>   */
>>
>> HTP.
>>
>> Thanks,
>> Stefan
> 
> And now I found this information in old Marvell U-Boot fork from 2013:
> https://github.com/MarvellEmbeddedProcessors/u-boot-marvell/blob/u-boot-2013.01-armada-18.06/tools/marvell/bin_hdr/inc/common/soc_spec.h#L84-L92
> 
> #if defined(MV_TEST_PLATFORM)
> 	#define RAM_TOP			0x81004000 /*  Use PEX memory - (16KB for MMU table) */
> #elif defined(MV88F6710) || defined(MV88F78X60)
> 	#define RAM_TOP			0x40004000 /*  L2 cache 512KB - (16KB for MMU table) */
> #elif defined(MV88F66XX) || defined(MV88F68XX) || defined(MV88F672X) || defined(MV88F69XX)
> 	#define RAM_TOP			0x40000000 /*  L2 cache 512KB */
> #else
> 	#define RAM_TOP			0x40004000 /*  L2 cache 512KB */
> #endif
> 
> IIRC this is chip conversion table:
> 
> 88F6710 = A370
> MV78X60 = AXP
> 88F66xx = Avanta
> 88F672x = A375
> 88F68xx = A38x
> 88F69xx = A39x
> 
> So it looks like that only AXP and A370 use address 0x40004000, other
> platforms use 0x40000000. AXP and A370 are the last SoCs which use
> Marvell custom CPU cores Sheeva, all later have ARM A9 cores. Also in
> functional specifications for AXP and A370 is written that executable
> code needs to be aligned to 128-bit boundary and in later SoCs specs
> there is no written restriction, like this. On A385 I tested that this
> alignment is not really required. So for me it looks like that this 16kB
> reservation is needed for Marvell custom CPU cores only and is some CPU
> core specific.

Makes perfect sense, yes.

> The only suspicious thing is that in configs/db-88f6720_defconfig is
> defined CONFIG_SPL_TEXT_BASE=0x40004030 and this is A375 (not A370!).
> But now I found your commit 606576d54b673 ("arm: mvebu: Add basic
> support for Armada 375 eval board db-88f6720") where you introduced this
> #define CONFIG_SPL_TEXT_BASE 0x40004030 and wrote "the SPL U-Boot is not
> fully functional.". So with this base address SPL would never work. I
> know that Serdes+ddr3 init code is missing, so no SPL for this platform.

AFAIR, this port was done not that thoroughly, which would explain this
mismatch you describe above. And could perhaps be fixed by changing this
SPL_TEXT_BASE - if someone would like to invest some more time here.

> So how to solve the problem that kwbimage needs to know if is building
> image for platform with 16kB reserved for MMU table?
> 
> Should I add a new kwbimage command which specifies this information,
> like building image for Marvell CPU cores (Sheeva)? Or do you have other
> idea? With this information I can adjust code to enable 128-bit boundary
> restrictions only for these CPUs...

The first idea is to change this error to a warning, with some comment
that this might work on these specific AXP and A370 platforms. Another
idea is to add a 2nd valid address area.

Thanks,
Stefan