diff mbox series

[v3,03/11] riscv: Implement cmpxchg8/16() using Zabha

Message ID 20240717061957.140712-4-alexghiti@rivosinc.com (mailing list archive)
State Superseded
Headers show
Series Zacas/Zabha support and qspinlocks | expand

Checks

Context Check Description
conchuod/vmtest-fixes-PR fail PR summary
conchuod/patch-3-test-1 success .github/scripts/patches/tests/build_rv32_defconfig.sh
conchuod/patch-3-test-2 success .github/scripts/patches/tests/build_rv64_clang_allmodconfig.sh
conchuod/patch-3-test-3 success .github/scripts/patches/tests/build_rv64_gcc_allmodconfig.sh
conchuod/patch-3-test-4 success .github/scripts/patches/tests/build_rv64_nommu_k210_defconfig.sh
conchuod/patch-3-test-5 success .github/scripts/patches/tests/build_rv64_nommu_virt_defconfig.sh
conchuod/patch-3-test-6 warning .github/scripts/patches/tests/checkpatch.sh
conchuod/patch-3-test-7 success .github/scripts/patches/tests/dtb_warn_rv64.sh
conchuod/patch-3-test-8 success .github/scripts/patches/tests/header_inline.sh
conchuod/patch-3-test-9 success .github/scripts/patches/tests/kdoc.sh
conchuod/patch-3-test-10 success .github/scripts/patches/tests/module_param.sh
conchuod/patch-3-test-11 success .github/scripts/patches/tests/verify_fixes.sh
conchuod/patch-3-test-12 success .github/scripts/patches/tests/verify_signedoff.sh

Commit Message

Alexandre Ghiti July 17, 2024, 6:19 a.m. UTC
This adds runtime support for Zabha in cmpxchg8/16() operations.

Note that in the absence of Zacas support in the toolchain, CAS
instructions from Zabha won't be used.

Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
 arch/riscv/Kconfig               | 17 ++++++++++++++++
 arch/riscv/Makefile              |  3 +++
 arch/riscv/include/asm/cmpxchg.h | 33 ++++++++++++++++++++++++++++++--
 arch/riscv/include/asm/hwcap.h   |  1 +
 arch/riscv/kernel/cpufeature.c   |  1 +
 5 files changed, 53 insertions(+), 2 deletions(-)

Comments

Andrew Jones July 17, 2024, 3:26 p.m. UTC | #1
On Wed, Jul 17, 2024 at 08:19:49AM GMT, Alexandre Ghiti wrote:
> This adds runtime support for Zabha in cmpxchg8/16() operations.
> 
> Note that in the absence of Zacas support in the toolchain, CAS
> instructions from Zabha won't be used.
> 
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> ---
>  arch/riscv/Kconfig               | 17 ++++++++++++++++
>  arch/riscv/Makefile              |  3 +++
>  arch/riscv/include/asm/cmpxchg.h | 33 ++++++++++++++++++++++++++++++--
>  arch/riscv/include/asm/hwcap.h   |  1 +
>  arch/riscv/kernel/cpufeature.c   |  1 +
>  5 files changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 1caaedec88c7..d3b0f92f92da 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -596,6 +596,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_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/halfword operations"
> +	depends on TOOLCHAIN_HAS_ZABHA
> +	default y
> +	help
> +	  Enable the use of the Zabha ISA-extension to implement kernel
> +	  byte/halfword atomic memory operations when it is detected at boot.
> +
> +	  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 9fd13d7a9cc6..78dcaaeebf4e 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -88,6 +88,9 @@ riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE) := $(riscv-march-y)_zihintpause
>  # Check if the toolchain supports Zacas
>  riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZACAS) := $(riscv-march-y)_zacas
>  
> +# Check if the toolchain supports Zabha
> +riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZABHA) := $(riscv-march-y)_zabha
> +
>  # 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 5d38153e2f13..c86722a101d0 100644
> --- a/arch/riscv/include/asm/cmpxchg.h
> +++ b/arch/riscv/include/asm/cmpxchg.h
> @@ -105,8 +105,30 @@
>   * 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__ no_zabha_zacas, end;					\
> +									\
> +	if (IS_ENABLED(CONFIG_RISCV_ISA_ZABHA) &&			\
> +	    IS_ENABLED(CONFIG_RISCV_ISA_ZACAS)) {			\
> +		asm goto(ALTERNATIVE("j %[no_zabha_zacas]", "nop", 0,	\
> +				     RISCV_ISA_EXT_ZABHA, 1)		\
> +			 : : : : no_zabha_zacas);			\
> +		asm goto(ALTERNATIVE("j %[no_zabha_zacas]", "nop", 0,	\
> +				     RISCV_ISA_EXT_ZACAS, 1)		\
> +			 : : : : no_zabha_zacas);			\

I came late to the call, but I guess trying to get rid of these asm gotos
was the topic of the discussion. The proposal was to try and use static
branches, but keep in mind that we've had trouble with static branches
inside macros in the past when those macros are used in many places[1]

[1] commit 0b1d60d6dd9e ("riscv: Fix build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y")

> +									\
> +		__asm__ __volatile__ (					\
> +			prepend						\
> +			"	amocas" cas_sfx " %0, %z2, %1\n"	\
> +			append						\
> +			: "+&r" (r), "+A" (*(p))			\
> +			: "rJ" (n)					\
> +			: "memory");					\
> +		goto end;						\
> +	}								\
> +									\
> +no_zabha_zacas:;							\

