lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHVXubi+XW=v=MKDov5j0v2QG-cAMjWxdqMRhgfLmo1JhCkryQ@mail.gmail.com>
Date: Thu, 30 May 2024 14:05:32 +0200
From: Alexandre Ghiti <alexghiti@...osinc.com>
To: Andrea Parri <parri.andrea@...il.com>
Cc: Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>, 
	Albert Ou <aou@...s.berkeley.edu>, Leonardo Bras <leobras@...hat.com>, 
	linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH -fixes] riscv: Fix fully ordered LR/SC xchg[8|16]() implementations

Andrea,

On Thu, May 30, 2024 at 1:54 PM Andrea Parri <parri.andrea@...il.com> wrote:
>
> > -#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.

And you already told me that privately...Sorry, my mind has been
elsewhere lately...I'll fix that right now.

Sorry again and thanks,

Alex

>
>   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)                                                 \
>  ({                                                                     \
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