* [Qemu-devel] [PATCH v3 1/5] hostmem: make hostmem single, not per Vring related
2013-05-06 12:48 [Qemu-devel] [PATCH v3 0/5] proposal to make hostmem listener RAM unplug safe Liu Ping Fan
@ 2013-05-06 12:48 ` Liu Ping Fan
2013-05-06 12:48 ` [Qemu-devel] [PATCH v3 2/5] hostmem: AddressSpace has its own map and maintained by RCU prepared style Liu Ping Fan
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Liu Ping Fan @ 2013-05-06 12:48 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Anthony Liguori, Jan Kiszka, Vasilis Liaskovitis,
Stefan Hajnoczi, Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
The hwaddr and hva mapping relation is system wide, no need to
be created for each Vring
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
exec.c | 2 ++
hw/virtio/dataplane/hostmem.c | 33 +++++++++++++++++++--------------
hw/virtio/dataplane/vring.c | 11 ++++-------
include/hw/virtio/dataplane/hostmem.h | 13 +++++++++----
include/hw/virtio/dataplane/vring.h | 1 -
5 files changed, 34 insertions(+), 26 deletions(-)
diff --git a/exec.c b/exec.c
index fa1e0c3..1ec36a9 100644
--- a/exec.c
+++ b/exec.c
@@ -49,6 +49,7 @@
#include "translate-all.h"
#include "exec/memory-internal.h"
+#include "hw/virtio/dataplane/hostmem.h"
//#define DEBUG_UNASSIGNED
//#define DEBUG_SUBPAGE
@@ -1809,6 +1810,7 @@ static void memory_map_init(void)
memory_listener_register(&core_memory_listener, &address_space_memory);
memory_listener_register(&io_memory_listener, &address_space_io);
memory_listener_register(&tcg_memory_listener, &address_space_memory);
+ hostmem_init();
dma_context_init(&dma_context_memory, &address_space_memory,
NULL, NULL, NULL);
diff --git a/hw/virtio/dataplane/hostmem.c b/hw/virtio/dataplane/hostmem.c
index 37292ff..756b09f 100644
--- a/hw/virtio/dataplane/hostmem.c
+++ b/hw/virtio/dataplane/hostmem.c
@@ -14,6 +14,10 @@
#include "exec/address-spaces.h"
#include "hw/virtio/dataplane/hostmem.h"
+HostMem *system_mem;
+
+static void hostmem_finalize(void);
+
static int hostmem_lookup_cmp(const void *phys_, const void *region_)
{
hwaddr phys = *(const hwaddr *)phys_;
@@ -31,11 +35,12 @@ static int hostmem_lookup_cmp(const void *phys_, const void *region_)
/**
* Map guest physical address to host pointer
*/
-void *hostmem_lookup(HostMem *hostmem, hwaddr phys, hwaddr len, bool is_write)
+void *hostmem_lookup(hwaddr phys, hwaddr len, bool is_write)
{
HostMemRegion *region;
void *host_addr = NULL;
hwaddr offset_within_region;
+ HostMem *hostmem = system_mem;
qemu_mutex_lock(&hostmem->current_regions_lock);
region = bsearch(&phys, hostmem->current_regions,
@@ -137,13 +142,12 @@ static void hostmem_listener_coalesced_mmio_dummy(MemoryListener *listener,
{
}
-void hostmem_init(HostMem *hostmem)
+void hostmem_init(void)
{
- memset(hostmem, 0, sizeof(*hostmem));
+ system_mem = g_new0(HostMem, 1);
+ qemu_mutex_init(&system_mem->current_regions_lock);
- qemu_mutex_init(&hostmem->current_regions_lock);
-
- hostmem->listener = (MemoryListener){
+ system_mem->listener = (MemoryListener) {
.begin = hostmem_listener_dummy,
.commit = hostmem_listener_commit,
.region_add = hostmem_listener_append_region,
@@ -161,16 +165,17 @@ void hostmem_init(HostMem *hostmem)
.priority = 10,
};
- memory_listener_register(&hostmem->listener, &address_space_memory);
- if (hostmem->num_new_regions > 0) {
- hostmem_listener_commit(&hostmem->listener);
+ memory_listener_register(&system_mem->listener, &address_space_memory);
+ if (system_mem->num_new_regions > 0) {
+ hostmem_listener_commit(&system_mem->listener);
}
+ atexit(hostmem_finalize);
}
-void hostmem_finalize(HostMem *hostmem)
+static void hostmem_finalize(void)
{
- memory_listener_unregister(&hostmem->listener);
- g_free(hostmem->new_regions);
- g_free(hostmem->current_regions);
- qemu_mutex_destroy(&hostmem->current_regions_lock);
+ memory_listener_unregister(&system_mem->listener);
+ g_free(system_mem->new_regions);
+ g_free(system_mem->current_regions);
+ qemu_mutex_destroy(&system_mem->current_regions_lock);
}
diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c
index e0d6e83..4d6d735 100644
--- a/hw/virtio/dataplane/vring.c
+++ b/hw/virtio/dataplane/vring.c
@@ -27,8 +27,7 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n)
vring->broken = false;
- hostmem_init(&vring->hostmem);
- vring_ptr = hostmem_lookup(&vring->hostmem, vring_addr, vring_size, true);
+ vring_ptr = hostmem_lookup(vring_addr, vring_size, true);
if (!vring_ptr) {
error_report("Failed to map vring "
"addr %#" HWADDR_PRIx " size %" HWADDR_PRIu,
@@ -51,7 +50,6 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n)
void vring_teardown(Vring *vring)
{
- hostmem_finalize(&vring->hostmem);
}
/* Disable guest->host notifies */
@@ -138,8 +136,7 @@ static int get_indirect(Vring *vring,
struct vring_desc *desc_ptr;
/* Translate indirect descriptor */
- desc_ptr = hostmem_lookup(&vring->hostmem,
- indirect->addr + found * sizeof(desc),
+ desc_ptr = hostmem_lookup(indirect->addr + found * sizeof(desc),
sizeof(desc), false);
if (!desc_ptr) {
error_report("Failed to map indirect descriptor "
@@ -172,7 +169,7 @@ static int get_indirect(Vring *vring,
return -ENOBUFS;
}
- iov->iov_base = hostmem_lookup(&vring->hostmem, desc.addr, desc.len,
+ iov->iov_base = hostmem_lookup(desc.addr, desc.len,
desc.flags & VRING_DESC_F_WRITE);
if (!iov->iov_base) {
error_report("Failed to map indirect descriptor"
@@ -300,7 +297,7 @@ int vring_pop(VirtIODevice *vdev, Vring *vring,
}
/* TODO handle non-contiguous memory across region boundaries */
- iov->iov_base = hostmem_lookup(&vring->hostmem, desc.addr, desc.len,
+ iov->iov_base = hostmem_lookup(desc.addr, desc.len,
desc.flags & VRING_DESC_F_WRITE);
if (!iov->iov_base) {
error_report("Failed to map vring desc addr %#" PRIx64 " len %u",
diff --git a/include/hw/virtio/dataplane/hostmem.h b/include/hw/virtio/dataplane/hostmem.h
index b2cf093..7999991 100644
--- a/include/hw/virtio/dataplane/hostmem.h
+++ b/include/hw/virtio/dataplane/hostmem.h
@@ -14,6 +14,7 @@
#ifndef HOSTMEM_H
#define HOSTMEM_H
+#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
#include "exec/memory.h"
#include "qemu/thread.h"
@@ -41,9 +42,6 @@ typedef struct {
size_t num_current_regions;
} HostMem;
-void hostmem_init(HostMem *hostmem);
-void hostmem_finalize(HostMem *hostmem);
-
/**
* Map a guest physical address to a pointer
*
@@ -52,6 +50,13 @@ void hostmem_finalize(HostMem *hostmem);
* can be done with other mechanisms like bdrv_drain_all() that quiesce
* in-flight I/O.
*/
-void *hostmem_lookup(HostMem *hostmem, hwaddr phys, hwaddr len, bool is_write);
+void *hostmem_lookup(hwaddr phys, hwaddr len, bool is_write);
+void hostmem_init(void);
+
+#else
+void hostmem_init(void)
+{
+}
+#endif
#endif /* HOSTMEM_H */
diff --git a/include/hw/virtio/dataplane/vring.h b/include/hw/virtio/dataplane/vring.h
index 9380cb5..56acffb 100644
--- a/include/hw/virtio/dataplane/vring.h
+++ b/include/hw/virtio/dataplane/vring.h
@@ -23,7 +23,6 @@
#include "hw/virtio/virtio.h"
typedef struct {
- HostMem hostmem; /* guest memory mapper */
struct vring vr; /* virtqueue vring mapped to host memory */
uint16_t last_avail_idx; /* last processed avail ring index */
uint16_t last_used_idx; /* last processed used ring index */
--
1.7.4.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH v3 2/5] hostmem: AddressSpace has its own map and maintained by RCU prepared style
2013-05-06 12:48 [Qemu-devel] [PATCH v3 0/5] proposal to make hostmem listener RAM unplug safe Liu Ping Fan
2013-05-06 12:48 ` [Qemu-devel] [PATCH v3 1/5] hostmem: make hostmem single, not per Vring related Liu Ping Fan
@ 2013-05-06 12:48 ` Liu Ping Fan
2013-05-06 12:48 ` [Qemu-devel] [PATCH v3 3/5] memory: add ref/unref interface for MemroyRegionOps Liu Ping Fan
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Liu Ping Fan @ 2013-05-06 12:48 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Anthony Liguori, Jan Kiszka, Vasilis Liaskovitis,
Stefan Hajnoczi, Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
Each address space will have a map between its hwaddr and hva,
this is expressed as the struct AddrSpaceMem. Currently only
address space of system memory's map is used by virtio device,
and the map is stored in AddrSpaceMem *system_mem.
The map is maintained by RCU prepared style, cur_hostmem,
next_hostmem, cur_lock fields in AddrSpaceMem help to access
the aim. cur_hostmem is used to search, next_hostmem is used to
update and will substitue cur_hostmem when done.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
hw/virtio/dataplane/hostmem.c | 133 +++++++++++++++++++++++----------
include/hw/virtio/dataplane/hostmem.h | 20 +++--
2 files changed, 103 insertions(+), 50 deletions(-)
diff --git a/hw/virtio/dataplane/hostmem.c b/hw/virtio/dataplane/hostmem.c
index 756b09f..f31a703 100644
--- a/hw/virtio/dataplane/hostmem.c
+++ b/hw/virtio/dataplane/hostmem.c
@@ -14,7 +14,7 @@
#include "exec/address-spaces.h"
#include "hw/virtio/dataplane/hostmem.h"
-HostMem *system_mem;
+static AddrSpaceMem *system_mem;
static void hostmem_finalize(void);
@@ -32,17 +32,39 @@ static int hostmem_lookup_cmp(const void *phys_, const void *region_)
}
}
-/**
- * Map guest physical address to host pointer
- */
-void *hostmem_lookup(hwaddr phys, hwaddr len, bool is_write)
+static void hostmem_ref(HostMem *hostmem)
+{
+ int t;
+
+ t = __sync_add_and_fetch(&hostmem->ref, 1);
+ assert(t > 0);
+}
+
+static void hostmem_unref(HostMem *hostmem)
+{
+ int t;
+
+ t = __sync_sub_and_fetch(&hostmem->ref, 1);
+ assert(t >= 0);
+ if (!t) {
+ g_free(hostmem->current_regions);
+ g_free(hostmem);
+ }
+}
+
+static void *address_space_mem_lookup(AddrSpaceMem *as_mem, hwaddr phys,
+ hwaddr len, bool is_write)
{
HostMemRegion *region;
void *host_addr = NULL;
hwaddr offset_within_region;
- HostMem *hostmem = system_mem;
+ HostMem *hostmem;
+
+ qemu_mutex_lock(&as_mem->cur_lock);
+ hostmem = as_mem->cur_hostmem;
+ hostmem_ref(hostmem);
+ qemu_mutex_unlock(&as_mem->cur_lock);
- qemu_mutex_lock(&hostmem->current_regions_lock);
region = bsearch(&phys, hostmem->current_regions,
hostmem->num_current_regions,
sizeof(hostmem->current_regions[0]),
@@ -57,28 +79,45 @@ void *hostmem_lookup(hwaddr phys, hwaddr len, bool is_write)
if (len <= region->size - offset_within_region) {
host_addr = region->host_addr + offset_within_region;
}
-out:
- qemu_mutex_unlock(&hostmem->current_regions_lock);
+out:
+ hostmem_unref(hostmem);
return host_addr;
}
/**
- * Install new regions list
+ * Map guest physical address to host pointer
*/
-static void hostmem_listener_commit(MemoryListener *listener)
+void *hostmem_lookup(hwaddr phys, hwaddr len, bool is_write)
{
- HostMem *hostmem = container_of(listener, HostMem, listener);
+ return address_space_mem_lookup(system_mem, phys, len, is_write);
+}
- qemu_mutex_lock(&hostmem->current_regions_lock);
- g_free(hostmem->current_regions);
- hostmem->current_regions = hostmem->new_regions;
- hostmem->num_current_regions = hostmem->num_new_regions;
- qemu_mutex_unlock(&hostmem->current_regions_lock);
+static void hostmem_listener_begin(MemoryListener *listener)
+{
+ AddrSpaceMem *as_mem = container_of(listener, AddrSpaceMem, listener);
+
+ as_mem->next_hostmem = g_new0(HostMem, 1);
+ as_mem->next_hostmem->ref = 1;
+}
- /* Reset new regions list */
- hostmem->new_regions = NULL;
- hostmem->num_new_regions = 0;
+/**
+ * Install new regions list
+ */
+static void hostmem_listener_commit(MemoryListener *listener)
+{
+ HostMem *tmp;
+ AddrSpaceMem *as_mem = container_of(listener, AddrSpaceMem, listener);
+
+ /* writer of cur_hostmem &next_hostmem is serialized by biglock
+ * in hotplug path. So only take care of r/w on cur_hostmem
+ */
+ tmp = as_mem->cur_hostmem;
+ qemu_mutex_lock(&as_mem->cur_lock);
+ as_mem->cur_hostmem = as_mem->next_hostmem;
+ qemu_mutex_unlock(&as_mem->cur_lock);
+ as_mem->next_hostmem = NULL;
+ hostmem_unref(tmp);
}
/**
@@ -88,23 +127,23 @@ static void hostmem_append_new_region(HostMem *hostmem,
MemoryRegionSection *section)
{
void *ram_ptr = memory_region_get_ram_ptr(section->mr);
- size_t num = hostmem->num_new_regions;
- size_t new_size = (num + 1) * sizeof(hostmem->new_regions[0]);
+ size_t num = hostmem->num_current_regions;
+ size_t new_size = (num + 1) * sizeof(hostmem->current_regions[0]);
- hostmem->new_regions = g_realloc(hostmem->new_regions, new_size);
- hostmem->new_regions[num] = (HostMemRegion){
+ hostmem->current_regions = g_realloc(hostmem->current_regions, new_size);
+ hostmem->current_regions[num] = (HostMemRegion){
.host_addr = ram_ptr + section->offset_within_region,
.guest_addr = section->offset_within_address_space,
.size = section->size,
.readonly = section->readonly,
};
- hostmem->num_new_regions++;
+ hostmem->num_current_regions++;
}
static void hostmem_listener_append_region(MemoryListener *listener,
MemoryRegionSection *section)
{
- HostMem *hostmem = container_of(listener, HostMem, listener);
+ AddrSpaceMem *as_mem = container_of(listener, AddrSpaceMem, listener);
/* Ignore non-RAM regions, we may not be able to map them */
if (!memory_region_is_ram(section->mr)) {
@@ -116,7 +155,7 @@ static void hostmem_listener_append_region(MemoryListener *listener,
return;
}
- hostmem_append_new_region(hostmem, section);
+ hostmem_append_new_region(as_mem->next_hostmem, section);
}
/* We don't implement most MemoryListener callbacks, use these nop stubs */
@@ -142,13 +181,13 @@ static void hostmem_listener_coalesced_mmio_dummy(MemoryListener *listener,
{
}
-void hostmem_init(void)
+static AddrSpaceMem *address_space_mem_create(AddressSpace *as)
{
- system_mem = g_new0(HostMem, 1);
- qemu_mutex_init(&system_mem->current_regions_lock);
+ AddrSpaceMem *as_mem = g_new0(AddrSpaceMem, 1);
- system_mem->listener = (MemoryListener) {
- .begin = hostmem_listener_dummy,
+ qemu_mutex_init(&as_mem->cur_lock);
+ as_mem->listener = (MemoryListener) {
+ .begin = hostmem_listener_begin,
.commit = hostmem_listener_commit,
.region_add = hostmem_listener_append_region,
.region_del = hostmem_listener_section_dummy,
@@ -164,18 +203,30 @@ void hostmem_init(void)
.coalesced_mmio_del = hostmem_listener_coalesced_mmio_dummy,
.priority = 10,
};
+ as_mem->cur_hostmem = g_new0(HostMem, 1);
+ as_mem->cur_hostmem->ref = 1;
+ memory_listener_register(&as_mem->listener, as);
- memory_listener_register(&system_mem->listener, &address_space_memory);
- if (system_mem->num_new_regions > 0) {
- hostmem_listener_commit(&system_mem->listener);
- }
+ return as_mem;
+}
+
+static void address_space_mem_destroy(AddrSpaceMem *as_mem)
+{
+ memory_listener_unregister(&as_mem->listener);
+ qemu_mutex_destroy(&as_mem->cur_lock);
+ hostmem_unref(as_mem->cur_hostmem);
+ assert(!as_mem->next_hostmem);
+ g_free(as_mem);
+}
+
+void hostmem_init(void)
+{
+ system_mem = address_space_mem_create(&address_space_memory);
atexit(hostmem_finalize);
}
-static void hostmem_finalize(void)
+void hostmem_finalize(void)
{
- memory_listener_unregister(&system_mem->listener);
- g_free(system_mem->new_regions);
- g_free(system_mem->current_regions);
- qemu_mutex_destroy(&system_mem->current_regions_lock);
+ address_space_mem_destroy(system_mem);
+ system_mem = NULL;
}
diff --git a/include/hw/virtio/dataplane/hostmem.h b/include/hw/virtio/dataplane/hostmem.h
index 7999991..d04ce55 100644
--- a/include/hw/virtio/dataplane/hostmem.h
+++ b/include/hw/virtio/dataplane/hostmem.h
@@ -29,19 +29,21 @@ typedef struct {
/* The listener is invoked when regions change and a new list of regions is
* built up completely before they are installed.
*/
- MemoryListener listener;
- HostMemRegion *new_regions;
- size_t num_new_regions;
-
- /* Current regions are accessed from multiple threads either to lookup
- * addresses or to install a new list of regions. The lock protects the
- * pointer and the regions.
- */
- QemuMutex current_regions_lock;
+ int ref;
HostMemRegion *current_regions;
size_t num_current_regions;
} HostMem;
+/* Helper to map address space's hw_addr to hva */
+typedef struct {
+ /* lock to protect the r/w access to cur_hostmem */
+ QemuMutex cur_lock;
+ /* switch from cur_hostmem to next_hostmem to adopt RCU */
+ HostMem *cur_hostmem;
+ HostMem *next_hostmem;
+ MemoryListener listener;
+} AddrSpaceMem;
+
/**
* Map a guest physical address to a pointer
*
--
1.7.4.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH v3 3/5] memory: add ref/unref interface for MemroyRegionOps
2013-05-06 12:48 [Qemu-devel] [PATCH v3 0/5] proposal to make hostmem listener RAM unplug safe Liu Ping Fan
2013-05-06 12:48 ` [Qemu-devel] [PATCH v3 1/5] hostmem: make hostmem single, not per Vring related Liu Ping Fan
2013-05-06 12:48 ` [Qemu-devel] [PATCH v3 2/5] hostmem: AddressSpace has its own map and maintained by RCU prepared style Liu Ping Fan
@ 2013-05-06 12:48 ` Liu Ping Fan
2013-05-06 12:48 ` [Qemu-devel] [PATCH v3 4/5] hostmem: hostmem listener pin RAM-Device by refcnt Liu Ping Fan
2013-05-06 12:49 ` [Qemu-devel] [PATCH v3 5/5] Vring: use hostmem's RAM safe api Liu Ping Fan
4 siblings, 0 replies; 6+ messages in thread
From: Liu Ping Fan @ 2013-05-06 12:48 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Anthony Liguori, Jan Kiszka, Vasilis Liaskovitis,
Stefan Hajnoczi, Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
This pair of interface are optinal, except for those device which is
used outside the biglock's protection for hot unplug. Currently,
HostMem used by virtio-blk dataplane is outside biglock, so the RAM
device should implement this.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
include/exec/memory.h | 10 ++++++++++
memory.c | 18 ++++++++++++++++++
2 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 9e88320..7e38fc1 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -54,6 +54,12 @@ struct MemoryRegionIORange {
* Memory region callbacks
*/
struct MemoryRegionOps {
+
+ /* ref/unref pair is optional; ref.
+ * inc refcnt of object who store MemoryRegion
+ */
+ void (*ref)(void);
+ void (*unref)(void);
/* Read from the memory region. @addr is relative to @mr; @size is
* in bytes. */
uint64_t (*read)(void *opaque,
@@ -223,6 +229,10 @@ struct MemoryListener {
QTAILQ_ENTRY(MemoryListener) link;
};
+/**/
+bool memory_region_ref(MemoryRegion *mr);
+bool memory_region_unref(MemoryRegion *mr);
+
/**
* memory_region_init: Initialize a memory region
*
diff --git a/memory.c b/memory.c
index 75ca281..c29998d 100644
--- a/memory.c
+++ b/memory.c
@@ -786,6 +786,24 @@ static bool memory_region_wrong_endianness(MemoryRegion *mr)
#endif
}
+bool memory_region_ref(MemoryRegion *mr)
+{
+ if (mr->ops && mr->ops->ref) {
+ mr->ops->ref();
+ return true;
+ }
+ return false;
+}
+
+bool memory_region_unref(MemoryRegion *mr)
+{
+ if (mr->ops && mr->ops->unref) {
+ mr->ops->unref();
+ return true;
+ }
+ return false;
+}
+
void memory_region_init(MemoryRegion *mr,
const char *name,
uint64_t size)
--
1.7.4.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH v3 4/5] hostmem: hostmem listener pin RAM-Device by refcnt
2013-05-06 12:48 [Qemu-devel] [PATCH v3 0/5] proposal to make hostmem listener RAM unplug safe Liu Ping Fan
` (2 preceding siblings ...)
2013-05-06 12:48 ` [Qemu-devel] [PATCH v3 3/5] memory: add ref/unref interface for MemroyRegionOps Liu Ping Fan
@ 2013-05-06 12:48 ` Liu Ping Fan
2013-05-06 12:49 ` [Qemu-devel] [PATCH v3 5/5] Vring: use hostmem's RAM safe api Liu Ping Fan
4 siblings, 0 replies; 6+ messages in thread
From: Liu Ping Fan @ 2013-05-06 12:48 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Anthony Liguori, Jan Kiszka, Vasilis Liaskovitis,
Stefan Hajnoczi, Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
With ref()/unref() interface of MemoryRegion, we can pin RAM-Device
when using its memory, and release it when done.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
hw/virtio/dataplane/hostmem.c | 24 +++++++++++++++++++-----
hw/virtio/dataplane/vring.c | 8 ++++----
include/hw/virtio/dataplane/hostmem.h | 4 +++-
3 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/hw/virtio/dataplane/hostmem.c b/hw/virtio/dataplane/hostmem.c
index f31a703..420c9be 100644
--- a/hw/virtio/dataplane/hostmem.c
+++ b/hw/virtio/dataplane/hostmem.c
@@ -42,18 +42,23 @@ static void hostmem_ref(HostMem *hostmem)
static void hostmem_unref(HostMem *hostmem)
{
- int t;
+ int i, t;
+ HostMemRegion *hmr;
t = __sync_sub_and_fetch(&hostmem->ref, 1);
assert(t >= 0);
if (!t) {
+ for (i = 0; i < hostmem->num_current_regions; i++) {
+ hmr = &hostmem->current_regions[i];
+ memory_region_unref(hmr->mr);
+ }
g_free(hostmem->current_regions);
g_free(hostmem);
}
}
static void *address_space_mem_lookup(AddrSpaceMem *as_mem, hwaddr phys,
- hwaddr len, bool is_write)
+ hwaddr len, MemoryRegion **mr, bool is_write)
{
HostMemRegion *region;
void *host_addr = NULL;
@@ -65,6 +70,9 @@ static void *address_space_mem_lookup(AddrSpaceMem *as_mem, hwaddr phys,
hostmem_ref(hostmem);
qemu_mutex_unlock(&as_mem->cur_lock);
+ if (mr) {
+ *mr = NULL;
+ }
region = bsearch(&phys, hostmem->current_regions,
hostmem->num_current_regions,
sizeof(hostmem->current_regions[0]),
@@ -79,7 +87,10 @@ static void *address_space_mem_lookup(AddrSpaceMem *as_mem, hwaddr phys,
if (len <= region->size - offset_within_region) {
host_addr = region->host_addr + offset_within_region;
}
-
+ if (mr) {
+ *mr = region->mr;
+ memory_region_ref(*mr);
+ }
out:
hostmem_unref(hostmem);
return host_addr;
@@ -88,9 +99,10 @@ out:
/**
* Map guest physical address to host pointer
*/
-void *hostmem_lookup(hwaddr phys, hwaddr len, bool is_write)
+void *hostmem_lookup(hwaddr phys, hwaddr len, MemoryRegion **mr,
+ bool is_write)
{
- return address_space_mem_lookup(system_mem, phys, len, is_write);
+ return address_space_mem_lookup(system_mem, phys, len, mr, is_write);
}
static void hostmem_listener_begin(MemoryListener *listener)
@@ -134,6 +146,7 @@ static void hostmem_append_new_region(HostMem *hostmem,
hostmem->current_regions[num] = (HostMemRegion){
.host_addr = ram_ptr + section->offset_within_region,
.guest_addr = section->offset_within_address_space,
+ .mr = section->mr,
.size = section->size,
.readonly = section->readonly,
};
@@ -155,6 +168,7 @@ static void hostmem_listener_append_region(MemoryListener *listener,
return;
}
+ memory_region_ref(section->mr);
hostmem_append_new_region(as_mem->next_hostmem, section);
}
diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c
index 4d6d735..e3c3afb 100644
--- a/hw/virtio/dataplane/vring.c
+++ b/hw/virtio/dataplane/vring.c
@@ -27,7 +27,7 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n)
vring->broken = false;
- vring_ptr = hostmem_lookup(vring_addr, vring_size, true);
+ vring_ptr = hostmem_lookup(vring_addr, vring_size, NULL, true);
if (!vring_ptr) {
error_report("Failed to map vring "
"addr %#" HWADDR_PRIx " size %" HWADDR_PRIu,
@@ -137,7 +137,7 @@ static int get_indirect(Vring *vring,
/* Translate indirect descriptor */
desc_ptr = hostmem_lookup(indirect->addr + found * sizeof(desc),
- sizeof(desc), false);
+ sizeof(desc), NULL, false);
if (!desc_ptr) {
error_report("Failed to map indirect descriptor "
"addr %#" PRIx64 " len %zu",
@@ -169,7 +169,7 @@ static int get_indirect(Vring *vring,
return -ENOBUFS;
}
- iov->iov_base = hostmem_lookup(desc.addr, desc.len,
+ iov->iov_base = hostmem_lookup(desc.addr, desc.len, NULL,
desc.flags & VRING_DESC_F_WRITE);
if (!iov->iov_base) {
error_report("Failed to map indirect descriptor"
@@ -297,7 +297,7 @@ int vring_pop(VirtIODevice *vdev, Vring *vring,
}
/* TODO handle non-contiguous memory across region boundaries */
- iov->iov_base = hostmem_lookup(desc.addr, desc.len,
+ iov->iov_base = hostmem_lookup(desc.addr, desc.len, NULL,
desc.flags & VRING_DESC_F_WRITE);
if (!iov->iov_base) {
error_report("Failed to map vring desc addr %#" PRIx64 " len %u",
diff --git a/include/hw/virtio/dataplane/hostmem.h b/include/hw/virtio/dataplane/hostmem.h
index d04ce55..094102b 100644
--- a/include/hw/virtio/dataplane/hostmem.h
+++ b/include/hw/virtio/dataplane/hostmem.h
@@ -21,6 +21,7 @@
typedef struct {
void *host_addr;
hwaddr guest_addr;
+ MemoryRegion *mr;
uint64_t size;
bool readonly;
} HostMemRegion;
@@ -52,7 +53,8 @@ typedef struct {
* can be done with other mechanisms like bdrv_drain_all() that quiesce
* in-flight I/O.
*/
-void *hostmem_lookup(hwaddr phys, hwaddr len, bool is_write);
+void *hostmem_lookup(hwaddr phys, hwaddr len, MemoryRegion **mr,
+ bool is_write);
void hostmem_init(void);
#else
--
1.7.4.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH v3 5/5] Vring: use hostmem's RAM safe api
2013-05-06 12:48 [Qemu-devel] [PATCH v3 0/5] proposal to make hostmem listener RAM unplug safe Liu Ping Fan
` (3 preceding siblings ...)
2013-05-06 12:48 ` [Qemu-devel] [PATCH v3 4/5] hostmem: hostmem listener pin RAM-Device by refcnt Liu Ping Fan
@ 2013-05-06 12:49 ` Liu Ping Fan
4 siblings, 0 replies; 6+ messages in thread
From: Liu Ping Fan @ 2013-05-06 12:49 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Anthony Liguori, Jan Kiszka, Vasilis Liaskovitis,
Stefan Hajnoczi, Paolo Bonzini
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
Before mm-ops done, we should gurantee the validaion of regions which is
used by Vring self and the chunck pointed by vring desc. We acheive
this goal by inc refcnt of RAM-Device. When finished, we dec this cnt
through the interface of MemoryRegion.
We keep the MemoryRegion's reference info totally in Vring, so the caller
can not be aware of the reference.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
hw/block/dataplane/virtio-blk.c | 8 ---
hw/virtio/dataplane/vring.c | 93 +++++++++++++++++++++++++++-------
include/hw/virtio/dataplane/vring.h | 15 ++++++
3 files changed, 89 insertions(+), 27 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 0356665..4babda1 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -25,14 +25,6 @@
#include "block/aio.h"
#include "hw/virtio/virtio-bus.h"
-enum {
- SEG_MAX = 126, /* maximum number of I/O segments */
- VRING_MAX = SEG_MAX + 2, /* maximum number of vring descriptors */
- REQ_MAX = VRING_MAX, /* maximum number of requests in the vring,
- * is VRING_MAX / 2 with traditional and
- * VRING_MAX with indirect descriptors */
-};
-
typedef struct {
struct iocb iocb; /* Linux AIO control block */
QEMUIOVector *inhdr; /* iovecs for virtio_blk_inhdr */
diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c
index e3c3afb..2cfd6d0 100644
--- a/hw/virtio/dataplane/vring.c
+++ b/hw/virtio/dataplane/vring.c
@@ -27,7 +27,7 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n)
vring->broken = false;
- vring_ptr = hostmem_lookup(vring_addr, vring_size, NULL, true);
+ vring_ptr = hostmem_lookup(vring_addr, vring_size, &vring->vring_mr, true);
if (!vring_ptr) {
error_report("Failed to map vring "
"addr %#" HWADDR_PRIx " size %" HWADDR_PRIu,
@@ -50,6 +50,7 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n)
void vring_teardown(Vring *vring)
{
+ memory_region_unref(vring->vring_mr);
}
/* Disable guest->host notifies */
@@ -109,11 +110,14 @@ bool vring_should_notify(VirtIODevice *vdev, Vring *vring)
static int get_indirect(Vring *vring,
struct iovec iov[], struct iovec *iov_end,
unsigned int *out_num, unsigned int *in_num,
- struct vring_desc *indirect)
+ struct vring_desc *indirect,
+ MemoryRegion ***mrs)
{
struct vring_desc desc;
unsigned int i = 0, count, found = 0;
-
+ MemoryRegion **cur = *mrs;
+ int ret = 0;
+ MemoryRegion *desc_mr;
/* Sanity check */
if (unlikely(indirect->len % sizeof(desc))) {
error_report("Invalid length in indirect descriptor: "
@@ -137,49 +141,58 @@ static int get_indirect(Vring *vring,
/* Translate indirect descriptor */
desc_ptr = hostmem_lookup(indirect->addr + found * sizeof(desc),
- sizeof(desc), NULL, false);
+ sizeof(desc),
+ &desc_mr,
+ false);
if (!desc_ptr) {
error_report("Failed to map indirect descriptor "
"addr %#" PRIx64 " len %zu",
(uint64_t)indirect->addr + found * sizeof(desc),
sizeof(desc));
vring->broken = true;
- return -EFAULT;
+ ret = -EFAULT;
+ goto fail;
}
desc = *desc_ptr;
/* Ensure descriptor has been loaded before accessing fields */
barrier(); /* read_barrier_depends(); */
+ memory_region_unref(desc_mr);
if (unlikely(++found > count)) {
error_report("Loop detected: last one at %u "
"indirect size %u", i, count);
vring->broken = true;
- return -EFAULT;
+ ret = -EFAULT;
+ goto fail;
}
if (unlikely(desc.flags & VRING_DESC_F_INDIRECT)) {
error_report("Nested indirect descriptor");
vring->broken = true;
- return -EFAULT;
+ ret = -EFAULT;
+ goto fail;
}
/* Stop for now if there are not enough iovecs available. */
if (iov >= iov_end) {
- return -ENOBUFS;
+ ret = -ENOBUFS;
+ goto fail;
}
- iov->iov_base = hostmem_lookup(desc.addr, desc.len, NULL,
+ iov->iov_base = hostmem_lookup(desc.addr, desc.len, cur,
desc.flags & VRING_DESC_F_WRITE);
if (!iov->iov_base) {
error_report("Failed to map indirect descriptor"
"addr %#" PRIx64 " len %u",
(uint64_t)desc.addr, desc.len);
vring->broken = true;
- return -EFAULT;
+ ret = -EFAULT;
+ goto fail;
}
iov->iov_len = desc.len;
iov++;
+ cur++;
/* If this is an input descriptor, increment that count. */
if (desc.flags & VRING_DESC_F_WRITE) {
@@ -191,13 +204,23 @@ static int get_indirect(Vring *vring,
error_report("Indirect descriptor "
"has out after in: idx %u", i);
vring->broken = true;
- return -EFAULT;
+ ret = -EFAULT;
+ goto fail;
}
*out_num += 1;
}
i = desc.next;
} while (desc.flags & VRING_DESC_F_NEXT);
+ *mrs = cur;
return 0;
+
+fail:
+ /* We rely on mrs[] initialized as zero */
+ for (i = 0; *mrs[i] && cur >= *mrs+i; cur--) {
+ memory_region_unref(*mrs[i]);
+ }
+
+ return ret;
}
/* This looks in the virtqueue and for the first available buffer, and converts
@@ -209,6 +232,8 @@ static int get_indirect(Vring *vring,
* never a valid descriptor number) if none was found. A negative code is
* returned on error.
*
+ * @mrs should be the same cnt as iov[]
+ *
* Stolen from linux/drivers/vhost/vhost.c.
*/
int vring_pop(VirtIODevice *vdev, Vring *vring,
@@ -218,7 +243,10 @@ int vring_pop(VirtIODevice *vdev, Vring *vring,
struct vring_desc desc;
unsigned int i, head, found = 0, num = vring->vr.num;
uint16_t avail_idx, last_avail_idx;
+ MemoryRegion **indirect, **cur, **mrs;
+ int ret = 0;
+ cur = mrs = g_malloc0((iov_end - iov)*sizeof(void *));
/* If there was a fatal error then refuse operation */
if (vring->broken) {
return -EFAULT;
@@ -267,13 +295,15 @@ int vring_pop(VirtIODevice *vdev, Vring *vring,
if (unlikely(i >= num)) {
error_report("Desc index is %u > %u, head = %u", i, num, head);
vring->broken = true;
- return -EFAULT;
+ ret = -EFAULT;
+ goto fail;
}
if (unlikely(++found > num)) {
error_report("Loop detected: last one at %u vq size %u head %u",
i, num, head);
vring->broken = true;
- return -EFAULT;
+ ret = -EFAULT;
+ goto fail;
}
desc = vring->vr.desc[i];
@@ -281,10 +311,13 @@ int vring_pop(VirtIODevice *vdev, Vring *vring,
barrier();
if (desc.flags & VRING_DESC_F_INDIRECT) {
- int ret = get_indirect(vring, iov, iov_end, out_num, in_num, &desc);
+ indirect = cur;
+ int ret = get_indirect(vring, iov, iov_end, out_num, in_num, &desc,
+ &indirect);
if (ret < 0) {
- return ret;
+ goto fail;
}
+ cur = indirect;
continue;
}
@@ -293,20 +326,23 @@ int vring_pop(VirtIODevice *vdev, Vring *vring,
* with the current set.
*/
if (iov >= iov_end) {
- return -ENOBUFS;
+ ret = -ENOBUFS;
+ goto fail;
}
/* TODO handle non-contiguous memory across region boundaries */
- iov->iov_base = hostmem_lookup(desc.addr, desc.len, NULL,
+ iov->iov_base = hostmem_lookup(desc.addr, desc.len, cur,
desc.flags & VRING_DESC_F_WRITE);
if (!iov->iov_base) {
error_report("Failed to map vring desc addr %#" PRIx64 " len %u",
(uint64_t)desc.addr, desc.len);
vring->broken = true;
- return -EFAULT;
+ ret = -EFAULT;
+ goto fail;
}
iov->iov_len = desc.len;
iov++;
+ cur++;
if (desc.flags & VRING_DESC_F_WRITE) {
/* If this is an input descriptor,
@@ -318,7 +354,8 @@ int vring_pop(VirtIODevice *vdev, Vring *vring,
if (unlikely(*in_num)) {
error_report("Descriptor has out after in: idx %d", i);
vring->broken = true;
- return -EFAULT;
+ ret = -EFAULT;
+ goto fail;
}
*out_num += 1;
}
@@ -327,7 +364,17 @@ int vring_pop(VirtIODevice *vdev, Vring *vring,
/* On success, increment avail index. */
vring->last_avail_idx++;
+ vring->mrs_vect[head].used_cnt = cur - mrs;
+ vring->mrs_vect[head].mrs = mrs;
return head;
+
+fail:
+ /* We rely on mrs[] initialized as zero */
+ for (i = 0; mrs[i]; i++) {
+ memory_region_unref(mrs[i]);
+ }
+
+ return ret;
}
/* After we've used one of their buffers, we tell them about it.
@@ -338,6 +385,8 @@ void vring_push(Vring *vring, unsigned int head, int len)
{
struct vring_used_elem *used;
uint16_t new;
+ int i;
+ MrsVect *mrs_vect;
/* Don't touch vring if a fatal error occurred */
if (vring->broken) {
@@ -357,4 +406,10 @@ void vring_push(Vring *vring, unsigned int head, int len)
if (unlikely((int16_t)(new - vring->signalled_used) < (uint16_t)1)) {
vring->signalled_used_valid = false;
}
+ mrs_vect = &vring->mrs_vect[head];
+ for (i = 0; i < mrs_vect->used_cnt; i++) {
+ memory_region_unref(mrs_vect->mrs[i]);
+ }
+ mrs_vect->used_cnt = 0;
+ g_free(mrs_vect->mrs);
}
diff --git a/include/hw/virtio/dataplane/vring.h b/include/hw/virtio/dataplane/vring.h
index 56acffb..88dac68 100644
--- a/include/hw/virtio/dataplane/vring.h
+++ b/include/hw/virtio/dataplane/vring.h
@@ -22,13 +22,28 @@
#include "hostmem.h"
#include "hw/virtio/virtio.h"
+enum {
+ SEG_MAX = 126, /* maximum number of I/O segments */
+ VRING_MAX = SEG_MAX + 2, /* maximum number of vring descriptors */
+ REQ_MAX = VRING_MAX, /* maximum number of requests in the vring,
+ * is VRING_MAX / 2 with traditional and
+ * VRING_MAX with indirect descriptors */
+};
+
+typedef struct {
+ int used_cnt;
+ MemoryRegion **mrs;
+} MrsVect;
+
typedef struct {
+ MemoryRegion *vring_mr; /* RAM's memoryRegion on which this vring sits */
struct vring vr; /* virtqueue vring mapped to host memory */
uint16_t last_avail_idx; /* last processed avail ring index */
uint16_t last_used_idx; /* last processed used ring index */
uint16_t signalled_used; /* EVENT_IDX state */
bool signalled_used_valid;
bool broken; /* was there a fatal error? */
+ MrsVect mrs_vect[VRING_MAX];
} Vring;
static inline unsigned int vring_get_num(Vring *vring)
--
1.7.4.4
^ permalink raw reply related [flat|nested] 6+ messages in thread