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

Blue Swirl
2008-12-15T18:35:38Z

[-- Attachment #1: Type: text/plain, Size: 4357 bytes --]

On 12/15/08, Anthony Liguori <anthony@codemonkey.ws> wrote:
> Avi Kivity wrote:
>
> > 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.
> >
>
> One way to think of what I'm suggesting, is that if for every
> cpu_register_physical_memory call for MMIO, we allocated a buffer, then
> whenever map() was called on MMIO, we would return that already allocated
> buffer. The overhead is fixed and honestly relatively small. Much smaller
> than dma.c proposes.
>
> But you can be smarter, and lazily allocate those buffers for MMIO. That
> will reduce the up front memory consumption. I'd be perfectly happy though
> if the first implementation just malloc() on cpu_register_physical_memory
> for the sake of simplicity.

Maybe the buffer thing should not be called DMA, but something like
bounce-buffers?

> > I'll enumerate the functions that dma.c provides:
> > - convert guest physical addresses to host virtual addresses
> >
>
> The map() api does this.
>
>
> > - construct an iovec for scatter/gather
> >
>
> The map() api does not do this.
>
>
> > - 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)
> >
>
> The map() api does all of this.
>
>
> > 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 think there's a disconnect between what you describe and what the current
> code does. I think there's a very simple solution, let's start with the
> map() api. I'm convinced that the virtio support for it will be trivial and
> that virtio will not benefit from the dma.c api proposed. I'm willing to do
> the virtio map implementation to demonstrate this. Let's see two
> implementations that use the dma.c api before we commit to it. I'd like to
> see at least a network device and a block device. I don't believe network
> devices will benefit from it because they don't support partial submissions.
>
> I like any API that reduces duplicate code. It's easy to demonstrate that
> with patches. Based on my current understanding of the API and what I
> expect from the devices using it, I don't believe the API will actually do
> that. It's quite easy to prove me wrong though.

I changed the ESP SCSI and Lance Ethernet on Sparc32 to resolve the IO
address to physical memory (see patch). ESP works (no zero copy yet),
Lance doesn't. It looks much better. Because the resolving activity is
performed in serial steps, unbounded IO vector allocation does not
happen, but we still could launch as many IO as there are free IO
vectors.

There are still some issues I'm not happy yet:
- handling of access violations: resolving should stop before the bad
page, the transfers should be done until that and then post error.
- bounce buffers needed for Lance byte swapping are not well designed (stack)

This lead me to the thought that maybe we should not hide the bounce
buffer activity, but instead make it more explicit for the device that
needs bouncing. For the other device, the buffering or lack of it
should be opaque.

Also the virtual-to-physical address resolution API could be generic,
ie all resolver functions should take same parameters so that the
devices would not need to know the next higher level device.

[-- Attachment #2: translation-api-sparc.diff --]
[-- Type: plain/text, Size: 13400 bytes --]