All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Support SMP on RISC-V cores with Zalrsc only
@ 2025-08-02  9:21 Yao Zi
  2025-08-02  9:21 ` [PATCH 1/2] riscv: Add Kconfig options to distinguish Zaamo and Zalrsc Yao Zi
  2025-08-02  9:21 ` [PATCH 2/2] riscv: Add a Zalrsc-only alternative for synchronization in start.S Yao Zi
  0 siblings, 2 replies; 9+ messages in thread
From: Yao Zi @ 2025-08-02  9:21 UTC (permalink / raw)
  To: Rick Chen, Leo, Tom Rini, Chia-Wei, Wang, Simon Glass; +Cc: u-boot, Yao Zi

Previously, we use AMO instructions to synchronize cores at start when
SMP is used, which requires Zaamo extension to present. This series
implements an alternative implementation of synchronization with LR/SC
instructions and add Kconfig options to use them, make it possible to
enable SMP on cores without Zaamo extension.

This is tested on both TH1520 Lichee Pi 4A board and QEMU. With QEMU,
building and testing looks lile,

	$ make qemu-riscv64_defconfig
	$ ./scripts/config -d CONFIG_RISCV_ISA_ZAAMO
	$ make
	$ qemu-system-riscv64 -M virt \
		-cpu rv64,a=off,zaamo=off,zawrs=off,zalrsc=on	\
		-nographic -bios u-boot-nodtb.bin -smp 8
	U-Boot 2025.10-rc1-00100-gc5bdc8820ee2-dirty (Aug 02 2025 - 09:16:21 +0000)

	CPU:   riscv
	Model: riscv-virtio,qemu
	DRAM:  128 MiB
	using memory 0x86eb5000-0x876d5000 for malloc()
	Core:  34 devices, 13 uclasses, devicetree: board
	Flash: 32 MiB
	Loading Environment from nowhere... OK
	In:    serial,usbkbd
	Out:   serial,vidconsole
	Err:   serial,vidconsole
	No USB controllers found
	Net:   No ethernet found.

without this series, U-Boot won't run if Zaamo is disabled with
-cpu rv64,zaamo=off.

Yao Zi (2):
  riscv: Add Kconfig options to distinguish Zaamo and Zalrsc
  riscv: Add a Zalrsc-only alternative for synchronization in start.S

 arch/riscv/Kconfig             | 17 +++++++++++++++++
 arch/riscv/Makefile            |  7 ++++++-
 arch/riscv/cpu/start.S         | 26 +++++++++++++++++++++++++-
 configs/ibex-ast2700_defconfig |  3 ++-
 4 files changed, 50 insertions(+), 3 deletions(-)

-- 
2.50.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] riscv: Add Kconfig options to distinguish Zaamo and Zalrsc
  2025-08-02  9:21 [PATCH 0/2] Support SMP on RISC-V cores with Zalrsc only Yao Zi
@ 2025-08-02  9:21 ` Yao Zi
  2025-08-02 23:59   ` Yixun Lan
  2025-08-03  1:24   ` Yixun Lan
  2025-08-02  9:21 ` [PATCH 2/2] riscv: Add a Zalrsc-only alternative for synchronization in start.S Yao Zi
  1 sibling, 2 replies; 9+ messages in thread
From: Yao Zi @ 2025-08-02  9:21 UTC (permalink / raw)
  To: Rick Chen, Leo, Tom Rini, Chia-Wei, Wang, Simon Glass; +Cc: u-boot, Yao Zi

Ratified on Apr. 2024, the original RISC-V "A" extension is now split
into two separate extensions, "Zaamo" for atomic operations and "Zalrsc"
for load-reserved/store-conditional instructions.

For now, we've already seen real-world designs implement the Zalrsc
extension only[2]. As U-Boot mainly runs with only one HART, we could
easily support these designs by not using AMO instructions in the
hard-written assembly if necessary, for which this patch introduces two
new Kconfig options to indicate the availability of "Zaamo" and "Zalrsc".

Note that even with this patch, "A" extension is specified in the ISA
string passed to the compiler as long as one of "Zaamo" or "Zalrsc" is
available, since they're only recognized with a quite recent version of
GCC/Clang. The compiler usually doesn't automatically generate atomic
instructions unless the source explicitly instructs it to do so, thus
this should be safe.

