* [Qemu-devel] [RFC PATCH 1/8] memory: add ref/unref calls
2013-05-06 14:25 [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, replace hostmem with memory_region_find Paolo Bonzini
@ 2013-05-06 14:25 ` Paolo Bonzini
2013-05-06 14:25 ` [Qemu-devel] [RFC PATCH 2/8] exec: check MRU in qemu_ram_addr_from_host Paolo Bonzini
` (9 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2013-05-06 14:25 UTC (permalink / raw)
To: qemu-devel; +Cc: jan.kiszka, qemulist, stefanha
Add ref/unref calls at the following places:
- places where memory regions are stashed by a listener and
used outside the BQL (including in Xen or KVM).
- memory_region_find callsites
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 6 +++++-
hw/core/loader.c | 1 +
hw/display/exynos4210_fimd.c | 6 ++++++
hw/display/framebuffer.c | 10 ++++++----
hw/i386/kvm/ioapic.c | 2 ++
hw/i386/kvmvapic.c | 1 +
hw/misc/vfio.c | 2 ++
hw/virtio/dataplane/hostmem.c | 7 +++++++
hw/virtio/vhost.c | 2 ++
hw/virtio/virtio-balloon.c | 1 +
hw/xen/xen_pt.c | 4 ++++
include/exec/memory.h | 9 +++++++++
include/hw/virtio/dataplane/hostmem.h | 1 +
kvm-all.c | 2 ++
memory.c | 16 ++++++++++++++++
target-arm/kvm.c | 2 ++
target-sparc/mmu_helper.c | 1 +
xen-all.c | 2 ++
18 files changed, 70 insertions(+), 5 deletions(-)
diff --git a/exec.c b/exec.c
index 91cd28f..9f324bb 100644
--- a/exec.c
+++ b/exec.c
@@ -753,12 +753,16 @@ static uint16_t phys_section_add(MemoryRegionSection *section)
phys_sections_nb_alloc);
}
phys_sections[phys_sections_nb] = *section;
+ memory_region_ref(section->mr);
return phys_sections_nb++;
}
static void phys_sections_clear(void)
{
- phys_sections_nb = 0;
+ while (phys_sections_nb > 0) {
+ MemoryRegionSection *section = &phys_sections[--phys_sections_nb];
+ memory_region_unref(section->mr);
+ }
}
static void register_subpage(AddressSpaceDispatch *d, MemoryRegionSection *section)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 7507914..97e7ba2 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -727,6 +727,7 @@ int rom_load_all(void)
addr += rom->romsize;
section = memory_region_find(get_system_memory(), rom->addr, 1);
rom->isrom = section.size && memory_region_is_rom(section.mr);
+ memory_region_unref(section.mr);
}
qemu_register_reset(rom_reset, NULL);
roms_loaded = 1;
diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
index 6cb5016..afa2e54 100644
--- a/hw/display/exynos4210_fimd.c
+++ b/hw/display/exynos4210_fimd.c
@@ -1126,6 +1126,11 @@ static void fimd_update_memory_section(Exynos4210fimdState *s, unsigned win)
/* Total number of bytes of virtual screen used by current window */
w->fb_len = fb_mapped_len = (w->virtpage_width + w->virtpage_offsize) *
(w->rightbot_y - w->lefttop_y + 1);
+
+ /* TODO: add .exit and unref the region there. Not needed yet since sysbus
+ * does not support hot-unplug.
+ */
+ memory_region_unref(w->mem_section.mr);
w->mem_section = memory_region_find(sysbus_address_space(&s->busdev),
fb_start_addr, w->fb_len);
assert(w->mem_section.mr);
@@ -1154,6 +1159,7 @@ static void fimd_update_memory_section(Exynos4210fimdState *s, unsigned win)
return;
error_return:
+ memory_region_unref(w->mem_section.mr);
w->mem_section.mr = NULL;
w->mem_section.size = 0;
w->host_fb_addr = NULL;
diff --git a/hw/display/framebuffer.c b/hw/display/framebuffer.c
index 6be31db..8288b93 100644
--- a/hw/display/framebuffer.c
+++ b/hw/display/framebuffer.c
@@ -54,10 +54,10 @@ void framebuffer_update_display(
src_len = src_width * rows;
mem_section = memory_region_find(address_space, base, src_len);
+ mem = mem_section.mr;
if (mem_section.size != src_len || !memory_region_is_ram(mem_section.mr)) {
- return;
+ goto out;
}
- mem = mem_section.mr;
assert(mem);
assert(mem_section.offset_within_address_space == base);
@@ -67,10 +67,10 @@ void framebuffer_update_display(
but it's not really worth it as dirty flag tracking will probably
already have failed above. */
if (!src_base)
- return;
+ goto out;
if (src_len != src_width * rows) {
cpu_physical_memory_unmap(src_base, src_len, 0, 0);
- return;
+ goto out;
}
src = src_base;
dest = surface_data(ds);
@@ -107,4 +107,6 @@ void framebuffer_update_display(
DIRTY_MEMORY_VGA);
*first_row = first;
*last_row = last;
+out:
+ memory_region_unref(mem);
}
diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
index 3ad951e..9192b39 100644
--- a/hw/i386/kvm/ioapic.c
+++ b/hw/i386/kvm/ioapic.c
@@ -114,6 +114,8 @@ static void kvm_ioapic_put(IOAPICCommonState *s)
fprintf(stderr, "KVM_GET_IRQCHIP failed: %s\n", strerror(ret));
abort();
}
+
+ memory_region_unref(mrs.mr);
}
static void kvm_ioapic_reset(DeviceState *dev)
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 5b558aa..6a98c7f 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -605,6 +605,7 @@ static void vapic_map_rom_writable(VAPICROMState *s)
rom_size);
memory_region_add_subregion_overlap(as, rom_paddr, &s->rom, 1000);
s->rom_mapped_writable = true;
+ memory_region_unref(section.mr);
}
static int vapic_prepare(VAPICROMState *s)
diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 178dd11..0ae6878 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -1969,6 +1969,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
DPRINTF("region_add %"HWADDR_PRIx" - %"HWADDR_PRIx" [%p]\n",
iova, end - 1, vaddr);
+ memory_region_ref(section->mr);
ret = vfio_dma_map(container, iova, end - iova, vaddr, section->readonly);
if (ret) {
error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
@@ -2010,6 +2011,7 @@ static void vfio_listener_region_del(MemoryListener *listener,
iova, end - 1);
ret = vfio_dma_unmap(container, iova, end - iova);
+ memory_region_unref(section->mr);
if (ret) {
error_report("vfio_dma_unmap(%p, 0x%"HWADDR_PRIx", "
"0x%"HWADDR_PRIx") = %d (%m)",
diff --git a/hw/virtio/dataplane/hostmem.c b/hw/virtio/dataplane/hostmem.c
index 37292ff..66829bb 100644
--- a/hw/virtio/dataplane/hostmem.c
+++ b/hw/virtio/dataplane/hostmem.c
@@ -64,8 +64,12 @@ out:
static void hostmem_listener_commit(MemoryListener *listener)
{
HostMem *hostmem = container_of(listener, HostMem, listener);
+ int i;
qemu_mutex_lock(&hostmem->current_regions_lock);
+ for (i = 0; i < hostmem->num_current_regions; i++) {
+ memory_region_unref(hostmem->current_regions[i].mr);
+ }
g_free(hostmem->current_regions);
hostmem->current_regions = hostmem->new_regions;
hostmem->num_current_regions = hostmem->num_new_regions;
@@ -92,8 +96,11 @@ static void hostmem_append_new_region(HostMem *hostmem,
.guest_addr = section->offset_within_address_space,
.size = section->size,
.readonly = section->readonly,
+ .mr = section->mr,
};
hostmem->num_new_regions++;
+
+ memory_region_ref(section->mr);
}
static void hostmem_listener_append_region(MemoryListener *listener,
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index fbabf99..190522b 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -497,6 +497,7 @@ static void vhost_region_add(MemoryListener *listener,
dev->mem_sections = g_renew(MemoryRegionSection, dev->mem_sections,
dev->n_mem_sections);
dev->mem_sections[dev->n_mem_sections - 1] = *section;
+ memory_region_ref(section->mr);
vhost_set_memory(listener, section, true);
}
@@ -512,6 +513,7 @@ static void vhost_region_del(MemoryListener *listener,
}
vhost_set_memory(listener, section, false);
+ memory_region_unref(section->mr);
for (i = 0; i < dev->n_mem_sections; ++i) {
if (dev->mem_sections[i].offset_within_address_space
== section->offset_within_address_space) {
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index d669756..fac8800 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -205,6 +205,7 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
addr = section.offset_within_region;
balloon_page(memory_region_get_ram_ptr(section.mr) + addr,
!!(vq == s->dvq));
+ memory_region_unref(section.mr);
}
virtqueue_push(vq, &elem, offset);
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index c199818..be1fd52 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -606,6 +606,7 @@ static void xen_pt_region_add(MemoryListener *l, MemoryRegionSection *sec)
XenPCIPassthroughState *s = container_of(l, XenPCIPassthroughState,
memory_listener);
+ memory_region_ref(sec->mr);
xen_pt_region_update(s, sec, true);
}
@@ -615,6 +616,7 @@ static void xen_pt_region_del(MemoryListener *l, MemoryRegionSection *sec)
memory_listener);
xen_pt_region_update(s, sec, false);
+ memory_region_unref(sec->mr);
}
static void xen_pt_io_region_add(MemoryListener *l, MemoryRegionSection *sec)
@@ -622,6 +624,7 @@ static void xen_pt_io_region_add(MemoryListener *l, MemoryRegionSection *sec)
XenPCIPassthroughState *s = container_of(l, XenPCIPassthroughState,
io_listener);
+ memory_region_ref(sec->mr);
xen_pt_region_update(s, sec, true);
}
@@ -631,6 +634,7 @@ static void xen_pt_io_region_del(MemoryListener *l, MemoryRegionSection *sec)
io_listener);
xen_pt_region_update(s, sec, false);
+ memory_region_unref(sec->mr);
}
static const MemoryListener xen_pt_memory_listener = {
diff --git a/include/exec/memory.h b/include/exec/memory.h
index d2bbfb7..fb1b42f 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -252,6 +252,15 @@ struct MemoryListener {
void memory_region_init(MemoryRegion *mr,
const char *name,
uint64_t size);
+
+static inline void memory_region_ref(MemoryRegion *mr)
+{
+}
+
+static inline void memory_region_unref(MemoryRegion *mr)
+{
+}
+
/**
* memory_region_init_io: Initialize an I/O memory region.
*
diff --git a/include/hw/virtio/dataplane/hostmem.h b/include/hw/virtio/dataplane/hostmem.h
index b2cf093..2810f4b 100644
--- a/include/hw/virtio/dataplane/hostmem.h
+++ b/include/hw/virtio/dataplane/hostmem.h
@@ -18,6 +18,7 @@
#include "qemu/thread.h"
typedef struct {
+ MemoryRegion *mr;
void *host_addr;
hwaddr guest_addr;
uint64_t size;
diff --git a/kvm-all.c b/kvm-all.c
index 3a31602..1d876aa 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -766,6 +766,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
static void kvm_region_add(MemoryListener *listener,
MemoryRegionSection *section)
{
+ memory_region_ref(section->mr);
kvm_set_phys_mem(section, true);
}
@@ -773,6 +774,7 @@ static void kvm_region_del(MemoryListener *listener,
MemoryRegionSection *section)
{
kvm_set_phys_mem(section, false);
+ memory_region_unref(section->mr);
}
static void kvm_log_sync(MemoryListener *listener,
diff --git a/memory.c b/memory.c
index aa0eeb7..bc4cf4b 100644
--- a/memory.c
+++ b/memory.c
@@ -145,6 +145,7 @@ static bool memory_listener_match(MemoryListener *listener,
} \
} while (0)
+/* No need to ref/unref .mr, the FlatRange keeps it alive. */
#define MEMORY_LISTENER_UPDATE_REGION(fr, as, dir, callback) \
MEMORY_LISTENER_CALL(callback, dir, (&(MemoryRegionSection) { \
.mr = (fr)->mr, \
@@ -260,11 +261,17 @@ static void flatview_insert(FlatView *view, unsigned pos, FlatRange *range)
memmove(view->ranges + pos + 1, view->ranges + pos,
(view->nr - pos) * sizeof(FlatRange));
view->ranges[pos] = *range;
+ memory_region_ref(range->mr);
++view->nr;
}
static void flatview_destroy(FlatView *view)
{
+ int i;
+
+ for (i = 0; i < view->nr; i++) {
+ memory_region_unref(view->ranges[i].mr);
+ }
g_free(view->ranges);
}
@@ -763,6 +770,11 @@ static void memory_region_destructor_ram(MemoryRegion *mr)
qemu_ram_free(mr->ram_addr);
}
+static void memory_region_destructor_alias(MemoryRegion *mr)
+{
+ memory_region_unref(mr->alias);
+}
+
static void memory_region_destructor_ram_from_ptr(MemoryRegion *mr)
{
qemu_ram_free_from_ptr(mr->ram_addr);
@@ -960,6 +972,8 @@ void memory_region_init_alias(MemoryRegion *mr,
uint64_t size)
{
memory_region_init(mr, name, size);
+ memory_region_ref(orig);
+ mr->destructor = memory_region_destructor_alias;
mr->alias = orig;
mr->alias_offset = offset;
}
@@ -1504,6 +1518,8 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr,
ret.size = int128_get64(range.size);
ret.offset_within_address_space = int128_get64(range.start);
ret.readonly = fr->readonly;
+ memory_region_ref(ret.mr);
+
return ret;
}
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index b7bdc03..b9051a4 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -127,6 +127,7 @@ static void kvm_arm_machine_init_done(Notifier *notifier, void *data)
abort();
}
}
+ memory_region_unref(kd->mr);
g_free(kd);
}
}
@@ -152,6 +153,7 @@ void kvm_arm_register_device(MemoryRegion *mr, uint64_t devid)
kd->kda.id = devid;
kd->kda.addr = -1;
QSLIST_INSERT_HEAD(&kvm_devices_head, kd, entries);
+ memory_region_ref(kd->mr);
}
typedef struct Reg {
diff --git a/target-sparc/mmu_helper.c b/target-sparc/mmu_helper.c
index a9649ae..3c1ccc2 100644
--- a/target-sparc/mmu_helper.c
+++ b/target-sparc/mmu_helper.c
@@ -845,6 +845,7 @@ hwaddr cpu_get_phys_page_debug(CPUSPARCState *env, target_ulong addr)
}
}
section = memory_region_find(get_system_memory(), phys_addr, 1);
+ memory_region_unref(section.mr);
if (!section.size) {
return -1;
}
diff --git a/xen-all.c b/xen-all.c
index 539a154..2f24ced 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -459,6 +459,7 @@ static void xen_set_memory(struct MemoryListener *listener,
static void xen_region_add(MemoryListener *listener,
MemoryRegionSection *section)
{
+ memory_region_ref(section->mr);
xen_set_memory(listener, section, true);
}
@@ -466,6 +467,7 @@ static void xen_region_del(MemoryListener *listener,
MemoryRegionSection *section)
{
xen_set_memory(listener, section, false);
+ memory_region_unref(section->mr);
}
static void xen_sync_dirty_bitmap(XenIOState *state,
--
1.7.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [RFC PATCH 2/8] exec: check MRU in qemu_ram_addr_from_host
2013-05-06 14:25 [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, replace hostmem with memory_region_find Paolo Bonzini
2013-05-06 14:25 ` [Qemu-devel] [RFC PATCH 1/8] memory: add ref/unref calls Paolo Bonzini
@ 2013-05-06 14:25 ` Paolo Bonzini
2013-05-06 14:25 ` [Qemu-devel] [RFC PATCH 3/8] memory: return MemoryRegion from qemu_ram_addr_from_host Paolo Bonzini
` (8 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2013-05-06 14:25 UTC (permalink / raw)
To: qemu-devel; +Cc: jan.kiszka, qemulist, stefanha
This function is not used outside the iothread mutex, so it
can use ram_list.mru_block.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/exec.c b/exec.c
index 9f324bb..8e46228 100644
--- a/exec.c
+++ b/exec.c
@@ -1404,18 +1404,26 @@ int qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr)
return 0;
}
+ block = ram_list.mru_block;
+ if (block && block->host && host - block->host < block->length) {
+ goto found;
+ }
+
QTAILQ_FOREACH(block, &ram_list.blocks, next) {
/* This case append when the block is not mapped. */
if (block->host == NULL) {
continue;
}
if (host - block->host < block->length) {
- *ram_addr = block->offset + (host - block->host);
- return 0;
+ goto found;
}
}
return -1;
+
+found:
+ *ram_addr = block->offset + (host - block->host);
+ return 0;
}
/* Some of the softmmu routines need to translate from a host pointer
--
1.7.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [RFC PATCH 3/8] memory: return MemoryRegion from qemu_ram_addr_from_host
2013-05-06 14:25 [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, replace hostmem with memory_region_find Paolo Bonzini
2013-05-06 14:25 ` [Qemu-devel] [RFC PATCH 1/8] memory: add ref/unref calls Paolo Bonzini
2013-05-06 14:25 ` [Qemu-devel] [RFC PATCH 2/8] exec: check MRU in qemu_ram_addr_from_host Paolo Bonzini
@ 2013-05-06 14:25 ` Paolo Bonzini
2013-05-06 14:25 ` [Qemu-devel] [RFC PATCH 4/8] memory: ref/unref memory across address_space_map/unmap Paolo Bonzini
` (7 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2013-05-06 14:25 UTC (permalink / raw)
To: qemu-devel; +Cc: jan.kiszka, qemulist, stefanha
It will be needed in the next patch.
The common parts of qemu_get_ram_ptr and qemu_ram_addr_from_host are
moved to a new function qemu_get_ram_block.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 35 +++++++++++++++++++++--------------
include/exec/cpu-common.h | 2 +-
target-i386/kvm.c | 4 ++--
3 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/exec.c b/exec.c
index 8e46228..54b57fc 100644
--- a/exec.c
+++ b/exec.c
@@ -1287,15 +1287,7 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
}
#endif /* !_WIN32 */
-/* Return a host pointer to ram allocated with qemu_ram_alloc.
- With the exception of the softmmu code in this file, this should
- only be used for local memory (e.g. video ram) that the device owns,
- and knows it isn't going to access beyond the end of the block.
-
- It should not be used for general purpose DMA.
- Use cpu_physical_memory_map/cpu_physical_memory_rw instead.
- */
-void *qemu_get_ram_ptr(ram_addr_t addr)
+static RAMBlock *qemu_get_ram_block(ram_addr_t addr)
{
RAMBlock *block;
@@ -1315,6 +1307,21 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
found:
ram_list.mru_block = block;
+ return block;
+}
+
+/* Return a host pointer to ram allocated with qemu_ram_alloc.
+ With the exception of the softmmu code in this file, this should
+ only be used for local memory (e.g. video ram) that the device owns,
+ and knows it isn't going to access beyond the end of the block.
+
+ It should not be used for general purpose DMA.
+ Use cpu_physical_memory_map/cpu_physical_memory_rw instead.
+ */
+void *qemu_get_ram_ptr(ram_addr_t addr)
+{
+ RAMBlock *block = qemu_get_ram_block(addr);
+
if (xen_enabled()) {
/* We need to check if the requested address is in the RAM
* because we don't want to map the entire memory in QEMU.
@@ -1394,14 +1401,14 @@ void qemu_put_ram_ptr(void *addr)
trace_qemu_put_ram_ptr(addr);
}
-int qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr)
+MemoryRegion *qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr)
{
RAMBlock *block;
uint8_t *host = ptr;
if (xen_enabled()) {
*ram_addr = xen_ram_addr_from_mapcache(ptr);
- return 0;
+ return qemu_get_ram_block(*ram_addr)->mr;
}
block = ram_list.mru_block;
@@ -1419,11 +1426,11 @@ int qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr)
}
}
- return -1;
+ return NULL;
found:
*ram_addr = block->offset + (host - block->host);
- return 0;
+ return block->mr;
}
/* Some of the softmmu routines need to translate from a host pointer
@@ -1432,7 +1439,7 @@ ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr)
{
ram_addr_t ram_addr;
- if (qemu_ram_addr_from_host(ptr, &ram_addr)) {
+ if (qemu_ram_addr_from_host(ptr, &ram_addr) == NULL) {
fprintf(stderr, "Bad ram pointer %p\n", ptr);
abort();
}
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 2e5f11f..84dfd3b 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -53,7 +53,7 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length);
void *qemu_get_ram_ptr(ram_addr_t addr);
void qemu_put_ram_ptr(void *addr);
/* This should not be used by devices. */
-int qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr);
+MemoryRegion *qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr);
ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr);
void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 9ffb6ca..7ba98cd 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -318,7 +318,7 @@ int kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
if ((env->mcg_cap & MCG_SER_P) && addr
&& (code == BUS_MCEERR_AR || code == BUS_MCEERR_AO)) {
- if (qemu_ram_addr_from_host(addr, &ram_addr) ||
+ if (qemu_ram_addr_from_host(addr, &ram_addr) == NULL ||
!kvm_physical_memory_addr_from_host(c->kvm_state, addr, &paddr)) {
fprintf(stderr, "Hardware memory error for memory used by "
"QEMU itself instead of guest system!\n");
@@ -350,7 +350,7 @@ int kvm_arch_on_sigbus(int code, void *addr)
hwaddr paddr;
/* Hope we are lucky for AO MCE */
- if (qemu_ram_addr_from_host(addr, &ram_addr) ||
+ if (qemu_ram_addr_from_host(addr, &ram_addr) == NULL ||
!kvm_physical_memory_addr_from_host(CPU(first_cpu)->kvm_state,
addr, &paddr)) {
fprintf(stderr, "Hardware memory error for memory used by "
--
1.7.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [RFC PATCH 4/8] memory: ref/unref memory across address_space_map/unmap
2013-05-06 14:25 [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, replace hostmem with memory_region_find Paolo Bonzini
` (2 preceding siblings ...)
2013-05-06 14:25 ` [Qemu-devel] [RFC PATCH 3/8] memory: return MemoryRegion from qemu_ram_addr_from_host Paolo Bonzini
@ 2013-05-06 14:25 ` Paolo Bonzini
2013-05-06 14:25 ` [Qemu-devel] [RFC PATCH 5/8] memory: access FlatView from a local variable Paolo Bonzini
` (6 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2013-05-06 14:25 UTC (permalink / raw)
To: qemu-devel; +Cc: jan.kiszka, qemulist, stefanha
The iothread mutex might be released between map and unmap, so the
mapped region might disappear.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/exec.c b/exec.c
index 54b57fc..54ed203 100644
--- a/exec.c
+++ b/exec.c
@@ -2077,6 +2077,7 @@ void cpu_physical_memory_write_rom(hwaddr addr,
}
typedef struct {
+ MemoryRegion *mr;
void *buffer;
hwaddr addr;
hwaddr len;
@@ -2171,15 +2172,18 @@ void *address_space_map(AddressSpace *as,
bounce.buffer = qemu_memalign(TARGET_PAGE_SIZE, TARGET_PAGE_SIZE);
bounce.addr = addr;
bounce.len = l;
+ bounce.mr = section->mr;
if (!is_write) {
address_space_read(as, addr, bounce.buffer, l);
}
*plen = l;
+ memory_region_ref(section->mr);
return bounce.buffer;
}
if (!todo) {
raddr = memory_region_get_ram_addr(section->mr) + xlat;
+ memory_region_ref(section->mr);
} else {
if (memory_region_get_ram_addr(section->mr) + xlat != raddr + todo) {
break;
@@ -2204,8 +2208,12 @@ void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
int is_write, hwaddr access_len)
{
if (buffer != bounce.buffer) {
+ MemoryRegion *mr;
+ ram_addr_t addr1;
+
+ mr = qemu_ram_addr_from_host(buffer, &addr1);
+ assert(mr);
if (is_write) {
- ram_addr_t addr1 = qemu_ram_addr_from_host_nofail(buffer);
while (access_len) {
unsigned l;
l = TARGET_PAGE_SIZE;
@@ -2219,6 +2227,7 @@ void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
if (xen_enabled()) {
xen_invalidate_map_cache_entry(buffer);
}
+ memory_region_unref(mr);
return;
}
if (is_write) {
@@ -2226,6 +2235,7 @@ void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
}
qemu_vfree(bounce.buffer);
bounce.buffer = NULL;
+ memory_region_unref(bounce.mr);
cpu_notify_map_clients();
}
--
1.7.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [RFC PATCH 5/8] memory: access FlatView from a local variable
2013-05-06 14:25 [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, replace hostmem with memory_region_find Paolo Bonzini
` (3 preceding siblings ...)
2013-05-06 14:25 ` [Qemu-devel] [RFC PATCH 4/8] memory: ref/unref memory across address_space_map/unmap Paolo Bonzini
@ 2013-05-06 14:25 ` Paolo Bonzini
2013-05-06 14:25 ` [Qemu-devel] [RFC PATCH 6/8] memory: use a new FlatView pointer on every topology update Paolo Bonzini
` (5 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2013-05-06 14:25 UTC (permalink / raw)
To: qemu-devel; +Cc: jan.kiszka, qemulist, stefanha
We will soon require accesses to as->current_map to be placed under
a lock (with reference counting so as to keep the critical section
small). To simplify this change, always fetch as->current_map into
a local variable and access it through that variable.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
memory.c | 31 +++++++++++++++++++++----------
1 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/memory.c b/memory.c
index bc4cf4b..553b04c 100644
--- a/memory.c
+++ b/memory.c
@@ -632,13 +632,15 @@ static void address_space_add_del_ioeventfds(AddressSpace *as,
static void address_space_update_ioeventfds(AddressSpace *as)
{
+ FlatView *view;
FlatRange *fr;
unsigned ioeventfd_nb = 0;
MemoryRegionIoeventfd *ioeventfds = NULL;
AddrRange tmp;
unsigned i;
- FOR_EACH_FLAT_RANGE(fr, as->current_map) {
+ view = as->current_map;
+ FOR_EACH_FLAT_RANGE(fr, view) {
for (i = 0; i < fr->mr->ioeventfd_nb; ++i) {
tmp = addrrange_shift(fr->mr->ioeventfds[i].addr,
int128_sub(fr->addr.start,
@@ -1134,7 +1136,8 @@ void memory_region_sync_dirty_bitmap(MemoryRegion *mr)
FlatRange *fr;
QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
- FOR_EACH_FLAT_RANGE(fr, as->current_map) {
+ FlatView *view = as->current_map;
+ FOR_EACH_FLAT_RANGE(fr, view) {
if (fr->mr == mr) {
MEMORY_LISTENER_UPDATE_REGION(fr, as, Forward, log_sync);
}
@@ -1184,12 +1187,14 @@ void *memory_region_get_ram_ptr(MemoryRegion *mr)
static void memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpace *as)
{
+ FlatView *view;
FlatRange *fr;
CoalescedMemoryRange *cmr;
AddrRange tmp;
MemoryRegionSection section;
- FOR_EACH_FLAT_RANGE(fr, as->current_map) {
+ view = as->current_map;
+ FOR_EACH_FLAT_RANGE(fr, view) {
if (fr->mr == mr) {
section = (MemoryRegionSection) {
.address_space = as,
@@ -1476,9 +1481,9 @@ static int cmp_flatrange_addr(const void *addr_, const void *fr_)
return 0;
}
-static FlatRange *address_space_lookup(AddressSpace *as, AddrRange addr)
+static FlatRange *flatview_lookup(FlatView *view, AddrRange addr)
{
- return bsearch(&addr, as->current_map->ranges, as->current_map->nr,
+ return bsearch(&addr, view->ranges, view->nr,
sizeof(FlatRange), cmp_flatrange_addr);
}
@@ -1489,6 +1494,7 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr,
MemoryRegion *root;
AddressSpace *as;
AddrRange range;
+ FlatView *view;
FlatRange *fr;
addr += mr->addr;
@@ -1499,13 +1505,14 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr,
as = memory_region_to_address_space(root);
range = addrrange_make(int128_make64(addr), int128_make64(size));
- fr = address_space_lookup(as, range);
+
+ view = as->current_map;
+ fr = flatview_lookup(view, range);
if (!fr) {
return ret;
}
- while (fr > as->current_map->ranges
- && addrrange_intersects(fr[-1].addr, range)) {
+ while (fr > view->ranges && addrrange_intersects(fr[-1].addr, range)) {
--fr;
}
@@ -1525,9 +1532,11 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr,
void address_space_sync_dirty_bitmap(AddressSpace *as)
{
+ FlatView *view;
FlatRange *fr;
- FOR_EACH_FLAT_RANGE(fr, as->current_map) {
+ view = as->current_map;
+ FOR_EACH_FLAT_RANGE(fr, view) {
MEMORY_LISTENER_UPDATE_REGION(fr, as, Forward, log_sync);
}
}
@@ -1547,6 +1556,7 @@ void memory_global_dirty_log_stop(void)
static void listener_add_address_space(MemoryListener *listener,
AddressSpace *as)
{
+ FlatView *view;
FlatRange *fr;
if (listener->address_space_filter
@@ -1560,7 +1570,8 @@ static void listener_add_address_space(MemoryListener *listener,
}
}
- FOR_EACH_FLAT_RANGE(fr, as->current_map) {
+ view = as->current_map;
+ FOR_EACH_FLAT_RANGE(fr, view) {
MemoryRegionSection section = {
.mr = fr->mr,
.address_space = as,
--
1.7.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [RFC PATCH 6/8] memory: use a new FlatView pointer on every topology update
2013-05-06 14:25 [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, replace hostmem with memory_region_find Paolo Bonzini
` (4 preceding siblings ...)
2013-05-06 14:25 ` [Qemu-devel] [RFC PATCH 5/8] memory: access FlatView from a local variable Paolo Bonzini
@ 2013-05-06 14:25 ` Paolo Bonzini
2013-05-06 14:25 ` [Qemu-devel] [RFC PATCH 7/8] memory: add reference counting to FlatView Paolo Bonzini
` (4 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2013-05-06 14:25 UTC (permalink / raw)
To: qemu-devel; +Cc: jan.kiszka, qemulist, stefanha
This is the first step towards converting as->current_map to
RCU-style updates, where the FlatView updates run concurrently
with uses of an old FlatView.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
memory.c | 34 ++++++++++++++++++----------------
1 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/memory.c b/memory.c
index 553b04c..fbb2657 100644
--- a/memory.c
+++ b/memory.c
@@ -273,6 +273,7 @@ static void flatview_destroy(FlatView *view)
memory_region_unref(view->ranges[i].mr);
}
g_free(view->ranges);
+ g_free(view);
}
static bool can_merge(FlatRange *r1, FlatRange *r2)
@@ -566,17 +567,18 @@ static void render_memory_region(FlatView *view,
}
/* Render a memory topology into a list of disjoint absolute ranges. */
-static FlatView generate_memory_topology(MemoryRegion *mr)
+static FlatView *generate_memory_topology(MemoryRegion *mr)
{
- FlatView view;
+ FlatView *view;
- flatview_init(&view);
+ view = g_new(FlatView, 1);
+ flatview_init(view);
if (mr) {
- render_memory_region(&view, mr, int128_zero(),
+ render_memory_region(view, mr, int128_zero(),
addrrange_make(int128_zero(), int128_2_64()), false);
}
- flatview_simplify(&view);
+ flatview_simplify(view);
return view;
}
@@ -664,8 +666,8 @@ static void address_space_update_ioeventfds(AddressSpace *as)
}
static void address_space_update_topology_pass(AddressSpace *as,
- FlatView old_view,
- FlatView new_view,
+ const FlatView *old_view,
+ const FlatView *new_view,
bool adding)
{
unsigned iold, inew;
@@ -675,14 +677,14 @@ static void address_space_update_topology_pass(AddressSpace *as,
* Kill ranges in the old map, and instantiate ranges in the new map.
*/
iold = inew = 0;
- while (iold < old_view.nr || inew < new_view.nr) {
- if (iold < old_view.nr) {
- frold = &old_view.ranges[iold];
+ while (iold < old_view->nr || inew < new_view->nr) {
+ if (iold < old_view->nr) {
+ frold = &old_view->ranges[iold];
} else {
frold = NULL;
}
- if (inew < new_view.nr) {
- frnew = &new_view.ranges[inew];
+ if (inew < new_view->nr) {
+ frnew = &new_view->ranges[inew];
} else {
frnew = NULL;
}
@@ -728,14 +730,14 @@ static void address_space_update_topology_pass(AddressSpace *as,
static void address_space_update_topology(AddressSpace *as)
{
- FlatView old_view = *as->current_map;
- FlatView new_view = generate_memory_topology(as->root);
+ FlatView *old_view = as->current_map;
+ FlatView *new_view = generate_memory_topology(as->root);
address_space_update_topology_pass(as, old_view, new_view, false);
address_space_update_topology_pass(as, old_view, new_view, true);
- *as->current_map = new_view;
- flatview_destroy(&old_view);
+ as->current_map = new_view;
+ flatview_destroy(old_view);
address_space_update_ioeventfds(as);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [RFC PATCH 7/8] memory: add reference counting to FlatView
2013-05-06 14:25 [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, replace hostmem with memory_region_find Paolo Bonzini
` (5 preceding siblings ...)
2013-05-06 14:25 ` [Qemu-devel] [RFC PATCH 6/8] memory: use a new FlatView pointer on every topology update Paolo Bonzini
@ 2013-05-06 14:25 ` Paolo Bonzini
2013-05-06 14:25 ` [Qemu-devel] [RFC PATCH 8/8] dataplane: replace hostmem with memory_region_find Paolo Bonzini
` (3 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2013-05-06 14:25 UTC (permalink / raw)
To: qemu-devel; +Cc: jan.kiszka, qemulist, stefanha
With this change, a FlatView can be used even after a concurrent
update has replaced it. Because we do not have RCU, we use a
mutex to protect the small critical sections that read/write the
as->current_map pointer. Accesses to the FlatView can be done
outside the mutex.
If a MemoryRegion will be used after the FlatView is unref-ed (or after
a MemoryListener callback is returned), a reference has to be added to
that MemoryRegion. For example, memory_region_find adds a reference to
the MemoryRegion that it returns.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
memory.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 67 insertions(+), 8 deletions(-)
diff --git a/memory.c b/memory.c
index fbb2657..f02a3cc 100644
--- a/memory.c
+++ b/memory.c
@@ -26,12 +26,26 @@ static unsigned memory_region_transaction_depth;
static bool memory_region_update_pending;
static bool global_dirty_log = false;
+/* Either the flat_view_mutex or the iothread mutex can be taken around reads
+ * of as->current_map; the critical section is extremely short, so I'm using a
+ * single mutex for every AS. We could also RCU for the read-side.
+ *
+ * Both locks are taken while writing to as->current_map (with the iothread
+ * mutex outside).
+ */
+static QemuMutex flat_view_mutex;
+
static QTAILQ_HEAD(memory_listeners, MemoryListener) memory_listeners
= QTAILQ_HEAD_INITIALIZER(memory_listeners);
static QTAILQ_HEAD(, AddressSpace) address_spaces
= QTAILQ_HEAD_INITIALIZER(address_spaces);
+static void memory_init(void)
+{
+ qemu_mutex_init(&flat_view_mutex);
+}
+
typedef struct AddrRange AddrRange;
/*
@@ -222,6 +236,7 @@ struct FlatRange {
* order.
*/
struct FlatView {
+ unsigned ref;
FlatRange *ranges;
unsigned nr;
unsigned nr_allocated;
@@ -243,6 +258,7 @@ static bool flatrange_equal(FlatRange *a, FlatRange *b)
static void flatview_init(FlatView *view)
{
+ view->ref = 1;
view->ranges = NULL;
view->nr = 0;
view->nr_allocated = 0;
@@ -276,6 +292,18 @@ static void flatview_destroy(FlatView *view)
g_free(view);
}
+static void flatview_ref(FlatView *view)
+{
+ __sync_fetch_and_add(&view->ref, 1);
+}
+
+static void flatview_unref(FlatView *view)
+{
+ if (__sync_fetch_and_sub(&view->ref, 1) == 1) {
+ flatview_destroy(view);
+ }
+}
+
static bool can_merge(FlatRange *r1, FlatRange *r2)
{
return int128_eq(addrrange_end(r1->addr), r2->addr.start)
@@ -728,16 +756,38 @@ static void address_space_update_topology_pass(AddressSpace *as,
}
+static FlatView *address_space_get_flatview(AddressSpace *as)
+{
+ FlatView *view;
+
+ qemu_mutex_lock(&flat_view_mutex);
+ view = as->current_map;
+ flatview_ref(view);
+ qemu_mutex_unlock(&flat_view_mutex);
+ return view;
+}
+
static void address_space_update_topology(AddressSpace *as)
{
- FlatView *old_view = as->current_map;
+ FlatView *old_view = address_space_get_flatview(as);
FlatView *new_view = generate_memory_topology(as->root);
address_space_update_topology_pass(as, old_view, new_view, false);
address_space_update_topology_pass(as, old_view, new_view, true);
+ qemu_mutex_lock(&flat_view_mutex);
+ flatview_unref(as->current_map);
as->current_map = new_view;
- flatview_destroy(old_view);
+ qemu_mutex_unlock(&flat_view_mutex);
+
+ /* Note that all the old MemoryRegions are still alive up to this
+ * point. This relieves most MemoryListeners from the need to
+ * ref/unref the MemoryRegions they get---unless they use them
+ * outside the iothread mutex, in which case precise reference
+ * counting is necessary.
+ */
+ flatview_unref(old_view);
+
address_space_update_ioeventfds(as);
}
@@ -1138,12 +1188,13 @@ void memory_region_sync_dirty_bitmap(MemoryRegion *mr)
FlatRange *fr;
QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
- FlatView *view = as->current_map;
+ FlatView *view = address_space_get_flatview(as);
FOR_EACH_FLAT_RANGE(fr, view) {
if (fr->mr == mr) {
MEMORY_LISTENER_UPDATE_REGION(fr, as, Forward, log_sync);
}
}
+ flatview_unref(view);
}
}
@@ -1195,7 +1246,7 @@ static void memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpa
AddrRange tmp;
MemoryRegionSection section;
- view = as->current_map;
+ view = address_space_get_flatview(as);
FOR_EACH_FLAT_RANGE(fr, view) {
if (fr->mr == mr) {
section = (MemoryRegionSection) {
@@ -1221,6 +1272,7 @@ static void memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpa
}
}
}
+ flatview_unref(view);
}
static void memory_region_update_coalesced_range(MemoryRegion *mr)
@@ -1508,7 +1560,7 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr,
as = memory_region_to_address_space(root);
range = addrrange_make(int128_make64(addr), int128_make64(size));
- view = as->current_map;
+ view = address_space_get_flatview(as);
fr = flatview_lookup(view, range);
if (!fr) {
return ret;
@@ -1529,6 +1581,7 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr,
ret.readonly = fr->readonly;
memory_region_ref(ret.mr);
+ flatview_unref(view);
return ret;
}
@@ -1537,10 +1590,11 @@ void address_space_sync_dirty_bitmap(AddressSpace *as)
FlatView *view;
FlatRange *fr;
- view = as->current_map;
+ view = address_space_get_flatview(as);
FOR_EACH_FLAT_RANGE(fr, view) {
MEMORY_LISTENER_UPDATE_REGION(fr, as, Forward, log_sync);
}
+ flatview_unref(view);
}
void memory_global_dirty_log_start(void)
@@ -1572,7 +1626,7 @@ static void listener_add_address_space(MemoryListener *listener,
}
}
- view = as->current_map;
+ view = address_space_get_flatview(as);
FOR_EACH_FLAT_RANGE(fr, view) {
MemoryRegionSection section = {
.mr = fr->mr,
@@ -1586,6 +1640,7 @@ static void listener_add_address_space(MemoryListener *listener,
listener->region_add(listener, §ion);
}
}
+ flatview_unref(view);
}
void memory_listener_register(MemoryListener *listener, AddressSpace *filter)
@@ -1619,6 +1674,10 @@ void memory_listener_unregister(MemoryListener *listener)
void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
{
+ if (QTAILQ_EMPTY(&address_spaces)) {
+ memory_init();
+ }
+
memory_region_transaction_begin();
as->root = root;
as->current_map = g_new(FlatView, 1);
@@ -1640,7 +1699,7 @@ void address_space_destroy(AddressSpace *as)
memory_region_transaction_commit();
QTAILQ_REMOVE(&address_spaces, as, address_spaces_link);
address_space_destroy_dispatch(as);
- flatview_destroy(as->current_map);
+ flatview_unref(as->current_map);
g_free(as->name);
g_free(as->current_map);
g_free(as->ioeventfds);
--
1.7.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [RFC PATCH 8/8] dataplane: replace hostmem with memory_region_find
2013-05-06 14:25 [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, replace hostmem with memory_region_find Paolo Bonzini
` (6 preceding siblings ...)
2013-05-06 14:25 ` [Qemu-devel] [RFC PATCH 7/8] memory: add reference counting to FlatView Paolo Bonzini
@ 2013-05-06 14:25 ` Paolo Bonzini
2013-05-08 6:20 ` [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, " liu ping fan
` (2 subsequent siblings)
10 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2013-05-06 14:25 UTC (permalink / raw)
To: qemu-devel; +Cc: jan.kiszka, qemulist, stefanha
This is incomplete, because the region is unref-ed immediately.
It would not be safe against memory hot-unplug. Otherwise it
works.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/virtio/dataplane/Makefile.objs | 2 +-
hw/virtio/dataplane/hostmem.c | 183 ---------------------------------
hw/virtio/dataplane/vring.c | 56 ++++++++--
include/hw/virtio/dataplane/hostmem.h | 58 -----------
include/hw/virtio/dataplane/vring.h | 3 +-
5 files changed, 48 insertions(+), 254 deletions(-)
delete mode 100644 hw/virtio/dataplane/hostmem.c
delete mode 100644 include/hw/virtio/dataplane/hostmem.h
diff --git a/hw/virtio/dataplane/Makefile.objs b/hw/virtio/dataplane/Makefile.objs
index a91bf33..9a8cfc0 100644
--- a/hw/virtio/dataplane/Makefile.objs
+++ b/hw/virtio/dataplane/Makefile.objs
@@ -1 +1 @@
-common-obj-y += hostmem.o vring.o
+common-obj-y += vring.o
diff --git a/hw/virtio/dataplane/hostmem.c b/hw/virtio/dataplane/hostmem.c
deleted file mode 100644
index 66829bb..0000000
--- a/hw/virtio/dataplane/hostmem.c
+++ /dev/null
@@ -1,183 +0,0 @@
-/*
- * Thread-safe guest to host memory mapping
- *
- * Copyright 2012 Red Hat, Inc. and/or its affiliates
- *
- * Authors:
- * Stefan Hajnoczi <stefanha@redhat.com>
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- *
- */
-
-#include "exec/address-spaces.h"
-#include "hw/virtio/dataplane/hostmem.h"
-
-static int hostmem_lookup_cmp(const void *phys_, const void *region_)
-{
- hwaddr phys = *(const hwaddr *)phys_;
- const HostMemRegion *region = region_;
-
- if (phys < region->guest_addr) {
- return -1;
- } else if (phys >= region->guest_addr + region->size) {
- return 1;
- } else {
- return 0;
- }
-}
-
-/**
- * Map guest physical address to host pointer
- */
-void *hostmem_lookup(HostMem *hostmem, hwaddr phys, hwaddr len, bool is_write)
-{
- HostMemRegion *region;
- void *host_addr = NULL;
- hwaddr offset_within_region;
-
- qemu_mutex_lock(&hostmem->current_regions_lock);
- region = bsearch(&phys, hostmem->current_regions,
- hostmem->num_current_regions,
- sizeof(hostmem->current_regions[0]),
- hostmem_lookup_cmp);
- if (!region) {
- goto out;
- }
- if (is_write && region->readonly) {
- goto out;
- }
- offset_within_region = phys - region->guest_addr;
- if (len <= region->size - offset_within_region) {
- host_addr = region->host_addr + offset_within_region;
- }
-out:
- qemu_mutex_unlock(&hostmem->current_regions_lock);
-
- return host_addr;
-}
-
-/**
- * Install new regions list
- */
-static void hostmem_listener_commit(MemoryListener *listener)
-{
- HostMem *hostmem = container_of(listener, HostMem, listener);
- int i;
-
- qemu_mutex_lock(&hostmem->current_regions_lock);
- for (i = 0; i < hostmem->num_current_regions; i++) {
- memory_region_unref(hostmem->current_regions[i].mr);
- }
- 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);
-
- /* Reset new regions list */
- hostmem->new_regions = NULL;
- hostmem->num_new_regions = 0;
-}
-
-/**
- * Add a MemoryRegionSection to the new regions list
- */
-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]);
-
- hostmem->new_regions = g_realloc(hostmem->new_regions, new_size);
- hostmem->new_regions[num] = (HostMemRegion){
- .host_addr = ram_ptr + section->offset_within_region,
- .guest_addr = section->offset_within_address_space,
- .size = section->size,
- .readonly = section->readonly,
- .mr = section->mr,
- };
- hostmem->num_new_regions++;
-
- memory_region_ref(section->mr);
-}
-
-static void hostmem_listener_append_region(MemoryListener *listener,
- MemoryRegionSection *section)
-{
- HostMem *hostmem = container_of(listener, HostMem, listener);
-
- /* Ignore non-RAM regions, we may not be able to map them */
- if (!memory_region_is_ram(section->mr)) {
- return;
- }
-
- /* Ignore regions with dirty logging, we cannot mark them dirty */
- if (memory_region_is_logging(section->mr)) {
- return;
- }
-
- hostmem_append_new_region(hostmem, section);
-}
-
-/* We don't implement most MemoryListener callbacks, use these nop stubs */
-static void hostmem_listener_dummy(MemoryListener *listener)
-{
-}
-
-static void hostmem_listener_section_dummy(MemoryListener *listener,
- MemoryRegionSection *section)
-{
-}
-
-static void hostmem_listener_eventfd_dummy(MemoryListener *listener,
- MemoryRegionSection *section,
- bool match_data, uint64_t data,
- EventNotifier *e)
-{
-}
-
-static void hostmem_listener_coalesced_mmio_dummy(MemoryListener *listener,
- MemoryRegionSection *section,
- hwaddr addr, hwaddr len)
-{
-}
-
-void hostmem_init(HostMem *hostmem)
-{
- memset(hostmem, 0, sizeof(*hostmem));
-
- qemu_mutex_init(&hostmem->current_regions_lock);
-
- hostmem->listener = (MemoryListener){
- .begin = hostmem_listener_dummy,
- .commit = hostmem_listener_commit,
- .region_add = hostmem_listener_append_region,
- .region_del = hostmem_listener_section_dummy,
- .region_nop = hostmem_listener_append_region,
- .log_start = hostmem_listener_section_dummy,
- .log_stop = hostmem_listener_section_dummy,
- .log_sync = hostmem_listener_section_dummy,
- .log_global_start = hostmem_listener_dummy,
- .log_global_stop = hostmem_listener_dummy,
- .eventfd_add = hostmem_listener_eventfd_dummy,
- .eventfd_del = hostmem_listener_eventfd_dummy,
- .coalesced_mmio_add = hostmem_listener_coalesced_mmio_dummy,
- .coalesced_mmio_del = hostmem_listener_coalesced_mmio_dummy,
- .priority = 10,
- };
-
- memory_listener_register(&hostmem->listener, &address_space_memory);
- if (hostmem->num_new_regions > 0) {
- hostmem_listener_commit(&hostmem->listener);
- }
-}
-
-void hostmem_finalize(HostMem *hostmem)
-{
- memory_listener_unregister(&hostmem->listener);
- g_free(hostmem->new_regions);
- g_free(hostmem->current_regions);
- qemu_mutex_destroy(&hostmem->current_regions_lock);
-}
diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c
index e0d6e83..3b94279 100644
--- a/hw/virtio/dataplane/vring.c
+++ b/hw/virtio/dataplane/vring.c
@@ -15,9 +15,41 @@
*/
#include "trace.h"
+#include "hw/hw.h"
+#include "exec/memory.h"
+#include "exec/address-spaces.h"
#include "hw/virtio/dataplane/vring.h"
#include "qemu/error-report.h"
+static void *vring_map(MemoryRegion **mr, hwaddr phys, hwaddr len,
+ bool is_write)
+{
+ MemoryRegionSection section = memory_region_find(get_system_memory(), phys, len);
+
+ if (!section.mr || section.size < len) {
+ goto out;
+ }
+ if (is_write && section.readonly) {
+ goto out;
+ }
+ if (!memory_region_is_ram(section.mr)) {
+ goto out;
+ }
+
+ /* Ignore regions with dirty logging, we cannot mark them dirty */
+ if (memory_region_is_logging(section.mr)) {
+ goto out;
+ }
+
+ *mr = section.mr;
+ return memory_region_get_ram_ptr(section.mr) + section.offset_within_region;
+
+out:
+ memory_region_unref(section.mr);
+ *mr = NULL;
+ return NULL;
+}
+
/* Map the guest's vring to host memory */
bool vring_setup(Vring *vring, VirtIODevice *vdev, int n)
{
@@ -27,8 +59,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 = vring_map(&vring->mr, vring_addr, vring_size, true);
if (!vring_ptr) {
error_report("Failed to map vring "
"addr %#" HWADDR_PRIx " size %" HWADDR_PRIu,
@@ -51,7 +82,7 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n)
void vring_teardown(Vring *vring)
{
- hostmem_finalize(&vring->hostmem);
+ memory_region_unref(vring->mr);
}
/* Disable guest->host notifies */
@@ -136,11 +167,13 @@ static int get_indirect(Vring *vring,
do {
struct vring_desc *desc_ptr;
+ MemoryRegion *mr;
/* Translate indirect descriptor */
- desc_ptr = hostmem_lookup(&vring->hostmem,
- indirect->addr + found * sizeof(desc),
- sizeof(desc), false);
+ desc_ptr = vring_map(&mr,
+ indirect->addr + found * sizeof(desc),
+ sizeof(desc), false);
+ memory_region_unref(mr); /* TODO */
if (!desc_ptr) {
error_report("Failed to map indirect descriptor "
"addr %#" PRIx64 " len %zu",
@@ -172,8 +205,9 @@ static int get_indirect(Vring *vring,
return -ENOBUFS;
}
- iov->iov_base = hostmem_lookup(&vring->hostmem, desc.addr, desc.len,
- desc.flags & VRING_DESC_F_WRITE);
+ iov->iov_base = vring_map(&mr, desc.addr, desc.len,
+ desc.flags & VRING_DESC_F_WRITE);
+ memory_region_unref(mr); /* TODO */
if (!iov->iov_base) {
error_report("Failed to map indirect descriptor"
"addr %#" PRIx64 " len %u",
@@ -221,6 +255,7 @@ 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 *mr;
/* If there was a fatal error then refuse operation */
if (vring->broken) {
@@ -300,8 +335,9 @@ 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,
- desc.flags & VRING_DESC_F_WRITE);
+ iov->iov_base = vring_map(&mr, desc.addr, desc.len,
+ desc.flags & VRING_DESC_F_WRITE);
+ memory_region_unref(mr); /* TODO */
if (!iov->iov_base) {
error_report("Failed to map vring desc addr %#" PRIx64 " len %u",
(uint64_t)desc.addr, desc.len);
diff --git a/include/hw/virtio/dataplane/hostmem.h b/include/hw/virtio/dataplane/hostmem.h
deleted file mode 100644
index 2810f4b..0000000
--- a/include/hw/virtio/dataplane/hostmem.h
+++ /dev/null
@@ -1,58 +0,0 @@
-/*
- * Thread-safe guest to host memory mapping
- *
- * Copyright 2012 Red Hat, Inc. and/or its affiliates
- *
- * Authors:
- * Stefan Hajnoczi <stefanha@redhat.com>
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- *
- */
-
-#ifndef HOSTMEM_H
-#define HOSTMEM_H
-
-#include "exec/memory.h"
-#include "qemu/thread.h"
-
-typedef struct {
- MemoryRegion *mr;
- void *host_addr;
- hwaddr guest_addr;
- uint64_t size;
- bool readonly;
-} HostMemRegion;
-
-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;
- HostMemRegion *current_regions;
- size_t num_current_regions;
-} HostMem;
-
-void hostmem_init(HostMem *hostmem);
-void hostmem_finalize(HostMem *hostmem);
-
-/**
- * Map a guest physical address to a pointer
- *
- * Note that there is map/unmap mechanism here. The caller must ensure that
- * mapped memory is no longer used across events like hot memory unplug. This
- * 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);
-
-#endif /* HOSTMEM_H */
diff --git a/include/hw/virtio/dataplane/vring.h b/include/hw/virtio/dataplane/vring.h
index 9380cb5..caf5a41 100644
--- a/include/hw/virtio/dataplane/vring.h
+++ b/include/hw/virtio/dataplane/vring.h
@@ -19,11 +19,10 @@
#include <linux/virtio_ring.h>
#include "qemu-common.h"
-#include "hostmem.h"
#include "hw/virtio/virtio.h"
typedef struct {
- HostMem hostmem; /* guest memory mapper */
+ MemoryRegion *mr; /* memory region containing the vring */
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.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, replace hostmem with memory_region_find
2013-05-06 14:25 [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, replace hostmem with memory_region_find Paolo Bonzini
` (7 preceding siblings ...)
2013-05-06 14:25 ` [Qemu-devel] [RFC PATCH 8/8] dataplane: replace hostmem with memory_region_find Paolo Bonzini
@ 2013-05-08 6:20 ` liu ping fan
2013-05-08 15:44 ` Paolo Bonzini
2013-05-08 9:18 ` Stefan Hajnoczi
2013-05-08 9:25 ` Stefan Hajnoczi
10 siblings, 1 reply; 16+ messages in thread
From: liu ping fan @ 2013-05-08 6:20 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: jan.kiszka, qemu-devel, stefanha
On Mon, May 6, 2013 at 10:25 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Hi,
>
> this is an alternative approach to refactoring of dataplane's HostMem
> code. Here, I take Ping Fan's idea of RCU-style updating of the
> region list and apply it to the AddressSpace's FlatView. With this
In fact, I am worrying about the priority of MemoryListener, if it is
true, then we should drop RCU-style idea. Also if it is true, there is
already a bug with hostmem listener. It should use region_del, not
region_nop to reconstruct the local view. But just let me have a deep
thinking.
Regards,
Pingfan
> change, dataplane can simply use memory_region_find instead of
> hostmem.
>
> This is a somewhat larger change, but I prefer it for two reasons.
>
> 1) it splits the task of adding BQL-less memory dispatch in two parts,
> tacking memory_region_find first (which is simpler because locking
> is left to the caller).
>
> 2) HostMem duplicates a lot of the FlatView logic, and adding the
> RCU-style update in FlatView benefits everyone.
>
> The missing ingredients here are:
>
> 1) remember and unreference the MemoryRegions that are used in
> a vring entry. In order to implement this, it is probably simpler
> to change vring.c to use virtio.c's VirtQueueElement data structure.
> We want something like that anyway in order to support migration.
>
> 2) add an owner field to MemoryRegion, and set it for all MemoryRegions
> for hot-unpluggable devices. In this series, ref/unref are stubs.
>
> For simplicity I based the patches on my IOMMU rebase. I placed the
> tree at git://github.com/bonzini/qemu.git, branch iommu.
>
> Paolo
>
> Paolo Bonzini (8):
> memory: add ref/unref calls
> exec: check MRU in qemu_ram_addr_from_host
> memory: return MemoryRegion from qemu_ram_addr_from_host
> memory: ref/unref memory across address_space_map/unmap
> memory: access FlatView from a local variable
> memory: use a new FlatView pointer on every topology update
> memory: add reference counting to FlatView
> dataplane: replace hostmem with memory_region_find
>
> exec.c | 63 +++++++++---
> hw/core/loader.c | 1 +
> hw/display/exynos4210_fimd.c | 6 +
> hw/display/framebuffer.c | 10 +-
> hw/i386/kvm/ioapic.c | 2 +
> hw/i386/kvmvapic.c | 1 +
> hw/misc/vfio.c | 2 +
> hw/virtio/dataplane/Makefile.objs | 2 +-
> hw/virtio/dataplane/hostmem.c | 176 ---------------------------------
> hw/virtio/dataplane/vring.c | 56 +++++++++--
> hw/virtio/vhost.c | 2 +
> hw/virtio/virtio-balloon.c | 1 +
> hw/xen/xen_pt.c | 4 +
> include/exec/cpu-common.h | 2 +-
> include/exec/memory.h | 9 ++
> include/hw/virtio/dataplane/hostmem.h | 57 -----------
> include/hw/virtio/dataplane/vring.h | 3 +-
> kvm-all.c | 2 +
> memory.c | 142 +++++++++++++++++++++-----
> target-arm/kvm.c | 2 +
> target-i386/kvm.c | 4 +-
> target-sparc/mmu_helper.c | 1 +
> xen-all.c | 2 +
> 23 files changed, 253 insertions(+), 297 deletions(-)
> delete mode 100644 hw/virtio/dataplane/hostmem.c
> delete mode 100644 include/hw/virtio/dataplane/hostmem.h
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, replace hostmem with memory_region_find
2013-05-08 6:20 ` [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, " liu ping fan
@ 2013-05-08 15:44 ` Paolo Bonzini
2013-05-09 0:53 ` liu ping fan
0 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2013-05-08 15:44 UTC (permalink / raw)
To: liu ping fan; +Cc: jan.kiszka, qemu-devel, stefanha
Il 08/05/2013 08:20, liu ping fan ha scritto:
> On Mon, May 6, 2013 at 10:25 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Hi,
>>
>> this is an alternative approach to refactoring of dataplane's HostMem
>> code. Here, I take Ping Fan's idea of RCU-style updating of the
>> region list and apply it to the AddressSpace's FlatView. With this
>
> In fact, I am worrying about the priority of MemoryListener, if it is
> true, then we should drop RCU-style idea.
You mean in hostmem, or in general as in this patch? Note that this
patch releases the old FlatView at the end of all MemoryListener operations.
Paolo
> Also if it is true, there is
> already a bug with hostmem listener. It should use region_del, not
> region_nop to reconstruct the local view. But just let me have a deep
> thinking.
>
> Regards,
> Pingfan
>> change, dataplane can simply use memory_region_find instead of
>> hostmem.
>>
>> This is a somewhat larger change, but I prefer it for two reasons.
>>
>> 1) it splits the task of adding BQL-less memory dispatch in two parts,
>> tacking memory_region_find first (which is simpler because locking
>> is left to the caller).
>>
>> 2) HostMem duplicates a lot of the FlatView logic, and adding the
>> RCU-style update in FlatView benefits everyone.
>>
>> The missing ingredients here are:
>>
>> 1) remember and unreference the MemoryRegions that are used in
>> a vring entry. In order to implement this, it is probably simpler
>> to change vring.c to use virtio.c's VirtQueueElement data structure.
>> We want something like that anyway in order to support migration.
>>
>> 2) add an owner field to MemoryRegion, and set it for all MemoryRegions
>> for hot-unpluggable devices. In this series, ref/unref are stubs.
>>
>> For simplicity I based the patches on my IOMMU rebase. I placed the
>> tree at git://github.com/bonzini/qemu.git, branch iommu.
>>
>> Paolo
>>
>> Paolo Bonzini (8):
>> memory: add ref/unref calls
>> exec: check MRU in qemu_ram_addr_from_host
>> memory: return MemoryRegion from qemu_ram_addr_from_host
>> memory: ref/unref memory across address_space_map/unmap
>> memory: access FlatView from a local variable
>> memory: use a new FlatView pointer on every topology update
>> memory: add reference counting to FlatView
>> dataplane: replace hostmem with memory_region_find
>>
>> exec.c | 63 +++++++++---
>> hw/core/loader.c | 1 +
>> hw/display/exynos4210_fimd.c | 6 +
>> hw/display/framebuffer.c | 10 +-
>> hw/i386/kvm/ioapic.c | 2 +
>> hw/i386/kvmvapic.c | 1 +
>> hw/misc/vfio.c | 2 +
>> hw/virtio/dataplane/Makefile.objs | 2 +-
>> hw/virtio/dataplane/hostmem.c | 176 ---------------------------------
>> hw/virtio/dataplane/vring.c | 56 +++++++++--
>> hw/virtio/vhost.c | 2 +
>> hw/virtio/virtio-balloon.c | 1 +
>> hw/xen/xen_pt.c | 4 +
>> include/exec/cpu-common.h | 2 +-
>> include/exec/memory.h | 9 ++
>> include/hw/virtio/dataplane/hostmem.h | 57 -----------
>> include/hw/virtio/dataplane/vring.h | 3 +-
>> kvm-all.c | 2 +
>> memory.c | 142 +++++++++++++++++++++-----
>> target-arm/kvm.c | 2 +
>> target-i386/kvm.c | 4 +-
>> target-sparc/mmu_helper.c | 1 +
>> xen-all.c | 2 +
>> 23 files changed, 253 insertions(+), 297 deletions(-)
>> delete mode 100644 hw/virtio/dataplane/hostmem.c
>> delete mode 100644 include/hw/virtio/dataplane/hostmem.h
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, replace hostmem with memory_region_find
2013-05-08 15:44 ` Paolo Bonzini
@ 2013-05-09 0:53 ` liu ping fan
2013-05-09 14:50 ` Paolo Bonzini
0 siblings, 1 reply; 16+ messages in thread
From: liu ping fan @ 2013-05-09 0:53 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: jan.kiszka, qemu-devel, stefanha
On Wed, May 8, 2013 at 11:44 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 08/05/2013 08:20, liu ping fan ha scritto:
>> On Mon, May 6, 2013 at 10:25 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> Hi,
>>>
>>> this is an alternative approach to refactoring of dataplane's HostMem
>>> code. Here, I take Ping Fan's idea of RCU-style updating of the
>>> region list and apply it to the AddressSpace's FlatView. With this
>>
>> In fact, I am worrying about the priority of MemoryListener, if it is
>> true, then we should drop RCU-style idea.
>
> You mean in hostmem, or in general as in this patch? Note that this
> patch releases the old FlatView at the end of all MemoryListener operations.
>
Both in hostmem and this patch, they all broke the original design of
the MemoryListener, see notes for priority in code.
I have set out 2 patches to highlight this issue, and have CC you and Stefanha.
Regards,
Pingfan
> Paolo
>
>> Also if it is true, there is
>> already a bug with hostmem listener. It should use region_del, not
>> region_nop to reconstruct the local view. But just let me have a deep
>> thinking.
>>
>> Regards,
>> Pingfan
>>> change, dataplane can simply use memory_region_find instead of
>>> hostmem.
>>>
>>> This is a somewhat larger change, but I prefer it for two reasons.
>>>
>>> 1) it splits the task of adding BQL-less memory dispatch in two parts,
>>> tacking memory_region_find first (which is simpler because locking
>>> is left to the caller).
>>>
>>> 2) HostMem duplicates a lot of the FlatView logic, and adding the
>>> RCU-style update in FlatView benefits everyone.
>>>
>>> The missing ingredients here are:
>>>
>>> 1) remember and unreference the MemoryRegions that are used in
>>> a vring entry. In order to implement this, it is probably simpler
>>> to change vring.c to use virtio.c's VirtQueueElement data structure.
>>> We want something like that anyway in order to support migration.
>>>
>>> 2) add an owner field to MemoryRegion, and set it for all MemoryRegions
>>> for hot-unpluggable devices. In this series, ref/unref are stubs.
>>>
>>> For simplicity I based the patches on my IOMMU rebase. I placed the
>>> tree at git://github.com/bonzini/qemu.git, branch iommu.
>>>
>>> Paolo
>>>
>>> Paolo Bonzini (8):
>>> memory: add ref/unref calls
>>> exec: check MRU in qemu_ram_addr_from_host
>>> memory: return MemoryRegion from qemu_ram_addr_from_host
>>> memory: ref/unref memory across address_space_map/unmap
>>> memory: access FlatView from a local variable
>>> memory: use a new FlatView pointer on every topology update
>>> memory: add reference counting to FlatView
>>> dataplane: replace hostmem with memory_region_find
>>>
>>> exec.c | 63 +++++++++---
>>> hw/core/loader.c | 1 +
>>> hw/display/exynos4210_fimd.c | 6 +
>>> hw/display/framebuffer.c | 10 +-
>>> hw/i386/kvm/ioapic.c | 2 +
>>> hw/i386/kvmvapic.c | 1 +
>>> hw/misc/vfio.c | 2 +
>>> hw/virtio/dataplane/Makefile.objs | 2 +-
>>> hw/virtio/dataplane/hostmem.c | 176 ---------------------------------
>>> hw/virtio/dataplane/vring.c | 56 +++++++++--
>>> hw/virtio/vhost.c | 2 +
>>> hw/virtio/virtio-balloon.c | 1 +
>>> hw/xen/xen_pt.c | 4 +
>>> include/exec/cpu-common.h | 2 +-
>>> include/exec/memory.h | 9 ++
>>> include/hw/virtio/dataplane/hostmem.h | 57 -----------
>>> include/hw/virtio/dataplane/vring.h | 3 +-
>>> kvm-all.c | 2 +
>>> memory.c | 142 +++++++++++++++++++++-----
>>> target-arm/kvm.c | 2 +
>>> target-i386/kvm.c | 4 +-
>>> target-sparc/mmu_helper.c | 1 +
>>> xen-all.c | 2 +
>>> 23 files changed, 253 insertions(+), 297 deletions(-)
>>> delete mode 100644 hw/virtio/dataplane/hostmem.c
>>> delete mode 100644 include/hw/virtio/dataplane/hostmem.h
>>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, replace hostmem with memory_region_find
2013-05-09 0:53 ` liu ping fan
@ 2013-05-09 14:50 ` Paolo Bonzini
2013-05-10 0:23 ` liu ping fan
0 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2013-05-09 14:50 UTC (permalink / raw)
To: liu ping fan; +Cc: jan.kiszka, qemu-devel, stefanha
Il 09/05/2013 02:53, liu ping fan ha scritto:
> On Wed, May 8, 2013 at 11:44 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 08/05/2013 08:20, liu ping fan ha scritto:
>>> On Mon, May 6, 2013 at 10:25 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>> Hi,
>>>>
>>>> this is an alternative approach to refactoring of dataplane's HostMem
>>>> code. Here, I take Ping Fan's idea of RCU-style updating of the
>>>> region list and apply it to the AddressSpace's FlatView. With this
>>>
>>> In fact, I am worrying about the priority of MemoryListener, if it is
>>> true, then we should drop RCU-style idea.
>>
>> You mean in hostmem, or in general as in this patch? Note that this
>> patch releases the old FlatView at the end of all MemoryListener operations.
>>
> Both in hostmem and this patch, they all broke the original design of
> the MemoryListener, see notes for priority in code.
I think both hostmem and this patch are fine. The hypervisor is never
involved, all accesses go through the "old" FlatView and regions cannot
disappear thanks to ref/unref.
In fact, we need _more_ RCU-style updates, not less. For BQL-less
dispatch, address space mapping/translation can race against the
MemoryListeners in exec.c. To fix this, phys_sections and
AddressSpaceDispatch need to be reference counted and RCU-ified as well.
Paolo
> I have set out 2 patches to highlight this issue, and have CC you and Stefanha.
>
> Regards,
> Pingfan
>
>> Paolo
>>
>>> Also if it is true, there is
>>> already a bug with hostmem listener. It should use region_del, not
>>> region_nop to reconstruct the local view. But just let me have a deep
>>> thinking.
>>>
>>> Regards,
>>> Pingfan
>>>> change, dataplane can simply use memory_region_find instead of
>>>> hostmem.
>>>>
>>>> This is a somewhat larger change, but I prefer it for two reasons.
>>>>
>>>> 1) it splits the task of adding BQL-less memory dispatch in two parts,
>>>> tacking memory_region_find first (which is simpler because locking
>>>> is left to the caller).
>>>>
>>>> 2) HostMem duplicates a lot of the FlatView logic, and adding the
>>>> RCU-style update in FlatView benefits everyone.
>>>>
>>>> The missing ingredients here are:
>>>>
>>>> 1) remember and unreference the MemoryRegions that are used in
>>>> a vring entry. In order to implement this, it is probably simpler
>>>> to change vring.c to use virtio.c's VirtQueueElement data structure.
>>>> We want something like that anyway in order to support migration.
>>>>
>>>> 2) add an owner field to MemoryRegion, and set it for all MemoryRegions
>>>> for hot-unpluggable devices. In this series, ref/unref are stubs.
>>>>
>>>> For simplicity I based the patches on my IOMMU rebase. I placed the
>>>> tree at git://github.com/bonzini/qemu.git, branch iommu.
>>>>
>>>> Paolo
>>>>
>>>> Paolo Bonzini (8):
>>>> memory: add ref/unref calls
>>>> exec: check MRU in qemu_ram_addr_from_host
>>>> memory: return MemoryRegion from qemu_ram_addr_from_host
>>>> memory: ref/unref memory across address_space_map/unmap
>>>> memory: access FlatView from a local variable
>>>> memory: use a new FlatView pointer on every topology update
>>>> memory: add reference counting to FlatView
>>>> dataplane: replace hostmem with memory_region_find
>>>>
>>>> exec.c | 63 +++++++++---
>>>> hw/core/loader.c | 1 +
>>>> hw/display/exynos4210_fimd.c | 6 +
>>>> hw/display/framebuffer.c | 10 +-
>>>> hw/i386/kvm/ioapic.c | 2 +
>>>> hw/i386/kvmvapic.c | 1 +
>>>> hw/misc/vfio.c | 2 +
>>>> hw/virtio/dataplane/Makefile.objs | 2 +-
>>>> hw/virtio/dataplane/hostmem.c | 176 ---------------------------------
>>>> hw/virtio/dataplane/vring.c | 56 +++++++++--
>>>> hw/virtio/vhost.c | 2 +
>>>> hw/virtio/virtio-balloon.c | 1 +
>>>> hw/xen/xen_pt.c | 4 +
>>>> include/exec/cpu-common.h | 2 +-
>>>> include/exec/memory.h | 9 ++
>>>> include/hw/virtio/dataplane/hostmem.h | 57 -----------
>>>> include/hw/virtio/dataplane/vring.h | 3 +-
>>>> kvm-all.c | 2 +
>>>> memory.c | 142 +++++++++++++++++++++-----
>>>> target-arm/kvm.c | 2 +
>>>> target-i386/kvm.c | 4 +-
>>>> target-sparc/mmu_helper.c | 1 +
>>>> xen-all.c | 2 +
>>>> 23 files changed, 253 insertions(+), 297 deletions(-)
>>>> delete mode 100644 hw/virtio/dataplane/hostmem.c
>>>> delete mode 100644 include/hw/virtio/dataplane/hostmem.h
>>>>
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, replace hostmem with memory_region_find
2013-05-09 14:50 ` Paolo Bonzini
@ 2013-05-10 0:23 ` liu ping fan
0 siblings, 0 replies; 16+ messages in thread
From: liu ping fan @ 2013-05-10 0:23 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: jan.kiszka, qemu-devel, stefanha
On Thu, May 9, 2013 at 10:50 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 09/05/2013 02:53, liu ping fan ha scritto:
>> On Wed, May 8, 2013 at 11:44 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> Il 08/05/2013 08:20, liu ping fan ha scritto:
>>>> On Mon, May 6, 2013 at 10:25 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>> Hi,
>>>>>
>>>>> this is an alternative approach to refactoring of dataplane's HostMem
>>>>> code. Here, I take Ping Fan's idea of RCU-style updating of the
>>>>> region list and apply it to the AddressSpace's FlatView. With this
>>>>
>>>> In fact, I am worrying about the priority of MemoryListener, if it is
>>>> true, then we should drop RCU-style idea.
>>>
>>> You mean in hostmem, or in general as in this patch? Note that this
>>> patch releases the old FlatView at the end of all MemoryListener operations.
>>>
>> Both in hostmem and this patch, they all broke the original design of
>> the MemoryListener, see notes for priority in code.
>
> I think both hostmem and this patch are fine. The hypervisor is never
> involved, all accesses go through the "old" FlatView and regions cannot
> disappear thanks to ref/unref.
>
Here, we worry about add, not del.
> In fact, we need _more_ RCU-style updates, not less. For BQL-less
> dispatch, address space mapping/translation can race against the
> MemoryListeners in exec.c. To fix this, phys_sections and
> AddressSpaceDispatch need to be reference counted and RCU-ified as well.
>
Agree, I like RCU too.
> Paolo
>
>> I have set out 2 patches to highlight this issue, and have CC you and Stefanha.
>>
>> Regards,
>> Pingfan
>>
>>> Paolo
>>>
>>>> Also if it is true, there is
>>>> already a bug with hostmem listener. It should use region_del, not
>>>> region_nop to reconstruct the local view. But just let me have a deep
>>>> thinking.
>>>>
>>>> Regards,
>>>> Pingfan
>>>>> change, dataplane can simply use memory_region_find instead of
>>>>> hostmem.
>>>>>
>>>>> This is a somewhat larger change, but I prefer it for two reasons.
>>>>>
>>>>> 1) it splits the task of adding BQL-less memory dispatch in two parts,
>>>>> tacking memory_region_find first (which is simpler because locking
>>>>> is left to the caller).
>>>>>
>>>>> 2) HostMem duplicates a lot of the FlatView logic, and adding the
>>>>> RCU-style update in FlatView benefits everyone.
>>>>>
>>>>> The missing ingredients here are:
>>>>>
>>>>> 1) remember and unreference the MemoryRegions that are used in
>>>>> a vring entry. In order to implement this, it is probably simpler
>>>>> to change vring.c to use virtio.c's VirtQueueElement data structure.
>>>>> We want something like that anyway in order to support migration.
>>>>>
>>>>> 2) add an owner field to MemoryRegion, and set it for all MemoryRegions
>>>>> for hot-unpluggable devices. In this series, ref/unref are stubs.
>>>>>
>>>>> For simplicity I based the patches on my IOMMU rebase. I placed the
>>>>> tree at git://github.com/bonzini/qemu.git, branch iommu.
>>>>>
>>>>> Paolo
>>>>>
>>>>> Paolo Bonzini (8):
>>>>> memory: add ref/unref calls
>>>>> exec: check MRU in qemu_ram_addr_from_host
>>>>> memory: return MemoryRegion from qemu_ram_addr_from_host
>>>>> memory: ref/unref memory across address_space_map/unmap
>>>>> memory: access FlatView from a local variable
>>>>> memory: use a new FlatView pointer on every topology update
>>>>> memory: add reference counting to FlatView
>>>>> dataplane: replace hostmem with memory_region_find
>>>>>
>>>>> exec.c | 63 +++++++++---
>>>>> hw/core/loader.c | 1 +
>>>>> hw/display/exynos4210_fimd.c | 6 +
>>>>> hw/display/framebuffer.c | 10 +-
>>>>> hw/i386/kvm/ioapic.c | 2 +
>>>>> hw/i386/kvmvapic.c | 1 +
>>>>> hw/misc/vfio.c | 2 +
>>>>> hw/virtio/dataplane/Makefile.objs | 2 +-
>>>>> hw/virtio/dataplane/hostmem.c | 176 ---------------------------------
>>>>> hw/virtio/dataplane/vring.c | 56 +++++++++--
>>>>> hw/virtio/vhost.c | 2 +
>>>>> hw/virtio/virtio-balloon.c | 1 +
>>>>> hw/xen/xen_pt.c | 4 +
>>>>> include/exec/cpu-common.h | 2 +-
>>>>> include/exec/memory.h | 9 ++
>>>>> include/hw/virtio/dataplane/hostmem.h | 57 -----------
>>>>> include/hw/virtio/dataplane/vring.h | 3 +-
>>>>> kvm-all.c | 2 +
>>>>> memory.c | 142 +++++++++++++++++++++-----
>>>>> target-arm/kvm.c | 2 +
>>>>> target-i386/kvm.c | 4 +-
>>>>> target-sparc/mmu_helper.c | 1 +
>>>>> xen-all.c | 2 +
>>>>> 23 files changed, 253 insertions(+), 297 deletions(-)
>>>>> delete mode 100644 hw/virtio/dataplane/hostmem.c
>>>>> delete mode 100644 include/hw/virtio/dataplane/hostmem.h
>>>>>
>>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, replace hostmem with memory_region_find
2013-05-06 14:25 [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, replace hostmem with memory_region_find Paolo Bonzini
` (8 preceding siblings ...)
2013-05-08 6:20 ` [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, " liu ping fan
@ 2013-05-08 9:18 ` Stefan Hajnoczi
2013-05-08 9:25 ` Stefan Hajnoczi
10 siblings, 0 replies; 16+ messages in thread
From: Stefan Hajnoczi @ 2013-05-08 9:18 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: jan.kiszka, qemu-devel, qemulist
On Mon, May 06, 2013 at 04:25:13PM +0200, Paolo Bonzini wrote:
> this is an alternative approach to refactoring of dataplane's HostMem
> code. Here, I take Ping Fan's idea of RCU-style updating of the
> region list and apply it to the AddressSpace's FlatView. With this
> change, dataplane can simply use memory_region_find instead of
> hostmem.
>
> This is a somewhat larger change, but I prefer it for two reasons.
>
> 1) it splits the task of adding BQL-less memory dispatch in two parts,
> tacking memory_region_find first (which is simpler because locking
> is left to the caller).
>
> 2) HostMem duplicates a lot of the FlatView logic, and adding the
> RCU-style update in FlatView benefits everyone.
>
> The missing ingredients here are:
>
> 1) remember and unreference the MemoryRegions that are used in
> a vring entry. In order to implement this, it is probably simpler
> to change vring.c to use virtio.c's VirtQueueElement data structure.
> We want something like that anyway in order to support migration.
Agreed. I want to drop vring.c and have virtio.c use thread-safe APIs
so it can be used from dataplane. VirtQueueElement can hide the
MemoryRegion reference so the virtio device caller (net, block, etc)
doesn't need to juggle references manually.
> 2) add an owner field to MemoryRegion, and set it for all MemoryRegions
> for hot-unpluggable devices. In this series, ref/unref are stubs.
>
> For simplicity I based the patches on my IOMMU rebase. I placed the
> tree at git://github.com/bonzini/qemu.git, branch iommu.
Are you hoping that Ping Fan will pick up this RFC or will you push it
yourself?
Either way, I'm very interested in a thread-safe memory API.
Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, replace hostmem with memory_region_find
2013-05-06 14:25 [Qemu-devel] [RFC PATCH 0/8] MemoryRegion and FlatView refcounting, replace hostmem with memory_region_find Paolo Bonzini
` (9 preceding siblings ...)
2013-05-08 9:18 ` Stefan Hajnoczi
@ 2013-05-08 9:25 ` Stefan Hajnoczi
10 siblings, 0 replies; 16+ messages in thread
From: Stefan Hajnoczi @ 2013-05-08 9:25 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: jan.kiszka, qemu-devel, qemulist
On Mon, May 06, 2013 at 04:25:13PM +0200, Paolo Bonzini wrote:
> this is an alternative approach to refactoring of dataplane's HostMem
> code. Here, I take Ping Fan's idea of RCU-style updating of the
> region list and apply it to the AddressSpace's FlatView. With this
> change, dataplane can simply use memory_region_find instead of
> hostmem.
Getting rid of hostmem is nice since we nearly have the necessary
infrastructure in the core memory API already.
Stefan
^ permalink raw reply [flat|nested] 16+ messages in thread