[U-Boot] [RFC PATCH 0/5] move boot0 hook in the beginning for armv7

Fri Jun 9 12:59:36 UTC 2017

Simon,

> On 09 Jun 2017, at 14:27, Simon Glass <sjg at chromium.org> wrote:
> 
> Hi Kever,
> 
> On 7 June 2017 at 04:55, Kever Yang <kever.yang at rock-chips.com> wrote:
>> Simon,
>> 
>> 
>> 
>> On 06/07/2017 11:15 AM, Simon Glass wrote:
>>> 
>>> Hi Kever,
>>> 
>>> On 6 June 2017 at 20:41, Kever Yang <kever.yang at rock-chips.com> wrote:
>>>> 
>>>> Simon,
>>>> 
>>>> 
>>>> 
>>>> On 06/07/2017 05:08 AM, Simon Glass wrote:
>>>>> 
>>>>> Hi Kever,
>>>>> 
>>>>> On 31 May 2017 at 04:50, Kever Yang <kever.yang at rock-chips.com> wrote:
>>>>>> 
>>>>>> I think the boot0 hook is suppose to add some data in the very
>>>>>> beginning
>>>>>> of the SPL image, am I right?
>>>>>> 
>>>>>> Rockchip SoCs bootrom design is like this:
>>>>>> - First 2KB or 4KB internal memory is for bootrom stack and heap;
>>>>>> - Then the first 4-byte suppose to be a TAG like 'RK33';
>>>>>> - The the following memory address end with '0004' is the first
>>>>>>    instruction load and running by bootrom;
>>>>>> 
>>>>>> Example for RK3288:
>>>>>> Before this patch set, the SPL_TEXT_BASE is ff704004, and image write
>>>>>> to
>>>>>> media device after mkimage like this:
>>>>>> 
>>>>>> ff704000: 32334b52 00000000 00000000 00000000    RK32............
>>>>>> ff704010: 00000000 00000000 00000000 00000000    ................
>>>>>> ff704020: ea00000f e59ff014 e59ff014 e59ff014    ................
>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>> 
>>>>>> Where the first instruction from bootrom is '00000000', which is a
>>>>>> undefined instruction.
>>>>>> The '_start' and 'reset' have to align to 0x20 for the requirement of
>>>>>> VBAR, the memory offset '004'~'01c' are filled with '00000000'.
>>>>>> 
>>>>>> We can use the boot0 hook to fix this issue, after this patch set,
>>>>>> the SPL_TEXT_BASE is ff704000 and image write to media device after
>>>>>> mkimage like this:
>>>>>> 
>>>>>> ff704000: 32334b52 ea00001d e320f000 e320f000    RK32...... ... .
>>>>>> ff704010: e320f000 e320f000 e320f000 e320f000    .. ... ... ... .
>>>>>> ff704020: ea000016 e59ff014 e59ff014 e59ff014    ................
>>>>>> ff704030: e59ff014 e59ff014 e59ff014 e59ff014    ................
>>>>>> 
>>>>>> The first instruction from bootrom is a 'b reset', and memory of
>>>>>> '008'~'01c' are filled with 'nop' instruction.
>>>>>> 
>>>>>> This patch set does not provide patch for socfpga, bcm and sunxi SoCs
>>>>>> which also
>>>>>> enable BOOT0_HOOK, so this is a RFC patch, please advice how to make it
>>>>>> compatible with those three platforms.
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> Kever Yang (5):
>>>>>>    armv7: move boot hook before '_start'
>>>>>>    rockchip: boot0: align to 0x20 for armv7 '_start'
>>>>>>    rockchip: enable BOOT0_HOOK for SoCs
>>>>>>    rockchip: configs: use aligned address for SPL_TEXT_BASE
>>>>>>    rockchip: mkimage: use spl_boot0 for all Rockchip SoCs
>>>>>> 
>>>>>>   arch/arm/include/asm/arch-rockchip/boot0.h |  9 ++++++++-
>>>>>>   arch/arm/lib/vectors.S                     | 19 ++++++++++---------
>>>>>>   arch/arm/mach-rockchip/Kconfig             |  3 +++
>>>>>>   include/configs/rk3036_common.h            |  2 +-
>>>>>>   include/configs/rk3288_common.h            |  2 +-
>>>>>>   tools/rkcommon.c                           |  8 ++++----
>>>>>>   6 files changed, 27 insertions(+), 16 deletions(-)
>>>>>> 
>>>>>> --
>>>>>> 1.9.1
>>>>>> 
>>>>> Do will still need this series now that (I think) we have a fix for
>>>>> the return-to-brom feature in u-boot-rockchip/master?
>>>> 
>>>> 
>>>> Could you point me out exactly which fix do you talking about?
>>> 
>>> These ones:
>>> 
>>> a079e52d35 rockchip: mkimage: set init_boot_size to avoid confusing the
>>> boot ROM
>>> ee2c63912b rockchip: mkimage: force 2KB alignment for init_size
>>> 99c700c794 rockchip: mkimage: add support for verify_header/print_header
>>> 
>>>> This is not about return-to-brom, it's about the first instruction from
>>>> Bootrom to SPL.
>>>> So this is need for all Rockchip armv7 SoCs.
>>> 
>>> OK, how did we survive before? What has changed to make this series
>>> needed?
>> 
>> 
>> After check with JTAG, I find that I'm wrong with cmd code '00000000',
>> this is 'andeq r0, r0, r0', but not undefined in armv7, so it can work.
>> 
>> I still want this patch set applied because it's better to make all the
>> Rockchip's
>> SPL have the same format(with 4-byte TAG space reserved), and the ddr binary
>> from Rockchip always have pre-padding 4-byte TAG, with this patch set, we
>> can replace each other easily and work with one mkimage tool.
> 
> I'm not sure how to apply this since on the other thread[1] Marek says
> it will break socfpga.

To me it looks as if we need to fix the BOOT0 handling across all ARMv7
platforms, as it looks as if the current implementation and its documentation
contradict each other.

Here’s how BOOT0 was intended:

1. from Kconfig:
          If the SoC's BOOT0 requires a header area filled with (magic)
          values, then choose this option, and create a define called
          ARM_SOC_BOOT0_HOOK which contains the required assembler
          preprocessor code.

2. from the code in arch/arm/lib/vectors.S:
/*
 * Various SoCs need something special and SoC-specific up front in
 * order to boot, allow them to set that in their boot0.h file and then
 * use it here.
 */

Can we just resolve this by changing arch/arm/lib/vectors.S to replace
the entire content starting after the “_start:” label with the BOOT0 hook,
if one is defined?
This would then make it the responsibiliy of the respective BOOT0 hook
to appropriately insert the vectors before or after its other magic and
allow all architectures to do whatever their boot ROM requires...

> Would it be possible to change the DDR binary to not include the padding?

I would rather have all SPL binaries for Rockchip prepadded (and even
insert the appropriate ‘RK30’ .. ‘RK33’ word from asm/arch-rockchip/boot0.h
instead of from mkimage) as this is how the Rockchip ROM defines
a SPL payload… which in turn makes the usage of the boot0-hook exactly
what the docs for the SOC_BOOT0_HOOK want.

> [1] armv7: move boot hook before '_start'

Regards,
Philipp.