dma api v3

From: Andrea Arcangeli <[email protected]>

One major limitation for KVM today is the lack of a proper way to write drivers in a way that allows the host OS to use direct DMA to the guest physical memory to avoid any intermediate copy. The only API provided to drivers seems to be the cpu_physical_memory_rw and that enforces all drivers to bounce and trash cpu caches and be memory bound. This new DMA API instead allows drivers to use a pci_dma_sg method for SG I/O that will translate the guest physical addresses to host virutal addresses and it will call two operation, one is a submit method and one is the complete method. The pci_dma_sg may have to bounce buffer internally and to limit the max bounce size it may have to submit I/O in pieces with multiple submit calls.

All we care about is the performance of the direct path, so I tried to avoid dynamic allocations there to avoid entering glibc.

Signed-off-by Andrea Arcangeli <[email protected]>

+ * QEMU PCI DMA operations

+typedef struct QEMUPciDmaSgParam {

 +    QEMUPciDmaSgSubmit *pci_dma_sg_submit;
 +    QEMUPciDmaSgComplete *pci_dma_sg_complete;
 +    void *pci_dma_sg_opaque;

+ QEMUPciDmaSg *sg;

+ struct QEMUPciDmaSgParam *next;

+} QEMUPciDmaSgParam;

From: Andrea Arcangeli <[email protected]>

Add can_dma and post_dma methods needed before/after direct IO to guest physical memory.

Signed-off-by Andrea Arcangeli <[email protected]>

+ /* nonlinear range */
+ if (pd_first != pd)
+ return NULL;

From: Andrea Arcangeli <[email protected]>

Be consistent and have length be size_t for all methods.

From: Andrea Arcangeli <[email protected]>

Add can_dma and post_dma methods needed before/after direct IO to guest physical memory.

Quoted 7 lines · Andrea Arcangeli

From: Andrea Arcangeli <[email protected]>

Add can_dma and post_dma methods needed before/after direct IO to guest physical memory.

Signed-off-by Andrea Arcangeli <[email protected]>

Quoted 4 lines

+ /* nonlinear range */
+ if (pd_first != pd)
+ return NULL;

In my tests on Sparc32, IOMMU can map a linear DVMA range to several non-linear physical pages, so this case should be handled correctly.

Quoted 4 lines · Andrea Arcangeli

From: Andrea Arcangeli <[email protected]>

Be consistent and have length be size_t for all methods.

ram_addr_t would be better than size_t here.

1) You attempt to map a physical address. This effectively is a lock or pin operation.

a) In the process of this, you get a virtual address that you can
manipulate.

2) You do your IO to the virtual address

3) You indicate how much of the memory you have dirtied

4) You unmap or unlock that memory region. The virtual address is now no longer valid.

This could correspond to a:

void *cpu_physical_memory_map(target_phys_addr_t addr, ram_addr_t size, int is_write);

void cpu_physical_memory_unmap(target_physical_addr_t addr, ram_addr_t size, void *mapping, int is_dirty);

Quoted 5 lines · Andrea Arcangeli

From: Andrea Arcangeli <[email protected]>

Add can_dma and post_dma methods needed before/after direct IO to guest physical memory.

I think any API based on a can_dma abstraction is wrong. The write_post_dma thing is equally wrong.

The concept of "dma" that you're introducing is not correct.

The DMA API should have the following properties:

1) You attempt to map a physical address. This effectively is a lock or pin operation.
a) In the process of this, you get a virtual address that you can
manipulate.
2) You do your IO to the virtual address
3) You indicate how much of the memory you have dirtied
4) You unmap or unlock that memory region. The virtual address is now no longer valid.

This could correspond to a:

void *cpu_physical_memory_map(target_phys_addr_t addr, ram_addr_t size, int is_write);

void cpu_physical_memory_unmap(target_physical_addr_t addr, ram_addr_t size, void *mapping, int is_dirty);

Quoted 20 lines · Blue Swirl

Quoted 16 lines · Andrea Arcangeli

From: Andrea Arcangeli <[email protected]>

Add can_dma and post_dma methods needed before/after direct IO to guest physical memory.

Signed-off-by Andrea Arcangeli <[email protected]>

+ /* nonlinear range */
+ if (pd_first != pd)
+ return NULL;

Quoted 4 lines

