On 10/18/2011 03:38 AM, David Gibson wrote: > On Mon, Oct 17, 2011 at 12:34:19PM +0200, Avi Kivity wrote: > > On 10/17/2011 07:31 AM, David Gibson wrote: > > > > > > > > In terms of how the code looks, it's seriously more ugly (see the > > > > patches I sent out). Conceptually it's cleaner, since we're not dodging > > > > the issue that we need to deal with a full 64-bit domain. > > > > > > We don't have to dodge that issue. I know how to remove the > > > requirement for intermediate negative values, I just haven't made up a > > > patch yet. With that we can change to uint64 and cover the full 64 > > > bit range. In fact I think I can make it so that size==0 represents > > > size=2^64 and even handle the full 64-bit, inclusive range properly. > > > > That means you can't do a real size == 0. > > Yeah... a memory range with size 0 has no effect by definition, I > think we can do without it.
How do we make sure all callers know this? > > > > But my main concern is maintainability. The 64-bit blanket is to short, > > > > if we keep pulling it in various directions we'll just expose ourselves > > > > in new ways. > > > > > > Nonsense, dealing with full X-bit range calculations in X-bit types is > > > a fairly standard problem. The kernel does it in VMA handling for > > > one. It just requires thinking about overflow cases. > > > > We discovered three bugs already (you found two, and I had one during > > development). Even if it can probably be done with extreme care, but is > > it worth spending all that development time on? > > > > I'm not sure there is a parallel with vmas, since we're offsetting in > > both the positive and negative directions. > > I think the so-called "negative offsetting" is just an artifact of our > implementation. I don't see that it's any different from having a VMA > whose file offset is larger than its memory address. > Consider the vga window at 0xa0000 pointing into the framebuffer at alias_offset 0x1a0000. To the system, it looks like a copy of the framebuffer starts at -0x1000000, becomes visible 0x1a0000 bytes later (at 0xa0000), then becomes invisible again at 0xa8000. Yes, it's an artifact, but I don't want to spend too much time worrying about it, if I can throw a few more bits at the problem. The API is too central to make a case by case analysis of where things can go wrong, it needs to be robust. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.