Re: [PATCH v3 13/34] xen/riscv: introduce cmpxchg.h

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Mon, 2024-01-22 at 17:27 +0100, Jan Beulich wrote:
> > +#define __xchg_acquire(ptr, new, size) \
> > +({ \
> > +    __typeof__(ptr) ptr__ = (ptr); \
> > +    __typeof__(new) new__ = (new); \
> > +    __typeof__(*(ptr)) ret__; \
> > +    switch (size) \
> > +   { \
> > +    case 4: \
> > +        asm volatile( \
> > +            "      amoswap.w %0, %2, %1\n" \
> > +            RISCV_ACQUIRE_BARRIER \
> > +            : "=r" (ret__), "+A" (*ptr__) \
> > +            : "r" (new__) \
> > +            : "memory" ); \
> > +        break; \
> > +    case 8: \
> > +        asm volatile( \
> > +            "      amoswap.d %0, %2, %1\n" \
> > +            RISCV_ACQUIRE_BARRIER \
> > +            : "=r" (ret__), "+A" (*ptr__) \
> > +            : "r" (new__) \
> > +            : "memory" ); \
> > +        break; \
> > +    default: \
> > +        ASSERT_UNREACHABLE(); \
> > +    } \
> > +    ret__; \
> > +})
> 
> If I'm not mistaken this differs from __xchg_relaxed() only in the
> use
> of RISCV_ACQUIRE_BARRIER, and ...
> 
> > +#define xchg_acquire(ptr, x) \
> > +({ \
> > +    __typeof__(*(ptr)) x_ = (x); \
> > +    (__typeof__(*(ptr))) __xchg_acquire((ptr), x_,
> > sizeof(*(ptr))); \
> > +})
> > +
> > +#define __xchg_release(ptr, new, size) \
> > +({ \
> > +    __typeof__(ptr) ptr__ = (ptr); \
> > +    __typeof__(new) new__ = (new); \
> > +    __typeof__(*(ptr)) ret__; \
> > +    switch (size) \
> > +   { \
> > +    case 4: \
> > +        asm volatile ( \
> > +            RISCV_RELEASE_BARRIER \
> > +            "      amoswap.w %0, %2, %1\n" \
> > +            : "=r" (ret__), "+A" (*ptr__) \
> > +            : "r" (new__) \
> > +            : "memory"); \
> > +        break; \
> > +    case 8: \
> > +        asm volatile ( \
> > +            RISCV_RELEASE_BARRIER \
> > +            "      amoswap.d %0, %2, %1\n" \
> > +            : "=r" (ret__), "+A" (*ptr__) \
> > +            : "r" (new__) \
> > +            : "memory"); \
> > +        break; \
> > +    default: \
> > +        ASSERT_UNREACHABLE(); \
> > +    } \
> > +    ret__; \
> > +})
> 
> this only in the use of RISCV_RELEASE_BARRIER. If so they likely want
> folding, to limit redundancy and make eventual updating easier. (Same
> for the cmpxchg helper further down, as it seems.)
Also the difference is in where to place barrier before or after atomic
instruction. I am not sure that we can easily folded this macros.

~ Oleksii