unnecessary ;

>  	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)	\
> @@ -133,6 +155,8 @@
>  		: "memory");						\
>  									\
>  	r = (__typeof__(*(p)))((__retx & __mask) >> __s);		\
> +									\
> +end:;									\
>  })
>  
>  #define __arch_cmpxchg(lr_sfx, sc_cas_sfx, prepend, append, r, p, co, o, n)	\
> @@ -180,8 +204,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:								\
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index e17d0078a651..f71ddd2ca163 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -81,6 +81,7 @@
>  #define RISCV_ISA_EXT_ZTSO		72
>  #define RISCV_ISA_EXT_ZACAS		73
>  #define RISCV_ISA_EXT_XANDESPMU		74
> +#define RISCV_ISA_EXT_ZABHA		75
>  
>  #define RISCV_ISA_EXT_XLINUXENVCFG	127
>  
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 5ef48cb20ee1..c125d82c894b 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -257,6 +257,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
>  	__RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
>  	__RISCV_ISA_EXT_DATA(zihpm, RISCV_ISA_EXT_ZIHPM),
>  	__RISCV_ISA_EXT_DATA(zacas, RISCV_ISA_EXT_ZACAS),
> +	__RISCV_ISA_EXT_DATA(zabha, RISCV_ISA_EXT_ZABHA),
>  	__RISCV_ISA_EXT_DATA(zfa, RISCV_ISA_EXT_ZFA),
>  	__RISCV_ISA_EXT_DATA(zfh, RISCV_ISA_EXT_ZFH),
>  	__RISCV_ISA_EXT_DATA(zfhmin, RISCV_ISA_EXT_ZFHMIN),
> -- 
> 2.39.2
>

