Re: [PATCH] riscv: setup per-hart stack earlier

[Bo Gan]

On 5/14/23 10:08 PM, Rick Chen wrote:

Hi Bo Gan,
It builds fail as below:

arch/riscv/cpu/start.S:97: Error: illegal operands `li
t0,CONFIG_SYS_INIT_SP_ADDR'

Thanks,
Rick



Hi Rick & Leo,

Please help take a look at v2 of the patch:
https://patchwork.ozlabs.org/project/uboot/patch/1684650044-313122-1-git-send-email-ganbo...@gmail.com/

I've fixed the macro and also did some testing on my vf2 board.

Thanks!

Re: [PATCH] riscv: setup per-hart stack earlier

[Rick Chen]

Hi Bo Gan,

> From: Bo Gan 
> Sent: Tuesday, May 09, 2023 9:46 AM
> To: u-boot@lists.denx.de
> Cc: Bo Gan ; Rick Jian-Zhi Chen(陳建志) 
> ; Leo Yu-Chi Liang(梁育齊) 
> Subject: [PATCH] riscv: setup per-hart stack earlier
>
> Harts need to use per-hart stack before any function call, even if that 
> function is a simple one. When the callee uses stack for register save/ 
> restore, especially RA, if nested call, concurrent access by multiple harts 
> on the same stack will cause data-race.
>
> This patch sets up SP before `board_init_f_alloc_reserve`. A side effect of 
> this is that the memory layout has changed as the following:
>
> ++++ <- SPL_STACK/
> |  ..||  hart 0 stack  |SYS_INIT_SP_ADDR
> |  malloc_base   |++
> ++|  hart 1 stack  |
> |  GD|++ If not SMP, N=1
> ++|  ..|
> |  hart 0 stack  |++
> ++   ==>  |  hart N-1 stack|
> |  hart 1 stack  |++
> ++|  ..|
> |  ..||  malloc_base   |
> ++++
> |  hart N-1 stack||  GD|
> ++++
> ||||
>
> Signed-off-by: Bo Gan 
> Cc: Rick Chen 
> Cc: Leo 
> ---
>  arch/riscv/cpu/start.S | 37 -
>  1 file changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 
> dad22bf..90015c2 100644
> --- a/arch/riscv/cpu/start.S
> +++ b/arch/riscv/cpu/start.S
> @@ -91,16 +91,35 @@ _start:
>   * Set stackpointer in internal/ex RAM to call board_init_f
>   */
>  call_board_init_f:
> -   li  t0, -16
>  #if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK)
> -   li  t1, CONFIG_SPL_STACK
> +   li  t0, CONFIG_SPL_STACK
>  #else
> -   li  t1, SYS_INIT_SP_ADDR
> +   li  t0, CONFIG_SYS_INIT_SP_ADDR

It builds fail as below:

arch/riscv/cpu/start.S:97: Error: illegal operands `li
t0,CONFIG_SYS_INIT_SP_ADDR'

Thanks,
Rick

[PATCH] riscv: setup per-hart stack earlier

[Bo Gan]

Harts need to use per-hart stack before any function call, even if that
function is a simple one. When the callee uses stack for register save/
restore, especially RA, if nested call, concurrent access by multiple
harts on the same stack will cause data-race.

This patch sets up SP before `board_init_f_alloc_reserve`. A side effect
of this is that the memory layout has changed as the following:

++++ <- SPL_STACK/
|  ..||  hart 0 stack  |SYS_INIT_SP_ADDR
|  malloc_base   |++
++|  hart 1 stack  |
|  GD|++ If not SMP, N=1
++|  ..|
|  hart 0 stack  |++
++   ==>  |  hart N-1 stack|
|  hart 1 stack  |++
++|  ..|
|  ..||  malloc_base   |
++++
|  hart N-1 stack||  GD|
++++
||||

Signed-off-by: Bo Gan 
Cc: Rick Chen 
Cc: Leo 
---
 arch/riscv/cpu/start.S | 37 -
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
index dad22bf..90015c2 100644
--- a/arch/riscv/cpu/start.S
+++ b/arch/riscv/cpu/start.S
@@ -91,16 +91,35 @@ _start:
  * Set stackpointer in internal/ex RAM to call board_init_f
  */
 call_board_init_f:
-   li  t0, -16
 #if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK)
-   li  t1, CONFIG_SPL_STACK
+   li  t0, CONFIG_SPL_STACK
 #else
-   li  t1, SYS_INIT_SP_ADDR
+   li  t0, CONFIG_SYS_INIT_SP_ADDR
 #endif
-   and sp, t1, t0  /* force 16 byte alignment */
+   and t0, t0, -16 /* force 16 byte alignment */
+
+   /* setup stack */
+#if CONFIG_IS_ENABLED(SMP)
+   /* tp: hart id */
+   sllit1, tp, CONFIG_STACK_SIZE_SHIFT
+   sub sp, t0, t1
+#else
+   mv  sp, t0
+#endif
+/*
+ * Now sp points to the right stack belonging to current CPU.
+ * It's essential before any function call, otherwise, we get data-race.
+ */
 
 call_board_init_f_0:
-   mv  a0, sp
+   /* find top of reserve space */
+#if CONFIG_IS_ENABLED(SMP)
+   li  t1, CONFIG_NR_CPUS
+#else
+   li  t1, 1
+#endif
+   sllit1, t1, CONFIG_STACK_SIZE_SHIFT
+   sub a0, t0, t1  /* t1 -> size of all CPU stacks */
jal board_init_f_alloc_reserve
 
/*
@@ -109,14 +128,6 @@ call_board_init_f_0:
 */
mv  s0, a0
 
-   /* setup stack */
-#if CONFIG_IS_ENABLED(SMP)
-   /* tp: hart id */
-   sllit0, tp, CONFIG_STACK_SIZE_SHIFT
-   sub sp, a0, t0
-#else
-   mv  sp, a0
-#endif
 
/* Configure proprietary settings and customized CSRs of harts */
 call_harts_early_init:
-- 
2.7.4