[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 14:14:27 +0800 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
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.
Thanks,
--
Peter Xu
- [Qemu-devel] [RFC PATCH 06/20] VFIO: add new notifier for binding PASID table, (continued)
- [Qemu-devel] [RFC PATCH 06/20] VFIO: add new notifier for binding PASID table, Liu, Yi L, 2017/04/26
- [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 <=
- 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, 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