Re: [Qemu-devel] [PATCH 2/2] vfio: Fix 128 bit handling

[Paolo Bonzini <pbonzini@redhat.com>]

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 <aik@ozlabs.ru>
> ---
>  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);
>