From dbddac6da01a13c9d5d162994a0a265173acecab Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 23 Mar 2015 10:31:53 +0100 Subject: memory: the only dirty memory flag for users is DIRTY_MEMORY_VGA DIRTY_MEMORY_MIGRATION is triggered by memory_global_dirty_log_start and memory_global_dirty_log_stop, so it cannot be used with memory_region_set_log. Specify this in the documentation and assert it. Reviewed-by: Fam Zheng Signed-off-by: Paolo Bonzini --- memory.c | 1 + 1 file changed, 1 insertion(+) (limited to 'memory.c') diff --git a/memory.c b/memory.c index 03c536b857..a8f8599079 100644 --- a/memory.c +++ b/memory.c @@ -1425,6 +1425,7 @@ void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client) { uint8_t mask = 1 << client; + assert(client == DIRTY_MEMORY_VGA); memory_region_transaction_begin(); mr->dirty_log_mask = (mr->dirty_log_mask & ~mask) | (log * mask); memory_region_update_pending |= mr->enabled; -- cgit v1.2.3-55-g7522 From 2d1a35bef0ed96b3f23535e459c552414ccdbafd Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 23 Mar 2015 10:50:57 +0100 Subject: memory: differentiate memory_region_is_logging and memory_region_get_dirty_log_mask For now memory regions only track DIRTY_MEMORY_VGA individually, but this will change soon. To support this, split memory_region_is_logging in two functions: one that returns a given bit from dirty_log_mask, and one that returns the entire mask. memory_region_is_logging gets an extra parameter so that the compiler flags misuse. While VGA-specific users (including the Xen listener!) will want to keep checking that bit, KVM and vhost check for "any bit except migration" (because migration is handled via the global start/stop listener callbacks). Reviewed-by: Fam Zheng Signed-off-by: Paolo Bonzini --- hw/display/vmware_vga.c | 2 +- hw/virtio/dataplane/vring.c | 2 +- hw/virtio/vhost.c | 3 ++- include/exec/memory.h | 16 ++++++++++++++-- kvm-all.c | 3 ++- memory.c | 7 ++++++- xen-hvm.c | 2 +- 7 files changed, 27 insertions(+), 8 deletions(-) (limited to 'memory.c') diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index c17ddd1fcd..7f397d3c2e 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -1124,7 +1124,7 @@ static void vmsvga_update_display(void *opaque) * Is it more efficient to look at vram VGA-dirty bits or wait * for the driver to issue SVGA_CMD_UPDATE? */ - if (memory_region_is_logging(&s->vga.vram)) { + if (memory_region_is_logging(&s->vga.vram, DIRTY_MEMORY_VGA)) { vga_sync_dirty_bitmap(&s->vga); dirty = memory_region_get_dirty(&s->vga.vram, 0, surface_stride(surface) * surface_height(surface), diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c index 5c7b8c20fa..e37873384f 100644 --- a/hw/virtio/dataplane/vring.c +++ b/hw/virtio/dataplane/vring.c @@ -42,7 +42,7 @@ static void *vring_map(MemoryRegion **mr, hwaddr phys, hwaddr len, } /* Ignore regions with dirty logging, we cannot mark them dirty */ - if (memory_region_is_logging(section.mr)) { + if (memory_region_get_dirty_log_mask(section.mr)) { goto out; } diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 01f1e0490f..53d23d281d 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -416,7 +416,8 @@ static void vhost_set_memory(MemoryListener *listener, memory_listener); hwaddr start_addr = section->offset_within_address_space; ram_addr_t size = int128_get64(section->size); - bool log_dirty = memory_region_is_logging(section->mr); + bool log_dirty = + memory_region_get_dirty_log_mask(section->mr) & ~(1 << DIRTY_MEMORY_MIGRATION); int s = offsetof(struct vhost_memory, regions) + (dev->mem->nregions + 1) * sizeof dev->mem->regions[0]; void *ram; diff --git a/include/exec/memory.h b/include/exec/memory.h index 7b0929d54f..156d506063 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -591,11 +591,23 @@ const char *memory_region_name(const MemoryRegion *mr); /** * memory_region_is_logging: return whether a memory region is logging writes * - * Returns %true if the memory region is logging writes + * Returns %true if the memory region is logging writes for the given client * * @mr: the memory region being queried + * @client: the client being queried */ -bool memory_region_is_logging(MemoryRegion *mr); +bool memory_region_is_logging(MemoryRegion *mr, uint8_t client); + +/** + * memory_region_get_dirty_log_mask: return the clients for which a + * memory region is logging writes. + * + * Returns a bitmap of clients, which right now will be either 0 or + * (1 << DIRTY_MEMORY_VGA). + * + * @mr: the memory region being queried + */ +uint8_t memory_region_get_dirty_log_mask(MemoryRegion *mr); /** * memory_region_is_rom: check whether a memory region is ROM diff --git a/kvm-all.c b/kvm-all.c index 44a34a52d8..d5416bbd74 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -663,7 +663,8 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add) KVMSlot *mem, old; int err; MemoryRegion *mr = section->mr; - bool log_dirty = memory_region_is_logging(mr); + bool log_dirty = + memory_region_get_dirty_log_mask(mr) & ~(1 << DIRTY_MEMORY_MIGRATION); bool writeable = !mr->readonly && !mr->rom_device; bool readonly_flag = mr->readonly || memory_region_is_romd(mr); hwaddr start_addr = section->offset_within_address_space; diff --git a/memory.c b/memory.c index a8f8599079..72e33e853b 100644 --- a/memory.c +++ b/memory.c @@ -1389,11 +1389,16 @@ bool memory_region_is_skip_dump(MemoryRegion *mr) return mr->skip_dump; } -bool memory_region_is_logging(MemoryRegion *mr) +uint8_t memory_region_get_dirty_log_mask(MemoryRegion *mr) { return mr->dirty_log_mask; } +bool memory_region_is_logging(MemoryRegion *mr, uint8_t client) +{ + return memory_region_get_dirty_log_mask(mr) & (1 << client); +} + bool memory_region_is_rom(MemoryRegion *mr) { return mr->ram && mr->readonly; diff --git a/xen-hvm.c b/xen-hvm.c index 315864ca70..338ab291f8 100644 --- a/xen-hvm.c +++ b/xen-hvm.c @@ -488,7 +488,7 @@ static void xen_set_memory(struct MemoryListener *listener, XenIOState *state = container_of(listener, XenIOState, memory_listener); hwaddr start_addr = section->offset_within_address_space; ram_addr_t size = int128_get64(section->size); - bool log_dirty = memory_region_is_logging(section->mr); + bool log_dirty = memory_region_is_logging(section->mr, DIRTY_MEMORY_VGA); hvmmem_type_t mem_type; if (section->mr == &ram_memory) { -- cgit v1.2.3-55-g7522 From b2dfd71c4843a762f2befe702adb249cf55baf66 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Sat, 25 Apr 2015 14:38:30 +0200 Subject: memory: prepare for multiple bits in the dirty log mask When the dirty log mask will also cover other bits than DIRTY_MEMORY_VGA, some listeners may be interested in the overall zero/non-zero value of the dirty log mask; others may be interested in the value of single bits. For this reason, always call log_start/log_stop if bits have respectively appeared or disappeared, and pass the old and new values of the dirty log mask so that listeners can distinguish the kinds of change. For example, KVM checks if dirty logging used to be completely disabled (in log_start) or is now completely disabled (in log_stop). On the other hand, Xen has to check manually if DIRTY_MEMORY_VGA changed, since that is the only bit it cares about. Reviewed-by: Fam Zheng Signed-off-by: Paolo Bonzini --- hw/virtio/vhost.c | 6 ++++-- include/exec/memory.h | 6 ++++-- kvm-all.c | 14 ++++++++++++-- memory.c | 17 +++++++++++------ xen-hvm.c | 20 +++++++++++++------- 5 files changed, 44 insertions(+), 19 deletions(-) (limited to 'memory.c') diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 53d23d281d..a7fe3c5104 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -676,13 +676,15 @@ static void vhost_log_global_stop(MemoryListener *listener) } static void vhost_log_start(MemoryListener *listener, - MemoryRegionSection *section) + MemoryRegionSection *section, + int old, int new) { /* FIXME: implement */ } static void vhost_log_stop(MemoryListener *listener, - MemoryRegionSection *section) + MemoryRegionSection *section, + int old, int new) { /* FIXME: implement */ } diff --git a/include/exec/memory.h b/include/exec/memory.h index 156d506063..55dc11d55c 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -206,8 +206,10 @@ struct MemoryListener { void (*region_add)(MemoryListener *listener, MemoryRegionSection *section); void (*region_del)(MemoryListener *listener, MemoryRegionSection *section); void (*region_nop)(MemoryListener *listener, MemoryRegionSection *section); - void (*log_start)(MemoryListener *listener, MemoryRegionSection *section); - void (*log_stop)(MemoryListener *listener, MemoryRegionSection *section); + void (*log_start)(MemoryListener *listener, MemoryRegionSection *section, + int old, int new); + void (*log_stop)(MemoryListener *listener, MemoryRegionSection *section, + int old, int new); void (*log_sync)(MemoryListener *listener, MemoryRegionSection *section); void (*log_global_start)(MemoryListener *listener); void (*log_global_stop)(MemoryListener *listener); diff --git a/kvm-all.c b/kvm-all.c index d5416bbd74..c713b22f8c 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -344,10 +344,15 @@ static int kvm_dirty_pages_log_change(hwaddr phys_addr, } static void kvm_log_start(MemoryListener *listener, - MemoryRegionSection *section) + MemoryRegionSection *section, + int old, int new) { int r; + if (old != 0) { + return; + } + r = kvm_dirty_pages_log_change(section->offset_within_address_space, int128_get64(section->size), true); if (r < 0) { @@ -356,10 +361,15 @@ static void kvm_log_start(MemoryListener *listener, } static void kvm_log_stop(MemoryListener *listener, - MemoryRegionSection *section) + MemoryRegionSection *section, + int old, int new) { int r; + if (new != 0) { + return; + } + r = kvm_dirty_pages_log_change(section->offset_within_address_space, int128_get64(section->size), false); if (r < 0) { diff --git a/memory.c b/memory.c index 72e33e853b..3e3d5b8ad6 100644 --- a/memory.c +++ b/memory.c @@ -152,7 +152,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) \ +#define MEMORY_LISTENER_UPDATE_REGION(fr, as, dir, callback, _args...) \ MEMORY_LISTENER_CALL(callback, dir, (&(MemoryRegionSection) { \ .mr = (fr)->mr, \ .address_space = (as), \ @@ -160,7 +160,7 @@ static bool memory_listener_match(MemoryListener *listener, .size = (fr)->addr.size, \ .offset_within_address_space = int128_get64((fr)->addr.start), \ .readonly = (fr)->readonly, \ - })) + }), ##_args) struct CoalescedMemoryRange { AddrRange addr; @@ -774,10 +774,15 @@ static void address_space_update_topology_pass(AddressSpace *as, if (adding) { MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, region_nop); - if (frold->dirty_log_mask && !frnew->dirty_log_mask) { - MEMORY_LISTENER_UPDATE_REGION(frnew, as, Reverse, log_stop); - } else if (frnew->dirty_log_mask && !frold->dirty_log_mask) { - MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, log_start); + if (frnew->dirty_log_mask & ~frold->dirty_log_mask) { + MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, log_start, + frold->dirty_log_mask, + frnew->dirty_log_mask); + } + if (frold->dirty_log_mask & ~frnew->dirty_log_mask) { + MEMORY_LISTENER_UPDATE_REGION(frnew, as, Reverse, log_stop, + frold->dirty_log_mask, + frnew->dirty_log_mask); } } diff --git a/xen-hvm.c b/xen-hvm.c index 338ab291f8..42356b836a 100644 --- a/xen-hvm.c +++ b/xen-hvm.c @@ -646,21 +646,27 @@ static void xen_sync_dirty_bitmap(XenIOState *state, } static void xen_log_start(MemoryListener *listener, - MemoryRegionSection *section) + MemoryRegionSection *section, + int old, int new) { XenIOState *state = container_of(listener, XenIOState, memory_listener); - xen_sync_dirty_bitmap(state, section->offset_within_address_space, - int128_get64(section->size)); + if (new & ~old & (1 << DIRTY_MEMORY_VGA)) { + xen_sync_dirty_bitmap(state, section->offset_within_address_space, + int128_get64(section->size)); + } } -static void xen_log_stop(MemoryListener *listener, MemoryRegionSection *section) +static void xen_log_stop(MemoryListener *listener, MemoryRegionSection *section, + int old, int new) { XenIOState *state = container_of(listener, XenIOState, memory_listener); - state->log_for_dirtybit = NULL; - /* Disable dirty bit tracking */ - xc_hvm_track_dirty_vram(xen_xc, xen_domid, 0, 0, NULL); + if (old & ~new & (1 << DIRTY_MEMORY_VGA)) { + state->log_for_dirtybit = NULL; + /* Disable dirty bit tracking */ + xc_hvm_track_dirty_vram(xen_xc, xen_domid, 0, 0, NULL); + } } static void xen_log_sync(MemoryListener *listener, MemoryRegionSection *section) -- cgit v1.2.3-55-g7522 From 677e7805cf95f3b2bca8baf0888d1ebed7f0c606 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 23 Mar 2015 10:53:21 +0100 Subject: memory: track DIRTY_MEMORY_CODE in mr->dirty_log_mask DIRTY_MEMORY_CODE is only needed for TCG. By adding it directly to mr->dirty_log_mask, we avoid testing for TCG everywhere a region is checked for the enabled/disabled state of dirty logging. Reviewed-by: Fam Zheng Signed-off-by: Paolo Bonzini --- include/exec/memory.h | 4 ++-- memory.c | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) (limited to 'memory.c') diff --git a/include/exec/memory.h b/include/exec/memory.h index 55dc11d55c..8ae004eb06 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -604,8 +604,8 @@ bool memory_region_is_logging(MemoryRegion *mr, uint8_t client); * memory_region_get_dirty_log_mask: return the clients for which a * memory region is logging writes. * - * Returns a bitmap of clients, which right now will be either 0 or - * (1 << DIRTY_MEMORY_VGA). + * Returns a bitmap of clients, in which the DIRTY_MEMORY_* constants + * are the bit indices. * * @mr: the memory region being queried */ diff --git a/memory.c b/memory.c index 3e3d5b8ad6..47cc181f66 100644 --- a/memory.c +++ b/memory.c @@ -1212,6 +1212,7 @@ void memory_region_init_ram(MemoryRegion *mr, mr->terminates = true; mr->destructor = memory_region_destructor_ram; mr->ram_addr = qemu_ram_alloc(size, mr, errp); + mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0; } void memory_region_init_resizeable_ram(MemoryRegion *mr, @@ -1229,6 +1230,7 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr, mr->terminates = true; mr->destructor = memory_region_destructor_ram; mr->ram_addr = qemu_ram_alloc_resizeable(size, max_size, resized, mr, errp); + mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0; } #ifdef __linux__ @@ -1245,6 +1247,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr, mr->terminates = true; mr->destructor = memory_region_destructor_ram; mr->ram_addr = qemu_ram_alloc_from_file(size, mr, share, path, errp); + mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0; } #endif @@ -1258,6 +1261,7 @@ void memory_region_init_ram_ptr(MemoryRegion *mr, mr->ram = true; mr->terminates = true; mr->destructor = memory_region_destructor_ram_from_ptr; + mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0; /* qemu_ram_alloc_from_ptr cannot fail with ptr != NULL. */ assert(ptr != NULL); -- cgit v1.2.3-55-g7522 From 6f6a5ef3e429f92f987678ea8c396aab4dc6aa19 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 23 Mar 2015 10:57:21 +0100 Subject: memory: include DIRTY_MEMORY_MIGRATION in the dirty log mask The separate handling of DIRTY_MEMORY_MIGRATION, which does not call log_start/log_stop callbacks when it changes in a region's dirty logging mask, has caused several bugs. One recent example is commit 4cc856f (kvm-all: Sync dirty-bitmap from kvm before kvm destroy the corresponding dirty_bitmap, 2015-04-02). Another performance problem is that KVM keeps tracking dirty pages after a failed live migration, which causes bad performance due to disallowing huge page mapping. This patch removes the root cause of the problem by reporting DIRTY_MEMORY_MIGRATION changes via log_start and log_stop. Note that we now have to rebuild the FlatView when global dirty logging is enabled or disabled; this ensures that log_start and log_stop callbacks are invoked. This will also be used to make the setting of bitmaps conditional. In general, this patch lets users of the memory API ignore the global state of dirty logging if they handle dirty logging generically per region. Reviewed-by: Fam Zheng Signed-off-by: Paolo Bonzini --- memory.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) (limited to 'memory.c') diff --git a/memory.c b/memory.c index 47cc181f66..b7ca987c07 100644 --- a/memory.c +++ b/memory.c @@ -588,7 +588,7 @@ static void render_memory_region(FlatView *view, remain = clip.size; fr.mr = mr; - fr.dirty_log_mask = mr->dirty_log_mask; + fr.dirty_log_mask = memory_region_get_dirty_log_mask(mr); fr.romd_mode = mr->romd_mode; fr.readonly = readonly; @@ -1400,7 +1400,11 @@ bool memory_region_is_skip_dump(MemoryRegion *mr) uint8_t memory_region_get_dirty_log_mask(MemoryRegion *mr) { - return mr->dirty_log_mask; + uint8_t mask = mr->dirty_log_mask; + if (global_dirty_log) { + mask |= (1 << DIRTY_MEMORY_MIGRATION); + } + return mask; } bool memory_region_is_logging(MemoryRegion *mr, uint8_t client) @@ -1962,12 +1966,24 @@ void address_space_sync_dirty_bitmap(AddressSpace *as) void memory_global_dirty_log_start(void) { global_dirty_log = true; + MEMORY_LISTENER_CALL_GLOBAL(log_global_start, Forward); + + /* Refresh DIRTY_LOG_MIGRATION bit. */ + memory_region_transaction_begin(); + memory_region_update_pending = true; + memory_region_transaction_commit(); } void memory_global_dirty_log_stop(void) { global_dirty_log = false; + + /* Refresh DIRTY_LOG_MIGRATION bit. */ + memory_region_transaction_begin(); + memory_region_update_pending = true; + memory_region_transaction_commit(); + MEMORY_LISTENER_CALL_GLOBAL(log_global_stop, Reverse); } -- cgit v1.2.3-55-g7522 From 58d2707e8713ef17b89b8b4c9ce586c76655a385 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 23 Mar 2015 11:56:01 +0100 Subject: exec: pass client mask to cpu_physical_memory_set_dirty_range This cuts in half the cost of bitmap operations (which will become more expensive when made atomic) during migration on non-VRAM regions. Reviewed-by: Fam Zheng Signed-off-by: Paolo Bonzini --- exec.c | 20 +++++++++++--------- include/exec/ram_addr.h | 33 ++++++++++++++++----------------- memory.c | 3 ++- 3 files changed, 29 insertions(+), 27 deletions(-) (limited to 'memory.c') diff --git a/exec.c b/exec.c index 650cfa8cb8..fe137bd725 100644 --- a/exec.c +++ b/exec.c @@ -1351,7 +1351,8 @@ int qemu_ram_resize(ram_addr_t base, ram_addr_t newsize, Error **errp) cpu_physical_memory_clear_dirty_range(block->offset, block->used_length); block->used_length = newsize; - cpu_physical_memory_set_dirty_range(block->offset, block->used_length); + cpu_physical_memory_set_dirty_range(block->offset, block->used_length, + DIRTY_CLIENTS_ALL); memory_region_set_size(block->mr, newsize); if (block->resized) { block->resized(block->idstr, newsize, block->host); @@ -1425,7 +1426,8 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp) } } cpu_physical_memory_set_dirty_range(new_block->offset, - new_block->used_length); + new_block->used_length, + DIRTY_CLIENTS_ALL); if (new_block->host) { qemu_ram_setup_dump(new_block->host, new_block->max_length); @@ -1813,7 +1815,11 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr, default: abort(); } - cpu_physical_memory_set_dirty_range_nocode(ram_addr, size); + /* Set both VGA and migration bits for simplicity and to remove + * the notdirty callback faster. + */ + cpu_physical_memory_set_dirty_range(ram_addr, size, + DIRTY_CLIENTS_NOCODE); /* we remove the notdirty callback only if the code has been flushed */ if (!cpu_physical_memory_is_clean(ram_addr)) { @@ -2259,9 +2265,7 @@ static void invalidate_and_set_dirty(MemoryRegion *mr, hwaddr addr, tb_invalidate_phys_range(addr, addr + length); dirty_log_mask &= ~(1 << DIRTY_MEMORY_CODE); } - if (dirty_log_mask) { - cpu_physical_memory_set_dirty_range_nocode(addr, length); - } + cpu_physical_memory_set_dirty_range(addr, length, dirty_log_mask); } else { xen_modified_memory(addr, length); } @@ -3014,9 +3018,7 @@ void address_space_stl_notdirty(AddressSpace *as, hwaddr addr, uint32_t val, dirty_log_mask = memory_region_get_dirty_log_mask(mr); dirty_log_mask &= ~(1 << DIRTY_MEMORY_CODE); - if (dirty_log_mask) { - cpu_physical_memory_set_dirty_range_nocode(addr1, 4); - } + cpu_physical_memory_set_dirty_range(addr1, 4, dirty_log_mask); r = MEMTX_OK; } if (result) { diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h index 7f6e928a7f..5bbc7bb41d 100644 --- a/include/exec/ram_addr.h +++ b/include/exec/ram_addr.h @@ -41,6 +41,9 @@ void qemu_ram_free_from_ptr(ram_addr_t addr); int qemu_ram_resize(ram_addr_t base, ram_addr_t newsize, Error **errp); +#define DIRTY_CLIENTS_ALL ((1 << DIRTY_MEMORY_NUM) - 1) +#define DIRTY_CLIENTS_NOCODE (DIRTY_CLIENTS_ALL & ~(1 << DIRTY_MEMORY_CODE)) + static inline bool cpu_physical_memory_get_dirty(ram_addr_t start, ram_addr_t length, unsigned client) @@ -103,28 +106,23 @@ static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr, set_bit(addr >> TARGET_PAGE_BITS, ram_list.dirty_memory[client]); } -static inline void cpu_physical_memory_set_dirty_range_nocode(ram_addr_t start, - ram_addr_t length) -{ - unsigned long end, page; - - end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS; - page = start >> TARGET_PAGE_BITS; - bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION], page, end - page); - bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_VGA], page, end - page); - xen_modified_memory(start, length); -} - static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start, - ram_addr_t length) + ram_addr_t length, + uint8_t mask) { unsigned long end, page; end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS; page = start >> TARGET_PAGE_BITS; - bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION], page, end - page); - bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_VGA], page, end - page); - bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_CODE], page, end - page); + if (likely(mask & (1 << DIRTY_MEMORY_MIGRATION))) { + bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION], page, end - page); + } + if (unlikely(mask & (1 << DIRTY_MEMORY_VGA))) { + bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_VGA], page, end - page); + } + if (unlikely(mask & (1 << DIRTY_MEMORY_CODE))) { + bitmap_set(ram_list.dirty_memory[DIRTY_MEMORY_CODE], page, end - page); + } xen_modified_memory(start, length); } @@ -172,7 +170,8 @@ static inline void cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap, addr = page_number * TARGET_PAGE_SIZE; ram_addr = start + addr; cpu_physical_memory_set_dirty_range(ram_addr, - TARGET_PAGE_SIZE * hpratio); + TARGET_PAGE_SIZE * hpratio, + DIRTY_CLIENTS_ALL); } while (c != 0); } } diff --git a/memory.c b/memory.c index b7ca987c07..418cac7ce1 100644 --- a/memory.c +++ b/memory.c @@ -1461,7 +1461,8 @@ void memory_region_set_dirty(MemoryRegion *mr, hwaddr addr, hwaddr size) { assert(mr->terminates); - cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size); + cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size, + memory_region_get_dirty_log_mask(mr)); } bool memory_region_test_and_clear_dirty(MemoryRegion *mr, hwaddr addr, -- cgit v1.2.3-55-g7522 From 03eebc9e3246b9b3f5925aa41f7dfd7c1e467875 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 2 Dec 2014 11:23:18 +0000 Subject: memory: replace cpu_physical_memory_reset_dirty() with test-and-clear The cpu_physical_memory_reset_dirty() function is sometimes used together with cpu_physical_memory_get_dirty(). This is not atomic since two separate accesses to the dirty memory bitmap are made. Turn cpu_physical_memory_reset_dirty() and cpu_physical_memory_clear_dirty_range_type() into the atomic cpu_physical_memory_test_and_clear_dirty(). Signed-off-by: Stefan Hajnoczi Message-Id: <1417519399-3166-6-git-send-email-stefanha@redhat.com> Reviewed-by: Fam Zheng Signed-off-by: Paolo Bonzini --- cputlb.c | 4 ++-- exec.c | 23 +++++++++++++++++------ include/exec/ram_addr.h | 33 ++++++++++----------------------- memory.c | 11 ++++------- 4 files changed, 33 insertions(+), 38 deletions(-) (limited to 'memory.c') diff --git a/cputlb.c b/cputlb.c index dd1203b938..a50608676c 100644 --- a/cputlb.c +++ b/cputlb.c @@ -125,8 +125,8 @@ void tlb_flush_page(CPUState *cpu, target_ulong addr) can be detected */ void tlb_protect_code(ram_addr_t ram_addr) { - cpu_physical_memory_reset_dirty(ram_addr, TARGET_PAGE_SIZE, - DIRTY_MEMORY_CODE); + cpu_physical_memory_test_and_clear_dirty(ram_addr, TARGET_PAGE_SIZE, + DIRTY_MEMORY_CODE); } /* update the TLB so that writes in physical page 'phys_addr' are no longer diff --git a/exec.c b/exec.c index 162c57908e..487583b1bd 100644 --- a/exec.c +++ b/exec.c @@ -852,16 +852,27 @@ static void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t length) } /* Note: start and end must be within the same ram block. */ -void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length, - unsigned client) +bool cpu_physical_memory_test_and_clear_dirty(ram_addr_t start, + ram_addr_t length, + unsigned client) { - if (length == 0) - return; - cpu_physical_memory_clear_dirty_range_type(start, length, client); + unsigned long end, page; + bool dirty; - if (tcg_enabled()) { + if (length == 0) { + return false; + } + + end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS; + page = start >> TARGET_PAGE_BITS; + dirty = bitmap_test_and_clear_atomic(ram_list.dirty_memory[client], + page, end - page); + + if (dirty && tcg_enabled()) { tlb_reset_dirty_range_all(start, length); } + + return dirty; } /* Called from RCU critical section */ diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h index 63db371850..a2beea7925 100644 --- a/include/exec/ram_addr.h +++ b/include/exec/ram_addr.h @@ -194,30 +194,19 @@ static inline void cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap, } #endif /* not _WIN32 */ -static inline void cpu_physical_memory_clear_dirty_range_type(ram_addr_t start, - ram_addr_t length, - unsigned client) -{ - unsigned long end, page; - - assert(client < DIRTY_MEMORY_NUM); - end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS; - page = start >> TARGET_PAGE_BITS; - bitmap_clear(ram_list.dirty_memory[client], page, end - page); -} +bool cpu_physical_memory_test_and_clear_dirty(ram_addr_t start, + ram_addr_t length, + unsigned client); static inline void cpu_physical_memory_clear_dirty_range(ram_addr_t start, ram_addr_t length) { - cpu_physical_memory_clear_dirty_range_type(start, length, DIRTY_MEMORY_MIGRATION); - cpu_physical_memory_clear_dirty_range_type(start, length, DIRTY_MEMORY_VGA); - cpu_physical_memory_clear_dirty_range_type(start, length, DIRTY_MEMORY_CODE); + cpu_physical_memory_test_and_clear_dirty(start, length, DIRTY_MEMORY_MIGRATION); + cpu_physical_memory_test_and_clear_dirty(start, length, DIRTY_MEMORY_VGA); + cpu_physical_memory_test_and_clear_dirty(start, length, DIRTY_MEMORY_CODE); } -void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length, - unsigned client); - static inline uint64_t cpu_physical_memory_sync_dirty_bitmap(unsigned long *dest, ram_addr_t start, @@ -245,16 +234,14 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(unsigned long *dest, } } else { for (addr = 0; addr < length; addr += TARGET_PAGE_SIZE) { - if (cpu_physical_memory_get_dirty(start + addr, - TARGET_PAGE_SIZE, - DIRTY_MEMORY_MIGRATION)) { + if (cpu_physical_memory_test_and_clear_dirty( + start + addr, + TARGET_PAGE_SIZE, + DIRTY_MEMORY_MIGRATION)) { long k = (start + addr) >> TARGET_PAGE_BITS; if (!test_and_set_bit(k, dest)) { num_dirty++; } - cpu_physical_memory_reset_dirty(start + addr, - TARGET_PAGE_SIZE, - DIRTY_MEMORY_MIGRATION); } } } diff --git a/memory.c b/memory.c index 418cac7ce1..0c5d32807e 100644 --- a/memory.c +++ b/memory.c @@ -1468,13 +1468,9 @@ void memory_region_set_dirty(MemoryRegion *mr, hwaddr addr, bool memory_region_test_and_clear_dirty(MemoryRegion *mr, hwaddr addr, hwaddr size, unsigned client) { - bool ret; assert(mr->terminates); - ret = cpu_physical_memory_get_dirty(mr->ram_addr + addr, size, client); - if (ret) { - cpu_physical_memory_reset_dirty(mr->ram_addr + addr, size, client); - } - return ret; + return cpu_physical_memory_test_and_clear_dirty(mr->ram_addr + addr, + size, client); } @@ -1518,7 +1514,8 @@ void memory_region_reset_dirty(MemoryRegion *mr, hwaddr addr, hwaddr size, unsigned client) { assert(mr->terminates); - cpu_physical_memory_reset_dirty(mr->ram_addr + addr, size, client); + cpu_physical_memory_test_and_clear_dirty(mr->ram_addr + addr, size, + client); } int memory_region_get_fd(MemoryRegion *mr) -- cgit v1.2.3-55-g7522 From ec05ec26f940564b1e07bf88857035ec27e21dd8 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Sun, 29 Mar 2015 09:31:43 +0200 Subject: memory: use mr->ram_addr in "is this RAM?" assertions mr->terminates alone doesn't guarantee that we are looking at a RAM region. mr->ram_addr also has to be checked, in order to distinguish RAM and I/O regions. So, do the following: 1) add a new define RAM_ADDR_INVALID, and test it in the assertions instead of mr->terminates 2) IOMMU regions were not setting mr->ram_addr to a bogus value, initialize it in the instance_init function so that the new assertions would fire for IOMMU regions as well. Reviewed-by: Fam Zheng Signed-off-by: Paolo Bonzini --- memory.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'memory.c') diff --git a/memory.c b/memory.c index 0c5d32807e..3ac0bd20d2 100644 --- a/memory.c +++ b/memory.c @@ -28,6 +28,8 @@ //#define DEBUG_UNASSIGNED +#define RAM_ADDR_INVALID (~(ram_addr_t)0) + static unsigned memory_region_transaction_depth; static bool memory_region_update_pending; static bool ioeventfd_update_pending; @@ -1007,6 +1009,7 @@ static void memory_region_initfn(Object *obj) ObjectProperty *op; mr->ops = &unassigned_mem_ops; + mr->ram_addr = RAM_ADDR_INVALID; mr->enabled = true; mr->romd_mode = true; mr->destructor = memory_region_destructor_none; @@ -1198,7 +1201,6 @@ void memory_region_init_io(MemoryRegion *mr, mr->ops = ops; mr->opaque = opaque; mr->terminates = true; - mr->ram_addr = ~(ram_addr_t)0; } void memory_region_init_ram(MemoryRegion *mr, @@ -1453,14 +1455,14 @@ void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client) bool memory_region_get_dirty(MemoryRegion *mr, hwaddr addr, hwaddr size, unsigned client) { - assert(mr->terminates); + assert(mr->ram_addr != RAM_ADDR_INVALID); return cpu_physical_memory_get_dirty(mr->ram_addr + addr, size, client); } void memory_region_set_dirty(MemoryRegion *mr, hwaddr addr, hwaddr size) { - assert(mr->terminates); + assert(mr->ram_addr != RAM_ADDR_INVALID); cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size, memory_region_get_dirty_log_mask(mr)); } @@ -1468,7 +1470,7 @@ void memory_region_set_dirty(MemoryRegion *mr, hwaddr addr, bool memory_region_test_and_clear_dirty(MemoryRegion *mr, hwaddr addr, hwaddr size, unsigned client) { - assert(mr->terminates); + assert(mr->ram_addr != RAM_ADDR_INVALID); return cpu_physical_memory_test_and_clear_dirty(mr->ram_addr + addr, size, client); } @@ -1513,7 +1515,7 @@ void memory_region_rom_device_set_romd(MemoryRegion *mr, bool romd_mode) void memory_region_reset_dirty(MemoryRegion *mr, hwaddr addr, hwaddr size, unsigned client) { - assert(mr->terminates); + assert(mr->ram_addr != RAM_ADDR_INVALID); cpu_physical_memory_test_and_clear_dirty(mr->ram_addr + addr, size, client); } @@ -1524,7 +1526,7 @@ int memory_region_get_fd(MemoryRegion *mr) return memory_region_get_fd(mr->alias); } - assert(mr->terminates); + assert(mr->ram_addr != RAM_ADDR_INVALID); return qemu_get_ram_fd(mr->ram_addr & TARGET_PAGE_MASK); } @@ -1535,14 +1537,14 @@ void *memory_region_get_ram_ptr(MemoryRegion *mr) return memory_region_get_ram_ptr(mr->alias) + mr->alias_offset; } - assert(mr->terminates); + assert(mr->ram_addr != RAM_ADDR_INVALID); return qemu_get_ram_ptr(mr->ram_addr & TARGET_PAGE_MASK); } void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize, Error **errp) { - assert(mr->terminates); + assert(mr->ram_addr != RAM_ADDR_INVALID); qemu_ram_resize(mr->ram_addr, newsize, errp); } -- cgit v1.2.3-55-g7522