Re: [Qemu-devel] [RFC 1/1] pci-dma-api-v2

[Avi Kivity]

Anthony Liguori wrote:
> I think you need something more sophisticated that can split up a 
> scatter/gather list into a set of partial bounce buffers and partial 
> direct copies.  Just checking the vector once is a bit hackish.

That code path would never be used or tested.  As it is, bouncing will 
only be invoked by dma to or from mmio, and I expect most guests never 
to invoke it.  Why would we complicate the code even more?

> What should be fixed?  Are these emulated functions wrong?
>
> There's a lack of symmetry here.  We should have a bdrv_readv and 
> bdrv_aio_readv.  bdrv_read and bdrv_aio_read should disappear.  We can 
> maintain wrappers that create a compatible interface for older code 
> but just adding a new API is wrong.

It was understood a real aio readv/writev was being worked on, so the 
emulation could be a temporary step.

> > +BlockDriverAIOCB *bdrv_aio_readv(BlockDriverState *bs, int64_t 
> > sector_num,
> > +                 struct iovec *iov, int iovcnt, size_t len,
> > +                 BlockDriverCompletionFunc *cb, void *opaque)
> > +{
> >   
>
> struct iovec does not exist on Windows.  I also don't think you've got 
> the abstraction right.  Reading and writing may require actions to 
> happen.  I don't think you can just introduce something as simple as 
> an iovec here.  I think we need something more active.

Can you elaborate?  Actions happen in the completion callback.  This is 
just an straightforward extension of the block API, I don't think a dma 
api should materially change the block api.

> I think you missed the mark here.  This API needs to go through the 
> PCIBus and be pluggable at that level.  There can be a default 
> implementation but it may differ for each PCI bus.

I think this can serve as the default implementation.  Perhaps when a 
concrete user of pluggable dma emerges we can understand the 
requirements better.


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