Although memory_region_del_subregion() removes MemoryRegion from current address space, it's possible that it's still in use/referenced until old address space view is destroyed. That doesn't allow to unmap it from HVA region at the time of memory_region_del_subregion(). As a solution track HVA mapped MemoryRegions in a list and don't allow to map another MemoryRegion at the same address until respective MemoryRegion is destroyed, delaying unmapping from HVA range to the time MemoryRegion destructor is called.
In memory hotplug terms it would mean that user should delete corresponding backend along with pc-dimm device: device_del dimm1 object_del dimm1_backend_memdev after that dimm1_backend_memdev's MemoryRegion will be destroyed once all accesses to it are gone and old flatview is destroyed as well. As result it's possible that a following "device_add pc-dimm" at the same address may fail due to old mapping is still being present, hence add error argument to memory_region_add_subregion() API so it could report error and hotplug could be cancelled gracefully. Signed-off-by: Igor Mammedov <imamm...@redhat.com> --- hw/mem/pc-dimm.c | 6 +++++- include/exec/memory.h | 2 ++ memory.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 1 deletion(-) diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index 8cc9118..d17c22f 100644 --- a/hw/mem/pc-dimm.c +++ b/hw/mem/pc-dimm.c @@ -95,7 +95,11 @@ void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms, goto out; } - memory_region_add_subregion(&hpms->mr, addr - hpms->base, mr, &error_abort); + memory_region_add_subregion(&hpms->mr, addr - hpms->base, mr, &local_err); + if (local_err) { + goto out; + } + vmstate_register_ram(mr, dev); numa_set_mem_node_id(addr, memory_region_size(mr), dimm->node); diff --git a/include/exec/memory.h b/include/exec/memory.h index ce0320a..d9c53f9 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -174,6 +174,7 @@ struct MemoryRegion { bool romd_mode; bool ram; void *rsvd_hva; + bool hva_mapped; bool skip_dump; bool readonly; /* For RAM regions */ bool enabled; @@ -188,6 +189,7 @@ struct MemoryRegion { QTAILQ_HEAD(subregions, MemoryRegion) subregions; QTAILQ_ENTRY(MemoryRegion) subregions_link; QTAILQ_HEAD(coalesced_ranges, CoalescedMemoryRange) coalesced; + QTAILQ_ENTRY(MemoryRegion) hva_link; const char *name; uint8_t dirty_log_mask; unsigned ioeventfd_nb; diff --git a/memory.c b/memory.c index 360a5b8..e3c1b36 100644 --- a/memory.c +++ b/memory.c @@ -34,6 +34,7 @@ static unsigned memory_region_transaction_depth; static bool memory_region_update_pending; static bool ioeventfd_update_pending; static bool global_dirty_log = false; +static QemuMutex hva_lock; static QTAILQ_HEAD(memory_listeners, MemoryListener) memory_listeners = QTAILQ_HEAD_INITIALIZER(memory_listeners); @@ -1761,6 +1762,24 @@ done: memory_region_transaction_commit(); } +static QTAILQ_HEAD(, MemoryRegion) hva_mapped_head = + QTAILQ_HEAD_INITIALIZER(hva_mapped_head); + +static void memory_region_destructor_hva_ram(MemoryRegion *mr) +{ + MemoryRegion *h, *tmp; + + qemu_mutex_lock(&hva_lock); + qemu_ram_unmap_hva(mr->ram_addr); + memory_region_destructor_ram(mr); + QTAILQ_FOREACH_SAFE(h, &hva_mapped_head, hva_link, tmp) { + if (mr == h) { + QTAILQ_REMOVE(&hva_mapped_head, h, hva_link); + } + } + qemu_mutex_unlock(&hva_lock); +} + static void memory_region_add_subregion_common(MemoryRegion *mr, hwaddr offset, MemoryRegion *subregion, @@ -1772,8 +1791,43 @@ static void memory_region_add_subregion_common(MemoryRegion *mr, if (subregion->ram) { if (mr->rsvd_hva) { + MemoryRegion *h, *tmp; + Int128 e, oe; + + assert(!subregion->hva_mapped); + qemu_mutex_lock(&hva_lock); + + QTAILQ_FOREACH_SAFE(h, &hva_mapped_head, hva_link, tmp) { + if (subregion == h) { + error_setg(errp, "HVA mapped memory region '%s' is not " + "reusable, use a new one instead", + subregion->name); + qemu_mutex_unlock(&hva_lock); + return; + } + + e = int128_add(int128_make64(h->addr), + int128_make64(memory_region_size(h))); + oe = int128_add(int128_make64(offset), + int128_make64(memory_region_size(subregion))); + if (offset >= h->addr && int128_le(oe, e)) { + MemoryRegionSection rsvd_hva; + rsvd_hva = memory_region_find_hva_range(mr); + error_setg(errp, "memory at 0x%" PRIx64 " is still in use" + "by HVA mapped region: %s", + rsvd_hva.offset_within_address_space + offset, + h->name); + qemu_mutex_unlock(&hva_lock); + return; + } + } + + QTAILQ_INSERT_TAIL(&hva_mapped_head, subregion, hva_link); + subregion->destructor = memory_region_destructor_hva_ram; + subregion->hva_mapped = true; qemu_ram_remap_hva(subregion->ram_addr, memory_region_get_ram_ptr(mr) + subregion->addr); + qemu_mutex_unlock(&hva_lock); } } @@ -2288,6 +2342,7 @@ static const TypeInfo memory_region_info = { static void memory_register_types(void) { type_register_static(&memory_region_info); + qemu_mutex_init(&hva_lock); } type_init(memory_register_types) -- 1.8.3.1