In my tests on Sparc32, IOMMU can map a linear DVMA range to several non-linear physical pages, so this case should be handled correctly.

I think we should go back to Fabrice's earliest suggestion here. We should
just have a simple map/unmap lock/unlock API for physical memory. That should be the base API IMHO.

As long the map function goes from guest physical => host virtual, it can
work for everything we care about.

This is orthogonal to an API dealing with scatter/gather lists and
translation to and from them.

Quoted 2 lines · Anthony Liguori

1) You attempt to map a physical address. This effectively is a lock or pin operation.

lock or pin for what? There's nothing to pin or lock here. Perhaps one day we'll have to lock or pin against something, dunno, then we'll add whatever pin or lock, otherwise why don't we also thread qcow2 while we're at it, sounds more worth it than adding a lock or pin that isn't actually needed.

Quoted 3 lines

a) In the process of this, you get a virtual address that you can
manipulate.

That's what can_dma does. If it can, it generates a dma address backed by ram it does. But if it can't, it won't. This only works for direct I/O.

Quoted 4 lines · Anthony Liguori

I think we should go back to Fabrice's earliest suggestion here. We should
just have a simple map/unmap lock/unlock API for physical memory. That should be the base API IMHO.

Beautiful!

Quoted 6 lines

As long the map function goes from guest physical => host virtual, it can
work for everything we care about.

This is orthogonal to an API dealing with scatter/gather lists and
translation to and from them.

So again we tried to solve too many problems at once.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 2 lines · Anthony Liguori

1) You attempt to map a physical address. This effectively is a lock or pin operation.

lock or pin for what? There's nothing to pin or lock here.

Quoted 2 lines

4) You unmap or unlock that memory region. The virtual address is now no longer valid.

Again not needed.

Quoted 14 lines

I think any API based on a can_dma abstraction is wrong. The write_post_dma thing is equally wrong.

The concept of "dma" that you're introducing is not correct.

The DMA API should have the following properties:

1) You attempt to map a physical address. This effectively is a lock or pin operation.
a) In the process of this, you get a virtual address that you can
manipulate.
2) You do your IO to the virtual address
3) You indicate how much of the memory you have dirtied
4) You unmap or unlock that memory region. The virtual address is now no longer valid.

Quoted 5 lines

This could correspond to a:

void *cpu_physical_memory_map(target_phys_addr_t addr, ram_addr_t size, int is_write);

void cpu_physical_memory_unmap(target_physical_addr_t addr, ram_addr_t size, void *mapping, int is_dirty);

Let me clarify this a bit more. The problem we're trying to address today is the encapsulating knowledge of phys_ram_base. We want to minimize the amount of code that makes any assumptions about phys_ram_base. Your current API still accesses phys_ram_base directly in the PCI DMA API. The only real improvement compared to the current virtio code is that you properly handle MMIO. This is not just about layout but this also includes the fact that in the future, guest memory could be discontiguous in QEMU (think memory hotplug).

void *cpu_physical_memory_map(target_phys_addr_t addr, ram_addr_t size, int is_write);

Quoted 18 lines · Anthony Liguori

Quoted 18 lines

I think any API based on a can_dma abstraction is wrong. The write_post_dma thing is equally wrong.

The concept of "dma" that you're introducing is not correct.

The DMA API should have the following properties:

1) You attempt to map a physical address. This effectively is a lock or pin operation.
a) In the process of this, you get a virtual address that you can
manipulate.
2) You do your IO to the virtual address
3) You indicate how much of the memory you have dirtied
4) You unmap or unlock that memory region. The virtual address is now no longer valid.

This could correspond to a:

void *cpu_physical_memory_map(target_phys_addr_t addr, ram_addr_t size, int is_write);

Quoted 3 lines

Quoted 1 line

void cpu_physical_memory_unmap(target_physical_addr_t addr, ram_addr_t size, void *mapping, int is_dirty);

Let me clarify this a bit more. The problem we're trying to address today is the encapsulating knowledge of phys_ram_base. We want to minimize the amount of code that makes any assumptions about phys_ram_base. Your current API still accesses phys_ram_base directly in the PCI DMA API. The only real improvement compared to the current virtio code is that you properly handle MMIO. This is not just about layout but this also includes the fact that in the future, guest memory could be discontiguous in QEMU (think memory hotplug).

