linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: "Arnd Bergmann" <arnd@arndb.de>
To: "Christophe Leroy" <christophe.leroy@csgroup.eu>,
	"Arnd Bergmann" <arnd@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "Vineet Gupta" <vgupta@kernel.org>,
	"Russell King" <linux@armlinux.org.uk>,
	"Neil Armstrong" <neil.armstrong@linaro.org>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Will Deacon" <will@kernel.org>, guoren <guoren@kernel.org>,
	"Brian Cain" <bcain@quicinc.com>,
	"Geert Uytterhoeven" <geert@linux-m68k.org>,
	"Michal Simek" <monstr@monstr.eu>,
	"Thomas Bogendoerfer" <tsbogend@alpha.franken.de>,
	"Dinh Nguyen" <dinguyen@kernel.org>,
	"Stafford Horne" <shorne@gmail.com>,
	"Helge Deller" <deller@gmx.de>,
	"Michael Ellerman" <mpe@ellerman.id.au>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Rich Felker" <dalias@libc.org>,
	"John Paul Adrian Glaubitz" <glaubitz@physik.fu-berlin.de>,
	"David S . Miller" <davem@davemloft.net>,
	"Max Filippov" <jcmvbkbc@gmail.com>,
	"Christoph Hellwig" <hch@lst.de>,
	"Robin Murphy" <robin.murphy@arm.com>,
	"Lad, Prabhakar" <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	"Conor.Dooley" <conor.dooley@microchip.com>,
	"linux-snps-arc@lists.infradead.org"
	<linux-snps-arc@lists.infradead.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-oxnas@groups.io" <linux-oxnas@groups.io>,
	"linux-csky@vger.kernel.org" <linux-csky@vger.kernel.org>,
	"linux-hexagon@vger.kernel.org" <linux-hexagon@vger.kernel.org>,
	"linux-m68k@lists.linux-m68k.org"
	<linux-m68k@lists.linux-m68k.org>,
	"linux-mips@vger.kernel.org" <linux-mips@vger.kernel.org>,
	"linux-openrisc@vger.kernel.org" <linux-openrisc@vger.kernel.org>,
	"linux-parisc@vger.kernel.org" <linux-parisc@vger.kernel.org>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	"linux-riscv@lists.infradead.org"
	<linux-riscv@lists.infradead.org>,
	"linux-sh@vger.kernel.org" <linux-sh@vger.kernel.org>,
	"sparclinux@vger.kernel.org" <sparclinux@vger.kernel.org>,
	"linux-xtensa@linux-xtensa.org" <linux-xtensa@linux-xtensa.org>
Subject: Re: [PATCH 06/21] powerpc: dma-mapping: minimize for_cpu flushing
Date: Mon, 27 Mar 2023 15:02:04 +0200	[thread overview]
Message-ID: <a0b2c6a1-9f9a-490a-af1b-52d856102bc7@app.fastmail.com> (raw)
In-Reply-To: <52eef604-085e-acf9-f3ee-41ba9d8441d2@csgroup.eu>

