----- Original Message ----- > From: "Fam Zheng" <f...@redhat.com> > To: "Paolo Bonzini" <pbonz...@redhat.com> > Cc: qemu-devel@nongnu.org, "arei gonglei" <arei.gong...@huawei.com>, > m...@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 <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.
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