There are two more problems the dma api solves:

- DMA into mmio regions; this requires bouncing

- DMA with an associated transform (xor, byteswap); also requires bouncing

In turn, bouncing requires splitting large requests to avoid unbounded memory allocation.

While I think _map/_unmap is an improvement over can_dma(), this API can't handle bounded bouncing, and so a separate layer (dma.c) is still necessary.

Quoted 2 lines · Anthony Liguori

void *cpu_physical_memory_map(target_phys_addr_t addr, ram_addr_t size, int is_write);

Just a side note (doesn't mean I agree with the above), it doesn't make sense to use an ram_addr_t size when you return a 32bit address on 32bit qemu build.

Quoted 10 lines · Andrea Arcangeli

Quoted 3 lines · Anthony Liguori

void *cpu_physical_memory_map(target_phys_addr_t addr, ram_addr_t size, int is_write);

Just a side note (doesn't mean I agree with the above), it doesn't make sense to use an ram_addr_t size when you return a 32bit address on 32bit qemu build.

size_t is completely wrong for 64-bit targets on 32-bit hosts. ram_addr_t is the type we use for guest ram size. It's 64-bit all of the time simply because it's easier to do that and we decided that the little bit of wasted space/computations were not a problem.

Quoted 12 lines

Quoted 10 lines · Andrea Arcangeli

Quoted 3 lines · Anthony Liguori

void *cpu_physical_memory_map(target_phys_addr_t addr, ram_addr_t size, int is_write);

Just a side note (doesn't mean I agree with the above), it doesn't make sense to use an ram_addr_t size when you return a 32bit address on 32bit qemu build.

Quoted 4 lines

size_t is completely wrong for 64-bit targets on 32-bit hosts. ram_addr_t is the type we use for guest ram size. It's 64-bit all of the time simply because it's easier to do that and we decided that the little bit of wasted space/computations were not a problem.

Not sure why you think I'm suggesting you to use size_t. I'm just trying to tell you that if you insist in this 64bit-guest-on-32bit-host-is-dead-and-obsolete-to-support (i.e. if you pass a ram_addr_t size to cpu_physical_memory_map) you've at least to return ram_addr_t too). 'void *' is like size_t so the above API getting ram_addr_t length and returning 'void *', can't possibly be sane.

Quoted 2 lines

- DMA into mmio regions; this requires bouncing

The map() API I proposed above should do bouncing to MMIO regions. To deal with unbounded allocation, you can simply fail when the mapping allocation has reached some high limit. Calling code needs to cope with the fact that map'ing may succeed or fail.

Quoted 3 lines

- DMA with an associated transform (xor, byteswap); also requires bouncing

At this layer of the API, this is unnecessary. At the PCI layer, you would need to handle this and I'd suggest taking the same approach as above.

Quoted 7 lines

In turn, bouncing requires splitting large requests to avoid unbounded memory allocation.

While I think _map/_unmap is an improvement over can_dma(), this API can't handle bounded bouncing, and so a separate layer (dma.c) is still necessary.

I think it can handled bounded bouncing fine. It's a matter of ensuring the bounce buffer max is sufficiently large and ensuring that any client code can cope with map failures. In fact, there probably needs to be a notifiers API that is invoked whenever an unmap() happens so that if an asynchronous request is waiting because of a map failure, it can be notified as to when it should try again.

Quoted 7 lines · Anthony Liguori

Quoted 2 lines

- DMA into mmio regions; this requires bouncing

The map() API I proposed above should do bouncing to MMIO regions. To deal with unbounded allocation, you can simply fail when the mapping allocation has reached some high limit. Calling code needs to cope with the fact that map'ing may succeed or fail.

There are N users of this code, all of which would need to cope with the failure. Or there could be one user (dma.c) which handles the failure and the bouncing.

dma.c _is_ a map/unmap api, except it doesn't expose the mapped data, which allows it to control scheduling as well as be easier to use.

Right, but who would it notify?

We need some place that can deal with this, and it isn't _map()/_unmap(), and it isn't ide.c or scsi.c.

On Sat, Dec 13, 2008 at 10:46:49AM -0600, Anthony Liguori wrote:

Not sure why you think I'm suggesting you to use size_t. I'm just trying to tell you that if you insist in this 64bit-guest-on-32bit-host-is-dead-and-obsolete-to-support (i.e. if you pass a ram_addr_t size to cpu_physical_memory_map) you've at least to return ram_addr_t too). 'void *' is like size_t so the above API getting ram_addr_t length and returning 'void *', can't possibly be sane.

This could correspond to a:

void *cpu_physical_memory_map(target_phys_addr_t addr, ram_addr_t size, int is_write);

void cpu_physical_memory_unmap(target_physical_addr_t addr, ram_addr_t size, void *mapping, int is_dirty);

The whole dma.c thing should not exist. If we're going to introduce a higher level API, it should be a PCI DMA API.

Something like virtio could use this API directly seeing as it doesn't really live on a PCI bus in real life.

Regards,

Anthony Liguori

Quoted 5 lines

There are N users of this code, all of which would need to cope with the failure. Or there could be one user (dma.c) which handles the failure and the bouncing.

N should be small long term. It should only be for places that would interact directly with CPU memory. This would be the PCI BUS, the ISA BUS, some speciality devices, and possibly virtio (although you could argue it should go through the PCI BUS).

map() has to fail and that has nothing to do with bouncing or not bouncing. In the case of Xen, you can have a guest that has 8GB of memory, and you only have 2GB of virtual address space. If you try to DMA to more than 2GB of memory, there will be a failure. Whoever is accessing memory directly in this fashion needs to cope with that.

Quoted 4 lines

dma.c _is_ a map/unmap api, except it doesn't expose the mapped data, which allows it to control scheduling as well as be easier to use.

As I understand dma.c, it performs the following action: map() as much as possible, call an actor on mapped memory, repeat until done, signal completion.

As an abstraction, it may be useful. I would argue that it should be a bit more generic though. It should take a function pointer for map and unmap too, and then you wouldn't need N versions of it for each different type of API.

Quoted 6 lines

Right, but who would it notify?

We need some place that can deal with this, and it isn't _map()/_unmap(), and it isn't ide.c or scsi.c.

The pattern of try to map(), do IO, unmap(), repeat only really works for block IO. It doesn't really work for network traffic. You have to map the entire packet and send it all at once. You cannot accept a partial mapping result. The IO pattern to send an IO packet is much simpler: try to map the packet, if the mapping fails, either wait until more space frees up or drop the packet. For the other uses of direct memory access, like kernel loading, the same is true.

What this is describing is not a DMA API. It's a very specific IO pattern. I think that's part of what's causing confusion in this series. It's certainly not at all related to PCI DMA.

I would argue, that you really want to add a block driver interface that takes the necessary information, and implements this pattern but that's not important. Reducing code duplication is a good thing so however it ends up working out is fine.

Quoted 6 lines · Anthony Liguori

This could correspond to a:

void *cpu_physical_memory_map(target_phys_addr_t addr, ram_addr_t size, int is_write);

void cpu_physical_memory_unmap(target_physical_addr_t addr, ram_addr_t size, void *mapping, int is_dirty);

A really nice touch here, and note this is optional and can be a follow up series later, would be to use the mapping itself to encode the physical address and size so the signatures were:

void *cpu_physical_memory_map(target_phys_addr_t addr, ram_addr_t size, int flags); void cpu_physical_memory_unmap(void *mapping);

flags could be PHYS_MAP_READ and/or PHYS_MAP_WRITE.

In unmap, you could check to see if the address is in phys_ram_base ... phys_ram_size. If so, you can get the physical address.

If you maintained a list of mappings, you could then search the list of mappings based on the physical address and check the flags to see if a flush was required.

cause an overflow. You will naturally validate this in the map() function because you cannot map something that is greater than can fit in a void *.

To me map/unmap looks backwards. There's absolutely no point at all to pretend that RAM isn't always mapped.

The whole concept of having to map something is flawed, there's nothing to map. At most you've to take a read lock to prevent future memory hotplug to change the memory layout from under you, but the concept of mapping has nothing to do with that. RAM is always mapped, and mmio has to be emulated anyway so it's worthless to map it.

Actually, with Xen, RAM may be unmapped due do Xen limitations when qemu runs on dom0 mode. But I think map/unmap makes sense even disregarding

Xen: if we add memory hotunplug, we need to make sure we don't unplug memory that has pending dma operations on it. map/unmap gives us the opportunity to refcount memory slots.

We can't get all dma to stop during hotunplug, since net rx operations are long-running (infinite if there is no activity on the link).

IMO, we do want map/unmap, but this would be just a rename of can_dma and friends, and wouldn't have at this time any additional functionality. Bouncing has to happen where we have the ability to schedule the actual operation, and that's clearly not map/unmap.

Let me clarify this a bit more. The problem we're trying to address today is the encapsulating knowledge of phys_ram_base. We want to minimize the amount of code that makes any assumptions about phys_ram_base. Your current API still accesses phys_ram_base directly in the PCI DMA API. The only real improvement compared to the current virtio code is that you properly handle MMIO. This is not just about layout but this also includes the fact that in the future, guest memory could be discontiguous in QEMU (think memory hotplug).

Quoted 7 lines · Anthony Liguori

Quoted 5 lines

There are N users of this code, all of which would need to cope with the failure. Or there could be one user (dma.c) which handles the failure and the bouncing.

N should be small long term. It should only be for places that would interact directly with CPU memory. This would be the PCI BUS, the ISA BUS, some speciality devices, and possibly virtio (although you could argue it should go through the PCI BUS).

Fine, then let's rename it pci-dma.c.

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).

Quoted 2 lines · Avi Kivity

Actually, with Xen, RAM may be unmapped due do Xen limitations when qemu runs on dom0 mode. But I think map/unmap makes sense even disregarding

I realize xen 32bit has issues... Qemu/KVM 32bit also has the same issues but there's no point in 2009 (that's when this stuff could go productive) in trying to run guests with >2G of ram on a 32bit host. The issues emerges (I guess with xen too) in trying to run those obsolete hardware configurations. Even the atom and extremely low power athlon have 64bit capability, and on embedded that runs a real 32bit cpu I can't see how somebody would want to run a >2G guest.

Quoted 2 lines

Xen: if we add memory hotunplug, we need to make sure we don't unplug memory that has pending dma operations on it. map/unmap gives us the opportunity to refcount memory slots.

So memory hotunplug here is considered differently than the real memory hotplug emulation that simulates removing dimm on the hardware. This is just the xen trick to handle >4G guest on a 32bit address space? Well that's just the thing I'm not interested to support. When 64bit wasn't mainstream it made some sense, these days it's good enough if we can boot any guest OS (including 64bit ones) on a 32bit build, but trying to run guests OS with >2G of ram doesn't look useful.

Quoted 4 lines

We can't get all dma to stop during hotunplug, since net rx operations are long-running (infinite if there is no activity on the link).

IMO, we do want map/unmap, but this would be just a rename of can_dma and friends, and wouldn't have at this time any additional functionality. Bouncing has to happen where we have the ability to schedule the actual operation, and that's clearly not map/unmap.

It would be a bit more than a rename, also keep in mind that in the longer term as said we need to build the iovec in the exec.c path, it's not enough to return a void *, I like to support a not 1:1 flat space to avoid wasting host virtual address space with guest memory holes. But that's about it, guest memory has to be always mapped, just not with a 1:1 mapping, and surely not with a per-page array that translates each page physical address to a host virtual address, but with ranges. So this map thing that returns a 'void *' won't be there for long even if I rename.

Quoted 3 lines · Anthony Liguori

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.

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.

Quoted 4 lines

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.

Quoted 5 lines

Quoted 5 lines

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.

Quoted 6 lines

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.

I Wouldn't be surprised if some machines had a large memory mapped IO space. Most of it might not be actively used, but once you start considering 64-bit machines on 32-bit hosts these allocations would become problematic.

Quoted 3 lines

I Wouldn't be surprised if some machines had a large memory mapped IO space. Most of it might not be actively used, but once you start considering 64-bit machines on 32-bit hosts these allocations would become problematic.

At some level, any DMA API would fall apart here. Certain IO operations simply cannot be split (such as network send). If you pass don't pass a flag to the map function that says you can handle partial results, you must cope with map() returning NULL (by dropping the packet).

Quoted 3 lines · Avi Kivity

Quoted 1 line · Anthony Liguori

I've thought quite a bit about it, and I'm becoming less convinced that

this sort of API is going to be helpful.

Quoted 2 lines

Quoted 2 lines

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).

