From mboxrd@z Thu Jan 1 00:00:00 1970 From: Robin Murphy Subject: Re: [RFC 3/3] iommu: dma-iommu: use common implementation also on ARM architecture Date: Tue, 15 Mar 2016 12:33:03 +0000 Message-ID: <56E800FF.6050705@arm.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 , Marek Szyprowski Cc: linaro-mm-sig@lists.linaro.org, Krzysztof Kozlowski , Russell King - ARM Linux , Bartlomiej Zolnierkiewicz , Catalin Marinas , Will Deacon , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, iommu@lists.linux-foundation.org, Sakari Ailus , Laurent Pinchart , linux-arm-kernel@lists.infradead.org List-Id: iommu@lists.linux-foundation.org SGkgTWFyZWssIEFybmQsCgpPbiAxOS8wMi8xNiAxMDozMCwgQXJuZCBCZXJnbWFubiB3cm90ZToK PiBPbiBGcmlkYXkgMTkgRmVicnVhcnkgMjAxNiAwOToyMjo0NCBNYXJlayBTenlwcm93c2tpIHdy b3RlOgo+PiBUaGlzIHBhdGNoIHJlcGxhY2VzIEFSTS1zcGVjaWZpYyBJT01NVS1iYXNlZCBETUEt bWFwcGluZyBpbXBsZW1lbnRhdGlvbgo+PiB3aXRoIGdlbmVyaWMgSU9NTVUgRE1BLW1hcHBpbmcg Y29kZSBzaGFyZWQgd2l0aCBBUk02NCBhcmNoaXRlY3R1cmUuIFRoZQo+PiBzaWRlLWVmZmVjdCBv ZiB0aGlzIGNoYW5nZSBpcyBhIHN3aXRjaCBmcm9tIGJpdG1hcC1iYXNlZCBJTyBhZGRyZXNzIHNw YWNlCj4+IG1hbmFnZW1lbnQgdG8gdHJlZS1iYXNlZCBjb2RlLiBUaGVyZSBzaG91bGQgYmUgbm8g ZnVuY3Rpb25hbCBjaGFuZ2VzCj4+IGZvciBkcml2ZXJzLCB3aGljaCByZWx5IG9uIGluaXRpYWxp emF0aW9uIGZyb20gZ2VuZXJpYyBhcmNoX3NldHVwX2RuYV9vcHMoKQo+PiBpbnRlcmZhY2UuIENv ZGUsIHdoaWNoIHVzZWQgb2xkIGFybV9pb21tdV8qIGZ1bmN0aW9ucyBtdXN0IGJlIHVwZGF0ZWQg dG8KPj4gbmV3IGludGVyZmFjZS4KPj4KPj4gU2lnbmVkLW9mZi1ieTogTWFyZWsgU3p5cHJvd3Nr aSA8bS5zenlwcm93c2tpQHNhbXN1bmcuY29tPgo+Cj4gSSBsaWtlIHRoZSBvdmVyYWxsIGlkZWEu IEhvd2V2ZXIsIHRoaXMgaW50ZXJmYWNlIGZyb20gdGhlIGlvbW11Cj4gc3Vic3lzdGVtIGludG8g YXJjaGl0ZWN0dXJlIHNwZWNpZmljIGNvZGU6Cj4KPj4gKy8qCj4+ICsgKiBUaGUgRE1BIEFQSSBp cyBidWlsdCB1cG9uIHRoZSBub3Rpb24gb2YgImJ1ZmZlciBvd25lcnNoaXAiLiAgQSBidWZmZXIK Pj4gKyAqIGlzIGVpdGhlciBleGNsdXNpdmVseSBvd25lZCBieSB0aGUgQ1BVIChhbmQgdGhlcmVm b3JlIG1heSBiZSBhY2Nlc3NlZAo+PiArICogYnkgaXQpIG9yIGV4Y2x1c2l2ZWx5IG93bmVkIGJ5 IHRoZSBETUEgZGV2aWNlLiAgVGhlc2UgaGVscGVyIGZ1bmN0aW9ucwo+PiArICogcmVwcmVzZW50 IHRoZSB0cmFuc2l0aW9ucyBiZXR3ZWVuIHRoZXNlIHR3byBvd25lcnNoaXAgc3RhdGVzLgo+PiAr ICoKPj4gKyAqIE5vdGUsIGhvd2V2ZXIsIHRoYXQgb24gbGF0ZXIgQVJNcywgdGhpcyBub3Rpb24g ZG9lcyBub3Qgd29yayBkdWUgdG8KPj4gKyAqIHNwZWN1bGF0aXZlIHByZWZldGNoZXMuICBXZSBt b2RlbCBvdXIgYXBwcm9hY2ggb24gdGhlIGFzc3VtcHRpb24gdGhhdAo+PiArICogdGhlIENQVSBk b2VzIGRvIHNwZWN1bGF0aXZlIHByZWZldGNoZXMsIHdoaWNoIG1lYW5zIHdlIGNsZWFuIGNhY2hl cwo+PiArICogYmVmb3JlIHRyYW5zZmVycyBhbmQgZGVsYXkgY2FjaGUgaW52YWxpZGF0aW9uIHVu dGlsIHRyYW5zZmVyIGNvbXBsZXRpb24uCj4+ICsgKgo+PiArICovCj4+ICtleHRlcm4gdm9pZCBf X2RtYV9wYWdlX2NwdV90b19kZXYoc3RydWN0IHBhZ2UgKiwgdW5zaWduZWQgbG9uZywgc2l6ZV90 LAo+PiArCQkJCSAgZW51bSBkbWFfZGF0YV9kaXJlY3Rpb24pOwo+PiArZXh0ZXJuIHZvaWQgX19k bWFfcGFnZV9kZXZfdG9fY3B1KHN0cnVjdCBwYWdlICosIHVuc2lnbmVkIGxvbmcsIHNpemVfdCwK Pj4gKwkJCQkgIGVudW0gZG1hX2RhdGFfZGlyZWN0aW9uKTsKPj4gKwo+PiArc3RhdGljIGlubGlu ZSB2b2lkIGFyY2hfZmx1c2hfcGFnZShzdHJ1Y3QgZGV2aWNlICpkZXYsIGNvbnN0IHZvaWQgKnZp cnQsCj4+ICsJCQkgICAgcGh5c19hZGRyX3QgcGh5cykKPj4gK3sKPj4gKwlkbWFjX2ZsdXNoX3Jh bmdlKHZpcnQsIHZpcnQgKyBQQUdFX1NJWkUpOwo+PiArCW91dGVyX2ZsdXNoX3JhbmdlKHBoeXMs IHBoeXMgKyBQQUdFX1NJWkUpOwo+PiArfQo+PiArCj4+ICtzdGF0aWMgaW5saW5lIHZvaWQgYXJj aF9kbWFfbWFwX2FyZWEocGh5c19hZGRyX3QgcGh5cywgc2l6ZV90IHNpemUsCj4+ICsJCQkJICAg ICBlbnVtIGRtYV9kYXRhX2RpcmVjdGlvbiBkaXIpCj4+ICt7Cj4+ICsJdW5zaWduZWQgaW50IG9m ZnNldCA9IHBoeXMgJiB+UEFHRV9NQVNLOwo+PiArCV9fZG1hX3BhZ2VfY3B1X3RvX2RldihwaHlz X3RvX3BhZ2UocGh5cyAmIFBBR0VfTUFTSyksIG9mZnNldCwgc2l6ZSwgZGlyKTsKPj4gK30KPj4g Kwo+PiArc3RhdGljIGlubGluZSB2b2lkIGFyY2hfZG1hX3VubWFwX2FyZWEocGh5c19hZGRyX3Qg cGh5cywgc2l6ZV90IHNpemUsCj4+ICsJCQkJICAgICAgIGVudW0gZG1hX2RhdGFfZGlyZWN0aW9u IGRpcikKPj4gK3sKPj4gKwl1bnNpZ25lZCBpbnQgb2Zmc2V0ID0gcGh5cyAmIH5QQUdFX01BU0s7 Cj4+ICsJX19kbWFfcGFnZV9kZXZfdG9fY3B1KHBoeXNfdG9fcGFnZShwaHlzICYgUEFHRV9NQVNL KSwgb2Zmc2V0LCBzaXplLCBkaXIpOwo+PiArfQo+PiArCj4+ICtzdGF0aWMgaW5saW5lIHBncHJv dF90IGFyY2hfZ2V0X2RtYV9wZ3Byb3Qoc3RydWN0IGRtYV9hdHRycyAqYXR0cnMsCj4+ICsJCQkJ CXBncHJvdF90IHByb3QsIGJvb2wgY29oZXJlbnQpCj4+ICt7Cj4+ICsJaWYgKGNvaGVyZW50KQo+ PiArCQlyZXR1cm4gcHJvdDsKPj4gKwo+PiArCXByb3QgPSBkbWFfZ2V0X2F0dHIoRE1BX0FUVFJf V1JJVEVfQ09NQklORSwgYXR0cnMpID8KPj4gKwkJCSAgICBwZ3Byb3Rfd3JpdGVjb21iaW5lKHBy b3QpIDoKPj4gKwkJCSAgICBwZ3Byb3RfZG1hY29oZXJlbnQocHJvdCk7Cj4+ICsJcmV0dXJuIHBy b3Q7Cj4+ICt9Cj4+ICsKPj4gK2V4dGVybiB2b2lkICphcmNoX2FsbG9jX2Zyb21fYXRvbWljX3Bv b2woc2l6ZV90IHNpemUsIHN0cnVjdCBwYWdlICoqcmV0X3BhZ2UsCj4+ICsJCQkJCSBnZnBfdCBm bGFncyk7Cj4+ICtleHRlcm4gYm9vbCBhcmNoX2luX2F0b21pY19wb29sKHZvaWQgKnN0YXJ0LCBz aXplX3Qgc2l6ZSk7Cj4+ICtleHRlcm4gaW50IGFyY2hfZnJlZV9mcm9tX2F0b21pY19wb29sKHZv aWQgKnN0YXJ0LCBzaXplX3Qgc2l6ZSk7Cj4+ICsKPj4gKwo+Cj4gZG9lc24ndCBmZWVsIGNvbXBs ZXRlbHkgcmlnaHQgeWV0LiBJbiBwYXJ0aWN1bGFyIHRoZSBhcmNoX2ZsdXNoX3BhZ2UoKQo+IGlu dGVyZmFjZSBpcyBwcm9iYWJseSBzdGlsbCB0b28gc3BlY2lmaWMgdG8gQVJNL0FSTTY0IGFuZCB3 b24ndCB3b3JrCj4gdGhhdCB3YXkgb24gb3RoZXIgYXJjaGl0ZWN0dXJlcy4KPgo+IEkgdGhpbmsg aXQgd291bGQgYmUgYmV0dGVyIHRvIGRvIHRoaXMgZWl0aGVyIG1vcmUgZ2VuZXJpYywgb3IgbGVz cyBnZW5lcmljOgo+Cj4gYSkgbGVhdmUgdGhlIGlvbW11X2RtYV9tYXBfb3BzIGRlZmluaXRpb24g aW4gdGhlIGFyY2hpdGVjdHVyZSBzcGVjaWZpYwo+ICAgICBjb2RlLCBidXQgbWFrZSBpdCBjYWxs IGhlbHBlciBmdW5jdGlvbnMgaW4gdGhlIGRyaXZlcnMvaW9tbXUgdG8gZG8gYWxsCj4gICAgIG9m IHRoZSByZWFsbHkgZ2VuZXJpYyBwYXJ0cy4KClRoaXMgd2FzIGNlcnRhaW5seSB0aGUgb3JpZ2lu YWwgaW50ZW50IG9mIHRoZSBhcm02NCBjb2RlLiBUaGUgZGl2aXNpb24gCm9mIHJlc3BvbnNpYmls aXR5IHRoZXJlIGlzIGEgY29uc2Npb3VzIGRlY2lzaW9uIC0gSU9NTVUtQVBJLXdyYW5nbGluZyAK Z29lcyBpbiB0aGUgY29tbW9uIGNvZGUsIGNhY2hlIG1haW50ZW5hbmNlIGFuZCBhY3R1YWwgZG1h X21hcF9vcHMgc3RheSAKaGlkZGVuIGluIGFyY2hpdGVjdHVyZS1wcml2YXRlIGNvZGUsIHNhZmUg ZnJvbSBhYnVzZS4gSXQncyB2ZXJ5IG11Y2ggCm1vZGVsbGVkIG9uIFNXSU9UTEIuCgpHaXZlbiBh bGwgdGhlIHdvcmsgUnVzc2VsbCBkaWQgbGFzdCB5ZWFyIGdldHRpbmcgcmlkIG9mIGRpcmVjdCB1 c2VzIG9mIAp0aGUgZG1hY18qIGNhY2hlIG1haW50ZW5hbmNlIGZ1bmN0aW9ucyBieSBBUk0gZHJp dmVycywgSSBkb24ndCB0aGluayAKYnJpbmdpbmcgYWxsIG9mIHRoYXQgYmFjayBpcyBhIGdvb2Qg d2F5IHRvIGdvIC0gUGVyc29uYWxseSBJJ2QgbXVjaCAKcmF0aGVyIHNlZSBzZXZlcmFsIGRvemVu IGxpbmVzIG9mIHZlcnkgc2ltaWxhciBsb29raW5nIChvdGhlciB0aGFuIApoaWdobWVtIGFuZCBv dXRlciBjYWNoZSBzdHVmZikgYXJjaC1wcml2YXRlIGNvZGUgaWYgaXQgbWFpbnRhaW5zIGEgCnJv YnVzdCBhbmQgY2xlYXJseS1kZWZpbmVkIGFic3RyYWN0aW9uIChhbmQgYXZvaWRzIHlldCBhbm90 aGVyIGxldmVsIG9mIAppbmRpcmVjdGlvbikuIEl0IGRvZXMgYWxzbyBzZWVtIGEgbGl0dGxlIG9k ZCB0byBmYWN0b3Igb3V0IG9ubHkgaGFsZiB0aGUgCmZpbGUgb24gdGhlIGdyb3VuZHMgb2YgYXJj aGl0ZWN0dXJhbCBzaW1pbGFyaXR5LCB3aGVuIHRoYXQgYXJndW1lbnQgCmFwcGxpZXMgZXF1YWxs eSB0byB0aGUgb3RoZXIgKG5vbi1JT01NVSkgaGFsZiB0b28uIEkgdGhpbmsgdGhlIHJlY2VudCAK dHJlZS13aWRlIGNvbnZlcnNpb24gdG8gZ2VuZXJpYyBkbWFfbWFwX29wcyB3YXMgaW4gcGFydCBt b3RpdmF0ZWQgYnkgdGhlIAp0aG91Z2h0IG9mIGNvbW1vbiBpbXBsZW1lbnRhdGlvbnMsIHNvIEkn bSBzdXJlIHRoYXQncyBzb21ldGhpbmcgd2UgY2FuIApyZXZpc2l0IGluIGR1ZSBjb3Vyc2UuCgpS b2Jpbi4KCj4KPiBiKSBjbGFyaWZ5IHRoYXQgdGhpcyBpcyBvbmx5IGFwcGxpY2FibGUgdG8gYXJj aC9hcm0gYW5kIGFyY2gvYXJtNjQsIGFuZAo+ICAgICB1bmlmeSB0aGluZ3MgZnVydGhlciBiZXR3 ZWVuIHRoZXNlIHR3bywgYXMgdGhleSBoYXZlIHZlcnkgc2ltaWxhcgo+ICAgICByZXF1aXJlbWVu dHMgaW4gdGhlIENQVSBhcmNoaXRlY3R1cmUuCj4KPiAJQXJuZAo+IF9fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCj4gaW9tbXUgbWFpbGluZyBsaXN0Cj4gaW9t bXVAbGlzdHMubGludXgtZm91bmRhdGlvbi5vcmcKPiBodHRwczovL2xpc3RzLmxpbnV4Zm91bmRh dGlvbi5vcmcvbWFpbG1hbi9saXN0aW5mby9pb21tdQo+CgpfX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZl bEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFp bG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 From: robin.murphy@arm.com (Robin Murphy) Date: Tue, 15 Mar 2016 12:33:03 +0000 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: <56E800FF.6050705@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Marek, Arnd, On 19/02/16 10: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. This was certainly the original intent of the arm64 code. The division of responsibility there is a conscious decision - IOMMU-API-wrangling goes in the common code, cache maintenance and actual dma_map_ops stay hidden in architecture-private code, safe from abuse. It's very much modelled on SWIOTLB. Given all the work Russell did last year getting rid of direct uses of the dmac_* cache maintenance functions by ARM drivers, I don't think bringing all of that back is a good way to go - Personally I'd much rather see several dozen lines of very similar looking (other than highmem and outer cache stuff) arch-private code if it maintains a robust and clearly-defined abstraction (and avoids yet another level of indirection). It does also seem a little odd to factor out only half the file on the grounds of architectural similarity, when that argument applies equally to the other (non-IOMMU) half too. I think the recent tree-wide conversion to generic dma_map_ops was in part motivated by the thought of common implementations, so I'm sure that's something we can revisit in due course. Robin. > > 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 > _______________________________________________ > iommu mailing list > iommu at lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934698AbcCOMeA (ORCPT ); Tue, 15 Mar 2016 08:34:00 -0400 Received: from foss.arm.com ([217.140.101.70]:36244 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752925AbcCOMdw (ORCPT ); Tue, 15 Mar 2016 08:33:52 -0400 Subject: Re: [RFC 3/3] iommu: dma-iommu: use common implementation also on ARM architecture To: Arnd Bergmann , Marek Szyprowski 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: Inki Dae , Krzysztof Kozlowski , Russell King - ARM Linux , Heiko Stuebner , Bartlomiej Zolnierkiewicz , Catalin Marinas , Will Deacon , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, iommu@lists.linux-foundation.org, Sakari Ailus , Laurent Pinchart , linux-arm-kernel@lists.infradead.org, Mark Yao From: Robin Murphy Message-ID: <56E800FF.6050705@arm.com> Date: Tue, 15 Mar 2016 12:33:03 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <2724572.qtPjgumFDJ@wuerfel> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Marek, Arnd, On 19/02/16 10: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. This was certainly the original intent of the arm64 code. The division of responsibility there is a conscious decision - IOMMU-API-wrangling goes in the common code, cache maintenance and actual dma_map_ops stay hidden in architecture-private code, safe from abuse. It's very much modelled on SWIOTLB. Given all the work Russell did last year getting rid of direct uses of the dmac_* cache maintenance functions by ARM drivers, I don't think bringing all of that back is a good way to go - Personally I'd much rather see several dozen lines of very similar looking (other than highmem and outer cache stuff) arch-private code if it maintains a robust and clearly-defined abstraction (and avoids yet another level of indirection). It does also seem a little odd to factor out only half the file on the grounds of architectural similarity, when that argument applies equally to the other (non-IOMMU) half too. I think the recent tree-wide conversion to generic dma_map_ops was in part motivated by the thought of common implementations, so I'm sure that's something we can revisit in due course. Robin. > > 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 > _______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu >