linux-kernel - Re: [PATCH] arch/powerpc/include/asm/barrier.h: redefine rmb and wmb to lwsync

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] [day] [month] [year] [list]

Date:   Wed, 22 Feb 2023 14:17:11 +0530
From:   Kautuk Consul <kconsul@...ux.vnet.ibm.com>
To:     Christophe Leroy <christophe.leroy@...roup.eu>
Cc:     Michael Ellerman <mpe@...erman.id.au>,
        Nicholas Piggin <npiggin@...il.com>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] arch/powerpc/include/asm/barrier.h: redefine rmb and wmb
 to lwsync

 
> No, I don't mean to use the existing #ifdef/elif/else.
> 
> Define an #ifdef /#else dedicated to xmb macros.
> 
> Something like that:
> 
> @@ -35,9 +35,15 @@
>    * However, on CPUs that don't support lwsync, lwsync actually maps to a
>    * heavy-weight sync, so smp_wmb() can be a lighter-weight eieio.
>    */
> +#if defined(CONFIG_PPC64) || defined(CONFIG_PPC_E500MC)
> +#define __mb()   asm volatile ("lwsync" : : : "memory")
> +#define __rmb()  asm volatile ("lwsync" : : : "memory")
> +#define __wmb()  asm volatile ("lwsync" : : : "memory")
> +#else
>   #define __mb()   __asm__ __volatile__ ("sync" : : : "memory")
>   #define __rmb()  __asm__ __volatile__ ("sync" : : : "memory")
>   #define __wmb()  __asm__ __volatile__ ("sync" : : : "memory")
> +#endif
Ok. Got it. Will do.

> >> Shouldn't you also consider the same for mb() ?
> > My change wasn't meant to address newer usages of as volatile
> > #defines. I just wanted to redefine the rmb and wmb #defines
> > to lwsync.
> 
> That's my point, why not also redefine mb to lwsync ?
That would be incorrect. lwsync will only work for one: load or store.
mb() is meant for barriering both loads as well as stores so the sync
instruction is correct for that one.
> 
> >>
> >> Note that your series will conflict with b6e259297a6b ("powerpc/kcsan:
> >> Memory barriers semantics") in powerpc/next tree.
> > I thought of defining the __rmb and __wmb macros but I decided against
> > it because I didn't understand kcsan completely.
> > I used the standard Linus' tree, not powerpc/next.
> > Can you tell me which branch or git repo I should make this patch on ?
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git
> 
> 'merge' branch is a merge of branches 'master', 'fixes' and 'next'.
> 
> That's the branch to use, allthough it is not always in sync with fixes 
> and next, in that case all you have to do is to locally merge 'next' and 
> 'fixes' branch until it's done remotely.
> 
> But just using 'next' branch also works most of the time.
> 
> Note that 'next' branch should already be part of linux-next so you may 
> also use linux-next if you prefer.
> 
Will send another patch on this.
Thanks. Will use linux-next branch on this git repo.