Quoted 3 lines

Quoted 1 line

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.

Quoted 1 line

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.

Quoted 3 lines

I'll enumerate the functions that dma.c provides:
- convert guest physical addresses to host virtual addresses

The map() api does this.

Quoted 2 lines

- construct an iovec for scatter/gather

The map() api does not do this.

Quoted 1 line

- handle guest physical addresses for which no host virtual addresses

exist, while controlling memory use

Quoted 2 lines

- 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)

Quoted 1 line

The map() api does all of this.

Quoted 1 line

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.

Quoted 1 line

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.

Quoted 6 lines

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)

I think you could approach the bouncing via a map/unmap API but I'm not sure. You would need a map() function to take a virtual address which is sort of weird. That would allow you to stack them in an arbitrary fashion though.

Quoted 5 lines

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.

I think that's reasonable.

Quoted 5 lines

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.

Yes. I think this is key. The only observation I would make is that the resolution API should have some sort of release function (so map/unmap, lock/unlock, whatever).

Quoted 8 lines · Blue Swirl

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.

It is a good cleanup.

Quoted 4 lines

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)

Quoted 2 lines

I think you could approach the bouncing via a map/unmap API but I'm not
sure. You would need a map() function to take a virtual address which is sort of weird. That would allow you to stack them in an arbitrary fashion though.

