|
From: | Anthony Liguori |
Subject: | Re: [Qemu-devel] [RFC 1/1] pci-dma-api-v2 |
Date: | Mon, 01 Dec 2008 10:37:58 -0600 |
User-agent: | Thunderbird 2.0.0.17 (X11/20080925) |
Avi Kivity wrote:
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?
I'm not sure it's strictly required but I think it would be a simplificaction.
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.
Yes, but that's orthogonal to what I'm saying here. I'm saying that instead of adding an optional ->aio_readv() member, we should eliminate the ->aio_read() member and replace it with ->aio_readv(). There are only three or four places in the code that implement aio so it's not a big change. It avoids introducing a new API too.
I also think we should have complimentary synchronous vector functions although I'd like to see the synchronous API disappear completely.
+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.
If we're not going to try and fold in the IOMMU/PCI BUS API at this pass, then this is less important. But to implement a proper PCI bus API, I think there has to be function pointers associated with each element of the scatter/gather list that control how data is copied in and out of each element.
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 think if we drop the pci_ prefixes and just treat it as a basic DMA API as Blue Swirl suggested, then this is closer to what we need.
Regards, Anthony Liguori
[Prev in Thread] | Current Thread | [Next in Thread] |