| Message ID | 20240528151052.313031-3-alexghiti@rivosinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Zacas/Zabha support and qspinlocks | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/vmtest-fixes-PR | fail | merge-conflict |
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@rivosinc.com> > --- > 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@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
> +zabha: \ > + __asm__ __volatile__ ( \ > + prepend \ > + " amocas" cas_sfx " %0, %z2, %1\n" \ > + append \ > + : "+&r" (r), "+A" (*(p)) \ > + : "rJ" (n) \ > + : "memory"); \ Couldn't a platform have Zabha but not have Zacas? I don't see how this asm goto could work in such case, what am I missing? Andrea
On Wed, May 29, 2024 at 1:54 AM Andrea Parri <parri.andrea@gmail.com> wrote: > > > +zabha: \ > > + __asm__ __volatile__ ( \ > > + prepend \ > > + " amocas" cas_sfx " %0, %z2, %1\n" \ > > + append \ > > + : "+&r" (r), "+A" (*(p)) \ > > + : "rJ" (n) \ > > + : "memory"); \ > > Couldn't a platform have Zabha but not have Zacas? I don't see how this > asm goto could work in such case, what am I missing? Zabha amocas.[b|h] instructions are only implemented if Zacas is present, as the specification states: "If Zacas [2] extension is also implemented, Zabha further provides the AMOCAS.[B|H] instructions." But the code you mention is only for 8 and 16bit operations, so I think we are good anyway? Thanks, Alex > > Andrea
Hi Nathan, On Tue, May 28, 2024 at 9:31 PM Nathan Chancellor <nathan@kernel.org> wrote: > > 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@rivosinc.com> > > --- > > 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. Then I missed that, I should have checked the generated code. Is the extension version "1p0" in '-march=' only required for experimental extensions? > > 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. But from Conor comment here [1], we should not enable extensions that are only experimental. In that case, we should be good with this. [1] https://lore.kernel.org/linux-riscv/20240528151052.313031-1-alexghiti@rivosinc.com/T/#mefb283477bce852f3713cbbb4ff002252281c9d5 > > > + 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. Ok, I will fix that, thanks. > > 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 That's a good idea to me, let's see what Conor thinks [2] [2] https://lore.kernel.org/linux-riscv/20240528151052.313031-1-alexghiti@rivosinc.com/T/#m1d798dfc4c27e5b6d9e14117d81b577ace123322 > > > +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)' Indeed, my test setup lacks a few things apparently, I will fix that, thanks. > > > + 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) \ Weird, I missed this too, I will fix that, thanks. > > > }) > > > > #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@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv Thanks for your thorough review! Alex
On Wed, May 29, 2024 at 2:29 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote: > > On Wed, May 29, 2024 at 1:54 AM Andrea Parri <parri.andrea@gmail.com> wrote: > > > > > +zabha: \ > > > + __asm__ __volatile__ ( \ > > > + prepend \ > > > + " amocas" cas_sfx " %0, %z2, %1\n" \ > > > + append \ > > > + : "+&r" (r), "+A" (*(p)) \ > > > + : "rJ" (n) \ > > > + : "memory"); \ > > > > Couldn't a platform have Zabha but not have Zacas? I don't see how this > > asm goto could work in such case, what am I missing? > > Zabha amocas.[b|h] instructions are only implemented if Zacas is > present, as the specification states: "If Zacas [2] extension is also > implemented, Zabha further provides the AMOCAS.[B|H] instructions." > > But the code you mention is only for 8 and 16bit operations, so I > think we are good anyway? And I was wrong like Andrea noted privately. So I'll fix that too, thanks! > > Thanks, > > Alex > > > > > Andrea
On Wed, May 29, 2024 at 02:49:58PM +0200, Alexandre Ghiti wrote: > Then I missed that, I should have checked the generated code. Is the > extension version "1p0" in '-march=' only required for experimental > extensions? I think so, if my understanding of the message is correct. > But from Conor comment here [1], we should not enable extensions that > are only experimental. In that case, we should be good with this. > > [1] https://lore.kernel.org/linux-riscv/20240528151052.313031-1-alexghiti@rivosinc.com/T/#mefb283477bce852f3713cbbb4ff002252281c9d5 Yeah, I tend to agree with Conor on that front. I had not noticed that part of the message when I was looking at other parts of this thread. I could see an argument for allowing experimental extensions for qualification purposes but I think it does create a bit of a support nightmare, especially when there are breaking changes across revisions. > > 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 Perhaps with that in mind, maybe EXPERIMENTAL_EXTENSIONS (or whatever) should be a user selectable option and the TOOLCHAIN values depend on it when the user has a clang version that does not support the ratified version. > That's a good idea to me, let's see what Conor thinks [2] > > [2] https://lore.kernel.org/linux-riscv/20240528151052.313031-1-alexghiti@rivosinc.com/T/#m1d798dfc4c27e5b6d9e14117d81b577ace123322 FWIW, I think your plan of removing support for the experimental version of the extension and pushing to remove the experimental status in LLVM (especially since it seems like it is ratified like zacas? https://jira.riscv.org/browse/RVS-1685) is probably the best thing going forward. If the LLVM folks are made aware soon, it should be easy to get that change into clang-19, which is branching at the end of July I believe. > Thanks for your thorough review! Thanks for taking LLVM support into consideration :) Cheers, Nathan
Hi Conor, Nathan, On 29/05/2024 17:57, Nathan Chancellor wrote: > On Wed, May 29, 2024 at 02:49:58PM +0200, Alexandre Ghiti wrote: >> Then I missed that, I should have checked the generated code. Is the >> extension version "1p0" in '-march=' only required for experimental >> extensions? > I think so, if my understanding of the message is correct. > >> But from Conor comment here [1], we should not enable extensions that >> are only experimental. In that case, we should be good with this. >> >> [1] https://lore.kernel.org/linux-riscv/20240528151052.313031-1-alexghiti@rivosinc.com/T/#mefb283477bce852f3713cbbb4ff002252281c9d5 > Yeah, I tend to agree with Conor on that front. I had not noticed that > part of the message when I was looking at other parts of this thread. I > could see an argument for allowing experimental extensions for > qualification purposes but I think it does create a bit of a support > nightmare, especially when there are breaking changes across revisions. > >>> 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 > Perhaps with that in mind, maybe EXPERIMENTAL_EXTENSIONS (or whatever) > should be a user selectable option and the TOOLCHAIN values depend on it > when the user has a clang version that does not support the ratified > version. > >> That's a good idea to me, let's see what Conor thinks [2] >> >> [2] https://lore.kernel.org/linux-riscv/20240528151052.313031-1-alexghiti@rivosinc.com/T/#m1d798dfc4c27e5b6d9e14117d81b577ace123322 > FWIW, I think your plan of removing support for the experimental version > of the extension and pushing to remove the experimental status in LLVM > (especially since it seems like it is ratified like zacas? > https://jira.riscv.org/browse/RVS-1685) is probably the best thing going > forward. If the LLVM folks are made aware soon, it should be easy to get > that change into clang-19, which is branching at the end of July I > believe. FYI, it was just merged https://github.com/llvm/llvm-project/pull/93831 Thanks again, Alex > >> Thanks for your thorough review! > Thanks for taking LLVM support into consideration :) > > Cheers, > Nathan > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
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) + 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 +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" \ + append \ + : "+&r" (r), "+A" (*(p)) \ + : "rJ" (n) \ + : "memory"); \ +end: \ }) #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: \
This adds runtime support for Zabha in cmpxchg8/16 operations. Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> --- arch/riscv/Kconfig | 16 ++++++++++++++++ arch/riscv/Makefile | 10 ++++++++++ arch/riscv/include/asm/cmpxchg.h | 26 ++++++++++++++++++++++++-- 3 files changed, 50 insertions(+), 2 deletions(-)