From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2992714AbcBSKb6 (ORCPT ); Fri, 19 Feb 2016 05:31:58 -0500 Received: from mout.kundenserver.de ([217.72.192.73]:64211 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757153AbcBSKby (ORCPT ); Fri, 19 Feb 2016 05:31:54 -0500 From: Arnd Bergmann To: Marek Szyprowski Cc: iommu@lists.linux-foundation.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linaro-mm-sig@lists.linaro.org, dri-devel@lists.freedesktop.org, Will Deacon , Catalin Marinas , Robin Murphy , Russell King - ARM Linux , Joerg Roedel , Laurent Pinchart , Sakari Ailus , Mark Yao , Heiko Stuebner , Tomasz Figa , Inki Dae , Bartlomiej Zolnierkiewicz , Krzysztof Kozlowski Subject: Re: [RFC 3/3] iommu: dma-iommu: use common implementation also on ARM architecture Date: Fri, 19 Feb 2016 11:30:55 +0100 Message-ID: <2724572.qtPjgumFDJ@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <1455870164-25337-4-git-send-email-m.szyprowski@samsung.com> References: <1455870164-25337-1-git-send-email-m.szyprowski@samsung.com> <1455870164-25337-4-git-send-email-m.szyprowski@samsung.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:crcfIOushurtHjcvmX+BK0on4EfociwRtHBMcD7NhZqzAURl1is VX/utaTrJU0MqjBWlra/pYKllqWJ0rawgnln7D+nEihcyogFJeRfGmUl9w7eny0hGP9znH4 PBvA/RDc8+6Oz99C0U8Mh6/wiU0Ask/rrTgqvhwrj4V9xj7DPM0wLYvOClHLEC7BvMxttLO VFmRLpTbbfV+jy5zrE/wA== X-UI-Out-Filterresults: notjunk:1;V01:K0:jpGOeGEOePs=:UDmLo7/nKLAjeDM1PzGSGc z8HAuGZAeUexqWM8IYtmV84si5hSSDuzPZsXbJJ45wkr4Jt8mB2fSzMBOg5RWIIeconu65Ina dJcZEDliiWfFeMFKx4H+guujpjgX3YuW18IiJ8jlvHDX0OsFEG1sI6/tJgG2rFvczS5VM7+Pe Mu2nAK70sTKtU5PDBct3hAYLiXnHr2FZ/7O5MS8C5J7Vh5Ects4L4mRZ7o/pYyPo1jrqJg9+T ZrqYPAsix9QgXlvcVCJSkj4fRkCrTp/bdtPrXj+W7KUwrDrR/VliLQlbpINU4jhMXbZIofQ4w dPMFwbmQjlWwrOtSDvM8TZpTLYh8oV/6VE33Gs3YuU9nDg0gM/dvEcSWN4fvwdupPytxWMWqd nooQoNZbr4eUSRYLssBp2VsI3QB1dc8RiJ0bpjusisLoS+8i/Zk6ZzkBHkjdFZIzbY84UcHDe W5+XWFZkyTQnmQ5rHVGe5mlWe305v9XAZY8YRtQ4GDgi7xCyWqp1GIXpdzEwFDaD+M8rMecoj bMSDFz6M52jOC4PprKWU0tuSsjZRH0dqJH7VL6C67k4XhHBotc0Ogq/IwGIiY/St5H3YzMgGm EUmONT0JkYIlRGwyu5NrS5/N9tWE4JeT9cHXR5HpbXFtZkDSGK+EPLBV+C02Rxw4BvP8RtXtV r/loFy7iNcsvzdMHcGh3MMtvTSu45nIJej9JzyKho0MnyA5DPLZ/WOmMlhlMCgqu0Vf0dVTV1 NTd6gTvto4XMJb+7 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Friday 19 February 2016 09:22:44 Marek Szyprowski wrote: > This patch replaces ARM-specific IOMMU-based DMA-mapping implementation > with generic IOMMU DMA-mapping code shared with ARM64 architecture. The > side-effect of this change is a switch from bitmap-based IO address space > management to tree-based code. There should be no functional changes > for drivers, which rely on initialization from generic arch_setup_dna_ops() > interface. Code, which used old arm_iommu_* functions must be updated to > new interface. > > Signed-off-by: Marek Szyprowski I like the overall idea. However, this interface from the iommu subsystem into architecture specific code: > +/* > + * The DMA API is built upon the notion of "buffer ownership". A buffer > + * is either exclusively owned by the CPU (and therefore may be accessed > + * by it) or exclusively owned by the DMA device. These helper functions > + * represent the transitions between these two ownership states. > + * > + * Note, however, that on later ARMs, this notion does not work due to > + * speculative prefetches. We model our approach on the assumption that > + * the CPU does do speculative prefetches, which means we clean caches > + * before transfers and delay cache invalidation until transfer completion. > + * > + */ > +extern void __dma_page_cpu_to_dev(struct page *, unsigned long, size_t, > + enum dma_data_direction); > +extern void __dma_page_dev_to_cpu(struct page *, unsigned long, size_t, > + enum dma_data_direction); > + > +static inline void arch_flush_page(struct device *dev, const void *virt, > + phys_addr_t phys) > +{ > + dmac_flush_range(virt, virt + PAGE_SIZE); > + outer_flush_range(phys, phys + PAGE_SIZE); > +} > + > +static inline void arch_dma_map_area(phys_addr_t phys, size_t size, > + enum dma_data_direction dir) > +{ > + unsigned int offset = phys & ~PAGE_MASK; > + __dma_page_cpu_to_dev(phys_to_page(phys & PAGE_MASK), offset, size, dir); > +} > + > +static inline void arch_dma_unmap_area(phys_addr_t phys, size_t size, > + enum dma_data_direction dir) > +{ > + unsigned int offset = phys & ~PAGE_MASK; > + __dma_page_dev_to_cpu(phys_to_page(phys & PAGE_MASK), offset, size, dir); > +} > + > +static inline pgprot_t arch_get_dma_pgprot(struct dma_attrs *attrs, > + pgprot_t prot, bool coherent) > +{ > + if (coherent) > + return prot; > + > + prot = dma_get_attr(DMA_ATTR_WRITE_COMBINE, attrs) ? > + pgprot_writecombine(prot) : > + pgprot_dmacoherent(prot); > + return prot; > +} > + > +extern void *arch_alloc_from_atomic_pool(size_t size, struct page **ret_page, > + gfp_t flags); > +extern bool arch_in_atomic_pool(void *start, size_t size); > +extern int arch_free_from_atomic_pool(void *start, size_t size); > + > + doesn't feel completely right yet. In particular the arch_flush_page() interface is probably still too specific to ARM/ARM64 and won't work that way on other architectures. I think it would be better to do this either more generic, or less generic: a) leave the iommu_dma_map_ops definition in the architecture specific code, but make it call helper functions in the drivers/iommu to do all of the really generic parts. b) clarify that this is only applicable to arch/arm and arch/arm64, and unify things further between these two, as they have very similar requirements in the CPU architecture. Arnd