Link: https://github.com/riscv/riscv-zaamo-zalrsc/commit/d94c64c63e9120d56bdeb540caf2e5dae60a8126 # [1]
Link: https://lore.kernel.org/u-boot/20250729162035.209849-9-uros.stajic@htecgroup.com/ # [2]
Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 arch/riscv/Kconfig             | 17 +++++++++++++++++
 arch/riscv/Makefile            |  7 ++++++-
 configs/ibex-ast2700_defconfig |  3 ++-
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 8c6feae5735..b1c2d657e99 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -339,10 +339,27 @@ endmenu
 
 config RISCV_ISA_A
 	bool "Standard extension for Atomic Instructions"
+	depends on RISCV_ISA_ZAAMO && RISCV_ISA_ZALRSC
 	default y
 	help
 	  Adds "A" to the ISA string passed to the compiler.
 
+config RISCV_ISA_ZAAMO
+	bool "Standard extension for Atomic Memory Operations"
+	default y
+	help
+	  Indicates the platform supports Zaamo extension for atomic memory
+	  operations. Assembly routines won't use AMO instructions if set
+	  to n.
+
+config RISCV_ISA_ZALRSC
+	bool "Standard extension for LR/SC instructions"
+	default y
+	help
+	  Indicates the platform supports Zalrsc extension for load-reserved
+	  and store-conditional instructions. Assembly rutines won't use
+	  LR/SC instructions if set to n.
+
 config RISCV_ISA_ZICBOM
 	bool "Zicbom support"
 	depends on !SYS_DISABLE_DCACHE_OPS
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 6f80f4a7108..fdda6da1df3 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -11,7 +11,12 @@ ifeq ($(CONFIG_ARCH_RV32I),y)
 	ARCH_BASE = rv32im
 	ABI_BASE = ilp32
 endif
-ifeq ($(CONFIG_RISCV_ISA_A),y)
+# GCC starts to recognize "Zaamo" and "Zalrsc" from version 15, which is quite
+# recent. We don't bother checking the exact compiler version, but pass "A"
+# extension for -march as long as one of "Zaamo" or "Zalrsc" is available.
+ifeq ($(findstring y,$(CONFIG_RISCV_ISA_A)	\
+		     $(CONFIG_RISCV_ISA_ZAAMO)	\
+		     $(CONFIG_RISCV_ISA_ZALRSC)),y)
 	ARCH_A = a
 endif
 ifeq ($(CONFIG_RISCV_ISA_F),y)
diff --git a/configs/ibex-ast2700_defconfig b/configs/ibex-ast2700_defconfig
index f088aec8716..eb5cab43645 100644
--- a/configs/ibex-ast2700_defconfig
+++ b/configs/ibex-ast2700_defconfig
@@ -23,7 +23,8 @@ CONFIG_SYS_MEM_TOP_HIDE=0x10000000
 CONFIG_BUILD_TARGET=""
 CONFIG_TARGET_ASPEED_AST2700_IBEX=y
 # CONFIG_RISCV_ISA_F is not set
-# CONFIG_RISCV_ISA_A is not set
+# CONFIG_RISCV_ISA_ZAAMO is not set
+# CONFIG_RISCV_ISA_ZALRSC is not set
 # CONFIG_SPL_SMP is not set
 CONFIG_XIP=y
 CONFIG_SPL_XIP=y
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] riscv: Add a Zalrsc-only alternative for synchronization in start.S
  2025-08-02  9:21 [PATCH 0/2] Support SMP on RISC-V cores with Zalrsc only Yao Zi
  2025-08-02  9:21 ` [PATCH 1/2] riscv: Add Kconfig options to distinguish Zaamo and Zalrsc Yao Zi
@ 2025-08-02  9:21 ` Yao Zi
  2025-08-03  1:21   ` Yixun Lan
  1 sibling, 1 reply; 9+ messages in thread
From: Yao Zi @ 2025-08-02  9:21 UTC (permalink / raw)
  To: Rick Chen, Leo, Tom Rini, Chia-Wei, Wang, Simon Glass; +Cc: u-boot, Yao Zi

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.

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] riscv: Add Kconfig options to distinguish Zaamo and Zalrsc
  2025-08-02  9:21 ` [PATCH 1/2] riscv: Add Kconfig options to distinguish Zaamo and Zalrsc Yao Zi
