Re: [Qemu-devel] Re: [PATCH 2 of 5] add can_dma/post_dma for direct IO

[Avi Kivity <avi@redhat.com>]

Anthony Liguori wrote:
> I've thought quite a bit about it, and I'm becoming less convinced 
> that this sort of API is going to be helpful.
>
> I was thinking that we need to make one minor change to the map API I 
> proposed.  It should return a mapped size as an output parameter and 
> take a flag as to whether partial mappings can be handled.  The effect 
> would be that you never bounce to RAM which means that you can also 
> quite accurately determine the maximum amount of bouncing (it should 
> be proportional to the amount of MMIO memory that's registered).

That's pointless; cirrus for example has 8MB of mmio while a cpu-to-vram 
blit is in progress, and some random device we'll add tomorrow could 
easily introduce more.  Our APIs shouldn't depend on properties of 
emulated hardware, at least as much as possible.

Of course you don't need the API if you replicate its functionality in 3 
block devices (+ out of tree), 4+ network devices (+1 out of tree), and 
any other new dma client that comes along.  But I think that coding the 
bouncing and rescheduling logic in all of those devices is a mistake, 
for reasons I don't even wish to point out.

Just as an example: as presented, the maximum bounce memory allocated is 
equal to the number of concurrent requests issues by all dma capable 
devices multiplied by the maximum dma buffer size.  Should we consider 
this to be a deficiency (and I do), it is easy to modify dma.c to limit 
the amount of allocated memory and defer requests when we are out.  It 
would be a horrible pain to do this using map()/unmap() alone.

The logic in dma.c cannot be made to go away.  You can either put it in 
the dma clients, or in map()/unmap(), or leave it in dma.c.  While 
putting it in map()/unmap() is possible (though undesirable IMO), 
letting the dma clients handle these details is a big mistake.

I'll enumerate the functions that dma.c provides:
- convert guest physical addresses to host virtual addresses
- construct an iovec for scatter/gather
- handle guest physical addresses for which no host virtual addresses 
exist, while controlling memory use
- take care of the dirty bit
- provide a place for adding hooks to hardware that can modify dma 
operations generically (emulated iommus, transforming dma engines)

I believe that a dma api that fails to address all of these requirements 
is trying to solve too few problems at the same time, and will either 
cause dma clients to be unduly complicated, or will require rewriting.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.