On 08/21/2013 08:07 PM, Paolo Bonzini wrote:
> 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 <a...@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
Like this? I am really scared to screw here :)
static inline Int128 int128_exts64(int64_t a)
{
return (Int128) { .lo = a, .hi = (a < 0) ? -1 : 0 };
}
> - use int128_neg(int128_make64(TARGET_PAGE_SIZE)) or something like that
Did you actually mean TARGET_PAGE_SIZE-1 (with -1)? I'll better use this
for now.
> - add a new int128_make function that takes a low/high pair and use
> int128_make(TARGET_PAGE_MASK, -1)
I liked this one actually but you called it "worst" :)
> 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);
>>
>
--
Alexey
