Re: [PATCH 2/2] riscv: Add a Zalrsc-only alternative for synchronization in start.S

[Yixun Lan <dlan@gentoo.org>]

Hi Yao,

On 09:21 Sat 02 Aug     , Yao Zi wrote:
> Add an alternative implementation that use Zalrsc extension only for
> HART lottery and SMP locking to support SMP on cores without "Zaamo"
> extension available. The Zaamo implementation is still used by default
> since since the Zalrsc one requires more instructions.
~~~~~~~~~~~~~two 'since'

to slightly improve it..
.., The Zaamo implementation is prioritized selected if both extension available,
since the Zalrsc one requires more instructions.

while I can understand the logic, but if we interpret from the code below,
it's a little bit weird:
 if (RISCV_ISA_ZAAMO) not enabled:
 	use zalrsc implementation

instead of 
 if (RISCV_ISA_ZALRSC) is enabled:
 	use zalrsc implementation

I mean, to select Zalrsc implementation, enabling RISCV_ISA_ZALRSC is not enough,
but RISCV_ISA_ZAAMO should be explicitly disabled, in fact RISCV_ISA_ZALRSC is
superfluous here

make it further, it would be great if we could do some Kconfig sanity check..
(I have one more comment for configs/ibex-ast2700_defconfig in patch 1/2)

> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  arch/riscv/cpu/start.S | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
> index 7bafdfd390a..6324ff585d4 100644
> --- a/arch/riscv/cpu/start.S
> +++ b/arch/riscv/cpu/start.S
> @@ -151,8 +151,15 @@ call_harts_early_init:
>  	 */
>  	la	t0, hart_lottery
>  	li	t1, 1
> +#if CONFIG_IS_ENABLED
>  	amoswap.w s2, t1, 0(t0)
>  	bnez	s2, wait_for_gd_init
> +#else
> +	lr.w	s2, (t0)
> +	bnez	s2, wait_for_gd_init
> +	sc.w	s2, t1, (t0)
> +	bnez	s2, wait_for_gd_init
> +#endif
>  #else
>  	/*
>  	 * FIXME: gp is set before it is initialized. If an XIP U-Boot ever
> @@ -177,7 +184,12 @@ call_harts_early_init:
>  #if !CONFIG_IS_ENABLED(XIP)
>  #ifdef CONFIG_AVAILABLE_HARTS
>  	la	t0, available_harts_lock
> +#if CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO)
>  	amoswap.w.rl zero, zero, 0(t0)
> +#else
> +	fence	rw, w
> +	sw	zero, 0(t0)
> +#endif
>  #endif
>  
>  wait_for_gd_init:
> @@ -190,7 +202,14 @@ wait_for_gd_init:
>  #ifdef CONFIG_AVAILABLE_HARTS
>  	la	t0, available_harts_lock
>  	li	t1, 1
> -1:	amoswap.w.aq t1, t1, 0(t0)
> +1:
> +#if CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO)
> +	amoswap.w.aq t1, t1, 0(t0)
> +#else
> +	lr.w.aq	t1, 0(t0)
> +	bnez	t1, 1b
> +	sc.w.rl t1, t1, 0(t0)
> +#endif
>  	bnez	t1, 1b
>  
>  	/* register available harts in the available_harts mask */
> @@ -200,7 +219,12 @@ wait_for_gd_init:
>  	or	t2, t2, t1
>  	SREG	t2, GD_AVAILABLE_HARTS(gp)
>  
> +#if CONFIG_IS_ENABLED(RISCV_ISA_ZAAMO)
>  	amoswap.w.rl zero, zero, 0(t0)
> +#else
> +	fence	rw, w
> +	sw	zero, 0(t0)
> +#endif
>  #endif
>  
>  	/*
> -- 
> 2.50.1
> 

-- 
Yixun Lan (dlan)