Thanks,
drew
Conor Dooley July 17, 2024, 3:29 p.m. UTC | #2
On Wed, Jul 17, 2024 at 10:26:34AM -0500, Andrew Jones wrote:
> On Wed, Jul 17, 2024 at 08:19:49AM GMT, Alexandre Ghiti wrote:
> > -#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__ no_zabha_zacas, end;					\
> > +									\
> > +	if (IS_ENABLED(CONFIG_RISCV_ISA_ZABHA) &&			\
> > +	    IS_ENABLED(CONFIG_RISCV_ISA_ZACAS)) {			\
> > +		asm goto(ALTERNATIVE("j %[no_zabha_zacas]", "nop", 0,	\
> > +				     RISCV_ISA_EXT_ZABHA, 1)		\
> > +			 : : : : no_zabha_zacas);			\
> > +		asm goto(ALTERNATIVE("j %[no_zabha_zacas]", "nop", 0,	\
> > +				     RISCV_ISA_EXT_ZACAS, 1)		\
> > +			 : : : : no_zabha_zacas);			\
> 
> I came late to the call, but I guess trying to get rid of these asm gotos
> was the topic of the discussion. The proposal was to try and use static
> branches, but keep in mind that we've had trouble with static branches
> inside macros in the past when those macros are used in many places[1]
> 
> [1] commit 0b1d60d6dd9e ("riscv: Fix build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y")

The other half of the suggestion was not using an asm goto, but instead
trying to patch the whole thing in the alternative, for the problematic
section with llvm < 17.

> 
> > +									\
> > +		__asm__ __volatile__ (					\
> > +			prepend						\
> > +			"	amocas" cas_sfx " %0, %z2, %1\n"	\
> > +			append						\
> > +			: "+&r" (r), "+A" (*(p))			\
> > +			: "rJ" (n)					\
> > +			: "memory");					\
> > +		goto end;						\
> > +	}								\
> > +									\
> > +no_zabha_zacas:;							\
> 
> unnecessary ;
> 
> >  	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)	\
> > @@ -133,6 +155,8 @@
> >  		: "memory");						\
> >  									\
> >  	r = (__typeof__(*(p)))((__retx & __mask) >> __s);		\
> > +									\
> > +end:;									\
> >  })
> >  
> >  #define __arch_cmpxchg(lr_sfx, sc_cas_sfx, prepend, append, r, p, co, o, n)	\
Alexandre Ghiti July 17, 2024, 3:34 p.m. UTC | #3
On 17/07/2024 17:29, Conor Dooley wrote:
> On Wed, Jul 17, 2024 at 10:26:34AM -0500, Andrew Jones wrote:
>> On Wed, Jul 17, 2024 at 08:19:49AM GMT, Alexandre Ghiti wrote:
>>> -#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__ no_zabha_zacas, end;					\
>>> +									\
>>> +	if (IS_ENABLED(CONFIG_RISCV_ISA_ZABHA) &&			\
>>> +	    IS_ENABLED(CONFIG_RISCV_ISA_ZACAS)) {			\
>>> +		asm goto(ALTERNATIVE("j %[no_zabha_zacas]", "nop", 0,	\
>>> +				     RISCV_ISA_EXT_ZABHA, 1)		\
>>> +			 : : : : no_zabha_zacas);			\
>>> +		asm goto(ALTERNATIVE("j %[no_zabha_zacas]", "nop", 0,	\
>>> +				     RISCV_ISA_EXT_ZACAS, 1)		\
>>> +			 : : : : no_zabha_zacas);			\
>> I came late to the call, but I guess trying to get rid of these asm gotos
>> was the topic of the discussion. The proposal was to try and use static
>> branches, but keep in mind that we've had trouble with static branches
>> inside macros in the past when those macros are used in many places[1]
>>
>> [1] commit 0b1d60d6dd9e ("riscv: Fix build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y")
> The other half of the suggestion was not using an asm goto, but instead
> trying to patch the whole thing in the alternative, for the problematic
> section with llvm < 17.


And I'm not a big fan of this solution since it would imply patching the 
5-7 instructions for LR/SC into nops which would probably slow (a bit) 
the amocas/amoswap sequence. I agree it should not be that big, but that 
it is just to fix an llvm issue, so not worth it to me!


>>> +									\
>>> +		__asm__ __volatile__ (					\
>>> +			prepend						\
>>> +			"	amocas" cas_sfx " %0, %z2, %1\n"	\
>>> +			append						\
>>> +			: "+&r" (r), "+A" (*(p))			\
>>> +			: "rJ" (n)					\
>>> +			: "memory");					\
>>> +		goto end;						\
>>> +	}								\
>>> +									\
>>> +no_zabha_zacas:;							\
>> unnecessary ;
>>
>>>   	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)	\
>>> @@ -133,6 +155,8 @@
>>>   		: "memory");						\
>>>   									\
>>>   	r = (__typeof__(*(p)))((__retx & __mask) >> __s);		\
>>> +									\
>>> +end:;									\
>>>   })
>>>   
>>>   #define __arch_cmpxchg(lr_sfx, sc_cas_sfx, prepend, append, r, p, co, o, n)	\
Alexandre Ghiti July 18, 2024, 12:50 p.m. UTC | #4
Hi Drew,

On 17/07/2024 17:26, Andrew Jones wrote:
> On Wed, Jul 17, 2024 at 08:19:49AM GMT, Alexandre Ghiti wrote:
>> This adds runtime support for Zabha in cmpxchg8/16() operations.
>>
>> Note that in the absence of Zacas support in the toolchain, CAS
>> instructions from Zabha won't be used.
>>
>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>> ---
>>   arch/riscv/Kconfig               | 17 ++++++++++++++++
>>   arch/riscv/Makefile              |  3 +++
>>   arch/riscv/include/asm/cmpxchg.h | 33 ++++++++++++++++++++++++++++++--
>>   arch/riscv/include/asm/hwcap.h   |  1 +
>>   arch/riscv/kernel/cpufeature.c   |  1 +
>>   5 files changed, 53 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 1caaedec88c7..d3b0f92f92da 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -596,6 +596,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_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/halfword operations"
>> +	depends on TOOLCHAIN_HAS_ZABHA
>> +	default y
>> +	help
>> +	  Enable the use of the Zabha ISA-extension to implement kernel
>> +	  byte/halfword atomic memory operations when it is detected at boot.
>> +
>> +	  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 9fd13d7a9cc6..78dcaaeebf4e 100644
>> --- a/arch/riscv/Makefile
>> +++ b/arch/riscv/Makefile
>> @@ -88,6 +88,9 @@ riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE) := $(riscv-march-y)_zihintpause
>>   # Check if the toolchain supports Zacas
>>   riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZACAS) := $(riscv-march-y)_zacas
>>   
>> +# Check if the toolchain supports Zabha
>> +riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZABHA) := $(riscv-march-y)_zabha
>> +
>>   # 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 5d38153e2f13..c86722a101d0 100644
>> --- a/arch/riscv/include/asm/cmpxchg.h
>> +++ b/arch/riscv/include/asm/cmpxchg.h
>> @@ -105,8 +105,30 @@
>>    * 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__ no_zabha_zacas, end;					\
>> +									\
>> +	if (IS_ENABLED(CONFIG_RISCV_ISA_ZABHA) &&			\
>> +	    IS_ENABLED(CONFIG_RISCV_ISA_ZACAS)) {			\
>> +		asm goto(ALTERNATIVE("j %[no_zabha_zacas]", "nop", 0,	\
>> +				     RISCV_ISA_EXT_ZABHA, 1)		\
>> +			 : : : : no_zabha_zacas);			\
>> +		asm goto(ALTERNATIVE("j %[no_zabha_zacas]", "nop", 0,	\
>> +				     RISCV_ISA_EXT_ZACAS, 1)		\
>> +			 : : : : no_zabha_zacas);			\
> I came late to the call, but I guess trying to get rid of these asm gotos
> was the topic of the discussion. The proposal was to try and use static
> branches, but keep in mind that we've had trouble with static branches
> inside macros in the past when those macros are used in many places[1]
>
> [1] commit 0b1d60d6dd9e ("riscv: Fix build with CONFIG_CC_OPTIMIZE_FOR_SIZE=y")


Thanks for the pointer, I was not aware of this. I came up with a 
solution with preprocessor guards, not the prettiest solution but at 
least it does not create more problems :)


>
>> +									\
>> +		__asm__ __volatile__ (					\
>> +			prepend						\
>> +			"	amocas" cas_sfx " %0, %z2, %1\n"	\
>> +			append						\
>> +			: "+&r" (r), "+A" (*(p))			\
>> +			: "rJ" (n)					\
>> +			: "memory");					\
>> +		goto end;						\
>> +	}								\
>> +									\
>> +no_zabha_zacas:;							\
> unnecessary ;


Actually it is, it fixes a warning encountered on llvm: 
https://lore.kernel.org/linux-riscv/20240528193110.GA2196855@thelio-3990X/

Thanks,

Alex


