[1/1] sunxi: CONFIG_INIT_SP_RELATIVE=y for Pine64 LTS

On 6/3/20 7:07 PM, Heinrich Schuchardt wrote:
> On 6/3/20 6:31 PM, Andr? Przywara wrote:
>> On 03/06/2020 16:47, Heinrich Schuchardt wrote:
>>
>> Hi Heinrich,
>>
>>> On 03.06.20 01:46, Andr? Przywara wrote:
>>>> On 02/06/2020 20:55, Tom Rini wrote:
>>>>
>>>> Hi,
>>>>
>>>>> On Tue, Jun 02, 2020 at 09:45:25PM +0200, Heinrich Schuchardt wrote:
>>>>>> On 6/2/20 5:51 PM, Tom Rini wrote:
>>>>>>> On Sun, May 31, 2020 at 10:43:00AM +0000, Heinrich Schuchardt wrote:
>>>>>>>
>>>>>>>> Booting pine64-lts_defconfig with either of CONFIG_RSA=y or CONFIG_LOG=y
>>>>>>>> fails if CONFIG_INIT_SP_RELATIVE is not set.
>>>>>>>>
>>>>>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>>>>>>> ---
>>>>>>>>  configs/pine64-lts_defconfig | 3 +++
>>>>>>>>  1 file changed, 3 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/configs/pine64-lts_defconfig b/configs/pine64-lts_defconfig
>>>>>>>> index ef108a1a31..b03bef01b1 100644
>>>>>>>> --- a/configs/pine64-lts_defconfig
>>>>>>>> +++ b/configs/pine64-lts_defconfig
>>>>>>>> @@ -1,5 +1,8 @@
>>>>>>>>  CONFIG_ARM=y
>>>>>>>> +CONFIG_INIT_SP_RELATIVE=y
>>>>>>>>  CONFIG_ARCH_SUNXI=y
>>>>>>>> +CONFIG_SYS_MALLOC_F_LEN=0x8000
>>>>>>>> +CONFIG_SPL_SYS_MALLOC_F_LEN=0x400
>>>>>>>>  CONFIG_SPL=y
>>>>>>>>  CONFIG_MACH_SUN50I=y
>>>>>>>>  CONFIG_SUNXI_DRAM_LPDDR3_STOCK=y
>>>>>>>
>>>>>>> Look at the option however.  This is something that all sunxi should be
>>>>>>> selecting.
>>>>>>
>>>>>> What indicates that all sunxi should select CONFIG_INIT_SP_RELATIVE?
>>>>>
>>>>> config INIT_SP_RELATIVE
>>>>>         bool "Specify the early stack pointer relative to the .bss section"
>>>>>         help
>>>>>           U-Boot typically uses a hard-coded value for the stack pointer
>>>>>           before relocation. Enable this option to instead calculate the
>>>>>           initial SP at run-time. This is useful to avoid hard-coding addresses
>>>>>           into U-Boot, so that it can be loaded and executed at arbitrary
>>>>>           addresses and thus avoid using arbitrary addresses at runtime.
>>>>>
>>>>>           If this option is enabled, the early stack pointer is set to
>>>>>           &_bss_start with a offset value added. The offset is specified by
>>>>>           SYS_INIT_SP_BSS_OFFSET.
>>>>>
>>>>> And that in turn tells me this is something that's done at the SoC
>>>>> level, especially for sunxi where things have been abstracted such that
>>>>> everyone (very roughly) shares the same board file, linker file, etc,
>>>>> and it's just defconfig+dts* files that change from board to board.
>>>>
>>>> I agree on that, this particular defconfig change is generic to all
>>>> sunxi boards and we should fix this properly for the whole platform.
>>>>
>>>>> So, to put it another way, what about the pine64-lts config is unique
>>>>> and causing this to be needed, but another arm64 sunxi platform would
>>>>> not?
>>>>
>>>> I think were Heinrich came from is selecting CONFIG_RSA for his
>>>> particular build. I guess this requires a bigger malloc area. Now this
>>>> somehow collides which how we calculate the stack pointer, but it is too
>>>> late here to really track this down ;-)
>>>>
>>>> Heinrich, can you please give an explanation what problem
>>>> CONFIG_INIT_SP_RELATIVE fixed and why? It seems like to cause some side
>>>> effect, but does not sound like the proper solution (because this is
>>>> more mean for the position independent build option).
>>>>
>>>> Cheers,
>>>> Andre
>>>>
>>>
>>> Hello Andr?,
>>>
>>> the problem is reproducible on the FriendlyARM NanoPi NEO Plus2. So Tom
>>> is right in that we should solve it for all Allwinner ARMv8 devices.
>>
>> Thanks for testing this, and yeah, I am not surprised. As Tom mentioned,
>> all (arm64) Allwinner boards are the same in those basic aspects.
>>
>>> I simply use a nanopi_neo_plus2_defconfig or pine64-lts_defconfig and
>>> add CONFIG_RSA=y.
>>>
>>> The result is that the whenever the BL31 loads main U-Boot the board
>>> falls back to SPL. This loops forever. See below.
>>>
>>> CONFIG_RSA=y is needed for signed UEFI variables and other use cases.
>>>
>>> But the problem is not specific to RSA. You get into the same kind of
>>> problems when selecting CONFIG_LOG=y.
>>>
>>> With CONFIG_INIT_SP_RELATIVE=y the problem disappears.
>>>
>>> As this all happens before we have a U-Boot console I was not able to
>>> debug further than what I wrote in the thread
>>> https://lists.denx.de/pipermail/u-boot/2020-May/414346.html. A
>>> DEBUG_UART configuration for sunxi would have been very helpful here.
>>
>> Many thanks for the info, that looks very helpful! I wasn't sure if the
>> problem is in the SPL (because you just increased the malloc area for
>> U-Boot proper), but it looks very much like it. I will dig out some
>> debug techniques I used in the past to see what's actually going on. The
>> code size for Allwinner arm64 SPL is very limited, so we had this issue
>> before, where the stack ran into the code (if it didn't already violate
>> the build time limit check).
>> I am just a bit puzzled how CONFIG_INIT_SP_RELATIVE fixes this, as it
>> looks to me that just this option alone is not enough to make this work.
>
> Why do you think there is a problem in SPL?
>
> SPL loads BL31.
> BL31 loads BL33 (main U-Boot)
> BL33 crashes
> the system reboots
>
> Please, read through the mail thread
> https://lists.denx.de/pipermail/u-boot/2020-May/414346.html and look at
> the reverted patch 17e117408571 ("drivers: crypto: rsa_mod_exp: avoid
> DM_FLAG_PRE_RELOC").
>
> Best regards
>
> Heinrich
>

With the following settings I was able to enable the DEBUG_UART on the
Pine64 A64 LTS board:

CONFIG_DEBUG_UART=y
CONFIG_DEBUG_UART_NS16550=y
CONFIG_DEBUG_UART_BASE=1c28000
CONFIG_DEBUG_UART_CLOCK=24000000
CONFIG_DEBUG_UART_SHIFT=2

Now we can see that alloc_simple() reports "alloc space exhausted" after
board_init_f() is called. This indicates that SYS_MALLOC_F_LEN is at the
root of the problem.

This is the output for pine64-lts_defconfig with CONFIG_LOG=y:

U-Boot SPL 2020.07-rc3-00087-g88bd5b1793-dirty (Jun 06 2020 - 08:18:49
+0000)
DRAM: 2048 MiB
Trying to boot from MMC1
NOTICE:  BL31: v2.2():v2.2-1138-g78460ced4
NOTICE:  BL31: Built : 05:50:47, Apr  7 2020
NOTICE:  BL31: Detected Allwinner A64/H64/R18 SoC (1689)
NOTICE:  BL31: Found U-Boot DTB at 0x4092100, model: Pine64 LTS
INFO:    ARM GICv2 driver initialized
INFO:    Configuring SPC Controller
INFO:    PMIC: Probing AXP803 on RSB
INFO:    PMIC: dcdc1 voltage: 3.300V
INFO:    PMIC: dcdc5 voltage: 1.200V
INFO:    PMIC: dcdc6 voltage: 1.100V
INFO:    PMIC: dldo1 voltage: 3.300V
INFO:    PMIC: Enabling DC SW
INFO:    BL31: Platform setup done
INFO:    BL31: Initializing runtime services
INFO:    BL31: cortex_a53: CPU workaround for 843419 was applied
INFO:    BL31: cortex_a53: CPU workaround for 855873 was applied
NOTICE:  PSCI: System suspend is unavailable
INFO:    BL31: Preparing for EL3 exit to normal world
INFO:    Entry point address = 0x4a000000
INFO:    SPSR = 0x3c9
Entering board_init_f()
alloc_simple() alloc space exhausted
alloc_simple() alloc space exhausted
alloc_simple() alloc space exhausted
alloc_simple() alloc space exhausted
No serial driver found
resetting ...
alloc_simple() alloc space exhausted

U-Boot SPL 2020.07-rc3-00087-g88bd5b1793-dirty (Jun 06 2020 - 08:18:49
+0000)
DRAM: 2048 MiB
Trying to boot from MMC1

When changing the value of CONFIG_SYS_MALLOC_F_LEN this does not lead to
recompiling common/dlmalloc.c. This is what put me on the wrong track
(with CONFIG_INIT_SP_RELATIVE=y).

Increasing CONFIG_SYS_MALLOC_F_LEN is all that is needed. This change is
sufficient:

Heinrich