On Mon, Mar 27, 2023, at 14:56, Christophe Leroy wrote:
> Le 27/03/2023 à 14:13, Arnd Bergmann a écrit :
>> From: Arnd Bergmann <arnd@arndb.de>
>> 
>> The powerpc dma_sync_*_for_cpu() variants do more flushes than on other
>> architectures. Reduce it to what everyone else does:
>> 
>>   - No flush is needed after data has been sent to a device
>> 
>>   - When data has been received from a device, the cache only needs to
>>     be invalidated to clear out cache lines that were speculatively
>>     prefetched.
>> 
>> In particular, the second flushing of partial cache lines of bidirectional
>> buffers is actively harmful -- if a single cache line is written by both
>> the CPU and the device, flushing it again does not maintain coherency
>> but instead overwrite the data that was just received from the device.
>
> Hum ..... Who is right ?
>
> That behaviour was introduced by commit 03d70617b8a7 ("powerpc: Prevent 
> memory corruption due to cache invalidation of unaligned DMA buffer")
>
> I think your commit log should explain why that commit was wrong, and 
> maybe say that your patch is a revert of that commit ?

Ok, I'll try to explain this better. To clarify here: the __dma_sync()
function in commit 03d70617b8a7 is used both before and after a DMA,
but my patch 05/21 splits this in two, and patch 06/21 only changes
the part that gets called after the DMA-from-device but leaves the
part before DMA-from-device unchanged, which Andrew's patch
addressed.

As I mentioned in the cover letter, it is still unclear whether
we want to consider this the expected behavior as the documentation
seems unclear, but my series does not attempt to answer that
question.

     Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2023-03-27 13:03 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-27 12:12 [PATCH 00/21] dma-mapping: unify support for cache flushes Arnd Bergmann
2023-03-27 12:12 ` [PATCH 01/21] openrisc: dma-mapping: flush bidirectional mappings Arnd Bergmann
2023-03-27 12:12 ` [PATCH 02/21] xtensa: dma-mapping: use normal cache invalidation rules Arnd Bergmann
2023-03-27 15:42   ` Max Filippov
2023-03-27 12:12 ` [PATCH 03/21] sparc32: flush caches in dma_sync_*for_device Arnd Bergmann
2023-03-27 12:13 ` [PATCH 04/21] microblaze: dma-mapping: skip extra DMA flushes Arnd Bergmann
2023-03-27 12:13 ` [PATCH 05/21] powerpc: dma-mapping: split out cache operation logic Arnd Bergmann
2023-03-27 12:13 ` [PATCH 06/21] powerpc: dma-mapping: minimize for_cpu flushing Arnd Bergmann
2023-03-27 12:56   ` Christophe Leroy
2023-03-27 13:02     ` Arnd Bergmann [this message]
2023-03-27 12:13 ` [PATCH 07/21] powerpc: dma-mapping: always clean cache in _for_device() op Arnd Bergmann
2023-03-27 12:13 ` [PATCH 08/21] riscv: dma-mapping: only invalidate after DMA, not flush Arnd Bergmann
2023-03-29 20:48   ` Conor Dooley
2023-03-30  7:10     ` Arnd Bergmann
2023-03-29 21:51   ` Jessica Clarke
2023-03-30 12:59   ` Lad, Prabhakar
2023-04-19 14:22   ` Palmer Dabbelt
2023-03-27 12:13 ` [PATCH 09/21] riscv: dma-mapping: skip invalidation before bidirectional DMA Arnd Bergmann
2023-03-29 20:16   ` Conor Dooley
2023-03-30 13:26   ` Lad, Prabhakar
2023-04-19 14:22   ` Palmer Dabbelt
2023-05-05  5:47   ` Guo Ren
2023-05-05 13:18     ` Arnd Bergmann
2023-05-06  7:25       ` Guo Ren
2023-05-06  7:53         ` Arnd Bergmann
2023-03-27 12:13 ` [PATCH 10/21] csky: dma-mapping: skip invalidating before DMA from device Arnd Bergmann
2023-03-27 13:37   ` Guo Ren
2023-03-27 12:13 ` [PATCH 11/21] mips: dma-mapping: skip invalidating before bidirectional DMA Arnd Bergmann
2023-03-27 12:13 ` [PATCH 12/21] mips: dma-mapping: split out cache operation logic Arnd Bergmann
2023-03-27 12:13 ` [PATCH 13/21] arc: dma-mapping: skip invalidating before bidirectional DMA Arnd Bergmann
2023-04-02  6:52   ` Vineet Gupta
2023-04-04  8:27     ` Shahab Vahedi
2023-04-06  9:01     ` Shahab Vahedi
2023-03-27 12:13 ` [PATCH 14/21] parisc: dma-mapping: use regular flush/invalidate ops Arnd Bergmann
2023-03-27 12:13 ` [PATCH 15/21] ARM: dma-mapping: always invalidate WT caches before DMA Arnd Bergmann
2023-03-31  9:01   ` Linus Walleij
2023-03-31  9:07   ` Russell King (Oracle)
2023-03-31  9:35     ` Russell King (Oracle)
2023-03-31 10:38       ` Arnd Bergmann
2023-03-31 11:01         ` David Laight
2023-03-31 11:08         ` Russell King (Oracle)
2023-03-31 12:32           ` Arnd Bergmann
2023-03-27 12:13 ` [PATCH 16/21] ARM: dma-mapping: bring back dmac_{clean,inv}_range Arnd Bergmann
2023-03-27 13:10   ` Russell King (Oracle)
2023-03-27 12:13 ` [PATCH 17/21] ARM: dma-mapping: use arch_sync_dma_for_{device,cpu}() internally Arnd Bergmann
2023-03-31  9:10   ` Linus Walleij
2023-03-31 12:48     ` Arnd Bergmann
2023-03-27 12:13 ` [PATCH 18/21] ARM: drop SMP support for ARM11MPCore Arnd Bergmann
2023-03-30  7:48   ` Neil Armstrong
2023-03-30 10:03     ` Arnd Bergmann
2023-03-30 16:40       ` Neil Armstrong
2023-03-30  8:12   ` Linus Walleij
2023-03-30 11:28   ` Joel Stanley
2023-03-31 12:54     ` Arnd Bergmann
2023-04-05  1:49       ` Joel Stanley
2023-03-30 11:51   ` Ard Biesheuvel
2023-03-31 17:09   ` Catalin Marinas
2023-03-27 12:13 ` [PATCH 19/21] ARM: dma-mapping: use generic form of arch_sync_dma_* helpers Arnd Bergmann
2023-03-27 12:13 ` [PATCH 20/21] ARM: dma-mapping: split out arch_dma_mark_clean() helper Arnd Bergmann
2023-03-27 12:48   ` Robin Murphy
2023-03-31 14:00     ` Arnd Bergmann
2023-03-31 15:12       ` Robin Murphy
2023-03-31 17:20         ` Arnd Bergmann
2023-03-27 15:01   ` Russell King (Oracle)
2023-03-31 14:06     ` Arnd Bergmann
2023-03-31 15:54       ` Russell King (Oracle)
2023-07-03  7:54   ` Geert Uytterhoeven
2023-07-06 14:11     ` Christoph Hellwig
2023-03-27 12:13 ` [PATCH 21/21] dma-mapping: replace custom code with generic implementation Arnd Bergmann
2023-03-27 22:25   ` Christoph Hellwig
2023-03-31 13:04     ` Arnd Bergmann
2023-03-30 14:06   ` Lad, Prabhakar
2023-04-13 12:13   ` Biju Das
2023-04-13 12:51     ` Arnd Bergmann
2023-06-27 16:52       ` Geert Uytterhoeven
2023-03-31 16:53 ` [PATCH 00/21] dma-mapping: unify support for cache flushes Catalin Marinas
2023-03-31 20:27   ` Arnd Bergmann
2023-05-25  7:46 ` Lad, Prabhakar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a0b2c6a1-9f9a-490a-af1b-52d856102bc7@app.fastmail.com \
    --to=arnd@arndb.de \
    --cc=arnd@kernel.org \
    --cc=bcain@quicinc.com \
    --cc=catalin.marinas@arm.com \
    --cc=christophe.leroy@csgroup.eu \
    --cc=conor.dooley@microchip.com \
    --cc=dalias@libc.org \
    --cc=davem@davemloft.net \
    --cc=deller@gmx.de \
    --cc=dinguyen@kernel.org \
    --cc=geert@linux-m68k.org \
    --cc=glaubitz@physik.fu-berlin.de \
    --cc=guoren@kernel.org \
    --cc=hch@lst.de \
    --cc=jcmvbkbc@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-csky@vger.kernel.org \
    --cc=linux-hexagon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-m68k@lists.linux-m68k.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-openrisc@vger.kernel.org \
    --cc=linux-oxnas@groups.io \
    --cc=linux-parisc@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=linux-snps-arc@lists.infradead.org \
    --cc=linux-xtensa@linux-xtensa.org \
    --cc=linux@armlinux.org.uk \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=monstr@monstr.eu \
    --cc=mpe@ellerman.id.au \
    --cc=neil.armstrong@linaro.org \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
    --cc=robin.murphy@arm.com \
    --cc=shorne@gmail.com \
    --cc=sparclinux@vger.kernel.org \
    --cc=tsbogend@alpha.franken.de \
    --cc=vgupta@kernel.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).