Hi Alexandre, On Tue, May 28, 2024 at 05:10:47PM +0200, Alexandre Ghiti wrote: > This adds runtime support for Zabha in cmpxchg8/16 operations. > > Signed-off-by: Alexandre Ghiti <alexghiti@xxxxxxxxxxxx> > --- > arch/riscv/Kconfig | 16 ++++++++++++++++ > arch/riscv/Makefile | 10 ++++++++++ > arch/riscv/include/asm/cmpxchg.h | 26 ++++++++++++++++++++++++-- > 3 files changed, 50 insertions(+), 2 deletions(-) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index b443def70139..05597719bb1c 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -579,6 +579,22 @@ 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_ZABHA > + bool > + default y > + depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64ima_zabha) > + depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zabha) This test does not take into account the need for '-menable-experimental-extensions' and '1p0' in the '-march=' value with clang 19, so it can never be enabled even if it is available. I am not really sure how to succinctly account for this though, other than duplicating and modifying the cc-option checks with a dependency on either CC_IS_GCC or CC_IS_CLANG. Another option is taking the same approach as the _SUPPORTS_DYNAMIC_FTRACE symbols and introduce CLANG_HAS_ZABHA and GCC_HAS_ZABHA? That might not make it too ugly. I think the ZACAS patch has a similar issue, it just isn't noticeable with clang 19 but it should be with clang 17 and 18. > + depends on AS_HAS_OPTION_ARCH > + > +config RISCV_ISA_ZABHA > + bool "Zabha extension support for atomic byte/half-word operations" > + depends on TOOLCHAIN_HAS_ZABHA > + default y > + help > + Adds support to use atomic byte/half-word operations in the kernel. > + > + If you don't know what to do here, say Y. > + > config TOOLCHAIN_HAS_ZACAS > bool > default y > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > index d5b60b87998c..f58ac921dece 100644 > --- a/arch/riscv/Makefile > +++ b/arch/riscv/Makefile > @@ -89,6 +89,16 @@ else > riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZACAS) := $(riscv-march-y)_zacas > endif > > +# Check if the toolchain supports Zabha > +ifdef CONFIG_AS_IS_LLVM > +# Support for experimental Zabha was merged in LLVM 19. > +KBUILD_CFLAGS += -menable-experimental-extensions > +KBUILD_AFLAGS += -menable-experimental-extensions > +riscv-march-y := $(riscv-march-y)_zabha1p0 This block should have some dependency on CONFIG_TOOLCHAIN_HAS_ZABHA as well right? Otherwise, the build breaks with LLVM toolchains that do not support zabha, like LLVM 18.1.x: clang: error: invalid arch name 'rv64imac_zihintpause_zacas1p0_zabha1p0', unsupported version number 1.0 for extension 'zabha' I think the zacas patch has the same bug. I think that it would be good to consolidate the adding of '-menable-experimental-extensions' to the compiler and assembler flags to perhaps having a hidden symbol like CONFIG_EXPERIMENTAL_EXTENSIONS that is selected by any extension that is experimental for the particular toolchain version. config EXPERIMENTAL_EXTENSIONS bool config TOOLCHAIN_HAS_ZABHA def_bool y select EXPERIMENTAL_EXETNSIONS if CC_IS_CLANG ... config TOOLCHAIN_HAS_ZACAS def_bool_y # ZACAS was experimental until Clang 19: https://github.com/llvm/llvm-project/commit/95aab69c109adf29e183090c25dc95c773215746 select EXPERIMENTAL_EXETNSIONS if CC_IS_CLANG && CLANG_VERSION < 190000 ... Then in the Makefile: ifdef CONFIG_EXPERIMENTAL_EXTENSIONS KBUILD_AFLAGS += -menable-experimental-extensions KBUILD_CFLAGS += -menable-experimental-extensions endif > +else > +riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZABHA) := $(riscv-march-y)_zabha > +endif > + > # Remove F,D,V from isa string for all. Keep extensions between "fd" and "v" by > # matching non-v and non-multi-letter extensions out with the filter ([^v_]*) > KBUILD_CFLAGS += -march=$(shell echo $(riscv-march-y) | sed -E 's/(rv32ima|rv64ima)fd([^v_]*)v?/\1\2/') > diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h > index 1c50b4821ac8..65de9771078e 100644 > --- a/arch/riscv/include/asm/cmpxchg.h > +++ b/arch/riscv/include/asm/cmpxchg.h > @@ -103,8 +103,14 @@ > * indicated by comparing RETURN with OLD. > */ > > -#define __arch_cmpxchg_masked(sc_sfx, prepend, append, r, p, o, n) \ > +#define __arch_cmpxchg_masked(sc_sfx, cas_sfx, prepend, append, r, p, o, n) \ > ({ \ > + __label__ zabha, end; \ > + \ > + asm goto(ALTERNATIVE("nop", "j %[zabha]", 0, \ > + RISCV_ISA_EXT_ZABHA, 1) \ > + : : : : zabha); \ > + \ > u32 *__ptr32b = (u32 *)((ulong)(p) & ~0x3); \ > ulong __s = ((ulong)(p) & (0x4 - sizeof(*p))) * BITS_PER_BYTE; \ > ulong __mask = GENMASK(((sizeof(*p)) * BITS_PER_BYTE) - 1, 0) \ > @@ -131,6 +137,17 @@ > : "memory"); \ > \ > r = (__typeof__(*(p)))((__retx & __mask) >> __s); \ > + goto end; \ > + \ > +zabha: \ > + __asm__ __volatile__ ( \ > + prepend \ > + " amocas" cas_sfx " %0, %z2, %1\n" \ This should probably have some dependency on CONFIG_RISCV_ISA_ZABHA? I get the following with GCC 13.2.0: include/linux/atomic/atomic-arch-fallback.h: Assembler messages: include/linux/atomic/atomic-arch-fallback.h:2108: Error: unrecognized opcode `amocas.w a4,a3,0(s1)' > + append \ > + : "+&r" (r), "+A" (*(p)) \ > + : "rJ" (n) \ > + : "memory"); \ > +end: \ I get a lot of warnings from this statement and the one added by the previous patch for zacas, which is a C23 extension: include/linux/atomic/atomic-arch-fallback.h:4234:9: warning: label at end of compound statement is a C23 extension [-Wc23-extensions] include/linux/atomic/atomic-arch-fallback.h:89:29: note: expanded from macro 'raw_cmpxchg_relaxed' 89 | #define raw_cmpxchg_relaxed arch_cmpxchg_relaxed | ^ arch/riscv/include/asm/cmpxchg.h:219:2: note: expanded from macro 'arch_cmpxchg_relaxed' 219 | _arch_cmpxchg((ptr), (o), (n), "", "", "") | ^ arch/riscv/include/asm/cmpxchg.h:200:3: note: expanded from macro '_arch_cmpxchg' 200 | __arch_cmpxchg_masked(sc_sfx, ".h" sc_sfx, \ | ^ arch/riscv/include/asm/cmpxchg.h:150:14: note: expanded from macro '__arch_cmpxchg_masked' 150 | end: \ | ^ This resolves it: diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h index ba3ffc2fcdd0..57aa4a554278 100644 --- a/arch/riscv/include/asm/cmpxchg.h +++ b/arch/riscv/include/asm/cmpxchg.h @@ -147,7 +147,7 @@ zabha: \ : "+&r" (r), "+A" (*(p)) \ : "rJ" (n) \ : "memory"); \ -end: \ +end:; \ }) #define __arch_cmpxchg(lr_sfx, sc_cas_sfx, prepend, append, r, p, co, o, n) \ @@ -180,7 +180,7 @@ zacas: \ : "+&r" (r), "+A" (*(p)) \ : "rJ" (n) \ : "memory"); \ -end: \ +end:; \ }) #define _arch_cmpxchg(ptr, old, new, sc_sfx, prepend, append) \ > }) > > #define __arch_cmpxchg(lr_sfx, sc_cas_sfx, prepend, append, r, p, co, o, n) \ > @@ -175,8 +192,13 @@ end: \ > \ > switch (sizeof(*__ptr)) { \ > case 1: \ > + __arch_cmpxchg_masked(sc_sfx, ".b" sc_sfx, \ > + prepend, append, \ > + __ret, __ptr, __old, __new); \ > + break; \ > case 2: \ > - __arch_cmpxchg_masked(sc_sfx, prepend, append, \ > + __arch_cmpxchg_masked(sc_sfx, ".h" sc_sfx, \ > + prepend, append, \ > __ret, __ptr, __old, __new); \ > break; \ > case 4: \ > -- > 2.39.2 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/linux-riscv