[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 12/20] Memory: Add func to fire pasidt_bind
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [RFC PATCH 12/20] Memory: Add func to fire pasidt_bind notifier |
Date: |
Thu, 27 Apr 2017 18:09:29 +0800 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Thu, Apr 27, 2017 at 02:14:27PM +0800, Peter Xu wrote:
> 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:
> > >
> > >
> > > 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 = 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;
> > > > + }
> > > > + }
> > >
> > > Peter,
> > >
> > > should this reuse ->notify, or should it be different function pointer
> > > in IOMMUNotifier?
> >
> > Hi Paolo,
> >
> > Thx for your review.
> >
> > I think it should be “->notify” here. In this patchset, the new notifier
> > is registered with the existing notifier registration API. So the all the
> > 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 “->notify”. The diagram below shows
> > my understanding , wish it helps to make me understood.
> >
> > VFIOContainer
> > |
> > giommu_list(VFIOGuestIOMMU)
> > \
> > VFIOGuestIOMMU1 -> VFIOGuestIOMMU2 -> VFIOGuestIOMMU3 ...
> > | | |
> > mr->iommu_notify: IOMMUNotifier -> IOMMUNotifier -> IOMMUNotifier
> > (Flag:MAP/UNMAP) (Flag:SVM bind) (Flag:tlb
> > invalidate)
> >
> >
> > Actually, compared with the MAP/UNMAP notifier, the newly added notifier has
> > no start/end check, and there may be other types of bind notfier flag in
> > 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.
Here it is (on qemu-devel):
[RFC PATCH 0/8] IOMMU: introduce common IOMMUObject
Thanks,
--
Peter Xu
- [Qemu-devel] [RFC PATCH 07/20] VFIO: check notifier flag in region_del(), (continued)
- [Qemu-devel] [RFC PATCH 07/20] VFIO: check notifier flag in region_del(), Liu, Yi L, 2017/04/26
- [Qemu-devel] [RFC PATCH 08/20] Memory: add notifier flag check in memory_replay(), Liu, Yi L, 2017/04/26
- [Qemu-devel] [RFC PATCH 09/20] Memory: introduce iommu_ops->record_device, Liu, Yi L, 2017/04/26
- [Qemu-devel] [RFC PATCH 10/20] VFIO: notify vIOMMU emulator when device is assigned, Liu, Yi L, 2017/04/26
- [Qemu-devel] [RFC PATCH 11/20] intel_iommu: provide iommu_ops->record_device, Liu, Yi L, 2017/04/26
- [Qemu-devel] [RFC PATCH 12/20] Memory: Add func to fire pasidt_bind notifier, Liu, Yi L, 2017/04/26
- Re: [Qemu-devel] [RFC PATCH 12/20] Memory: Add func to fire pasidt_bind notifier, Paolo Bonzini, 2017/04/26
- Re: [Qemu-devel] [RFC PATCH 12/20] Memory: Add func to fire pasidt_bind notifier, Liu, Yi L, 2017/04/26
- Re: [Qemu-devel] [RFC PATCH 12/20] Memory: Add func to fire pasidt_bind notifier, Peter Xu, 2017/04/27
- Re: [Qemu-devel] [RFC PATCH 12/20] Memory: Add func to fire pasidt_bind notifier,
Peter Xu <=
- Re: [Qemu-devel] [RFC PATCH 12/20] Memory: Add func to fire pasidt_bind notifier, Liu, Yi L, 2017/04/27
- Re: [Qemu-devel] [RFC PATCH 12/20] Memory: Add func to fire pasidt_bind notifier, Peter Xu, 2017/04/27
[Qemu-devel] [RFC PATCH 13/20] IOMMU: add pasid_table_info for guest pasid table, Liu, Yi L, 2017/04/26
[Qemu-devel] [RFC PATCH 14/20] intel_iommu: add FOR_EACH_ASSIGN_DEVICE macro, Liu, Yi L, 2017/04/26
[Qemu-devel] [RFC PATCH 15/20] intel_iommu: link whole guest pasid table to host, Liu, Yi L, 2017/04/26
[Qemu-devel] [RFC PATCH 16/20] VFIO: Add notifier for propagating IOMMU TLB invalidate, Liu, Yi L, 2017/04/26
[Qemu-devel] [RFC PATCH 17/20] Memory: Add func to fire TLB invalidate notifier, Liu, Yi L, 2017/04/26
[Qemu-devel] [RFC PATCH 18/20] intel_iommu: propagate Extended-IOTLB invalidate to host, Liu, Yi L, 2017/04/26
[Qemu-devel] [RFC PATCH 19/20] intel_iommu: propagate PASID-Cache invalidate to host, Liu, Yi L, 2017/04/26