Quoted 6 lines

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.

I think that's reasonable.

Quoted 5 lines

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.

Yes. I think this is key. The only observation I would make is that the
resolution API should have some sort of release function (so map/unmap, lock/unlock, whatever).

The generic resolving API should look something like

int (*resolve)(target_phys_addr_t address_in, target_phys_addr_t length_in, target_phys_addr_t &address_out, target_phys_addr_t &length_out)

Quoted 5 lines

The generic resolving API should look something like

int (*resolve)(target_phys_addr_t address_in, target_phys_addr_t length_in, target_phys_addr_t &address_out, target_phys_addr_t &length_out)

I don't think this is sufficient. A paged iommu may split a single range into
multiple disjoint sections. i.e. we need SG lists.

Quoted 5 lines · Anthony Liguori

Quoted 5 lines

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)

Quoted 4 lines

Quoted 2 lines

I think you could approach the bouncing via a map/unmap API but I'm not

sure. You would need a map() function to take a virtual address which is sort of weird. That would allow you to stack them in an arbitrary fashion though.

Quoted 2 lines

I think that's broken. iommus converts physical addresses to physical
addresses (or bus addresses), possibly generating faults along the way, and depending on the iommu context. map()/unmap() converts physical/bus addresses to virtual addresses, possibly bouncing. Except for both doing conversions, they're very different.

