Received-SPF: pass (google.com: domain of linux-kernel+bounces-195323-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Thu, 30 May 2024 13:54:43 +0200
From: Andrea Parri <parri.andrea@gmail.com>
To: Alexandre Ghiti <alexghiti@rivosinc.com>
Cc: Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Leonardo Bras <leobras@redhat.com>, linux-riscv@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH -fixes] riscv: Fix fully ordered LR/SC xchg[8|16]()
 implementations
Message-ID: <ZlhpA9NsgI0z6t/E@andrea>
References: <20240530075424.380557-1-alexghiti@rivosinc.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240530075424.380557-1-alexghiti@rivosinc.com>

> -#define _arch_xchg(ptr, new, sfx, prepend, append)			\
> +#define _arch_xchg(ptr, new, sc_sfx, swap_sfx, prepend, append)		\
>  ({									\
>  	__typeof__(ptr) __ptr = (ptr);					\
>  	__typeof__(*(__ptr)) __new = (new);				\
> @@ -55,15 +55,15 @@
>  	switch (sizeof(*__ptr)) {					\
>  	case 1:								\
>  	case 2:								\
> -		__arch_xchg_masked(prepend, append,			\
> +		__arch_xchg_masked(sc_sfx, prepend, append,		\
>  				   __ret, __ptr, __new);		\
>  		break;							\
>  	case 4:								\
> -		__arch_xchg(".w" sfx, prepend, append,			\
> +		__arch_xchg(".w" swap_sfx, prepend, append,		\
>  			      __ret, __ptr, __new);			\
>  		break;							\
>  	case 8:								\
> -		__arch_xchg(".d" sfx, prepend, append,			\
> +		__arch_xchg(".d" swap_sfx, prepend, append,		\
>  			      __ret, __ptr, __new);			\
>  		break;							\
>  	default:							\
> @@ -73,16 +73,16 @@
>  })
>  
>  #define arch_xchg_relaxed(ptr, x)					\
> -	_arch_xchg(ptr, x, "", "", "")
> +	_arch_xchg(ptr, x, "", "", "", "")
>  
>  #define arch_xchg_acquire(ptr, x)					\
> -	_arch_xchg(ptr, x, "", "", RISCV_ACQUIRE_BARRIER)
> +	_arch_xchg(ptr, x, "", "", "", RISCV_ACQUIRE_BARRIER)
>  
>  #define arch_xchg_release(ptr, x)					\
> -	_arch_xchg(ptr, x, "", RISCV_RELEASE_BARRIER, "")
> +	_arch_xchg(ptr, x, "", "", RISCV_RELEASE_BARRIER, "")
>  
>  #define arch_xchg(ptr, x)						\
> -	_arch_xchg(ptr, x, ".aqrl", "", "")
> +	_arch_xchg(ptr, x, ".rl", ".aqrl", "", "     fence rw, rw\n")

This does indeed fix the fully-ordered variant of xchg8/16().  But this
also changes the fully-ordered xchg32() to

  amoswap.w.aqrl  a4,a5,(s1)
  fence   rw,rw

(and similarly for xchg64()); we should be able to restore the original
mapping with the diff below on top of this patch.

  Andrea

P.S. Perhaps expand the width of the macros to avoid newlines (I didn't
do it keep the diff smaller).

P.S. With Zabha, we'd probably like to pass swap_sfx and swap_append as
well to __arch_xchg_masked().


diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
index e1e564f5dc7ba..88c8bb7ec1c34 100644
--- a/arch/riscv/include/asm/cmpxchg.h
+++ b/arch/riscv/include/asm/cmpxchg.h
@@ -46,7 +46,8 @@
 		: "memory");						\
 })
 
-#define _arch_xchg(ptr, new, sc_sfx, swap_sfx, prepend, append)		\
+#define _arch_xchg(ptr, new, sc_sfx, swap_sfx, prepend,			\
+		   sc_append, swap_append)				\
 ({									\
 	__typeof__(ptr) __ptr = (ptr);					\
 	__typeof__(*(__ptr)) __new = (new);				\
@@ -55,15 +56,15 @@
 	switch (sizeof(*__ptr)) {					\
 	case 1:								\
 	case 2:								\
-		__arch_xchg_masked(sc_sfx, prepend, append,		\
+		__arch_xchg_masked(sc_sfx, prepend, sc_append,		\
 				   __ret, __ptr, __new);		\
 		break;							\
 	case 4:								\
-		__arch_xchg(".w" swap_sfx, prepend, append,		\
+		__arch_xchg(".w" swap_sfx, prepend, swap_append,	\
 			      __ret, __ptr, __new);			\
 		break;							\
 	case 8:								\
-		__arch_xchg(".d" swap_sfx, prepend, append,		\
+		__arch_xchg(".d" swap_sfx, prepend, swap_append,	\
 			      __ret, __ptr, __new);			\
 		break;							\
 	default:							\
@@ -73,16 +74,16 @@
 })
 
 #define arch_xchg_relaxed(ptr, x)					\
-	_arch_xchg(ptr, x, "", "", "", "")
+	_arch_xchg(ptr, x, "", "", "", "", "")
 
 #define arch_xchg_acquire(ptr, x)					\
-	_arch_xchg(ptr, x, "", "", "", RISCV_ACQUIRE_BARRIER)
+	_arch_xchg(ptr, x, "", "", "", RISCV_ACQUIRE_BARRIER, RISCV_ACQUIRE_BARRIER)
 
 #define arch_xchg_release(ptr, x)					\
-	_arch_xchg(ptr, x, "", "", RISCV_RELEASE_BARRIER, "")
+	_arch_xchg(ptr, x, "", "", RISCV_RELEASE_BARRIER, "", "")
 
 #define arch_xchg(ptr, x)						\
-	_arch_xchg(ptr, x, ".rl", ".aqrl", "", "     fence rw, rw\n")
+	_arch_xchg(ptr, x, ".rl", ".aqrl", "", "     fence rw, rw\n", "")
 
 #define xchg32(ptr, x)							\
 ({									\