>
>>   	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)	\
>> @@ -133,6 +155,8 @@
>>   		: "memory");						\
>>   									\
>>   	r = (__typeof__(*(p)))((__retx & __mask) >> __s);		\
>> +									\
>> +end:;									\
>>   })
>>   
>>   #define __arch_cmpxchg(lr_sfx, sc_cas_sfx, prepend, append, r, p, co, o, n)	\
>> @@ -180,8 +204,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:								\
>> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
>> index e17d0078a651..f71ddd2ca163 100644
>> --- a/arch/riscv/include/asm/hwcap.h
>> +++ b/arch/riscv/include/asm/hwcap.h
>> @@ -81,6 +81,7 @@
>>   #define RISCV_ISA_EXT_ZTSO		72
>>   #define RISCV_ISA_EXT_ZACAS		73
>>   #define RISCV_ISA_EXT_XANDESPMU		74
>> +#define RISCV_ISA_EXT_ZABHA		75
>>   
>>   #define RISCV_ISA_EXT_XLINUXENVCFG	127
>>   
>> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>> index 5ef48cb20ee1..c125d82c894b 100644
>> --- a/arch/riscv/kernel/cpufeature.c
>> +++ b/arch/riscv/kernel/cpufeature.c
>> @@ -257,6 +257,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
>>   	__RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
>>   	__RISCV_ISA_EXT_DATA(zihpm, RISCV_ISA_EXT_ZIHPM),
>>   	__RISCV_ISA_EXT_DATA(zacas, RISCV_ISA_EXT_ZACAS),
>> +	__RISCV_ISA_EXT_DATA(zabha, RISCV_ISA_EXT_ZABHA),
>>   	__RISCV_ISA_EXT_DATA(zfa, RISCV_ISA_EXT_ZFA),
>>   	__RISCV_ISA_EXT_DATA(zfh, RISCV_ISA_EXT_ZFH),
>>   	__RISCV_ISA_EXT_DATA(zfhmin, RISCV_ISA_EXT_ZFHMIN),
>> -- 
>> 2.39.2
>>
> Thanks,
> drew
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
Andrew Jones July 18, 2024, 4:06 p.m. UTC | #5
On Thu, Jul 18, 2024 at 02:50:28PM GMT, Alexandre Ghiti wrote:
...
> > > +									\
> > > +		__asm__ __volatile__ (					\
> > > +			prepend						\
> > > +			"	amocas" cas_sfx " %0, %z2, %1\n"	\
> > > +			append						\
> > > +			: "+&r" (r), "+A" (*(p))			\
> > > +			: "rJ" (n)					\
> > > +			: "memory");					\
> > > +		goto end;						\
> > > +	}								\
> > > +									\
> > > +no_zabha_zacas:;							\
> > unnecessary ;
> 
> 
> Actually it is, it fixes a warning encountered on llvm:
> https://lore.kernel.org/linux-riscv/20240528193110.GA2196855@thelio-3990X/

I'm not complaining about the 'end:' label. That one we need ';' because
there's no following statement and labels must be followed by a statement.
But no_zabha_zacas always has following statements.

Thanks,
drew
Alexandre Ghiti July 18, 2024, 4:20 p.m. UTC | #6
On Thu, Jul 18, 2024 at 6:06 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Thu, Jul 18, 2024 at 02:50:28PM GMT, Alexandre Ghiti wrote:
> ...
> > > > +                                                                 \
> > > > +         __asm__ __volatile__ (                                  \
> > > > +                 prepend                                         \
> > > > +                 "       amocas" cas_sfx " %0, %z2, %1\n"        \
> > > > +                 append                                          \
> > > > +                 : "+&r" (r), "+A" (*(p))                        \
> > > > +                 : "rJ" (n)                                      \
> > > > +                 : "memory");                                    \
> > > > +         goto end;                                               \
> > > > + }                                                               \
> > > > +                                                                 \
> > > > +no_zabha_zacas:;                                                 \
> > > unnecessary ;
> >
> >
> > Actually it is, it fixes a warning encountered on llvm:
> > https://lore.kernel.org/linux-riscv/20240528193110.GA2196855@thelio-3990X/
>
> I'm not complaining about the 'end:' label. That one we need ';' because
> there's no following statement and labels must be followed by a statement.
> But no_zabha_zacas always has following statements.

My bad, that's another warning that is emitted by llvm and requires the ';':

../include/linux/atomic/atomic-arch-fallback.h:2026:9: warning: label
followed by a declaration is a C23 extension [-Wc23-extensions]
 2026 |         return raw_cmpxchg(&v->counter, old, new);
      |                ^
../include/linux/atomic/atomic-arch-fallback.h:55:21: note: expanded
from macro 'raw_cmpxchg'
   55 | #define raw_cmpxchg arch_cmpxchg
      |                     ^
