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

[Heinrich Schuchardt]

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