Re: [Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[Avi Kivity <avi@redhat.com>]

On 11/01/2011 03:48 PM, Andreas Färber wrote:
> > 
> > Since it's just internal, I'll just pull this series and if we want to
> > change it post 1.0, we can.
>
> FWIW I must say I don't like where this is heading... iiuc just because
> of a zero-or-full-64-bits issue with start+end 

It's not just that issue.

> we're doubling the
> internal storage format for all memory ranges. 

It's not doubled, since the MemoryRegion structure has many other
members.  And anyway there are too few such structures to matter.

On the other hand, when we get to rewrite the core we can eliminate the
PageDesc array at 16 bytes per guest page, that is a really significant
saving.

> If having the size
> unsigned would eliminate the overflow issue at hand, can't we move the
> signedness to some flag field instead?

You mean a 65-bit integer?

> I don't see a problem with using macros/inlines, just with the seemingly
> unnecessary 128-bitness. In particular I'm thinking of ARM.

We could rename Int128 something like AddrArith and make it 64-bit for
archs with 32-bit target_phys_addr_t.  However I don't think there's
much point.  This code is executed very rarely, at initialization time
and when the physical address map changes (like when mapping a BAR). 
There's no point in optimizing it (and every point for making it robust).

> Since this seems to be addressing an overflow bug in ppc64, the
> hard-freeze date shouldn't make us rush this IMO.

The bug it fixes is actually in code not yet merged.  But I'm certain
there are more lurking in there, we're taking guest supplied 64-bit
values and using 64-bit arithmetic on them.  It has to overflow.

-- 
error compiling committee.c: too many arguments to function