On Tue, May 28, 2024 at 05:10:46PM +0200, Alexandre Ghiti wrote: > This adds runtime support for Zacas in cmpxchg operations. > > Signed-off-by: Alexandre Ghiti <alexghiti@xxxxxxxxxxxx> > --- > arch/riscv/Kconfig | 17 +++++++++++++++++ > arch/riscv/Makefile | 11 +++++++++++ > arch/riscv/include/asm/cmpxchg.h | 23 ++++++++++++++++++++--- > 3 files changed, 48 insertions(+), 3 deletions(-) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 8a0f403432e8..b443def70139 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -579,6 +579,23 @@ config RISCV_ISA_V_PREEMPTIVE > preemption. Enabling this config will result in higher memory > consumption due to the allocation of per-task's kernel Vector context. > > +config TOOLCHAIN_HAS_ZACAS > + bool > + default y > + depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64ima_zacas) > + depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zacas) > + depends on AS_HAS_OPTION_ARCH > + > +config RISCV_ISA_ZACAS > + bool "Zacas extension support for atomic CAS" > + depends on TOOLCHAIN_HAS_ZACAS > + default y > + help > + Adds support to use atomic CAS instead of LR/SC to implement kernel > + atomic cmpxchg operation. If you were a person compiling a kernel, would you be able to read this and realise that this is safe to enable when their system does not support atomic CAS? Please take a look at other how other extensions handle this, or the patch that I have been sending that tries to make things clearer: https://patchwork.kernel.org/project/linux-riscv/patch/20240528-varnish-status-9c22973093a0@spud/ > + > + If you don't know what to do here, say Y. > + > config TOOLCHAIN_HAS_ZBB > bool > default y > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > index 5b3115a19852..d5b60b87998c 100644 > --- a/arch/riscv/Makefile > +++ b/arch/riscv/Makefile > @@ -78,6 +78,17 @@ endif > # Check if the toolchain supports Zihintpause extension > riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE) := $(riscv-march-y)_zihintpause > > +# Check if the toolchain supports Zacas > +ifdef CONFIG_AS_IS_LLVM > +# Support for experimental Zacas was merged in LLVM 17, but the removal of > +# the "experimental" was merged in LLVM 19. > +KBUILD_CFLAGS += -menable-experimental-extensions > +KBUILD_AFLAGS += -menable-experimental-extensions > +riscv-march-y := $(riscv-march-y)_zacas1p0 > +else > +riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZACAS) := $(riscv-march-y)_zacas > +endif I'm almost certain that we discussed this before for vector and it was decided to not enable experimental extensions (particularly as it is a global option), and instead require the non-experimental versions. This isn't even consistent with your TOOLCHAIN_HAS_ZACAS checks, that will only enable the option for the ratified version. I think we should continue to avoid enabling experimental extensions, even if that imposes a requirement of having a bleeding edge toolchain to actually use the extension. Thanks, Conor.
Attachment:
signature.asc
Description: PGP signature
