From mboxrd@z Thu Jan 1 00:00:00 1970 From: Robin Murphy Subject: Re: [PATCH v2 2/7] dma-mapping: Generalise dma_32bit_limit flag Date: Fri, 27 Jul 2018 21:11:17 +0100 Message-ID: <5354ef69-c54e-170b-62d4-5110dc60aa8f@arm.com> References: <7872d914-8ea7-06e4-4a0c-489023e098d6@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <7872d914-8ea7-06e4-4a0c-489023e098d6-l0cyMroinI0@public.gmane.org> Content-Language: en-GB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Grygorii Strashko , hch-jcswGhMUV9g@public.gmane.org, m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org, x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, sudeep.holla-5wv7dgnIgG8@public.gmane.org, frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org List-Id: linux-acpi@vger.kernel.org T24gMjAxOC0wNy0yNyA2OjQ1IFBNLCBHcnlnb3JpaSBTdHJhc2hrbyB3cm90ZToKPiBPbiAwNy8y My8yMDE4IDA1OjE2IFBNLCBSb2JpbiBNdXJwaHkgd3JvdGU6Cj4+IFdoaWxzdCB0aGUgbm90aW9u IG9mIGFuIHVwc3RyZWFtIERNQSByZXN0cmljdGlvbiBpcyBtb3N0IGNvbW1vbmx5IHNlZW4KPj4g aW4gUENJIGhvc3QgYnJpZGdlcyBzYWRkbGVkIHdpdGggYSAzMi1iaXQgbmF0aXZlIGludGVyZmFj ZSwgYSBtb3JlCj4+IGdlbmVyYWwgdmVyc2lvbiBvZiB0aGUgc2FtZSBpc3N1ZSBjYW4gZXhpc3Qg b24gY29tcGxleCBTb0NzIHdoZXJlIGEgYnVzCj4+IG9yIHBvaW50LXRvLXBvaW50IGludGVyY29u bmVjdCBsaW5rIGZyb20gYSBkZXZpY2UncyBETUEgbWFzdGVyIGludGVyZmFjZQo+PiB0byBhbm90 aGVyIGNvbXBvbmVudCBhbG9uZyB0aGUgcGF0aCB0byBtZW1vcnkgKG9mdGVuIGFuIElPTU1VKSBt YXkgY2FycnkKPj4gZmV3ZXIgYWRkcmVzcyBiaXRzIHRoYW4gdGhlIGludGVyZmFjZXMgYXQgYm90 aCBlbmRzIG5vbWluYWxseSBzdXBwb3J0Lgo+PiBJbiBvcmRlciB0byBwcm9wZXJseSBkZWFsIHdp dGggdGhpcywgdGhlIGZpcnN0IHN0ZXAgaXMgdG8gZXhwYW5kIHRoZQo+PiBkbWFfMzJiaXRfbGlt aXQgZmxhZyBpbnRvIGFuIGFyYml0cmFyeSBtYXNrLgo+Pgo+PiBUbyBtaW5pbWlzZSB0aGUgaW1w YWN0IG9uIGV4aXN0aW5nIGNvZGUsIHdlJ2xsIG1ha2Ugc3VyZSB0byBvbmx5Cj4+IGNvbnNpZGVy IHRoaXMgbmV3IG1hc2sgdmFsaWQgaWYgc2V0LiBUaGF0IG1ha2VzIHNlbnNlIGFueXdheSwgc2lu Y2UgYQo+PiBtYXNrIG9mIHplcm8gd291bGQgcmVwcmVzZW50IERNQSBub3QgYmVpbmcgd2lyZWQg dXAgYXQgYWxsLCBhbmQgdGhhdAo+PiB3b3VsZCBiZSBiZXR0ZXIgaGFuZGxlZCBieSBub3QgcHJv dmlkaW5nIHZhbGlkIG9wcyBpbiB0aGUgZmlyc3QgcGxhY2UuCj4+Cj4+IFNpZ25lZC1vZmYtYnk6 IFJvYmluIE11cnBoeSA8cm9iaW4ubXVycGh5QGFybS5jb20+Cj4gCj4gSSdkIGxpa2UgdG8gbm90 ZSBhYm91dCBzb21lIHBvc3NpYmxlIGlzc3VlIHJlbGF0ZWQgdG8gdGhpcyBjaGFuZ2UuCj4gCj4g VGhlcmUgYXJlIHNvbWUgcGxhY2VzIGluIGtlcm5lbCB3aGVyZSBwYXJlbnQgRE1BIGNvbmZpZ3Vy YXRpb24gaXMgY29waWVkIAo+IHRvIHRoZSBtYW51YWxseSBjcmVhdGVkIGNoaWxkIGRldmljZXMs IGxpa2U6Cj4gbWZkLWNvcmUuYwo+IG1mZF9hZGRfZGV2aWNlKCkKPiAgwqDCoMKgwqBwZGV2LT5k ZXYucGFyZW50ID0gcGFyZW50Owo+ICDCoMKgwqDCoHBkZXYtPmRldi50eXBlID0gJm1mZF9kZXZf dHlwZTsKPiAgwqDCoMKgwqBwZGV2LT5kZXYuZG1hX21hc2sgPSBwYXJlbnQtPmRtYV9tYXNrOwo+ ICDCoMKgwqDCoHBkZXYtPmRldi5kbWFfcGFybXMgPSBwYXJlbnQtPmRtYV9wYXJtczsKPiAgwqDC oMKgwqBwZGV2LT5kZXYuY29oZXJlbnRfZG1hX21hc2sgPSBwYXJlbnQtPmNvaGVyZW50X2RtYV9t YXNrOwo+IAo+IEFkZGluZyBvciBjaGFuZ2luZyBnZW5lcmljIERNQSBkZXZpY2UgcHJvcGVydGll cyBtaWdodCBhZmZlY3Qgb24gc3VjaAo+IHN1YnN5c3RlbXMvZHJpdmVycy4gSGF2ZSB5b3UgY29u c2lkZXJlZCBzdWNoIGNhc2VzPwoKWWVzLCB0aGF0J3MgYSBsb3ZlbHkgZXhhbXBsZSBvZiB3aGF0 IEkgY2xhc3MgYXMgImJ1cyBjb2RlIiBjcmVhdGluZyBhIApjaGlsZCBkZXZpY2UgYW5kIGluaXRp YWxpc2luZyBpdHMgRE1BIHBhcmFtZXRlcnMgYXBwcm9wcmlhdGVseS4gVGhlIApzdWJkZXZpY2Ug Z29lcyBvbiB0byBnZXQgYXNzb2NpYXRlZCB3aXRoIGFuIE9GIG5vZGUgb3IgQUNQSSBjb21wYW5p b24sIApzbyB3aGVuIHRoZSBzdWJkcml2ZXIgZm9yIHRoYXQgZnVuY3Rpb24gYmluZHMgaXQgc2hv dWxkIGdvIHRocm91Z2ggaXRzIApvd24gZG1hX2NvbmZpZ3VyZSgpIHByb2Nlc3MgYW5kIHBpY2sg dXAgYW55IGZ1cnRoZXIgcHJvcGVydGllcyBhY2NvcmRpbmdseS4KCkNvZGUgd2hpY2gganVzdCB0 cmllcyB0byBjb3B5IHRoZSBETUEgY29uZmlndXJhdGlvbiBmcm9tIGFuIGV4aXN0aW5nIApkZXZp Y2UgdG8gYSBuZXcgb25lIGhhcyBuZXZlciB3b3JrZWQgcHJvcGVybHksIGJlY2F1c2UgdGhlcmUg aXMgb2Z0ZW4gCmFkZGl0aW9uYWwgRE1BIGNvbmZpZ3VyYXRpb24gaW4gYXJjaGRhdGEgYW5kIG90 aGVyIHBsYWNlcyBpdCBjYW5ub3QgCnBvc3NpYmx5IGtub3cgYWJvdXQuIExhc3QgdGltZSBJIGxv b2tlZCB0aGVyZSB3ZXJlIHN0aWxsIHNvbWUgc3BlY2lmaWMgCmhhY2tzIGluIHRoZSBVU0IgbGF5 ZXIgaW4gb3JkZXIgdG8gaW50ZXJhY3QgY29ycmVjdGx5IHdpdGggdGhlIGJsb2NrIApsYXllciBi b3VuY2UgbGltaXQsIGJ1dCBJIHRoaW5rIGFueXRoaW5nIHRydWx5IHdyb25nIGhhcyBiZWVuIG1v cmUgb3IgCmxlc3MgZmx1c2hlZCBvdXQgYnkgbm93ICh0aGUgRE1BIG9wcyBjaGFuZ2VzIGZvciBh cm02NCBBQ1BJIHN1cHBvcnQgCmNhdWdodCBhIGZhaXIgZmV3IElJUkMpLgoKUm9iaW4uCl9fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmlvbW11IG1haWxpbmcg bGlzdAppb21tdUBsaXN0cy5saW51eC1mb3VuZGF0aW9uLm9yZwpodHRwczovL2xpc3RzLmxpbnV4 Zm91bmRhdGlvbi5vcmcvbWFpbG1hbi9saXN0aW5mby9pb21tdQ== From mboxrd@z Thu Jan 1 00:00:00 1970 From: robin.murphy@arm.com (Robin Murphy) Date: Fri, 27 Jul 2018 21:11:17 +0100 Subject: [PATCH v2 2/7] dma-mapping: Generalise dma_32bit_limit flag In-Reply-To: <7872d914-8ea7-06e4-4a0c-489023e098d6@ti.com> References: <7872d914-8ea7-06e4-4a0c-489023e098d6@ti.com> Message-ID: <5354ef69-c54e-170b-62d4-5110dc60aa8f@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 2018-07-27 6:45 PM, Grygorii Strashko wrote: > On 07/23/2018 05:16 PM, Robin Murphy wrote: >> Whilst the notion of an upstream DMA restriction is most commonly seen >> in PCI host bridges saddled with a 32-bit native interface, a more >> general version of the same issue can exist on complex SoCs where a bus >> or point-to-point interconnect link from a device's DMA master interface >> to another component along the path to memory (often an IOMMU) may carry >> fewer address bits than the interfaces at both ends nominally support. >> In order to properly deal with this, the first step is to expand the >> dma_32bit_limit flag into an arbitrary mask. >> >> To minimise the impact on existing code, we'll make sure to only >> consider this new mask valid if set. That makes sense anyway, since a >> mask of zero would represent DMA not being wired up at all, and that >> would be better handled by not providing valid ops in the first place. >> >> Signed-off-by: Robin Murphy > > I'd like to note about some possible issue related to this change. > > There are some places in kernel where parent DMA configuration is copied > to the manually created child devices, like: > mfd-core.c > mfd_add_device() > ????pdev->dev.parent = parent; > ????pdev->dev.type = &mfd_dev_type; > ????pdev->dev.dma_mask = parent->dma_mask; > ????pdev->dev.dma_parms = parent->dma_parms; > ????pdev->dev.coherent_dma_mask = parent->coherent_dma_mask; > > Adding or changing generic DMA device properties might affect on such > subsystems/drivers. Have you considered such cases? Yes, that's a lovely example of what I class as "bus code" creating a child device and initialising its DMA parameters appropriately. The subdevice goes on to get associated with an OF node or ACPI companion, so when the subdriver for that function binds it should go through its own dma_configure() process and pick up any further properties accordingly. Code which just tries to copy the DMA configuration from an existing device to a new one has never worked properly, because there is often additional DMA configuration in archdata and other places it cannot possibly know about. Last time I looked there were still some specific hacks in the USB layer in order to interact correctly with the block layer bounce limit, but I think anything truly wrong has been more or less flushed out by now (the DMA ops changes for arm64 ACPI support caught a fair few IIRC). Robin.