@ 2025-08-02 23:59   ` Yixun Lan
  2025-08-03  0:16     ` Yao Zi
  2025-08-03  1:24   ` Yixun Lan
  1 sibling, 1 reply; 9+ messages in thread
From: Yixun Lan @ 2025-08-02 23:59 UTC (permalink / raw)
  To: Yao Zi; +Cc: Rick Chen, Leo, Tom Rini, Chia-Wei, Wang, Simon Glass, u-boot

Hi Yao,

On 09:21 Sat 02 Aug     , Yao Zi wrote:
> Ratified on Apr. 2024, the original RISC-V "A" extension is now split
> into two separate extensions, "Zaamo" for atomic operations and "Zalrsc"
> for load-reserved/store-conditional instructions.
> 
> For now, we've already seen real-world designs implement the Zalrsc
> extension only[2]. As U-Boot mainly runs with only one HART, we could
> easily support these designs by not using AMO instructions in the
> hard-written assembly if necessary, for which this patch introduces two
> new Kconfig options to indicate the availability of "Zaamo" and "Zalrsc".
> 
> Note that even with this patch, "A" extension is specified in the ISA
> string passed to the compiler as long as one of "Zaamo" or "Zalrsc" is
> available, since they're only recognized with a quite recent version of
> GCC/Clang. The compiler usually doesn't automatically generate atomic
> instructions unless the source explicitly instructs it to do so, thus
> this should be safe.
> 
> Link: https://github.com/riscv/riscv-zaamo-zalrsc/commit/d94c64c63e9120d56bdeb540caf2e5dae60a8126 # [1]
> Link: https://lore.kernel.org/u-boot/20250729162035.209849-9-uros.stajic@htecgroup.com/ # [2]
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  arch/riscv/Kconfig             | 17 +++++++++++++++++
>  arch/riscv/Makefile            |  7 ++++++-
>  configs/ibex-ast2700_defconfig |  3 ++-
>  3 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 8c6feae5735..b1c2d657e99 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -339,10 +339,27 @@ endmenu
>  
>  config RISCV_ISA_A
>  	bool "Standard extension for Atomic Instructions"
> +	depends on RISCV_ISA_ZAAMO && RISCV_ISA_ZALRSC
>  	default y
>  	help
>  	  Adds "A" to the ISA string passed to the compiler.
>  
> +config RISCV_ISA_ZAAMO
> +	bool "Standard extension for Atomic Memory Operations"
> +	default y
> +	help
> +	  Indicates the platform supports Zaamo extension for atomic memory
> +	  operations. Assembly routines won't use AMO instructions if set
> +	  to n.
> +
> +config RISCV_ISA_ZALRSC
> +	bool "Standard extension for LR/SC instructions"
> +	default y
> +	help
> +	  Indicates the platform supports Zalrsc extension for load-reserved
> +	  and store-conditional instructions. Assembly rutines won't use
                                                   s/rutines/routines/
while to be more precise, "Assembly routines" should be interpreted as
instructions generated by compiler? if yes, can you improve the wording?
> +	  LR/SC instructions if set to n.
> +
>  config RISCV_ISA_ZICBOM
>  	bool "Zicbom support"
>  	depends on !SYS_DISABLE_DCACHE_OPS
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index 6f80f4a7108..fdda6da1df3 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -11,7 +11,12 @@ ifeq ($(CONFIG_ARCH_RV32I),y)
>  	ARCH_BASE = rv32im
>  	ABI_BASE = ilp32
>  endif
> -ifeq ($(CONFIG_RISCV_ISA_A),y)
> +# GCC starts to recognize "Zaamo" and "Zalrsc" from version 15, which is quite
> +# recent. We don't bother checking the exact compiler version, but pass "A"
> +# extension for -march as long as one of "Zaamo" or "Zalrsc" is available.
> +ifeq ($(findstring y,$(CONFIG_RISCV_ISA_A)	\
> +		     $(CONFIG_RISCV_ISA_ZAAMO)	\
> +		     $(CONFIG_RISCV_ISA_ZALRSC)),y)
>  	ARCH_A = a
>  endif
>  ifeq ($(CONFIG_RISCV_ISA_F),y)
> diff --git a/configs/ibex-ast2700_defconfig b/configs/ibex-ast2700_defconfig
> index f088aec8716..eb5cab43645 100644
> --- a/configs/ibex-ast2700_defconfig
> +++ b/configs/ibex-ast2700_defconfig
> @@ -23,7 +23,8 @@ CONFIG_SYS_MEM_TOP_HIDE=0x10000000
>  CONFIG_BUILD_TARGET=""
>  CONFIG_TARGET_ASPEED_AST2700_IBEX=y
>  # CONFIG_RISCV_ISA_F is not set
> -# CONFIG_RISCV_ISA_A is not set
> +# CONFIG_RISCV_ISA_ZAAMO is not set
> +# CONFIG_RISCV_ISA_ZALRSC is not set
>  # CONFIG_SPL_SMP is not set
>  CONFIG_XIP=y
>  CONFIG_SPL_XIP=y
> -- 
> 2.50.1
> 

-- 
Yixun Lan (dlan)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] riscv: Add Kconfig options to distinguish Zaamo and Zalrsc
  2025-08-02 23:59   ` Yixun Lan
