* [Qemu-devel] [PATCH 0/2] memory: cleanup users of memory_region_get_ram_addr @ 2016-03-24 11:03 Paolo Bonzini 2016-03-24 11:03 ` [Qemu-devel] [PATCH 1/2] memory: remove unnecessary masking of MemoryRegion ram_addr Paolo Bonzini 2016-03-24 11:03 ` [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users Paolo Bonzini 0 siblings, 2 replies; 14+ messages in thread From: Paolo Bonzini @ 2016-03-24 11:03 UTC (permalink / raw) To: qemu-devel; +Cc: arei.gonglei, famz, mst The result of memory_region_get_ram_addr is often added in the caller of qemu_get_ram_ptr and then subtracted in qemu_get_ram_ptr; avoid the indirection. Prompted by mst's remark on confusion between hwaddr and ram_addr_t. Paolo Paolo Bonzini (2): memory: remove unnecessary masking of MemoryRegion ram_addr memory: hide ram_addr_t from qemu_get_ram_ptr users exec.c | 49 +++++++++++++++----------------------------- include/exec/memory.h | 1 - memory.c | 7 +++---- migration/savevm.c | 4 ++-- scripts/dump-guest-memory.py | 19 +++-------------- translate-all.c | 3 +-- 6 files changed, 25 insertions(+), 58 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 1/2] memory: remove unnecessary masking of MemoryRegion ram_addr 2016-03-24 11:03 [Qemu-devel] [PATCH 0/2] memory: cleanup users of memory_region_get_ram_addr Paolo Bonzini @ 2016-03-24 11:03 ` Paolo Bonzini 2016-03-25 6:20 ` Fam Zheng 2016-04-03 13:48 ` Michael S. Tsirkin 2016-03-24 11:03 ` [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users Paolo Bonzini 1 sibling, 2 replies; 14+ messages in thread From: Paolo Bonzini @ 2016-03-24 11:03 UTC (permalink / raw) To: qemu-devel; +Cc: arei.gonglei, famz, mst mr->ram_block->offset is already aligned to both host and target size (see qemu_ram_alloc_internal). Remove further masking as it is unnecessary. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- exec.c | 21 +++++++-------------- memory.c | 5 ++--- migration/savevm.c | 4 ++-- translate-all.c | 3 +-- 4 files changed, 12 insertions(+), 21 deletions(-) diff --git a/exec.c b/exec.c index f398d21..001b669 100644 --- a/exec.c +++ b/exec.c @@ -1042,8 +1042,7 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu, if (memory_region_is_ram(section->mr)) { /* Normal RAM. */ - iotlb = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK) - + xlat; + iotlb = memory_region_get_ram_addr(section->mr) + xlat; if (!section->readonly) { iotlb |= PHYS_SECTION_NOTDIRTY; } else { @@ -3093,9 +3092,7 @@ static inline uint32_t address_space_ldl_internal(AddressSpace *as, hwaddr addr, } else { /* RAM case */ ptr = qemu_get_ram_ptr(mr->ram_block, - (memory_region_get_ram_addr(mr) - & TARGET_PAGE_MASK) - + addr1); + memory_region_get_ram_addr(mr) + addr1); switch (endian) { case DEVICE_LITTLE_ENDIAN: val = ldl_le_p(ptr); @@ -3189,9 +3186,7 @@ static inline uint64_t address_space_ldq_internal(AddressSpace *as, hwaddr addr, } else { /* RAM case */ ptr = qemu_get_ram_ptr(mr->ram_block, - (memory_region_get_ram_addr(mr) - & TARGET_PAGE_MASK) - + addr1); + memory_region_get_ram_addr(mr) + addr1); switch (endian) { case DEVICE_LITTLE_ENDIAN: val = ldq_le_p(ptr); @@ -3305,9 +3300,7 @@ static inline uint32_t address_space_lduw_internal(AddressSpace *as, } else { /* RAM case */ ptr = qemu_get_ram_ptr(mr->ram_block, - (memory_region_get_ram_addr(mr) - & TARGET_PAGE_MASK) - + addr1); + memory_region_get_ram_addr(mr) + addr1); switch (endian) { case DEVICE_LITTLE_ENDIAN: val = lduw_le_p(ptr); @@ -3389,7 +3382,7 @@ 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) & TARGET_PAGE_MASK; + addr1 += memory_region_get_ram_addr(mr); ptr = qemu_get_ram_ptr(mr->ram_block, addr1); stl_p(ptr, val); @@ -3444,7 +3437,7 @@ 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) & TARGET_PAGE_MASK; + addr1 += memory_region_get_ram_addr(mr); ptr = qemu_get_ram_ptr(mr->ram_block, addr1); switch (endian) { case DEVICE_LITTLE_ENDIAN: @@ -3554,7 +3547,7 @@ 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) & TARGET_PAGE_MASK; + 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/memory.c b/memory.c index 95f7209..49c9b14 100644 --- a/memory.c +++ b/memory.c @@ -1640,7 +1640,7 @@ int memory_region_get_fd(MemoryRegion *mr) assert(mr->ram_block); - return qemu_get_ram_fd(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK); + return qemu_get_ram_fd(memory_region_get_ram_addr(mr)); } void *memory_region_get_ram_ptr(MemoryRegion *mr) @@ -1654,8 +1654,7 @@ 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) & TARGET_PAGE_MASK); + ptr = qemu_get_ram_ptr(mr->ram_block, memory_region_get_ram_addr(mr)); rcu_read_unlock(); return ptr + offset; diff --git a/migration/savevm.c b/migration/savevm.c index 0a33c22..cbba062 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -2229,13 +2229,13 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict) void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev) { - qemu_ram_set_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK, + qemu_ram_set_idstr(memory_region_get_ram_addr(mr), memory_region_name(mr), dev); } void vmstate_unregister_ram(MemoryRegion *mr, DeviceState *dev) { - qemu_ram_unset_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK); + qemu_ram_unset_idstr(memory_region_get_ram_addr(mr)); } void vmstate_register_ram_global(MemoryRegion *mr) diff --git a/translate-all.c b/translate-all.c index e9f409b..6820d8e 100644 --- a/translate-all.c +++ b/translate-all.c @@ -1550,8 +1550,7 @@ void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr) rcu_read_unlock(); return; } - ram_addr = (memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK) - + addr; + ram_addr = memory_region_get_ram_addr(mr) + addr; tb_invalidate_phys_page_range(ram_addr, ram_addr + 1, 0); rcu_read_unlock(); } -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] memory: remove unnecessary masking of MemoryRegion ram_addr 2016-03-24 11:03 ` [Qemu-devel] [PATCH 1/2] memory: remove unnecessary masking of MemoryRegion ram_addr Paolo Bonzini @ 2016-03-25 6:20 ` Fam Zheng 2016-04-03 13:48 ` Michael S. Tsirkin 1 sibling, 0 replies; 14+ messages in thread From: Fam Zheng @ 2016-03-25 6:20 UTC (permalink / raw) To: Paolo Bonzini; +Cc: arei.gonglei, qemu-devel, mst On Thu, 03/24 12:03, Paolo Bonzini wrote: > mr->ram_block->offset is already aligned to both host and target size > (see qemu_ram_alloc_internal). Remove further masking as it is > unnecessary. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] memory: remove unnecessary masking of MemoryRegion ram_addr 2016-03-24 11:03 ` [Qemu-devel] [PATCH 1/2] memory: remove unnecessary masking of MemoryRegion ram_addr Paolo Bonzini 2016-03-25 6:20 ` Fam Zheng @ 2016-04-03 13:48 ` Michael S. Tsirkin 1 sibling, 0 replies; 14+ messages in thread From: Michael S. Tsirkin @ 2016-04-03 13:48 UTC (permalink / raw) To: Paolo Bonzini; +Cc: arei.gonglei, famz, qemu-devel On Thu, Mar 24, 2016 at 12:03:34PM +0100, Paolo Bonzini wrote: > mr->ram_block->offset is already aligned to both host and target size > (see qemu_ram_alloc_internal). Remove further masking as it is > unnecessary. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > --- > exec.c | 21 +++++++-------------- > memory.c | 5 ++--- > migration/savevm.c | 4 ++-- > translate-all.c | 3 +-- > 4 files changed, 12 insertions(+), 21 deletions(-) > > diff --git a/exec.c b/exec.c > index f398d21..001b669 100644 > --- a/exec.c > +++ b/exec.c > @@ -1042,8 +1042,7 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu, > > if (memory_region_is_ram(section->mr)) { > /* Normal RAM. */ > - iotlb = (memory_region_get_ram_addr(section->mr) & TARGET_PAGE_MASK) > - + xlat; > + iotlb = memory_region_get_ram_addr(section->mr) + xlat; > if (!section->readonly) { > iotlb |= PHYS_SECTION_NOTDIRTY; > } else { > @@ -3093,9 +3092,7 @@ static inline uint32_t address_space_ldl_internal(AddressSpace *as, hwaddr addr, > } else { > /* RAM case */ > ptr = qemu_get_ram_ptr(mr->ram_block, > - (memory_region_get_ram_addr(mr) > - & TARGET_PAGE_MASK) > - + addr1); > + memory_region_get_ram_addr(mr) + addr1); > switch (endian) { > case DEVICE_LITTLE_ENDIAN: > val = ldl_le_p(ptr); > @@ -3189,9 +3186,7 @@ static inline uint64_t address_space_ldq_internal(AddressSpace *as, hwaddr addr, > } else { > /* RAM case */ > ptr = qemu_get_ram_ptr(mr->ram_block, > - (memory_region_get_ram_addr(mr) > - & TARGET_PAGE_MASK) > - + addr1); > + memory_region_get_ram_addr(mr) + addr1); > switch (endian) { > case DEVICE_LITTLE_ENDIAN: > val = ldq_le_p(ptr); > @@ -3305,9 +3300,7 @@ static inline uint32_t address_space_lduw_internal(AddressSpace *as, > } else { > /* RAM case */ > ptr = qemu_get_ram_ptr(mr->ram_block, > - (memory_region_get_ram_addr(mr) > - & TARGET_PAGE_MASK) > - + addr1); > + memory_region_get_ram_addr(mr) + addr1); > switch (endian) { > case DEVICE_LITTLE_ENDIAN: > val = lduw_le_p(ptr); > @@ -3389,7 +3382,7 @@ 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) & TARGET_PAGE_MASK; > + addr1 += memory_region_get_ram_addr(mr); > ptr = qemu_get_ram_ptr(mr->ram_block, addr1); > stl_p(ptr, val); > > @@ -3444,7 +3437,7 @@ 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) & TARGET_PAGE_MASK; > + addr1 += memory_region_get_ram_addr(mr); > ptr = qemu_get_ram_ptr(mr->ram_block, addr1); > switch (endian) { > case DEVICE_LITTLE_ENDIAN: > @@ -3554,7 +3547,7 @@ 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) & TARGET_PAGE_MASK; > + 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/memory.c b/memory.c > index 95f7209..49c9b14 100644 > --- a/memory.c > +++ b/memory.c > @@ -1640,7 +1640,7 @@ int memory_region_get_fd(MemoryRegion *mr) > > assert(mr->ram_block); > > - return qemu_get_ram_fd(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK); > + return qemu_get_ram_fd(memory_region_get_ram_addr(mr)); > } > > void *memory_region_get_ram_ptr(MemoryRegion *mr) > @@ -1654,8 +1654,7 @@ 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) & TARGET_PAGE_MASK); > + ptr = qemu_get_ram_ptr(mr->ram_block, memory_region_get_ram_addr(mr)); > rcu_read_unlock(); > > return ptr + offset; > diff --git a/migration/savevm.c b/migration/savevm.c > index 0a33c22..cbba062 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -2229,13 +2229,13 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict) > > void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev) > { > - qemu_ram_set_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK, > + qemu_ram_set_idstr(memory_region_get_ram_addr(mr), > memory_region_name(mr), dev); > } > > void vmstate_unregister_ram(MemoryRegion *mr, DeviceState *dev) > { > - qemu_ram_unset_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK); > + qemu_ram_unset_idstr(memory_region_get_ram_addr(mr)); > } > > void vmstate_register_ram_global(MemoryRegion *mr) > diff --git a/translate-all.c b/translate-all.c > index e9f409b..6820d8e 100644 > --- a/translate-all.c > +++ b/translate-all.c > @@ -1550,8 +1550,7 @@ void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr) > rcu_read_unlock(); > return; > } > - ram_addr = (memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK) > - + addr; > + ram_addr = memory_region_get_ram_addr(mr) + addr; > tb_invalidate_phys_page_range(ram_addr, ram_addr + 1, 0); > rcu_read_unlock(); > } > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users 2016-03-24 11:03 [Qemu-devel] [PATCH 0/2] memory: cleanup users of memory_region_get_ram_addr Paolo Bonzini 2016-03-24 11:03 ` [Qemu-devel] [PATCH 1/2] memory: remove unnecessary masking of MemoryRegion ram_addr Paolo Bonzini @ 2016-03-24 11:03 ` Paolo Bonzini 2016-03-25 6:20 ` Fam Zheng 2016-04-03 13:49 ` Michael S. Tsirkin 1 sibling, 2 replies; 14+ messages in thread From: Paolo Bonzini @ 2016-03-24 11:03 UTC (permalink / raw) To: qemu-devel; +Cc: arei.gonglei, famz, mst 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 <pbonzini@redhat.com> --- 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users 2016-03-24 11:03 ` [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users Paolo Bonzini @ 2016-03-25 6:20 ` Fam Zheng 2016-03-25 11:58 ` Paolo Bonzini 2016-04-03 13:49 ` Michael S. Tsirkin 1 sibling, 1 reply; 14+ messages in thread From: Fam Zheng @ 2016-03-25 6:20 UTC (permalink / raw) To: Paolo Bonzini; +Cc: arei.gonglei, qemu-devel, mst On Thu, 03/24 12:03, 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 <pbonzini@redhat.com> > --- > 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) Shall we rename the parameter to "offset" then? I don't know, but that seems easier to read for me. > > 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); > + If called by address_space_unmap, is this addition still correct? 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 != NULL); if (is_write) { invalidate_and_set_dirty(mr, addr1, access_len); ^ `-- IIUC this is not an offset into mr, is it? > /* 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); > } > > @@ -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, Is this line too long? > + 4, dirty_log_mask); > r = MEMTX_OK; > } > if (result) { ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users 2016-03-25 6:20 ` Fam Zheng @ 2016-03-25 11:58 ` Paolo Bonzini 2016-03-25 12:13 ` Fam Zheng 0 siblings, 1 reply; 14+ messages in thread From: Paolo Bonzini @ 2016-03-25 11:58 UTC (permalink / raw) To: Fam Zheng; +Cc: arei gonglei, qemu-devel, mst ----- Original Message ----- > From: "Fam Zheng" <famz@redhat.com> > To: "Paolo Bonzini" <pbonzini@redhat.com> > Cc: qemu-devel@nongnu.org, "arei gonglei" <arei.gonglei@huawei.com>, mst@redhat.com > Sent: Friday, March 25, 2016 7:20:38 AM > Subject: Re: [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users > > On Thu, 03/24 12:03, 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 <pbonzini@redhat.com> > > --- > > 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) > > Shall we rename the parameter to "offset" then? I don't know, but that seems > easier to read for me. Good question. I'm not sure about that because of the block == NULL case, where the address is absolute. > > @@ -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); > > + > > If called by address_space_unmap, is this addition still correct? No, thanks for the careful review! That's another opportunity for cleanup actually, splitting the (few) users of qemu_ram_addr_from_host that really need a ram_addr_t and those (the majority) that need a MemoryRegion and offset. They can use two different functions. I'll defer this to 2.7 and post the patches to do so later. > > @@ -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, > > Is this line too long? It's 82 characters Paolo ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users 2016-03-25 11:58 ` Paolo Bonzini @ 2016-03-25 12:13 ` Fam Zheng 2016-03-25 12:19 ` Paolo Bonzini 0 siblings, 1 reply; 14+ messages in thread From: Fam Zheng @ 2016-03-25 12:13 UTC (permalink / raw) To: Paolo Bonzini; +Cc: arei gonglei, qemu-devel, mst On Fri, 03/25 07:58, Paolo Bonzini wrote: > > > ----- Original Message ----- > > From: "Fam Zheng" <famz@redhat.com> > > To: "Paolo Bonzini" <pbonzini@redhat.com> > > Cc: qemu-devel@nongnu.org, "arei gonglei" <arei.gonglei@huawei.com>, mst@redhat.com > > Sent: Friday, March 25, 2016 7:20:38 AM > > Subject: Re: [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users > > > > On Thu, 03/24 12:03, 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 <pbonzini@redhat.com> > > > --- > > > 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) > > > > Shall we rename the parameter to "offset" then? I don't know, but that seems > > easier to read for me. > > Good question. I'm not sure about that because of the block == NULL case, > where the address is absolute. > > > > @@ -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); > > > + > > > > If called by address_space_unmap, is this addition still correct? > > No, thanks for the careful review! That's another opportunity > for cleanup actually, splitting the (few) users of qemu_ram_addr_from_host > that really need a ram_addr_t and those (the majority) that need a > MemoryRegion and offset. They can use two different functions. I'll > defer this to 2.7 and post the patches to do so later. Good idea. The above "block == NULL" qemu_get_ram_ptr callers could use a separate function, too - frankly I don't like that function interface too much. What do you think? Fam ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users 2016-03-25 12:13 ` Fam Zheng @ 2016-03-25 12:19 ` Paolo Bonzini 0 siblings, 0 replies; 14+ messages in thread From: Paolo Bonzini @ 2016-03-25 12:19 UTC (permalink / raw) To: Fam Zheng; +Cc: arei gonglei, qemu-devel, mst > > > If called by address_space_unmap, is this addition still correct? > > > > No, thanks for the careful review! That's another opportunity > > for cleanup actually, splitting the (few) users of qemu_ram_addr_from_host > > that really need a ram_addr_t and those (the majority) that need a > > MemoryRegion and offset. They can use two different functions. I'll > > defer this to 2.7 and post the patches to do so later. > > Good idea. The above "block == NULL" qemu_get_ram_ptr callers could use a > separate function, too - frankly I don't like that function interface too > much. > What do you think? I don't know, at least block == NULL has a clear meaning. It's not entirely satisfying, but the users are readable and the ones that pass NULL stand out. In the case of qemu_ram_addr_from_host, on the other hand, there's a clear opportunity to avoid bugs. Paolo ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users 2016-03-24 11:03 ` [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users Paolo Bonzini 2016-03-25 6:20 ` Fam Zheng @ 2016-04-03 13:49 ` Michael S. Tsirkin 2016-04-03 17:14 ` Paolo Bonzini 2016-04-04 8:03 ` Paolo Bonzini 1 sibling, 2 replies; 14+ messages in thread From: Michael S. Tsirkin @ 2016-04-03 13:49 UTC (permalink / raw) To: Paolo Bonzini; +Cc: arei.gonglei, famz, qemu-devel 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 <pbonzini@redhat.com> 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users 2016-04-03 13:49 ` Michael S. Tsirkin @ 2016-04-03 17:14 ` Paolo Bonzini 2016-04-04 8:03 ` Paolo Bonzini 1 sibling, 0 replies; 14+ messages in thread From: Paolo Bonzini @ 2016-04-03 17:14 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: arei.gonglei, famz, qemu-devel On 03/04/2016 15:49, Michael S. Tsirkin wrote: > > 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. Yes, this makes sense. If it were all in 2.6, there would be no released version with an absolute ram_addr_t argument *and* a RAMBlock* argument, but we will do the incompatible change in 2.7 and then it makes sense to rename it. Paolo ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users 2016-04-03 13:49 ` Michael S. Tsirkin 2016-04-03 17:14 ` Paolo Bonzini @ 2016-04-04 8:03 ` Paolo Bonzini 2016-04-04 8:38 ` Michael S. Tsirkin 1 sibling, 1 reply; 14+ messages in thread From: Paolo Bonzini @ 2016-04-04 8:03 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: arei.gonglei, famz, qemu-devel On 03/04/2016 15:49, Michael S. Tsirkin wrote: > I agree but I think we need a better name for this function. > qemu_ram_offset_to_ptr? What about qemu_map_ram_ptr? Paolo > Will also serve to make sure backporting patches across this > API change does not cause issues. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users 2016-04-04 8:03 ` Paolo Bonzini @ 2016-04-04 8:38 ` Michael S. Tsirkin 2016-04-04 9:02 ` Paolo Bonzini 0 siblings, 1 reply; 14+ messages in thread From: Michael S. Tsirkin @ 2016-04-04 8:38 UTC (permalink / raw) To: Paolo Bonzini; +Cc: arei.gonglei, famz, qemu-devel On Mon, Apr 04, 2016 at 10:03:12AM +0200, Paolo Bonzini wrote: > > > On 03/04/2016 15:49, Michael S. Tsirkin wrote: > > I agree but I think we need a better name for this function. > > qemu_ram_offset_to_ptr? > > What about qemu_map_ram_ptr? > > Paolo OK but this seems to imply there's also an unmap operation? > > Will also serve to make sure backporting patches across this > > API change does not cause issues. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users 2016-04-04 8:38 ` Michael S. Tsirkin @ 2016-04-04 9:02 ` Paolo Bonzini 0 siblings, 0 replies; 14+ messages in thread From: Paolo Bonzini @ 2016-04-04 9:02 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: arei.gonglei, famz, qemu-devel On 04/04/2016 10:38, Michael S. Tsirkin wrote: > > > I agree but I think we need a better name for this function. > > > qemu_ram_offset_to_ptr? > > > > What about qemu_map_ram_ptr? > > > > Paolo > > OK but this seems to imply there's also an unmap operation? Actually there is one, even though it's almost always a nop---it's xen_invalidate_map_cache_entry. Paolo ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-04-04 9:02 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-03-24 11:03 [Qemu-devel] [PATCH 0/2] memory: cleanup users of memory_region_get_ram_addr Paolo Bonzini 2016-03-24 11:03 ` [Qemu-devel] [PATCH 1/2] memory: remove unnecessary masking of MemoryRegion ram_addr Paolo Bonzini 2016-03-25 6:20 ` Fam Zheng 2016-04-03 13:48 ` Michael S. Tsirkin 2016-03-24 11:03 ` [Qemu-devel] [PATCH 2/2] memory: hide mr->ram_addr from qemu_get_ram_ptr users Paolo Bonzini 2016-03-25 6:20 ` Fam Zheng 2016-03-25 11:58 ` Paolo Bonzini 2016-03-25 12:13 ` Fam Zheng 2016-03-25 12:19 ` Paolo Bonzini 2016-04-03 13:49 ` Michael S. Tsirkin 2016-04-03 17:14 ` Paolo Bonzini 2016-04-04 8:03 ` Paolo Bonzini 2016-04-04 8:38 ` Michael S. Tsirkin 2016-04-04 9:02 ` Paolo Bonzini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).