From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38907) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VC5LK-00080C-Mo for qemu-devel@nongnu.org; Wed, 21 Aug 2013 06:08:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VC5LF-0008OZ-N8 for qemu-devel@nongnu.org; Wed, 21 Aug 2013 06:08:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48900) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VC5LF-0008OP-Fy for qemu-devel@nongnu.org; Wed, 21 Aug 2013 06:08:41 -0400 Message-ID: <5214917B.4040607@redhat.com> Date: Wed, 21 Aug 2013 12:07:55 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1377077318-12966-1-git-send-email-aik@ozlabs.ru> <1377077318-12966-3-git-send-email-aik@ozlabs.ru> In-Reply-To: <1377077318-12966-3-git-send-email-aik@ozlabs.ru> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] vfio: Fix 128 bit handling List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: Alex Williamson , qemu-devel@nongnu.org, Alexander Graf Il 21/08/2013 11:28, Alexey Kardashevskiy ha scritto: > Upcoming VFIO on SPAPR PPC64 support will initialize the IOMMU > memory region with UINT64_MAX (2^64 bytes) size so int128_get64() > will assert. > > The patch takes care of this check. The existing type1 IOMMU code > is not expected to map all 64 bits of RAM so the patch does not > touch that part. > > Signed-off-by: Alexey Kardashevskiy > --- > hw/misc/vfio.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c > index e917f03..1889225 100644 > --- a/hw/misc/vfio.c > +++ b/hw/misc/vfio.c > @@ -1920,6 +1920,7 @@ static void vfio_listener_region_add(MemoryListener *listener, > VFIOContainer *container = container_of(listener, VFIOContainer, > iommu_data.listener); > hwaddr iova, end; > + Int128 llend; > void *vaddr; > int ret; > > @@ -1940,13 +1941,17 @@ static void vfio_listener_region_add(MemoryListener *listener, > } > > iova = TARGET_PAGE_ALIGN(section->offset_within_address_space); > - end = (section->offset_within_address_space + int128_get64(section->size)) & > - TARGET_PAGE_MASK; > + llend = int128_make64(section->offset_within_address_space); > + int128_addto(&llend, section->size); Please use int128_add. > + llend.lo &= TARGET_PAGE_MASK; Int128 is opaque, please use int128_and. To build the constant you have three choices (from my preferred to IMHO worst): - add a new int128_exts64 function that sign-extends an int64_t - use int128_neg(int128_make64(TARGET_PAGE_SIZE)) or something like that - add a new int128_make function that takes a low/high pair and use int128_make(TARGET_PAGE_MASK, -1) Otherwise looks good! Paolo > > - if (iova >= end) { > + if (int128_ge(int128_make64(iova), llend)) { > return; > } > > + end = (section->offset_within_address_space + int128_get64(section->size)) & > + TARGET_PAGE_MASK; > + > vaddr = memory_region_get_ram_ptr(section->mr) + > section->offset_within_region + > (iova - section->offset_within_address_space); >