@ 2025-08-03  0:16     ` Yao Zi
  0 siblings, 0 replies; 9+ messages in thread
From: Yao Zi @ 2025-08-03  0:16 UTC (permalink / raw)
  To: Yixun Lan; +Cc: Rick Chen, Leo, Tom Rini, Chia-Wei, Wang, Simon Glass, u-boot

On Sun, Aug 03, 2025 at 07:59:05AM +0800, Yixun Lan wrote:
> Hi Yao,
> 
> On 09:21 Sat 02 Aug     , Yao Zi wrote:
> > Ratified on Apr. 2024, the original RISC-V "A" extension is now split
> > into two separate extensions, "Zaamo" for atomic operations and "Zalrsc"
> > for load-reserved/store-conditional instructions.
> > 
> > For now, we've already seen real-world designs implement the Zalrsc
> > extension only[2]. As U-Boot mainly runs with only one HART, we could
> > easily support these designs by not using AMO instructions in the
> > hard-written assembly if necessary, for which this patch introduces two
> > new Kconfig options to indicate the availability of "Zaamo" and "Zalrsc".
> > 
> > Note that even with this patch, "A" extension is specified in the ISA
> > string passed to the compiler as long as one of "Zaamo" or "Zalrsc" is
> > available, since they're only recognized with a quite recent version of
> > GCC/Clang. The compiler usually doesn't automatically generate atomic
> > instructions unless the source explicitly instructs it to do so, thus
> > this should be safe.
> > 
> > Link: https://github.com/riscv/riscv-zaamo-zalrsc/commit/d94c64c63e9120d56bdeb540caf2e5dae60a8126 # [1]
> > Link: https://lore.kernel.org/u-boot/20250729162035.209849-9-uros.stajic@htecgroup.com/ # [2]
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > ---
> >  arch/riscv/Kconfig             | 17 +++++++++++++++++
> >  arch/riscv/Makefile            |  7 ++++++-
> >  configs/ibex-ast2700_defconfig |  3 ++-
> >  3 files changed, 25 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 8c6feae5735..b1c2d657e99 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -339,10 +339,27 @@ endmenu
> >  
> >  config RISCV_ISA_A
> >  	bool "Standard extension for Atomic Instructions"
> > +	depends on RISCV_ISA_ZAAMO && RISCV_ISA_ZALRSC
> >  	default y
> >  	help
> >  	  Adds "A" to the ISA string passed to the compiler.
> >  
> > +config RISCV_ISA_ZAAMO
> > +	bool "Standard extension for Atomic Memory Operations"
> > +	default y
> > +	help
> > +	  Indicates the platform supports Zaamo extension for atomic memory
> > +	  operations. Assembly routines won't use AMO instructions if set
> > +	  to n.
> > +
> > +config RISCV_ISA_ZALRSC
> > +	bool "Standard extension for LR/SC instructions"
> > +	default y
> > +	help
> > +	  Indicates the platform supports Zalrsc extension for load-reserved
> > +	  and store-conditional instructions. Assembly rutines won't use
>                                                    s/rutines/routines/
> while to be more precise, "Assembly routines" should be interpreted as
> instructions generated by compiler? if yes, can you improve the wording?

Thanks for catching the typo. I was meant to refer the handwritten ones,
instead of the compiler-generated ones. As you could see in the commit
message and the Makefile changes, we always add "A" to march argument as
long as one of "Zalrsc" and "Zaamo" is available, so these two new
options basically have nothing to do with compiler's code generation.

Anyway, it seems helpful to be more precise here. Maybe something like,

	Indicates the platform supports Zalrsc extension for load-reserved
	and store-conditional isntructions. Hand-written assembly routines
	won't use LR/SC instructions if set to n.

I'll wait a little more time to see whether there're more comments on
the series, then send v2.

Thanks,
Yao Zi

> > +	  LR/SC instructions if set to n.
> > +
> >  config RISCV_ISA_ZICBOM
> >  	bool "Zicbom support"
> >  	depends on !SYS_DISABLE_DCACHE_OPS
> > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > index 6f80f4a7108..fdda6da1df3 100644
> > --- a/arch/riscv/Makefile
> > +++ b/arch/riscv/Makefile
> > @@ -11,7 +11,12 @@ ifeq ($(CONFIG_ARCH_RV32I),y)
> >  	ARCH_BASE = rv32im
> >  	ABI_BASE = ilp32
> >  endif
> > -ifeq ($(CONFIG_RISCV_ISA_A),y)
> > +# GCC starts to recognize "Zaamo" and "Zalrsc" from version 15, which is quite
> > +# recent. We don't bother checking the exact compiler version, but pass "A"
> > +# extension for -march as long as one of "Zaamo" or "Zalrsc" is available.
> > +ifeq ($(findstring y,$(CONFIG_RISCV_ISA_A)	\
> > +		     $(CONFIG_RISCV_ISA_ZAAMO)	\
> > +		     $(CONFIG_RISCV_ISA_ZALRSC)),y)
> >  	ARCH_A = a
> >  endif
> >  ifeq ($(CONFIG_RISCV_ISA_F),y)
> > diff --git a/configs/ibex-ast2700_defconfig b/configs/ibex-ast2700_defconfig
> > index f088aec8716..eb5cab43645 100644
> > --- a/configs/ibex-ast2700_defconfig
> > +++ b/configs/ibex-ast2700_defconfig
> > @@ -23,7 +23,8 @@ CONFIG_SYS_MEM_TOP_HIDE=0x10000000
> >  CONFIG_BUILD_TARGET=""
> >  CONFIG_TARGET_ASPEED_AST2700_IBEX=y
> >  # CONFIG_RISCV_ISA_F is not set
> > -# CONFIG_RISCV_ISA_A is not set
> > +# CONFIG_RISCV_ISA_ZAAMO is not set
> > +# CONFIG_RISCV_ISA_ZALRSC is not set
> >  # CONFIG_SPL_SMP is not set
> >  CONFIG_XIP=y
> >  CONFIG_SPL_XIP=y
> > -- 
> > 2.50.1
> > 
> 
> -- 
> Yixun Lan (dlan)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] riscv: Add a Zalrsc-only alternative for synchronization in start.S
  2025-08-02  9:21 ` [PATCH 2/2] riscv: Add a Zalrsc-only alternative for synchronization in start.S Yao Zi
