From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48833) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xol4w-0006ov-7x for qemu-devel@nongnu.org; Wed, 12 Nov 2014 22:28:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xol4r-00022R-44 for qemu-devel@nongnu.org; Wed, 12 Nov 2014 22:28:14 -0500 Received: from ozlabs.org ([103.22.144.67]:60130) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xol4q-000221-F2 for qemu-devel@nongnu.org; Wed, 12 Nov 2014 22:28:09 -0500 Date: Thu, 13 Nov 2014 13:59:21 +1100 From: David Gibson Message-ID: <20141113025921.GC7291@voom.fritz.box> References: <1412358473-31398-1-git-send-email-dgilbert@redhat.com> <1412358473-31398-42-git-send-email-dgilbert@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="R+My9LyyhiUvIEro" Content-Disposition: inline In-Reply-To: <1412358473-31398-42-git-send-email-dgilbert@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4 41/47] qemu_ram_block_from_host List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert (git)" Cc: aarcange@redhat.com, yamahata@private.email.ne.jp, lilei@linux.vnet.ibm.com, quintela@redhat.com, cristian.klein@cs.umu.se, qemu-devel@nongnu.org, amit.shah@redhat.com, yanghy@cn.fujitsu.com --R+My9LyyhiUvIEro Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Oct 03, 2014 at 06:47:47PM +0100, Dr. David Alan Gilbert (git) wrot= e: > From: "Dr. David Alan Gilbert" >=20 > Postcopy sends RAMBlock names and offsets over the wire (since it can't > rely on the order of ramaddr being the same), and it starts out with > HVA fault addresses from the kernel. >=20 > qemu_ram_block_from_host translates a HVA into a RAMBlock, an offset > in the RAMBlock, the global ram_addr_t value and it's bitmap position. s/it's/its/ I find most of the passing around of bitmap positions confusing in this patch series. Would it make things simpler if you broke up the bitmap into (aligned) per-ramblock chunks. Then the offset would determine the bitmap position, which is easier to understand since it has an "inherent" meaning outside of the secondary data structure used to track things. > Rewrite qemu_ram_addr_from_host to use qemu_ram_block_from_host. >=20 > Provide qemu_ram_get_idstr since it's the actual name text sent on the > wire. >=20 > Signed-off-by: Dr. David Alan Gilbert > --- > exec.c | 56 +++++++++++++++++++++++++++++++++++++++++= +----- > include/exec/cpu-common.h | 4 ++++ > 2 files changed, 55 insertions(+), 5 deletions(-) >=20 > diff --git a/exec.c b/exec.c > index 65ee612..07722b3 100644 > --- a/exec.c > +++ b/exec.c > @@ -1246,6 +1246,11 @@ static RAMBlock *find_ram_block(ram_addr_t addr) > return NULL; > } > =20 > +const char *qemu_ram_get_idstr(RAMBlock *rb) > +{ > + return rb->idstr; > +} > + > void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *= dev) > { > RAMBlock *new_block =3D find_ram_block(addr); > @@ -1603,16 +1608,35 @@ static void *qemu_ram_ptr_length(ram_addr_t addr,= hwaddr *size) > } > } > =20 > -/* Some of the softmmu routines need to translate from a host pointer > - (typically a TLB entry) back to a ram offset. */ > -MemoryRegion *qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr) > +/* > + * Translates a host ptr back to a RAMBlock, a ram_addr and an offset > + * in that RAMBlock. > + * > + * ptr: Host pointer to look up > + * round_offset: If true round the result offset down to a page boundary > + * *ram_addr: set to result ram_addr > + * *offset: set to result offset within the RAMBlock > + * *bm_index: bitmap index (i.e. scaled ram_addr for use where the scale > + * isn't available) > + * > + * Returns: RAMBlock (or NULL if not found) > + */ > +RAMBlock *qemu_ram_block_from_host(void *ptr, bool round_offset, > + ram_addr_t *ram_addr, > + ram_addr_t *offset, > + unsigned long *bm_index) > { > RAMBlock *block; > uint8_t *host =3D ptr; > =20 > if (xen_enabled()) { > *ram_addr =3D xen_ram_addr_from_mapcache(ptr); > - return qemu_get_ram_block(*ram_addr)->mr; > + block =3D qemu_get_ram_block(*ram_addr); > + if (!block) { > + return NULL; > + } > + *offset =3D (host - block->host); > + return block; > } > =20 > block =3D ram_list.mru_block; > @@ -1633,7 +1657,29 @@ MemoryRegion *qemu_ram_addr_from_host(void *ptr, r= am_addr_t *ram_addr) > return NULL; > =20 > found: > - *ram_addr =3D block->offset + (host - block->host); > + *offset =3D (host - block->host); > + if (round_offset) { > + *offset &=3D TARGET_PAGE_MASK; > + } This seems clumsy. Surely the caller can apply the mask itself it it wants that. > + *ram_addr =3D block->offset + *offset; > + *bm_index =3D *ram_addr >> TARGET_PAGE_BITS; > + return block; > +} > + > +/* Some of the softmmu routines need to translate from a host pointer > + (typically a TLB entry) back to a ram offset. */ > +MemoryRegion *qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr) > +{ > + RAMBlock *block; > + ram_addr_t offset; /* Not used */ > + unsigned long index; /* Not used */ > + > + block =3D qemu_ram_block_from_host(ptr, false, ram_addr, &offset, &i= ndex); > + > + if (!block) { > + return NULL; > + } > + > return block->mr; > } > =20 > diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h > index 8042f50..ae25407 100644 > --- a/include/exec/cpu-common.h > +++ b/include/exec/cpu-common.h > @@ -55,8 +55,12 @@ typedef uint32_t CPUReadMemoryFunc(void *opaque, hwadd= r addr); > void qemu_ram_remap(ram_addr_t addr, ram_addr_t length); > /* This should not be used by devices. */ > MemoryRegion *qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr); > +RAMBlock *qemu_ram_block_from_host(void *ptr, bool round_offset, > + ram_addr_t *ram_addr, ram_addr_t *off= set, > + unsigned long *bm_index); > void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *= dev); > void qemu_ram_unset_idstr(ram_addr_t addr); > +const char *qemu_ram_get_idstr(RAMBlock *rb); > =20 > void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf, > int len, int is_write); --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --R+My9LyyhiUvIEro Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUZB6JAAoJEGw4ysog2bOS1aYP/iKdsvIRcnJCpYZkOS1vGVSM GOpzZCbQHBp7AAHRkFQVlofwlDuLQ1bHDIIJtl1GHBcnavV8eTPviljvUU+EDewA 1bWF2PZO5SeWYx41+E23hRsxinDczHUn8kMWrctY6uvVkAESCdISFBbBX0pZthHl OFmHJZgquWhMstAJyjSrxx3epuy75Gna3ojj4wOnSyfqi8/a91lT9PTk23graCWH bsHsg/79XTWmrJ58h590izckfGVTFntJrpxL4o+i3oE9JgDJtaLOCpwO0IC769vV M45CgwmR73J/IYkpROwYBZfPWzv6vxF+FARdKoMgJPa2ge+DJ9+A5vXIipehh7jz dJV/nbVHZ40U/FGMlobo4KeDnohIkLgrFIVUEpGfhYRigDrHdCQ0gEB+K8ZMjnwH 3x8BlmmgSir79CLrHHTxePXgOeuAw1PNSovmVCsuakOJThReziv2/W+6sVGax7pa NFDrgbS3i2Kphs5gPRjh8ahsF/sFEjMNmJyjo/CZW+Lh1Nt2qmMbD17D5ENVx8Ky 5IFzEI6D+QFtdCkGUoWoJVDIA/rx8SkgkoCboR8iNhvQecwqKNdadACYanlgJRY/ 8vS5hdSHQ65SLD/937zLCiuJkp8T3aQdYzAzFkInC/iG/KRTYJPSYc8MGS3fAXwK iazXo1+iymiBL+QVI3HW =I8iu -----END PGP SIGNATURE----- --R+My9LyyhiUvIEro--