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 <pbonz...@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) {