[RFC] Implement dma_boundary in the generic device

[RFC] Implement dma_boundary in the generic device

[James Bottomley]

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

Problem:

Currently, the block layer assumes the queue dma_boundary represents a
fundamental limit of the system which the IOMMU implicitly observes (it
was really set up for the DAC 4GB boundary).  However, it can usefully
be used by devices, like IDE, that have a DMA boundary as part of their
design (For IDE, all DMA's may not cross a 64k boundary).  This would
allow the elimination of the IDE reprocessing of the sg list after
pci_map_sg.

Solution:

Add dma_boundary to the generic device so that if the driver needs to
set a limit different from the fundamental system one, it can be relayed
to the IOMMU to respect.

I've attached two patches: one is the generic implementation, which is
simple (including updating SCSI to use it), and the second is an
illustrative platform implementation of the limit in the parisc CCIO
IOMMU.  The PA implementation turns out to be quite simple, since we
have a huge IOMMU address space (1GB) and we always allocate on a power
of 2 boundary.  However, I can see implementations that simply find a
covering free region having difficulty (since you can end up with an
extra unexpected segment because of alignment), so I think the rule for
them will have to be that if the total sg length is greater than the
dma_boundary, then the transaction *must* begin aligned on the
dma_boundary.

I tested this out by artificially lowering the dma_boundary of my root
device to 8k on a parisc C360.

James


[-- Attachment #2: dma_boundary_generic.diff --]
[-- Type: text/plain, Size: 2356 bytes --]

===== Documentation/DMA-API.txt 1.5 vs edited =====
--- 1.5/Documentation/DMA-API.txt	Wed Feb  4 08:01:19 2004
+++ edited/Documentation/DMA-API.txt	Sun Feb 29 09:12:13 2004
@@ -410,3 +410,12 @@
 boundaries when doing this.
 
 
+void
+dma_set_boundary(struct device *dev, u64 boundary)
+
+Set the device boundary for a DMA transfer.  This only applies to
+platforms which can do virtual merging (i.e. have an IOMMU).  It sets
+a boundary across which a merge must not be done.   i.e. for IDE,
+which has a 64k boundary, the IOMMU may not merge two adjacent 64k
+regions into one contiguous 128k region.  This parameter represents a
+fundamental DMA transfer limit of the device.
===== drivers/base/core.c 1.81 vs edited =====
--- 1.81/drivers/base/core.c	Mon Feb  9 17:40:25 2004
+++ edited/drivers/base/core.c	Sat Feb 28 18:53:07 2004
@@ -195,6 +195,8 @@
 	INIT_LIST_HEAD(&dev->driver_list);
 	INIT_LIST_HEAD(&dev->bus_list);
 	INIT_LIST_HEAD(&dev->dma_pools);
+	/* default dma boundary to 4GB */
+	dev->dma_boundary = 0xFFFFFFFFULL;
 }
 
 /**
===== drivers/scsi/hosts.c 1.96 vs edited =====
--- 1.96/drivers/scsi/hosts.c	Mon Dec 29 15:38:10 2003
+++ edited/drivers/scsi/hosts.c	Sun Feb 29 09:08:09 2004
@@ -111,6 +111,9 @@
 	if (!shost->shost_gendev.parent)
 		shost->shost_gendev.parent = dev ? dev : &platform_bus;
 
+	if(dev)
+		dma_set_boundary(dev, shost->dma_boundary);
+
 	error = device_add(&shost->shost_gendev);
 	if (error)
 		goto out;
===== include/linux/device.h 1.115 vs edited =====
--- 1.115/include/linux/device.h	Mon Feb  9 17:40:25 2004
+++ edited/include/linux/device.h	Sat Feb 28 18:51:34 2004
@@ -285,6 +285,7 @@
 					   detached from its driver. */
 
 	u64		*dma_mask;	/* dma mask (if dma'able device) */
+	u64		dma_boundary;	/* The default dma boundary */
 	struct list_head	dma_pools;	/* dma pools (if dma'ble) */
 
 	void	(*release)(struct device * dev);
===== include/linux/dma-mapping.h 1.1 vs edited =====
--- 1.1/include/linux/dma-mapping.h	Sat Dec 21 22:37:05 2002
+++ edited/include/linux/dma-mapping.h	Sun Feb 29 09:12:02 2004
@@ -12,6 +12,15 @@
 
 #include <asm/dma-mapping.h>
 
+#ifndef ARCH_HAS_DMA_SET_BOUNDARY
+static inline void
+dma_set_boundary(struct device *dev, u64 boundary)
+{
+	dev->dma_boundary = boundary;
+}
+#endif
+
+
 #endif
 
 

[-- Attachment #3: dma_boundary_ccio.diff --]
[-- Type: text/plain, Size: 2089 bytes --]

===== drivers/parisc/ccio-dma.c 1.15 vs edited =====
--- 1.15/drivers/parisc/ccio-dma.c	Tue Feb  3 23:42:34 2004
+++ edited/drivers/parisc/ccio-dma.c	Sun Feb 29 09:15:55 2004
@@ -944,7 +944,8 @@
 */
 
 static CCIO_INLINE int
-ccio_coalesce_chunks(struct ioc *ioc, struct scatterlist *startsg, int nents)
+ccio_coalesce_chunks(struct device *dev, struct ioc *ioc,
+		     struct scatterlist *startsg, int nents)
 {
 	struct scatterlist *vcontig_sg;    /* VCONTIG chunk head */
 	unsigned long vcontig_len;         /* len of VCONTIG chunk */
@@ -991,6 +992,17 @@
 				   IOVP_SIZE) > DMA_CHUNK_SIZE)
 				break;
 
+			/* next check to see if this will coalesce
+			 * over the dma boundary.  This is very simple
+			 * for us, since all allocations begin on a
+			 * power of two boundary, just check that the
+			 * length doesn't go over the dma_boundary
+			 * mask */
+			if(dma_len + dma_offset + startsg->length > (dev->dma_boundary + 1)) {
+				printk("CCIO DMA: refusing to coalesce %lx and %lx\n", dma_len + dma_offset, dma_len + dma_offset + startsg->length);
+				break;
+			}
+
 			/*
 			** Append the next transaction?
 			*/
@@ -1082,7 +1094,7 @@
 	** w/o this association, we wouldn't have coherent DMA!
 	** Access to the virtual address is what forces a two pass algorithm.
 	*/
-	coalesced = ccio_coalesce_chunks(ioc, sglist, nents);
+	coalesced = ccio_coalesce_chunks(dev, ioc, sglist, nents);
 
 	/*
 	** Program the I/O Pdir
@@ -1098,6 +1110,14 @@
 
 	ASSERT(coalesced == filled);
 	DBG_RUN_SG("%s() DONE %d mappings\n", __FUNCTION__, filled);
+
+	/* JEJB DEBUG */
+	for(nents = 0; nents < filled; nents++) {
+		if((sg_dma_address(&sglist[nents]) & ~dev->dma_boundary)
+		   != ((sg_dma_address(&sglist[nents]) + sg_dma_len(&sglist[nents]) - 1) & ~dev->dma_boundary))
+			printk(KERN_ERR "CCIO ERROR, sglist entry start %lx and end %lx cross dma boundary %lx\n", sg_dma_address(&sglist[nents]), sg_dma_address(&sglist[nents]) + sg_dma_len(&sglist[nents]), dev->dma_boundary);
+	}
+			       
 
 	return filled;
 }