@ 2025-08-03  1:21   ` Yixun Lan
  2025-08-03  3:53     ` Yao Zi
  0 siblings, 1 reply; 9+ messages in thread
From: Yixun Lan @ 2025-08-03  1:21 UTC (permalink / raw)
  To: Yao Zi; +Cc: Rick Chen, Leo, Tom Rini, Chia-Wei, Wang, Simon Glass, u-boot

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)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] riscv: Add Kconfig options to distinguish Zaamo and Zalrsc
  2025-08-02  9:21 ` [PATCH 1/2] riscv: Add Kconfig options to distinguish Zaamo and Zalrsc Yao Zi
  2025-08-02 23:59   ` Yixun Lan
@ 2025-08-03  1:24   ` Yixun Lan
  2025-08-03  3:28     ` Yao Zi
  1 sibling, 1 reply; 9+ messages in thread
From: Yixun Lan @ 2025-08-03  1:24 UTC (permalink / raw)
  To: Yao Zi; +Cc: Rick Chen, Leo, Tom Rini, Chia-Wei, Wang, Simon Glass, u-boot

Hi Yao,

On 09:21 Sat 02 Aug     , Yao Zi wrote:
> Ratified on Apr. 2024, the original RISC-V "A" extension is now split
> into two separate extensions, "Zaamo" for atomic operations and "Zalrsc"
> for load-reserved/store-conditional instructions.
> 
> For now, we've already seen real-world designs implement the Zalrsc
> extension only[2]. As U-Boot mainly runs with only one HART, we could
> easily support these designs by not using AMO instructions in the
> hard-written assembly if necessary, for which this patch introduces two
> new Kconfig options to indicate the availability of "Zaamo" and "Zalrsc".
> 
> Note that even with this patch, "A" extension is specified in the ISA
> string passed to the compiler as long as one of "Zaamo" or "Zalrsc" is
> available, since they're only recognized with a quite recent version of
> GCC/Clang. The compiler usually doesn't automatically generate atomic
> instructions unless the source explicitly instructs it to do so, thus
> this should be safe.
> 
> Link: https://github.com/riscv/riscv-zaamo-zalrsc/commit/d94c64c63e9120d56bdeb540caf2e5dae60a8126 # [1]
> Link: https://lore.kernel.org/u-boot/20250729162035.209849-9-uros.stajic@htecgroup.com/ # [2]
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  arch/riscv/Kconfig             | 17 +++++++++++++++++
>  arch/riscv/Makefile            |  7 ++++++-
>  configs/ibex-ast2700_defconfig |  3 ++-
>  3 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 8c6feae5735..b1c2d657e99 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -339,10 +339,27 @@ endmenu
>  
>  config RISCV_ISA_A
>  	bool "Standard extension for Atomic Instructions"
> +	depends on RISCV_ISA_ZAAMO && RISCV_ISA_ZALRSC
>  	default y
>  	help
>  	  Adds "A" to the ISA string passed to the compiler.
>  
> +config RISCV_ISA_ZAAMO
> +	bool "Standard extension for Atomic Memory Operations"
> +	default y
> +	help
> +	  Indicates the platform supports Zaamo extension for atomic memory
> +	  operations. Assembly routines won't use AMO instructions if set
> +	  to n.
> +
> +config RISCV_ISA_ZALRSC
> +	bool "Standard extension for LR/SC instructions"
> +	default y
> +	help
> +	  Indicates the platform supports Zalrsc extension for load-reserved
> +	  and store-conditional instructions. Assembly rutines won't use
> +	  LR/SC instructions if set to n.
> +
>  config RISCV_ISA_ZICBOM
>  	bool "Zicbom support"
>  	depends on !SYS_DISABLE_DCACHE_OPS
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index 6f80f4a7108..fdda6da1df3 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -11,7 +11,12 @@ ifeq ($(CONFIG_ARCH_RV32I),y)
>  	ARCH_BASE = rv32im
>  	ABI_BASE = ilp32
>  endif
> -ifeq ($(CONFIG_RISCV_ISA_A),y)
> +# GCC starts to recognize "Zaamo" and "Zalrsc" from version 15, which is quite
> +# recent. We don't bother checking the exact compiler version, but pass "A"
> +# extension for -march as long as one of "Zaamo" or "Zalrsc" is available.
> +ifeq ($(findstring y,$(CONFIG_RISCV_ISA_A)	\
> +		     $(CONFIG_RISCV_ISA_ZAAMO)	\
> +		     $(CONFIG_RISCV_ISA_ZALRSC)),y)
>  	ARCH_A = a
>  endif
>  ifeq ($(CONFIG_RISCV_ISA_F),y)
> diff --git a/configs/ibex-ast2700_defconfig b/configs/ibex-ast2700_defconfig
> index f088aec8716..eb5cab43645 100644
> --- a/configs/ibex-ast2700_defconfig
> +++ b/configs/ibex-ast2700_defconfig
> @@ -23,7 +23,8 @@ CONFIG_SYS_MEM_TOP_HIDE=0x10000000
>  CONFIG_BUILD_TARGET=""
>  CONFIG_TARGET_ASPEED_AST2700_IBEX=y
>  # CONFIG_RISCV_ISA_F is not set
> -# CONFIG_RISCV_ISA_A is not set
> +# CONFIG_RISCV_ISA_ZAAMO is not set
> +# CONFIG_RISCV_ISA_ZALRSC is not set

