From: Vineet Gupta <Vineet.Gupta1-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> To: Alexey Brodkin <Alexey.Brodkin-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>, "hch-jcswGhMUV9g@public.gmane.org" <hch-jcswGhMUV9g@public.gmane.org> Cc: "linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "linux-xtensa-PjhNF2WwrV/0Sa2dR60CXw@public.gmane.org" <linux-xtensa-PjhNF2WwrV/0Sa2dR60CXw@public.gmane.org>, "monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org" <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>, "deanbo422-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" <deanbo422-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, "linux-c6x-dev-jPsnJVOj+W6hPH1hqNUYSQ@public.gmane.org" <linux-c6x-dev-jPsnJVOj+W6hPH1hqNUYSQ@public.gmane.org>, "linux-parisc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-parisc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "linux-sh-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-sh-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "linux-m68k-cunTk1MwBs8S/qaLPR03pWD2FQJk+8+b@public.gmane.org" <linux-m68k-cunTk1MwBs8S/qaLPR03pWD2FQJk+8+b@public.gmane.org>, "linux-hexagon-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-hexagon-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org" <linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org>, "iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org" <iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>, "openrisc-cunTk1MwBs9a3B2Vnqf2dGD2FQJk+8+b@public.gmane.org" <openrisc-cunTk1MwBs9a3B2Vnqf2dGD2FQJk+8+b@public.gmane.org>, "green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, "linux-alpha-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-alpha-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, sparclinux@vg Subject: dma_sync_*_for_cpu and direction=TO_DEVICE (was Re: [PATCH 02/20] dma-mapping: provide a generic dma-noncoherent implementation) Date: Fri, 18 May 2018 10:20:02 -0700 [thread overview] Message-ID: <5ac5b1e3-9b96-9c7c-4dfe-f65be45ec179@synopsys.com> (raw) In-Reply-To: <bad125dff49f6e49c895e818c9d1abb346a46e8e.camel-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> On 05/18/2018 06:11 AM, Alexey Brodkin wrote: > void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr, > size_t size, enum dma_data_direction dir) > { > + if (dir != DMA_TO_DEVICE){ > + dump_stack(); > + printk(" *** %s@%d: DMA direction is %s instead of %s\n", > + __func__, __LINE__, dir_to_str(dir), dir_to_str(DMA_TO_DEVICE)); > + } > + > return _dma_cache_sync(dev, paddr, size, dir); > } > > void arch_sync_dma_for_cpu(struct device *dev, phys_addr_t paddr, > size_t size, enum dma_data_direction dir) > { > + if (dir != DMA_FROM_DEVICE) { > + dump_stack(); > + printk(" *** %s@%d: DMA direction is %s instead of %s\n", > + __func__, __LINE__, dir_to_str(dir), dir_to_str(DMA_FROM_DEVICE)); > + } > + > return _dma_cache_sync(dev, paddr, size, dir); > } ... > In case of MMC/DW_MCI (AKA DesignWare MobileStorage controller) that's an execution flow: > 1) __dw_mci_start_request() > 2) dw_mci_pre_dma_transfer() > 3) dma_map_sg(..., mmc_get_dma_dir(data)) > > Note mmc_get_dma_dir() is just "data->flags & MMC_DATA_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE". > I.e. if we're preparing for sending data dma_noncoherent_map_sg() will have DMA_TO_DEVICE which > is quite OK for passing to dma_noncoherent_sync_sg_for_device() but in case of reading we'll have > DMA_FROM_DEVICE which we'll pass to dma_noncoherent_sync_sg_for_device() in dma_noncoherent_map_sg(). > > I'd say this is not entirely correct because IMHO arch_sync_dma_for_cpu() is supposed to only be used > in case of DMA_FROM_DEVICE and arch_sync_dma_for_device() only in case of DMA_TO_DEVICE. So roughly 10 years ago, some kernel rookie name Vineet Gupta, asked the exact same question :-) http://kernelnewbies.kernelnewbies.narkive.com/aGW1QcDv/query-about-dma-sync-for-cpu-and-direction-to-device I never understood the need for this direction. And if memory serves me right, at that time I was seeing twice the amount of cache flushing ! > But the real fix of my problem is: > ---------------------------------------->8------------------------------------ > --- a/lib/dma-noncoherent.c > +++ b/lib/dma-noncoherent.c > @@ -35,7 +35,7 @@ static dma_addr_t dma_noncoherent_map_page(struct device *dev, struct page *page > > addr = dma_direct_map_page(dev, page, offset, size, dir, attrs); > if (!dma_mapping_error(dev, addr) && !(attrs & DMA_ATTR_SKIP_CPU_SYNC)) > - arch_sync_dma_for_device(dev, page_to_phys(page), size, dir); > + arch_sync_dma_for_device(dev, page_to_phys(page) + offset, size, dir); > return addr; > } > ---------------------------------------->8------------------------------------ > > You seem to lost an offset in the page so if we happen to have a buffer not aligned to > a page boundary then we were obviously corrupting data outside our data :) Neat !
WARNING: multiple messages have this Message-ID (diff)
From: Vineet Gupta <Vineet.Gupta1@synopsys.com> To: Alexey Brodkin <Alexey.Brodkin@synopsys.com>, "hch@lst.de" <hch@lst.de> Cc: "linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>, "linux-xtensa@linux-xtensa.org" <linux-xtensa@linux-xtensa.org>, "monstr@monstr.eu" <monstr@monstr.eu>, "linux-snps-arc@lists.infradead.org" <linux-snps-arc@lists.infradead.org>, "linux-c6x-dev@linux-c6x.org" <linux-c6x-dev@linux-c6x.org>, "linux-parisc@vger.kernel.org" <linux-parisc@vger.kernel.org>, "linux-sh@vger.kernel.org" <linux-sh@vger.kernel.org>, "linux-hexagon@vger.kernel.org" <linux-hexagon@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>, "linux-m68k@lists.linux-m68k.org" <linux-m68k@lists.linux-m68k.org>, "openrisc@lists.librecores.org" <openrisc@lists.librecores.org>, "green.hu@gmail.com" <green.hu@gmail.com>, "linux-alpha@vger.kernel.org" <linux-alpha@vger.kernel.org>, "sparclinux@vger.kernel.org" <sparclinux@vger.kernel.org>, "nios2-dev@lists.rocketboards.org" <nios2-dev@lists.rocketboards.org>, "deanbo422@gmail.com" <deanbo422@gmail.com>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "linux-mm@kvack.org" <linux-mm@kvack.org>, Andrew Morton <akpm@linux-foundation.org> Subject: dma_sync_*_for_cpu and direction=TO_DEVICE (was Re: [PATCH 02/20] dma-mapping: provide a generic dma-noncoherent implementation) Date: Fri, 18 May 2018 10:20:02 -0700 [thread overview] Message-ID: <5ac5b1e3-9b96-9c7c-4dfe-f65be45ec179@synopsys.com> (raw) Message-ID: <20180518172002.H6OzSiySjVzRYn7i_v-juNZywRsPLzfqbgso5Q5-2l0@z> (raw) In-Reply-To: <bad125dff49f6e49c895e818c9d1abb346a46e8e.camel@synopsys.com> On 05/18/2018 06:11 AM, Alexey Brodkin wrote: > void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr, > size_t size, enum dma_data_direction dir) > { > + if (dir != DMA_TO_DEVICE){ > + dump_stack(); > + printk(" *** %s@%d: DMA direction is %s instead of %s\n", > + __func__, __LINE__, dir_to_str(dir), dir_to_str(DMA_TO_DEVICE)); > + } > + > return _dma_cache_sync(dev, paddr, size, dir); > } > > void arch_sync_dma_for_cpu(struct device *dev, phys_addr_t paddr, > size_t size, enum dma_data_direction dir) > { > + if (dir != DMA_FROM_DEVICE) { > + dump_stack(); > + printk(" *** %s@%d: DMA direction is %s instead of %s\n", > + __func__, __LINE__, dir_to_str(dir), dir_to_str(DMA_FROM_DEVICE)); > + } > + > return _dma_cache_sync(dev, paddr, size, dir); > } ... > In case of MMC/DW_MCI (AKA DesignWare MobileStorage controller) that's an execution flow: > 1) __dw_mci_start_request() > 2) dw_mci_pre_dma_transfer() > 3) dma_map_sg(..., mmc_get_dma_dir(data)) > > Note mmc_get_dma_dir() is just "data->flags & MMC_DATA_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE". > I.e. if we're preparing for sending data dma_noncoherent_map_sg() will have DMA_TO_DEVICE which > is quite OK for passing to dma_noncoherent_sync_sg_for_device() but in case of reading we'll have > DMA_FROM_DEVICE which we'll pass to dma_noncoherent_sync_sg_for_device() in dma_noncoherent_map_sg(). > > I'd say this is not entirely correct because IMHO arch_sync_dma_for_cpu() is supposed to only be used > in case of DMA_FROM_DEVICE and arch_sync_dma_for_device() only in case of DMA_TO_DEVICE. So roughly 10 years ago, some kernel rookie name Vineet Gupta, asked the exact same question :-) http://kernelnewbies.kernelnewbies.narkive.com/aGW1QcDv/query-about-dma-sync-for-cpu-and-direction-to-device I never understood the need for this direction. And if memory serves me right, at that time I was seeing twice the amount of cache flushing ! > But the real fix of my problem is: > ---------------------------------------->8------------------------------------ > --- a/lib/dma-noncoherent.c > +++ b/lib/dma-noncoherent.c > @@ -35,7 +35,7 @@ static dma_addr_t dma_noncoherent_map_page(struct device *dev, struct page *page > > addr = dma_direct_map_page(dev, page, offset, size, dir, attrs); > if (!dma_mapping_error(dev, addr) && !(attrs & DMA_ATTR_SKIP_CPU_SYNC)) > - arch_sync_dma_for_device(dev, page_to_phys(page), size, dir); > + arch_sync_dma_for_device(dev, page_to_phys(page) + offset, size, dir); > return addr; > } > ---------------------------------------->8------------------------------------ > > You seem to lost an offset in the page so if we happen to have a buffer not aligned to > a page boundary then we were obviously corrupting data outside our data :) Neat !
next prev parent reply other threads:[~2018-05-18 17:20 UTC|newest] Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-05-11 7:59 common non-cache coherent direct dma mapping ops Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig 2018-05-11 7:59 ` [PATCH 01/20] dma-mapping: simplify Kconfig dependencies Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig [not found] ` <20180511075945.16548-1-hch-jcswGhMUV9g@public.gmane.org> 2018-05-11 7:59 ` [PATCH 02/20] dma-mapping: provide a generic dma-noncoherent implementation Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig [not found] ` <20180511075945.16548-3-hch-jcswGhMUV9g@public.gmane.org> 2018-05-18 13:03 ` Alexey Brodkin 2018-05-18 13:03 ` Alexey Brodkin [not found] ` <bad125dff49f6e49c895e818c9d1abb346a46e8e.camel-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> 2018-05-18 13:27 ` hch-jcswGhMUV9g 2018-05-18 13:27 ` hch [not found] ` <20180518132731.GA31125-jcswGhMUV9g@public.gmane.org> 2018-05-18 14:13 ` Alexey Brodkin 2018-05-18 14:13 ` Alexey Brodkin 2018-05-18 17:28 ` Vineet Gupta 2018-05-18 17:28 ` Vineet Gupta 2018-05-18 17:20 ` Vineet Gupta [this message] 2018-05-18 17:20 ` dma_sync_*_for_cpu and direction=TO_DEVICE (was Re: [PATCH 02/20] dma-mapping: provide a generic dma-noncoherent implementation) Vineet Gupta 2018-05-18 17:50 ` Russell King - ARM Linux 2018-05-18 17:50 ` Russell King - ARM Linux [not found] ` <20180518175004.GF17671-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org> 2018-05-18 19:57 ` Alexey Brodkin 2018-05-18 19:57 ` Alexey Brodkin [not found] ` <182840dedb4890a88c672b1c5d556920bf89a8fb.camel-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> 2018-05-18 21:33 ` Russell King - ARM Linux 2018-05-18 21:33 ` Russell King - ARM Linux 2018-05-18 20:35 ` Vineet Gupta 2018-05-18 20:35 ` Vineet Gupta 2018-05-18 21:55 ` Russell King - ARM Linux 2018-05-18 21:55 ` Russell King - ARM Linux 2018-05-18 20:05 ` [PATCH 02/20] dma-mapping: provide a generic dma-noncoherent implementation Helge Deller 2018-05-18 20:05 ` Helge Deller [not found] ` <0c5d27e9-2799-eb38-8b09-47a04c48b5c7-Mmb7MZpHnFY@public.gmane.org> 2018-05-19 6:38 ` hch-jcswGhMUV9g 2018-05-19 6:38 ` hch 2018-05-11 7:59 ` [PATCH 03/20] arc: use generic dma_noncoherent_ops Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig [not found] ` <20180511075945.16548-4-hch-jcswGhMUV9g@public.gmane.org> 2018-05-11 12:44 ` Alexey Brodkin 2018-05-11 12:44 ` Alexey Brodkin 2018-05-11 7:59 ` [PATCH 04/20] arm-nommu: " Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig [not found] ` <20180511075945.16548-5-hch-jcswGhMUV9g@public.gmane.org> 2018-05-11 9:11 ` Russell King - ARM Linux 2018-05-11 9:11 ` Russell King - ARM Linux [not found] ` <20180511091114.GA16141-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org> 2018-05-22 11:53 ` Christoph Hellwig 2018-05-22 11:53 ` Christoph Hellwig 2018-05-11 13:56 ` John Garry 2018-05-11 13:56 ` John Garry 2018-05-11 7:59 ` [PATCH 05/20] c6x: " Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig [not found] ` <20180511075945.16548-6-hch-jcswGhMUV9g@public.gmane.org> 2018-05-15 0:25 ` [Linux-c6x-dev] " Mark Salter 2018-05-15 0:25 ` Mark Salter 2018-05-11 7:59 ` [PATCH 06/20] hexagon: " Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig 2018-05-11 7:59 ` [PATCH 07/20] m68k: " Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig 2018-05-11 7:59 ` [PATCH 08/20] microblaze: " Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig 2018-05-11 7:59 ` [PATCH 09/20] microblaze: remove the consistent_sync and consistent_sync_page Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig 2018-05-11 7:59 ` [PATCH 10/20] nds32: use generic dma_noncoherent_ops Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig 2018-05-11 7:59 ` [PATCH 11/20] nios2: " Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig 2018-05-11 7:59 ` [PATCH 12/20] openrisc: " Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig 2018-05-11 7:59 ` [PATCH 13/20] sh: simplify get_arch_dma_ops Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig 2018-05-11 7:59 ` [PATCH 14/20] sh: introduce a sh_cacheop_vaddr helper Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig 2018-05-11 7:59 ` [PATCH 15/20] sh: use dma_direct_ops for the CONFIG_DMA_COHERENT case Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig 2018-05-11 7:59 ` [PATCH 16/20] mm: split arch/sh/mm/consistent.c Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig 2018-05-11 7:59 ` [PATCH 17/20] sh: use generic dma_noncoherent_ops Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig 2018-05-11 7:59 ` [PATCH 18/20] xtensa: " Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig 2018-05-11 7:59 ` [PATCH 19/20] sparc: " Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig 2018-05-11 7:59 ` [PATCH 20/20] parisc: " Christoph Hellwig 2018-05-11 7:59 ` Christoph Hellwig 2018-05-13 13:26 ` common non-cache coherent direct dma mapping ops Helge Deller 2018-05-13 13:26 ` Helge Deller
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=5ac5b1e3-9b96-9c7c-4dfe-f65be45ec179@synopsys.com \ --to=vineet.gupta1-hkixbcoqz3hwk0htik3j/w@public.gmane.org \ --cc=Alexey.Brodkin-HKixBCOQz3hWk0Htik3J/w@public.gmane.org \ --cc=deanbo422-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \ --cc=green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \ --cc=hch-jcswGhMUV9g@public.gmane.org \ --cc=iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \ --cc=linux-alpha-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=linux-c6x-dev-jPsnJVOj+W6hPH1hqNUYSQ@public.gmane.org \ --cc=linux-hexagon-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=linux-m68k-cunTk1MwBs8S/qaLPR03pWD2FQJk+8+b@public.gmane.org \ --cc=linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org \ --cc=linux-parisc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=linux-sh-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=linux-xtensa-PjhNF2WwrV/0Sa2dR60CXw@public.gmane.org \ --cc=monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org \ --cc=openrisc-cunTk1MwBs9a3B2Vnqf2dGD2FQJk+8+b@public.gmane.org \ --cc=sparclinux@vg \ /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 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).