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

[Pali Rohár]

On Wednesday 12 January 2022 16:06:53 Stefan Roese wrote:
> On 1/12/22 15:16, Pali Rohár wrote:
> > On Wednesday 12 January 2022 14:53:23 Stefan Roese wrote:
> > > 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
> > 
> > I do not think that changing error to warning would help here. With
> > these changes kwbimage really try to sets load address to specified one.
> > And if kwbimage thinks that base address is on different location then
> > it would generate something unbootable.
> 
> Makes sense, okay.
> 
> > Now I played with it... could you try to test following diff? It
> > propagates information about CPU into kwbimage and then it do different
> > checks and use different base address based on CPU.
> 
> Looks good, please see the boot log here:
> 
> $ ./tools/kwboot -B 115200 -b u-boot-spl.kwb -t
...
> U-Boot 2022.01-00487-g655eb7d4bc34-dirty (Jan 12 2022 - 16:02:28 +0100)
> 
> SoC:   MV78260-B0 at 1333 MHz
> DRAM:  2 GiB (667 MHz, 64-bit, ECC not enabled)
> Core:  24 devices, 14 uclasses, devicetree: separate
> Loading Environment from SPIFlash... SF: Detected m25p128 with page size 256
> Bytes, erase size 256 KiB, total 16 MiB
> OK
> Model: Marvell Armada XP theadorable
> pcie0.0: Link up
> pcie2.0: Link up
> Net:   eth0: ethernet at 70000
> ...
> 
> :)
> 
> Thanks,
> Stefan

Perfect! So I will integrate that diff into patches in this series and
send V2.