linux-kernel - Re: [PATCH v2 01/21] docs/memory-barriers.txt: Rewrite "KERNEL I/O BARRIER EFFECTS" section

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 10 Apr 2019 12:58:33 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Will Deacon <will.deacon@....com>
Cc:     linux-arch@...r.kernel.org, linux-kernel@...r.kernel.org,
        "Paul E. McKenney" <paulmck@...ux.ibm.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Arnd Bergmann <arnd@...db.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Andrea Parri <andrea.parri@...rulasolutions.com>,
        Palmer Dabbelt <palmer@...ive.com>,
        Daniel Lustig <dlustig@...dia.com>,
        David Howells <dhowells@...hat.com>,
        Alan Stern <stern@...land.harvard.edu>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        "Maciej W. Rozycki" <macro@...ux-mips.org>,
        Paul Burton <paul.burton@...s.com>,
        Yoshinori Sato <ysato@...rs.sourceforge.jp>,
        Rich Felker <dalias@...c.org>, Tony Luck <tony.luck@...el.com>,
        Mikulas Patocka <mpatocka@...hat.com>,
        Akira Yokosawa <akiyks@...il.com>,
        Luis Chamberlain <mcgrof@...nel.org>,
        Nicholas Piggin <npiggin@...il.com>
Subject: Re: [PATCH v2 01/21] docs/memory-barriers.txt: Rewrite "KERNEL I/O
 BARRIER EFFECTS" section


Mostly minor grammer fixes:

* Will Deacon <will.deacon@....com> wrote:

> + (*) readX(), writeX():
>  
> +     The readX() and writeX() MMIO accessors take a pointer to the peripheral
> +     being accessed as an __iomem * parameter. For pointers mapped with the
> +     default I/O attributes (e.g. those returned by ioremap()), then the
> +     ordering guarantees are as follows:

s/then the
 /the

> +     1. All readX() and writeX() accesses to the same peripheral are ordered
> +        with respect to each other. For example, this ensures that MMIO register
> +	writes by the CPU to a particular device will arrive in program order.

Vertical alignment whitespace damage: some indentations are done via 
spaces, one via tabs. Please standardize to tabs.

I'd also suggest:

s/For example, this ensures
 /For example this ensures


for the rest of the text too. The comma after the 'For example,' 
introductory phrase is grammatically correct but stylistically confusing, 
because in reality there's a *second* introductory phrase via "this 
ensures".

>  
> +     2. A writeX() by the CPU to the peripheral will first wait for the
> +        completion of all prior CPU writes to memory. For example, this ensures
> +        that writes by the CPU to an outbound DMA buffer allocated by
> +        dma_alloc_coherent() will be visible to a DMA engine when the CPU writes
> +        to its MMIO control register to trigger the transfer.
>  
> +     3. A readX() by the CPU from the peripheral will complete before any
> +	subsequent CPU reads from memory can begin. For example, this ensures
> +	that reads by the CPU from an incoming DMA buffer allocated by
> +	dma_alloc_coherent() will not see stale data after reading from the DMA
> +	engine's MMIO status register to establish that the DMA transfer has
> +	completed.
>  
> +     4. A readX() by the CPU from the peripheral will complete before any
> +	subsequent delay() loop can begin execution. For example, this ensures
> +	that two MMIO register writes by the CPU to a peripheral will arrive at
> +	least 1us apart if the first write is immediately read back with readX()
> +	and udelay(1) is called prior to the second writeX().

This might be more readable via some short code sequence instead?

>  
> +     __iomem pointers obtained with non-default attributes (e.g. those returned
> +     by ioremap_wc()) are unlikely to provide many of these guarantees.

This part is a bit confusing I think, because it's so cryptic. "Unlikely" 
as in probabilistic? ;-) So I think we should at least give some scope of 
the exceptions and expected trouble, or at least direct people to those 
APIs to see what the semantics are?

>  
> + (*) readX_relaxed(), writeX_relaxed():
>  
> +     These are similar to readX() and writeX(), but provide weaker memory
> +     ordering guarantees. Specifically, they do not guarantee ordering with
> +     respect to normal memory accesses or delay() loops (i.e bullets 2-4 above)
> +     but they are still guaranteed to be ordered with respect to other accesses
> +     to the same peripheral when operating on __iomem pointers mapped with the
> +     default I/O attributes.
>  
> + (*) readsX(), writesX():
>  
> +     The readsX() and writesX() MMIO accessors are designed for accessing
> +     register-based, memory-mapped FIFOs residing on peripherals that are not
> +     capable of performing DMA. Consequently, they provide only the ordering
> +     guarantees of readX_relaxed() and writeX_relaxed(), as documented above.

So is there any difference between 'X_relaxed' and 'sX' variants? What is 
the 's' about?

Thanks,

	Ingo