From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joerg Roedel Subject: Re: [RFC PATCH] iommu/vt-d: Add IOTLB flush support for kernel addresses Date: Fri, 23 Oct 2015 13:03:59 +0200 Message-ID: <20151023110359.GA27420@8bytes.org> References: <1445356379.4486.56.camel@infradead.org> <20151020160328.GV27420@8bytes.org> <1445357824.4486.65.camel@infradead.org> <20151023102043.GZ27420@8bytes.org> <1445596413.4113.175.camel@infradead.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <1445596413.4113.175.camel-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> 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: David Woodhouse Cc: "linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org" , iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, Sudeep Dutt List-Id: iommu@lists.linux-foundation.org T24gRnJpLCBPY3QgMjMsIDIwMTUgYXQgMTE6MzM6MzNBTSArMDEwMCwgRGF2aWQgV29vZGhvdXNl IHdyb3RlOgo+IFdoaWNoIG1lYW5zIEknbSBwb25kZXJpbmcgKnJlbmFtaW5nKiB0bGJfZmx1c2hf a2VybmVsX3JhbmdlKCkgdG8KPiBzb21ldGhpbmcgbGlrZSBhcmNoX3RsYl9mbHVzaF9rZXJuZWxf cmFuZ2UoKSBldmVyeXdoZXJlLCB0aGVuIGhhdmluZyBhCj4gdGxiX2ZsdXNoX2tlcm5lbF9yYW5n ZSgpIGlubGluZSBmdW5jdGlvbiB3aGljaCBvcHRpb25hbGx5IGNhbGxzCj4gaW9tbXVfZmx1c2hf a2VybmVsX3JhbmdlKCkgZmlyc3QuCgpUaGF0IHNvdW5kcyBsaWtlIHNvbWUgd29yaywgYnV0IHdv dWxkIGJlIHRoZSBzdXBlciBjbGVhbiBzb2x1dGlvbiA6KQpHaXZlbiB0aGF0IG9ubHkgYSBoYW5k ZnVsIG9mIGFyY2hpdGVjdHVyZSBiZXNpZGVzIHg4NiB3aWxsIG5lZWQgaXQKKHRoaW5raW5nIG9m IEFSTTY0IGFuZCBQUEMpLCBJIHByZWZlciB0aGUgc29sdXRpb24gYmVsb3c6Cgo+IE9yIEkgY291 bGQgcmVkdWNlIHRoZSBjaHVybiBieSBhZGRpbmcgZXhwbGljaXQgY2FsbHMgdG8KPiBpb21tdV9m bHVzaF9rZXJuZWxfcmFuZ2UoKSBhdCBldmVyeSBsb2NhdGlvbiB0aGF0IGNhbGxzCj4gdGxiX2Zs dXNoX2tlcm5lbF9yYW5nZSgpLCBidXQgdGhhdCdzIGdvaW5nIHRvIGxlYWQgdG8gc29tZSBjYWxs ZXJzCj4gbWlzc2luZyB0aGUgSU9NTVUgZmx1c2guCgpFeGFjdGx5IGxpa2UgdGhpcywgYnV0IHdo ZW4gZG8gd2UgbWlzcyBhIGZsdXNoIGhlcmU/Cgo+IE5vdCBlbnRpcmVseS4gVGhlIGRldmljZSBz dGlsbCBnZXRzIHRvIHNwZWNpZnkgd2hldGhlciBpdCdzIGRvaW5nCj4gc3VwZXJ2aXNvciBvciB1 c2VyIG1vZGUgYWNjZXNzLCBmb3IgZWFjaCByZXF1ZXN0IGl0IG1ha2VzLiBJdCBkb2Vzbid0Cj4g b3BlbiB0aGUgZG9vciB0byB1c2VycyBqdXN0IHVzaW5nIGtlcm5lbCBhZGRyZXNzZXMgYW5kIGdl dHRpbmcgYXdheQo+IHdpdGggaXQhCj4gCj4gU3VyZSwgd2UgbmVlZCB0byB0cnVzdCB0aGUgKmRl dmljZSog4oCUIGJ1dCB3ZSBuZWVkIHRvIHRydXN0IGl0IHRvCj4gcHJvdmlkZSB0aGUgY29ycmVj dCBQQVNJRCB0b28uIFdoaWNoIGJhc2ljYWxseSBtZWFucyBpbiB0aGUgVkZJTyBjYXNlCj4gd2hl cmUgdGhlIHVzZXIgZ2V0cyAqZnVsbCogY29udHJvbCBvZiB0aGUgZGV2aWNlLCB3ZSBoYXZlIHRv IGVuc3VyZQo+IHRoYXQgaXQgZ2V0cyBpdHMgb3duIFBBU0lEIHRhYmxlIHdpdGggb25seSB0aGUg Km9uZSogUEFTSUQgaW4gaXQsIGFuZAo+ICp0aGF0KiBQQVNJRCBjYW4ndCBoYXZlIHN1cGVydmlz b3IgbW9kZS4KCkV4YWN0bHksIHdlIG5lZWQgdG8gdHJ1c3QgdGhlIGRldmljZSBhbmQgdGhlIGRl dmljZSBkcml2ZXIuIEJ1dCB0aGF0cwpub3QgZGlmZmVyZW50IHRvIGEgc2l0dWF0aW9uIHdpdGhv dXQgYW4gaW9tbXUuIFdlIGp1c3QgcnVuIGludG8gcHJvYmxlbXMKd2hlbiBhIGRldmljZS1kcml2 ZXIgYWxsb3dzIHNlbmRpbmcgcmVxdWVzdHMgdG8gYWNjZXNzIGtlcm5lbC1tZW1vcnkKZnJvbSB1 c2VyLXNwYWNlLCBzbyBpdCBuZWVkcyBtb3JlIGNhcmUgZnJvbSB0aGUgZHJpdmVyIHdyaXRlcnMv cmV2aWV3ZXJzCnRvby4KCj4gK3N0YXRpYyBpbmxpbmUgdm9pZCBkb19pb21tdV9mbHVzaF9rdGxi KHVuc2lnbmVkIGxvbmcgc3RhcnQsIHVuc2lnbmVkIGxvbmcgZW5kKQo+ICt7Cj4gKwlpb21tdV9m bHVzaF9rdGxiX2ZuICpmbjsKPiArCXJjdV9yZWFkX2xvY2soKTsKPiArCWZuID0gcmN1X2RlcmVm ZXJlbmNlKGlvbW11X2ZsdXNoX2t0bGIpOwo+ICsJaWYgKGZuKQo+ICsJCSgqZm4pKHN0YXJ0LCBl bmQpOwo+ICsJcmN1X3JlYWRfdW5sb2NrKCk7Cj4gK30KClllcywgdGhhdCdsbCB3b3JrIHRvby4g V2hlbiB5b3UgcmVhZC91cGRhdGUgdGhlIGlvbW11X2ZsdXNoX2t0bGIgcG9pbnRlcgphdG9taWNh bGx5LCB5b3UgY2FuIGV2ZW4gZ2V0IGF3YXkgd2l0aG91dCByY3UuIFRoZSBmdW5jdGlvbiBpdCBw b2ludHMgdG8Kd2lsbCBub3QgZ28gYXdheSwgc28geW91IGNhbiBzdGlsbCBjYWxsIGl0IGV2ZW4g d2hlbiB0aGUgcG9pbnRlciB0dXJuZWQKTlVMTC4KCj4gTWF5YmUgd2UgY291bGQga2VlcCBpdCBz aW1wbGUgYW5kIGp1c3QgZGVjbGFyZSB0aGF0IG9uY2UgdGhlIGZ1bmN0aW9uCj4gcG9pbnRlciBp cyBzZXQsIGl0IG1heSBuZXZlciBiZSBjbGVhcmVkPyBCdXQgSSB0aGluayB3ZSByZWFsbHkgZG8g d2FudAo+IHRvIGF2b2lkIHRoZSBvdXQtb2YtbGluZSBmdW5jdGlvbiBjYWxsIGFsdG9nZXRoZXIg aW4gdGhlIGNhc2Ugd2hlcmUKPiBrZXJuZWwgUEFTSURzIGFyZSBub3QgYmVpbmcgdXNlZC4gT3Ig YXQgKmxlYXN0KiB0aGUgY2FzZSB3aGVyZSBTVk0KPiBpc24ndCBiZWluZyB1c2VkIGF0IGFsbC4K ClllcywgdGhhdHMgd2h5IEkgdGhpbmsgYW4gaW5saW5lIGZ1bmN0aW9uIHdoaWNoIGRvZXMgdGhl IGNoZWNrcyB3b3VsZCBiZQphIGJldHRlciBzb2x1dGlvbi4gVGhlIG1tdV9ub3RpZmllcnMgaW1w bGVtZW50IGl0IGluIHRoZSBzYW1lIHdheSwgc28gd2UKd291bGQgc3RheSBjb25zaXN0ZW50IHdp dGggdGhlbS4KCgoJSm9lcmcKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fCmlvbW11IG1haWxpbmcgbGlzdAppb21tdUBsaXN0cy5saW51eC1mb3VuZGF0aW9u Lm9yZwpodHRwczovL2xpc3RzLmxpbnV4Zm91bmRhdGlvbi5vcmcvbWFpbG1hbi9saXN0aW5mby9p b21tdQ== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f178.google.com (mail-wi0-f178.google.com [209.85.212.178]) by kanga.kvack.org (Postfix) with ESMTP id 79CA96B0038 for ; Fri, 23 Oct 2015 07:04:01 -0400 (EDT) Received: by wijp11 with SMTP id p11so72023143wij.0 for ; Fri, 23 Oct 2015 04:04:01 -0700 (PDT) Received: from theia.8bytes.org (8bytes.org. [81.169.241.247]) by mx.google.com with ESMTPS id bz4si24235784wjb.25.2015.10.23.04.04.00 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 23 Oct 2015 04:04:00 -0700 (PDT) Date: Fri, 23 Oct 2015 13:03:59 +0200 From: Joerg Roedel Subject: Re: [RFC PATCH] iommu/vt-d: Add IOTLB flush support for kernel addresses Message-ID: <20151023110359.GA27420@8bytes.org> References: <1445356379.4486.56.camel@infradead.org> <20151020160328.GV27420@8bytes.org> <1445357824.4486.65.camel@infradead.org> <20151023102043.GZ27420@8bytes.org> <1445596413.4113.175.camel@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1445596413.4113.175.camel@infradead.org> Sender: owner-linux-mm@kvack.org List-ID: To: David Woodhouse Cc: "linux-mm@kvack.org" , iommu@lists.linux-foundation.org, Sudeep Dutt On Fri, Oct 23, 2015 at 11:33:33AM +0100, David Woodhouse wrote: > Which means I'm pondering *renaming* tlb_flush_kernel_range() to > something like arch_tlb_flush_kernel_range() everywhere, then having a > tlb_flush_kernel_range() inline function which optionally calls > iommu_flush_kernel_range() first. That sounds like some work, but would be the super clean solution :) Given that only a handful of architecture besides x86 will need it (thinking of ARM64 and PPC), I prefer the solution below: > Or I could reduce the churn by adding explicit calls to > iommu_flush_kernel_range() at every location that calls > tlb_flush_kernel_range(), but that's going to lead to some callers > missing the IOMMU flush. Exactly like this, but when do we miss a flush here? > Not entirely. The device still gets to specify whether it's doing > supervisor or user mode access, for each request it makes. It doesn't > open the door to users just using kernel addresses and getting away > with it! > > Sure, we need to trust the *device* a?? but we need to trust it to > provide the correct PASID too. Which basically means in the VFIO case > where the user gets *full* control of the device, we have to ensure > that it gets its own PASID table with only the *one* PASID in it, and > *that* PASID can't have supervisor mode. Exactly, we need to trust the device and the device driver. But thats not different to a situation without an iommu. We just run into problems when a device-driver allows sending requests to access kernel-memory from user-space, so it needs more care from the driver writers/reviewers too. > +static inline void do_iommu_flush_ktlb(unsigned long start, unsigned long end) > +{ > + iommu_flush_ktlb_fn *fn; > + rcu_read_lock(); > + fn = rcu_dereference(iommu_flush_ktlb); > + if (fn) > + (*fn)(start, end); > + rcu_read_unlock(); > +} Yes, that'll work too. When you read/update the iommu_flush_ktlb pointer atomically, you can even get away without rcu. The function it points to will not go away, so you can still call it even when the pointer turned NULL. > Maybe we could keep it simple and just declare that once the function > pointer is set, it may never be cleared? But I think we really do want > to avoid the out-of-line function call altogether in the case where > kernel PASIDs are not being used. Or at *least* the case where SVM > isn't being used at all. Yes, thats why I think an inline function which does the checks would be a better solution. The mmu_notifiers implement it in the same way, so we would stay consistent with them. Joerg -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org