generic device DMA implementation

Currently our only DMA API is highly PCI specific (making any non-pci bus with a DMA controller create fake PCI devices to help it function).

Now that we have the generic device model, it should be equally possible to rephrase the entire API for generic devices instead of pci_devs.

This patch does just that (for x86---although I also have working code for parisc, that's where I actually tested the DMA capability).

The API is substantially the same as the PCI DMA one, with one important exception with regard to consistent memory:

The PCI api has pci_alloc_consistent which allocates only consistent memory and fails the allocation if none is available thus leading to driver writers who might need to function with inconsistent memory to detect this and employ a fallback strategy.

The new DMA API allows a driver to advertise its level of consistent memory compliance to dma_alloc_consistent. There are essentially two levels:

- I only work with consistent memory, fail if I cannot get it, or
- I can work with inconsistent memory, try consistent first but return inconsistent if it's not available.

The idea is that the memory type can be coded into dma_addr_t which the subsequent memory sync operations can use to determine whether wback/invalidate should be a nop or not.

Using this scheme allows me to eliminate all the inconsistent memory fallbacks from my drivers.

Comments welcome.

James

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas:
# ChangeSet 1.926 -> 1.929
# arch/i386/kernel/pci-dma.c 1.8 -> 1.9
# drivers/pci/pci.c 1.50 -> 1.51
# include/asm-i386/pci.h 1.17 -> 1.18

#	 include/linux/pci.h	1.52    -> 1.54   
#	               (new)	        -> 1.3     include/asm-i386/dma-mapping.h
#	               (new)	        -> 1.1     include/asm-generic/dma-mapping.h
#	               (new)	        -> 1.1     include/linux/dma-mapping.h

#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/12/03 [email protected] 1.927
# Implement a generic device based DMA API
#

# implement a DMA mapping API based on generic devices instead of
# PCI ones.
# 
# The API is almost identical to the PCI one except for
# 
# - the dma_alloc_consistent takes a conformance level, so the driver
# may choose to support only consistent or also non-consistent memory.
# --------------------------------------------
# 02/12/03	[email protected]	1.928
# Update to include dma_supported in the API
# --------------------------------------------
# 02/12/03	[email protected]	1.929
# minor fixes to x86 dma implementation
# --------------------------------------------
#
diff -Nru a/arch/i386/kernel/pci-dma.c b/arch/i386/kernel/pci-dma.c
--- a/arch/i386/kernel/pci-dma.c	Wed Dec  4 11:25:59 2002
+++ b/arch/i386/kernel/pci-dma.c	Wed Dec  4 11:25:59 2002
@@ -13,13 +13,17 @@
 #include <linux/pci.h>
 #include <asm/io.h>

-void *pci_alloc_consistent(struct pci_dev *hwdev, size_t size,
- dma_addr_t *dma_handle)
+void *dma_alloc_consistent(struct device *dev, size_t size,
+ dma_addr_t *dma_handle,
+ enum dma_conformance_level level)
{
void *ret;
int gfp = GFP_ATOMIC;

- if (hwdev == NULL || ((u32)hwdev->dma_mask != 0xffffffff))
+ if(level == DMA_CONFORMANCE_NONE)
+ return NULL;
+
+ if (dev == NULL || ((u32)*dev->dma_mask != 0xffffffff))
gfp |= GFP_DMA;
ret = (void *)__get_free_pages(gfp, get_order(size));

@@ -30,7 +34,7 @@
return ret;
}

-void pci_free_consistent(struct pci_dev *hwdev, size_t size, +void dma_free_consistent(struct device *dev, size_t size,
void *vaddr, dma_addr_t dma_handle)
{
free_pages((unsigned long)vaddr, get_order(size));

diff -Nru a/drivers/pci/pci.c b/drivers/pci/pci.c
@@ -680,7 +680,7 @@
 int
 pci_set_dma_mask(struct pci_dev *dev, u64 mask)
 {
-	if (!pci_dma_supported(dev, mask))
+	if (!dma_supported(&dev->dev, mask))
 		return -EIO;
 
 	dev->dma_mask = mask;

diff -Nru a/include/asm-generic/dma-mapping.h b/include/asm-generic/dma-mapping.h
@@ -0,0 +1,29 @@
+#ifndef _ASM_GENERIC_DMA_MAPPING_H
+#define _ASM_GENERIC_DMA_MAPPING_H
+
+int dma_set_mask(struct device *dev, u64 dma_mask);
+void *dma_alloc_consistent(struct device *dev, size_t size,
+			   dma_addr_t *dma_handle,
+			   enum dma_conformance_level level);
+enum dma_conformance_level dma_get_conformance(dma_addr_t dma_handle);
+void dma_free_consistent(struct device *dev, size_t size, void *cpu_addr,
+			  dma_addr_t dma_handle);
+dma_addr_t dma_map_single(struct device *dev, void *cpu_addr, size_t size,
+			   enum dma_data_direction direction);
+void dma_unmap_single(struct device *dev, dma_addr_t dma_addr, size_t size,
+		      enum dma_data_direction direction);
+dma_addr_t dma_map_page(struct device *dev, struct page *page,
+			unsigned long offset, size_t size,
+			enum dma_data_direction direction);
+void dma_unmap_page(struct device *dev, dma_addr_t dma_address, size_t size,
+		    enum dma_data_direction direction);
+int dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
+	       enum dma_data_direction direction);
+void dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nhwentries,
+		  enum dma_data_direction direction);
+void dma_sync_single(struct device *dev, dma_addr_t dma_handle, size_t size,
+		     enum dma_data_direction direction);
+void dma_sync_sg(struct device *dev, struct scatterlist *sg, int nelems,
+		 enum dma_data_direction direction);
+#endif
+

diff -Nru a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
@@ -0,0 +1,32 @@
+#ifndef _ASM_LINUX_DMA_MAPPING_H
+#define _ASM_LINUX_DMA_MAPPING_H
+
+/* This is the level of conformance required for consistent allocation
+ *
+ * DMA_CONFORMANCE_NONE - debugging: always fail consistent allocation
+ * DMA_CONFORMANCE_CONSISTENT - only allocate consistent memory.  Fail
+ *	if consistent memory cannot be allocated.
+ * DMA_CONFORMANCE_NON_CONSISTENT - driver has full writeback/invalidate
+ *	compliance.  Return non-consistent memory if consistent cannot be
+ *	allocated.
+ */
+enum dma_conformance_level {
+	DMA_CONFORMANCE_NONE,
+	DMA_CONFORMANCE_CONSISTENT,
+	DMA_CONFORMANCE_NON_CONSISTENT,
+};
+
+/* These definitions mirror those in pci.h, so they can be used
+ * interchangeably with their PCI_ counterparts */
+enum dma_data_direction {
+	DMA_BIDIRECTIONAL = 0,
+	DMA_TO_DEVICE = 1,
+	DMA_FROM_DEVICE = 2,
+	DMA_NONE = 3,
+};
+
+#include <asm/dma-mapping.h>
+
+#endif
+
+

You should update Documentation/DMA-mapping.txt too :)

Oh, yes, and convert all the other arch's too. That's on my list of things todo (and will be there when I post a patch). I was just throwing out a request for comments to see what turned up.

James

Keep in mind that sometimes the actual _implementation_ is also highly PCI-specific -- that is, what works for PCI devices may not work for other devices and vice-versa.

So perhaps instead of just replacing pci_...' with dma_...', it would be better to add new function pointers to `struct bus_type' for all this stuff (or something like that).

How is the driver supposed to tell whether a given dma_addr_t value represents consistent memory or not? It seems like an (arch-specific) `dma_addr_is_consistent' function is necessary, but I couldn't see one in your patch.

Thanks,

-Miles

We are all lying in the gutter, but some of us are looking at the stars.
-Oscar Wilde

Keep in mind that sometimes the actual _implementation_ is also highly PCI-specific -- that is, what works for PCI devices may not work for other devices and vice-versa.

So perhaps instead of just replacing pci_...' with dma_...', it would be better to add new function pointers to `struct bus_type' for all this stuff (or something like that).

How is the driver supposed to tell whether a given dma_addr_t value represents consistent memory or not? It seems like an (arch-specific) `dma_addr_is_consistent' function is necessary, but I couldn't see one in your patch.

Thanks,

-Miles

We are all lying in the gutter, but some of us are looking at the stars.
-Oscar Wilde

Not really, that can all be taken care of in the platform implementation.

The parisc implementation has exactly that problem. The platform implementation uses the generic device platform_data to cache the iommu accessor methods (it actually finds the iommu by walking up the device parents until it gets to the iommu driver--which means it needs to walk off the PCI bus).

In general, the generic device already has enough information that the platform implementation can be highly bus specific---and, of course, once you know exactly what bus it's on, you can cast it to the bus device if you want.

James

I presume you mean something like (in an arch-specific file somewhere):

void *dma_alloc_consistent (struct device *dev, size_t size,
dma_addr_t *dma_handle, enum dma_conformance_level level)

{
    if (dev->SOME_FIELD == SOME_CONSTANT)
        return my_wierd_ass_pci_alloc_consistent ((struct pci_dev *)dev, ...);
    else
        return 0; /* or kmalloc(...); */
}

?

I did a bit of grovelling, but I'm still not quite sure what test I can do (i.e., what SOME_FIELD and SOME_CONSTANT should be, if it's really that simple).

