Received-SPF: pass (google.com: domain of linux-kernel+bounces-131582-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, 04 Apr 2024 07:15:40 -0700 (PDT)
Subject:     Re: [PATCH RFC cmpxchg 8/8] riscv: Emulate one-byte and two-byte cmpxchg
In-Reply-To: <20240401213950.3910531-8-paulmck@kernel.org>
CC: linux-kernel@vger.kernel.org, kernel-team@meta.com, paulmck@kernel.org,
  andi.shyti@linux.intel.com, andrzej.hajda@intel.com, linux-riscv@lists.infradead.org
From: Palmer Dabbelt <palmer@dabbelt.com>
To: paulmck@kernel.org
Message-ID: <mhng-d6a8a972-5054-4c48-a903-5a53a31da9ad@palmer-ri-x1c9a>
Precedence: bulk
Mime-Version: 1.0 (MHng)
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit

On Mon, 01 Apr 2024 14:39:50 PDT (-0700), paulmck@kernel.org wrote:
> Use the new cmpxchg_emu_u8() and cmpxchg_emu_u16() to emulate one-byte
> and two-byte cmpxchg() on riscv.
>
> [ paulmck: Apply kernel test robot feedback. ]

I'm not entirely following the thread, but sounds like there's going to 
be generic kernel users of this now?  Before we'd said "no" to the 
byte/half atomic emulation routines beacuse they weren't used, but if 
it's a generic thing then I'm find adding them.

There's a patch set over here 
<https://lore.kernel.org/all/20240103163203.72768-2-leobras@redhat.com/> 
that implements these more directly using LR/SC.  I was sort of on the 
fence about just taking it even with no direct users right now, as the 
byte/half atomic extension is working its way through the spec process 
so we'll have them for real soon.  I stopped right there for the last 
merge window, though, as I figured it was too late to be messing with 
the atomics...

So

Acked-by: Palmer Dabbelt <palmer@rivosinc.com>

if you guys want to take some sort of tree-wide change to make the 
byte/half stuff be required everywhere.  We'll eventually end up with 
arch routines for the extension, so at that point we might as well also 
have the more direct LR/SC flavors.

If you want I can go review/merge that RISC-V patch set and then it'll 
have time to bake for a shared tag you can pick up for all this stuff?  
No rush on my end, just LMK.

> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> Cc: Andi Shyti <andi.shyti@linux.intel.com>
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: <linux-riscv@lists.infradead.org>
> ---
>  arch/riscv/Kconfig               |  1 +
>  arch/riscv/include/asm/cmpxchg.h | 25 +++++++++++++++++++++++++
>  2 files changed, 26 insertions(+)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index be09c8836d56b..4eaf40d0a52ec 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -44,6 +44,7 @@ config RISCV
>  	select ARCH_HAS_UBSAN
>  	select ARCH_HAS_VDSO_DATA
>  	select ARCH_KEEP_MEMBLOCK if ACPI
> +	select ARCH_NEED_CMPXCHG_1_2_EMU
>  	select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX
>  	select ARCH_OPTIONAL_KERNEL_RWX_DEFAULT
>  	select ARCH_STACKWALK
> diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
> index 2fee65cc84432..a5b377481785c 100644
> --- a/arch/riscv/include/asm/cmpxchg.h
> +++ b/arch/riscv/include/asm/cmpxchg.h
> @@ -9,6 +9,7 @@
>  #include <linux/bug.h>
>
>  #include <asm/fence.h>
> +#include <linux/cmpxchg-emu.h>
>
>  #define __xchg_relaxed(ptr, new, size)					\
>  ({									\
> @@ -170,6 +171,12 @@
>  	__typeof__(*(ptr)) __ret;					\
>  	register unsigned int __rc;					\
>  	switch (size) {							\
> +	case 1:								\
> +		__ret = cmpxchg_emu_u8((volatile u8 *)__ptr, __old, __new); \
> +		break;							\
> +	case 2:								\
> +		break;							\
> +		__ret = cmpxchg_emu_u16((volatile u16 *)__ptr, __old, __new); \
>  	case 4:								\
>  		__asm__ __volatile__ (					\
>  			"0:	lr.w %0, %2\n"				\
> @@ -214,6 +221,12 @@
>  	__typeof__(*(ptr)) __ret;					\
>  	register unsigned int __rc;					\
>  	switch (size) {							\
> +	case 1:								\
> +		__ret = cmpxchg_emu_u8((volatile u8 *)__ptr, __old, __new); \
> +		break;							\
> +	case 2:								\
> +		break;							\
> +		__ret = cmpxchg_emu_u16((volatile u16 *)__ptr, __old, __new); \
>  	case 4:								\
>  		__asm__ __volatile__ (					\
>  			"0:	lr.w %0, %2\n"				\
> @@ -260,6 +273,12 @@
>  	__typeof__(*(ptr)) __ret;					\
>  	register unsigned int __rc;					\
>  	switch (size) {							\
> +	case 1:								\
> +		__ret = cmpxchg_emu_u8((volatile u8 *)__ptr, __old, __new); \
> +		break;							\
> +	case 2:								\
> +		break;							\
> +		__ret = cmpxchg_emu_u16((volatile u16 *)__ptr, __old, __new); \
>  	case 4:								\
>  		__asm__ __volatile__ (					\
>  			RISCV_RELEASE_BARRIER				\
> @@ -306,6 +325,12 @@
>  	__typeof__(*(ptr)) __ret;					\
>  	register unsigned int __rc;					\
>  	switch (size) {							\
> +	case 1:								\
> +		__ret = cmpxchg_emu_u8((volatile u8 *)__ptr, __old, __new); \
> +		break;							\
> +	case 2:								\
> +		break;							\
> +		__ret = cmpxchg_emu_u16((volatile u16 *)__ptr, __old, __new); \
>  	case 4:								\
>  		__asm__ __volatile__ (					\
>  			"0:	lr.w %0, %2\n"				\