Re: [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations

[Pierre Morel <pmorel@linux.vnet.ibm.com>]

On 11/05/2015 05:32 PM, Paolo Bonzini wrote:
>
> On 05/11/2015 17:18, Pierre Morel wrote:
>> The size of a memory area can never be negative.
>> It follows it must be defined as an unsigned value.
>>      
>> Let's modify the memory regions size to unsigned 128 integer
>> and modify accordingly the 128 bit arithmetic.
>>      
>> This makes memory size calculations easier and easier to understand.
>>      
>> I fear loud protest but really, it is a broken concept that
>> obfuscate the code.
> You are right in fearing loud protest, though the protest is for the
> lack of explanation of what is broken.
>
> Since the values are never going to be > 2^65, there is no chance of
> overflow.  On the other hand there are cases where we compute
> start+size-1, and size-1 *could* overflow if you use unsigned integers.
>
> So I am not sure... why?
>
> Paolo
>
Paolo,

The calculation are not broken and it works for actual usage but:

For me, it is the design that is broken, as it uses an integer to represent
something that is fundamentally unsigned like the size of a memory area.

This leads to have UINT64_MAX represented with {1, 0} instead of {0, 
UINT64_MAX}
while {1, 0} is 2^64.
This again leads to have unnecessary and obfuscating transformations 
with int128_2_64()
to test for UINT64_MAX and return {1,0} in memory_region_init() while using
inverse translation test{1,0} and return UINT64_MAX in memory_region_size()

Another concern is that pushing the 31th bit to the 32nd bit may be 
confusing when
used, if ever, with IOMMU or other hardware components using 128bits 
registers.
Considering some Intel IOMMU for example.

The modifications I proposed are also not perfect, I just seen I forgot 
to change a -1 to UINT64_MAX
and I forgot some unnecessary cast. At least.

Pierre