Ah well, as long as it's possible I guess I'll figure it out when the source hits the fan...

-Miles

I have seen the enemy, and he is us.  -- Pogo

well, the patch was only for x86, which is fully consistent. For parisc, that becomes a field for the dma accessor functions.

However, even on parisc, the (supported) machines are either entirely consistent or entirely inconsistent.

If you have a machine that has both consistent and inconsistent blocks, you need to encode that in dma_addr_t (which is a platform definable type).

The sync functions would just decode the type and either nop or perform the sync.

James

My thinking was that a driver might want to do things like --

  if (dma_addr_is_consistent (some_funky_addr)) {
    do it quickly;
  } else
    do_it_the_slow_way (some_funky_addr);

in other words, something besides just calling the sync functions, in the case where the memory was consistent.

-Miles

Suburbia: where they tear out the trees and then name streets after them.

Yes, but using consistent memory is not necessarily the fast way - in fact it probably won't be. Machines which don't do DMA cache snooping will need to disable caching to get consistent memory, so using consistent memory is very slow - on such a machine explicit syncs are preferable wherever possible.

On a machine which is nicely consistent, the cache "flushes" should become NOPs, so we'd expect the two sides of that if to do the same thing.

David Gibson			| For every complex problem there is a
[email protected]	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson

I don't agree here. Encoding things in dma_addr_t, then special casing in consistent_{map,unmap,sync,....) looks really ugly to me ! You want dma_addr_t to contain a bus address for the given bus you are working with and pass that to your device, period.

