From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52894) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1amiPY-0005aJ-VY for qemu-devel@nongnu.org; Sun, 03 Apr 2016 09:49:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1amiPX-0004gy-CV for qemu-devel@nongnu.org; Sun, 03 Apr 2016 09:49:52 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38240) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1amiPX-0004gt-4g for qemu-devel@nongnu.org; Sun, 03 Apr 2016 09:49:51 -0400 Date: Sun, 3 Apr 2016 16:49:47 +0300 From: "Michael S. Tsirkin" Message-ID: <20160403164839-mutt-send-email-mst@redhat.com> References: <1458817415-29247-1-git-send-email-pbonzini@redhat.com> <1458817415-29247-3-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1458817415-29247-3-git-send-email-pbonzini@redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: arei.gonglei@huawei.com, famz@redhat.com, qemu-devel@nongnu.org On Thu, Mar 24, 2016 at 12:03:35PM +0100, Paolo Bonzini wrote: > Let users of qemu_get_ram_ptr and qemu_ram_ptr_length pass in an > address that is relative to the MemoryRegion. This basically means > what address_space_translate returns. > > invalidate_and_set_dirty has to add back mr->ram_addr, but reads do > not need it at all. > > Signed-off-by: Paolo Bonzini I agree but I think we need a better name for this function. qemu_ram_offset_to_ptr? Will also serve to make sure backporting patches across this API change does not cause issues. > --- > exec.c | 40 +++++++++++++++------------------------- > include/exec/memory.h | 1 - > memory.c | 4 ++-- > scripts/dump-guest-memory.py | 19 +++---------------- > 4 files changed, 20 insertions(+), 44 deletions(-) > > diff --git a/exec.c b/exec.c > index 001b669..ca9e3b6 100644 > --- a/exec.c > +++ b/exec.c > @@ -1876,6 +1876,7 @@ void *qemu_get_ram_ptr(RAMBlock *ram_block, ram_addr_t addr) > > if (block == NULL) { > block = qemu_get_ram_block(addr); > + addr -= block->offset; > } > > if (xen_enabled() && block->host == NULL) { > @@ -1889,7 +1890,7 @@ void *qemu_get_ram_ptr(RAMBlock *ram_block, ram_addr_t addr) > > block->host = xen_map_cache(block->offset, block->max_length, 1); > } > - return ramblock_ptr(block, addr - block->offset); > + return ramblock_ptr(block, addr); > } > > /* Return a host pointer to guest's ram. Similar to qemu_get_ram_ptr > @@ -1901,16 +1902,15 @@ static void *qemu_ram_ptr_length(RAMBlock *ram_block, ram_addr_t addr, > hwaddr *size) > { > RAMBlock *block = ram_block; > - ram_addr_t offset_inside_block; > if (*size == 0) { > return NULL; > } > > if (block == NULL) { > block = qemu_get_ram_block(addr); > + addr -= block->offset; > } > - offset_inside_block = addr - block->offset; > - *size = MIN(*size, block->max_length - offset_inside_block); > + *size = MIN(*size, block->max_length - addr); > > if (xen_enabled() && block->host == NULL) { > /* We need to check if the requested address is in the RAM > @@ -1924,7 +1924,7 @@ static void *qemu_ram_ptr_length(RAMBlock *ram_block, ram_addr_t addr, > block->host = xen_map_cache(block->offset, block->max_length, 1); > } > > - return ramblock_ptr(block, offset_inside_block); > + return ramblock_ptr(block, addr); > } > > /* > @@ -2504,6 +2504,8 @@ static void invalidate_and_set_dirty(MemoryRegion *mr, hwaddr addr, > hwaddr length) > { > uint8_t dirty_log_mask = memory_region_get_dirty_log_mask(mr); > + addr += memory_region_get_ram_addr(mr); > + > /* No early return if dirty_log_mask is or becomes 0, because > * cpu_physical_memory_set_dirty_range will still call > * xen_modified_memory. > @@ -2616,7 +2618,6 @@ static MemTxResult address_space_write_continue(AddressSpace *as, hwaddr addr, > abort(); > } > } else { > - addr1 += memory_region_get_ram_addr(mr); > /* RAM case */ > ptr = qemu_get_ram_ptr(mr->ram_block, addr1); > memcpy(ptr, buf, l); > @@ -2709,8 +2710,7 @@ MemTxResult address_space_read_continue(AddressSpace *as, hwaddr addr, > } > } else { > /* RAM case */ > - ptr = qemu_get_ram_ptr(mr->ram_block, > - memory_region_get_ram_addr(mr) + addr1); > + ptr = qemu_get_ram_ptr(mr->ram_block, addr1); > memcpy(buf, ptr, l); > } > > @@ -2793,7 +2793,6 @@ static inline void cpu_physical_memory_write_rom_internal(AddressSpace *as, > memory_region_is_romd(mr))) { > l = memory_access_size(mr, l, addr1); > } else { > - addr1 += memory_region_get_ram_addr(mr); > /* ROM/RAM case */ > ptr = qemu_get_ram_ptr(mr->ram_block, addr1); > switch (type) { > @@ -2953,7 +2952,6 @@ void *address_space_map(AddressSpace *as, > hwaddr done = 0; > hwaddr l, xlat, base; > MemoryRegion *mr, *this_mr; > - ram_addr_t raddr; > void *ptr; > > if (len == 0) { > @@ -2962,7 +2960,7 @@ void *address_space_map(AddressSpace *as, > > l = len; > rcu_read_lock(); > - mr = address_space_translate(as, addr, &xlat, &l, is_write); > + mr = address_space_translate(as, addr, &base, &l, is_write); > > if (!memory_access_is_direct(mr, is_write)) { > if (atomic_xchg(&bounce.in_use, true)) { > @@ -2987,9 +2985,6 @@ void *address_space_map(AddressSpace *as, > return bounce.buffer; > } > > - base = xlat; > - raddr = memory_region_get_ram_addr(mr); > - > for (;;) { > len -= l; > addr += l; > @@ -3007,7 +3002,7 @@ void *address_space_map(AddressSpace *as, > > memory_region_ref(mr); > *plen = done; > - ptr = qemu_ram_ptr_length(mr->ram_block, raddr + base, plen); > + ptr = qemu_ram_ptr_length(mr->ram_block, base, plen); > rcu_read_unlock(); > > return ptr; > @@ -3091,8 +3086,7 @@ static inline uint32_t address_space_ldl_internal(AddressSpace *as, hwaddr addr, > #endif > } else { > /* RAM case */ > - ptr = qemu_get_ram_ptr(mr->ram_block, > - memory_region_get_ram_addr(mr) + addr1); > + ptr = qemu_get_ram_ptr(mr->ram_block, addr1); > switch (endian) { > case DEVICE_LITTLE_ENDIAN: > val = ldl_le_p(ptr); > @@ -3185,8 +3179,7 @@ static inline uint64_t address_space_ldq_internal(AddressSpace *as, hwaddr addr, > #endif > } else { > /* RAM case */ > - ptr = qemu_get_ram_ptr(mr->ram_block, > - memory_region_get_ram_addr(mr) + addr1); > + ptr = qemu_get_ram_ptr(mr->ram_block, addr1); > switch (endian) { > case DEVICE_LITTLE_ENDIAN: > val = ldq_le_p(ptr); > @@ -3299,8 +3292,7 @@ static inline uint32_t address_space_lduw_internal(AddressSpace *as, > #endif > } else { > /* RAM case */ > - ptr = qemu_get_ram_ptr(mr->ram_block, > - memory_region_get_ram_addr(mr) + addr1); > + ptr = qemu_get_ram_ptr(mr->ram_block, addr1); > switch (endian) { > case DEVICE_LITTLE_ENDIAN: > val = lduw_le_p(ptr); > @@ -3382,13 +3374,13 @@ void address_space_stl_notdirty(AddressSpace *as, hwaddr addr, uint32_t val, > > r = memory_region_dispatch_write(mr, addr1, val, 4, attrs); > } else { > - addr1 += memory_region_get_ram_addr(mr); > ptr = qemu_get_ram_ptr(mr->ram_block, addr1); > stl_p(ptr, val); > > dirty_log_mask = memory_region_get_dirty_log_mask(mr); > dirty_log_mask &= ~(1 << DIRTY_MEMORY_CODE); > - cpu_physical_memory_set_dirty_range(addr1, 4, dirty_log_mask); > + cpu_physical_memory_set_dirty_range(memory_region_get_ram_addr(mr) + addr, > + 4, dirty_log_mask); > r = MEMTX_OK; > } > if (result) { > @@ -3437,7 +3429,6 @@ static inline void address_space_stl_internal(AddressSpace *as, > r = memory_region_dispatch_write(mr, addr1, val, 4, attrs); > } else { > /* RAM case */ > - addr1 += memory_region_get_ram_addr(mr); > ptr = qemu_get_ram_ptr(mr->ram_block, addr1); > switch (endian) { > case DEVICE_LITTLE_ENDIAN: > @@ -3547,7 +3538,6 @@ static inline void address_space_stw_internal(AddressSpace *as, > r = memory_region_dispatch_write(mr, addr1, val, 2, attrs); > } else { > /* RAM case */ > - addr1 += memory_region_get_ram_addr(mr); > ptr = qemu_get_ram_ptr(mr->ram_block, addr1); > switch (endian) { > case DEVICE_LITTLE_ENDIAN: > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 2de7898..326c2a2 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -1419,7 +1419,6 @@ MemTxResult address_space_read(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, > l = len; > mr = address_space_translate(as, addr, &addr1, &l, false); > if (len == l && memory_access_is_direct(mr, false)) { > - addr1 += memory_region_get_ram_addr(mr); > ptr = qemu_get_ram_ptr(mr->ram_block, addr1); > memcpy(buf, ptr, len); > } else { > diff --git a/memory.c b/memory.c > index 49c9b14..bc47d0b 100644 > --- a/memory.c > +++ b/memory.c > @@ -1654,10 +1654,10 @@ void *memory_region_get_ram_ptr(MemoryRegion *mr) > mr = mr->alias; > } > assert(mr->ram_block); > - ptr = qemu_get_ram_ptr(mr->ram_block, memory_region_get_ram_addr(mr)); > + ptr = qemu_get_ram_ptr(mr->ram_block, offset); > rcu_read_unlock(); > > - return ptr + offset; > + return ptr; > } > > ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr) > diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py > index c0a2e99..ae21f97 100644 > --- a/scripts/dump-guest-memory.py > +++ b/scripts/dump-guest-memory.py > @@ -328,23 +328,10 @@ def qlist_foreach(head, field_str): > yield var > > > -def qemu_get_ram_block(ram_addr): > - """Returns the RAMBlock struct to which the given address belongs.""" > - > - ram_blocks = gdb.parse_and_eval("ram_list.blocks") > - > - for block in qlist_foreach(ram_blocks, "next"): > - if (ram_addr - block["offset"]) < block["used_length"]: > - return block > - > - raise gdb.GdbError("Bad ram offset %x" % ram_addr) > - > - > -def qemu_get_ram_ptr(ram_addr): > +def qemu_get_ram_ptr(block, offset): > """Returns qemu vaddr for given guest physical address.""" > > - block = qemu_get_ram_block(ram_addr) > - return block["host"] + (ram_addr - block["offset"]) > + return block["host"] + offset > > > def memory_region_get_ram_ptr(memory_region): > @@ -352,7 +339,7 @@ def memory_region_get_ram_ptr(memory_region): > return (memory_region_get_ram_ptr(memory_region["alias"].dereference()) > + memory_region["alias_offset"]) > > - return qemu_get_ram_ptr(memory_region["ram_block"]["offset"]) > + return qemu_get_ram_ptr(memory_region["ram_block"], 0) > > > def get_guest_phys_blocks(): > -- > 1.8.3.1