this is confusing, while in this patchset it actually equal to :
# CONFIG_RISCV_ISA_ZAAMO is not set
CONFIG_RISCV_ISA_ZALRSC=y

also I believe changes for configs/ibex-ast2700_defconfig should go as
an independent patch, please separate it

>  # CONFIG_SPL_SMP is not set
>  CONFIG_XIP=y
>  CONFIG_SPL_XIP=y
> -- 
> 2.50.1
> 

-- 
Yixun Lan (dlan)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] riscv: Add Kconfig options to distinguish Zaamo and Zalrsc
  2025-08-03  1:24   ` Yixun Lan
@ 2025-08-03  3:28     ` Yao Zi
  0 siblings, 0 replies; 9+ messages in thread
From: Yao Zi @ 2025-08-03  3:28 UTC (permalink / raw)
  To: Yixun Lan; +Cc: Rick Chen, Leo, Tom Rini, Chia-Wei, Wang, Simon Glass, u-boot

On Sun, Aug 03, 2025 at 09:24:22AM +0800, Yixun Lan wrote:
> Hi Yao,
> 
> On 09:21 Sat 02 Aug     , Yao Zi wrote:
> > Ratified on Apr. 2024, the original RISC-V "A" extension is now split
> > into two separate extensions, "Zaamo" for atomic operations and "Zalrsc"
> > for load-reserved/store-conditional instructions.
> > 
> > For now, we've already seen real-world designs implement the Zalrsc
> > extension only[2]. As U-Boot mainly runs with only one HART, we could
> > easily support these designs by not using AMO instructions in the
> > hard-written assembly if necessary, for which this patch introduces two
> > new Kconfig options to indicate the availability of "Zaamo" and "Zalrsc".
> > 
> > Note that even with this patch, "A" extension is specified in the ISA
> > string passed to the compiler as long as one of "Zaamo" or "Zalrsc" is
> > available, since they're only recognized with a quite recent version of
> > GCC/Clang. The compiler usually doesn't automatically generate atomic
> > instructions unless the source explicitly instructs it to do so, thus
> > this should be safe.
> > 
> > Link: https://github.com/riscv/riscv-zaamo-zalrsc/commit/d94c64c63e9120d56bdeb540caf2e5dae60a8126 # [1]
> > Link: https://lore.kernel.org/u-boot/20250729162035.209849-9-uros.stajic@htecgroup.com/ # [2]
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > ---
> >  arch/riscv/Kconfig             | 17 +++++++++++++++++
> >  arch/riscv/Makefile            |  7 ++++++-
> >  configs/ibex-ast2700_defconfig |  3 ++-
> >  3 files changed, 25 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 8c6feae5735..b1c2d657e99 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -339,10 +339,27 @@ endmenu
> >  
> >  config RISCV_ISA_A
> >  	bool "Standard extension for Atomic Instructions"
> > +	depends on RISCV_ISA_ZAAMO && RISCV_ISA_ZALRSC
> >  	default y
> >  	help
> >  	  Adds "A" to the ISA string passed to the compiler.
> >  
> > +config RISCV_ISA_ZAAMO
> > +	bool "Standard extension for Atomic Memory Operations"
> > +	default y
> > +	help
> > +	  Indicates the platform supports Zaamo extension for atomic memory
> > +	  operations. Assembly routines won't use AMO instructions if set
> > +	  to n.
> > +
> > +config RISCV_ISA_ZALRSC
> > +	bool "Standard extension for LR/SC instructions"
> > +	default y
> > +	help
> > +	  Indicates the platform supports Zalrsc extension for load-reserved
> > +	  and store-conditional instructions. Assembly rutines won't use
> > +	  LR/SC instructions if set to n.
> > +
> >  config RISCV_ISA_ZICBOM
> >  	bool "Zicbom support"
> >  	depends on !SYS_DISABLE_DCACHE_OPS
> > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > index 6f80f4a7108..fdda6da1df3 100644
> > --- a/arch/riscv/Makefile
> > +++ b/arch/riscv/Makefile
> > @@ -11,7 +11,12 @@ ifeq ($(CONFIG_ARCH_RV32I),y)
> >  	ARCH_BASE = rv32im
> >  	ABI_BASE = ilp32
> >  endif
> > -ifeq ($(CONFIG_RISCV_ISA_A),y)
> > +# GCC starts to recognize "Zaamo" and "Zalrsc" from version 15, which is quite
> > +# recent. We don't bother checking the exact compiler version, but pass "A"
> > +# extension for -march as long as one of "Zaamo" or "Zalrsc" is available.
> > +ifeq ($(findstring y,$(CONFIG_RISCV_ISA_A)	\
> > +		     $(CONFIG_RISCV_ISA_ZAAMO)	\
> > +		     $(CONFIG_RISCV_ISA_ZALRSC)),y)
> >  	ARCH_A = a
> >  endif
> >  ifeq ($(CONFIG_RISCV_ISA_F),y)
> > diff --git a/configs/ibex-ast2700_defconfig b/configs/ibex-ast2700_defconfig
> > index f088aec8716..eb5cab43645 100644
> > --- a/configs/ibex-ast2700_defconfig
> > +++ b/configs/ibex-ast2700_defconfig
> > @@ -23,7 +23,8 @@ CONFIG_SYS_MEM_TOP_HIDE=0x10000000
> >  CONFIG_BUILD_TARGET=""
> >  CONFIG_TARGET_ASPEED_AST2700_IBEX=y
> >  # CONFIG_RISCV_ISA_F is not set
> > -# CONFIG_RISCV_ISA_A is not set
> > +# CONFIG_RISCV_ISA_ZAAMO is not set
> > +# CONFIG_RISCV_ISA_ZALRSC is not set
> 
> this is confusing, while in this patchset it actually equal to :
> # CONFIG_RISCV_ISA_ZAAMO is not set
> CONFIG_RISCV_ISA_ZALRSC=y

