linux-kernel - Re: [PATCH] locking/memory-barriers.txt: Improve documentation for writel() usage

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]

Date:   Thu, 15 Sep 2022 17:16:24 +0200
From:   "Arnd Bergmann" <arnd@...db.de>
To:     "Parav Pandit" <parav@...dia.com>,
        "stern@...land.harvard.edu" <stern@...land.harvard.edu>,
        "parri.andrea@...il.com" <parri.andrea@...il.com>,
        "Will Deacon" <will@...nel.org>,
        "Peter Zijlstra" <peterz@...radead.org>,
        "boqun.feng@...il.com" <boqun.feng@...il.com>,
        "Nicholas Piggin" <npiggin@...il.com>,
        "dhowells@...hat.com" <dhowells@...hat.com>,
        "j.alglave@....ac.uk" <j.alglave@....ac.uk>,
        "luc.maranget@...ia.fr" <luc.maranget@...ia.fr>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        "akiyks@...il.com" <akiyks@...il.com>,
        "Dan Lustig" <dlustig@...dia.com>,
        "joel@...lfernandes.org" <joel@...lfernandes.org>,
        "Jonathan Corbet" <corbet@....net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linux-Arch <linux-arch@...r.kernel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>
Subject: Re: [PATCH] locking/memory-barriers.txt: Improve documentation for writel()
 usage

On Thu, Sep 15, 2022, at 4:18 PM, Parav Pandit wrote:
>> From: Arnd Bergmann <arnd@...db.de>
>> Sent: Thursday, September 15, 2022 8:38 AM
>> 
>> On Thu, Sep 15, 2022, at 7:01 AM, Parav Pandit wrote:
>> > The cited commit [1] describes that when using writel(), explcit wmb()
>> > is not needed. However, it should have said that dma_wmb() is not
>> > needed.
>> 
>> Are you sure? As I understand it, the dma_wmb() only serializes a set of
>> memory accesses, but does not serialized against an MMIO access, which
>> depending on the CPU architecture may require a different type of barrier.
>> 
>> E.g. on arm, writel() uses __iowmb(), which like wmb() is defined as "dsb(x);
>> arm_heavy_mb();", while dma_wmb() is a "dmb(oshst)".
>
> You are right, on arm heavy barrier dsb() is needed, while on arm64, 
> dmb(oshst) is sufficient.
>
> So more accurate documentation is to say that 
> 'when using writel() a prior IO barrier is not needed ...'
>
> How about that?

That's probably fine, not sure if it's worth changing.

> It started with my cleanup efforts to two drivers [1] and [2] that had 
> difficulty in using writel() on 32-bit system, and it ended up open 
> coding writel() as wmb() + mlx5_write64().
>
> I am cleaning up the repetitive pattern of, 
> wmb();
> mlx5_write64()
>
> Before I fix drivers, I thought to improve the documentation that I can 
> follow. :)

Right, that is definitely a good idea.

I see that there is more going on with that function, at least
the loop in post_send_nop() probably just wants to use
__iowrite64_copy(), but that also has no barrier in it, while
changing mlx5_write64() to use iowrite64be() or similar would
of course add excessive barriers inside of the loop.

       Arnd