From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54063) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VBTaJ-0003O9-Fa for qemu-devel@nongnu.org; Mon, 19 Aug 2013 13:49:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VBTaD-0003iZ-Gu for qemu-devel@nongnu.org; Mon, 19 Aug 2013 13:49:43 -0400 Received: from mx1.redhat.com ([209.132.183.28]:14959) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VBTaD-0003iK-7e for qemu-devel@nongnu.org; Mon, 19 Aug 2013 13:49:37 -0400 Date: Mon, 19 Aug 2013 20:51:23 +0300 From: "Michael S. Tsirkin" Message-ID: <20130819175123.GC7737@redhat.com> References: <1376922370-5681-1-git-send-email-mst@redhat.com> <1376922370-5681-2-git-send-email-mst@redhat.com> <521257E8.6050800@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v4 1/2] arch_init: align MR size to target page size List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: pbonzini@redhat.com, kraxel@redhat.com, Laszlo Ersek , qemu-devel@nongnu.org, quintela@redhat.com On Mon, Aug 19, 2013 at 06:45:10PM +0100, Peter Maydell wrote: > On 19 August 2013 18:37, Laszlo Ersek wrote: > > ((3) memory_region_size() is slightly different from > > int128_get64(mr->size); it has a special case for int128_2_64() -- and I > > don't understand that. > > The special case is because valid memory region sizes range > from 0 to 2^64, *inclusive*. [2^64-sized regions tend to be > containers representing address spaces like PCI or the system > memory space.] Inside memory.c we store the size > in an int128 in the way you'd expect. However some of the > memory region APIs take or return the size of the region > as a uint64_t, with the convention that UINT64_MAX means > "actually 2^64" (with a size of really 2^64 - 1 not being > valid). So the special case here is doing the conversion > from an int128 representation of the size to the uint64_t > representation. You can see the same thing going the other > way in memory_region_init(), where we take the size as a > uint64_t and convert it to an int128 with > > mr->size = int128_make64(size); > if (size == UINT64_MAX) { > mr->size = int128_2_64(); > } > > (Note that int128_get64() of an int128 which == 2^64 or > more will assert, so int128_get64(mr->size) is a bit of a > code smell; it happens to be OK here because we know that a > RAM-backed MR will never be a 2^64-sized region.) > > -- PMM Note code smell is in the original code - this patch uses the API. We could add memory_region_get_target_pages as that will fit in uint64_t. But other migration code shifts it right back by target page size, so I don't think it's worth it.