Re: [RFC] Implement dma_boundary in the generic device

[Matthew Wilcox]

On Sun, Feb 29, 2004 at 09:56:21AM -0600, James Bottomley wrote:
> ===== drivers/scsi/hosts.c 1.96 vs edited =====
> --- 1.96/drivers/scsi/hosts.c	Mon Dec 29 15:38:10 2003
> +++ edited/drivers/scsi/hosts.c	Sun Feb 29 09:08:09 2004
> @@ -111,6 +111,9 @@
>  	if (!shost->shost_gendev.parent)
>  		shost->shost_gendev.parent = dev ? dev : &platform_bus;
>  
> +	if(dev)

<nitpick style=janitor> Space between "if" and "(" </nitpick>

> ===== include/linux/dma-mapping.h 1.1 vs edited =====
> --- 1.1/include/linux/dma-mapping.h	Sat Dec 21 22:37:05 2002
> +++ edited/include/linux/dma-mapping.h	Sun Feb 29 09:12:02 2004
> @@ -12,6 +12,15 @@
>  
>  #include <asm/dma-mapping.h>
>  
> +#ifndef ARCH_HAS_DMA_SET_BOUNDARY
> +static inline void
> +dma_set_boundary(struct device *dev, u64 boundary)
> +{
> +	dev->dma_boundary = boundary;
> +}
> +#endif

Don't all architectures want a limit to 4GB boundaries, to avoid crossing
the first 4GB?

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: [RFC] Implement dma_boundary in the generic device

[James Bottomley]

On Sun, 2004-02-29 at 10:46, Matthew Wilcox wrote:
> <nitpick style=janitor> Space between "if" and "(" </nitpick>

Well, I think I've already proved I'm not a very good janitor with the
attempt to shift coherent_dma_mask ...

> Don't all architectures want a limit to 4GB boundaries, to avoid crossing
> the first 4GB?

That is the fundamental limit, that's why I set it up to 4GB at
initialisation.  I anticipate that if platforms want to enforce an upper
bound, they would override dma_set_boundary().

James

Re: [RFC] Implement dma_boundary in the generic device

[Benjamin Herrenschmidt]

Ok, I have a ppc64 implementation that appear to work after a quick
test, I'll have to hammer it more tomorrow though and will send a patch.

One thing we discussed on irc but I want to raise here so we all agree
about the "limitation" and can wire it deep into the documentation:

The boundary stuff is only respected as long as the segment doesn't
start with an "offset". If I have a 64k boundary and get passed a
64k segment but which start with an "offset" in the first page, I
will _not_ break it up. Same as without iommu, I don't break
existing segments. I just prevent merging and align big segments
properly so they stay within the boundary.

Oh ... and I would still have liked (both at the struct device and
the BIO layer) a separate "max_seg_size" .... I just figured out that
stupid DBDMA controller used all over the place on Macs (though only
for CD/DVD ATA/100 and sound driver on G5s with an iommu) has a limit
of 64k ... -1 ! (0xffff is the max, 0 does NOT mean 64k shit !)

Right now, I work around that in IDE since I can allocate a larger
DMA descriptor table that the normal IDE PRD, so I allocate 512 of
them... (drivers/ide doesn't have hooks to let the low lever driver
set different queus sizes, sg list sizes, boundaries etc... I have
to mimmic the "legacy" ones).

But maybe it's not worth bothering...

Ben.