[U-Boot] armv8: release slave cores from CPU_RELEASE_ADDR

[Oded Gabbay]

On Thu, Jan 19, 2017 at 2:22 PM, Masahiro Yamada
<yamada.masahiro at socionext.com> wrote:
> 2017-01-17 18:20 GMT+09:00 Oded Gabbay <oded.gabbay at gmail.com>:
>> On Tue, Jan 17, 2017 at 11:07 AM, Masahiro Yamada
>> <yamada.masahiro at socionext.com> wrote:
>>> 2017-01-16 3:30 GMT+09:00 Tom Rini <trini at konsulko.com>:
>>>> On Wed, Dec 28, 2016 at 01:38:35PM +0200, Oded Gabbay wrote:
>>>>
>>>>> When using ARMv8 with ARMV8_SPIN_TABLE=y, we want the slave cores to
>>>>> wait on spin_table_cpu_release_addr, until the Linux kernel will "wake" them
>>>>> by writing to that location. The address of spin_table_cpu_release_addr is
>>>>> transferred to the kernel using the device tree that is updated by
>>>>> spin_table_update_dt().
>>>>>
>>>>> However, if we also use SPL, then the slave cores are stuck at
>>>>> CPU_RELEASE_ADDR instead and as a result, never wake up.
>>>>>
>>>>> This patch releases the slave cores by writing spl_image->entry_point to
>>>>> CPU_RELEASE_ADDR location before the end of the SPL code
>>>>> (at jump_to_image_no_args()).
>>>>>
>>>>> That way, the slave cores will start to execute the u-boot and will get to
>>>>> the spin-table code and wait on the correct address
>>>>> (spin_table_cpu_release_addr).
>>>>>
>>>>> Signed-off-by: Oded Gabbay <oded.gabbay at gmail.com>
>>>>> Cc: Simon Glass <sjg at chromium.org>
>>>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>>>
>>>> Applied to u-boot/master, thanks!
>>>
>>>
>>> I am the author of the spin table support of U-Boot.
>>>
>>> I should have checked this patch, but unfortunately
>>> I had not noticed it until I saw the commit in the upstream code.
>>>
>>> I am planning to revert it with the following log:
>>>
>>>
>>> Revert "armv8: release slave cores from CPU_RELEASE_ADDR"
>>>
>>> This reverts commit 8c36e99f211104fd7dcbf0669a35a47ce5e154f5.
>>>
>>> There is misunderstanding in commit 8c36e99f2111 ("armv8: release
>>> slave cores from CPU_RELEASE_ADDR").  How to bring the slave cores
>>> into U-Boot proper is platform-specific.  So, it should be cared
>>> in SoC/board files instead of common/spl/spl.c.  As you see SPL
>>> is the acronym of Secondary Program Loader, there is generally
>>> something that runs before SPL (the First one is usually Boot ROM).
>>>
>>> How to wake up slave cores from the Boot ROM is really SoC specific.
>>> So, the intention for the spin table support is to bring the slave
>>> cores into U-Boot proper (this must be done after relocation.  see
>>> below) in an SoC specific manner.  It is still possible to let the
>>> slave cores start from SPL, but it is not a common usecase that should
>>> be supported in the common code.
>>>
>> Then that should be written with CAPITAL letters somewhere in the
>> code/documentation, because what happens now is that in the case all
>> the cores do wake up at the start of the SPL, the slave cores will be
>> forever stuck.
>
> I mentioned "bring in the slaves in a board specific manner"
> in the current Kconfig help.
>
> I meant "U-Boot" as "U-Boot proper" (or aka U-Boot full image)
> instead of SPL.   I reworded to clarify it even more.
>
> http://patchwork.ozlabs.org/patch/717038/
>
> Please let me know if there is something is unclear,
> or you are not convinced with the explanation.
>
I think it is much more clearer now. I sent an r-b to the ML.
>
>
>> Is there a common place to wake up secondary cores / release them from
>> CPU_RELEASE_ADDR ? arch_early_init_r() perhaps ?
>
> Anywhere suitable for you
> if it is run after relocation.
>
> If you look at common/board_r.c, you will see
> various callbacks, board_early_init_r(), board_init(), board_late_init, etc.
>
> For your information, my own code for spin-table is
> arch/arm/mach-uniphier/smp_kick_cpus.c
> this is called from board_init().
>
Thanks for the tip, I'll take a look.
Oded
>
>
>
>
>
> --
> Best Regards
> Masahiro Yamada