* [Qemu-devel] [RFC PATCH 0/8] IOMMU: introduce common IOMMUObject
@ 2017-04-27 9:34 Peter Xu
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 1/8] memory: rename IOMMU_NOTIFIER_* Peter Xu
` (7 more replies)
0 siblings, 8 replies; 30+ messages in thread
From: Peter Xu @ 2017-04-27 9:34 UTC (permalink / raw)
To: qemu-devel
Cc: tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu, peterx,
Jason Wang, David Gibson, Alex Williamson
>From time to time, I was always thinking whether we should have a
IOMMU object in common, not depending on platform (x86/ppc/...), or
bus (PCI/...). Virt-svm is a case that may use it now - it needs to
register some notifiers so that when some event triggered the callback
can be invoked. However current MemoryRegion IOMMU notifiers do not
suite well with that purpose, since the notifiers are mostly binded to
memory regions and tailored to IOTLB notifications, while for virt-svm
it should be listening to one IOMMU device, rather than a specific
memory region.
This series tried to do several things:
- renaming old "iommu notifiers" into "iotlb notifiers", to avoid
confusion of what it is used for
- introduce a way to fetch an IOMMU object from a device DMA address
space (for x86, it should be a pci device)
- introduce a common IOMMU object, along with a similar notifier
mechanism for it
There is a use-case example in the last patch to show how to use it,
on how to modify customized IOMMU device to have such a common object,
and on how to use the notifiers (merely the same as old one).
If with luck, this series can be a pre-requisite for the following
patchset:
[RFC PATCH 00/20] Qemu: Extend intel_iommu emulator to support
Shared Virtual Memory
But before that, looking forward to any of your comments.
Test done: compile test only.
Please kindly review. Thanks.
Peter Xu (8):
memory: rename IOMMU_NOTIFIER_*
memory: rename IOMMUNotifier
memory: rename iommu_notifier_init()
memory: rename *_notify_iommu*
memory: rename *iommu_notifier*
memory: introduce AddressSpaceOps
intel_iommu: provide AddressSpaceOps.iommu_get()
iommu: introduce hw/core/iommu
hw/core/Makefile.objs | 1 +
hw/core/iommu.c | 61 +++++++++++++++++++++++++++++
hw/i386/amd_iommu.c | 6 +--
hw/i386/intel_iommu.c | 47 +++++++++++++---------
hw/ppc/spapr_iommu.c | 10 ++---
hw/s390x/s390-pci-inst.c | 2 +-
hw/vfio/common.c | 16 ++++----
hw/virtio/vhost.c | 14 +++----
include/exec/memory.h | 91 +++++++++++++++++++++++++++----------------
include/hw/core/iommu.h | 72 ++++++++++++++++++++++++++++++++++
include/hw/i386/intel_iommu.h | 10 +++--
include/hw/vfio/vfio-common.h | 2 +-
include/hw/virtio/vhost.h | 4 +-
memory.c | 54 ++++++++++++++-----------
14 files changed, 283 insertions(+), 107 deletions(-)
create mode 100644 hw/core/iommu.c
create mode 100644 include/hw/core/iommu.h
--
2.7.4
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC PATCH 1/8] memory: rename IOMMU_NOTIFIER_*
2017-04-27 9:34 [Qemu-devel] [RFC PATCH 0/8] IOMMU: introduce common IOMMUObject Peter Xu
@ 2017-04-27 9:34 ` Peter Xu
2017-05-01 4:50 ` David Gibson
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 2/8] memory: rename IOMMUNotifier Peter Xu
` (6 subsequent siblings)
7 siblings, 1 reply; 30+ messages in thread
From: Peter Xu @ 2017-04-27 9:34 UTC (permalink / raw)
To: qemu-devel
Cc: tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu, peterx,
Jason Wang, David Gibson, Alex Williamson
IOMMU notifiers before are mostly used for [dev-]IOTLB stuffs. It is not
suitable for other kind of notifiers (one example would be the future
virt-svm support). Considering that current notifiers are targeted for
per memory region, renaming all the notifier types from IOMMU_NOTIFIER_*
prefix into IOMMU_MR_EVENT_* to better show its usage (for memory
regions).
Alongside, renaming IOMMUNotifierFlag into IOMMUMREventFlag.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
hw/i386/amd_iommu.c | 6 +++---
hw/i386/intel_iommu.c | 12 ++++++------
hw/ppc/spapr_iommu.c | 8 ++++----
hw/vfio/common.c | 2 +-
hw/virtio/vhost.c | 2 +-
include/exec/memory.h | 20 ++++++++++----------
memory.c | 12 ++++++------
7 files changed, 31 insertions(+), 31 deletions(-)
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index f86a40a..315ec2a 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1067,12 +1067,12 @@ static const MemoryRegionOps mmio_mem_ops = {
};
static void amdvi_iommu_notify_flag_changed(MemoryRegion *iommu,
- IOMMUNotifierFlag old,
- IOMMUNotifierFlag new)
+ IOMMUMREventFlags old,
+ IOMMUMREventFlags new)
{
AMDVIAddressSpace *as = container_of(iommu, AMDVIAddressSpace, iommu);
- if (new & IOMMU_NOTIFIER_MAP) {
+ if (new & IOMMU_MR_EVENT_MAP) {
error_report("device %02x.%02x.%x requires iommu notifier which is not "
"currently supported", as->bus_num, PCI_SLOT(as->devfn),
PCI_FUNC(as->devfn));
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 02f047c..dce1ee8 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -1132,7 +1132,7 @@ static void vtd_context_device_invalidate(IntelIOMMUState *s,
/*
* So a device is moving out of (or moving into) a
* domain, a replay() suites here to notify all the
- * IOMMU_NOTIFIER_MAP registers about this change.
+ * IOMMU_MR_EVENT_MAP registers about this change.
* This won't bring bad even if we have no such
* notifier registered - the IOMMU notification
* framework will skip MAP notifications if that
@@ -2253,21 +2253,21 @@ static IOMMUTLBEntry vtd_iommu_translate(MemoryRegion *iommu, hwaddr addr,
}
static void vtd_iommu_notify_flag_changed(MemoryRegion *iommu,
- IOMMUNotifierFlag old,
- IOMMUNotifierFlag new)
+ IOMMUMREventFlags old,
+ IOMMUMREventFlags new)
{
VTDAddressSpace *vtd_as = container_of(iommu, VTDAddressSpace, iommu);
IntelIOMMUState *s = vtd_as->iommu_state;
IntelIOMMUNotifierNode *node = NULL;
IntelIOMMUNotifierNode *next_node = NULL;
- if (!s->caching_mode && new & IOMMU_NOTIFIER_MAP) {
+ if (!s->caching_mode && new & IOMMU_MR_EVENT_MAP) {
error_report("We need to set cache_mode=1 for intel-iommu to enable "
"device assignment with IOMMU protection.");
exit(1);
}
- if (old == IOMMU_NOTIFIER_NONE) {
+ if (old == IOMMU_MR_EVENT_NONE) {
node = g_malloc0(sizeof(*node));
node->vtd_as = vtd_as;
QLIST_INSERT_HEAD(&s->notifiers_list, node, next);
@@ -2277,7 +2277,7 @@ static void vtd_iommu_notify_flag_changed(MemoryRegion *iommu,
/* update notifier node with new flags */
QLIST_FOREACH_SAFE(node, &s->notifiers_list, next, next_node) {
if (node->vtd_as == vtd_as) {
- if (new == IOMMU_NOTIFIER_NONE) {
+ if (new == IOMMU_MR_EVENT_NONE) {
QLIST_REMOVE(node, next);
g_free(node);
}
diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index 29c80bb..5e6e70b 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -158,14 +158,14 @@ static uint64_t spapr_tce_get_min_page_size(MemoryRegion *iommu)
}
static void spapr_tce_notify_flag_changed(MemoryRegion *iommu,
- IOMMUNotifierFlag old,
- IOMMUNotifierFlag new)
+ IOMMUMREventFlags old,
+ IOMMUMREventFlags new)
{
struct sPAPRTCETable *tbl = container_of(iommu, sPAPRTCETable, iommu);
- if (old == IOMMU_NOTIFIER_NONE && new != IOMMU_NOTIFIER_NONE) {
+ if (old == IOMMU_MR_EVENT_NONE && new != IOMMU_MR_EVENT_NONE) {
spapr_tce_set_need_vfio(tbl, true);
- } else if (old != IOMMU_NOTIFIER_NONE && new == IOMMU_NOTIFIER_NONE) {
+ } else if (old != IOMMU_MR_EVENT_NONE && new == IOMMU_MR_EVENT_NONE) {
spapr_tce_set_need_vfio(tbl, false);
}
}
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 6b33b9f..a6ca10b 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -482,7 +482,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
section->size);
llend = int128_sub(llend, int128_one());
iommu_notifier_init(&giommu->n, vfio_iommu_map_notify,
- IOMMU_NOTIFIER_ALL,
+ IOMMU_MR_EVENT_ALL,
section->offset_within_region,
int128_get64(llend));
QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next);
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 0001e60..2e8d8fc 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -747,7 +747,7 @@ static void vhost_iommu_region_add(MemoryListener *listener,
section->size);
end = int128_sub(end, int128_one());
iommu_notifier_init(&iommu->n, vhost_iommu_unmap_notify,
- IOMMU_NOTIFIER_UNMAP,
+ IOMMU_MR_EVENT_UNMAP,
section->offset_within_region,
int128_get64(end));
iommu->mr = section->mr;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 99e0f54..90730b1 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -70,14 +70,14 @@ struct IOMMUTLBEntry {
* register with one or multiple IOMMU Notifier capability bit(s).
*/
typedef enum {
- IOMMU_NOTIFIER_NONE = 0,
+ IOMMU_MR_EVENT_NONE = 0,
/* Notify cache invalidations */
- IOMMU_NOTIFIER_UNMAP = 0x1,
+ IOMMU_MR_EVENT_UNMAP = 0x1,
/* Notify entry changes (newly created entries) */
- IOMMU_NOTIFIER_MAP = 0x2,
-} IOMMUNotifierFlag;
+ IOMMU_MR_EVENT_MAP = 0x2,
+} IOMMUMREventFlags;
-#define IOMMU_NOTIFIER_ALL (IOMMU_NOTIFIER_MAP | IOMMU_NOTIFIER_UNMAP)
+#define IOMMU_MR_EVENT_ALL (IOMMU_MR_EVENT_MAP | IOMMU_MR_EVENT_UNMAP)
struct IOMMUNotifier;
typedef void (*IOMMUNotify)(struct IOMMUNotifier *notifier,
@@ -85,7 +85,7 @@ typedef void (*IOMMUNotify)(struct IOMMUNotifier *notifier,
struct IOMMUNotifier {
IOMMUNotify notify;
- IOMMUNotifierFlag notifier_flags;
+ IOMMUMREventFlags notifier_flags;
/* Notify for address space range start <= addr <= end */
hwaddr start;
hwaddr end;
@@ -94,7 +94,7 @@ struct IOMMUNotifier {
typedef struct IOMMUNotifier IOMMUNotifier;
static inline void iommu_notifier_init(IOMMUNotifier *n, IOMMUNotify fn,
- IOMMUNotifierFlag flags,
+ IOMMUMREventFlags flags,
hwaddr start, hwaddr end)
{
n->notify = fn;
@@ -191,8 +191,8 @@ struct MemoryRegionIOMMUOps {
uint64_t (*get_min_page_size)(MemoryRegion *iommu);
/* Called when IOMMU Notifier flag changed */
void (*notify_flag_changed)(MemoryRegion *iommu,
- IOMMUNotifierFlag old_flags,
- IOMMUNotifierFlag new_flags);
+ IOMMUMREventFlags old_flags,
+ IOMMUMREventFlags new_flags);
/* Set this up to provide customized IOMMU replay function */
void (*replay)(MemoryRegion *iommu, IOMMUNotifier *notifier);
};
@@ -240,7 +240,7 @@ struct MemoryRegion {
unsigned ioeventfd_nb;
MemoryRegionIoeventfd *ioeventfds;
QLIST_HEAD(, IOMMUNotifier) iommu_notify;
- IOMMUNotifierFlag iommu_notify_flags;
+ IOMMUMREventFlags iommu_notify_flags;
};
#define IOMMU_NOTIFIER_FOREACH(n, mr) \
diff --git a/memory.c b/memory.c
index b727f5e..6237631 100644
--- a/memory.c
+++ b/memory.c
@@ -1483,7 +1483,7 @@ void memory_region_init_iommu(MemoryRegion *mr,
mr->iommu_ops = ops,
mr->terminates = true; /* then re-forwards */
QLIST_INIT(&mr->iommu_notify);
- mr->iommu_notify_flags = IOMMU_NOTIFIER_NONE;
+ mr->iommu_notify_flags = IOMMU_MR_EVENT_NONE;
}
static void memory_region_finalize(Object *obj)
@@ -1580,7 +1580,7 @@ bool memory_region_is_logging(MemoryRegion *mr, uint8_t client)
static void memory_region_update_iommu_notify_flags(MemoryRegion *mr)
{
- IOMMUNotifierFlag flags = IOMMU_NOTIFIER_NONE;
+ IOMMUMREventFlags flags = IOMMU_MR_EVENT_NONE;
IOMMUNotifier *iommu_notifier;
IOMMU_NOTIFIER_FOREACH(iommu_notifier, mr) {
@@ -1605,7 +1605,7 @@ void memory_region_register_iommu_notifier(MemoryRegion *mr,
}
/* We need to register for at least one bitfield */
- assert(n->notifier_flags != IOMMU_NOTIFIER_NONE);
+ assert(n->notifier_flags != IOMMU_MR_EVENT_NONE);
assert(n->start <= n->end);
QLIST_INSERT_HEAD(&mr->iommu_notify, n, node);
memory_region_update_iommu_notify_flags(mr);
@@ -1671,7 +1671,7 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr,
void memory_region_notify_one(IOMMUNotifier *notifier,
IOMMUTLBEntry *entry)
{
- IOMMUNotifierFlag request_flags;
+ IOMMUMREventFlags request_flags;
/*
* Skip the notification if the notification does not overlap
@@ -1683,9 +1683,9 @@ void memory_region_notify_one(IOMMUNotifier *notifier,
}
if (entry->perm & IOMMU_RW) {
- request_flags = IOMMU_NOTIFIER_MAP;
+ request_flags = IOMMU_MR_EVENT_MAP;
} else {
- request_flags = IOMMU_NOTIFIER_UNMAP;
+ request_flags = IOMMU_MR_EVENT_UNMAP;
}
if (notifier->notifier_flags & request_flags) {
--
2.7.4
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC PATCH 2/8] memory: rename IOMMUNotifier
2017-04-27 9:34 [Qemu-devel] [RFC PATCH 0/8] IOMMU: introduce common IOMMUObject Peter Xu
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 1/8] memory: rename IOMMU_NOTIFIER_* Peter Xu
@ 2017-04-27 9:34 ` Peter Xu
2017-05-01 4:51 ` David Gibson
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 3/8] memory: rename iommu_notifier_init() Peter Xu
` (5 subsequent siblings)
7 siblings, 1 reply; 30+ messages in thread
From: Peter Xu @ 2017-04-27 9:34 UTC (permalink / raw)
To: qemu-devel
Cc: tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu, peterx,
Jason Wang, David Gibson, Alex Williamson
Renaming it to IOMMUMRNotifier. This is a corresponding change to
previous patch, to emphasize that these notifiers are based on memory
regions.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
hw/i386/intel_iommu.c | 20 ++++++++++----------
hw/vfio/common.c | 2 +-
hw/virtio/vhost.c | 2 +-
include/exec/memory.h | 28 ++++++++++++++--------------
include/hw/i386/intel_iommu.h | 8 ++++----
include/hw/vfio/vfio-common.h | 2 +-
include/hw/virtio/vhost.h | 4 ++--
memory.c | 14 +++++++-------
8 files changed, 40 insertions(+), 40 deletions(-)
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index dce1ee8..3306e6a 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -1041,7 +1041,7 @@ static void vtd_interrupt_remap_table_setup(IntelIOMMUState *s)
static void vtd_iommu_replay_all(IntelIOMMUState *s)
{
- IntelIOMMUNotifierNode *node;
+ IntelIOMMUMRNotifierNode *node;
QLIST_FOREACH(node, &s->notifiers_list, next) {
memory_region_iommu_replay_all(&node->vtd_as->iommu);
@@ -1185,7 +1185,7 @@ static void vtd_iotlb_global_invalidate(IntelIOMMUState *s)
static void vtd_iotlb_domain_invalidate(IntelIOMMUState *s, uint16_t domain_id)
{
- IntelIOMMUNotifierNode *node;
+ IntelIOMMUMRNotifierNode *node;
VTDContextEntry ce;
VTDAddressSpace *vtd_as;
@@ -1213,7 +1213,7 @@ static void vtd_iotlb_page_invalidate_notify(IntelIOMMUState *s,
uint16_t domain_id, hwaddr addr,
uint8_t am)
{
- IntelIOMMUNotifierNode *node;
+ IntelIOMMUMRNotifierNode *node;
VTDContextEntry ce;
int ret;
@@ -2258,8 +2258,8 @@ static void vtd_iommu_notify_flag_changed(MemoryRegion *iommu,
{
VTDAddressSpace *vtd_as = container_of(iommu, VTDAddressSpace, iommu);
IntelIOMMUState *s = vtd_as->iommu_state;
- IntelIOMMUNotifierNode *node = NULL;
- IntelIOMMUNotifierNode *next_node = NULL;
+ IntelIOMMUMRNotifierNode *node = NULL;
+ IntelIOMMUMRNotifierNode *next_node = NULL;
if (!s->caching_mode && new & IOMMU_MR_EVENT_MAP) {
error_report("We need to set cache_mode=1 for intel-iommu to enable "
@@ -2702,7 +2702,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
}
/* Unmap the whole range in the notifier's scope. */
-static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n)
+static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUMRNotifier *n)
{
IOMMUTLBEntry entry;
hwaddr size;
@@ -2757,9 +2757,9 @@ static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n)
static void vtd_address_space_unmap_all(IntelIOMMUState *s)
{
- IntelIOMMUNotifierNode *node;
+ IntelIOMMUMRNotifierNode *node;
VTDAddressSpace *vtd_as;
- IOMMUNotifier *n;
+ IOMMUMRNotifier *n;
QLIST_FOREACH(node, &s->notifiers_list, next) {
vtd_as = node->vtd_as;
@@ -2771,11 +2771,11 @@ static void vtd_address_space_unmap_all(IntelIOMMUState *s)
static int vtd_replay_hook(IOMMUTLBEntry *entry, void *private)
{
- memory_region_notify_one((IOMMUNotifier *)private, entry);
+ memory_region_notify_one((IOMMUMRNotifier *)private, entry);
return 0;
}
-static void vtd_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n)
+static void vtd_iommu_replay(MemoryRegion *mr, IOMMUMRNotifier *n)
{
VTDAddressSpace *vtd_as = container_of(mr, VTDAddressSpace, iommu);
IntelIOMMUState *s = vtd_as->iommu_state;
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index a6ca10b..bd113b7 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -332,7 +332,7 @@ static bool vfio_get_vaddr(IOMMUTLBEntry *iotlb, void **vaddr,
return true;
}
-static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
+static void vfio_iommu_map_notify(IOMMUMRNotifier *n, IOMMUTLBEntry *iotlb)
{
VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
VFIOContainer *container = giommu->container;
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 2e8d8fc..81a8aea 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -718,7 +718,7 @@ static void vhost_region_del(MemoryListener *listener,
}
}
-static void vhost_iommu_unmap_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
+static void vhost_iommu_unmap_notify(IOMMUMRNotifier *n, IOMMUTLBEntry *iotlb)
{
struct vhost_iommu *iommu = container_of(n, struct vhost_iommu, n);
struct vhost_dev *hdev = iommu->hdev;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 90730b1..6be0c02 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -66,7 +66,7 @@ struct IOMMUTLBEntry {
};
/*
- * Bitmap for different IOMMUNotifier capabilities. Each notifier can
+ * Bitmap for different IOMMUMRNotifier capabilities. Each notifier can
* register with one or multiple IOMMU Notifier capability bit(s).
*/
typedef enum {
@@ -79,21 +79,21 @@ typedef enum {
#define IOMMU_MR_EVENT_ALL (IOMMU_MR_EVENT_MAP | IOMMU_MR_EVENT_UNMAP)
-struct IOMMUNotifier;
-typedef void (*IOMMUNotify)(struct IOMMUNotifier *notifier,
+struct IOMMUMRNotifier;
+typedef void (*IOMMUNotify)(struct IOMMUMRNotifier *notifier,
IOMMUTLBEntry *data);
-struct IOMMUNotifier {
+struct IOMMUMRNotifier {
IOMMUNotify notify;
IOMMUMREventFlags notifier_flags;
/* Notify for address space range start <= addr <= end */
hwaddr start;
hwaddr end;
- QLIST_ENTRY(IOMMUNotifier) node;
+ QLIST_ENTRY(IOMMUMRNotifier) node;
};
-typedef struct IOMMUNotifier IOMMUNotifier;
+typedef struct IOMMUMRNotifier IOMMUMRNotifier;
-static inline void iommu_notifier_init(IOMMUNotifier *n, IOMMUNotify fn,
+static inline void iommu_notifier_init(IOMMUMRNotifier *n, IOMMUNotify fn,
IOMMUMREventFlags flags,
hwaddr start, hwaddr end)
{
@@ -194,7 +194,7 @@ struct MemoryRegionIOMMUOps {
IOMMUMREventFlags old_flags,
IOMMUMREventFlags new_flags);
/* Set this up to provide customized IOMMU replay function */
- void (*replay)(MemoryRegion *iommu, IOMMUNotifier *notifier);
+ void (*replay)(MemoryRegion *iommu, IOMMUMRNotifier *notifier);
};
typedef struct CoalescedMemoryRange CoalescedMemoryRange;
@@ -239,7 +239,7 @@ struct MemoryRegion {
const char *name;
unsigned ioeventfd_nb;
MemoryRegionIoeventfd *ioeventfds;
- QLIST_HEAD(, IOMMUNotifier) iommu_notify;
+ QLIST_HEAD(, IOMMUMRNotifier) iommu_notify;
IOMMUMREventFlags iommu_notify_flags;
};
@@ -703,7 +703,7 @@ void memory_region_notify_iommu(MemoryRegion *mr,
* replaces all old entries for the same virtual I/O address range.
* Deleted entries have .@perm == 0.
*/
-void memory_region_notify_one(IOMMUNotifier *notifier,
+void memory_region_notify_one(IOMMUMRNotifier *notifier,
IOMMUTLBEntry *entry);
/**
@@ -711,12 +711,12 @@ void memory_region_notify_one(IOMMUNotifier *notifier,
* IOMMU translation entries.
*
* @mr: the memory region to observe
- * @n: the IOMMUNotifier to be added; the notify callback receives a
+ * @n: the IOMMUMRNotifier to be added; the notify callback receives a
* pointer to an #IOMMUTLBEntry as the opaque value; the pointer
* ceases to be valid on exit from the notifier.
*/
void memory_region_register_iommu_notifier(MemoryRegion *mr,
- IOMMUNotifier *n);
+ IOMMUMRNotifier *n);
/**
* memory_region_iommu_replay: replay existing IOMMU translations to
@@ -728,7 +728,7 @@ void memory_region_register_iommu_notifier(MemoryRegion *mr,
* @is_write: Whether to treat the replay as a translate "write"
* through the iommu
*/
-void memory_region_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n,
+void memory_region_iommu_replay(MemoryRegion *mr, IOMMUMRNotifier *n,
bool is_write);
/**
@@ -748,7 +748,7 @@ void memory_region_iommu_replay_all(MemoryRegion *mr);
* @n: the notifier to be removed.
*/
void memory_region_unregister_iommu_notifier(MemoryRegion *mr,
- IOMMUNotifier *n);
+ IOMMUMRNotifier *n);
/**
* memory_region_name: get a memory region's name
diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index 3e51876..f9ac0ec 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -63,7 +63,7 @@ typedef union VTD_IR_TableEntry VTD_IR_TableEntry;
typedef union VTD_IR_MSIAddress VTD_IR_MSIAddress;
typedef struct VTDIrq VTDIrq;
typedef struct VTD_MSIMessage VTD_MSIMessage;
-typedef struct IntelIOMMUNotifierNode IntelIOMMUNotifierNode;
+typedef struct IntelIOMMUMRNotifierNode IntelIOMMUMRNotifierNode;
/* Context-Entry */
struct VTDContextEntry {
@@ -250,9 +250,9 @@ struct VTD_MSIMessage {
/* When IR is enabled, all MSI/MSI-X data bits should be zero */
#define VTD_IR_MSI_DATA (0)
-struct IntelIOMMUNotifierNode {
+struct IntelIOMMUMRNotifierNode {
VTDAddressSpace *vtd_as;
- QLIST_ENTRY(IntelIOMMUNotifierNode) next;
+ QLIST_ENTRY(IntelIOMMUMRNotifierNode) next;
};
/* The iommu (DMAR) device state struct */
@@ -293,7 +293,7 @@ struct IntelIOMMUState {
GHashTable *vtd_as_by_busptr; /* VTDBus objects indexed by PCIBus* reference */
VTDBus *vtd_as_by_bus_num[VTD_PCI_BUS_MAX]; /* VTDBus objects indexed by bus number */
/* list of registered notifiers */
- QLIST_HEAD(, IntelIOMMUNotifierNode) notifiers_list;
+ QLIST_HEAD(, IntelIOMMUMRNotifierNode) notifiers_list;
/* interrupt remapping */
bool intr_enabled; /* Whether guest enabled IR */
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index c582de1..348d408 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -96,7 +96,7 @@ typedef struct VFIOGuestIOMMU {
VFIOContainer *container;
MemoryRegion *iommu;
hwaddr iommu_offset;
- IOMMUNotifier n;
+ IOMMUMRNotifier n;
QLIST_ENTRY(VFIOGuestIOMMU) giommu_next;
} VFIOGuestIOMMU;
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index a450321..091cfad 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -42,7 +42,7 @@ struct vhost_iommu {
struct vhost_dev *hdev;
MemoryRegion *mr;
hwaddr iommu_offset;
- IOMMUNotifier n;
+ IOMMUMRNotifier n;
QLIST_ENTRY(vhost_iommu) iommu_next;
};
@@ -75,7 +75,7 @@ struct vhost_dev {
struct vhost_log *log;
QLIST_ENTRY(vhost_dev) entry;
QLIST_HEAD(, vhost_iommu) iommu_list;
- IOMMUNotifier n;
+ IOMMUMRNotifier n;
};
int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
diff --git a/memory.c b/memory.c
index 6237631..ffbd020 100644
--- a/memory.c
+++ b/memory.c
@@ -1581,7 +1581,7 @@ bool memory_region_is_logging(MemoryRegion *mr, uint8_t client)
static void memory_region_update_iommu_notify_flags(MemoryRegion *mr)
{
IOMMUMREventFlags flags = IOMMU_MR_EVENT_NONE;
- IOMMUNotifier *iommu_notifier;
+ IOMMUMRNotifier *iommu_notifier;
IOMMU_NOTIFIER_FOREACH(iommu_notifier, mr) {
flags |= iommu_notifier->notifier_flags;
@@ -1597,7 +1597,7 @@ static void memory_region_update_iommu_notify_flags(MemoryRegion *mr)
}
void memory_region_register_iommu_notifier(MemoryRegion *mr,
- IOMMUNotifier *n)
+ IOMMUMRNotifier *n)
{
if (mr->alias) {
memory_region_register_iommu_notifier(mr->alias, n);
@@ -1620,7 +1620,7 @@ uint64_t memory_region_iommu_get_min_page_size(MemoryRegion *mr)
return TARGET_PAGE_SIZE;
}
-void memory_region_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n,
+void memory_region_iommu_replay(MemoryRegion *mr, IOMMUMRNotifier *n,
bool is_write)
{
hwaddr addr, granularity;
@@ -1650,7 +1650,7 @@ void memory_region_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n,
void memory_region_iommu_replay_all(MemoryRegion *mr)
{
- IOMMUNotifier *notifier;
+ IOMMUMRNotifier *notifier;
IOMMU_NOTIFIER_FOREACH(notifier, mr) {
memory_region_iommu_replay(mr, notifier, false);
@@ -1658,7 +1658,7 @@ void memory_region_iommu_replay_all(MemoryRegion *mr)
}
void memory_region_unregister_iommu_notifier(MemoryRegion *mr,
- IOMMUNotifier *n)
+ IOMMUMRNotifier *n)
{
if (mr->alias) {
memory_region_unregister_iommu_notifier(mr->alias, n);
@@ -1668,7 +1668,7 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr,
memory_region_update_iommu_notify_flags(mr);
}
-void memory_region_notify_one(IOMMUNotifier *notifier,
+void memory_region_notify_one(IOMMUMRNotifier *notifier,
IOMMUTLBEntry *entry)
{
IOMMUMREventFlags request_flags;
@@ -1696,7 +1696,7 @@ void memory_region_notify_one(IOMMUNotifier *notifier,
void memory_region_notify_iommu(MemoryRegion *mr,
IOMMUTLBEntry entry)
{
- IOMMUNotifier *iommu_notifier;
+ IOMMUMRNotifier *iommu_notifier;
assert(memory_region_is_iommu(mr));
--
2.7.4
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC PATCH 3/8] memory: rename iommu_notifier_init()
2017-04-27 9:34 [Qemu-devel] [RFC PATCH 0/8] IOMMU: introduce common IOMMUObject Peter Xu
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 1/8] memory: rename IOMMU_NOTIFIER_* Peter Xu
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 2/8] memory: rename IOMMUNotifier Peter Xu
@ 2017-04-27 9:34 ` Peter Xu
2017-05-01 4:53 ` David Gibson
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 4/8] memory: rename *_notify_iommu* Peter Xu
` (4 subsequent siblings)
7 siblings, 1 reply; 30+ messages in thread
From: Peter Xu @ 2017-04-27 9:34 UTC (permalink / raw)
To: qemu-devel
Cc: tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu, peterx,
Jason Wang, David Gibson, Alex Williamson
It's new name is iommu_mr_notifier_init(). Again, literal changes only.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
hw/vfio/common.c | 8 ++++----
hw/virtio/vhost.c | 8 ++++----
include/exec/memory.h | 7 ++++---
3 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index bd113b7..6b3953b 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -481,10 +481,10 @@ static void vfio_listener_region_add(MemoryListener *listener,
llend = int128_add(int128_make64(section->offset_within_region),
section->size);
llend = int128_sub(llend, int128_one());
- iommu_notifier_init(&giommu->n, vfio_iommu_map_notify,
- IOMMU_MR_EVENT_ALL,
- section->offset_within_region,
- int128_get64(llend));
+ iommu_mr_notifier_init(&giommu->n, vfio_iommu_map_notify,
+ IOMMU_MR_EVENT_ALL,
+ section->offset_within_region,
+ int128_get64(llend));
QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next);
memory_region_register_iommu_notifier(giommu->iommu, &giommu->n);
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 81a8aea..7449cf8 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -746,10 +746,10 @@ static void vhost_iommu_region_add(MemoryListener *listener,
end = int128_add(int128_make64(section->offset_within_region),
section->size);
end = int128_sub(end, int128_one());
- iommu_notifier_init(&iommu->n, vhost_iommu_unmap_notify,
- IOMMU_MR_EVENT_UNMAP,
- section->offset_within_region,
- int128_get64(end));
+ iommu_mr_notifier_init(&iommu->n, vhost_iommu_unmap_notify,
+ IOMMU_MR_EVENT_UNMAP,
+ section->offset_within_region,
+ int128_get64(end));
iommu->mr = section->mr;
iommu->iommu_offset = section->offset_within_address_space -
section->offset_within_region;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 6be0c02..8d8dcb2 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -93,9 +93,10 @@ struct IOMMUMRNotifier {
};
typedef struct IOMMUMRNotifier IOMMUMRNotifier;
-static inline void iommu_notifier_init(IOMMUMRNotifier *n, IOMMUNotify fn,
- IOMMUMREventFlags flags,
- hwaddr start, hwaddr end)
+static inline void iommu_mr_notifier_init(IOMMUMRNotifier *n,
+ IOMMUNotify fn,
+ IOMMUMREventFlags flags,
+ hwaddr start, hwaddr end)
{
n->notify = fn;
n->notifier_flags = flags;
--
2.7.4
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC PATCH 4/8] memory: rename *_notify_iommu*
2017-04-27 9:34 [Qemu-devel] [RFC PATCH 0/8] IOMMU: introduce common IOMMUObject Peter Xu
` (2 preceding siblings ...)
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 3/8] memory: rename iommu_notifier_init() Peter Xu
@ 2017-04-27 9:34 ` Peter Xu
2017-05-01 4:55 ` David Gibson
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 5/8] memory: rename *iommu_notifier* Peter Xu
` (3 subsequent siblings)
7 siblings, 1 reply; 30+ messages in thread
From: Peter Xu @ 2017-04-27 9:34 UTC (permalink / raw)
To: qemu-devel
Cc: tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu, peterx,
Jason Wang, David Gibson, Alex Williamson
Actually it's notifying IOTLB updates (map, or unmap). Let's be explicit
on the wording - replacing it with *_notify_iotlb*.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
hw/i386/intel_iommu.c | 8 ++++----
hw/ppc/spapr_iommu.c | 2 +-
hw/s390x/s390-pci-inst.c | 2 +-
include/exec/memory.h | 12 ++++++------
memory.c | 8 ++++----
5 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 3306e6a..609732b 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -1205,7 +1205,7 @@ static void vtd_iotlb_domain_invalidate(IntelIOMMUState *s, uint16_t domain_id)
static int vtd_page_invalidate_notify_hook(IOMMUTLBEntry *entry,
void *private)
{
- memory_region_notify_iommu((MemoryRegion *)private, *entry);
+ memory_region_notify_iotlb((MemoryRegion *)private, *entry);
return 0;
}
@@ -1709,7 +1709,7 @@ static bool vtd_process_device_iotlb_desc(IntelIOMMUState *s,
entry.iova = addr;
entry.perm = IOMMU_NONE;
entry.translated_addr = 0;
- memory_region_notify_iommu(&vtd_dev_as->iommu, entry);
+ memory_region_notify_iotlb(&vtd_dev_as->iommu, entry);
done:
return true;
@@ -2752,7 +2752,7 @@ static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUMRNotifier *n)
VTD_PCI_FUNC(as->devfn),
entry.iova, size);
- memory_region_notify_one(n, &entry);
+ memory_region_notify_iotlb_one(n, &entry);
}
static void vtd_address_space_unmap_all(IntelIOMMUState *s)
@@ -2771,7 +2771,7 @@ static void vtd_address_space_unmap_all(IntelIOMMUState *s)
static int vtd_replay_hook(IOMMUTLBEntry *entry, void *private)
{
- memory_region_notify_one((IOMMUMRNotifier *)private, entry);
+ memory_region_notify_iotlb_one((IOMMUMRNotifier *)private, entry);
return 0;
}
diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index 5e6e70b..9a34495 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -414,7 +414,7 @@ static target_ulong put_tce_emu(sPAPRTCETable *tcet, target_ulong ioba,
entry.translated_addr = tce & page_mask;
entry.addr_mask = ~page_mask;
entry.perm = spapr_tce_iommu_access_flags(tce);
- memory_region_notify_iommu(&tcet->iommu, entry);
+ memory_region_notify_iotlb(&tcet->iommu, entry);
return H_SUCCESS;
}
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 314a9cb..566eeda 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -635,7 +635,7 @@ int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
goto out;
}
- memory_region_notify_iommu(mr, entry);
+ memory_region_notify_iotlb(mr, entry);
start += entry.addr_mask + 1;
}
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 8d8dcb2..09188a6 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -673,7 +673,7 @@ static inline bool memory_region_is_iommu(MemoryRegion *mr)
uint64_t memory_region_iommu_get_min_page_size(MemoryRegion *mr);
/**
- * memory_region_notify_iommu: notify a change in an IOMMU translation entry.
+ * memory_region_notify_iotlb: notify a change in an IOMMU translation entry.
*
* The notification type will be decided by entry.perm bits:
*
@@ -689,12 +689,12 @@ uint64_t memory_region_iommu_get_min_page_size(MemoryRegion *mr);
* replaces all old entries for the same virtual I/O address range.
* Deleted entries have .@perm == 0.
*/
-void memory_region_notify_iommu(MemoryRegion *mr,
+void memory_region_notify_iotlb(MemoryRegion *mr,
IOMMUTLBEntry entry);
/**
- * memory_region_notify_one: notify a change in an IOMMU translation
- * entry to a single notifier
+ * memory_region_notify_iotlb_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.
@@ -704,8 +704,8 @@ void memory_region_notify_iommu(MemoryRegion *mr,
* replaces all old entries for the same virtual I/O address range.
* Deleted entries have .@perm == 0.
*/
-void memory_region_notify_one(IOMMUMRNotifier *notifier,
- IOMMUTLBEntry *entry);
+void memory_region_notify_iotlb_one(IOMMUMRNotifier *notifier,
+ IOMMUTLBEntry *entry);
/**
* memory_region_register_iommu_notifier: register a notifier for changes to
diff --git a/memory.c b/memory.c
index ffbd020..c6532e4 100644
--- a/memory.c
+++ b/memory.c
@@ -1668,8 +1668,8 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr,
memory_region_update_iommu_notify_flags(mr);
}
-void memory_region_notify_one(IOMMUMRNotifier *notifier,
- IOMMUTLBEntry *entry)
+void memory_region_notify_iotlb_one(IOMMUMRNotifier *notifier,
+ IOMMUTLBEntry *entry)
{
IOMMUMREventFlags request_flags;
@@ -1693,7 +1693,7 @@ void memory_region_notify_one(IOMMUMRNotifier *notifier,
}
}
-void memory_region_notify_iommu(MemoryRegion *mr,
+void memory_region_notify_iotlb(MemoryRegion *mr,
IOMMUTLBEntry entry)
{
IOMMUMRNotifier *iommu_notifier;
@@ -1701,7 +1701,7 @@ void memory_region_notify_iommu(MemoryRegion *mr,
assert(memory_region_is_iommu(mr));
IOMMU_NOTIFIER_FOREACH(iommu_notifier, mr) {
- memory_region_notify_one(iommu_notifier, &entry);
+ memory_region_notify_iotlb_one(iommu_notifier, &entry);
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC PATCH 5/8] memory: rename *iommu_notifier*
2017-04-27 9:34 [Qemu-devel] [RFC PATCH 0/8] IOMMU: introduce common IOMMUObject Peter Xu
` (3 preceding siblings ...)
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 4/8] memory: rename *_notify_iommu* Peter Xu
@ 2017-04-27 9:34 ` Peter Xu
2017-05-01 4:56 ` David Gibson
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 6/8] memory: introduce AddressSpaceOps Peter Xu
` (2 subsequent siblings)
7 siblings, 1 reply; 30+ messages in thread
From: Peter Xu @ 2017-04-27 9:34 UTC (permalink / raw)
To: qemu-devel
Cc: tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu, peterx,
Jason Wang, David Gibson, Alex Williamson
Renaming *iommu_notifiers* into *iotlb_notifiers*. Again, let's reserve
the iommu_notifier keyword to the notifiers that will be for per-iommu,
and let the old per-mr notifier be iotlb_notifiers.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
hw/vfio/common.c | 6 +++---
hw/virtio/vhost.c | 4 ++--
include/exec/memory.h | 8 ++++----
memory.c | 20 ++++++++++----------
4 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 6b3953b..7b639ea 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -487,7 +487,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
int128_get64(llend));
QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next);
- memory_region_register_iommu_notifier(giommu->iommu, &giommu->n);
+ memory_region_register_iotlb_notifier(giommu->iommu, &giommu->n);
memory_region_iommu_replay(giommu->iommu, &giommu->n, false);
return;
@@ -557,7 +557,7 @@ static void vfio_listener_region_del(MemoryListener *listener,
QLIST_FOREACH(giommu, &container->giommu_list, giommu_next) {
if (giommu->iommu == section->mr &&
giommu->n.start == section->offset_within_region) {
- memory_region_unregister_iommu_notifier(giommu->iommu,
+ memory_region_unregister_iotlb_notifier(giommu->iommu,
&giommu->n);
QLIST_REMOVE(giommu, giommu_next);
g_free(giommu);
@@ -1147,7 +1147,7 @@ static void vfio_disconnect_container(VFIOGroup *group)
QLIST_REMOVE(container, next);
QLIST_FOREACH_SAFE(giommu, &container->giommu_list, giommu_next, tmp) {
- memory_region_unregister_iommu_notifier(giommu->iommu, &giommu->n);
+ memory_region_unregister_iotlb_notifier(giommu->iommu, &giommu->n);
QLIST_REMOVE(giommu, giommu_next);
g_free(giommu);
}
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 7449cf8..a1e03ed 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -754,7 +754,7 @@ static void vhost_iommu_region_add(MemoryListener *listener,
iommu->iommu_offset = section->offset_within_address_space -
section->offset_within_region;
iommu->hdev = dev;
- memory_region_register_iommu_notifier(section->mr, &iommu->n);
+ memory_region_register_iotlb_notifier(section->mr, &iommu->n);
QLIST_INSERT_HEAD(&dev->iommu_list, iommu, iommu_next);
/* TODO: can replay help performance here? */
}
@@ -773,7 +773,7 @@ static void vhost_iommu_region_del(MemoryListener *listener,
QLIST_FOREACH(iommu, &dev->iommu_list, iommu_next) {
if (iommu->mr == section->mr &&
iommu->n.start == section->offset_within_region) {
- memory_region_unregister_iommu_notifier(iommu->mr,
+ memory_region_unregister_iotlb_notifier(iommu->mr,
&iommu->n);
QLIST_REMOVE(iommu, iommu_next);
g_free(iommu);
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 09188a6..e5707b3 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -708,7 +708,7 @@ void memory_region_notify_iotlb_one(IOMMUMRNotifier *notifier,
IOMMUTLBEntry *entry);
/**
- * memory_region_register_iommu_notifier: register a notifier for changes to
+ * memory_region_register_iotlb_notifier: register a notifier for changes to
* IOMMU translation entries.
*
* @mr: the memory region to observe
@@ -716,7 +716,7 @@ void memory_region_notify_iotlb_one(IOMMUMRNotifier *notifier,
* pointer to an #IOMMUTLBEntry as the opaque value; the pointer
* ceases to be valid on exit from the notifier.
*/
-void memory_region_register_iommu_notifier(MemoryRegion *mr,
+void memory_region_register_iotlb_notifier(MemoryRegion *mr,
IOMMUMRNotifier *n);
/**
@@ -741,14 +741,14 @@ void memory_region_iommu_replay(MemoryRegion *mr, IOMMUMRNotifier *n,
void memory_region_iommu_replay_all(MemoryRegion *mr);
/**
- * memory_region_unregister_iommu_notifier: unregister a notifier for
+ * memory_region_unregister_iotlb_notifier: unregister a notifier for
* changes to IOMMU translation entries.
*
* @mr: the memory region which was observed and for which notity_stopped()
* needs to be called
* @n: the notifier to be removed.
*/
-void memory_region_unregister_iommu_notifier(MemoryRegion *mr,
+void memory_region_unregister_iotlb_notifier(MemoryRegion *mr,
IOMMUMRNotifier *n);
/**
diff --git a/memory.c b/memory.c
index c6532e4..6af523e 100644
--- a/memory.c
+++ b/memory.c
@@ -1581,10 +1581,10 @@ bool memory_region_is_logging(MemoryRegion *mr, uint8_t client)
static void memory_region_update_iommu_notify_flags(MemoryRegion *mr)
{
IOMMUMREventFlags flags = IOMMU_MR_EVENT_NONE;
- IOMMUMRNotifier *iommu_notifier;
+ IOMMUMRNotifier *iotlb_notifier;
- IOMMU_NOTIFIER_FOREACH(iommu_notifier, mr) {
- flags |= iommu_notifier->notifier_flags;
+ IOMMU_NOTIFIER_FOREACH(iotlb_notifier, mr) {
+ flags |= iotlb_notifier->notifier_flags;
}
if (flags != mr->iommu_notify_flags &&
@@ -1596,11 +1596,11 @@ static void memory_region_update_iommu_notify_flags(MemoryRegion *mr)
mr->iommu_notify_flags = flags;
}
-void memory_region_register_iommu_notifier(MemoryRegion *mr,
+void memory_region_register_iotlb_notifier(MemoryRegion *mr,
IOMMUMRNotifier *n)
{
if (mr->alias) {
- memory_region_register_iommu_notifier(mr->alias, n);
+ memory_region_register_iotlb_notifier(mr->alias, n);
return;
}
@@ -1657,11 +1657,11 @@ void memory_region_iommu_replay_all(MemoryRegion *mr)
}
}
-void memory_region_unregister_iommu_notifier(MemoryRegion *mr,
+void memory_region_unregister_iotlb_notifier(MemoryRegion *mr,
IOMMUMRNotifier *n)
{
if (mr->alias) {
- memory_region_unregister_iommu_notifier(mr->alias, n);
+ memory_region_unregister_iotlb_notifier(mr->alias, n);
return;
}
QLIST_REMOVE(n, node);
@@ -1696,12 +1696,12 @@ void memory_region_notify_iotlb_one(IOMMUMRNotifier *notifier,
void memory_region_notify_iotlb(MemoryRegion *mr,
IOMMUTLBEntry entry)
{
- IOMMUMRNotifier *iommu_notifier;
+ IOMMUMRNotifier *iotlb_notifier;
assert(memory_region_is_iommu(mr));
- IOMMU_NOTIFIER_FOREACH(iommu_notifier, mr) {
- memory_region_notify_iotlb_one(iommu_notifier, &entry);
+ IOMMU_NOTIFIER_FOREACH(iotlb_notifier, mr) {
+ memory_region_notify_iotlb_one(iotlb_notifier, &entry);
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC PATCH 6/8] memory: introduce AddressSpaceOps
2017-04-27 9:34 [Qemu-devel] [RFC PATCH 0/8] IOMMU: introduce common IOMMUObject Peter Xu
` (4 preceding siblings ...)
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 5/8] memory: rename *iommu_notifier* Peter Xu
@ 2017-04-27 9:34 ` Peter Xu
2017-05-01 4:58 ` David Gibson
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 7/8] intel_iommu: provide AddressSpaceOps.iommu_get() Peter Xu
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 8/8] iommu: introduce hw/core/iommu Peter Xu
7 siblings, 1 reply; 30+ messages in thread
From: Peter Xu @ 2017-04-27 9:34 UTC (permalink / raw)
To: qemu-devel
Cc: tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu, peterx,
Jason Wang, David Gibson, Alex Williamson
This is something similar to MemoryRegionOps, it's just for address
spaces to store arch-specific hooks.
The first hook I would like to introduce is iommu_get().
For systems that have IOMMUs, we will create a special address space per
device which is different from system default address space for
it (please refer to pci_device_iommu_address_space()). Normally when
that happens, there will be one specific IOMMU (or say, translation
unit) stands right behind that new address space.
This iommu_get() fetches that guy behind the address space. Here, the
guy is defined as IOMMUObject, which is currently a (void *). In the
future, maybe we can make it a better definition, but imho it's good
enough for now, considering it's arch-dependent.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/exec/memory.h | 30 ++++++++++++++++++++++++++++++
memory.c | 8 ++++++++
2 files changed, 38 insertions(+)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index e5707b3..0b0b58b 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -183,6 +183,15 @@ struct MemoryRegionOps {
const MemoryRegionMmio old_mmio;
};
+/*
+ * This stands for an IOMMU unit. Normally it should be exactly the
+ * IOMMU device, however this can also be actually anything which is
+ * related to that translation unit. What it is should be totally
+ * arch-dependent. Maybe one day we can have something better than a
+ * (void *) here.
+ */
+typedef void *IOMMUObject;
+
typedef struct MemoryRegionIOMMUOps MemoryRegionIOMMUOps;
struct MemoryRegionIOMMUOps {
@@ -282,6 +291,19 @@ struct MemoryListener {
};
/**
+ * AddressSpaceOps: callbacks structure for address space specific operations
+ *
+ * @iommu_get: returns an IOMMU object that backs the address space.
+ * Normally this should be NULL for generic address
+ * spaces, and it's only used when there is one
+ * translation unit behind this address space.
+ */
+struct AddressSpaceOps {
+ IOMMUObject *(*iommu_get)(AddressSpace *as);
+};
+typedef struct AddressSpaceOps AddressSpaceOps;
+
+/**
* AddressSpace: describes a mapping of addresses to #MemoryRegion objects
*/
struct AddressSpace {
@@ -302,6 +324,7 @@ struct AddressSpace {
MemoryListener dispatch_listener;
QTAILQ_HEAD(memory_listeners_as, MemoryListener) listeners;
QTAILQ_ENTRY(AddressSpace) address_spaces_link;
+ AddressSpaceOps as_ops;
};
/**
@@ -1800,6 +1823,13 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr,
address_space_write(cache->as, cache->xlat + addr, MEMTXATTRS_UNSPECIFIED, buf, len);
}
+/**
+ * address_space_iommu_get: Get the backend IOMMU for the address space
+ *
+ * @as: the address space to fetch IOMMU from
+ */
+IOMMUObject *address_space_iommu_get(AddressSpace *as);
+
#endif
#endif
diff --git a/memory.c b/memory.c
index 6af523e..6aaad45 100644
--- a/memory.c
+++ b/memory.c
@@ -2500,6 +2500,14 @@ void address_space_destroy(AddressSpace *as)
call_rcu(as, do_address_space_destroy, rcu);
}
+IOMMUObject *address_space_iommu_get(AddressSpace *as)
+{
+ if (!as->as_ops.iommu_get) {
+ return NULL;
+ }
+ return as->as_ops.iommu_get(as);
+}
+
static const char *memory_region_type(MemoryRegion *mr)
{
if (memory_region_is_ram_device(mr)) {
--
2.7.4
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC PATCH 7/8] intel_iommu: provide AddressSpaceOps.iommu_get()
2017-04-27 9:34 [Qemu-devel] [RFC PATCH 0/8] IOMMU: introduce common IOMMUObject Peter Xu
` (5 preceding siblings ...)
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 6/8] memory: introduce AddressSpaceOps Peter Xu
@ 2017-04-27 9:34 ` Peter Xu
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 8/8] iommu: introduce hw/core/iommu Peter Xu
7 siblings, 0 replies; 30+ messages in thread
From: Peter Xu @ 2017-04-27 9:34 UTC (permalink / raw)
To: qemu-devel
Cc: tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu, peterx,
Jason Wang, David Gibson, Alex Williamson
Then it'll be the first one to support address_space_iommu_get() API.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
hw/i386/intel_iommu.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 609732b..5131329 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2632,6 +2632,12 @@ static const MemoryRegionOps vtd_mem_ir_ops = {
},
};
+static IOMMUObject *vtd_as_iommu_get(AddressSpace *as)
+{
+ VTDAddressSpace *vtd_dev_as = container_of(as, VTDAddressSpace, as);
+ return (IOMMUObject *)vtd_dev_as->iommu_state;
+}
+
VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
{
uintptr_t key = (uintptr_t)bus;
@@ -2692,6 +2698,9 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
VTD_INTERRUPT_ADDR_FIRST,
&vtd_dev_as->iommu_ir, 64);
address_space_init(&vtd_dev_as->as, &vtd_dev_as->root, name);
+
+ vtd_dev_as->as.as_ops.iommu_get = vtd_as_iommu_get;
+
memory_region_add_subregion_overlap(&vtd_dev_as->root, 0,
&vtd_dev_as->sys_alias, 1);
memory_region_add_subregion_overlap(&vtd_dev_as->root, 0,
--
2.7.4
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [Qemu-devel] [RFC PATCH 8/8] iommu: introduce hw/core/iommu
2017-04-27 9:34 [Qemu-devel] [RFC PATCH 0/8] IOMMU: introduce common IOMMUObject Peter Xu
` (6 preceding siblings ...)
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 7/8] intel_iommu: provide AddressSpaceOps.iommu_get() Peter Xu
@ 2017-04-27 9:34 ` Peter Xu
2017-04-28 10:01 ` Liu, Yi L
2017-06-07 7:51 ` Liu, Yi L
7 siblings, 2 replies; 30+ messages in thread
From: Peter Xu @ 2017-04-27 9:34 UTC (permalink / raw)
To: qemu-devel
Cc: tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu, peterx,
Jason Wang, David Gibson, Alex Williamson
Time to consider a common stuff for IOMMU. Let's start from an common
IOMMU object (which should be inlayed in custom IOMMU implementations)
and a notifier mechanism.
Let VT-d IOMMU be the first user.
An example to use this per-iommu notifier:
(when registering)
iommu = address_space_iommu_get(pci_device_iommu_address_space(dev));
notifier = iommu_notifier_register(iommu, IOMMU_EVENT_SVM_PASID, func);
...
(when notify)
IOMMUEvent event = { .type = IOMMU_EVENT_SVM_PASID ... };
iommu_notify(iommu, &event);
...
(when releasing)
iommu_notifier_unregister(notifier);
notifier = NULL;
Signed-off-by: Peter Xu <peterx@redhat.com>
---
hw/core/Makefile.objs | 1 +
hw/core/iommu.c | 61 ++++++++++++++++++++++++++++++++++++
hw/i386/intel_iommu.c | 2 +-
include/exec/memory.h | 10 +-----
include/hw/core/iommu.h | 72 +++++++++++++++++++++++++++++++++++++++++++
include/hw/i386/intel_iommu.h | 2 ++
6 files changed, 138 insertions(+), 10 deletions(-)
create mode 100644 hw/core/iommu.c
create mode 100644 include/hw/core/iommu.h
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index 91450b2..85cca44 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -5,6 +5,7 @@ common-obj-y += fw-path-provider.o
# irq.o needed for qdev GPIO handling:
common-obj-y += irq.o
common-obj-y += hotplug.o
+common-obj-y += iommu.o
obj-y += nmi.o
common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
diff --git a/hw/core/iommu.c b/hw/core/iommu.c
new file mode 100644
index 0000000..e014e96
--- /dev/null
+++ b/hw/core/iommu.c
@@ -0,0 +1,61 @@
+/*
+ * QEMU emulation of IOMMU logic
+ *
+ * Copyright (C) 2017 Red Hat Inc.
+ *
+ * Authors: Peter Xu <peterx@redhat.com>,
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/core/iommu.h"
+
+IOMMUNotifier *iommu_notifier_register(IOMMUObject *iommu,
+ IOMMUNotifyFn fn,
+ uint64_t event_mask)
+{
+ IOMMUNotifier *notifier = g_new0(IOMMUNotifier, 1);
+
+ assert((event_mask & ~IOMMU_EVENT_MASK) == 0);
+ notifier->event_mask = event_mask;
+ notifier->iommu_notify = fn;
+ QLIST_INSERT_HEAD(&iommu->iommu_notifiers, notifier, node);
+
+ return notifier;
+}
+
+void iommu_notifier_unregister(IOMMUObject *iommu,
+ IOMMUNotifier *notifier)
+{
+ IOMMUNotifier *cur, *next;
+
+ QLIST_FOREACH_SAFE(cur, &iommu->iommu_notifiers, node, next) {
+ if (cur == notifier) {
+ QLIST_REMOVE(cur, node);
+ break;
+ }
+ }
+}
+
+void iommu_notify(IOMMUObject *iommu, IOMMUEvent *event)
+{
+ IOMMUNotifier *cur;
+
+ QLIST_FOREACH(cur, &iommu->iommu_notifiers, node) {
+ if (cur->event_mask & event->type && cur->iommu_notify) {
+ cur->iommu_notify(cur, event);
+ }
+ }
+}
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 5131329..d6f6701 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2635,7 +2635,7 @@ static const MemoryRegionOps vtd_mem_ir_ops = {
static IOMMUObject *vtd_as_iommu_get(AddressSpace *as)
{
VTDAddressSpace *vtd_dev_as = container_of(as, VTDAddressSpace, as);
- return (IOMMUObject *)vtd_dev_as->iommu_state;
+ return &vtd_dev_as->iommu_state->iommu_common;
}
VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 0b0b58b..5ca1dd0 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -27,6 +27,7 @@
#include "qemu/notify.h"
#include "qom/object.h"
#include "qemu/rcu.h"
+#include "hw/core/iommu.h"
#define RAM_ADDR_INVALID (~(ram_addr_t)0)
@@ -183,15 +184,6 @@ struct MemoryRegionOps {
const MemoryRegionMmio old_mmio;
};
-/*
- * This stands for an IOMMU unit. Normally it should be exactly the
- * IOMMU device, however this can also be actually anything which is
- * related to that translation unit. What it is should be totally
- * arch-dependent. Maybe one day we can have something better than a
- * (void *) here.
- */
-typedef void *IOMMUObject;
-
typedef struct MemoryRegionIOMMUOps MemoryRegionIOMMUOps;
struct MemoryRegionIOMMUOps {
diff --git a/include/hw/core/iommu.h b/include/hw/core/iommu.h
new file mode 100644
index 0000000..16e6adf
--- /dev/null
+++ b/include/hw/core/iommu.h
@@ -0,0 +1,72 @@
+/*
+ * QEMU emulation of IOMMU logic
+ *
+ * Copyright (C) 2017 Red Hat Inc.
+ *
+ * Authors: Peter Xu <peterx@redhat.com>,
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __PCI_IOMMU_H__
+#define __PCI_IOMMU_H__
+
+#include "qemu/queue.h"
+
+#define IOMMU_EVENT_SVM_PASID (0)
+#define IOMMU_EVENT_MASK (IOMMU_EVENT_SVM_PASID)
+
+struct IOMMUEvent {
+ uint64_t type;
+ union {
+ struct {
+ /* TODO: fill in correct stuff. */
+ int value;
+ } svm;
+ } data;
+};
+typedef struct IOMMUEvent IOMMUEvent;
+
+typedef struct IOMMUNotifier IOMMUNotifier;
+
+typedef void (*IOMMUNotifyFn)(IOMMUNotifier *notifier, IOMMUEvent *event);
+
+struct IOMMUNotifier {
+ IOMMUNotifyFn iommu_notify;
+ /*
+ * What events we are listening to. Let's allow multiple event
+ * registrations from beginning.
+ */
+ uint64_t event_mask;
+ QLIST_ENTRY(IOMMUNotifier) node;
+};
+
+/*
+ * This stands for an IOMMU unit. Any translation device should have
+ * this struct inside its own structure to make sure it can leverage
+ * common IOMMU functionalities.
+ */
+struct IOMMUObject {
+ QLIST_HEAD(, IOMMUNotifier) iommu_notifiers;
+};
+typedef struct IOMMUObject IOMMUObject;
+
+IOMMUNotifier *iommu_notifier_register(IOMMUObject *iommu,
+ IOMMUNotifyFn fn,
+ uint64_t event_mask);
+void iommu_notifier_unregister(IOMMUObject *iommu,
+ IOMMUNotifier *notifier);
+void iommu_notify(IOMMUObject *iommu, IOMMUEvent *event);
+
+#endif
diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index f9ac0ec..ec696f5 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -26,6 +26,7 @@
#include "hw/i386/x86-iommu.h"
#include "hw/i386/ioapic.h"
#include "hw/pci/msi.h"
+#include "hw/core/iommu.h"
#include "hw/sysbus.h"
#define TYPE_INTEL_IOMMU_DEVICE "intel-iommu"
@@ -258,6 +259,7 @@ struct IntelIOMMUMRNotifierNode {
/* The iommu (DMAR) device state struct */
struct IntelIOMMUState {
X86IOMMUState x86_iommu;
+ IOMMUObject iommu_common;
MemoryRegion csrmem;
uint8_t csr[DMAR_REG_SIZE]; /* register values */
uint8_t wmask[DMAR_REG_SIZE]; /* R/W bytes */
--
2.7.4
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 8/8] iommu: introduce hw/core/iommu
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 8/8] iommu: introduce hw/core/iommu Peter Xu
@ 2017-04-28 10:01 ` Liu, Yi L
2017-04-28 10:34 ` Peter Xu
2017-06-07 7:51 ` Liu, Yi L
1 sibling, 1 reply; 30+ messages in thread
From: Liu, Yi L @ 2017-04-28 10:01 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, tianyu.lan, kevin.tian, yi.l.liu, Jason Wang,
Alex Williamson, Paolo Bonzini, David Gibson
On Thu, Apr 27, 2017 at 05:34:20PM +0800, Peter Xu wrote:
> Time to consider a common stuff for IOMMU. Let's start from an common
> IOMMU object (which should be inlayed in custom IOMMU implementations)
> and a notifier mechanism.
>
> Let VT-d IOMMU be the first user.
>
> An example to use this per-iommu notifier:
>
> (when registering)
> iommu = address_space_iommu_get(pci_device_iommu_address_space(dev));
> notifier = iommu_notifier_register(iommu, IOMMU_EVENT_SVM_PASID, func);
> ...
> (when notify)
> IOMMUEvent event = { .type = IOMMU_EVENT_SVM_PASID ... };
> iommu_notify(iommu, &event);
> ...
> (when releasing)
> iommu_notifier_unregister(notifier);
> notifier = NULL;
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> hw/core/Makefile.objs | 1 +
> hw/core/iommu.c | 61 ++++++++++++++++++++++++++++++++++++
> hw/i386/intel_iommu.c | 2 +-
> include/exec/memory.h | 10 +-----
> include/hw/core/iommu.h | 72 +++++++++++++++++++++++++++++++++++++++++++
> include/hw/i386/intel_iommu.h | 2 ++
> 6 files changed, 138 insertions(+), 10 deletions(-)
> create mode 100644 hw/core/iommu.c
> create mode 100644 include/hw/core/iommu.h
>
> diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
> index 91450b2..85cca44 100644
> --- a/hw/core/Makefile.objs
> +++ b/hw/core/Makefile.objs
> @@ -5,6 +5,7 @@ common-obj-y += fw-path-provider.o
> # irq.o needed for qdev GPIO handling:
> common-obj-y += irq.o
> common-obj-y += hotplug.o
> +common-obj-y += iommu.o
> obj-y += nmi.o
>
> common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
> diff --git a/hw/core/iommu.c b/hw/core/iommu.c
> new file mode 100644
> index 0000000..e014e96
> --- /dev/null
> +++ b/hw/core/iommu.c
> @@ -0,0 +1,61 @@
> +/*
> + * QEMU emulation of IOMMU logic
> + *
> + * Copyright (C) 2017 Red Hat Inc.
> + *
> + * Authors: Peter Xu <peterx@redhat.com>,
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> +
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> +
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/core/iommu.h"
> +
> +IOMMUNotifier *iommu_notifier_register(IOMMUObject *iommu,
> + IOMMUNotifyFn fn,
> + uint64_t event_mask)
> +{
> + IOMMUNotifier *notifier = g_new0(IOMMUNotifier, 1);
> +
> + assert((event_mask & ~IOMMU_EVENT_MASK) == 0);
> + notifier->event_mask = event_mask;
> + notifier->iommu_notify = fn;
> + QLIST_INSERT_HEAD(&iommu->iommu_notifiers, notifier, node);
> +
> + return notifier;
> +}
> +
> +void iommu_notifier_unregister(IOMMUObject *iommu,
> + IOMMUNotifier *notifier)
> +{
> + IOMMUNotifier *cur, *next;
> +
> + QLIST_FOREACH_SAFE(cur, &iommu->iommu_notifiers, node, next) {
> + if (cur == notifier) {
> + QLIST_REMOVE(cur, node);
> + break;
> + }
> + }
> +}
> +
> +void iommu_notify(IOMMUObject *iommu, IOMMUEvent *event)
> +{
> + IOMMUNotifier *cur;
> +
> + QLIST_FOREACH(cur, &iommu->iommu_notifiers, node) {
> + if (cur->event_mask & event->type && cur->iommu_notify) {
> + cur->iommu_notify(cur, event);
> + }
> + }
> +}
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 5131329..d6f6701 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2635,7 +2635,7 @@ static const MemoryRegionOps vtd_mem_ir_ops = {
> static IOMMUObject *vtd_as_iommu_get(AddressSpace *as)
> {
> VTDAddressSpace *vtd_dev_as = container_of(as, VTDAddressSpace, as);
> - return (IOMMUObject *)vtd_dev_as->iommu_state;
> + return &vtd_dev_as->iommu_state->iommu_common;
> }
>
> VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 0b0b58b..5ca1dd0 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -27,6 +27,7 @@
> #include "qemu/notify.h"
> #include "qom/object.h"
> #include "qemu/rcu.h"
> +#include "hw/core/iommu.h"
>
> #define RAM_ADDR_INVALID (~(ram_addr_t)0)
>
> @@ -183,15 +184,6 @@ struct MemoryRegionOps {
> const MemoryRegionMmio old_mmio;
> };
>
> -/*
> - * This stands for an IOMMU unit. Normally it should be exactly the
> - * IOMMU device, however this can also be actually anything which is
> - * related to that translation unit. What it is should be totally
> - * arch-dependent. Maybe one day we can have something better than a
> - * (void *) here.
> - */
> -typedef void *IOMMUObject;
> -
> typedef struct MemoryRegionIOMMUOps MemoryRegionIOMMUOps;
>
> struct MemoryRegionIOMMUOps {
> diff --git a/include/hw/core/iommu.h b/include/hw/core/iommu.h
> new file mode 100644
> index 0000000..16e6adf
> --- /dev/null
> +++ b/include/hw/core/iommu.h
> @@ -0,0 +1,72 @@
> +/*
> + * QEMU emulation of IOMMU logic
> + *
> + * Copyright (C) 2017 Red Hat Inc.
> + *
> + * Authors: Peter Xu <peterx@redhat.com>,
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> +
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> +
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __PCI_IOMMU_H__
> +#define __PCI_IOMMU_H__
> +
> +#include "qemu/queue.h"
> +
> +#define IOMMU_EVENT_SVM_PASID (0)
> +#define IOMMU_EVENT_MASK (IOMMU_EVENT_SVM_PASID)
Peter,
would it better to define it just like the IOTLB notifier flag?
> +struct IOMMUEvent {
> + uint64_t type;
> + union {
> + struct {
> + /* TODO: fill in correct stuff. */
> + int value;
> + } svm;
> + } data;
> +};
> +typedef struct IOMMUEvent IOMMUEvent;
> +
> +typedef struct IOMMUNotifier IOMMUNotifier;
> +
> +typedef void (*IOMMUNotifyFn)(IOMMUNotifier *notifier, IOMMUEvent *event);
Regards to the IOMMUEvent definition, would it be helpful to use void*?
virt-SVM may have two notifiers. They will have different data to pass
in the notifier.
Thanks,
Yi L
> +struct IOMMUNotifier {
> + IOMMUNotifyFn iommu_notify;
> + /*
> + * What events we are listening to. Let's allow multiple event
> + * registrations from beginning.
> + */
> + uint64_t event_mask;
> + QLIST_ENTRY(IOMMUNotifier) node;
> +};
> +
> +/*
> + * This stands for an IOMMU unit. Any translation device should have
> + * this struct inside its own structure to make sure it can leverage
> + * common IOMMU functionalities.
> + */
> +struct IOMMUObject {
> + QLIST_HEAD(, IOMMUNotifier) iommu_notifiers;
> +};
> +typedef struct IOMMUObject IOMMUObject;
> +
> +IOMMUNotifier *iommu_notifier_register(IOMMUObject *iommu,
> + IOMMUNotifyFn fn,
> + uint64_t event_mask);
> +void iommu_notifier_unregister(IOMMUObject *iommu,
> + IOMMUNotifier *notifier);
> +void iommu_notify(IOMMUObject *iommu, IOMMUEvent *event);
> +
> +#endif
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index f9ac0ec..ec696f5 100644
> --- a/include/hw/i386/intel_iommu.h
> +++ b/include/hw/i386/intel_iommu.h
> @@ -26,6 +26,7 @@
> #include "hw/i386/x86-iommu.h"
> #include "hw/i386/ioapic.h"
> #include "hw/pci/msi.h"
> +#include "hw/core/iommu.h"
> #include "hw/sysbus.h"
>
> #define TYPE_INTEL_IOMMU_DEVICE "intel-iommu"
> @@ -258,6 +259,7 @@ struct IntelIOMMUMRNotifierNode {
> /* The iommu (DMAR) device state struct */
> struct IntelIOMMUState {
> X86IOMMUState x86_iommu;
> + IOMMUObject iommu_common;
> MemoryRegion csrmem;
> uint8_t csr[DMAR_REG_SIZE]; /* register values */
> uint8_t wmask[DMAR_REG_SIZE]; /* R/W bytes */
> --
> 2.7.4
>
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 8/8] iommu: introduce hw/core/iommu
2017-04-28 10:01 ` Liu, Yi L
@ 2017-04-28 10:34 ` Peter Xu
0 siblings, 0 replies; 30+ messages in thread
From: Peter Xu @ 2017-04-28 10:34 UTC (permalink / raw)
To: Liu, Yi L
Cc: qemu-devel, tianyu.lan, kevin.tian, yi.l.liu, Jason Wang,
Alex Williamson, Paolo Bonzini, David Gibson
On Fri, Apr 28, 2017 at 06:01:58PM +0800, Liu, Yi L wrote:
> On Thu, Apr 27, 2017 at 05:34:20PM +0800, Peter Xu wrote:
> > Time to consider a common stuff for IOMMU. Let's start from an common
> > IOMMU object (which should be inlayed in custom IOMMU implementations)
> > and a notifier mechanism.
> >
> > Let VT-d IOMMU be the first user.
> >
> > An example to use this per-iommu notifier:
> >
> > (when registering)
> > iommu = address_space_iommu_get(pci_device_iommu_address_space(dev));
> > notifier = iommu_notifier_register(iommu, IOMMU_EVENT_SVM_PASID, func);
> > ...
> > (when notify)
> > IOMMUEvent event = { .type = IOMMU_EVENT_SVM_PASID ... };
> > iommu_notify(iommu, &event);
> > ...
> > (when releasing)
> > iommu_notifier_unregister(notifier);
> > notifier = NULL;
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> > hw/core/Makefile.objs | 1 +
> > hw/core/iommu.c | 61 ++++++++++++++++++++++++++++++++++++
> > hw/i386/intel_iommu.c | 2 +-
> > include/exec/memory.h | 10 +-----
> > include/hw/core/iommu.h | 72 +++++++++++++++++++++++++++++++++++++++++++
> > include/hw/i386/intel_iommu.h | 2 ++
> > 6 files changed, 138 insertions(+), 10 deletions(-)
> > create mode 100644 hw/core/iommu.c
> > create mode 100644 include/hw/core/iommu.h
> >
> > diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
> > index 91450b2..85cca44 100644
> > --- a/hw/core/Makefile.objs
> > +++ b/hw/core/Makefile.objs
> > @@ -5,6 +5,7 @@ common-obj-y += fw-path-provider.o
> > # irq.o needed for qdev GPIO handling:
> > common-obj-y += irq.o
> > common-obj-y += hotplug.o
> > +common-obj-y += iommu.o
> > obj-y += nmi.o
> >
> > common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
> > diff --git a/hw/core/iommu.c b/hw/core/iommu.c
> > new file mode 100644
> > index 0000000..e014e96
> > --- /dev/null
> > +++ b/hw/core/iommu.c
> > @@ -0,0 +1,61 @@
> > +/*
> > + * QEMU emulation of IOMMU logic
> > + *
> > + * Copyright (C) 2017 Red Hat Inc.
> > + *
> > + * Authors: Peter Xu <peterx@redhat.com>,
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > +
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > +
> > + * You should have received a copy of the GNU General Public License along
> > + * with this program; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/core/iommu.h"
> > +
> > +IOMMUNotifier *iommu_notifier_register(IOMMUObject *iommu,
> > + IOMMUNotifyFn fn,
> > + uint64_t event_mask)
> > +{
> > + IOMMUNotifier *notifier = g_new0(IOMMUNotifier, 1);
> > +
> > + assert((event_mask & ~IOMMU_EVENT_MASK) == 0);
> > + notifier->event_mask = event_mask;
> > + notifier->iommu_notify = fn;
> > + QLIST_INSERT_HEAD(&iommu->iommu_notifiers, notifier, node);
> > +
> > + return notifier;
> > +}
> > +
> > +void iommu_notifier_unregister(IOMMUObject *iommu,
> > + IOMMUNotifier *notifier)
> > +{
> > + IOMMUNotifier *cur, *next;
> > +
> > + QLIST_FOREACH_SAFE(cur, &iommu->iommu_notifiers, node, next) {
> > + if (cur == notifier) {
> > + QLIST_REMOVE(cur, node);
> > + break;
> > + }
> > + }
> > +}
> > +
> > +void iommu_notify(IOMMUObject *iommu, IOMMUEvent *event)
> > +{
> > + IOMMUNotifier *cur;
> > +
> > + QLIST_FOREACH(cur, &iommu->iommu_notifiers, node) {
> > + if (cur->event_mask & event->type && cur->iommu_notify) {
> > + cur->iommu_notify(cur, event);
> > + }
> > + }
> > +}
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > index 5131329..d6f6701 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -2635,7 +2635,7 @@ static const MemoryRegionOps vtd_mem_ir_ops = {
> > static IOMMUObject *vtd_as_iommu_get(AddressSpace *as)
> > {
> > VTDAddressSpace *vtd_dev_as = container_of(as, VTDAddressSpace, as);
> > - return (IOMMUObject *)vtd_dev_as->iommu_state;
> > + return &vtd_dev_as->iommu_state->iommu_common;
> > }
> >
> > VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
> > diff --git a/include/exec/memory.h b/include/exec/memory.h
> > index 0b0b58b..5ca1dd0 100644
> > --- a/include/exec/memory.h
> > +++ b/include/exec/memory.h
> > @@ -27,6 +27,7 @@
> > #include "qemu/notify.h"
> > #include "qom/object.h"
> > #include "qemu/rcu.h"
> > +#include "hw/core/iommu.h"
> >
> > #define RAM_ADDR_INVALID (~(ram_addr_t)0)
> >
> > @@ -183,15 +184,6 @@ struct MemoryRegionOps {
> > const MemoryRegionMmio old_mmio;
> > };
> >
> > -/*
> > - * This stands for an IOMMU unit. Normally it should be exactly the
> > - * IOMMU device, however this can also be actually anything which is
> > - * related to that translation unit. What it is should be totally
> > - * arch-dependent. Maybe one day we can have something better than a
> > - * (void *) here.
> > - */
> > -typedef void *IOMMUObject;
> > -
> > typedef struct MemoryRegionIOMMUOps MemoryRegionIOMMUOps;
> >
> > struct MemoryRegionIOMMUOps {
> > diff --git a/include/hw/core/iommu.h b/include/hw/core/iommu.h
> > new file mode 100644
> > index 0000000..16e6adf
> > --- /dev/null
> > +++ b/include/hw/core/iommu.h
> > @@ -0,0 +1,72 @@
> > +/*
> > + * QEMU emulation of IOMMU logic
> > + *
> > + * Copyright (C) 2017 Red Hat Inc.
> > + *
> > + * Authors: Peter Xu <peterx@redhat.com>,
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > +
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > +
> > + * You should have received a copy of the GNU General Public License along
> > + * with this program; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#ifndef __PCI_IOMMU_H__
> > +#define __PCI_IOMMU_H__
> > +
> > +#include "qemu/queue.h"
> > +
> > +#define IOMMU_EVENT_SVM_PASID (0)
Oh here it should be (1 << 0) really.
> > +#define IOMMU_EVENT_MASK (IOMMU_EVENT_SVM_PASID)
>
> Peter,
>
> would it better to define it just like the IOTLB notifier flag?
Actually if we would allow registering to several events for a single
notifier, imho it'll be nice to not use enum. Even use enum, we'll
need:
enum {
FLAG_1 = 1,
FLAG_2 = 2,
FLAG_3 = 4,
FLAG_4 = 8,
}
So we'll still need to setup these flags one by one - since that'll be
bitmask, not really what enum is doing naturally.
>
> > +struct IOMMUEvent {
> > + uint64_t type;
> > + union {
> > + struct {
> > + /* TODO: fill in correct stuff. */
> > + int value;
> > + } svm;
> > + } data;
> > +};
> > +typedef struct IOMMUEvent IOMMUEvent;
> > +
> > +typedef struct IOMMUNotifier IOMMUNotifier;
> > +
> > +typedef void (*IOMMUNotifyFn)(IOMMUNotifier *notifier, IOMMUEvent *event);
>
> Regards to the IOMMUEvent definition, would it be helpful to use void*?
> virt-SVM may have two notifiers. They will have different data to pass
> in the notifier.
Hmm, I think that's why I used union in IOMMUEvent definition. You can
just extend it with:
struct IOMMUEvent {
uint64_t type;
union {
struct {
/* TODO: fill in correct stuff. */
int value;
} svm;
struct {
int value_2;
} svm_2
} data;
};
I would avoid using void * if possible, to have more type checks.
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 1/8] memory: rename IOMMU_NOTIFIER_*
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 1/8] memory: rename IOMMU_NOTIFIER_* Peter Xu
@ 2017-05-01 4:50 ` David Gibson
0 siblings, 0 replies; 30+ messages in thread
From: David Gibson @ 2017-05-01 4:50 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu,
Jason Wang, Alex Williamson
[-- Attachment #1: Type: text/plain, Size: 11727 bytes --]
On Thu, Apr 27, 2017 at 05:34:13PM +0800, Peter Xu wrote:
> IOMMU notifiers before are mostly used for [dev-]IOTLB stuffs. It is not
> suitable for other kind of notifiers (one example would be the future
> virt-svm support). Considering that current notifiers are targeted for
> per memory region, renaming all the notifier types from IOMMU_NOTIFIER_*
> prefix into IOMMU_MR_EVENT_* to better show its usage (for memory
> regions).
>
> Alongside, renaming IOMMUNotifierFlag into IOMMUMREventFlag.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
I don't entirely follow the rationale above, but the change seems
reasonable.
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/i386/amd_iommu.c | 6 +++---
> hw/i386/intel_iommu.c | 12 ++++++------
> hw/ppc/spapr_iommu.c | 8 ++++----
> hw/vfio/common.c | 2 +-
> hw/virtio/vhost.c | 2 +-
> include/exec/memory.h | 20 ++++++++++----------
> memory.c | 12 ++++++------
> 7 files changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index f86a40a..315ec2a 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -1067,12 +1067,12 @@ static const MemoryRegionOps mmio_mem_ops = {
> };
>
> static void amdvi_iommu_notify_flag_changed(MemoryRegion *iommu,
> - IOMMUNotifierFlag old,
> - IOMMUNotifierFlag new)
> + IOMMUMREventFlags old,
> + IOMMUMREventFlags new)
> {
> AMDVIAddressSpace *as = container_of(iommu, AMDVIAddressSpace, iommu);
>
> - if (new & IOMMU_NOTIFIER_MAP) {
> + if (new & IOMMU_MR_EVENT_MAP) {
> error_report("device %02x.%02x.%x requires iommu notifier which is not "
> "currently supported", as->bus_num, PCI_SLOT(as->devfn),
> PCI_FUNC(as->devfn));
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 02f047c..dce1ee8 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -1132,7 +1132,7 @@ static void vtd_context_device_invalidate(IntelIOMMUState *s,
> /*
> * So a device is moving out of (or moving into) a
> * domain, a replay() suites here to notify all the
> - * IOMMU_NOTIFIER_MAP registers about this change.
> + * IOMMU_MR_EVENT_MAP registers about this change.
> * This won't bring bad even if we have no such
> * notifier registered - the IOMMU notification
> * framework will skip MAP notifications if that
> @@ -2253,21 +2253,21 @@ static IOMMUTLBEntry vtd_iommu_translate(MemoryRegion *iommu, hwaddr addr,
> }
>
> static void vtd_iommu_notify_flag_changed(MemoryRegion *iommu,
> - IOMMUNotifierFlag old,
> - IOMMUNotifierFlag new)
> + IOMMUMREventFlags old,
> + IOMMUMREventFlags new)
> {
> VTDAddressSpace *vtd_as = container_of(iommu, VTDAddressSpace, iommu);
> IntelIOMMUState *s = vtd_as->iommu_state;
> IntelIOMMUNotifierNode *node = NULL;
> IntelIOMMUNotifierNode *next_node = NULL;
>
> - if (!s->caching_mode && new & IOMMU_NOTIFIER_MAP) {
> + if (!s->caching_mode && new & IOMMU_MR_EVENT_MAP) {
> error_report("We need to set cache_mode=1 for intel-iommu to enable "
> "device assignment with IOMMU protection.");
> exit(1);
> }
>
> - if (old == IOMMU_NOTIFIER_NONE) {
> + if (old == IOMMU_MR_EVENT_NONE) {
> node = g_malloc0(sizeof(*node));
> node->vtd_as = vtd_as;
> QLIST_INSERT_HEAD(&s->notifiers_list, node, next);
> @@ -2277,7 +2277,7 @@ static void vtd_iommu_notify_flag_changed(MemoryRegion *iommu,
> /* update notifier node with new flags */
> QLIST_FOREACH_SAFE(node, &s->notifiers_list, next, next_node) {
> if (node->vtd_as == vtd_as) {
> - if (new == IOMMU_NOTIFIER_NONE) {
> + if (new == IOMMU_MR_EVENT_NONE) {
> QLIST_REMOVE(node, next);
> g_free(node);
> }
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index 29c80bb..5e6e70b 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -158,14 +158,14 @@ static uint64_t spapr_tce_get_min_page_size(MemoryRegion *iommu)
> }
>
> static void spapr_tce_notify_flag_changed(MemoryRegion *iommu,
> - IOMMUNotifierFlag old,
> - IOMMUNotifierFlag new)
> + IOMMUMREventFlags old,
> + IOMMUMREventFlags new)
> {
> struct sPAPRTCETable *tbl = container_of(iommu, sPAPRTCETable, iommu);
>
> - if (old == IOMMU_NOTIFIER_NONE && new != IOMMU_NOTIFIER_NONE) {
> + if (old == IOMMU_MR_EVENT_NONE && new != IOMMU_MR_EVENT_NONE) {
> spapr_tce_set_need_vfio(tbl, true);
> - } else if (old != IOMMU_NOTIFIER_NONE && new == IOMMU_NOTIFIER_NONE) {
> + } else if (old != IOMMU_MR_EVENT_NONE && new == IOMMU_MR_EVENT_NONE) {
> spapr_tce_set_need_vfio(tbl, false);
> }
> }
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 6b33b9f..a6ca10b 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -482,7 +482,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
> section->size);
> llend = int128_sub(llend, int128_one());
> iommu_notifier_init(&giommu->n, vfio_iommu_map_notify,
> - IOMMU_NOTIFIER_ALL,
> + IOMMU_MR_EVENT_ALL,
> section->offset_within_region,
> int128_get64(llend));
> QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next);
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 0001e60..2e8d8fc 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -747,7 +747,7 @@ static void vhost_iommu_region_add(MemoryListener *listener,
> section->size);
> end = int128_sub(end, int128_one());
> iommu_notifier_init(&iommu->n, vhost_iommu_unmap_notify,
> - IOMMU_NOTIFIER_UNMAP,
> + IOMMU_MR_EVENT_UNMAP,
> section->offset_within_region,
> int128_get64(end));
> iommu->mr = section->mr;
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 99e0f54..90730b1 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -70,14 +70,14 @@ struct IOMMUTLBEntry {
> * register with one or multiple IOMMU Notifier capability bit(s).
> */
> typedef enum {
> - IOMMU_NOTIFIER_NONE = 0,
> + IOMMU_MR_EVENT_NONE = 0,
> /* Notify cache invalidations */
> - IOMMU_NOTIFIER_UNMAP = 0x1,
> + IOMMU_MR_EVENT_UNMAP = 0x1,
> /* Notify entry changes (newly created entries) */
> - IOMMU_NOTIFIER_MAP = 0x2,
> -} IOMMUNotifierFlag;
> + IOMMU_MR_EVENT_MAP = 0x2,
> +} IOMMUMREventFlags;
>
> -#define IOMMU_NOTIFIER_ALL (IOMMU_NOTIFIER_MAP | IOMMU_NOTIFIER_UNMAP)
> +#define IOMMU_MR_EVENT_ALL (IOMMU_MR_EVENT_MAP | IOMMU_MR_EVENT_UNMAP)
>
> struct IOMMUNotifier;
> typedef void (*IOMMUNotify)(struct IOMMUNotifier *notifier,
> @@ -85,7 +85,7 @@ typedef void (*IOMMUNotify)(struct IOMMUNotifier *notifier,
>
> struct IOMMUNotifier {
> IOMMUNotify notify;
> - IOMMUNotifierFlag notifier_flags;
> + IOMMUMREventFlags notifier_flags;
> /* Notify for address space range start <= addr <= end */
> hwaddr start;
> hwaddr end;
> @@ -94,7 +94,7 @@ struct IOMMUNotifier {
> typedef struct IOMMUNotifier IOMMUNotifier;
>
> static inline void iommu_notifier_init(IOMMUNotifier *n, IOMMUNotify fn,
> - IOMMUNotifierFlag flags,
> + IOMMUMREventFlags flags,
> hwaddr start, hwaddr end)
> {
> n->notify = fn;
> @@ -191,8 +191,8 @@ struct MemoryRegionIOMMUOps {
> uint64_t (*get_min_page_size)(MemoryRegion *iommu);
> /* Called when IOMMU Notifier flag changed */
> void (*notify_flag_changed)(MemoryRegion *iommu,
> - IOMMUNotifierFlag old_flags,
> - IOMMUNotifierFlag new_flags);
> + IOMMUMREventFlags old_flags,
> + IOMMUMREventFlags new_flags);
> /* Set this up to provide customized IOMMU replay function */
> void (*replay)(MemoryRegion *iommu, IOMMUNotifier *notifier);
> };
> @@ -240,7 +240,7 @@ struct MemoryRegion {
> unsigned ioeventfd_nb;
> MemoryRegionIoeventfd *ioeventfds;
> QLIST_HEAD(, IOMMUNotifier) iommu_notify;
> - IOMMUNotifierFlag iommu_notify_flags;
> + IOMMUMREventFlags iommu_notify_flags;
> };
>
> #define IOMMU_NOTIFIER_FOREACH(n, mr) \
> diff --git a/memory.c b/memory.c
> index b727f5e..6237631 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1483,7 +1483,7 @@ void memory_region_init_iommu(MemoryRegion *mr,
> mr->iommu_ops = ops,
> mr->terminates = true; /* then re-forwards */
> QLIST_INIT(&mr->iommu_notify);
> - mr->iommu_notify_flags = IOMMU_NOTIFIER_NONE;
> + mr->iommu_notify_flags = IOMMU_MR_EVENT_NONE;
> }
>
> static void memory_region_finalize(Object *obj)
> @@ -1580,7 +1580,7 @@ bool memory_region_is_logging(MemoryRegion *mr, uint8_t client)
>
> static void memory_region_update_iommu_notify_flags(MemoryRegion *mr)
> {
> - IOMMUNotifierFlag flags = IOMMU_NOTIFIER_NONE;
> + IOMMUMREventFlags flags = IOMMU_MR_EVENT_NONE;
> IOMMUNotifier *iommu_notifier;
>
> IOMMU_NOTIFIER_FOREACH(iommu_notifier, mr) {
> @@ -1605,7 +1605,7 @@ void memory_region_register_iommu_notifier(MemoryRegion *mr,
> }
>
> /* We need to register for at least one bitfield */
> - assert(n->notifier_flags != IOMMU_NOTIFIER_NONE);
> + assert(n->notifier_flags != IOMMU_MR_EVENT_NONE);
> assert(n->start <= n->end);
> QLIST_INSERT_HEAD(&mr->iommu_notify, n, node);
> memory_region_update_iommu_notify_flags(mr);
> @@ -1671,7 +1671,7 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr,
> void memory_region_notify_one(IOMMUNotifier *notifier,
> IOMMUTLBEntry *entry)
> {
> - IOMMUNotifierFlag request_flags;
> + IOMMUMREventFlags request_flags;
>
> /*
> * Skip the notification if the notification does not overlap
> @@ -1683,9 +1683,9 @@ void memory_region_notify_one(IOMMUNotifier *notifier,
> }
>
> if (entry->perm & IOMMU_RW) {
> - request_flags = IOMMU_NOTIFIER_MAP;
> + request_flags = IOMMU_MR_EVENT_MAP;
> } else {
> - request_flags = IOMMU_NOTIFIER_UNMAP;
> + request_flags = IOMMU_MR_EVENT_UNMAP;
> }
>
> if (notifier->notifier_flags & request_flags) {
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 2/8] memory: rename IOMMUNotifier
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 2/8] memory: rename IOMMUNotifier Peter Xu
@ 2017-05-01 4:51 ` David Gibson
0 siblings, 0 replies; 30+ messages in thread
From: David Gibson @ 2017-05-01 4:51 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu,
Jason Wang, Alex Williamson
[-- Attachment #1: Type: text/plain, Size: 15002 bytes --]
On Thu, Apr 27, 2017 at 05:34:14PM +0800, Peter Xu wrote:
> Renaming it to IOMMUMRNotifier. This is a corresponding change to
> previous patch, to emphasize that these notifiers are based on memory
> regions.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
I think this patch could be folded with the previous one.
Reviwed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/i386/intel_iommu.c | 20 ++++++++++----------
> hw/vfio/common.c | 2 +-
> hw/virtio/vhost.c | 2 +-
> include/exec/memory.h | 28 ++++++++++++++--------------
> include/hw/i386/intel_iommu.h | 8 ++++----
> include/hw/vfio/vfio-common.h | 2 +-
> include/hw/virtio/vhost.h | 4 ++--
> memory.c | 14 +++++++-------
> 8 files changed, 40 insertions(+), 40 deletions(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index dce1ee8..3306e6a 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -1041,7 +1041,7 @@ static void vtd_interrupt_remap_table_setup(IntelIOMMUState *s)
>
> static void vtd_iommu_replay_all(IntelIOMMUState *s)
> {
> - IntelIOMMUNotifierNode *node;
> + IntelIOMMUMRNotifierNode *node;
>
> QLIST_FOREACH(node, &s->notifiers_list, next) {
> memory_region_iommu_replay_all(&node->vtd_as->iommu);
> @@ -1185,7 +1185,7 @@ static void vtd_iotlb_global_invalidate(IntelIOMMUState *s)
>
> static void vtd_iotlb_domain_invalidate(IntelIOMMUState *s, uint16_t domain_id)
> {
> - IntelIOMMUNotifierNode *node;
> + IntelIOMMUMRNotifierNode *node;
> VTDContextEntry ce;
> VTDAddressSpace *vtd_as;
>
> @@ -1213,7 +1213,7 @@ static void vtd_iotlb_page_invalidate_notify(IntelIOMMUState *s,
> uint16_t domain_id, hwaddr addr,
> uint8_t am)
> {
> - IntelIOMMUNotifierNode *node;
> + IntelIOMMUMRNotifierNode *node;
> VTDContextEntry ce;
> int ret;
>
> @@ -2258,8 +2258,8 @@ static void vtd_iommu_notify_flag_changed(MemoryRegion *iommu,
> {
> VTDAddressSpace *vtd_as = container_of(iommu, VTDAddressSpace, iommu);
> IntelIOMMUState *s = vtd_as->iommu_state;
> - IntelIOMMUNotifierNode *node = NULL;
> - IntelIOMMUNotifierNode *next_node = NULL;
> + IntelIOMMUMRNotifierNode *node = NULL;
> + IntelIOMMUMRNotifierNode *next_node = NULL;
>
> if (!s->caching_mode && new & IOMMU_MR_EVENT_MAP) {
> error_report("We need to set cache_mode=1 for intel-iommu to enable "
> @@ -2702,7 +2702,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
> }
>
> /* Unmap the whole range in the notifier's scope. */
> -static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n)
> +static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUMRNotifier *n)
> {
> IOMMUTLBEntry entry;
> hwaddr size;
> @@ -2757,9 +2757,9 @@ static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n)
>
> static void vtd_address_space_unmap_all(IntelIOMMUState *s)
> {
> - IntelIOMMUNotifierNode *node;
> + IntelIOMMUMRNotifierNode *node;
> VTDAddressSpace *vtd_as;
> - IOMMUNotifier *n;
> + IOMMUMRNotifier *n;
>
> QLIST_FOREACH(node, &s->notifiers_list, next) {
> vtd_as = node->vtd_as;
> @@ -2771,11 +2771,11 @@ static void vtd_address_space_unmap_all(IntelIOMMUState *s)
>
> static int vtd_replay_hook(IOMMUTLBEntry *entry, void *private)
> {
> - memory_region_notify_one((IOMMUNotifier *)private, entry);
> + memory_region_notify_one((IOMMUMRNotifier *)private, entry);
> return 0;
> }
>
> -static void vtd_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n)
> +static void vtd_iommu_replay(MemoryRegion *mr, IOMMUMRNotifier *n)
> {
> VTDAddressSpace *vtd_as = container_of(mr, VTDAddressSpace, iommu);
> IntelIOMMUState *s = vtd_as->iommu_state;
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index a6ca10b..bd113b7 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -332,7 +332,7 @@ static bool vfio_get_vaddr(IOMMUTLBEntry *iotlb, void **vaddr,
> return true;
> }
>
> -static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
> +static void vfio_iommu_map_notify(IOMMUMRNotifier *n, IOMMUTLBEntry *iotlb)
> {
> VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
> VFIOContainer *container = giommu->container;
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 2e8d8fc..81a8aea 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -718,7 +718,7 @@ static void vhost_region_del(MemoryListener *listener,
> }
> }
>
> -static void vhost_iommu_unmap_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
> +static void vhost_iommu_unmap_notify(IOMMUMRNotifier *n, IOMMUTLBEntry *iotlb)
> {
> struct vhost_iommu *iommu = container_of(n, struct vhost_iommu, n);
> struct vhost_dev *hdev = iommu->hdev;
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 90730b1..6be0c02 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -66,7 +66,7 @@ struct IOMMUTLBEntry {
> };
>
> /*
> - * Bitmap for different IOMMUNotifier capabilities. Each notifier can
> + * Bitmap for different IOMMUMRNotifier capabilities. Each notifier can
> * register with one or multiple IOMMU Notifier capability bit(s).
> */
> typedef enum {
> @@ -79,21 +79,21 @@ typedef enum {
>
> #define IOMMU_MR_EVENT_ALL (IOMMU_MR_EVENT_MAP | IOMMU_MR_EVENT_UNMAP)
>
> -struct IOMMUNotifier;
> -typedef void (*IOMMUNotify)(struct IOMMUNotifier *notifier,
> +struct IOMMUMRNotifier;
> +typedef void (*IOMMUNotify)(struct IOMMUMRNotifier *notifier,
> IOMMUTLBEntry *data);
>
> -struct IOMMUNotifier {
> +struct IOMMUMRNotifier {
> IOMMUNotify notify;
> IOMMUMREventFlags notifier_flags;
> /* Notify for address space range start <= addr <= end */
> hwaddr start;
> hwaddr end;
> - QLIST_ENTRY(IOMMUNotifier) node;
> + QLIST_ENTRY(IOMMUMRNotifier) node;
> };
> -typedef struct IOMMUNotifier IOMMUNotifier;
> +typedef struct IOMMUMRNotifier IOMMUMRNotifier;
>
> -static inline void iommu_notifier_init(IOMMUNotifier *n, IOMMUNotify fn,
> +static inline void iommu_notifier_init(IOMMUMRNotifier *n, IOMMUNotify fn,
> IOMMUMREventFlags flags,
> hwaddr start, hwaddr end)
> {
> @@ -194,7 +194,7 @@ struct MemoryRegionIOMMUOps {
> IOMMUMREventFlags old_flags,
> IOMMUMREventFlags new_flags);
> /* Set this up to provide customized IOMMU replay function */
> - void (*replay)(MemoryRegion *iommu, IOMMUNotifier *notifier);
> + void (*replay)(MemoryRegion *iommu, IOMMUMRNotifier *notifier);
> };
>
> typedef struct CoalescedMemoryRange CoalescedMemoryRange;
> @@ -239,7 +239,7 @@ struct MemoryRegion {
> const char *name;
> unsigned ioeventfd_nb;
> MemoryRegionIoeventfd *ioeventfds;
> - QLIST_HEAD(, IOMMUNotifier) iommu_notify;
> + QLIST_HEAD(, IOMMUMRNotifier) iommu_notify;
> IOMMUMREventFlags iommu_notify_flags;
> };
>
> @@ -703,7 +703,7 @@ void memory_region_notify_iommu(MemoryRegion *mr,
> * replaces all old entries for the same virtual I/O address range.
> * Deleted entries have .@perm == 0.
> */
> -void memory_region_notify_one(IOMMUNotifier *notifier,
> +void memory_region_notify_one(IOMMUMRNotifier *notifier,
> IOMMUTLBEntry *entry);
>
> /**
> @@ -711,12 +711,12 @@ void memory_region_notify_one(IOMMUNotifier *notifier,
> * IOMMU translation entries.
> *
> * @mr: the memory region to observe
> - * @n: the IOMMUNotifier to be added; the notify callback receives a
> + * @n: the IOMMUMRNotifier to be added; the notify callback receives a
> * pointer to an #IOMMUTLBEntry as the opaque value; the pointer
> * ceases to be valid on exit from the notifier.
> */
> void memory_region_register_iommu_notifier(MemoryRegion *mr,
> - IOMMUNotifier *n);
> + IOMMUMRNotifier *n);
>
> /**
> * memory_region_iommu_replay: replay existing IOMMU translations to
> @@ -728,7 +728,7 @@ void memory_region_register_iommu_notifier(MemoryRegion *mr,
> * @is_write: Whether to treat the replay as a translate "write"
> * through the iommu
> */
> -void memory_region_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n,
> +void memory_region_iommu_replay(MemoryRegion *mr, IOMMUMRNotifier *n,
> bool is_write);
>
> /**
> @@ -748,7 +748,7 @@ void memory_region_iommu_replay_all(MemoryRegion *mr);
> * @n: the notifier to be removed.
> */
> void memory_region_unregister_iommu_notifier(MemoryRegion *mr,
> - IOMMUNotifier *n);
> + IOMMUMRNotifier *n);
>
> /**
> * memory_region_name: get a memory region's name
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index 3e51876..f9ac0ec 100644
> --- a/include/hw/i386/intel_iommu.h
> +++ b/include/hw/i386/intel_iommu.h
> @@ -63,7 +63,7 @@ typedef union VTD_IR_TableEntry VTD_IR_TableEntry;
> typedef union VTD_IR_MSIAddress VTD_IR_MSIAddress;
> typedef struct VTDIrq VTDIrq;
> typedef struct VTD_MSIMessage VTD_MSIMessage;
> -typedef struct IntelIOMMUNotifierNode IntelIOMMUNotifierNode;
> +typedef struct IntelIOMMUMRNotifierNode IntelIOMMUMRNotifierNode;
>
> /* Context-Entry */
> struct VTDContextEntry {
> @@ -250,9 +250,9 @@ struct VTD_MSIMessage {
> /* When IR is enabled, all MSI/MSI-X data bits should be zero */
> #define VTD_IR_MSI_DATA (0)
>
> -struct IntelIOMMUNotifierNode {
> +struct IntelIOMMUMRNotifierNode {
> VTDAddressSpace *vtd_as;
> - QLIST_ENTRY(IntelIOMMUNotifierNode) next;
> + QLIST_ENTRY(IntelIOMMUMRNotifierNode) next;
> };
>
> /* The iommu (DMAR) device state struct */
> @@ -293,7 +293,7 @@ struct IntelIOMMUState {
> GHashTable *vtd_as_by_busptr; /* VTDBus objects indexed by PCIBus* reference */
> VTDBus *vtd_as_by_bus_num[VTD_PCI_BUS_MAX]; /* VTDBus objects indexed by bus number */
> /* list of registered notifiers */
> - QLIST_HEAD(, IntelIOMMUNotifierNode) notifiers_list;
> + QLIST_HEAD(, IntelIOMMUMRNotifierNode) notifiers_list;
>
> /* interrupt remapping */
> bool intr_enabled; /* Whether guest enabled IR */
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index c582de1..348d408 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -96,7 +96,7 @@ typedef struct VFIOGuestIOMMU {
> VFIOContainer *container;
> MemoryRegion *iommu;
> hwaddr iommu_offset;
> - IOMMUNotifier n;
> + IOMMUMRNotifier n;
> QLIST_ENTRY(VFIOGuestIOMMU) giommu_next;
> } VFIOGuestIOMMU;
>
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index a450321..091cfad 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -42,7 +42,7 @@ struct vhost_iommu {
> struct vhost_dev *hdev;
> MemoryRegion *mr;
> hwaddr iommu_offset;
> - IOMMUNotifier n;
> + IOMMUMRNotifier n;
> QLIST_ENTRY(vhost_iommu) iommu_next;
> };
>
> @@ -75,7 +75,7 @@ struct vhost_dev {
> struct vhost_log *log;
> QLIST_ENTRY(vhost_dev) entry;
> QLIST_HEAD(, vhost_iommu) iommu_list;
> - IOMMUNotifier n;
> + IOMMUMRNotifier n;
> };
>
> int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> diff --git a/memory.c b/memory.c
> index 6237631..ffbd020 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1581,7 +1581,7 @@ bool memory_region_is_logging(MemoryRegion *mr, uint8_t client)
> static void memory_region_update_iommu_notify_flags(MemoryRegion *mr)
> {
> IOMMUMREventFlags flags = IOMMU_MR_EVENT_NONE;
> - IOMMUNotifier *iommu_notifier;
> + IOMMUMRNotifier *iommu_notifier;
>
> IOMMU_NOTIFIER_FOREACH(iommu_notifier, mr) {
> flags |= iommu_notifier->notifier_flags;
> @@ -1597,7 +1597,7 @@ static void memory_region_update_iommu_notify_flags(MemoryRegion *mr)
> }
>
> void memory_region_register_iommu_notifier(MemoryRegion *mr,
> - IOMMUNotifier *n)
> + IOMMUMRNotifier *n)
> {
> if (mr->alias) {
> memory_region_register_iommu_notifier(mr->alias, n);
> @@ -1620,7 +1620,7 @@ uint64_t memory_region_iommu_get_min_page_size(MemoryRegion *mr)
> return TARGET_PAGE_SIZE;
> }
>
> -void memory_region_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n,
> +void memory_region_iommu_replay(MemoryRegion *mr, IOMMUMRNotifier *n,
> bool is_write)
> {
> hwaddr addr, granularity;
> @@ -1650,7 +1650,7 @@ void memory_region_iommu_replay(MemoryRegion *mr, IOMMUNotifier *n,
>
> void memory_region_iommu_replay_all(MemoryRegion *mr)
> {
> - IOMMUNotifier *notifier;
> + IOMMUMRNotifier *notifier;
>
> IOMMU_NOTIFIER_FOREACH(notifier, mr) {
> memory_region_iommu_replay(mr, notifier, false);
> @@ -1658,7 +1658,7 @@ void memory_region_iommu_replay_all(MemoryRegion *mr)
> }
>
> void memory_region_unregister_iommu_notifier(MemoryRegion *mr,
> - IOMMUNotifier *n)
> + IOMMUMRNotifier *n)
> {
> if (mr->alias) {
> memory_region_unregister_iommu_notifier(mr->alias, n);
> @@ -1668,7 +1668,7 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr,
> memory_region_update_iommu_notify_flags(mr);
> }
>
> -void memory_region_notify_one(IOMMUNotifier *notifier,
> +void memory_region_notify_one(IOMMUMRNotifier *notifier,
> IOMMUTLBEntry *entry)
> {
> IOMMUMREventFlags request_flags;
> @@ -1696,7 +1696,7 @@ void memory_region_notify_one(IOMMUNotifier *notifier,
> void memory_region_notify_iommu(MemoryRegion *mr,
> IOMMUTLBEntry entry)
> {
> - IOMMUNotifier *iommu_notifier;
> + IOMMUMRNotifier *iommu_notifier;
>
> assert(memory_region_is_iommu(mr));
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 3/8] memory: rename iommu_notifier_init()
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 3/8] memory: rename iommu_notifier_init() Peter Xu
@ 2017-05-01 4:53 ` David Gibson
2017-05-08 5:50 ` Peter Xu
0 siblings, 1 reply; 30+ messages in thread
From: David Gibson @ 2017-05-01 4:53 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu,
Jason Wang, Alex Williamson
[-- Attachment #1: Type: text/plain, Size: 3760 bytes --]
On Thu, Apr 27, 2017 at 05:34:15PM +0800, Peter Xu wrote:
> It's new name is iommu_mr_notifier_init(). Again, literal changes only.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
Again, I think this could be folded with the previous two patches.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
I also note that these patches will conflict with Alexey's proposed
changes to make IOMMU MR's a QOM subtype of the ordinary MR (thus
removing some of the MR specific fields from other MRs.
> ---
> hw/vfio/common.c | 8 ++++----
> hw/virtio/vhost.c | 8 ++++----
> include/exec/memory.h | 7 ++++---
> 3 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index bd113b7..6b3953b 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -481,10 +481,10 @@ static void vfio_listener_region_add(MemoryListener *listener,
> llend = int128_add(int128_make64(section->offset_within_region),
> section->size);
> llend = int128_sub(llend, int128_one());
> - iommu_notifier_init(&giommu->n, vfio_iommu_map_notify,
> - IOMMU_MR_EVENT_ALL,
> - section->offset_within_region,
> - int128_get64(llend));
> + iommu_mr_notifier_init(&giommu->n, vfio_iommu_map_notify,
> + IOMMU_MR_EVENT_ALL,
> + section->offset_within_region,
> + int128_get64(llend));
> QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next);
>
> memory_region_register_iommu_notifier(giommu->iommu, &giommu->n);
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 81a8aea..7449cf8 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -746,10 +746,10 @@ static void vhost_iommu_region_add(MemoryListener *listener,
> end = int128_add(int128_make64(section->offset_within_region),
> section->size);
> end = int128_sub(end, int128_one());
> - iommu_notifier_init(&iommu->n, vhost_iommu_unmap_notify,
> - IOMMU_MR_EVENT_UNMAP,
> - section->offset_within_region,
> - int128_get64(end));
> + iommu_mr_notifier_init(&iommu->n, vhost_iommu_unmap_notify,
> + IOMMU_MR_EVENT_UNMAP,
> + section->offset_within_region,
> + int128_get64(end));
> iommu->mr = section->mr;
> iommu->iommu_offset = section->offset_within_address_space -
> section->offset_within_region;
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 6be0c02..8d8dcb2 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -93,9 +93,10 @@ struct IOMMUMRNotifier {
> };
> typedef struct IOMMUMRNotifier IOMMUMRNotifier;
>
> -static inline void iommu_notifier_init(IOMMUMRNotifier *n, IOMMUNotify fn,
> - IOMMUMREventFlags flags,
> - hwaddr start, hwaddr end)
> +static inline void iommu_mr_notifier_init(IOMMUMRNotifier *n,
> + IOMMUNotify fn,
> + IOMMUMREventFlags flags,
> + hwaddr start, hwaddr end)
> {
> n->notify = fn;
> n->notifier_flags = flags;
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 4/8] memory: rename *_notify_iommu*
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 4/8] memory: rename *_notify_iommu* Peter Xu
@ 2017-05-01 4:55 ` David Gibson
0 siblings, 0 replies; 30+ messages in thread
From: David Gibson @ 2017-05-01 4:55 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu,
Jason Wang, Alex Williamson
[-- Attachment #1: Type: text/plain, Size: 6775 bytes --]
On Thu, Apr 27, 2017 at 05:34:16PM +0800, Peter Xu wrote:
> Actually it's notifying IOTLB updates (map, or unmap). Let's be explicit
> on the wording - replacing it with *_notify_iotlb*.
I really don't see the distinction here. This is notifying of a
change in IOMMU mappings. We use the IOTLBEntry data type to
represent a single mapping. There need not be an actual IOTLB (in the
sense of a cache backed by a different data structure) in either
hardware or software here.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> hw/i386/intel_iommu.c | 8 ++++----
> hw/ppc/spapr_iommu.c | 2 +-
> hw/s390x/s390-pci-inst.c | 2 +-
> include/exec/memory.h | 12 ++++++------
> memory.c | 8 ++++----
> 5 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 3306e6a..609732b 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -1205,7 +1205,7 @@ static void vtd_iotlb_domain_invalidate(IntelIOMMUState *s, uint16_t domain_id)
> static int vtd_page_invalidate_notify_hook(IOMMUTLBEntry *entry,
> void *private)
> {
> - memory_region_notify_iommu((MemoryRegion *)private, *entry);
> + memory_region_notify_iotlb((MemoryRegion *)private, *entry);
> return 0;
> }
>
> @@ -1709,7 +1709,7 @@ static bool vtd_process_device_iotlb_desc(IntelIOMMUState *s,
> entry.iova = addr;
> entry.perm = IOMMU_NONE;
> entry.translated_addr = 0;
> - memory_region_notify_iommu(&vtd_dev_as->iommu, entry);
> + memory_region_notify_iotlb(&vtd_dev_as->iommu, entry);
>
> done:
> return true;
> @@ -2752,7 +2752,7 @@ static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUMRNotifier *n)
> VTD_PCI_FUNC(as->devfn),
> entry.iova, size);
>
> - memory_region_notify_one(n, &entry);
> + memory_region_notify_iotlb_one(n, &entry);
> }
>
> static void vtd_address_space_unmap_all(IntelIOMMUState *s)
> @@ -2771,7 +2771,7 @@ static void vtd_address_space_unmap_all(IntelIOMMUState *s)
>
> static int vtd_replay_hook(IOMMUTLBEntry *entry, void *private)
> {
> - memory_region_notify_one((IOMMUMRNotifier *)private, entry);
> + memory_region_notify_iotlb_one((IOMMUMRNotifier *)private, entry);
> return 0;
> }
>
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index 5e6e70b..9a34495 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -414,7 +414,7 @@ static target_ulong put_tce_emu(sPAPRTCETable *tcet, target_ulong ioba,
> entry.translated_addr = tce & page_mask;
> entry.addr_mask = ~page_mask;
> entry.perm = spapr_tce_iommu_access_flags(tce);
> - memory_region_notify_iommu(&tcet->iommu, entry);
> + memory_region_notify_iotlb(&tcet->iommu, entry);
>
> return H_SUCCESS;
> }
> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
> index 314a9cb..566eeda 100644
> --- a/hw/s390x/s390-pci-inst.c
> +++ b/hw/s390x/s390-pci-inst.c
> @@ -635,7 +635,7 @@ int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
> goto out;
> }
>
> - memory_region_notify_iommu(mr, entry);
> + memory_region_notify_iotlb(mr, entry);
> start += entry.addr_mask + 1;
> }
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 8d8dcb2..09188a6 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -673,7 +673,7 @@ static inline bool memory_region_is_iommu(MemoryRegion *mr)
> uint64_t memory_region_iommu_get_min_page_size(MemoryRegion *mr);
>
> /**
> - * memory_region_notify_iommu: notify a change in an IOMMU translation entry.
> + * memory_region_notify_iotlb: notify a change in an IOMMU translation entry.
> *
> * The notification type will be decided by entry.perm bits:
> *
> @@ -689,12 +689,12 @@ uint64_t memory_region_iommu_get_min_page_size(MemoryRegion *mr);
> * replaces all old entries for the same virtual I/O address range.
> * Deleted entries have .@perm == 0.
> */
> -void memory_region_notify_iommu(MemoryRegion *mr,
> +void memory_region_notify_iotlb(MemoryRegion *mr,
> IOMMUTLBEntry entry);
>
> /**
> - * memory_region_notify_one: notify a change in an IOMMU translation
> - * entry to a single notifier
> + * memory_region_notify_iotlb_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.
> @@ -704,8 +704,8 @@ void memory_region_notify_iommu(MemoryRegion *mr,
> * replaces all old entries for the same virtual I/O address range.
> * Deleted entries have .@perm == 0.
> */
> -void memory_region_notify_one(IOMMUMRNotifier *notifier,
> - IOMMUTLBEntry *entry);
> +void memory_region_notify_iotlb_one(IOMMUMRNotifier *notifier,
> + IOMMUTLBEntry *entry);
>
> /**
> * memory_region_register_iommu_notifier: register a notifier for changes to
> diff --git a/memory.c b/memory.c
> index ffbd020..c6532e4 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1668,8 +1668,8 @@ void memory_region_unregister_iommu_notifier(MemoryRegion *mr,
> memory_region_update_iommu_notify_flags(mr);
> }
>
> -void memory_region_notify_one(IOMMUMRNotifier *notifier,
> - IOMMUTLBEntry *entry)
> +void memory_region_notify_iotlb_one(IOMMUMRNotifier *notifier,
> + IOMMUTLBEntry *entry)
> {
> IOMMUMREventFlags request_flags;
>
> @@ -1693,7 +1693,7 @@ void memory_region_notify_one(IOMMUMRNotifier *notifier,
> }
> }
>
> -void memory_region_notify_iommu(MemoryRegion *mr,
> +void memory_region_notify_iotlb(MemoryRegion *mr,
> IOMMUTLBEntry entry)
> {
> IOMMUMRNotifier *iommu_notifier;
> @@ -1701,7 +1701,7 @@ void memory_region_notify_iommu(MemoryRegion *mr,
> assert(memory_region_is_iommu(mr));
>
> IOMMU_NOTIFIER_FOREACH(iommu_notifier, mr) {
> - memory_region_notify_one(iommu_notifier, &entry);
> + memory_region_notify_iotlb_one(iommu_notifier, &entry);
> }
> }
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 5/8] memory: rename *iommu_notifier*
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 5/8] memory: rename *iommu_notifier* Peter Xu
@ 2017-05-01 4:56 ` David Gibson
0 siblings, 0 replies; 30+ messages in thread
From: David Gibson @ 2017-05-01 4:56 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu,
Jason Wang, Alex Williamson
[-- Attachment #1: Type: text/plain, Size: 7953 bytes --]
On Thu, Apr 27, 2017 at 05:34:17PM +0800, Peter Xu wrote:
> Renaming *iommu_notifiers* into *iotlb_notifiers*. Again, let's reserve
> the iommu_notifier keyword to the notifiers that will be for per-iommu,
> and let the old per-mr notifier be iotlb_notifiers.
As with the previous patch, I really don't see that "iotlb" is an
clearer than "iommu" here. This is notifying of changes in mapping on
an IOMMU region. There may or may not be an IOTLB of some sort
involved.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> hw/vfio/common.c | 6 +++---
> hw/virtio/vhost.c | 4 ++--
> include/exec/memory.h | 8 ++++----
> memory.c | 20 ++++++++++----------
> 4 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 6b3953b..7b639ea 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -487,7 +487,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
> int128_get64(llend));
> QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next);
>
> - memory_region_register_iommu_notifier(giommu->iommu, &giommu->n);
> + memory_region_register_iotlb_notifier(giommu->iommu, &giommu->n);
> memory_region_iommu_replay(giommu->iommu, &giommu->n, false);
>
> return;
> @@ -557,7 +557,7 @@ static void vfio_listener_region_del(MemoryListener *listener,
> QLIST_FOREACH(giommu, &container->giommu_list, giommu_next) {
> if (giommu->iommu == section->mr &&
> giommu->n.start == section->offset_within_region) {
> - memory_region_unregister_iommu_notifier(giommu->iommu,
> + memory_region_unregister_iotlb_notifier(giommu->iommu,
> &giommu->n);
> QLIST_REMOVE(giommu, giommu_next);
> g_free(giommu);
> @@ -1147,7 +1147,7 @@ static void vfio_disconnect_container(VFIOGroup *group)
> QLIST_REMOVE(container, next);
>
> QLIST_FOREACH_SAFE(giommu, &container->giommu_list, giommu_next, tmp) {
> - memory_region_unregister_iommu_notifier(giommu->iommu, &giommu->n);
> + memory_region_unregister_iotlb_notifier(giommu->iommu, &giommu->n);
> QLIST_REMOVE(giommu, giommu_next);
> g_free(giommu);
> }
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 7449cf8..a1e03ed 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -754,7 +754,7 @@ static void vhost_iommu_region_add(MemoryListener *listener,
> iommu->iommu_offset = section->offset_within_address_space -
> section->offset_within_region;
> iommu->hdev = dev;
> - memory_region_register_iommu_notifier(section->mr, &iommu->n);
> + memory_region_register_iotlb_notifier(section->mr, &iommu->n);
> QLIST_INSERT_HEAD(&dev->iommu_list, iommu, iommu_next);
> /* TODO: can replay help performance here? */
> }
> @@ -773,7 +773,7 @@ static void vhost_iommu_region_del(MemoryListener *listener,
> QLIST_FOREACH(iommu, &dev->iommu_list, iommu_next) {
> if (iommu->mr == section->mr &&
> iommu->n.start == section->offset_within_region) {
> - memory_region_unregister_iommu_notifier(iommu->mr,
> + memory_region_unregister_iotlb_notifier(iommu->mr,
> &iommu->n);
> QLIST_REMOVE(iommu, iommu_next);
> g_free(iommu);
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 09188a6..e5707b3 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -708,7 +708,7 @@ void memory_region_notify_iotlb_one(IOMMUMRNotifier *notifier,
> IOMMUTLBEntry *entry);
>
> /**
> - * memory_region_register_iommu_notifier: register a notifier for changes to
> + * memory_region_register_iotlb_notifier: register a notifier for changes to
> * IOMMU translation entries.
> *
> * @mr: the memory region to observe
> @@ -716,7 +716,7 @@ void memory_region_notify_iotlb_one(IOMMUMRNotifier *notifier,
> * pointer to an #IOMMUTLBEntry as the opaque value; the pointer
> * ceases to be valid on exit from the notifier.
> */
> -void memory_region_register_iommu_notifier(MemoryRegion *mr,
> +void memory_region_register_iotlb_notifier(MemoryRegion *mr,
> IOMMUMRNotifier *n);
>
> /**
> @@ -741,14 +741,14 @@ void memory_region_iommu_replay(MemoryRegion *mr, IOMMUMRNotifier *n,
> void memory_region_iommu_replay_all(MemoryRegion *mr);
>
> /**
> - * memory_region_unregister_iommu_notifier: unregister a notifier for
> + * memory_region_unregister_iotlb_notifier: unregister a notifier for
> * changes to IOMMU translation entries.
> *
> * @mr: the memory region which was observed and for which notity_stopped()
> * needs to be called
> * @n: the notifier to be removed.
> */
> -void memory_region_unregister_iommu_notifier(MemoryRegion *mr,
> +void memory_region_unregister_iotlb_notifier(MemoryRegion *mr,
> IOMMUMRNotifier *n);
>
> /**
> diff --git a/memory.c b/memory.c
> index c6532e4..6af523e 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1581,10 +1581,10 @@ bool memory_region_is_logging(MemoryRegion *mr, uint8_t client)
> static void memory_region_update_iommu_notify_flags(MemoryRegion *mr)
> {
> IOMMUMREventFlags flags = IOMMU_MR_EVENT_NONE;
> - IOMMUMRNotifier *iommu_notifier;
> + IOMMUMRNotifier *iotlb_notifier;
>
> - IOMMU_NOTIFIER_FOREACH(iommu_notifier, mr) {
> - flags |= iommu_notifier->notifier_flags;
> + IOMMU_NOTIFIER_FOREACH(iotlb_notifier, mr) {
> + flags |= iotlb_notifier->notifier_flags;
> }
>
> if (flags != mr->iommu_notify_flags &&
> @@ -1596,11 +1596,11 @@ static void memory_region_update_iommu_notify_flags(MemoryRegion *mr)
> mr->iommu_notify_flags = flags;
> }
>
> -void memory_region_register_iommu_notifier(MemoryRegion *mr,
> +void memory_region_register_iotlb_notifier(MemoryRegion *mr,
> IOMMUMRNotifier *n)
> {
> if (mr->alias) {
> - memory_region_register_iommu_notifier(mr->alias, n);
> + memory_region_register_iotlb_notifier(mr->alias, n);
> return;
> }
>
> @@ -1657,11 +1657,11 @@ void memory_region_iommu_replay_all(MemoryRegion *mr)
> }
> }
>
> -void memory_region_unregister_iommu_notifier(MemoryRegion *mr,
> +void memory_region_unregister_iotlb_notifier(MemoryRegion *mr,
> IOMMUMRNotifier *n)
> {
> if (mr->alias) {
> - memory_region_unregister_iommu_notifier(mr->alias, n);
> + memory_region_unregister_iotlb_notifier(mr->alias, n);
> return;
> }
> QLIST_REMOVE(n, node);
> @@ -1696,12 +1696,12 @@ void memory_region_notify_iotlb_one(IOMMUMRNotifier *notifier,
> void memory_region_notify_iotlb(MemoryRegion *mr,
> IOMMUTLBEntry entry)
> {
> - IOMMUMRNotifier *iommu_notifier;
> + IOMMUMRNotifier *iotlb_notifier;
>
> assert(memory_region_is_iommu(mr));
>
> - IOMMU_NOTIFIER_FOREACH(iommu_notifier, mr) {
> - memory_region_notify_iotlb_one(iommu_notifier, &entry);
> + IOMMU_NOTIFIER_FOREACH(iotlb_notifier, mr) {
> + memory_region_notify_iotlb_one(iotlb_notifier, &entry);
> }
> }
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 6/8] memory: introduce AddressSpaceOps
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 6/8] memory: introduce AddressSpaceOps Peter Xu
@ 2017-05-01 4:58 ` David Gibson
2017-05-08 5:48 ` Peter Xu
0 siblings, 1 reply; 30+ messages in thread
From: David Gibson @ 2017-05-01 4:58 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu,
Jason Wang, Alex Williamson
[-- Attachment #1: Type: text/plain, Size: 4151 bytes --]
On Thu, Apr 27, 2017 at 05:34:18PM +0800, Peter Xu wrote:
> This is something similar to MemoryRegionOps, it's just for address
> spaces to store arch-specific hooks.
>
> The first hook I would like to introduce is iommu_get().
>
> For systems that have IOMMUs, we will create a special address space per
> device which is different from system default address space for
> it (please refer to pci_device_iommu_address_space()). Normally when
> that happens, there will be one specific IOMMU (or say, translation
> unit) stands right behind that new address space.
>
> This iommu_get() fetches that guy behind the address space. Here, the
> guy is defined as IOMMUObject, which is currently a (void *). In the
> future, maybe we can make it a better definition, but imho it's good
> enough for now, considering it's arch-dependent.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
This doesn't make sense to me. It would be entirely possible for a
single address space to have different regions mapped by different
IOMMUs. Or some regions mapped by IOMMUs and others direct mapped to
a device or memory block.
> ---
> include/exec/memory.h | 30 ++++++++++++++++++++++++++++++
> memory.c | 8 ++++++++
> 2 files changed, 38 insertions(+)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index e5707b3..0b0b58b 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -183,6 +183,15 @@ struct MemoryRegionOps {
> const MemoryRegionMmio old_mmio;
> };
>
> +/*
> + * This stands for an IOMMU unit. Normally it should be exactly the
> + * IOMMU device, however this can also be actually anything which is
> + * related to that translation unit. What it is should be totally
> + * arch-dependent. Maybe one day we can have something better than a
> + * (void *) here.
> + */
> +typedef void *IOMMUObject;
> +
> typedef struct MemoryRegionIOMMUOps MemoryRegionIOMMUOps;
>
> struct MemoryRegionIOMMUOps {
> @@ -282,6 +291,19 @@ struct MemoryListener {
> };
>
> /**
> + * AddressSpaceOps: callbacks structure for address space specific operations
> + *
> + * @iommu_get: returns an IOMMU object that backs the address space.
> + * Normally this should be NULL for generic address
> + * spaces, and it's only used when there is one
> + * translation unit behind this address space.
> + */
> +struct AddressSpaceOps {
> + IOMMUObject *(*iommu_get)(AddressSpace *as);
> +};
> +typedef struct AddressSpaceOps AddressSpaceOps;
> +
> +/**
> * AddressSpace: describes a mapping of addresses to #MemoryRegion objects
> */
> struct AddressSpace {
> @@ -302,6 +324,7 @@ struct AddressSpace {
> MemoryListener dispatch_listener;
> QTAILQ_HEAD(memory_listeners_as, MemoryListener) listeners;
> QTAILQ_ENTRY(AddressSpace) address_spaces_link;
> + AddressSpaceOps as_ops;
> };
>
> /**
> @@ -1800,6 +1823,13 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr,
> address_space_write(cache->as, cache->xlat + addr, MEMTXATTRS_UNSPECIFIED, buf, len);
> }
>
> +/**
> + * address_space_iommu_get: Get the backend IOMMU for the address space
> + *
> + * @as: the address space to fetch IOMMU from
> + */
> +IOMMUObject *address_space_iommu_get(AddressSpace *as);
> +
> #endif
>
> #endif
> diff --git a/memory.c b/memory.c
> index 6af523e..6aaad45 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -2500,6 +2500,14 @@ void address_space_destroy(AddressSpace *as)
> call_rcu(as, do_address_space_destroy, rcu);
> }
>
> +IOMMUObject *address_space_iommu_get(AddressSpace *as)
> +{
> + if (!as->as_ops.iommu_get) {
> + return NULL;
> + }
> + return as->as_ops.iommu_get(as);
> +}
> +
> static const char *memory_region_type(MemoryRegion *mr)
> {
> if (memory_region_is_ram_device(mr)) {
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 6/8] memory: introduce AddressSpaceOps
2017-05-08 7:32 ` Peter Xu
@ 2017-05-07 9:44 ` Liu, Yi L
2017-05-10 7:04 ` David Gibson
1 sibling, 0 replies; 30+ messages in thread
From: Liu, Yi L @ 2017-05-07 9:44 UTC (permalink / raw)
To: Peter Xu
Cc: David Gibson, tianyu.lan, kevin.tian, yi.l.liu, Jason Wang,
qemu-devel, Alex Williamson, Paolo Bonzini
On Mon, May 08, 2017 at 03:32:17PM +0800, Peter Xu wrote:
> On Mon, May 08, 2017 at 04:07:44PM +1000, David Gibson wrote:
> > On Mon, May 08, 2017 at 01:48:14PM +0800, Peter Xu wrote:
> > > On Mon, May 01, 2017 at 02:58:22PM +1000, David Gibson wrote:
> > > > On Thu, Apr 27, 2017 at 05:34:18PM +0800, Peter Xu wrote:
> > > > > This is something similar to MemoryRegionOps, it's just for address
> > > > > spaces to store arch-specific hooks.
> > > > >
> > > > > The first hook I would like to introduce is iommu_get().
> > > > >
> > > > > For systems that have IOMMUs, we will create a special address space per
> > > > > device which is different from system default address space for
> > > > > it (please refer to pci_device_iommu_address_space()). Normally when
> > > > > that happens, there will be one specific IOMMU (or say, translation
> > > > > unit) stands right behind that new address space.
> > > > >
> > > > > This iommu_get() fetches that guy behind the address space. Here, the
> > > > > guy is defined as IOMMUObject, which is currently a (void *). In the
> > > > > future, maybe we can make it a better definition, but imho it's good
> > > > > enough for now, considering it's arch-dependent.
> > > > >
> > > > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > >
> > > > This doesn't make sense to me. It would be entirely possible for a
> > > > single address space to have different regions mapped by different
> > > > IOMMUs. Or some regions mapped by IOMMUs and others direct mapped to
> > > > a device or memory block.
> > >
> > > Oh, so it's more complicated than I thought... Then, do we really have
> > > existing use case that one device is managed by more than one IOMMU
> > > (on any of the platform)? Frankly speaking I haven't thought about
> > > complicated scenarios like this, or nested IOMMUs yet.
> >
> > Sort of, it depends what you count as "more than one IOMMU".
> >
> > spapr can - depending on guest configuration - have two IOMMU windows
> > for each guest PCI domain. In theory the guest can set these up
> > however it wants, in practice there's usually a small (~256MiB) at PCI
> > address 0 for the benefit of 32-bit PCI devices, then a much larger
> > window up at a high address to allow better performance for 64-bit
> > capable devices.
> >
> > Those are the same IOMMU in the sense that they're both implemented by
> > logic built into the same virtual PCI host bridge. However, they're
> > different IOMMUs in the sense that they have independent data
> > structures describing the mappings and are currently modelled as two
> > different IOMMU memory regions.
> >
> >
> > I don't believe we have any existing platforms with both an IOMMU and
> > a direct mapped window in a device's address space. But it seems to
> > be just too plausible a setup to not plan for it. [1]
> >
> > > This patch derived from a requirement in virt-svm project (on x86).
> > > Virt-svm needs some notification mechanism for each IOMMU (or say, the
> > > IOMMU that managers the SVM-enabled device). For now, all IOMMU
> > > notifiers are per-memory-region not per-iommu, and that's imho not
> > > what virt-svm wants. Any suggestions?
> >
> > I don't know SVM, so I can't really make sense of that. What format
> > does this identifier need? What does "for one IOMMU" mean in this
> > context - i.e. what guest observable properties require the IDs to be
> > the same or to be different.
>
> Virt-svm should need to trap the content of a register (actually the
> data is in the memory, but, let's assume it's a mmio operation for
> simplicity, considering it is finally delivered via invalidation
> requests), then pass that info down to kernel. So the listened element
> is per-iommu not per-mr this time. When the content changed, vfio will
> need to be notified, then pass this info down.
>
> Yi/others, please feel free to correct me.
Hi David,
May be I can provide some detail to assist your review.
virt-SVM wants two notifiers so far.
* notifier1: link guest pasid table to host, it would be triggered when
context entry is changed in guest
* notifier2: passdown iommu tlb invalidate, the mappings to be invalidated
is GVA->GPA mappings. It just wants to flush IOTLB in host, no need to
change the page table in memory with this notifer.
The two new notifiers must be registered when vIOMMU is exposed to guest.
Context entry flush triggers notifier1. iommu tlb flushing(extended-IOTLB/
pasid-cache/extended-device-IOTLB flushing in VT-d) triggers notifier2.
Current MAP/UNMAP notifiers are actually registered when there is memory
region event. In the latest code, the event is address space switching.
They are triggered when guest changed IOVA->GPA mappings.
Both the existed notifier and the new notifier would be triggered by iommu
events(iotlb flush)in guest. However, the registration is different. New
notifier depends on vIOMMU exposure. It's per-iommu unit as Peter mentioned.
While MAP/UNMAP registration depends on iommu memory region, so it's more
like memory region notifier.
Thanks,
Yi L
> Thanks,
>
> >
> >
> > [1] My reasoning here is similar to the reason sPAPR allows the two
> > windows. For PAPR, the guest is paravirtualized, so both windows
> > essentially have to be remapped IOMMU windows. For a bare metal
> > platform it seems a very reasonable tradeoff would be to have a
> > small(ish) 32-bit IOMMU window to allow 32-bit devices to work on a
> > large RAM machine, along with a large direct mapped "bypass" window
> > for maxmimum performance for 64-bit devices.
> >
> > --
> > David Gibson | I'll have my music baroque, and my code
> > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> > | _way_ _around_!
> > http://www.ozlabs.org/~dgibson
>
> --
> Peter Xu
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 6/8] memory: introduce AddressSpaceOps
2017-05-01 4:58 ` David Gibson
@ 2017-05-08 5:48 ` Peter Xu
2017-05-08 6:07 ` David Gibson
0 siblings, 1 reply; 30+ messages in thread
From: Peter Xu @ 2017-05-08 5:48 UTC (permalink / raw)
To: David Gibson
Cc: qemu-devel, tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu,
Jason Wang, Alex Williamson
On Mon, May 01, 2017 at 02:58:22PM +1000, David Gibson wrote:
> On Thu, Apr 27, 2017 at 05:34:18PM +0800, Peter Xu wrote:
> > This is something similar to MemoryRegionOps, it's just for address
> > spaces to store arch-specific hooks.
> >
> > The first hook I would like to introduce is iommu_get().
> >
> > For systems that have IOMMUs, we will create a special address space per
> > device which is different from system default address space for
> > it (please refer to pci_device_iommu_address_space()). Normally when
> > that happens, there will be one specific IOMMU (or say, translation
> > unit) stands right behind that new address space.
> >
> > This iommu_get() fetches that guy behind the address space. Here, the
> > guy is defined as IOMMUObject, which is currently a (void *). In the
> > future, maybe we can make it a better definition, but imho it's good
> > enough for now, considering it's arch-dependent.
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
>
> This doesn't make sense to me. It would be entirely possible for a
> single address space to have different regions mapped by different
> IOMMUs. Or some regions mapped by IOMMUs and others direct mapped to
> a device or memory block.
Oh, so it's more complicated than I thought... Then, do we really have
existing use case that one device is managed by more than one IOMMU
(on any of the platform)? Frankly speaking I haven't thought about
complicated scenarios like this, or nested IOMMUs yet.
This patch derived from a requirement in virt-svm project (on x86).
Virt-svm needs some notification mechanism for each IOMMU (or say, the
IOMMU that managers the SVM-enabled device). For now, all IOMMU
notifiers are per-memory-region not per-iommu, and that's imho not
what virt-svm wants. Any suggestions?
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 3/8] memory: rename iommu_notifier_init()
2017-05-01 4:53 ` David Gibson
@ 2017-05-08 5:50 ` Peter Xu
0 siblings, 0 replies; 30+ messages in thread
From: Peter Xu @ 2017-05-08 5:50 UTC (permalink / raw)
To: David Gibson
Cc: qemu-devel, tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu,
Jason Wang, Alex Williamson
On Mon, May 01, 2017 at 02:53:35PM +1000, David Gibson wrote:
> On Thu, Apr 27, 2017 at 05:34:15PM +0800, Peter Xu wrote:
> > It's new name is iommu_mr_notifier_init(). Again, literal changes only.
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
>
> Again, I think this could be folded with the previous two patches.
Sure. I can squash them all after I know whether we'll need this
series.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>
> I also note that these patches will conflict with Alexey's proposed
> changes to make IOMMU MR's a QOM subtype of the ordinary MR (thus
> removing some of the MR specific fields from other MRs.
No problem. Due to some reason I just came back to read all the mails,
so haven't read them yet. I can rebase when needed. Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 6/8] memory: introduce AddressSpaceOps
2017-05-08 5:48 ` Peter Xu
@ 2017-05-08 6:07 ` David Gibson
2017-05-08 7:32 ` Peter Xu
0 siblings, 1 reply; 30+ messages in thread
From: David Gibson @ 2017-05-08 6:07 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu,
Jason Wang, Alex Williamson
[-- Attachment #1: Type: text/plain, Size: 3788 bytes --]
On Mon, May 08, 2017 at 01:48:14PM +0800, Peter Xu wrote:
> On Mon, May 01, 2017 at 02:58:22PM +1000, David Gibson wrote:
> > On Thu, Apr 27, 2017 at 05:34:18PM +0800, Peter Xu wrote:
> > > This is something similar to MemoryRegionOps, it's just for address
> > > spaces to store arch-specific hooks.
> > >
> > > The first hook I would like to introduce is iommu_get().
> > >
> > > For systems that have IOMMUs, we will create a special address space per
> > > device which is different from system default address space for
> > > it (please refer to pci_device_iommu_address_space()). Normally when
> > > that happens, there will be one specific IOMMU (or say, translation
> > > unit) stands right behind that new address space.
> > >
> > > This iommu_get() fetches that guy behind the address space. Here, the
> > > guy is defined as IOMMUObject, which is currently a (void *). In the
> > > future, maybe we can make it a better definition, but imho it's good
> > > enough for now, considering it's arch-dependent.
> > >
> > > Signed-off-by: Peter Xu <peterx@redhat.com>
> >
> > This doesn't make sense to me. It would be entirely possible for a
> > single address space to have different regions mapped by different
> > IOMMUs. Or some regions mapped by IOMMUs and others direct mapped to
> > a device or memory block.
>
> Oh, so it's more complicated than I thought... Then, do we really have
> existing use case that one device is managed by more than one IOMMU
> (on any of the platform)? Frankly speaking I haven't thought about
> complicated scenarios like this, or nested IOMMUs yet.
Sort of, it depends what you count as "more than one IOMMU".
spapr can - depending on guest configuration - have two IOMMU windows
for each guest PCI domain. In theory the guest can set these up
however it wants, in practice there's usually a small (~256MiB) at PCI
address 0 for the benefit of 32-bit PCI devices, then a much larger
window up at a high address to allow better performance for 64-bit
capable devices.
Those are the same IOMMU in the sense that they're both implemented by
logic built into the same virtual PCI host bridge. However, they're
different IOMMUs in the sense that they have independent data
structures describing the mappings and are currently modelled as two
different IOMMU memory regions.
I don't believe we have any existing platforms with both an IOMMU and
a direct mapped window in a device's address space. But it seems to
be just too plausible a setup to not plan for it. [1]
> This patch derived from a requirement in virt-svm project (on x86).
> Virt-svm needs some notification mechanism for each IOMMU (or say, the
> IOMMU that managers the SVM-enabled device). For now, all IOMMU
> notifiers are per-memory-region not per-iommu, and that's imho not
> what virt-svm wants. Any suggestions?
I don't know SVM, so I can't really make sense of that. What format
does this identifier need? What does "for one IOMMU" mean in this
context - i.e. what guest observable properties require the IDs to be
the same or to be different.
[1] My reasoning here is similar to the reason sPAPR allows the two
windows. For PAPR, the guest is paravirtualized, so both windows
essentially have to be remapped IOMMU windows. For a bare metal
platform it seems a very reasonable tradeoff would be to have a
small(ish) 32-bit IOMMU window to allow 32-bit devices to work on a
large RAM machine, along with a large direct mapped "bypass" window
for maxmimum performance for 64-bit devices.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 6/8] memory: introduce AddressSpaceOps
2017-05-08 6:07 ` David Gibson
@ 2017-05-08 7:32 ` Peter Xu
2017-05-07 9:44 ` Liu, Yi L
2017-05-10 7:04 ` David Gibson
0 siblings, 2 replies; 30+ messages in thread
From: Peter Xu @ 2017-05-08 7:32 UTC (permalink / raw)
To: David Gibson
Cc: qemu-devel, tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu,
Jason Wang, Alex Williamson
On Mon, May 08, 2017 at 04:07:44PM +1000, David Gibson wrote:
> On Mon, May 08, 2017 at 01:48:14PM +0800, Peter Xu wrote:
> > On Mon, May 01, 2017 at 02:58:22PM +1000, David Gibson wrote:
> > > On Thu, Apr 27, 2017 at 05:34:18PM +0800, Peter Xu wrote:
> > > > This is something similar to MemoryRegionOps, it's just for address
> > > > spaces to store arch-specific hooks.
> > > >
> > > > The first hook I would like to introduce is iommu_get().
> > > >
> > > > For systems that have IOMMUs, we will create a special address space per
> > > > device which is different from system default address space for
> > > > it (please refer to pci_device_iommu_address_space()). Normally when
> > > > that happens, there will be one specific IOMMU (or say, translation
> > > > unit) stands right behind that new address space.
> > > >
> > > > This iommu_get() fetches that guy behind the address space. Here, the
> > > > guy is defined as IOMMUObject, which is currently a (void *). In the
> > > > future, maybe we can make it a better definition, but imho it's good
> > > > enough for now, considering it's arch-dependent.
> > > >
> > > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > >
> > > This doesn't make sense to me. It would be entirely possible for a
> > > single address space to have different regions mapped by different
> > > IOMMUs. Or some regions mapped by IOMMUs and others direct mapped to
> > > a device or memory block.
> >
> > Oh, so it's more complicated than I thought... Then, do we really have
> > existing use case that one device is managed by more than one IOMMU
> > (on any of the platform)? Frankly speaking I haven't thought about
> > complicated scenarios like this, or nested IOMMUs yet.
>
> Sort of, it depends what you count as "more than one IOMMU".
>
> spapr can - depending on guest configuration - have two IOMMU windows
> for each guest PCI domain. In theory the guest can set these up
> however it wants, in practice there's usually a small (~256MiB) at PCI
> address 0 for the benefit of 32-bit PCI devices, then a much larger
> window up at a high address to allow better performance for 64-bit
> capable devices.
>
> Those are the same IOMMU in the sense that they're both implemented by
> logic built into the same virtual PCI host bridge. However, they're
> different IOMMUs in the sense that they have independent data
> structures describing the mappings and are currently modelled as two
> different IOMMU memory regions.
>
>
> I don't believe we have any existing platforms with both an IOMMU and
> a direct mapped window in a device's address space. But it seems to
> be just too plausible a setup to not plan for it. [1]
>
> > This patch derived from a requirement in virt-svm project (on x86).
> > Virt-svm needs some notification mechanism for each IOMMU (or say, the
> > IOMMU that managers the SVM-enabled device). For now, all IOMMU
> > notifiers are per-memory-region not per-iommu, and that's imho not
> > what virt-svm wants. Any suggestions?
>
> I don't know SVM, so I can't really make sense of that. What format
> does this identifier need? What does "for one IOMMU" mean in this
> context - i.e. what guest observable properties require the IDs to be
> the same or to be different.
Virt-svm should need to trap the content of a register (actually the
data is in the memory, but, let's assume it's a mmio operation for
simplicity, considering it is finally delivered via invalidation
requests), then pass that info down to kernel. So the listened element
is per-iommu not per-mr this time. When the content changed, vfio will
need to be notified, then pass this info down.
Yi/others, please feel free to correct me.
Thanks,
>
>
> [1] My reasoning here is similar to the reason sPAPR allows the two
> windows. For PAPR, the guest is paravirtualized, so both windows
> essentially have to be remapped IOMMU windows. For a bare metal
> platform it seems a very reasonable tradeoff would be to have a
> small(ish) 32-bit IOMMU window to allow 32-bit devices to work on a
> large RAM machine, along with a large direct mapped "bypass" window
> for maxmimum performance for 64-bit devices.
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
--
Peter Xu
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 6/8] memory: introduce AddressSpaceOps
2017-05-08 7:32 ` Peter Xu
2017-05-07 9:44 ` Liu, Yi L
@ 2017-05-10 7:04 ` David Gibson
2017-05-11 5:04 ` Peter Xu
1 sibling, 1 reply; 30+ messages in thread
From: David Gibson @ 2017-05-10 7:04 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu,
Jason Wang, Alex Williamson
[-- Attachment #1: Type: text/plain, Size: 5275 bytes --]
On Mon, May 08, 2017 at 03:32:17PM +0800, Peter Xu wrote:
> On Mon, May 08, 2017 at 04:07:44PM +1000, David Gibson wrote:
> > On Mon, May 08, 2017 at 01:48:14PM +0800, Peter Xu wrote:
> > > On Mon, May 01, 2017 at 02:58:22PM +1000, David Gibson wrote:
> > > > On Thu, Apr 27, 2017 at 05:34:18PM +0800, Peter Xu wrote:
> > > > > This is something similar to MemoryRegionOps, it's just for address
> > > > > spaces to store arch-specific hooks.
> > > > >
> > > > > The first hook I would like to introduce is iommu_get().
> > > > >
> > > > > For systems that have IOMMUs, we will create a special address space per
> > > > > device which is different from system default address space for
> > > > > it (please refer to pci_device_iommu_address_space()). Normally when
> > > > > that happens, there will be one specific IOMMU (or say, translation
> > > > > unit) stands right behind that new address space.
> > > > >
> > > > > This iommu_get() fetches that guy behind the address space. Here, the
> > > > > guy is defined as IOMMUObject, which is currently a (void *). In the
> > > > > future, maybe we can make it a better definition, but imho it's good
> > > > > enough for now, considering it's arch-dependent.
> > > > >
> > > > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > >
> > > > This doesn't make sense to me. It would be entirely possible for a
> > > > single address space to have different regions mapped by different
> > > > IOMMUs. Or some regions mapped by IOMMUs and others direct mapped to
> > > > a device or memory block.
> > >
> > > Oh, so it's more complicated than I thought... Then, do we really have
> > > existing use case that one device is managed by more than one IOMMU
> > > (on any of the platform)? Frankly speaking I haven't thought about
> > > complicated scenarios like this, or nested IOMMUs yet.
> >
> > Sort of, it depends what you count as "more than one IOMMU".
> >
> > spapr can - depending on guest configuration - have two IOMMU windows
> > for each guest PCI domain. In theory the guest can set these up
> > however it wants, in practice there's usually a small (~256MiB) at PCI
> > address 0 for the benefit of 32-bit PCI devices, then a much larger
> > window up at a high address to allow better performance for 64-bit
> > capable devices.
> >
> > Those are the same IOMMU in the sense that they're both implemented by
> > logic built into the same virtual PCI host bridge. However, they're
> > different IOMMUs in the sense that they have independent data
> > structures describing the mappings and are currently modelled as two
> > different IOMMU memory regions.
> >
> >
> > I don't believe we have any existing platforms with both an IOMMU and
> > a direct mapped window in a device's address space. But it seems to
> > be just too plausible a setup to not plan for it. [1]
> >
> > > This patch derived from a requirement in virt-svm project (on x86).
> > > Virt-svm needs some notification mechanism for each IOMMU (or say, the
> > > IOMMU that managers the SVM-enabled device). For now, all IOMMU
> > > notifiers are per-memory-region not per-iommu, and that's imho not
> > > what virt-svm wants. Any suggestions?
> >
> > I don't know SVM, so I can't really make sense of that. What format
> > does this identifier need? What does "for one IOMMU" mean in this
> > context - i.e. what guest observable properties require the IDs to be
> > the same or to be different.
>
> Virt-svm should need to trap the content of a register (actually the
> data is in the memory, but, let's assume it's a mmio operation for
> simplicity, considering it is finally delivered via invalidation
> requests), then pass that info down to kernel. So the listened element
> is per-iommu not per-mr this time. When the content changed, vfio will
> need to be notified, then pass this info down.
I don't entirely follow what you're saying. When the virtual hardware
gets an invalidate request, it looks up the unit to invalidate in
memory? Which component gets to decide that ID? How is it advertised
to the guest OS?
If your ID is tied to the AS now, you could just iterate through the
AS and invalidate any IOMMU MRs that are present within it.
Alternatively, if the ID is tied to something more concrete, like a
specific PCI host bridge (which incorporates the IOMMU logic), then
that device probably already has a handle on the right IOMMU MR to
invalidate it.
>
> Yi/others, please feel free to correct me.
>
> Thanks,
>
> >
> >
> > [1] My reasoning here is similar to the reason sPAPR allows the two
> > windows. For PAPR, the guest is paravirtualized, so both windows
> > essentially have to be remapped IOMMU windows. For a bare metal
> > platform it seems a very reasonable tradeoff would be to have a
> > small(ish) 32-bit IOMMU window to allow 32-bit devices to work on a
> > large RAM machine, along with a large direct mapped "bypass" window
> > for maxmimum performance for 64-bit devices.
> >
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 6/8] memory: introduce AddressSpaceOps
2017-05-10 7:04 ` David Gibson
@ 2017-05-11 5:04 ` Peter Xu
2017-05-15 5:32 ` David Gibson
0 siblings, 1 reply; 30+ messages in thread
From: Peter Xu @ 2017-05-11 5:04 UTC (permalink / raw)
To: David Gibson
Cc: qemu-devel, tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu,
Jason Wang, Alex Williamson
On Wed, May 10, 2017 at 05:04:06PM +1000, David Gibson wrote:
> On Mon, May 08, 2017 at 03:32:17PM +0800, Peter Xu wrote:
> > On Mon, May 08, 2017 at 04:07:44PM +1000, David Gibson wrote:
> > > On Mon, May 08, 2017 at 01:48:14PM +0800, Peter Xu wrote:
> > > > On Mon, May 01, 2017 at 02:58:22PM +1000, David Gibson wrote:
> > > > > On Thu, Apr 27, 2017 at 05:34:18PM +0800, Peter Xu wrote:
> > > > > > This is something similar to MemoryRegionOps, it's just for address
> > > > > > spaces to store arch-specific hooks.
> > > > > >
> > > > > > The first hook I would like to introduce is iommu_get().
> > > > > >
> > > > > > For systems that have IOMMUs, we will create a special address space per
> > > > > > device which is different from system default address space for
> > > > > > it (please refer to pci_device_iommu_address_space()). Normally when
> > > > > > that happens, there will be one specific IOMMU (or say, translation
> > > > > > unit) stands right behind that new address space.
> > > > > >
> > > > > > This iommu_get() fetches that guy behind the address space. Here, the
> > > > > > guy is defined as IOMMUObject, which is currently a (void *). In the
> > > > > > future, maybe we can make it a better definition, but imho it's good
> > > > > > enough for now, considering it's arch-dependent.
> > > > > >
> > > > > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > > >
> > > > > This doesn't make sense to me. It would be entirely possible for a
> > > > > single address space to have different regions mapped by different
> > > > > IOMMUs. Or some regions mapped by IOMMUs and others direct mapped to
> > > > > a device or memory block.
> > > >
> > > > Oh, so it's more complicated than I thought... Then, do we really have
> > > > existing use case that one device is managed by more than one IOMMU
> > > > (on any of the platform)? Frankly speaking I haven't thought about
> > > > complicated scenarios like this, or nested IOMMUs yet.
> > >
> > > Sort of, it depends what you count as "more than one IOMMU".
> > >
> > > spapr can - depending on guest configuration - have two IOMMU windows
> > > for each guest PCI domain. In theory the guest can set these up
> > > however it wants, in practice there's usually a small (~256MiB) at PCI
> > > address 0 for the benefit of 32-bit PCI devices, then a much larger
> > > window up at a high address to allow better performance for 64-bit
> > > capable devices.
> > >
> > > Those are the same IOMMU in the sense that they're both implemented by
> > > logic built into the same virtual PCI host bridge. However, they're
> > > different IOMMUs in the sense that they have independent data
> > > structures describing the mappings and are currently modelled as two
> > > different IOMMU memory regions.
> > >
> > >
> > > I don't believe we have any existing platforms with both an IOMMU and
> > > a direct mapped window in a device's address space. But it seems to
> > > be just too plausible a setup to not plan for it. [1]
> > >
> > > > This patch derived from a requirement in virt-svm project (on x86).
> > > > Virt-svm needs some notification mechanism for each IOMMU (or say, the
> > > > IOMMU that managers the SVM-enabled device). For now, all IOMMU
> > > > notifiers are per-memory-region not per-iommu, and that's imho not
> > > > what virt-svm wants. Any suggestions?
> > >
> > > I don't know SVM, so I can't really make sense of that. What format
> > > does this identifier need? What does "for one IOMMU" mean in this
> > > context - i.e. what guest observable properties require the IDs to be
> > > the same or to be different.
> >
> > Virt-svm should need to trap the content of a register (actually the
> > data is in the memory, but, let's assume it's a mmio operation for
> > simplicity, considering it is finally delivered via invalidation
> > requests), then pass that info down to kernel. So the listened element
> > is per-iommu not per-mr this time. When the content changed, vfio will
> > need to be notified, then pass this info down.
>
> I don't entirely follow what you're saying. When the virtual hardware
> gets an invalidate request, it looks up the unit to invalidate in
> memory? Which component gets to decide that ID? How is it advertised
> to the guest OS?
>
> If your ID is tied to the AS now, you could just iterate through the
> AS and invalidate any IOMMU MRs that are present within it.
>
> Alternatively, if the ID is tied to something more concrete, like a
> specific PCI host bridge (which incorporates the IOMMU logic), then
> that device probably already has a handle on the right IOMMU MR to
> invalidate it.
Sorry to be unclear on the requirement. I don't know what's the ID you
mentioned above... Anyway, let me try to further simplify the use
case.
Just assume we have such a requirement: when one register of vIOMMU
changes, we need to pass this register data to the hardware IOMMU by
some way. And, let's assume this is a notification mechanism, so that
every device in the system can listen to this register change, then
capture what has changed to what. Here the point is, in all cases this
event is not related to memory region at all. So imho we need some
other way to do it besides memory region IOMMU notifiers.
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 6/8] memory: introduce AddressSpaceOps
2017-05-11 5:04 ` Peter Xu
@ 2017-05-15 5:32 ` David Gibson
2017-05-25 7:24 ` Peter Xu
0 siblings, 1 reply; 30+ messages in thread
From: David Gibson @ 2017-05-15 5:32 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu,
Jason Wang, Alex Williamson
[-- Attachment #1: Type: text/plain, Size: 6941 bytes --]
On Thu, May 11, 2017 at 01:04:26PM +0800, Peter Xu wrote:
> On Wed, May 10, 2017 at 05:04:06PM +1000, David Gibson wrote:
> > On Mon, May 08, 2017 at 03:32:17PM +0800, Peter Xu wrote:
> > > On Mon, May 08, 2017 at 04:07:44PM +1000, David Gibson wrote:
> > > > On Mon, May 08, 2017 at 01:48:14PM +0800, Peter Xu wrote:
> > > > > On Mon, May 01, 2017 at 02:58:22PM +1000, David Gibson wrote:
> > > > > > On Thu, Apr 27, 2017 at 05:34:18PM +0800, Peter Xu wrote:
> > > > > > > This is something similar to MemoryRegionOps, it's just for address
> > > > > > > spaces to store arch-specific hooks.
> > > > > > >
> > > > > > > The first hook I would like to introduce is iommu_get().
> > > > > > >
> > > > > > > For systems that have IOMMUs, we will create a special address space per
> > > > > > > device which is different from system default address space for
> > > > > > > it (please refer to pci_device_iommu_address_space()). Normally when
> > > > > > > that happens, there will be one specific IOMMU (or say, translation
> > > > > > > unit) stands right behind that new address space.
> > > > > > >
> > > > > > > This iommu_get() fetches that guy behind the address space. Here, the
> > > > > > > guy is defined as IOMMUObject, which is currently a (void *). In the
> > > > > > > future, maybe we can make it a better definition, but imho it's good
> > > > > > > enough for now, considering it's arch-dependent.
> > > > > > >
> > > > > > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > > > >
> > > > > > This doesn't make sense to me. It would be entirely possible for a
> > > > > > single address space to have different regions mapped by different
> > > > > > IOMMUs. Or some regions mapped by IOMMUs and others direct mapped to
> > > > > > a device or memory block.
> > > > >
> > > > > Oh, so it's more complicated than I thought... Then, do we really have
> > > > > existing use case that one device is managed by more than one IOMMU
> > > > > (on any of the platform)? Frankly speaking I haven't thought about
> > > > > complicated scenarios like this, or nested IOMMUs yet.
> > > >
> > > > Sort of, it depends what you count as "more than one IOMMU".
> > > >
> > > > spapr can - depending on guest configuration - have two IOMMU windows
> > > > for each guest PCI domain. In theory the guest can set these up
> > > > however it wants, in practice there's usually a small (~256MiB) at PCI
> > > > address 0 for the benefit of 32-bit PCI devices, then a much larger
> > > > window up at a high address to allow better performance for 64-bit
> > > > capable devices.
> > > >
> > > > Those are the same IOMMU in the sense that they're both implemented by
> > > > logic built into the same virtual PCI host bridge. However, they're
> > > > different IOMMUs in the sense that they have independent data
> > > > structures describing the mappings and are currently modelled as two
> > > > different IOMMU memory regions.
> > > >
> > > >
> > > > I don't believe we have any existing platforms with both an IOMMU and
> > > > a direct mapped window in a device's address space. But it seems to
> > > > be just too plausible a setup to not plan for it. [1]
> > > >
> > > > > This patch derived from a requirement in virt-svm project (on x86).
> > > > > Virt-svm needs some notification mechanism for each IOMMU (or say, the
> > > > > IOMMU that managers the SVM-enabled device). For now, all IOMMU
> > > > > notifiers are per-memory-region not per-iommu, and that's imho not
> > > > > what virt-svm wants. Any suggestions?
> > > >
> > > > I don't know SVM, so I can't really make sense of that. What format
> > > > does this identifier need? What does "for one IOMMU" mean in this
> > > > context - i.e. what guest observable properties require the IDs to be
> > > > the same or to be different.
> > >
> > > Virt-svm should need to trap the content of a register (actually the
> > > data is in the memory, but, let's assume it's a mmio operation for
> > > simplicity, considering it is finally delivered via invalidation
> > > requests), then pass that info down to kernel. So the listened element
> > > is per-iommu not per-mr this time. When the content changed, vfio will
> > > need to be notified, then pass this info down.
> >
> > I don't entirely follow what you're saying. When the virtual hardware
> > gets an invalidate request, it looks up the unit to invalidate in
> > memory? Which component gets to decide that ID? How is it advertised
> > to the guest OS?
> >
> > If your ID is tied to the AS now, you could just iterate through the
> > AS and invalidate any IOMMU MRs that are present within it.
> >
> > Alternatively, if the ID is tied to something more concrete, like a
> > specific PCI host bridge (which incorporates the IOMMU logic), then
> > that device probably already has a handle on the right IOMMU MR to
> > invalidate it.
>
> Sorry to be unclear on the requirement. I don't know what's the ID you
> mentioned above... Anyway, let me try to further simplify the use
> case.
Right, the ID was me guessing badly at what's going on here, so I
think it confused rather than clarifying.
> Just assume we have such a requirement: when one register of vIOMMU
> changes, we need to pass this register data to the hardware IOMMU by
> some way. And, let's assume this is a notification mechanism, so that
> every device in the system can listen to this register change, then
> capture what has changed to what. Here the point is, in all cases this
> event is not related to memory region at all. So imho we need some
> other way to do it besides memory region IOMMU notifiers.
Ok. So is this right?
* You have a single bank of vIOMMU registers
* Which control two (or more) IOMMU regions in in the guest's
address space
* Assuming the host also has an AMD IOMMU, those will be
backed by a single IOMMU on the host ("single" meaning
controlled by a single bank of host registers)
I'm assuming the guest IOMMU code must know which IOMMU regions it is
managing, so getting from the guest registers to the set of IOMMU MRs
should be easy.
What's the operation that needs to happen on the host IOMMU, in terms
of the VFIO IOMMU interface?
Is this inherently only possible if both host and guest have an AMD
IOMMU? Could it be made to work if the guest had an AMD IOMMU but the
host had an Intel one, or the other way around?
Would it make sense to have a single IOMMU MR in the guest, but
instead of mapping it whole into the guest address space, have two
(or more) alias MRs in the AS which each allow access to a portion of
the IOMMU MR?
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 6/8] memory: introduce AddressSpaceOps
2017-05-15 5:32 ` David Gibson
@ 2017-05-25 7:24 ` Peter Xu
2017-05-26 5:30 ` David Gibson
0 siblings, 1 reply; 30+ messages in thread
From: Peter Xu @ 2017-05-25 7:24 UTC (permalink / raw)
To: David Gibson
Cc: qemu-devel, tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu,
Jason Wang, Alex Williamson
On Mon, May 15, 2017 at 03:32:11PM +1000, David Gibson wrote:
> On Thu, May 11, 2017 at 01:04:26PM +0800, Peter Xu wrote:
> > On Wed, May 10, 2017 at 05:04:06PM +1000, David Gibson wrote:
> > > On Mon, May 08, 2017 at 03:32:17PM +0800, Peter Xu wrote:
> > > > On Mon, May 08, 2017 at 04:07:44PM +1000, David Gibson wrote:
> > > > > On Mon, May 08, 2017 at 01:48:14PM +0800, Peter Xu wrote:
> > > > > > On Mon, May 01, 2017 at 02:58:22PM +1000, David Gibson wrote:
> > > > > > > On Thu, Apr 27, 2017 at 05:34:18PM +0800, Peter Xu wrote:
> > > > > > > > This is something similar to MemoryRegionOps, it's just for address
> > > > > > > > spaces to store arch-specific hooks.
> > > > > > > >
> > > > > > > > The first hook I would like to introduce is iommu_get().
> > > > > > > >
> > > > > > > > For systems that have IOMMUs, we will create a special address space per
> > > > > > > > device which is different from system default address space for
> > > > > > > > it (please refer to pci_device_iommu_address_space()). Normally when
> > > > > > > > that happens, there will be one specific IOMMU (or say, translation
> > > > > > > > unit) stands right behind that new address space.
> > > > > > > >
> > > > > > > > This iommu_get() fetches that guy behind the address space. Here, the
> > > > > > > > guy is defined as IOMMUObject, which is currently a (void *). In the
> > > > > > > > future, maybe we can make it a better definition, but imho it's good
> > > > > > > > enough for now, considering it's arch-dependent.
> > > > > > > >
> > > > > > > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > > > > >
> > > > > > > This doesn't make sense to me. It would be entirely possible for a
> > > > > > > single address space to have different regions mapped by different
> > > > > > > IOMMUs. Or some regions mapped by IOMMUs and others direct mapped to
> > > > > > > a device or memory block.
> > > > > >
> > > > > > Oh, so it's more complicated than I thought... Then, do we really have
> > > > > > existing use case that one device is managed by more than one IOMMU
> > > > > > (on any of the platform)? Frankly speaking I haven't thought about
> > > > > > complicated scenarios like this, or nested IOMMUs yet.
> > > > >
> > > > > Sort of, it depends what you count as "more than one IOMMU".
> > > > >
> > > > > spapr can - depending on guest configuration - have two IOMMU windows
> > > > > for each guest PCI domain. In theory the guest can set these up
> > > > > however it wants, in practice there's usually a small (~256MiB) at PCI
> > > > > address 0 for the benefit of 32-bit PCI devices, then a much larger
> > > > > window up at a high address to allow better performance for 64-bit
> > > > > capable devices.
> > > > >
> > > > > Those are the same IOMMU in the sense that they're both implemented by
> > > > > logic built into the same virtual PCI host bridge. However, they're
> > > > > different IOMMUs in the sense that they have independent data
> > > > > structures describing the mappings and are currently modelled as two
> > > > > different IOMMU memory regions.
[1]
> > > > >
> > > > >
> > > > > I don't believe we have any existing platforms with both an IOMMU and
> > > > > a direct mapped window in a device's address space. But it seems to
> > > > > be just too plausible a setup to not plan for it. [1]
> > > > >
> > > > > > This patch derived from a requirement in virt-svm project (on x86).
> > > > > > Virt-svm needs some notification mechanism for each IOMMU (or say, the
> > > > > > IOMMU that managers the SVM-enabled device). For now, all IOMMU
> > > > > > notifiers are per-memory-region not per-iommu, and that's imho not
> > > > > > what virt-svm wants. Any suggestions?
> > > > >
> > > > > I don't know SVM, so I can't really make sense of that. What format
> > > > > does this identifier need? What does "for one IOMMU" mean in this
> > > > > context - i.e. what guest observable properties require the IDs to be
> > > > > the same or to be different.
> > > >
> > > > Virt-svm should need to trap the content of a register (actually the
> > > > data is in the memory, but, let's assume it's a mmio operation for
> > > > simplicity, considering it is finally delivered via invalidation
> > > > requests), then pass that info down to kernel. So the listened element
> > > > is per-iommu not per-mr this time. When the content changed, vfio will
> > > > need to be notified, then pass this info down.
> > >
> > > I don't entirely follow what you're saying. When the virtual hardware
> > > gets an invalidate request, it looks up the unit to invalidate in
> > > memory? Which component gets to decide that ID? How is it advertised
> > > to the guest OS?
> > >
> > > If your ID is tied to the AS now, you could just iterate through the
> > > AS and invalidate any IOMMU MRs that are present within it.
> > >
> > > Alternatively, if the ID is tied to something more concrete, like a
> > > specific PCI host bridge (which incorporates the IOMMU logic), then
> > > that device probably already has a handle on the right IOMMU MR to
> > > invalidate it.
> >
> > Sorry to be unclear on the requirement. I don't know what's the ID you
> > mentioned above... Anyway, let me try to further simplify the use
> > case.
>
> Right, the ID was me guessing badly at what's going on here, so I
> think it confused rather than clarifying.
>
> > Just assume we have such a requirement: when one register of vIOMMU
> > changes, we need to pass this register data to the hardware IOMMU by
> > some way. And, let's assume this is a notification mechanism, so that
> > every device in the system can listen to this register change, then
> > capture what has changed to what. Here the point is, in all cases this
> > event is not related to memory region at all. So imho we need some
> > other way to do it besides memory region IOMMU notifiers.
>
> Ok. So is this right?
> * You have a single bank of vIOMMU registers
> * Which control two (or more) IOMMU regions in in the guest's
> address space
> * Assuming the host also has an AMD IOMMU, those will be
> backed by a single IOMMU on the host ("single" meaning
> controlled by a single bank of host registers)
>
> I'm assuming the guest IOMMU code must know which IOMMU regions it is
> managing, so getting from the guest registers to the set of IOMMU MRs
> should be easy.
>
> What's the operation that needs to happen on the host IOMMU, in terms
> of the VFIO IOMMU interface?
(Sorry to respond so late...)
It'll pass the captured data downward to host IOMMU in some way.
IMHO if we are discussing the notifier thing only, we don't really
need to know what would it do after it gets the data. The point is how
we should define this kind if notifies, which differs from current
memory region based notifiers.
>
> Is this inherently only possible if both host and guest have an AMD
> IOMMU? Could it be made to work if the guest had an AMD IOMMU but the
> host had an Intel one, or the other way around?
>
> Would it make sense to have a single IOMMU MR in the guest, but
> instead of mapping it whole into the guest address space, have two
> (or more) alias MRs in the AS which each allow access to a portion of
> the IOMMU MR?
For these questions, again I don't know whether it'll affect how we
design a notifier mechanism to the remapping unit... Would it really?
Or maybe I missed anything?
Till now, after I know the case for SPAPR you have explained [1]
(thanks btw!), could I say that these multiple IOMMU windows still be
backed by some unified hardware in the PCI host bridge? Can that be
the so-called single "IOMMU" object behind that device? And, would it
possible that we might have similar requirement in the future just
like what Yi has met with virt-svm? (I don't know whether Power would
support SVM or similar, but I guess ARM should support it?)
I am just thinking how we can define a better and general (for all
platforms) IOMMU model in QEMU that can best suite our needs. And
currently that should be a model that can satisfy Yi's requirement.
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 6/8] memory: introduce AddressSpaceOps
2017-05-25 7:24 ` Peter Xu
@ 2017-05-26 5:30 ` David Gibson
2017-06-08 8:24 ` Liu, Yi L
0 siblings, 1 reply; 30+ messages in thread
From: David Gibson @ 2017-05-26 5:30 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, tianyu.lan, Paolo Bonzini, kevin.tian, yi.l.liu,
Jason Wang, Alex Williamson
[-- Attachment #1: Type: text/plain, Size: 9425 bytes --]
On Thu, May 25, 2017 at 03:24:30PM +0800, Peter Xu wrote:
> On Mon, May 15, 2017 at 03:32:11PM +1000, David Gibson wrote:
> > On Thu, May 11, 2017 at 01:04:26PM +0800, Peter Xu wrote:
> > > On Wed, May 10, 2017 at 05:04:06PM +1000, David Gibson wrote:
> > > > On Mon, May 08, 2017 at 03:32:17PM +0800, Peter Xu wrote:
> > > > > On Mon, May 08, 2017 at 04:07:44PM +1000, David Gibson wrote:
> > > > > > On Mon, May 08, 2017 at 01:48:14PM +0800, Peter Xu wrote:
> > > > > > > On Mon, May 01, 2017 at 02:58:22PM +1000, David Gibson wrote:
> > > > > > > > On Thu, Apr 27, 2017 at 05:34:18PM +0800, Peter Xu wrote:
> > > > > > > > > This is something similar to MemoryRegionOps, it's just for address
> > > > > > > > > spaces to store arch-specific hooks.
> > > > > > > > >
> > > > > > > > > The first hook I would like to introduce is iommu_get().
> > > > > > > > >
> > > > > > > > > For systems that have IOMMUs, we will create a special address space per
> > > > > > > > > device which is different from system default address space for
> > > > > > > > > it (please refer to pci_device_iommu_address_space()). Normally when
> > > > > > > > > that happens, there will be one specific IOMMU (or say, translation
> > > > > > > > > unit) stands right behind that new address space.
> > > > > > > > >
> > > > > > > > > This iommu_get() fetches that guy behind the address space. Here, the
> > > > > > > > > guy is defined as IOMMUObject, which is currently a (void *). In the
> > > > > > > > > future, maybe we can make it a better definition, but imho it's good
> > > > > > > > > enough for now, considering it's arch-dependent.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > > > > > >
> > > > > > > > This doesn't make sense to me. It would be entirely possible for a
> > > > > > > > single address space to have different regions mapped by different
> > > > > > > > IOMMUs. Or some regions mapped by IOMMUs and others direct mapped to
> > > > > > > > a device or memory block.
> > > > > > >
> > > > > > > Oh, so it's more complicated than I thought... Then, do we really have
> > > > > > > existing use case that one device is managed by more than one IOMMU
> > > > > > > (on any of the platform)? Frankly speaking I haven't thought about
> > > > > > > complicated scenarios like this, or nested IOMMUs yet.
> > > > > >
> > > > > > Sort of, it depends what you count as "more than one IOMMU".
> > > > > >
> > > > > > spapr can - depending on guest configuration - have two IOMMU windows
> > > > > > for each guest PCI domain. In theory the guest can set these up
> > > > > > however it wants, in practice there's usually a small (~256MiB) at PCI
> > > > > > address 0 for the benefit of 32-bit PCI devices, then a much larger
> > > > > > window up at a high address to allow better performance for 64-bit
> > > > > > capable devices.
> > > > > >
> > > > > > Those are the same IOMMU in the sense that they're both implemented by
> > > > > > logic built into the same virtual PCI host bridge. However, they're
> > > > > > different IOMMUs in the sense that they have independent data
> > > > > > structures describing the mappings and are currently modelled as two
> > > > > > different IOMMU memory regions.
>
> [1]
>
> > > > > >
> > > > > >
> > > > > > I don't believe we have any existing platforms with both an IOMMU and
> > > > > > a direct mapped window in a device's address space. But it seems to
> > > > > > be just too plausible a setup to not plan for it. [1]
> > > > > >
> > > > > > > This patch derived from a requirement in virt-svm project (on x86).
> > > > > > > Virt-svm needs some notification mechanism for each IOMMU (or say, the
> > > > > > > IOMMU that managers the SVM-enabled device). For now, all IOMMU
> > > > > > > notifiers are per-memory-region not per-iommu, and that's imho not
> > > > > > > what virt-svm wants. Any suggestions?
> > > > > >
> > > > > > I don't know SVM, so I can't really make sense of that. What format
> > > > > > does this identifier need? What does "for one IOMMU" mean in this
> > > > > > context - i.e. what guest observable properties require the IDs to be
> > > > > > the same or to be different.
> > > > >
> > > > > Virt-svm should need to trap the content of a register (actually the
> > > > > data is in the memory, but, let's assume it's a mmio operation for
> > > > > simplicity, considering it is finally delivered via invalidation
> > > > > requests), then pass that info down to kernel. So the listened element
> > > > > is per-iommu not per-mr this time. When the content changed, vfio will
> > > > > need to be notified, then pass this info down.
> > > >
> > > > I don't entirely follow what you're saying. When the virtual hardware
> > > > gets an invalidate request, it looks up the unit to invalidate in
> > > > memory? Which component gets to decide that ID? How is it advertised
> > > > to the guest OS?
> > > >
> > > > If your ID is tied to the AS now, you could just iterate through the
> > > > AS and invalidate any IOMMU MRs that are present within it.
> > > >
> > > > Alternatively, if the ID is tied to something more concrete, like a
> > > > specific PCI host bridge (which incorporates the IOMMU logic), then
> > > > that device probably already has a handle on the right IOMMU MR to
> > > > invalidate it.
> > >
> > > Sorry to be unclear on the requirement. I don't know what's the ID you
> > > mentioned above... Anyway, let me try to further simplify the use
> > > case.
> >
> > Right, the ID was me guessing badly at what's going on here, so I
> > think it confused rather than clarifying.
> >
> > > Just assume we have such a requirement: when one register of vIOMMU
> > > changes, we need to pass this register data to the hardware IOMMU by
> > > some way. And, let's assume this is a notification mechanism, so that
> > > every device in the system can listen to this register change, then
> > > capture what has changed to what. Here the point is, in all cases this
> > > event is not related to memory region at all. So imho we need some
> > > other way to do it besides memory region IOMMU notifiers.
> >
> > Ok. So is this right?
> > * You have a single bank of vIOMMU registers
> > * Which control two (or more) IOMMU regions in in the guest's
> > address space
> > * Assuming the host also has an AMD IOMMU, those will be
> > backed by a single IOMMU on the host ("single" meaning
> > controlled by a single bank of host registers)
> >
> > I'm assuming the guest IOMMU code must know which IOMMU regions it is
> > managing, so getting from the guest registers to the set of IOMMU MRs
> > should be easy.
> >
> > What's the operation that needs to happen on the host IOMMU, in terms
> > of the VFIO IOMMU interface?
>
> (Sorry to respond so late...)
>
> It'll pass the captured data downward to host IOMMU in some way.
>
> IMHO if we are discussing the notifier thing only, we don't really
> need to know what would it do after it gets the data. The point is how
> we should define this kind if notifies, which differs from current
> memory region based notifiers.
I'm trying to understand how it differs - I still don't have a clear
picture. That's why I'm asking what needs to be passed to the host
MMU, so I can see why you need this different notifier.
> > Is this inherently only possible if both host and guest have an AMD
> > IOMMU? Could it be made to work if the guest had an AMD IOMMU but the
> > host had an Intel one, or the other way around?
> >
> > Would it make sense to have a single IOMMU MR in the guest, but
> > instead of mapping it whole into the guest address space, have two
> > (or more) alias MRs in the AS which each allow access to a portion of
> > the IOMMU MR?
>
> For these questions, again I don't know whether it'll affect how we
> design a notifier mechanism to the remapping unit... Would it really?
> Or maybe I missed anything?
>
> Till now, after I know the case for SPAPR you have explained [1]
> (thanks btw!), could I say that these multiple IOMMU windows still be
> backed by some unified hardware in the PCI host bridge? Can that be
> the so-called single "IOMMU" object behind that device? And, would it
> possible that we might have similar requirement in the future just
> like what Yi has met with virt-svm? (I don't know whether Power would
> support SVM or similar, but I guess ARM should support it?)
Well, yes, as I've said the two IOMMU windows in sPAPR are the same
IOMMU in the sense that they're implemented by basically the same
logic in the host bridge.
But what consitutes one or multiple IOMMUs all depends on your
definitions, and I'm still not understanding what about your structure
impacts on the notifier design.
> I am just thinking how we can define a better and general (for all
> platforms) IOMMU model in QEMU that can best suite our needs. And
> currently that should be a model that can satisfy Yi's requirement.
I'm still trying to wrap my head around what those requirements are.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 8/8] iommu: introduce hw/core/iommu
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 8/8] iommu: introduce hw/core/iommu Peter Xu
2017-04-28 10:01 ` Liu, Yi L
@ 2017-06-07 7:51 ` Liu, Yi L
2017-06-07 8:28 ` Peter Xu
1 sibling, 1 reply; 30+ messages in thread
From: Liu, Yi L @ 2017-06-07 7:51 UTC (permalink / raw)
To: Peter Xu, qemu-devel
Cc: Lan, Tianyu, Paolo Bonzini, Tian, Kevin, Jason Wang,
David Gibson, Alex Williamson
Hi Peter,
Some updates on it.
> -----Original Message-----
> From: Peter Xu [mailto:peterx@redhat.com]
> Sent: Thursday, April 27, 2017 5:34 PM
> To: qemu-devel@nongnu.org
> Cc: Lan, Tianyu <tianyu.lan@intel.com>; Paolo Bonzini <pbonzini@redhat.com>;
> Tian, Kevin <kevin.tian@intel.com>; Liu, Yi L <yi.l.liu@intel.com>;
> peterx@redhat.com; Jason Wang <jasowang@redhat.com>; David Gibson
> <david@gibson.dropbear.id.au>; Alex Williamson <alex.williamson@redhat.com>
> Subject: [RFC PATCH 8/8] iommu: introduce hw/core/iommu
>
> Time to consider a common stuff for IOMMU. Let's start from an common IOMMU
> object (which should be inlayed in custom IOMMU implementations) and a notifier
> mechanism.
>
> Let VT-d IOMMU be the first user.
>
> An example to use this per-iommu notifier:
>
> (when registering)
> iommu = address_space_iommu_get(pci_device_iommu_address_space(dev));
> notifier = iommu_notifier_register(iommu, IOMMU_EVENT_SVM_PASID, func);
> ...
> (when notify)
> IOMMUEvent event = { .type = IOMMU_EVENT_SVM_PASID ... };
> iommu_notify(iommu, &event);
> ...
> (when releasing)
> iommu_notifier_unregister(notifier);
> notifier = NULL;
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
[...]
> +#include "qemu/osdep.h"
> +#include "hw/core/iommu.h"
> +
> +IOMMUNotifier *iommu_notifier_register(IOMMUObject *iommu,
> + IOMMUNotifyFn fn,
> + uint64_t event_mask) {
> + IOMMUNotifier *notifier = g_new0(IOMMUNotifier, 1);
For this part, I think may need to consider to alloc the memory in a
similar way with IOMMUMRNotifier. The notifier surely needs to
be connect with vfio container so that it could manipulate
pIOMMU through vfio IOCTL.
I'm thinking of adding a new struct VFIOGuestIOMMUObject which
is similar to strcut VFIOGuestIOMMU. And have the original struct
VFIOGuestIOMMU modified to be VFIOGuestIOMMUMR.
Then there would be following definition in
"include\hw\vfio\vfio-common.h":
typedef struct VFIOGuestIOMMUObject {
VFIOContainer *container;
IOMMUObject *iommu;
IOMMUNotifier n; // n is for non-MemoryRegion related events, e.g. pasid table binding
QLIST_ENTRY(VFIOGuestIOMMUObject) giommu_next;
} VFIOGuestIOMMUObject;
typedef struct VFIOGuestIOMMUMR {
VFIOContainer *container;
MemoryRegion *iommu;
hwaddr iommu_offset;
IOMMUNotifier n;
QLIST_ENTRY(VFIOGuestIOMMUMR) giommu_next;
} VFIOGuestIOMMUMR;
How about your opinion?
Thanks,
Yi L
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 8/8] iommu: introduce hw/core/iommu
2017-06-07 7:51 ` Liu, Yi L
@ 2017-06-07 8:28 ` Peter Xu
0 siblings, 0 replies; 30+ messages in thread
From: Peter Xu @ 2017-06-07 8:28 UTC (permalink / raw)
To: Liu, Yi L
Cc: qemu-devel, Lan, Tianyu, Paolo Bonzini, Tian, Kevin, Jason Wang,
David Gibson, Alex Williamson
On Wed, Jun 07, 2017 at 07:51:55AM +0000, Liu, Yi L wrote:
> Hi Peter,
>
> Some updates on it.
>
> > -----Original Message-----
> > From: Peter Xu [mailto:peterx@redhat.com]
> > Sent: Thursday, April 27, 2017 5:34 PM
> > To: qemu-devel@nongnu.org
> > Cc: Lan, Tianyu <tianyu.lan@intel.com>; Paolo Bonzini <pbonzini@redhat.com>;
> > Tian, Kevin <kevin.tian@intel.com>; Liu, Yi L <yi.l.liu@intel.com>;
> > peterx@redhat.com; Jason Wang <jasowang@redhat.com>; David Gibson
> > <david@gibson.dropbear.id.au>; Alex Williamson <alex.williamson@redhat.com>
> > Subject: [RFC PATCH 8/8] iommu: introduce hw/core/iommu
> >
> > Time to consider a common stuff for IOMMU. Let's start from an common IOMMU
> > object (which should be inlayed in custom IOMMU implementations) and a notifier
> > mechanism.
> >
> > Let VT-d IOMMU be the first user.
> >
> > An example to use this per-iommu notifier:
> >
> > (when registering)
> > iommu = address_space_iommu_get(pci_device_iommu_address_space(dev));
> > notifier = iommu_notifier_register(iommu, IOMMU_EVENT_SVM_PASID, func);
> > ...
> > (when notify)
> > IOMMUEvent event = { .type = IOMMU_EVENT_SVM_PASID ... };
> > iommu_notify(iommu, &event);
> > ...
> > (when releasing)
> > iommu_notifier_unregister(notifier);
> > notifier = NULL;
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
>
> [...]
>
> > +#include "qemu/osdep.h"
> > +#include "hw/core/iommu.h"
> > +
> > +IOMMUNotifier *iommu_notifier_register(IOMMUObject *iommu,
> > + IOMMUNotifyFn fn,
> > + uint64_t event_mask) {
> > + IOMMUNotifier *notifier = g_new0(IOMMUNotifier, 1);
>
> For this part, I think may need to consider to alloc the memory in a
> similar way with IOMMUMRNotifier. The notifier surely needs to
> be connect with vfio container so that it could manipulate
> pIOMMU through vfio IOCTL.
Hmm yes. Or we can add one more parameter for it?
IOMMUNotifier *iommu_notifier_register(IOMMUObject *iommu,
IOMMUNotifyFn fn,
void *private,
uint64_t event_mask);
Both works imho.
>
> I'm thinking of adding a new struct VFIOGuestIOMMUObject which
> is similar to strcut VFIOGuestIOMMU. And have the original struct
> VFIOGuestIOMMU modified to be VFIOGuestIOMMUMR.
>
> Then there would be following definition in
> "include\hw\vfio\vfio-common.h":
>
> typedef struct VFIOGuestIOMMUObject {
> VFIOContainer *container;
> IOMMUObject *iommu;
> IOMMUNotifier n; // n is for non-MemoryRegion related events, e.g. pasid table binding
> QLIST_ENTRY(VFIOGuestIOMMUObject) giommu_next;
> } VFIOGuestIOMMUObject;
>
> typedef struct VFIOGuestIOMMUMR {
> VFIOContainer *container;
> MemoryRegion *iommu;
> hwaddr iommu_offset;
> IOMMUNotifier n;
> QLIST_ENTRY(VFIOGuestIOMMUMR) giommu_next;
> } VFIOGuestIOMMUMR;
>
> How about your opinion?
Currently this series "blocks" at the assumption that "for each
address space we have one IOMMU backend". If you see this series, it
did not do too much thing: it tried to get the IOMMU object for a
device, then it added a notifier for it. David proposed possible model
that is against this, say, is it possible that there are more than one
IOMMUs behind one device? So before we move on to "how VFIO will
tackle with this interface", we may need to first settle down on how
we can provide such a IOMMU-oriented notifier that no one dislike.
IMHO this series may still be okay before we move on to more
complicated IOMMU models, however I cannot really persuade David since
my reasoning is not strong enough. :-)
Maybe you can think out something better so that everyone will like?
Any thoughts?
(I think I'll move my "investigate system IOMMU hierachy" TODO item
with higher priority - imho we need to know exactly all the scenarios
of IOMMU usage, like nested IOMMU? parallel IOMMU to separate
translation window? etc. only after that could we finally provide a
general and good vIOMMU framework for QEMU, then the notifier thing
should be far easier)
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 6/8] memory: introduce AddressSpaceOps
2017-05-26 5:30 ` David Gibson
@ 2017-06-08 8:24 ` Liu, Yi L
0 siblings, 0 replies; 30+ messages in thread
From: Liu, Yi L @ 2017-06-08 8:24 UTC (permalink / raw)
To: David Gibson, Peter Xu
Cc: qemu-devel, Lan, Tianyu, Paolo Bonzini, Tian, Kevin, Jason Wang,
Alex Williamson
Hi David,
I added some update on your question. See if it make any sense.
> -----Original Message-----
> From: David Gibson [mailto:david@gibson.dropbear.id.au]
> Sent: Friday, May 26, 2017 1:30 PM
> To: Peter Xu <peterx@redhat.com>
> On Thu, May 25, 2017 at 03:24:30PM +0800, Peter Xu wrote:
[...]
> > > > > > >
> > > > > > > I don't believe we have any existing platforms with both an
> > > > > > > IOMMU and a direct mapped window in a device's address
> > > > > > > space. But it seems to be just too plausible a setup to not
> > > > > > > plan for it. [1]
> > > > > > >
> > > > > > > > This patch derived from a requirement in virt-svm project (on x86).
> > > > > > > > Virt-svm needs some notification mechanism for each IOMMU
> > > > > > > > (or say, the IOMMU that managers the SVM-enabled device).
> > > > > > > > For now, all IOMMU notifiers are per-memory-region not
> > > > > > > > per-iommu, and that's imho not what virt-svm wants. Any suggestions?
> > > > > > >
> > > > > > > I don't know SVM, so I can't really make sense of that.
> > > > > > > What format does this identifier need? What does "for one
> > > > > > > IOMMU" mean in this context - i.e. what guest observable
> > > > > > > properties require the IDs to be the same or to be different.
> > > > > >
> > > > > > Virt-svm should need to trap the content of a register
> > > > > > (actually the data is in the memory, but, let's assume it's a
> > > > > > mmio operation for simplicity, considering it is finally
> > > > > > delivered via invalidation requests), then pass that info down
> > > > > > to kernel. So the listened element is per-iommu not per-mr
> > > > > > this time. When the content changed, vfio will need to be notified, then
> pass this info down.
> > > > >
> > > > > I don't entirely follow what you're saying. When the virtual
> > > > > hardware gets an invalidate request, it looks up the unit to
> > > > > invalidate in memory? Which component gets to decide that ID?
> > > > > How is it advertised to the guest OS?
> > > > >
> > > > > If your ID is tied to the AS now, you could just iterate through
> > > > > the AS and invalidate any IOMMU MRs that are present within it.
> > > > >
> > > > > Alternatively, if the ID is tied to something more concrete,
> > > > > like a specific PCI host bridge (which incorporates the IOMMU
> > > > > logic), then that device probably already has a handle on the
> > > > > right IOMMU MR to invalidate it.
> > > >
> > > > Sorry to be unclear on the requirement. I don't know what's the ID
> > > > you mentioned above... Anyway, let me try to further simplify the
> > > > use case.
> > >
> > > Right, the ID was me guessing badly at what's going on here, so I
> > > think it confused rather than clarifying.
> > >
> > > > Just assume we have such a requirement: when one register of
> > > > vIOMMU changes, we need to pass this register data to the hardware
> > > > IOMMU by some way. And, let's assume this is a notification
> > > > mechanism, so that every device in the system can listen to this
> > > > register change, then capture what has changed to what. Here the
> > > > point is, in all cases this event is not related to memory region
> > > > at all. So imho we need some other way to do it besides memory region
> IOMMU notifiers.
> > >
> > > Ok. So is this right?
> > > * You have a single bank of vIOMMU registers
> > > * Which control two (or more) IOMMU regions in in the guest's
> > > address space
> > > * Assuming the host also has an AMD IOMMU, those will be
> > > backed by a single IOMMU on the host ("single" meaning
> > > controlled by a single bank of host registers)
> > >
> > > I'm assuming the guest IOMMU code must know which IOMMU regions it
> > > is managing, so getting from the guest registers to the set of IOMMU
> > > MRs should be easy.
> > >
> > > What's the operation that needs to happen on the host IOMMU, in
> > > terms of the VFIO IOMMU interface?
> >
> > (Sorry to respond so late...)
> >
> > It'll pass the captured data downward to host IOMMU in some way.
> >
> > IMHO if we are discussing the notifier thing only, we don't really
> > need to know what would it do after it gets the data. The point is how
> > we should define this kind if notifies, which differs from current
> > memory region based notifiers.
>
> I'm trying to understand how it differs - I still don't have a clear picture. That's why
> I'm asking what needs to be passed to the host MMU, so I can see why you need this
> different notifier.
It differs in the registration. For the MemoryRegion based notifier(MAP/UNMAP), it
is registered when IOMMU MemoryRegion is used. On VT-d, it is the time switch
system memory address space to vtd address space. This is introduced by the patch
below:
[Qemu-devel] [PATCH v9 8/9] intel_iommu: allow dynamic switch of IOMMU region
For virt-SVM, it wants to add two more notfiers. If we add the notifiers at the same
time with the MAP/UNMAP notifier, then virt-SVM cannot work if there is no IOMMU
region switch. This scenario happens when passthru-mode is used in the vIOMMU.
So in order to solve the scenario above, I discussed with Peter. My initial plan is to move
the registration of new notifiers to be in vfio_realize(). The logic is if vIOMMU is exposed
to guest, then the new notifiers would be registered. This approach is proposed in my
RFC patch for virt-SVM.
https://www.spinics.net/lists/kvm/msg148803.html
However, Peter mentioned the approach in the RFC patch may not be compatible with
sPAPR since there may be multiple MemoryRegion for sPARR. It would result in multiple
notifier registration on sPARR.
Due to it, Peter tries to introduce the IOMMUObject. It would help the detection in
vfio_realize() to check if vIOMMU is exposed to guest. Also it moves the iommu specific
notifiers to be tied on the exposure of vIOMMU instead of IOMMU Region switching.
Any further comments, pls let me know.
Thanks,
Yi L
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2017-06-08 8:24 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-27 9:34 [Qemu-devel] [RFC PATCH 0/8] IOMMU: introduce common IOMMUObject Peter Xu
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 1/8] memory: rename IOMMU_NOTIFIER_* Peter Xu
2017-05-01 4:50 ` David Gibson
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 2/8] memory: rename IOMMUNotifier Peter Xu
2017-05-01 4:51 ` David Gibson
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 3/8] memory: rename iommu_notifier_init() Peter Xu
2017-05-01 4:53 ` David Gibson
2017-05-08 5:50 ` Peter Xu
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 4/8] memory: rename *_notify_iommu* Peter Xu
2017-05-01 4:55 ` David Gibson
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 5/8] memory: rename *iommu_notifier* Peter Xu
2017-05-01 4:56 ` David Gibson
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 6/8] memory: introduce AddressSpaceOps Peter Xu
2017-05-01 4:58 ` David Gibson
2017-05-08 5:48 ` Peter Xu
2017-05-08 6:07 ` David Gibson
2017-05-08 7:32 ` Peter Xu
2017-05-07 9:44 ` Liu, Yi L
2017-05-10 7:04 ` David Gibson
2017-05-11 5:04 ` Peter Xu
2017-05-15 5:32 ` David Gibson
2017-05-25 7:24 ` Peter Xu
2017-05-26 5:30 ` David Gibson
2017-06-08 8:24 ` Liu, Yi L
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 7/8] intel_iommu: provide AddressSpaceOps.iommu_get() Peter Xu
2017-04-27 9:34 ` [Qemu-devel] [RFC PATCH 8/8] iommu: introduce hw/core/iommu Peter Xu
2017-04-28 10:01 ` Liu, Yi L
2017-04-28 10:34 ` Peter Xu
2017-06-07 7:51 ` Liu, Yi L
2017-06-07 8:28 ` Peter Xu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.