From patchwork Wed Feb 3 13:31:16 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Robin Murphy X-Patchwork-Id: 8203131 X-Patchwork-Delegate: geert@linux-m68k.org Return-Path: X-Original-To: patchwork-linux-renesas-soc@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 97A2DBEEE5 for ; Wed, 3 Feb 2016 13:31:30 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id BEBB120253 for ; Wed, 3 Feb 2016 13:31:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0D44B201F2 for ; Wed, 3 Feb 2016 13:31:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754108AbcBCNbW (ORCPT ); Wed, 3 Feb 2016 08:31:22 -0500 Received: from foss.arm.com ([217.140.101.70]:34280 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751952AbcBCNbU (ORCPT ); Wed, 3 Feb 2016 08:31:20 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id BFCD13A1; Wed, 3 Feb 2016 05:30:35 -0800 (PST) Received: from [10.1.205.42] (e104324-lin.cambridge.arm.com [10.1.205.42]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 706C73F21A; Wed, 3 Feb 2016 05:31:18 -0800 (PST) Subject: Re: [PATCH v2 1/5] dma-mapping: add dma_{map,unmap}_page_attrs To: =?UTF-8?Q?Niklas_S=c3=b6derlund?= , dmaengine@vger.kernel.org, linux-renesas-soc@vger.kernel.org, vinod.koul@intel.com References: <1453384895-20395-1-git-send-email-niklas.soderlund+renesas@ragnatech.se> <1453384895-20395-2-git-send-email-niklas.soderlund+renesas@ragnatech.se> Cc: linus.walleij@linaro.org, laurent.pinchart@ideasonboard.com, geert+renesas@glider.be, dan.j.williams@intel.com From: Robin Murphy Message-ID: <56B20124.2030809@arm.com> Date: Wed, 3 Feb 2016 13:31:16 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <1453384895-20395-2-git-send-email-niklas.soderlund+renesas@ragnatech.se> Sender: linux-renesas-soc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-renesas-soc@vger.kernel.org X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 21/01/16 14:01, Niklas Söderlund wrote: > Add a version of dmap_{map,unmap}_page that can pass on attributes to > the underlaying map_page. This already exists for dma_{map,unmap}_single > and dmap_{map,unmap}_sg versions. This looks reasonable in isolation, but for the task at hand I'm pretty sure it's the wrong thing to do. The problem is that the DMA mapping and IOMMU APIs are all geared around dealing with RAM, so what you're going to end up with if you use this on an ARM system is the slave device's MMIO registers mapped in the IOMMU as Normal memory. The net result of that is that the interconnects between the IOMMU's downstream port and the slave device are going to have free reign to reorder or merge the DMA engine's transactions, and I'm sure you can imagine how utterly disastrous that would be for e.g. reading/writing a FIFO. It's even worse if the DMA engine is cache-coherent (either naturally, or by virtue of the IOMMU), in which case the prospect of reads and writes coming out of the IOMMU marked as Normal-Cacheable and allocating into system caches is even more terrifyingly unpredictable. I spent a little time recently looking into how we might handle platform MSIs with IOMMU-backed DMA mapping, and the notion of slave DMA being a similar problem did cross my mind, so I'm glad it's already being looked at :) My initial thought was that a robust general solution probably involves extending the DMA API with something like dma_map_resource(), which would be a no-op with no IOMMU (or with IOMMUs like VT-d where magic hardware solves the problem), interacting with something like the below extension of the IOMMU API (plucked from my local MSI proof-of-concept hacks). Robin. --->8--- From: Robin Murphy Date: Wed, 23 Sep 2015 15:49:05 +0100 Subject: [PATCH] iommu: Add MMIO mapping type On some platforms, MMIO regions might need slightly different treatment compared to mapping regular memory; add the notion of MMIO mappings to the IOMMU API's memory type flags, so that callers can let the IOMMU drivers know to do the right thing. Signed-off-by: Robin Murphy Acked-by: Laurent Pinchart --- drivers/iommu/io-pgtable-arm.c | 4 +++- include/linux/iommu.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) --->8--- > Signed-off-by: Niklas Söderlund > --- > include/asm-generic/dma-mapping-common.h | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/include/asm-generic/dma-mapping-common.h b/include/asm-generic/dma-mapping-common.h > index b1bc954..bb08302 100644 > --- a/include/asm-generic/dma-mapping-common.h > +++ b/include/asm-generic/dma-mapping-common.h > @@ -74,29 +74,33 @@ static inline void dma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg > ops->unmap_sg(dev, sg, nents, dir, attrs); > } > > -static inline dma_addr_t dma_map_page(struct device *dev, struct page *page, > - size_t offset, size_t size, > - enum dma_data_direction dir) > +static inline dma_addr_t dma_map_page_attrs(struct device *dev, > + struct page *page, > + size_t offset, size_t size, > + enum dma_data_direction dir, > + struct dma_attrs *attrs) > { > struct dma_map_ops *ops = get_dma_ops(dev); > dma_addr_t addr; > > kmemcheck_mark_initialized(page_address(page) + offset, size); > BUG_ON(!valid_dma_direction(dir)); > - addr = ops->map_page(dev, page, offset, size, dir, NULL); > + addr = ops->map_page(dev, page, offset, size, dir, attrs); > debug_dma_map_page(dev, page, offset, size, dir, addr, false); > > return addr; > } > > -static inline void dma_unmap_page(struct device *dev, dma_addr_t addr, > - size_t size, enum dma_data_direction dir) > +static inline void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr, > + size_t size, > + enum dma_data_direction dir, > + struct dma_attrs *attrs) > { > struct dma_map_ops *ops = get_dma_ops(dev); > > BUG_ON(!valid_dma_direction(dir)); > if (ops->unmap_page) > - ops->unmap_page(dev, addr, size, dir, NULL); > + ops->unmap_page(dev, addr, size, dir, attrs); > debug_dma_unmap_page(dev, addr, size, dir, false); > } > > @@ -181,6 +185,8 @@ dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg, > #define dma_unmap_single(d, a, s, r) dma_unmap_single_attrs(d, a, s, r, NULL) > #define dma_map_sg(d, s, n, r) dma_map_sg_attrs(d, s, n, r, NULL) > #define dma_unmap_sg(d, s, n, r) dma_unmap_sg_attrs(d, s, n, r, NULL) > +#define dma_map_page(d, p, o, s, r) dma_map_page_attrs(d, p, o, s, r, NULL) > +#define dma_unmap_page(d, a, s, r) dma_unmap_page_attrs(d, a, s, r, NULL) > > extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, > void *cpu_addr, dma_addr_t dma_addr, size_t size); > diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c index 8bbcbfe..5b5c299 100644 --- a/drivers/iommu/io-pgtable-arm.c +++ b/drivers/iommu/io-pgtable-arm.c @@ -363,7 +363,9 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable *data, pte |= ARM_LPAE_PTE_HAP_READ; if (prot & IOMMU_WRITE) pte |= ARM_LPAE_PTE_HAP_WRITE; - if (prot & IOMMU_CACHE) + if (prot & IOMMU_MMIO) + pte |= ARM_LPAE_PTE_MEMATTR_DEV; + else if (prot & IOMMU_CACHE) pte |= ARM_LPAE_PTE_MEMATTR_OIWB; else pte |= ARM_LPAE_PTE_MEMATTR_NC; diff --git a/include/linux/iommu.h b/include/linux/iommu.h index f28dff3..addd25d 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -30,6 +30,7 @@ #define IOMMU_WRITE (1 << 1) #define IOMMU_CACHE (1 << 2) /* DMA cache coherency */ #define IOMMU_NOEXEC (1 << 3) +#define IOMMU_MMIO (1 << 4) /* e.g. things like MSI doorbells */ struct iommu_ops; struct iommu_group;