../arch/riscv/include/asm/cmpxchg.h:310:2: note: expanded from macro
'arch_cmpxchg'
  310 |         _arch_cmpxchg((ptr), (o), (n), ".rl", ".aqrl",
         \
      |         ^
../arch/riscv/include/asm/cmpxchg.h:269:3: note: expanded from macro
'_arch_cmpxchg'
  269 |                 __arch_cmpxchg_masked(sc_sfx, ".b" cas_sfx,
         \
      |                 ^
../arch/riscv/include/asm/cmpxchg.h:178:2: note: expanded from macro
'__arch_cmpxchg_masked'
  178 |         u32 *__ptr32b = (u32 *)((ulong)(p) & ~0x3);
         \
      |         ^


>
> Thanks,
> drew
diff mbox series

Patch

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 1caaedec88c7..d3b0f92f92da 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -596,6 +596,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_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/halfword operations"
+	depends on TOOLCHAIN_HAS_ZABHA
+	default y
+	help
+	  Enable the use of the Zabha ISA-extension to implement kernel
+	  byte/halfword atomic memory operations when it is detected at boot.
+
+	  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 9fd13d7a9cc6..78dcaaeebf4e 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -88,6 +88,9 @@  riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE) := $(riscv-march-y)_zihintpause
 # Check if the toolchain supports Zacas
 riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZACAS) := $(riscv-march-y)_zacas
 
+# Check if the toolchain supports Zabha
+riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZABHA) := $(riscv-march-y)_zabha
+
 # 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 5d38153e2f13..c86722a101d0 100644
--- a/arch/riscv/include/asm/cmpxchg.h
+++ b/arch/riscv/include/asm/cmpxchg.h
@@ -105,8 +105,30 @@ 
  * 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__ no_zabha_zacas, end;					\
+									\
+	if (IS_ENABLED(CONFIG_RISCV_ISA_ZABHA) &&			\
+	    IS_ENABLED(CONFIG_RISCV_ISA_ZACAS)) {			\
+		asm goto(ALTERNATIVE("j %[no_zabha_zacas]", "nop", 0,	\
+				     RISCV_ISA_EXT_ZABHA, 1)		\
+			 : : : : no_zabha_zacas);			\
+		asm goto(ALTERNATIVE("j %[no_zabha_zacas]", "nop", 0,	\
+				     RISCV_ISA_EXT_ZACAS, 1)		\
+			 : : : : no_zabha_zacas);			\
+									\
+		__asm__ __volatile__ (					\
+			prepend						\
+			"	amocas" cas_sfx " %0, %z2, %1\n"	\
+			append						\
+			: "+&r" (r), "+A" (*(p))			\
+			: "rJ" (n)					\
+			: "memory");					\
+		goto end;						\
+	}								\
+									\
+no_zabha_zacas:;							\
 	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)	\
@@ -133,6 +155,8 @@ 
 		: "memory");						\
 									\
 	r = (__typeof__(*(p)))((__retx & __mask) >> __s);		\
+									\
+end:;									\
 })
 
 #define __arch_cmpxchg(lr_sfx, sc_cas_sfx, prepend, append, r, p, co, o, n)	\
@@ -180,8 +204,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:								\
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index e17d0078a651..f71ddd2ca163 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -81,6 +81,7 @@ 
 #define RISCV_ISA_EXT_ZTSO		72
 #define RISCV_ISA_EXT_ZACAS		73
 #define RISCV_ISA_EXT_XANDESPMU		74
+#define RISCV_ISA_EXT_ZABHA		75
 
 #define RISCV_ISA_EXT_XLINUXENVCFG	127
 
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 5ef48cb20ee1..c125d82c894b 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -257,6 +257,7 @@  const struct riscv_isa_ext_data riscv_isa_ext[] = {
 	__RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
 	__RISCV_ISA_EXT_DATA(zihpm, RISCV_ISA_EXT_ZIHPM),
 	__RISCV_ISA_EXT_DATA(zacas, RISCV_ISA_EXT_ZACAS),
+	__RISCV_ISA_EXT_DATA(zabha, RISCV_ISA_EXT_ZABHA),
 	__RISCV_ISA_EXT_DATA(zfa, RISCV_ISA_EXT_ZFA),
 	__RISCV_ISA_EXT_DATA(zfh, RISCV_ISA_EXT_ZFH),
 	__RISCV_ISA_EXT_DATA(zfhmin, RISCV_ISA_EXT_ZFHMIN),