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

[Masahiro Yamada]

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.



> 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().






-- 
Best Regards
Masahiro Yamada