From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Szyprowski Subject: Re: [RFC 3/3] iommu: dma-iommu: use common implementation also on ARM architecture Date: Thu, 25 Feb 2016 13:26:17 +0100 Message-ID: <56CEF2E9.5040706@samsung.com> References: <1455870164-25337-1-git-send-email-m.szyprowski@samsung.com> <1455870164-25337-4-git-send-email-m.szyprowski@samsung.com> <2724572.qtPjgumFDJ@wuerfel> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-reply-to: <2724572.qtPjgumFDJ@wuerfel> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Arnd Bergmann Cc: Krzysztof Kozlowski , Russell King - ARM Linux , Bartlomiej Zolnierkiewicz , Catalin Marinas , Will Deacon , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Tomasz Figa , linaro-mm-sig@lists.linaro.org, iommu@lists.linux-foundation.org, Sakari Ailus , Laurent Pinchart , Robin Murphy , linux-arm-kernel@lists.infradead.org List-Id: iommu@lists.linux-foundation.org SGVsbG8sCgpPbiAyMDE2LTAyLTE5IDExOjMwLCBBcm5kIEJlcmdtYW5uIHdyb3RlOgo+IE9uIEZy aWRheSAxOSBGZWJydWFyeSAyMDE2IDA5OjIyOjQ0IE1hcmVrIFN6eXByb3dza2kgd3JvdGU6Cj4+ IFRoaXMgcGF0Y2ggcmVwbGFjZXMgQVJNLXNwZWNpZmljIElPTU1VLWJhc2VkIERNQS1tYXBwaW5n IGltcGxlbWVudGF0aW9uCj4+IHdpdGggZ2VuZXJpYyBJT01NVSBETUEtbWFwcGluZyBjb2RlIHNo YXJlZCB3aXRoIEFSTTY0IGFyY2hpdGVjdHVyZS4gVGhlCj4+IHNpZGUtZWZmZWN0IG9mIHRoaXMg Y2hhbmdlIGlzIGEgc3dpdGNoIGZyb20gYml0bWFwLWJhc2VkIElPIGFkZHJlc3Mgc3BhY2UKPj4g bWFuYWdlbWVudCB0byB0cmVlLWJhc2VkIGNvZGUuIFRoZXJlIHNob3VsZCBiZSBubyBmdW5jdGlv bmFsIGNoYW5nZXMKPj4gZm9yIGRyaXZlcnMsIHdoaWNoIHJlbHkgb24gaW5pdGlhbGl6YXRpb24g ZnJvbSBnZW5lcmljIGFyY2hfc2V0dXBfZG5hX29wcygpCj4+IGludGVyZmFjZS4gQ29kZSwgd2hp Y2ggdXNlZCBvbGQgYXJtX2lvbW11XyogZnVuY3Rpb25zIG11c3QgYmUgdXBkYXRlZCB0bwo+PiBu ZXcgaW50ZXJmYWNlLgo+Pgo+PiBTaWduZWQtb2ZmLWJ5OiBNYXJlayBTenlwcm93c2tpIDxtLnN6 eXByb3dza2lAc2Ftc3VuZy5jb20+Cj4gSSBsaWtlIHRoZSBvdmVyYWxsIGlkZWEuIEhvd2V2ZXIs IHRoaXMgaW50ZXJmYWNlIGZyb20gdGhlIGlvbW11Cj4gc3Vic3lzdGVtIGludG8gYXJjaGl0ZWN0 dXJlIHNwZWNpZmljIGNvZGU6Cj4KPj4gKy8qCj4+ICsgKiBUaGUgRE1BIEFQSSBpcyBidWlsdCB1 cG9uIHRoZSBub3Rpb24gb2YgImJ1ZmZlciBvd25lcnNoaXAiLiAgQSBidWZmZXIKPj4gKyAqIGlz IGVpdGhlciBleGNsdXNpdmVseSBvd25lZCBieSB0aGUgQ1BVIChhbmQgdGhlcmVmb3JlIG1heSBi ZSBhY2Nlc3NlZAo+PiArICogYnkgaXQpIG9yIGV4Y2x1c2l2ZWx5IG93bmVkIGJ5IHRoZSBETUEg ZGV2aWNlLiAgVGhlc2UgaGVscGVyIGZ1bmN0aW9ucwo+PiArICogcmVwcmVzZW50IHRoZSB0cmFu c2l0aW9ucyBiZXR3ZWVuIHRoZXNlIHR3byBvd25lcnNoaXAgc3RhdGVzLgo+PiArICoKPj4gKyAq IE5vdGUsIGhvd2V2ZXIsIHRoYXQgb24gbGF0ZXIgQVJNcywgdGhpcyBub3Rpb24gZG9lcyBub3Qg d29yayBkdWUgdG8KPj4gKyAqIHNwZWN1bGF0aXZlIHByZWZldGNoZXMuICBXZSBtb2RlbCBvdXIg YXBwcm9hY2ggb24gdGhlIGFzc3VtcHRpb24gdGhhdAo+PiArICogdGhlIENQVSBkb2VzIGRvIHNw ZWN1bGF0aXZlIHByZWZldGNoZXMsIHdoaWNoIG1lYW5zIHdlIGNsZWFuIGNhY2hlcwo+PiArICog YmVmb3JlIHRyYW5zZmVycyBhbmQgZGVsYXkgY2FjaGUgaW52YWxpZGF0aW9uIHVudGlsIHRyYW5z ZmVyIGNvbXBsZXRpb24uCj4+ICsgKgo+PiArICovCj4+ICtleHRlcm4gdm9pZCBfX2RtYV9wYWdl X2NwdV90b19kZXYoc3RydWN0IHBhZ2UgKiwgdW5zaWduZWQgbG9uZywgc2l6ZV90LAo+PiArCQkJ CSAgZW51bSBkbWFfZGF0YV9kaXJlY3Rpb24pOwo+PiArZXh0ZXJuIHZvaWQgX19kbWFfcGFnZV9k ZXZfdG9fY3B1KHN0cnVjdCBwYWdlICosIHVuc2lnbmVkIGxvbmcsIHNpemVfdCwKPj4gKwkJCQkg IGVudW0gZG1hX2RhdGFfZGlyZWN0aW9uKTsKPj4gKwo+PiArc3RhdGljIGlubGluZSB2b2lkIGFy Y2hfZmx1c2hfcGFnZShzdHJ1Y3QgZGV2aWNlICpkZXYsIGNvbnN0IHZvaWQgKnZpcnQsCj4+ICsJ CQkgICAgcGh5c19hZGRyX3QgcGh5cykKPj4gK3sKPj4gKwlkbWFjX2ZsdXNoX3JhbmdlKHZpcnQs IHZpcnQgKyBQQUdFX1NJWkUpOwo+PiArCW91dGVyX2ZsdXNoX3JhbmdlKHBoeXMsIHBoeXMgKyBQ QUdFX1NJWkUpOwo+PiArfQo+PiArCj4+ICtzdGF0aWMgaW5saW5lIHZvaWQgYXJjaF9kbWFfbWFw X2FyZWEocGh5c19hZGRyX3QgcGh5cywgc2l6ZV90IHNpemUsCj4+ICsJCQkJICAgICBlbnVtIGRt YV9kYXRhX2RpcmVjdGlvbiBkaXIpCj4+ICt7Cj4+ICsJdW5zaWduZWQgaW50IG9mZnNldCA9IHBo eXMgJiB+UEFHRV9NQVNLOwo+PiArCV9fZG1hX3BhZ2VfY3B1X3RvX2RldihwaHlzX3RvX3BhZ2Uo cGh5cyAmIFBBR0VfTUFTSyksIG9mZnNldCwgc2l6ZSwgZGlyKTsKPj4gK30KPj4gKwo+PiArc3Rh dGljIGlubGluZSB2b2lkIGFyY2hfZG1hX3VubWFwX2FyZWEocGh5c19hZGRyX3QgcGh5cywgc2l6 ZV90IHNpemUsCj4+ICsJCQkJICAgICAgIGVudW0gZG1hX2RhdGFfZGlyZWN0aW9uIGRpcikKPj4g K3sKPj4gKwl1bnNpZ25lZCBpbnQgb2Zmc2V0ID0gcGh5cyAmIH5QQUdFX01BU0s7Cj4+ICsJX19k bWFfcGFnZV9kZXZfdG9fY3B1KHBoeXNfdG9fcGFnZShwaHlzICYgUEFHRV9NQVNLKSwgb2Zmc2V0 LCBzaXplLCBkaXIpOwo+PiArfQo+PiArCj4+ICtzdGF0aWMgaW5saW5lIHBncHJvdF90IGFyY2hf Z2V0X2RtYV9wZ3Byb3Qoc3RydWN0IGRtYV9hdHRycyAqYXR0cnMsCj4+ICsJCQkJCXBncHJvdF90 IHByb3QsIGJvb2wgY29oZXJlbnQpCj4+ICt7Cj4+ICsJaWYgKGNvaGVyZW50KQo+PiArCQlyZXR1 cm4gcHJvdDsKPj4gKwo+PiArCXByb3QgPSBkbWFfZ2V0X2F0dHIoRE1BX0FUVFJfV1JJVEVfQ09N QklORSwgYXR0cnMpID8KPj4gKwkJCSAgICBwZ3Byb3Rfd3JpdGVjb21iaW5lKHByb3QpIDoKPj4g KwkJCSAgICBwZ3Byb3RfZG1hY29oZXJlbnQocHJvdCk7Cj4+ICsJcmV0dXJuIHByb3Q7Cj4+ICt9 Cj4+ICsKPj4gK2V4dGVybiB2b2lkICphcmNoX2FsbG9jX2Zyb21fYXRvbWljX3Bvb2woc2l6ZV90 IHNpemUsIHN0cnVjdCBwYWdlICoqcmV0X3BhZ2UsCj4+ICsJCQkJCSBnZnBfdCBmbGFncyk7Cj4+ ICtleHRlcm4gYm9vbCBhcmNoX2luX2F0b21pY19wb29sKHZvaWQgKnN0YXJ0LCBzaXplX3Qgc2l6 ZSk7Cj4+ICtleHRlcm4gaW50IGFyY2hfZnJlZV9mcm9tX2F0b21pY19wb29sKHZvaWQgKnN0YXJ0 LCBzaXplX3Qgc2l6ZSk7Cj4+ICsKPj4gKwo+IGRvZXNuJ3QgZmVlbCBjb21wbGV0ZWx5IHJpZ2h0 IHlldC4gSW4gcGFydGljdWxhciB0aGUgYXJjaF9mbHVzaF9wYWdlKCkKPiBpbnRlcmZhY2UgaXMg cHJvYmFibHkgc3RpbGwgdG9vIHNwZWNpZmljIHRvIEFSTS9BUk02NCBhbmQgd29uJ3Qgd29yawo+ IHRoYXQgd2F5IG9uIG90aGVyIGFyY2hpdGVjdHVyZXMuCj4KPiBJIHRoaW5rIGl0IHdvdWxkIGJl IGJldHRlciB0byBkbyB0aGlzIGVpdGhlciBtb3JlIGdlbmVyaWMsIG9yIGxlc3MgZ2VuZXJpYzoK Pgo+IGEpIGxlYXZlIHRoZSBpb21tdV9kbWFfbWFwX29wcyBkZWZpbml0aW9uIGluIHRoZSBhcmNo aXRlY3R1cmUgc3BlY2lmaWMKPiAgICAgY29kZSwgYnV0IG1ha2UgaXQgY2FsbCBoZWxwZXIgZnVu Y3Rpb25zIGluIHRoZSBkcml2ZXJzL2lvbW11IHRvIGRvIGFsbAo+ICAgICBvZiB0aGUgcmVhbGx5 IGdlbmVyaWMgcGFydHMuCj4KPiBiKSBjbGFyaWZ5IHRoYXQgdGhpcyBpcyBvbmx5IGFwcGxpY2Fi bGUgdG8gYXJjaC9hcm0gYW5kIGFyY2gvYXJtNjQsIGFuZAo+ICAgICB1bmlmeSB0aGluZ3MgZnVy dGhlciBiZXR3ZWVuIHRoZXNlIHR3bywgYXMgdGhleSBoYXZlIHZlcnkgc2ltaWxhcgo+ICAgICBy ZXF1aXJlbWVudHMgaW4gdGhlIENQVSBhcmNoaXRlY3R1cmUuCgpTb21lIHJlYWxseSBnZW5lcmlj IHBhcnRzIGFyZSBhbHJlYWR5IGluIGlvbW11L2RtYS1pb21tdS5jIGFuZCBvbmUgY2FuIGJ1aWxk Cml0J3Mgb3duLCBub24tQVJNIENQVSBhcmNoaXRlY3R1cmUgYmFzZWQgSU9NTVUvRE1BLW1hcHBp bmcgY29kZS4gSW5pdGlhbGx5IEkKYWxzbyB3YW50ZWQgdG8gdXNlIHRoYXQgZ2VuZXJpYyBjb2Rl IG9uIGJvdGggQVJNIGFuZCBBUk02NCwgYnV0IGl0IAp0dXJuZWQgb3V0CnRoYXQgYm90aCBhcmNo cywgQVJNIGFuZCBBUk02NCB3aWxsIGR1cGxpY2F0ZSA5OSUgb2YgY29kZSwgd2hpY2ggdXNlIHRo aXMKJ2dlbmVyaWMnIGZ1bmN0aW9ucy4gVGhpcyB3YXMgdGhlIHJlYXNvbiB3aHkgSSBkZWRpZGVk IHRvIG1vdmUgYWxsIHRoYXQKY29tbW9uIGNvZGUgZnJvbSBhcmNoL3thcm0sYXJtNjR9L21tL2Rt YS1tYXBwaW5nLmMgdG8KZHJpdmVycy9pb21tdS9kbWEtaW9tbXUtb3BzLmMKCkknbSBub3Qgc3Vy ZSBpZiBJIGNhbiBkZXNpZ24gYWxsIHRoZSBjaGFuZ2VzIHRoYXQgbmVlZCB0byBiZSBtYWRlIHRv CmRyaXZlcnMvaW9tbXUvZG1hLWlvbW11LW9wcy5jIHRvIG1ha2UgaXQgbW9yZSBnZW5lcmljLiBN YXliZSB3aGVuIG9uZSB3aWxsCnRyeSB0byB1c2UgdGhhdCBjb2RlIHdpdGggb3RoZXIsIG5vbi1B Uk0gYXJjaGl0ZWN0dXJlIGJhc2VkIGFyY2ggZ2x1ZSBjb2RlLAphIGJldHRlciBhYnN0cmFjdGlv biBjYW4gYmUgZGV2ZWxvcGVkLiBGb3Igbm93IEkgd291bGQgbGlrZSB0byBrZWVwIGFsbCB0aGlz CmNvZGUgaW4gYSBjb21tb24gcGxhY2Ugc28gYm90aCBhcm0gYW5kIGFybTY0IHdpbGwgYmVuZWZp dCBmcm9tIGltcHJvdmVtZW50cwpkb25lIHRoZXJlLgoKQmVzdCByZWdhcmRzCi0tIApNYXJlayBT enlwcm93c2tpLCBQaEQKU2Ftc3VuZyBSJkQgSW5zdGl0dXRlIFBvbGFuZAoKX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlz dApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0 b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: m.szyprowski@samsung.com (Marek Szyprowski) Date: Thu, 25 Feb 2016 13:26:17 +0100 Subject: [RFC 3/3] iommu: dma-iommu: use common implementation also on ARM architecture In-Reply-To: <2724572.qtPjgumFDJ@wuerfel> References: <1455870164-25337-1-git-send-email-m.szyprowski@samsung.com> <1455870164-25337-4-git-send-email-m.szyprowski@samsung.com> <2724572.qtPjgumFDJ@wuerfel> Message-ID: <56CEF2E9.5040706@samsung.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hello, On 2016-02-19 11:30, Arnd Bergmann wrote: > 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. Some really generic parts are already in iommu/dma-iommu.c and one can build it's own, non-ARM CPU architecture based IOMMU/DMA-mapping code. Initially I also wanted to use that generic code on both ARM and ARM64, but it turned out that both archs, ARM and ARM64 will duplicate 99% of code, which use this 'generic' functions. This was the reason why I dedided to move all that common code from arch/{arm,arm64}/mm/dma-mapping.c to drivers/iommu/dma-iommu-ops.c I'm not sure if I can design all the changes that need to be made to drivers/iommu/dma-iommu-ops.c to make it more generic. Maybe when one will try to use that code with other, non-ARM architecture based arch glue code, a better abstraction can be developed. For now I would like to keep all this code in a common place so both arm and arm64 will benefit from improvements done there. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760595AbcBYM0Y (ORCPT ); Thu, 25 Feb 2016 07:26:24 -0500 Received: from mailout1.w1.samsung.com ([210.118.77.11]:23128 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753606AbcBYM0W (ORCPT ); Thu, 25 Feb 2016 07:26:22 -0500 X-AuditID: cbfec7f5-f79b16d000005389-08-56cef2ebe907 Subject: Re: [RFC 3/3] iommu: dma-iommu: use common implementation also on ARM architecture To: Arnd Bergmann References: <1455870164-25337-1-git-send-email-m.szyprowski@samsung.com> <1455870164-25337-4-git-send-email-m.szyprowski@samsung.com> <2724572.qtPjgumFDJ@wuerfel> 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 From: Marek Szyprowski Message-id: <56CEF2E9.5040706@samsung.com> Date: Thu, 25 Feb 2016 13:26:17 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-version: 1.0 In-reply-to: <2724572.qtPjgumFDJ@wuerfel> Content-type: text/plain; charset=utf-8; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrOIsWRmVeSWpSXmKPExsVy+t/xa7qvP50LMzjzhMXi76Rj7BYbZ6xn tXi/rIfR4srX92wW/x+9ZrWYdH8Ci8WC/dYWnbM3sFu8fmFo0TlxCbvFlysPmSw2Pb7GanF5 1xw2i9uXeS12TDnAZHHwwxNWizP7V7JZfG79x2bx8uMJFgdhjycH5zF5rJm3htGjpbmHzeP3 r0mMHrMbLrJ4zO6Yyepx+OtCFo/73ceZPDYvqfe4/e8xs8fkG8sZPf7O2s/i0bdlFaPH9mvz mD0+b5IL4I/isklJzcksSy3St0vgytj/5g5bwTflitfLnrM3MP6R6WLk5JAQMJH4NfkIG4Qt JnHh3nogm4tDSGApo8SnhduYIJznjBINq7tZuhg5OIQFYiWeTqgEaRARUJSY+uIZM0TNMkaJ Gz8Os4A4zALnWSWW3F7FAlLFJmAo0fW2C2wFr4CWxJvvxxhBbBYBVYmH186B2aICMRLH30HY vAKCEj8m3wPr5RTQlDgw5So7iM0sYCbx5eVhVghbXmLzmrfMExgFZiFpmYWkbBaSsgWMzKsY RVNLkwuKk9JzjfSKE3OLS/PS9ZLzczcxQqL36w7GpcesDjEKcDAq8fAy/DwbJsSaWFZcmXuI UYKDWUmEN+LtuTAh3pTEyqrUovz4otKc1OJDjNIcLErivDN3vQ8REkhPLEnNTk0tSC2CyTJx cEo1MCoffLlOvLbvU87kBc/sta7O89gjfn3Cn6I7X0u3PcjSfGC41/3G54CIKxqrg790pjVE 881o5OF7e/cyq537IZ/9TrdnrVgqkfV42r9zi1/tmtDrk/Lfcs6H94fbF/ocOsPaF6f9YPma X3U//36rcTiw7eaKXK4Dkfc8X039oyLGlvFp9wkv7bvNSizFGYmGWsxFxYkAGSYY+9oCAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, On 2016-02-19 11:30, Arnd Bergmann wrote: > 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. Some really generic parts are already in iommu/dma-iommu.c and one can build it's own, non-ARM CPU architecture based IOMMU/DMA-mapping code. Initially I also wanted to use that generic code on both ARM and ARM64, but it turned out that both archs, ARM and ARM64 will duplicate 99% of code, which use this 'generic' functions. This was the reason why I dedided to move all that common code from arch/{arm,arm64}/mm/dma-mapping.c to drivers/iommu/dma-iommu-ops.c I'm not sure if I can design all the changes that need to be made to drivers/iommu/dma-iommu-ops.c to make it more generic. Maybe when one will try to use that code with other, non-ARM architecture based arch glue code, a better abstraction can be developed. For now I would like to keep all this code in a common place so both arm and arm64 will benefit from improvements done there. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland