From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Xu Subject: Re: [Qemu-devel] [RFC PATCH 12/20] Memory: Add func to fire pasidt_bind notifier Date: Thu, 27 Apr 2017 14:14:27 +0800 Message-ID: <20170427061427.GA1542@pxdev.xzpeter.org> References: <1493201210-14357-1-git-send-email-yi.l.liu@linux.intel.com> <1493201210-14357-13-git-send-email-yi.l.liu@linux.intel.com> <0f2966cf-4e5a-a2cc-5eb3-7e7d4f62bb85@redhat.com> <20170427023719.GA14925@sky-dev> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Cc: tianyu.lan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org, kevin.tian-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org, kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, jasowang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, qemu-devel-qX2TKyscuCcdnm+yROfE0A@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, jacob.jun.pan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org, Paolo Bonzini To: "Liu, Yi L" Return-path: Content-Disposition: inline In-Reply-To: <20170427023719.GA14925@sky-dev> 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 List-Id: kvm.vger.kernel.org T24gVGh1LCBBcHIgMjcsIDIwMTcgYXQgMTA6Mzc6MTlBTSArMDgwMCwgTGl1LCBZaSBMIHdyb3Rl Ogo+IE9uIFdlZCwgQXByIDI2LCAyMDE3IGF0IDAzOjUwOjE2UE0gKzAyMDAsIFBhb2xvIEJvbnpp bmkgd3JvdGU6Cj4gPiAKPiA+IAo+ID4gT24gMjYvMDQvMjAxNyAxMjowNiwgTGl1LCBZaSBMIHdy b3RlOgo+ID4gPiArdm9pZCBtZW1vcnlfcmVnaW9uX25vdGlmeV9pb21tdV9zdm1fYmluZChNZW1v cnlSZWdpb24gKm1yLAo+ID4gPiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICB2b2lkICpkYXRhKQo+ID4gPiArewo+ID4gPiArICAgIElPTU1VTm90aWZpZXIgKmlvbW11 X25vdGlmaWVyOwo+ID4gPiArICAgIElPTU1VTm90aWZpZXJGbGFnIHJlcXVlc3RfZmxhZ3M7Cj4g PiA+ICsKPiA+ID4gKyAgICBhc3NlcnQobWVtb3J5X3JlZ2lvbl9pc19pb21tdShtcikpOwo+ID4g PiArCj4gPiA+ICsgICAgLypUT0RPOiBzdXBwb3J0IG90aGVyIGJpbmQgcmVxdWVzdHMgd2l0aCBz bWFsbGVyIGdyYW4sCj4gPiA+ICsgICAgICogZS5nLiBiaW5kIHNpZ25sZSBwYXNpZCBlbnRyeQo+ ID4gPiArICAgICAqLwo+ID4gPiArICAgIHJlcXVlc3RfZmxhZ3MgPSBJT01NVV9OT1RJRklFUl9T Vk1fUEFTSURUX0JJTkQ7Cj4gPiA+ICsKPiA+ID4gKyAgICBRTElTVF9GT1JFQUNIKGlvbW11X25v dGlmaWVyLCAmbXItPmlvbW11X25vdGlmeSwgbm9kZSkgewo+ID4gPiArICAgICAgICBpZiAoaW9t bXVfbm90aWZpZXItPm5vdGlmaWVyX2ZsYWdzICYgcmVxdWVzdF9mbGFncykgewo+ID4gPiArICAg ICAgICAgICAgaW9tbXVfbm90aWZpZXItPm5vdGlmeShpb21tdV9ub3RpZmllciwgZGF0YSk7Cj4g PiA+ICsgICAgICAgICAgICBicmVhazsKPiA+ID4gKyAgICAgICAgfQo+ID4gPiArICAgIH0KPiA+ IAo+ID4gUGV0ZXIsCj4gPiAKPiA+IHNob3VsZCB0aGlzIHJldXNlIC0+bm90aWZ5LCBvciBzaG91 bGQgaXQgYmUgZGlmZmVyZW50IGZ1bmN0aW9uIHBvaW50ZXIKPiA+IGluIElPTU1VTm90aWZpZXI/ Cj4gCj4gSGkgUGFvbG8sCj4gCj4gVGh4IGZvciB5b3VyIHJldmlldy4KPiAKPiBJIHRoaW5rIGl0 IHNob3VsZCBiZSDigJwtPm5vdGlmeeKAnSBoZXJlLiBJbiB0aGlzIHBhdGNoc2V0LCB0aGUgbmV3 IG5vdGlmaWVyCj4gaXMgcmVnaXN0ZXJlZCB3aXRoIHRoZSBleGlzdGluZyBub3RpZmllciByZWdp c3RyYXRpb24gQVBJLiBTbyB0aGUgYWxsIHRoZQo+IG5vdGlmaWVycyBhcmUgaW4gdGhlIG1yLT5p b21tdV9ub3RpZnkgbGlzdC4gQW5kIG5vdGlmaWVycyBhcmUgbGFiZWxlZAo+IGJ5IG5vdGlmeSBm bGFnLCBzbyBpdCBpcyBhYmxlIHRvIGRpZmZlcmVudGlhdGUgdGhlIElPTU1VTm90aWZpZXIgbm9k ZXMuCj4gV2hlbiB0aGUgZmxhZyBtZWV0cywgdHJpZ2dlciBpdCBieSDigJwtPm5vdGlmeeKAnS4g VGhlIGRpYWdyYW0gYmVsb3cgc2hvd3MKPiBteSB1bmRlcnN0YW5kaW5nICwgd2lzaCBpdCBoZWxw cyB0byBtYWtlIG1lIHVuZGVyc3Rvb2QuCj4gCj4gVkZJT0NvbnRhaW5lcgo+ICAgICAgICB8Cj4g ICAgICAgIGdpb21tdV9saXN0KFZGSU9HdWVzdElPTU1VKQo+ICAgICAgICAgICAgICAgICBcCj4g ICAgICAgICAgICAgICAgICBWRklPR3Vlc3RJT01NVTEgLT4gICBWRklPR3Vlc3RJT01NVTIgLT4g VkZJT0d1ZXN0SU9NTVUzIC4uLgo+ICAgICAgICAgICAgICAgICAgICAgfCAgICAgICAgICAgICAg ICAgICAgIHwgICAgICAgICAgICAgICAgIHwKPiBtci0+aW9tbXVfbm90aWZ5OiBJT01NVU5vdGlm aWVyICAgLT4gICAgSU9NTVVOb3RpZmllciAgLT4gIElPTU1VTm90aWZpZXIKPiAgICAgICAgICAg ICAgICAgICAoRmxhZzpNQVAvVU5NQVApICAgICAoRmxhZzpTVk0gYmluZCkgIChGbGFnOnRsYiBp bnZhbGlkYXRlKQo+IAo+IAo+IEFjdHVhbGx5LCBjb21wYXJlZCB3aXRoIHRoZSBNQVAvVU5NQVAg bm90aWZpZXIsIHRoZSBuZXdseSBhZGRlZCBub3RpZmllciBoYXMKPiBubyBzdGFydC9lbmQgY2hl Y2ssIGFuZCB0aGVyZSBtYXkgYmUgb3RoZXIgdHlwZXMgb2YgYmluZCBub3RmaWVyIGZsYWcgaW4K PiBmdXR1cmUsIHNvIEkgYWRkZWQgYSBzZXBhcmF0ZSBmaXJlIGZ1bmMgZm9yIFNWTSBiaW5kIG5v dGlmaWVyLgoKSSBhZ3JlZSB3aXRoIFBhb2xvIHRoYXQgdGhpcyBpbnRlcmZhY2UgbWlnaHQgbm90 IGJlIHRoZSBzdWl0YWJsZSBwbGFjZQpmb3IgdGhlIFNWTSBub3RpZmllcnMgKGp1c3QgbGlrZSB3 aGF0IEkgd29ycmllZCBhYm91dCBpbiBwcmV2aW91cwpkaXNjdXNzaW9ucykuCgpUaGUgYmlnZ2Vz dCBwcm9ibGVtIGlzIHRoYXQsIGlmIHlvdSBzZWUgY3VycmVudCBub3RpZmllciBtZWNoYW5pc20s Cml0J3MgcGVyLW1lbW9yeS1yZWdpb24uIEhvd2V2ZXIgaWl1YyB5b3VyIG1lc3NhZ2VzIHNob3Vs ZCBiZQpwZXItaW9tbXUsIG9yIHNheSwgcGVyIHRyYW5zbGF0aW9uIHVuaXQuIFdoaWxlLCBmb3Ig ZWFjaCBpb21tdSwgdGhlcmUKY2FuIGJlIG1vcmUgdGhhbiBvbmUgbWVtb3J5IHJlZ2lvbnMgKHBw YyBjYW4gYmUgYW4gZXhhbXBsZSkuIFdoZW4KdGhlcmUgYXJlIG1vcmUgdGhhbiBvbmUgTVJzIGJp bmRlZCB0byB0aGUgc2FtZSBpb21tdSB1bml0LCB3aGljaAptZW1vcnkgcmVnaW9uIHNob3VsZCB5 b3UgcmVnaXN0ZXIgdG8/IEFueSBvbmUgb2YgdGhlbSwgb3IgYWxsPwoKU28gbXkgY29uY2x1c2lv biBpcywgaXQganVzdCBoYXMgbm90aGluZyB0byBkbyB3aXRoIG1lbW9yeSByZWdpb25zLi4uCgpJ bnN0ZWFkIG9mIGEgZGlmZmVyZW50IGZ1bmN0aW9uIHBvaW50ZXIgaW4gSU9NTVVOb3RpZmVyLCBJ TUhPIHdlIGNhbgpldmVuIG1vdmUgYSBzdGVwIGZ1cnRoZXIsIHRvIGlzb2xhdGUgSU9UTEIgbm90 aWZpY2F0aW9ucyAodGFyZ2V0ZWQgYXQKbWVtb3J5IHJlZ2lvbnMgYW5kIHdpdGggc3RhcnQvZW5k IHJhbmdlcykgb3V0IG9mIFNWTS9vdGhlcgpub3RpZmljYXRpb25zLCBzaW5jZSB0aGV5IGFyZSBk aWZmZXJlbnQgaW4gZ2VuZXJhbC4gU28gd2UgYmFzaWNhbGx5Cm5lZWQgdHdvIG5vdGlmaWNhdGlv biBtZWNoYW5pc206CgotIG9uZSBmb3IgbWVtb3J5IHJlZ2lvbnMsIGN1cnJlbnRseSB3aGF0IEkg Y2FuIHNlZSBpcyBJT1RMQgogIG5vdGlmaWNhdGlvbnMKCi0gb25lIGZvciB0cmFuc2xhdGlvbiB1 bml0cywgY3VycmVudGx5IEkgc2VlIGFsbCB0aGUgcmVzdCBvZgogIG5vdGlmaWNhdGlvbnMgbmVl ZGVkIGluIHZpcnQtc3ZtIGluIHRoaXMgY2F0ZWdvcnkKCk1heWJlIHNvbWUgUkZDIHBhdGNoZXMg d291bGQgYmUgZ29vZCB0byBzaG93IHdoYXQgSSBtZWFuLi4uIEknbGwgc2VlCndoZXRoZXIgSSBj YW4gcHJlcGFyZSBzb21lLgoKVGhhbmtzLAoKLS0gClBldGVyIFh1Cl9fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmlvbW11IG1haWxpbmcgbGlzdAppb21tdUBs aXN0cy5saW51eC1mb3VuZGF0aW9uLm9yZwpodHRwczovL2xpc3RzLmxpbnV4Zm91bmRhdGlvbi5v cmcvbWFpbG1hbi9saXN0aW5mby9pb21tdQ== From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36619) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d3chQ-00025p-9s for qemu-devel@nongnu.org; Thu, 27 Apr 2017 02:14:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d3chN-0000i5-5k for qemu-devel@nongnu.org; Thu, 27 Apr 2017 02:14:44 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42884) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d3chM-0000fT-Sk for qemu-devel@nongnu.org; Thu, 27 Apr 2017 02:14:41 -0400 Date: Thu, 27 Apr 2017 14:14:27 +0800 From: Peter Xu Message-ID: <20170427061427.GA1542@pxdev.xzpeter.org> References: <1493201210-14357-1-git-send-email-yi.l.liu@linux.intel.com> <1493201210-14357-13-git-send-email-yi.l.liu@linux.intel.com> <0f2966cf-4e5a-a2cc-5eb3-7e7d4f62bb85@redhat.com> <20170427023719.GA14925@sky-dev> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170427023719.GA14925@sky-dev> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC PATCH 12/20] Memory: Add func to fire pasidt_bind notifier List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Liu, Yi L" Cc: Paolo Bonzini , qemu-devel@nongnu.org, alex.williamson@redhat.com, tianyu.lan@intel.com, kevin.tian@intel.com, yi.l.liu@intel.com, ashok.raj@intel.com, kvm@vger.kernel.org, jean-philippe.brucker@arm.com, jasowang@redhat.com, iommu@lists.linux-foundation.org, jacob.jun.pan@intel.com On Thu, Apr 27, 2017 at 10:37:19AM +0800, Liu, Yi L wrote: > On Wed, Apr 26, 2017 at 03:50:16PM +0200, Paolo Bonzini wrote: > >=20 > >=20 > > On 26/04/2017 12:06, Liu, Yi L wrote: > > > +void memory_region_notify_iommu_svm_bind(MemoryRegion *mr, > > > + void *data) > > > +{ > > > + IOMMUNotifier *iommu_notifier; > > > + IOMMUNotifierFlag request_flags; > > > + > > > + assert(memory_region_is_iommu(mr)); > > > + > > > + /*TODO: support other bind requests with smaller gran, > > > + * e.g. bind signle pasid entry > > > + */ > > > + request_flags =3D IOMMU_NOTIFIER_SVM_PASIDT_BIND; > > > + > > > + QLIST_FOREACH(iommu_notifier, &mr->iommu_notify, node) { > > > + if (iommu_notifier->notifier_flags & request_flags) { > > > + iommu_notifier->notify(iommu_notifier, data); > > > + break; > > > + } > > > + } > >=20 > > Peter, > >=20 > > should this reuse ->notify, or should it be different function pointe= r > > in IOMMUNotifier? >=20 > Hi Paolo, >=20 > Thx for your review. >=20 > I think it should be =E2=80=9C->notify=E2=80=9D here. In this patchset,= the new notifier > is registered with the existing notifier registration API. So the all t= he > notifiers are in the mr->iommu_notify list. And notifiers are labeled > by notify flag, so it is able to differentiate the IOMMUNotifier nodes. > When the flag meets, trigger it by =E2=80=9C->notify=E2=80=9D. The diag= ram below shows > my understanding , wish it helps to make me understood. >=20 > VFIOContainer > | > giommu_list(VFIOGuestIOMMU) > \ > VFIOGuestIOMMU1 -> VFIOGuestIOMMU2 -> VFIOGuestIOMMU= 3 ... > | | | > mr->iommu_notify: IOMMUNotifier -> IOMMUNotifier -> IOMMUNotifie= r > (Flag:MAP/UNMAP) (Flag:SVM bind) (Flag:tlb inval= idate) >=20 >=20 > Actually, compared with the MAP/UNMAP notifier, the newly added notifie= r has > no start/end check, and there may be other types of bind notfier flag i= n > future, so I added a separate fire func for SVM bind notifier. I agree with Paolo that this interface might not be the suitable place for the SVM notifiers (just like what I worried about in previous discussions). The biggest problem is that, if you see current notifier mechanism, it's per-memory-region. However iiuc your messages should be per-iommu, or say, per translation unit. While, for each iommu, there can be more than one memory regions (ppc can be an example). When there are more than one MRs binded to the same iommu unit, which memory region should you register to? Any one of them, or all? So my conclusion is, it just has nothing to do with memory regions... Instead of a different function pointer in IOMMUNotifer, IMHO we can even move a step further, to isolate IOTLB notifications (targeted at memory regions and with start/end ranges) out of SVM/other notifications, since they are different in general. So we basically need two notification mechanism: - one for memory regions, currently what I can see is IOTLB notifications - one for translation units, currently I see all the rest of notifications needed in virt-svm in this category Maybe some RFC patches would be good to show what I mean... I'll see whether I can prepare some. Thanks, --=20 Peter Xu