Consistency of memory (or simply, in some cases, accessibility of system memory by a given device) is really a property of the bus. Tweaking magic bits in dma_addr_t and testing them later is a hack. The proper implementation is to have the consistent_{alloc,free,map,unmap,sync,...) functions be function pointers in the generic bus structure.

Actually, the device model defines a bus "type" structure rather than a "bus instance" structure (well, at least it did last I looked a couple of weeks ago). That's a problem I beleive here, as those functions are really a property of a given bus instance. One solution would eventually be to have the set of functions pointers in the generic struct device and by default be copied from parent to child.

Actually, to avoid bloat, I think a single pointer to a struct containing the whole set of consistent functions is enough though, as those will typically be statically defined.

Ben.

On an unrelated note, a "bus instance" structure would seem to be required for proper handling of bridges in combination with PCI segments.

Bill

Well, the bus in the generic device model is just a struct device as well.

The parisc implementation I'm working on stores this type of conversion information in the platform_data field of the generic device (although the function pointers that make use of it are global).

I did do an implementation which added a dma_accessors set of functions to the struct device (and also struct bus_type), but I eventually concluded they had to be so platform specific that there was no utility to exposing them.

actually, in parisc, the implementation is simple. The type of memory is determined globally per architecture (so it's not encoded in the dma_addr_t). As Adam said. The preferred platform implementation for a machine that did both (I believe this is the fabled parisc V class, which I've never seen) would be to implement a consistent region so you could tell if dma_addr_t fell in that region for whether it was consistent or not.

