Re: [PATCH] xen: Introduce cmpxchg64() and guest_cmpxchg64()

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Sat, Aug 15, 2020 at 06:21:43PM +0100, Julien Grall wrote:
> From: Julien Grall <jgrall@xxxxxxxxxx>
> 
> The IOREQ code is using cmpxchg() with 64-bit value. At the moment, this
> is x86 code, but there is plan to make it common.
> 
> To cater 32-bit arch, introduce two new helpers to deal with 64-bit
> cmpxchg.
> 
> The Arm 32-bit implementation of cmpxchg64() is based on the __cmpxchg64
> in Linux v5.8 (arch/arm/include/asm/cmpxchg.h).
> 
> Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
> Cc: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> ---
> diff --git a/xen/include/asm-x86/guest_atomics.h 
> b/xen/include/asm-x86/guest_atomics.h
> index 029417c8ffc1..f4de9d3631ff 100644
> --- a/xen/include/asm-x86/guest_atomics.h
> +++ b/xen/include/asm-x86/guest_atomics.h
> @@ -20,6 +20,8 @@
>      ((void)(d), test_and_change_bit(nr, p))
>  
>  #define guest_cmpxchg(d, ptr, o, n) ((void)(d), cmpxchg(ptr, o, n))
> +#define guest_cmpxchg64(d, ptr, o, n) ((void)(d), cmpxchg64(ptr, o, n))
> +
>  
>  #endif /* _X86_GUEST_ATOMICS_H */
>  /*
> diff --git a/xen/include/asm-x86/x86_64/system.h 
> b/xen/include/asm-x86/x86_64/system.h
> index f471859c19cc..c1b16105e9f2 100644
> --- a/xen/include/asm-x86/x86_64/system.h
> +++ b/xen/include/asm-x86/x86_64/system.h
> @@ -5,6 +5,8 @@
>      ((__typeof__(*(ptr)))__cmpxchg((ptr),(unsigned long)(o),            \
>                                     (unsigned long)(n),sizeof(*(ptr))))
>  
> +#define cmpxchg64(ptr, o, n) cmpxchg(ptr, o, n)

Why do you need to introduce an explicitly sized version of cmpxchg
for 64bit values?

There's no cmpxchg{8,16,32}, so I would expect cmpxchg64 to just be
handled by cmpxchg detecting the size of the parameter passed to the
function. I think it's worth adding to the commit message why such
differentiated helper is needed.

Thanks, Roger.