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