Quoted 8 lines

Quoted 7 lines

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.

Quoted 2 lines

I think that's reasonable.

I don't understand. It's not a device that needs bouncing, it's a
particular transfer. This could be either due to the transfer targeting mmio, or due to the transfer requiring a transformation.

Quoted 4 lines

I don't understand. It's not a device that needs bouncing, it's a
particular transfer. This could be either due to the transfer targeting mmio, or due to the transfer requiring a transformation.

Should the bouncing be something more much complex, for example negotiated between the devices? Or maybe the devices set up a transforming and non-transforming channel (which the other end should be able to transform some more) and use them as needed?

Quoted 10 lines · Blue Swirl

Quoted 4 lines

I don't understand. It's not a device that needs bouncing, it's a
particular transfer. This could be either due to the transfer targeting mmio, or due to the transfer requiring a transformation.

Should the bouncing be something more much complex, for example negotiated between the devices? Or maybe the devices set up a transforming and non-transforming channel (which the other end should be able to transform some more) and use them as needed?

Yes. We already have two cases:

- may do partial request: useful for block storage where requests can be huge
- need full request: networking, where you can't send or receive half a packet; on the other hand, packets are small (even with tso)

You're adding a third case, always bounce, when the data undergoes a transformation which can't happen in-place.

Quoted 5 lines · Avi Kivity

Quoted 3 lines · Blue Swirl

Quoted 3 lines

I don't understand. It's not a device that needs bouncing, it's a
particular transfer. This could be either due to the transfer

targeting

Quoted 11 lines

Quoted 10 lines

Quoted 3 lines

mmio, or due to the transfer requiring a transformation.