I fully recognise that dma_addr_t actually has to be freely convertable to the physical address by simple casting, because that's the way the pci_ functions use it.

James

Do you have an example of where the second option is useful? Off hand the only places I can think of where you'd use a consistent_alloc() rather than map_single() and friends is in cases where the hardware's behaviour means you absolutely positively have to have consistent memory.

David Gibson			| For every complex problem there is a
[email protected]	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson

agreed, good catch. Returning inconsistent memory when you asked for consistent makes not much sense: the programmer either knows what the hardware wants, or the programmer is silly and should not be using alloc_consistent anyway.

Well, it comes from parisc drivers. Here you'd really rather have consistent memory because it's more efficient, but on certain platforms it's just not possible.

In the drivers that do this, it leads to this type of awfulness:

consistent = 1;
if(!mem = pci_alloc_consistent() {
mem = __get_free_pages
mem = pci_map_single()
consistent = 1;
}
....
if(!consistent)
dma_cache_wback()

etc.

The idea is that this translates to

mem = dma_alloc_consistent(... DMA_CONFORMANCE_NON_CONSISTENT)

...

dma_sync_single(mem..)

Where if you have consistent memory then the sync is a nop.

Yes, I think the above makes this point. I'll change the names.

James

Hmm... that doesn't seem sufficient to explain it.

Some background: I work with PPC embedded chips (the 4xx family) whose only way to get consistent memory is by entirely disabling the cache. However in some cases you *have* to have consistent memory despite this very high cost. In all other cases you want to use inconsistent memory (just allocated with kmalloc() or get_free_pages()) and explicit cache flushes.

It seems the "try to get consistent memory, but otherwise give me inconsistent" is only useful on machines which:
(1) Are not fully consisent, BUT
(2) Can get consistent memory without disabling the cache, BUT
(3) Not very much of it, so you might run out.

The point is, there has to be an advantage to using consistent memory if it is available AND the possibility of it not being available.

Otherwise, drivers which absolutely need consistent memory, no matter the cost, should use consistent_alloc(), all other drivers just use kmalloc() (or whatever) then use the DMA flushing functions which compile to NOPs on platforms with consistent memory.

Are there actually any machines with the properties described above? The machines I know about don't:
- x86 and normal PPC are fully consistent, so the question
doesn't arise
- PPC 4xx and 8xx are incconsistent if cached, so you never
want consistent if you don't absolutely need it
- PA Risc is fully non-consistent (I'm told), so the question
doesn't arise.

David Gibson			| For every complex problem there is a
[email protected]	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson

I'm really thinking of this from the driver writer's point of view. The advantage of consistent memory is that you don't have to think about where to place all the sync points (sync points can be really subtle and nasty and an absolute pain---I shudder to recall all of the problems I ran into writing a driver on a fully inconsistent platform).

The advantage here is that you can code the driver only to use consistent memory and not bother with the sync points (whatever the cost of this is). Most platforms support reasonably cheap consistent memory, so most people simply don't want to bother with inconsistent memory if they can avoid it.

If you do the sync points, you can specify the DMA_CONFORMANCE_NON_CONSISTENT level and have the platform choose what type of memory you get. For a platform which makes memory consistent by turning off CPU caching at the page level, it's probably better to return non-consistent memory if the driver can cope with it.

James

But if you have the sync points, you don't need a special allocater for the memory at all - any old RAM will do. So why not just use kmalloc() to get it.

David Gibson			| For every complex problem there is a
[email protected]	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson

Because with kmalloc, you have to be aware of platform implementations. Most notably that cache flush/invalidate instructions only operate at the level of certain block of memory (called the cache line width). If kmalloc returns less than a cache line width you have the potential for severe cockups because of the possibility of interfering cache operations on adjacent kmalloc regions that share the same cache line.

the dma_alloc... function guarantees to avoid this for you by passing the allocation to the platform layer which knows the cache characteristics and requirements for the machine (and dma controller) you're using.

James

Having debugged a stack corruption problem when attempting to use USB on a PPC 4xx machine, which was due to improperly aligned DMA buffers, I am well aware of this issue.

Ok - now I begin to see the point of this: I was being misled by the emphasis on a preference for consistent allocation and the original "alloc_consistent" name you suggested. When consistent memory isn't strictly required it's as likely as not that it won't be preferred either.

Given this, and Miles example, I can see the point of a DMA mallocater that applies DMA constraints that are not to do with consistency. Then consistency could also be specified, but that's a separate issue.

So, to remove the misleading emphasis on the point of the allocated being consistent memory (your name change was a start, this goes further), I'd prefer to see something like:

void *dma_malloc(struct device *bus, unsigned long size, int flags,

		 dma_addr_t *dma_addr);

Which returns virtual and DMA pointers for a chunk of memory satisfying any DMA conditions for the specified bus. Then if flags includes DMA_CONSISTENT (or some such) the memory will be allocated consistent in addition to those constraints.

If DMA_CONSISTENT is not specified, the memory might be consistent, and there would be a preference for consistent only on platforms where consistent memory is actually preferable (I haven't yet heard of one).

David Gibson			| For every complex problem there is a
[email protected]	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson

...

As I mentioned in my previous message, one of my platforms is like that -- PCI consistent memory must be allocated from a special pool of memory, which is only 2 megabytes in size.

-Miles

`Life is a boundless sea of bitterness'

Ok, that starts to make sense then (what platform is it, incidentally). Is using consistent memory actually faster than doing the cache flushes expliticly? Much?

David Gibson			| For every complex problem there is a
[email protected]	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson

It's an embedded evaluation board (Midas RTE-MOTHER-A' and RTE-V850E-MA1-CB').

The thing is there _is_ no cache on this machine (it's very slow), so cache-consistency is actually not an issue (and the cache-flushing macros won't help me at all).

PCI devices are several busses removed from the CPU, and they only have this one 2MB area in common. So on this machine, PCI devices can _only_ use consistent memory.

When a driver uses the non-consistent interfaces, then:

* pci_map_single allocates a `shadow area' of consistent memory and pci_unmap_single deallocates it

* pci_dma_sync_... just does a memcpy to/from the shadow' consistent memory from/to the drivers kalloc'd block (in principle I think this is incorrect, because it uses the dir' parameter to determine the direction to copy, but it works in practice)

So you can see that for this platform, it would be better if drivers could _always_ use alloc_consistent, but many don't.

Yes this is a wierd and frustrating design, but I think it does credit to the linux PCI layer that I could get it work at all, without modifying any drivers! I guess my main goal in this discussion is to ensure that remains the case...

-Miles

.Numeric stability is probably not all that important when you're guessing.

That's a little misleading: all your memory is consistent, the point is that it is a shadow area of PCI-mappable memory.

Ah, ok, now I understand. The issue here is actually nothing to do with consistency, since your platform is fully consistent. The issue is that there are other constraints for DMAable memory and you want drivers to be able to easily mallocate with those constraints.

Actually, it occurs to me that PC ISA DMA is in a similar situation - there is a constraint on DMAable memory (sufficiently low physical address) which has nothing to do with consistency.

Ok... see also my reply to one of James's posts.

David Gibson			| For every complex problem there is a
[email protected]	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson

Well I suppose that's true if you're using the term in a cache-related sense (which I gather is the convention).

OTOH, if you think about it from the view point of the PCI framework, the terminology actually does make sense even in this odd case -- consistent' memory is indeed consistent (both CPU and device see a single image), but other memory used to communicate with the driver is inconsistent' (CPU and device see different things until a sync operation is done).

Indeed. What I'm doing is basically bounce-buffers.

-Miles

`The suburb is an obsolete and contradictory form of human settlement'

Agreed here. Add to this

(4) quite silly from an API taste perspective.

Ug. This is travelling backwards in time.

kmalloc is not intended to allocate memory for DMA'ing. I (and others) didn't spend all that time converting drivers to the PCI DMA API just to see all that work undone.

Note that I am speaking from a driver API perspective, however. If you are talking about using kmalloc "under the hood" while still presenting the same driver interface, that's fine.

Jeff

But if there aren't any consistency constraints on the memory, why not get it with kmalloc(). There are two approaches to handling DMA on a not-fully-consistent machine:
1) Allocate the memory specially so that it is consistent
2) Use any old memory, and make sure we have explicit cache
frobbing.

We have to have both: some hardware requires approach (1), and the structure of the kernel often requires (2) to avoid lots of copying (e.g. a network device doesn't allocate its own skbs to transmit, so it can't assume the memory has any special consistency properties).

Since in case (2), we can't make assumptions about where the memory came from, it might as well come from kmalloc() (or a slab, or get_free_pages() or whatever).

David Gibson			| For every complex problem there is a
[email protected]	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson

For me it's an API issue. kmalloc does not return DMA'able memory.

If "your way" is acceptable to most, then at the very least I would want

	#define get_any_old_dmaable_memory kmalloc

Ok - see my reply to James's post. I see the point of this given that there are constraints on DMAable memory which are not related to consistency (e.g. particular address ranges and cacheline alignment). A mallocater which can satisfy these constraints makes sense to me.

I just think it's a mistake to associate these constraints with cache consistency - they are not related. James's original patch does make this separation in practice, but it misleading suggests a link - which is what confused me.

I imagine platforms where any address is DMAable and which are fully consistent would do this.

David Gibson			| For every complex problem there is a
[email protected]	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson

Looking at our implementation (ppc32 on non-coherent CPUs like 405) of pci_map_single, which just flushes the cache, I still feel we need a consistent_alloc, that is an implementation that _disables_ caching for the area.

A typical example is an USB OHCI driver. You really don't want to play cache tricks with the shared area here. That will happen each time you have a shared area in memory in which both the CPU and the device may read/write in the same cache line.

For things like ring descriptors of a net driver, I feel it's very much simpler (and possibly more efficient too) to also allocate non-cacheable space for consistent instead of continuously flushing/invalidating. Actually, flush/invalidate here can also have nasty side effects if several descriptors fit in the same cache line.

The data buffers, of course (skbuffs typically) would preferably use pci_map_* like APIs (hrm... did we ever make sure skbuffs would _not_ mix the data buffer with control datas in the same cache line ? This have been a problem with non-coherent CPUs in the past).

Ben.

Indeed. Think about a 16-byte descriptor in a 32-byte cache line. The net chip has written status information to the first word, you've just written to the 4th word of that cache line.

To access the status word written by the chip, you need to invalidate (without writeback) that cache line. For the chip to access the word you've just written, you need to writeback that cache line.

In other words, you _will_ loose information in this case, guaranteed. I'd rather keep our existing pci_* API than be forced into this crap again.

Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

No question there: that's James's first option.

Indeed - the 405GP ethernet driver, which I've worked on, uses exactly this approach. consistent_alloc() is used for the descriptor ring buffer, and DMA syncs are used for the data buffers.

David Gibson			| For every complex problem there is a
[email protected]	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson