Re: [Qemu-devel] [PATCH 1/5] Add target memory mapping API

[Ian Jackson]

Avi Kivity writes ("Re: [Qemu-devel] [PATCH 1/5] Add target memory mapping 
API"):
> Ian Jackson wrote:
> > So it is important to us that this interface is [nice]
>
> Sure.

Thanks :-).

> Correct.  If you need to perform read-modify-write, you need to use 
> cpu_physical_memory_rw(), twice.  If we ever want to support RMW, we'll 
> need to add another value for is_write.  I don't think we have 
> interesting devices at this point which require efficient RMW.

Efficient read-modify-write may be very hard for some setups to
achieve.  It can't be done with the bounce buffer implementation.
I think ond good rule of thumb would be to make sure that the interface
as specified can be implemented in terms of cpu_physical_memory_rw.

> Right.  Moreover, the access sizes will be swallowed by the API; devices 
> are allowed to provide different implementations for different access sizes.

Yes, that's another important fact about the API.

> > What happens if the caller maps with is_write==1, writes part of the
> > buffer, and then unmaps ?  In your implementation undefined data is
> > written to guest memory.  I think that since not all callers can
> > guarantee to wish to write to the whole region, this means that the
> > interface is defective.  Callers need to be required to specify which
> > areas they have written.
...
> Alternatively, only use this interface with devices where this doesn't 
> matter.  Given that bouncing happens for mmio only, this would be all 
> devices which you'd want to use this interface with anyway.

That would be one alternative but isn't it the case that (for example)
with a partial DMA completion, the guest can assume that the
supposedly-untouched parts of the DMA target memory actually remain
untouched rather than (say) zeroed ?

In a system where we're trying to do zero copy, we may issue the map
request for a large transfer, before we know how much the host kernel
will actually provide.

> (I'm assuming that you'll implement the fastpath by directly mapping 
> guest memory, not bouncing).

Yes.  We can do that in Xen too but it's less of a priority for us
given that we expect people who really care about performance to
install PV drivers in the guest.

> A variant of this API (posted by Andrea) hid all of the scheduling away 
> within the implementation.

I remember seeing this before but I don't think your previous one
provided a callback for map completion ?  I thought it just blocked
the caller until the map could complete.  That's obviously not ideal.

> > This function should return a separate handle as well as the physical
> > memory pointer.  That will make it much easier to provide an
> > implementation which permits multiple bounce buffers or multiple
> > mappings simultaneously.
> 
> The downside to a separate handle is that device emulation code will now 
> need to maintain the handle in addition to the the virtual address.  
> Since the addresses will typically be maintained in an iovec, this means 
> another array to be allocated and resized.

Err, no, I don't really see that.  In my proposal the `handle' is
actually allocated by the caller.  The implementation provides the
private data and that can be empty.  There is no additional memory
allocation.

> The design goals here were to keep things as simple as possible for the 
> fast path.  Since the API fits all high-bandwidth devices that I know 
> of, I don't think it's a good tradeoff to make the API more complex in 
> order to be applicable to some corner cases.

I think my question about partial DMA writes is very relevant.  If we
don't care about that, nor about the corresponding notification for
reads, then the API can be a lot simpler.

Ian.