Re: [Qemu-devel] [PATCH RFC v3 09/14] memory: introduce memory_region_notify_one()
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
---
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.
Re: [Qemu-devel] [PATCH RFC v3 09/14] memory: introduce memory_region_notify_one()
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
> >---
> > 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
Re: [Qemu-devel] [PATCH RFC v3 09/14] memory: introduce memory_region_notify_one()
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
---
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);
}
}
