From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50833) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ajQc4-0003QQ-Im for qemu-devel@nongnu.org; Fri, 25 Mar 2016 08:13:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ajQc0-0007Hq-7J for qemu-devel@nongnu.org; Fri, 25 Mar 2016 08:13:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45819) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ajQc0-0007Hd-0F for qemu-devel@nongnu.org; Fri, 25 Mar 2016 08:13:08 -0400 Date: Fri, 25 Mar 2016 20:13:04 +0800 From: Fam Zheng Message-ID: <20160325121304.GA21300@ad.usersys.redhat.com> References: <1458817415-29247-1-git-send-email-pbonzini@redhat.com> <1458817415-29247-3-git-send-email-pbonzini@redhat.com> <20160325062038.GA20431@ad.usersys.redhat.com> <1379879614.40943389.1458907082389.JavaMail.zimbra@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1379879614.40943389.1458907082389.JavaMail.zimbra@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 , qemu-devel@nongnu.org, mst@redhat.com On Fri, 03/25 07:58, Paolo Bonzini wrote: > > > ----- Original Message ----- > > From: "Fam Zheng" > > To: "Paolo Bonzini" > > Cc: qemu-devel@nongnu.org, "arei gonglei" , 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 > > > --- > > > 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