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

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Tue, 2024-01-30 at 16:05 +0100, Jan Beulich wrote:
> On 30.01.2024 15:57, Oleksii wrote:
> > 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.
> 
> The folded macro would have two barrier parameters - on for acquire,
> one
> for release.
Yes, in such case it will work.

Thanks.

~ Oleksii