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

[Masahiro Yamada]

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.

One more thing is missing in the commit; spl_image->entry_point
points to the entry address of U-Boot *before* relocation.  U-Boot
relocates itself between board_init_f() and board_init_r().  This
means the master CPU sees the different copy of the spin table code
than the slave CPUs are really spinning.  The spin_table_update_dt()
protects the code *after* relocation.  As a result, the slave CPUs
spin in unprotected code, this leads to unstable behavior.




-- 
Best Regards
Masahiro Yamada