No, ibex-ast2700_defconfig enables XIP and SPL_XIP, thus isn't affected
by the change of the second patch. This defconfig change is only for
ensuring the march string passed to CC doesn't contain "A" extension
just as what has been done previously, which may help find out possible
breakages to the board as early as possible in the future, since it's
the only existing board that doesn't support A extension.

> also I believe changes for configs/ibex-ast2700_defconfig should go as
> an independent patch, please separate it

At my very first glance, I was worried about behavior changes for the
board: as you can see, what is passed as march affects the content of
.riscv.attributes section in the result ELF file, and if we disable
RISCV_ISA_ZA{AMO,LRSC} in a separate patch, "A" will be included in the
ISA string before it gets applied.

This seems to be somehow farsight and I'll separate the change into
another patch.

Best regards,
Yao Zi

> >  # CONFIG_SPL_SMP is not set
> >  CONFIG_XIP=y
> >  CONFIG_SPL_XIP=y
> > -- 
> > 2.50.1
> > 
> 
> -- 
> Yixun Lan (dlan)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] riscv: Add a Zalrsc-only alternative for synchronization in start.S
  2025-08-03  1:21   ` Yixun Lan
@ 2025-08-03  3:53     ` Yao Zi
  0 siblings, 0 replies; 9+ messages in thread
