Re: [PATCH 1/2] riscv: Add Kconfig options to distinguish Zaamo and Zalrsc

[Yao Zi <ziyao@disroot.org>]

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)