| Message ID | 1484276800-26814-10-git-send-email-peterx@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 2017年01月13日 11:06, Peter Xu wrote: > Generalizing the notify logic in memory_region_notify_iommu() into a > single function. This can be further used in customized replay() > functions for IOMMUs. > > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > include/exec/memory.h | 15 +++++++++++++++ > memory.c | 29 ++++++++++++++++++----------- > 2 files changed, 33 insertions(+), 11 deletions(-) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 2233f99..f367e54 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -669,6 +669,21 @@ void memory_region_notify_iommu(MemoryRegion *mr, > IOMMUTLBEntry entry); > > /** > + * memory_region_notify_one: notify a change in an IOMMU translation > + * entry to a single notifier > + * > + * This works just like memory_region_notify_iommu(), but it only > + * notifies a specific notifier, not all of them. > + * > + * @notifier: the notifier to be notified > + * @entry: the new entry in the IOMMU translation table. The entry > + * replaces all old entries for the same virtual I/O address range. > + * Deleted entries have .@perm == 0. > + */ > +void memory_region_notify_one(IOMMUNotifier *notifier, > + IOMMUTLBEntry *entry); > + > +/** > * memory_region_register_iommu_notifier: register a notifier for changes to > * IOMMU translation entries. > * > diff --git a/memory.c b/memory.c > index df62bd1..6e4c872 100644 > --- a/memory.c > +++ b/memory.c > @@ -1665,26 +1665,33 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr, > memory_region_update_iommu_notify_flags(mr); > } > > -void memory_region_notify_iommu(MemoryRegion *mr, > - IOMMUTLBEntry entry) > +void memory_region_notify_one(IOMMUNotifier *notifier, > + IOMMUTLBEntry *entry) > { > - IOMMUNotifier *iommu_notifier; > IOMMUNotifierFlag request_flags; > > - assert(memory_region_is_iommu(mr)); > - > - if (entry.perm & IOMMU_RW) { > + if (entry->perm & IOMMU_RW) { > request_flags = IOMMU_NOTIFIER_MAP; > } else { > request_flags = IOMMU_NOTIFIER_UNMAP; > } Nit: you can keep this outside the loop. Thanks > > + if (notifier->notifier_flags & request_flags && > + notifier->start <= entry->iova && > + notifier->end >= entry->iova) { > + notifier->notify(notifier, entry); > + } > +} > + > +void memory_region_notify_iommu(MemoryRegion *mr, > + IOMMUTLBEntry entry) > +{ > + IOMMUNotifier *iommu_notifier; > + > + assert(memory_region_is_iommu(mr)); > + > QLIST_FOREACH(iommu_notifier, &mr->iommu_notify, node) { > - if (iommu_notifier->notifier_flags & request_flags && > - iommu_notifier->start <= entry.iova && > - iommu_notifier->end >= entry.iova) { > - iommu_notifier->notify(iommu_notifier, &entry); > - } > + memory_region_notify_one(iommu_notifier, &entry); > } > } >
On Fri, Jan 13, 2017 at 03:58:59PM +0800, Jason Wang wrote: > > > On 2017年01月13日 11:06, Peter Xu wrote: > >Generalizing the notify logic in memory_region_notify_iommu() into a > >single function. This can be further used in customized replay() > >functions for IOMMUs. > > > >Signed-off-by: Peter Xu <peterx@redhat.com> > >--- > > include/exec/memory.h | 15 +++++++++++++++ > > memory.c | 29 ++++++++++++++++++----------- > > 2 files changed, 33 insertions(+), 11 deletions(-) > > > >diff --git a/include/exec/memory.h b/include/exec/memory.h > >index 2233f99..f367e54 100644 > >--- a/include/exec/memory.h > >+++ b/include/exec/memory.h > >@@ -669,6 +669,21 @@ void memory_region_notify_iommu(MemoryRegion *mr, > > IOMMUTLBEntry entry); > > /** > >+ * memory_region_notify_one: notify a change in an IOMMU translation > >+ * entry to a single notifier > >+ * > >+ * This works just like memory_region_notify_iommu(), but it only > >+ * notifies a specific notifier, not all of them. > >+ * > >+ * @notifier: the notifier to be notified > >+ * @entry: the new entry in the IOMMU translation table. The entry > >+ * replaces all old entries for the same virtual I/O address range. > >+ * Deleted entries have .@perm == 0. > >+ */ > >+void memory_region_notify_one(IOMMUNotifier *notifier, > >+ IOMMUTLBEntry *entry); > >+ > >+/** > > * memory_region_register_iommu_notifier: register a notifier for changes to > > * IOMMU translation entries. > > * > >diff --git a/memory.c b/memory.c > >index df62bd1..6e4c872 100644 > >--- a/memory.c > >+++ b/memory.c > >@@ -1665,26 +1665,33 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr, > > memory_region_update_iommu_notify_flags(mr); > > } > >-void memory_region_notify_iommu(MemoryRegion *mr, > >- IOMMUTLBEntry entry) > >+void memory_region_notify_one(IOMMUNotifier *notifier, > >+ IOMMUTLBEntry *entry) > > { > >- IOMMUNotifier *iommu_notifier; > > IOMMUNotifierFlag request_flags; > >- assert(memory_region_is_iommu(mr)); > >- > >- if (entry.perm & IOMMU_RW) { > >+ if (entry->perm & IOMMU_RW) { > > request_flags = IOMMU_NOTIFIER_MAP; > > } else { > > request_flags = IOMMU_NOTIFIER_UNMAP; > > } > > Nit: you can keep this outside the loop. Yes, but this function is used in vtd_replay_hook() as well in latter patch. If I keep the above outside loop (IIUC you mean the loop in memory_region_notify_iommu()), I'll need to set it up as well in future vtd_replay_hook(), then that'll be slightly awkward. Considering that the notification will only happen at mapping changes, I'll prefer to keep the interface cleaner like what this patch has done. Thanks, -- peterx
On 2017年01月16日 15:08, Peter Xu wrote: > On Fri, Jan 13, 2017 at 03:58:59PM +0800, Jason Wang wrote: >> >> On 2017年01月13日 11:06, Peter Xu wrote: >>> Generalizing the notify logic in memory_region_notify_iommu() into a >>> single function. This can be further used in customized replay() >>> functions for IOMMUs. >>> >>> Signed-off-by: Peter Xu <peterx@redhat.com> >>> --- >>> include/exec/memory.h | 15 +++++++++++++++ >>> memory.c | 29 ++++++++++++++++++----------- >>> 2 files changed, 33 insertions(+), 11 deletions(-) >>> >>> diff --git a/include/exec/memory.h b/include/exec/memory.h >>> index 2233f99..f367e54 100644 >>> --- a/include/exec/memory.h >>> +++ b/include/exec/memory.h >>> @@ -669,6 +669,21 @@ void memory_region_notify_iommu(MemoryRegion *mr, >>> IOMMUTLBEntry entry); >>> /** >>> + * memory_region_notify_one: notify a change in an IOMMU translation >>> + * entry to a single notifier >>> + * >>> + * This works just like memory_region_notify_iommu(), but it only >>> + * notifies a specific notifier, not all of them. >>> + * >>> + * @notifier: the notifier to be notified >>> + * @entry: the new entry in the IOMMU translation table. The entry >>> + * replaces all old entries for the same virtual I/O address range. >>> + * Deleted entries have .@perm == 0. >>> + */ >>> +void memory_region_notify_one(IOMMUNotifier *notifier, >>> + IOMMUTLBEntry *entry); >>> + >>> +/** >>> * memory_region_register_iommu_notifier: register a notifier for changes to >>> * IOMMU translation entries. >>> * >>> diff --git a/memory.c b/memory.c >>> index df62bd1..6e4c872 100644 >>> --- a/memory.c >>> +++ b/memory.c >>> @@ -1665,26 +1665,33 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr, >>> memory_region_update_iommu_notify_flags(mr); >>> } >>> -void memory_region_notify_iommu(MemoryRegion *mr, >>> - IOMMUTLBEntry entry) >>> +void memory_region_notify_one(IOMMUNotifier *notifier, >>> + IOMMUTLBEntry *entry) >>> { >>> - IOMMUNotifier *iommu_notifier; >>> IOMMUNotifierFlag request_flags; >>> - assert(memory_region_is_iommu(mr)); >>> - >>> - if (entry.perm & IOMMU_RW) { >>> + if (entry->perm & IOMMU_RW) { >>> request_flags = IOMMU_NOTIFIER_MAP; >>> } else { >>> request_flags = IOMMU_NOTIFIER_UNMAP; >>> } >> Nit: you can keep this outside the loop. > Yes, but this function is used in vtd_replay_hook() as well in latter > patch. If I keep the above outside loop (IIUC you mean the loop in > memory_region_notify_iommu()), I'll need to set it up as well in > future vtd_replay_hook(), then that'll be slightly awkward. > Considering that the notification will only happen at mapping changes, > I'll prefer to keep the interface cleaner like what this patch has > done. > > Thanks, > > -- peterx Ok, I see.
diff --git a/include/exec/memory.h b/include/exec/memory.h index 2233f99..f367e54 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -669,6 +669,21 @@ void memory_region_notify_iommu(MemoryRegion *mr, IOMMUTLBEntry entry); /** + * memory_region_notify_one: notify a change in an IOMMU translation + * entry to a single notifier + * + * This works just like memory_region_notify_iommu(), but it only + * notifies a specific notifier, not all of them. + * + * @notifier: the notifier to be notified + * @entry: the new entry in the IOMMU translation table. The entry + * replaces all old entries for the same virtual I/O address range. + * Deleted entries have .@perm == 0. + */ +void memory_region_notify_one(IOMMUNotifier *notifier, + IOMMUTLBEntry *entry); + +/** * memory_region_register_iommu_notifier: register a notifier for changes to * IOMMU translation entries. * diff --git a/memory.c b/memory.c index df62bd1..6e4c872 100644 --- a/memory.c +++ b/memory.c @@ -1665,26 +1665,33 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr, memory_region_update_iommu_notify_flags(mr); } -void memory_region_notify_iommu(MemoryRegion *mr, - IOMMUTLBEntry entry) +void memory_region_notify_one(IOMMUNotifier *notifier, + IOMMUTLBEntry *entry) { - IOMMUNotifier *iommu_notifier; IOMMUNotifierFlag request_flags; - assert(memory_region_is_iommu(mr)); - - if (entry.perm & IOMMU_RW) { + if (entry->perm & IOMMU_RW) { request_flags = IOMMU_NOTIFIER_MAP; } else { request_flags = IOMMU_NOTIFIER_UNMAP; } + if (notifier->notifier_flags & request_flags && + notifier->start <= entry->iova && + notifier->end >= entry->iova) { + notifier->notify(notifier, entry); + } +} + +void memory_region_notify_iommu(MemoryRegion *mr, + IOMMUTLBEntry entry) +{ + IOMMUNotifier *iommu_notifier; + + assert(memory_region_is_iommu(mr)); + QLIST_FOREACH(iommu_notifier, &mr->iommu_notify, node) { - if (iommu_notifier->notifier_flags & request_flags && - iommu_notifier->start <= entry.iova && - iommu_notifier->end >= entry.iova) { - iommu_notifier->notify(iommu_notifier, &entry); - } + memory_region_notify_one(iommu_notifier, &entry); } }
Generalizing the notify logic in memory_region_notify_iommu() into a single function. This can be further used in customized replay() functions for IOMMUs. Signed-off-by: Peter Xu <peterx@redhat.com> --- include/exec/memory.h | 15 +++++++++++++++ memory.c | 29 ++++++++++++++++++----------- 2 files changed, 33 insertions(+), 11 deletions(-)