From: Yao Zi @ 2025-08-03  3:53 UTC (permalink / raw)
  To: Yixun Lan; +Cc: Rick Chen, Leo, Tom Rini, Chia-Wei, Wang, Simon Glass, u-boot

On Sun, Aug 03, 2025 at 09:21:23AM +0800, Yixun Lan wrote:
> 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.

Thanks for catching the typo. This improvement also looks good to me.

> 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,

This is true, but I don't think it matters: the two implementations
aren't two different features, but code details to the same feature. I
couldn't come up with a reason to explicitly select the Zalrsc one when
Zaamo is available, especially when the Zaamo one takes less code space.

> in fact RISCV_ISA_ZALRSC is superfluous here

Yes, it's not used in this patch, but it has its place: without such an
option, one couldn't distinguish whether Zalrsc is available, it would
be hard to determine whether "A"/"Zalrsc" should be passed as part of
the ISA string when CONFIG_RISCV_ISA_ZAAMO is set to n.

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

If I understand it correctly, these code is actually for SMP systems.
Ideally we should group them inside "#if CONFIG_IS_ENABLED(SMP)" and
make SMP depend on RISCV_RISA_ZAAMO || RISCV_ISA_ZALRSC, which I
originally want to make a separate patch (it's not a must to make SMP
possible on Zalrsc-only systems).

Thanks,
Yao Zi

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-08-03  3:53 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-02  9:21 [PATCH 0/2] Support SMP on RISC-V cores with Zalrsc only Yao Zi
2025-08-02  9:21 ` [PATCH 1/2] riscv: Add Kconfig options to distinguish Zaamo and Zalrsc Yao Zi
2025-08-02 23:59   ` Yixun Lan
2025-08-03  0:16     ` Yao Zi
2025-08-03  1:24   ` Yixun Lan
2025-08-03  3:28     ` Yao Zi
2025-08-02  9:21 ` [PATCH 2/2] riscv: Add a Zalrsc-only alternative for synchronization in start.S Yao Zi
2025-08-03  1:21   ` Yixun Lan
2025-08-03  3:53     ` Yao Zi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.