Should the bouncing be something more much complex, for example negotiated between the devices? Or maybe the devices set up a transforming and non-transforming channel (which the other end should be able to transform some more) and use them as needed?

Quoted 3 lines

Yes. We already have two cases:

- may do partial request: useful for block storage where requests can be

huge

Quoted 1 line

- need full request: networking, where you can't send or receive half a

packet; on the other hand, packets are small (even with tso)

Quoted 1 line

Quoted 1 line

You're adding a third case, always bounce, when the data undergoes a

transformation which can't happen in-place.

Quoted 1 line

IMHO, IOMMU translation is distinct from mapping/data transformation. I
would think the IOMMU translation API would be different in that it took a physical address and returned a physical address.

The IOMMU DMA API (which could transform data potentially) should return a
virtual address and data a physical address. In the normal case (non-transforming IOMMU), it should just fall-through to CPU DMA API (which is just map/unmap).

The PCI DMA API would normally fall through to the IOMMU DMA API.

So we would have:

PCI DMA map(addr):
if IOMMU:
return IOMMU DMA map(addr)
return CPU DMA map(addr)

IOMMU DMA map(addr):
new_address = IOMMU translate(addr)
if transform:
return IOMMU byte-swap map(addr)
return CPU DMA map(addr)

Newer Xen shouldn't have this problem though; it runs qemu in kernel mode in a dedicated 64-bit domain.

Avi Kivity writes ("[Qemu-devel] Re: [PATCH 2 of 5] add can_dma/post_dma for direct IO"):

Quoted 2 lines

Newer Xen shouldn't have this problem though; it runs qemu in kernel mode in a dedicated 64-bit domain.

I think there is continued value in being able to emulate a guest whose physical address space exceeds the virtual address space in the host. The whole assumption that all guest accessible RAM is mapped at once, contiguously, is I think wrong.

Avi Kivity writes ("[Qemu-devel] Re: [PATCH 2 of 5] add can_dma/post_dma for direct IO"):

Quoted 2 lines

Newer Xen shouldn't have this problem though; it runs qemu in kernel mode in a dedicated 64-bit domain.

I think there is continued value in being able to emulate a guest whose physical address space exceeds the virtual address space in the host. The whole assumption that all guest accessible RAM is mapped at once, contiguously, is I think wrong.

Quoted 11 lines · Ian Jackson

Avi Kivity writes ("[Qemu-devel] Re: [PATCH 2 of 5] add can_dma/post_dma for direct IO"):

Quoted 3 lines

Newer Xen shouldn't have this problem though; it runs qemu in kernel mode in a dedicated 64-bit domain.

I think there is continued value in being able to emulate a guest whose physical address space exceeds the virtual address space in the host. The whole assumption that all guest accessible RAM is mapped at once, contiguously, is I think wrong.

I agree that we shouldn't expect memory to be contiguous, in order to properly support hotplug. But I see zero value in trying to support large memory configurations on 32-bit in 2008. This is what 64-bit systems are for! If Xen has a problem with 64-bit hosts, we can try to accommodate it, but to have 32-bit qemu/tcg

or qemu/kvm support large address spaces is pointless IMO.

Running x86-64 binaries on a (non-x86) 32-bit host is IMHO quite an obvious application for qemu/tcg.

Interestingly, Virtualbox just started to support
64-bit-target-on-32-bit-host.

Quoted 2 lines

I agree that we shouldn't expect memory to be contiguous, in order to properly support hotplug. But I see zero value in trying to support large memory configurations on 32-bit in 2008. This is what 64-bit systems are for! If Xen has a problem with 64-bit hosts, we can try to accommodate it, but to have 32-bit qemu/tcg

Running x86-64 binaries on a (non-x86) 32-bit host is IMHO quite an obvious application for qemu/tcg.

Quoted 2 lines

or qemu/kvm support large address spaces is pointless IMO.

Interestingly, Virtualbox just started to support
64-bit-target-on-32-bit-host.

Quoted 4 lines · Ian Jackson

I think there is continued value in being able to emulate a guest whose physical address space exceeds the virtual address space in the host. The whole assumption that all guest accessible RAM is mapped at once, contiguously, is I think wrong.

If Xen has a problem with 64-bit hosts, we can try to accommodate it,

but to have 32-bit qemu/tcg or qemu/kvm support large address spaces is pointless IMO.