From: Shawn Anastasio <shawn@anastas.io> To: Michael Ellerman <mpe@ellerman.id.au>, Arnd Bergmann <arnd@arndb.de>, Christoph Hellwig <hch@lst.de> Cc: Alexey Kardashevskiy <aik@ozlabs.ru>, Sam Bobroff <sbobroff@linux.ibm.com>, "open list:IOMMU DRIVERS" <iommu@lists.linux-foundation.org>, Oliver O'Halloran <oohall@gmail.com>, linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, Marek Szyprowski <m.szyprowski@samsung.com> Subject: Re: [PATCH] powerpc/dma: Fix invalid DMA mmap behavior Date: Mon, 22 Jul 2019 14:23:28 -0500 [thread overview] Message-ID: <ff0c5578-1cb4-af29-ca40-ef2c6b246d2f@anastas.io> (raw) In-Reply-To: <87ef2i6z99.fsf@concordia.ellerman.id.au> On 7/22/19 7:16 AM, Michael Ellerman wrote: > Arnd Bergmann <arnd@arndb.de> writes: >> On Thu, Jul 18, 2019 at 11:52 AM Christoph Hellwig <hch@lst.de> wrote: >>> On Thu, Jul 18, 2019 at 10:49:34AM +0200, Christoph Hellwig wrote: >>>> On Thu, Jul 18, 2019 at 01:45:16PM +1000, Oliver O'Halloran wrote: >>>>>> Other than m68k, mips, and arm64, everybody else that doesn't have >>>>>> ARCH_NO_COHERENT_DMA_MMAP set uses this default implementation, so >>>>>> I assume this behavior is acceptable on those architectures. >>>>> >>>>> It might be acceptable, but there's no reason to use pgport_noncached >>>>> if the platform supports cache-coherent DMA. >>>>> >>>>> Christoph (+cc) made the change so maybe he saw something we're missing. >>>> >>>> I always found the forcing of noncached access even for coherent >>>> devices a little odd, but this was inherited from the previous >>>> implementation, which surprised me a bit as the different attributes >>>> are usually problematic even on x86. Let me dig into the history a >>>> bit more, but I suspect the righ fix is to default to cached mappings >>>> for coherent devices. >>> >>> Ok, some history: >>> >>> The generic dma mmap implementation, which we are effectively still >>> using today was added by: >>> >>> commit 64ccc9c033c6089b2d426dad3c56477ab066c999 >>> Author: Marek Szyprowski <m.szyprowski@samsung.com> >>> Date: Thu Jun 14 13:03:04 2012 +0200 >>> >>> common: dma-mapping: add support for generic dma_mmap_* calls >>> >>> and unconditionally uses pgprot_noncached in dma_common_mmap, which is >>> then used as the fallback by dma_mmap_attrs if no ->mmap method is >>> present. At that point we already had the powerpc implementation >>> that only uses pgprot_noncached for non-coherent mappings, and >>> the arm one, which uses pgprot_writecombine if DMA_ATTR_WRITE_COMBINE >>> is set and otherwise pgprot_dmacoherent, which seems to be uncached. >>> Arm did support coherent platforms at that time, but they might have >>> been an afterthought and not handled properly. >> >> Cache-coherent devices are still very rare on 32-bit ARM. >> >> Among the callers of dma_mmap_coherent(), almost all are in platform >> specific device drivers that only ever run on noncoherent ARM SoCs, >> which explains why nobody would have noticed problems. >> >> There is also a difference in behavior between ARM and PowerPC >> when dealing with mismatched cacheability attributes: If the same >> page is mapped as both cached and uncached to, this may >> cause silent undefined behavior on ARM, while PowerPC should >> enter a checkstop as soon as it notices. > > On newer Power CPUs it's actually more like the ARM behaviour. > > I don't know for sure that it will *never* checkstop but there are at > least cases where it won't. There's some (not much) detail in the > Power8/9 user manuals. The issue was discovered due to sporadic checkstops on P9, so it seems like it will happen at least sometimes. > cheers
WARNING: multiple messages have this Message-ID (diff)
From: Shawn Anastasio via iommu <iommu@lists.linux-foundation.org> To: Michael Ellerman <mpe@ellerman.id.au>, Arnd Bergmann <arnd@arndb.de>, Christoph Hellwig <hch@lst.de> Cc: Sam Bobroff <sbobroff@linux.ibm.com>, "open list:IOMMU DRIVERS" <iommu@lists.linux-foundation.org>, Oliver O'Halloran <oohall@gmail.com>, linuxppc-dev <linuxppc-dev@lists.ozlabs.org> Subject: Re: [PATCH] powerpc/dma: Fix invalid DMA mmap behavior Date: Mon, 22 Jul 2019 14:23:28 -0500 [thread overview] Message-ID: <ff0c5578-1cb4-af29-ca40-ef2c6b246d2f@anastas.io> (raw) In-Reply-To: <87ef2i6z99.fsf@concordia.ellerman.id.au> On 7/22/19 7:16 AM, Michael Ellerman wrote: > Arnd Bergmann <arnd@arndb.de> writes: >> On Thu, Jul 18, 2019 at 11:52 AM Christoph Hellwig <hch@lst.de> wrote: >>> On Thu, Jul 18, 2019 at 10:49:34AM +0200, Christoph Hellwig wrote: >>>> On Thu, Jul 18, 2019 at 01:45:16PM +1000, Oliver O'Halloran wrote: >>>>>> Other than m68k, mips, and arm64, everybody else that doesn't have >>>>>> ARCH_NO_COHERENT_DMA_MMAP set uses this default implementation, so >>>>>> I assume this behavior is acceptable on those architectures. >>>>> >>>>> It might be acceptable, but there's no reason to use pgport_noncached >>>>> if the platform supports cache-coherent DMA. >>>>> >>>>> Christoph (+cc) made the change so maybe he saw something we're missing. >>>> >>>> I always found the forcing of noncached access even for coherent >>>> devices a little odd, but this was inherited from the previous >>>> implementation, which surprised me a bit as the different attributes >>>> are usually problematic even on x86. Let me dig into the history a >>>> bit more, but I suspect the righ fix is to default to cached mappings >>>> for coherent devices. >>> >>> Ok, some history: >>> >>> The generic dma mmap implementation, which we are effectively still >>> using today was added by: >>> >>> commit 64ccc9c033c6089b2d426dad3c56477ab066c999 >>> Author: Marek Szyprowski <m.szyprowski@samsung.com> >>> Date: Thu Jun 14 13:03:04 2012 +0200 >>> >>> common: dma-mapping: add support for generic dma_mmap_* calls >>> >>> and unconditionally uses pgprot_noncached in dma_common_mmap, which is >>> then used as the fallback by dma_mmap_attrs if no ->mmap method is >>> present. At that point we already had the powerpc implementation >>> that only uses pgprot_noncached for non-coherent mappings, and >>> the arm one, which uses pgprot_writecombine if DMA_ATTR_WRITE_COMBINE >>> is set and otherwise pgprot_dmacoherent, which seems to be uncached. >>> Arm did support coherent platforms at that time, but they might have >>> been an afterthought and not handled properly. >> >> Cache-coherent devices are still very rare on 32-bit ARM. >> >> Among the callers of dma_mmap_coherent(), almost all are in platform >> specific device drivers that only ever run on noncoherent ARM SoCs, >> which explains why nobody would have noticed problems. >> >> There is also a difference in behavior between ARM and PowerPC >> when dealing with mismatched cacheability attributes: If the same >> page is mapped as both cached and uncached to, this may >> cause silent undefined behavior on ARM, while PowerPC should >> enter a checkstop as soon as it notices. > > On newer Power CPUs it's actually more like the ARM behaviour. > > I don't know for sure that it will *never* checkstop but there are at > least cases where it won't. There's some (not much) detail in the > Power8/9 user manuals. The issue was discovered due to sporadic checkstops on P9, so it seems like it will happen at least sometimes. > cheers _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2019-07-22 19:25 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-17 23:54 [PATCH] powerpc/dma: Fix invalid DMA mmap behavior Shawn Anastasio 2019-07-18 2:59 ` Alexey Kardashevskiy 2019-07-18 3:14 ` Shawn Anastasio 2019-07-18 3:45 ` Oliver O'Halloran 2019-07-18 8:49 ` Christoph Hellwig 2019-07-18 9:52 ` Christoph Hellwig 2019-07-18 9:52 ` Christoph Hellwig 2019-07-18 19:46 ` Shawn Anastasio 2019-07-18 19:46 ` Shawn Anastasio via iommu 2019-07-19 7:06 ` Christoph Hellwig 2019-07-19 7:06 ` Christoph Hellwig 2019-07-19 7:36 ` Shawn Anastasio 2019-07-19 7:36 ` Shawn Anastasio via iommu 2019-07-19 11:18 ` Arnd Bergmann 2019-07-19 11:18 ` Arnd Bergmann 2019-07-22 12:16 ` Michael Ellerman 2019-07-22 12:16 ` Michael Ellerman 2019-07-22 19:23 ` Shawn Anastasio [this message] 2019-07-22 19:23 ` Shawn Anastasio via iommu 2019-07-22 23:09 ` Michael Ellerman 2019-07-22 23:09 ` Michael Ellerman 2019-07-22 2:48 ` Michael Ellerman
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=ff0c5578-1cb4-af29-ca40-ef2c6b246d2f@anastas.io \ --to=shawn@anastas.io \ --cc=aik@ozlabs.ru \ --cc=arnd@arndb.de \ --cc=hch@lst.de \ --cc=iommu@lists.linux-foundation.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=m.szyprowski@samsung.com \ --cc=mpe@ellerman.id.au \ --cc=oohall@gmail.com \ --cc=sbobroff@linux.ibm.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.