Workaround for iommu page alignment

PATCH In review Benjamin Herrenschmidt 3 participants 7 messages Jan 7, 2008 – Jan 8, 2008 PowerPC

Share & tools

Benjamin Herrenschmidt

author

2008-01-07 21:46 UTC #0 lore raw

PATCH 1/1 Workaround for iommu page alignment

Our iommu page size is currently always 4K. That means with our current code, drivers may do a dma_map_sg() of a 64K page and obtain a dma_addr_t that is only 4K aligned.

This works fine in most cases except some infiniband HW it seems, where they tell the HW about the page size and it ignores the low bits of the DMA address.

This works around it by making our IOMMU code enforce a PAGE_SIZE alignment for mappings of objects that are page aligned in the first place and whose size is larger or equal to a page.

Signed-off-by Benjamin Herrenschmidt <[email protected]>

---

Looks like it fell through the holiday cracks ...

It could still go in 2.6.24 I suppose... If not, put it in 2.6.25 and we'll backport to stable later.

arch/powerpc/kernel/iommu.c +13 -3

Index: linux-work/arch/powerpc/kernel/iommu.c
===================================================================
@@ -278,6 +278,7 @@ int iommu_map_sg(struct iommu_table *tbl
 	unsigned long flags;
 	struct scatterlist *s, *outs, *segstart;
 	int outcount, incount, i;
+	unsigned int align;
 	unsigned long handle;
 
 	BUG_ON(direction == DMA_NONE);

@@ -309,7 +310,11 @@ int iommu_map_sg(struct iommu_table *tbl
 		/* Allocate iommu entries for that segment */
 		vaddr = (unsigned long) sg_virt(s);
 		npages = iommu_num_pages(vaddr, slen);
-		entry = iommu_range_alloc(tbl, npages, &handle, mask >> IOMMU_PAGE_SHIFT, 0);
+		align = 0;
+		if (IOMMU_PAGE_SHIFT < PAGE_SHIFT && (vaddr & ~PAGE_MASK) == 0)
+			align = PAGE_SHIFT - IOMMU_PAGE_SHIFT;
+		entry = iommu_range_alloc(tbl, npages, &handle,
+					  mask >> IOMMU_PAGE_SHIFT, align);
 
 		DBG("  - vaddr: %lx, size: %lx\n", vaddr, slen);

@@ -572,7 +577,7 @@ dma_addr_t iommu_map_single(struct iommu
 {
 	dma_addr_t dma_handle = DMA_ERROR_CODE;
 	unsigned long uaddr;
-	unsigned int npages;
+	unsigned int npages, align;
 
 	BUG_ON(direction == DMA_NONE);

@@ -580,8 +585,13 @@ dma_addr_t iommu_map_single(struct iommu
 	npages = iommu_num_pages(uaddr, size);
 
 	if (tbl) {
+		align = 0;
+		if (IOMMU_PAGE_SHIFT < PAGE_SHIFT &&
+		    ((unsigned long)vaddr & ~PAGE_MASK) == 0)
+			align = PAGE_SHIFT - IOMMU_PAGE_SHIFT;
+
 		dma_handle = iommu_alloc(tbl, vaddr, npages, direction,
-					 mask >> IOMMU_PAGE_SHIFT, 0);
+					 mask >> IOMMU_PAGE_SHIFT, align);
 		if (dma_handle == DMA_ERROR_CODE) {
 			if (printk_ratelimit())  {
 				printk(KERN_INFO "iommu_alloc failed, "

Olof Johansson

Acked-by on v1 1/1

2008-01-07 21:48 UTC #1 lore raw

Quoted 8 lines · Benjamin Herrenschmidt

Our iommu page size is currently always 4K. That means with our current code, drivers may do a dma_map_sg() of a 64K page and obtain a dma_addr_t that is only 4K aligned.
This works fine in most cases except some infiniband HW it seems, where they tell the HW about the page size and it ignores the low bits of the DMA address.

Quoted 5 lines

This works around it by making our IOMMU code enforce a PAGE_SIZE alignment for mappings of objects that are page aligned in the first place and whose size is larger or equal to a page.
Signed-off-by Benjamin Herrenschmidt <[email protected]>

Acked-by Olof Johansson <[email protected]>

-Olof

Michael Ellerman

2008-01-07 22:41 UTC #2 lore raw

In reply to Benjamin Herrenschmidt

Quoted 11 lines · Benjamin Herrenschmidt

Our iommu page size is currently always 4K. That means with our current code, drivers may do a dma_map_sg() of a 64K page and obtain a dma_addr_t that is only 4K aligned.
This works fine in most cases except some infiniband HW it seems, where they tell the HW about the page size and it ignores the low bits of the DMA address.
This works around it by making our IOMMU code enforce a PAGE_SIZE alignment for mappings of objects that are page aligned in the first place and whose size is larger or equal to a page.

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I don't see in the code where you implement the "size > PAGE_SIZE" condition. But I only just got back from holidays .. :)

cheers

Signature · 8 lines

Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

Benjamin Herrenschmidt

author

2008-01-07 23:33 UTC #3 lore raw

Quoted 3 lines

I don't see in the code where you implement the "size > PAGE_SIZE" condition. But I only just got back from holidays .. :)

Indeed ... a "quilt ref" was missing :-(

New patch coming ...

Ben.

Benjamin Herrenschmidt

author

2008-01-07 23:34 UTC #4 lore raw

PATCH 1/1 Workaround for iommu page alignment (#2)

In reply to Benjamin Herrenschmidt

powerpc: Workaround for iommu page alignment

Our iommu page size is currently always 4K. That means with our current code, drivers may do a dma_map_sg() of a 64K page and obtain a dma_addr_t that is only 4K aligned.

This works fine in most cases except some infiniband HW it seems, where they tell the HW about the page size and it ignores the low bits of the DMA address.

This works around it by making our IOMMU code enforce a PAGE_SIZE alignment for mappings of objects that are page aligned in the first place and whose size is larger or equal to a page.

Signed-off-by Benjamin Herrenschmidt <[email protected]>

---

And this version actually does what the comment says (I had forgotten to quilt ref... a common mistake).

arch/powerpc/kernel/iommu.c +14 -3

Index: linux-work/arch/powerpc/kernel/iommu.c
===================================================================
@@ -278,6 +278,7 @@ int iommu_map_sg(struct iommu_table *tbl
 	unsigned long flags;
 	struct scatterlist *s, *outs, *segstart;
 	int outcount, incount, i;
+	unsigned int align;
 	unsigned long handle;
 
 	BUG_ON(direction == DMA_NONE);

@@ -309,7 +310,12 @@ int iommu_map_sg(struct iommu_table *tbl
 		/* Allocate iommu entries for that segment */
 		vaddr = (unsigned long) sg_virt(s);
 		npages = iommu_num_pages(vaddr, slen);
-		entry = iommu_range_alloc(tbl, npages, &handle, mask >> IOMMU_PAGE_SHIFT, 0);
+		align = 0;
+		if (IOMMU_PAGE_SHIFT < PAGE_SHIFT && slen >= PAGE_SIZE &&
+		    (vaddr & ~PAGE_MASK) == 0)
+			align = PAGE_SHIFT - IOMMU_PAGE_SHIFT;
+		entry = iommu_range_alloc(tbl, npages, &handle,
+					  mask >> IOMMU_PAGE_SHIFT, align);
 
 		DBG("  - vaddr: %lx, size: %lx\n", vaddr, slen);

@@ -572,7 +578,7 @@ dma_addr_t iommu_map_single(struct iommu
 {
 	dma_addr_t dma_handle = DMA_ERROR_CODE;
 	unsigned long uaddr;
-	unsigned int npages;
+	unsigned int npages, align;
 
 	BUG_ON(direction == DMA_NONE);

@@ -580,8 +586,13 @@ dma_addr_t iommu_map_single(struct iommu
 	npages = iommu_num_pages(uaddr, size);
 
 	if (tbl) {
+		align = 0;
+		if (IOMMU_PAGE_SHIFT < PAGE_SHIFT && size >= PAGE_SIZE &&
+		    ((unsigned long)vaddr & ~PAGE_MASK) == 0)
+			align = PAGE_SHIFT - IOMMU_PAGE_SHIFT;
+
 		dma_handle = iommu_alloc(tbl, vaddr, npages, direction,
-					 mask >> IOMMU_PAGE_SHIFT, 0);
+					 mask >> IOMMU_PAGE_SHIFT, align);
 		if (dma_handle == DMA_ERROR_CODE) {
 			if (printk_ratelimit())  {
 				printk(KERN_INFO "iommu_alloc failed, "

Olof Johansson

Acked-by on v1 1/1

2008-01-08 03:39 UTC #5 lore raw

Re: [PATCH][POWERPC] Workaround for iommu page alignment (#2)

Quoted 14 lines · Benjamin Herrenschmidt

powerpc: Workaround for iommu page alignment
Our iommu page size is currently always 4K. That means with our current code, drivers may do a dma_map_sg() of a 64K page and obtain a dma_addr_t that is only 4K aligned.
This works fine in most cases except some infiniband HW it seems, where they tell the HW about the page size and it ignores the low bits of the DMA address.
This works around it by making our IOMMU code enforce a PAGE_SIZE alignment for mappings of objects that are page aligned in the first place and whose size is larger or equal to a page.

Quoted 5 lines

Signed-off-by Benjamin Herrenschmidt <[email protected]>
---
And this version actually does what the comment says (I had forgotten to quilt ref... a common mistake).

And sloppy of me to not catch it. Anyway:

Acked-by Olof Johansson <[email protected]>

I wonder how long until there's a device that has some other < PAGE_SIZE alignment bug^Wrequirement that we'll need to meet too. :(

-Olof

Benjamin Herrenschmidt

author

2008-01-08 04:02 UTC #6 lore raw

Re: [PATCH][POWERPC] Workaround for iommu page alignment (#2)

Quoted 6 lines

And sloppy of me to not catch it. Anyway:
Acked-by Olof Johansson <[email protected]>
I wonder how long until there's a device that has some other < PAGE_SIZE alignment bug^Wrequirement that we'll need to meet too. :(

Yeah, it's a worry...

Ben.