Re: [Qemu-devel] [PATCH 10/21] memory: make section size a 128-bit integer

[Alexey Kardashevskiy]

On 06/07/2013 11:09 AM, Paolo Bonzini wrote:
> Il 06/06/2013 04:36, Alexey Kardashevskiy ha scritto:
>>>> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
>>>> index 693a9ff..c89676b 100644
>>>> --- a/hw/misc/vfio.c
>>>> +++ b/hw/misc/vfio.c
>>>> @@ -1953,7 +1953,7 @@ static void vfio_listener_region_add(MemoryListener 
>>>> *listener,
>>>>      }
>>>>  
>>>>      iova = TARGET_PAGE_ALIGN(section->offset_within_address_space);
>>>> -    end = (section->offset_within_address_space + section->size) &
>>>> +    end = (section->offset_within_address_space + 
>>>> int128_get64(section->size)) &
>>>>            TARGET_PAGE_MASK;
>>
>>
>> Another problem with this patch. Here is some more context (***):
> 
> By the time you get here, this should have already crashed at this
> code that patch 13 adds:
> 
> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> index c89676b..52fb036 100644
> --- a/hw/misc/vfio.c
> +++ b/hw/misc/vfio.c
> @@ -1939,6 +1939,8 @@ static void vfio_listener_region_add(MemoryListener 
> *listener,
>      void *vaddr;
>      int ret;
>  
> +    assert(!memory_region_is_iommu(section->mr));
> +
> 
> so it seems like a bug in your VFIO patches.


No, I have David's patches which fix all of this, I'm planning to post them
soon.

My question is rather what is the whole point of calling
memory_region_init_iommu with size==UINT64_MAX?

mem_add() tries to do register_subpage() when size is not aligned
(UINT64_MAX is not) and fails.

So if we want to init memory region with the size as big as possible on
64bit systems, we either have to replace all 64bit sizes with 64bit end
addresses (and then use 0-ffffffffffffffff) or support int128 sizes
everywhere (even if it is just hi=1, lo=0) or stop initializing memory
regions with sizes way bigger than we really need in next 5 years.

Am I missing a bigger picture?


-- 
Alexey