[PATCH -mm] x86 calgary: fix handling of devces that aren't behind the Calgary

[PATCH -mm] x86 calgary: fix handling of devces that aren't behind the Calgary

[FUJITA Tomonori]

The calgary code can give drivers addresses above 4GB which is very
bad for hardware that is only 32bit DMA addressable:

http://lkml.org/lkml/2008/5/8/423

This patch tries to fix the problem by using per-device
dma_mapping_ops support. This fixes the calgary code to use swiotlb or
nommu properly for devices which are not behind the Calgary/CalIOC2.

With this patch, the calgary code sets the global dma_ops to swiotlb
or nommu, and the dma_ops of devices behind the Calgary/CalIOC2 to
calgary_dma_ops. So the calgary code can handle devices safely that
aren't behind the Calgary/CalIOC2.

I think that bus_register_notifier works nicely for hotplugging (the
calgary code can register a hook to set the device-per ops to
calgary_dma_ops) though I don't know the calgary code needs it.

This patch is against -mm (depends on the per-device dma_mapping_ops
patchset in -mm).

This is only compile tested.

Thanks,

==
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: [PATCH] x86 calgary: fix handling of devces that aren't behind the Calgary

The calgary code can give drivers addresses above 4GB which is very
bad for hardware that is only 32bit DMA addressable.

With this patch, the calgary code sets the global dma_ops to swiotlb
or nommu properly, and the dma_ops of devices behind the
Calgary/CalIOC2 to calgary_dma_ops. So the calgary code can handle
devices safely that aren't behind the Calgary/CalIOC2.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 arch/x86/kernel/pci-calgary_64.c |   63 +++++++++++---------------------------
 include/asm-x86/iommu.h          |    1 +
 2 files changed, 19 insertions(+), 45 deletions(-)

diff --git a/arch/x86/kernel/pci-calgary_64.c b/arch/x86/kernel/pci-calgary_64.c
index dca9a82..1839968 100644
--- a/arch/x86/kernel/pci-calgary_64.c
+++ b/arch/x86/kernel/pci-calgary_64.c
@@ -36,6 +36,7 @@
 #include <linux/delay.h>
 #include <linux/scatterlist.h>
 #include <linux/iommu-helper.h>
+#include <asm/iommu.h>
 #include <asm/gart.h>
 #include <asm/calgary.h>
 #include <asm/tce.h>
@@ -410,22 +411,6 @@ static void calgary_unmap_sg(struct device *dev,
 	}
 }
 
-static int calgary_nontranslate_map_sg(struct device* dev,
-	struct scatterlist *sg, int nelems, int direction)
-{
-	struct scatterlist *s;
-	int i;
-
-	for_each_sg(sg, s, nelems, i) {
-		struct page *p = sg_page(s);
-
-		BUG_ON(!p);
-		s->dma_address = virt_to_bus(sg_virt(s));
-		s->dma_length = s->length;
-	}
-	return nelems;
-}
-
 static int calgary_map_sg(struct device *dev, struct scatterlist *sg,
 	int nelems, int direction)
 {
@@ -436,9 +421,6 @@ static int calgary_map_sg(struct device *dev, struct scatterlist *sg,
 	unsigned long entry;
 	int i;
 
-	if (!translation_enabled(tbl))
-		return calgary_nontranslate_map_sg(dev, sg, nelems, direction);
-
 	for_each_sg(sg, s, nelems, i) {
 		BUG_ON(!sg_page(s));
 
@@ -474,7 +456,6 @@ error:
 static dma_addr_t calgary_map_single(struct device *dev, phys_addr_t paddr,
 	size_t size, int direction)
 {
-	dma_addr_t dma_handle = bad_dma_address;
 	void *vaddr = phys_to_virt(paddr);
 	unsigned long uaddr;
 	unsigned int npages;
@@ -483,12 +464,7 @@ static dma_addr_t calgary_map_single(struct device *dev, phys_addr_t paddr,
 	uaddr = (unsigned long)vaddr;
 	npages = num_dma_pages(uaddr, size);
 
-	if (translation_enabled(tbl))
-		dma_handle = iommu_alloc(dev, tbl, vaddr, npages, direction);
-	else
-		dma_handle = virt_to_bus(vaddr);
-
-	return dma_handle;
+	return iommu_alloc(dev, tbl, vaddr, npages, direction);
 }
 
 static void calgary_unmap_single(struct device *dev, dma_addr_t dma_handle,
@@ -497,9 +473,6 @@ static void calgary_unmap_single(struct device *dev, dma_addr_t dma_handle,
 	struct iommu_table *tbl = find_iommu_table(dev);
 	unsigned int npages;
 
-	if (!translation_enabled(tbl))
-		return;
-
 	npages = num_dma_pages(dma_handle, size);
 	iommu_free(tbl, dma_handle, npages);
 }
@@ -522,18 +495,12 @@ static void* calgary_alloc_coherent(struct device *dev, size_t size,
 		goto error;
 	memset(ret, 0, size);
 
-	if (translation_enabled(tbl)) {
-		/* set up tces to cover the allocated range */
-		mapping = iommu_alloc(dev, tbl, ret, npages, DMA_BIDIRECTIONAL);
-		if (mapping == bad_dma_address)
-			goto free;
-
-		*dma_handle = mapping;
-	} else /* non translated slot */
-		*dma_handle = virt_to_bus(ret);
-
+	/* set up tces to cover the allocated range */
+	mapping = iommu_alloc(dev, tbl, ret, npages, DMA_BIDIRECTIONAL);
+	if (mapping == bad_dma_address)
+		goto free;
+	*dma_handle = mapping;
 	return ret;
-
 free:
 	free_pages((unsigned long)ret, get_order(size));
 	ret = NULL;
@@ -1138,6 +1105,7 @@ static int __init calgary_init_one(struct pci_dev *dev)
 	tbl->chip_ops->handle_quirks(tbl, dev);
 
 	calgary_enable_translation(dev);
+	dev->dev.archdata.dma_ops = &calgary_dma_ops;
 
 	return 0;
 
@@ -1251,6 +1219,7 @@ error:
 		calgary_disable_translation(dev);
 		calgary_free_bus(dev);
 		pci_dev_put(dev); /* Undo calgary_init_one()'s pci_dev_get() */
+		dev->dev.archdata.dma_ops = NULL;
 	} while (1);
 
 	return ret;
@@ -1430,6 +1399,10 @@ void __init detect_calgary(void)
 		printk(KERN_INFO "PCI-DMA: Calgary TCE table spec is %d, "
 		       "CONFIG_IOMMU_DEBUG is %s.\n", specified_table_size,
 		       debugging ? "enabled" : "disabled");
+
+		/* swiotlb for devices that aren't behind the Calgary. */
+		if (end_pfn > MAX_DMA32_PFN)
+			swiotlb = 1;
 	}
 	return;
 
@@ -1446,7 +1419,7 @@ int __init calgary_iommu_init(void)
 {
 	int ret;
 
-	if (no_iommu || swiotlb)
+	if (no_iommu || (swiotlb && !calgary_detected))
 		return -ENODEV;
 
 	if (!calgary_detected)
@@ -1459,15 +1432,15 @@ int __init calgary_iommu_init(void)
 	if (ret) {
 		printk(KERN_ERR "PCI-DMA: Calgary init failed %d, "
 		       "falling back to no_iommu\n", ret);
-		if (end_pfn > MAX_DMA32_PFN)
-			printk(KERN_ERR "WARNING more than 4GB of memory, "
-					"32bit PCI may malfunction.\n");
 		return ret;
 	}
 
 	force_iommu = 1;
 	bad_dma_address = 0x0;
-	dma_ops = &calgary_dma_ops;
+
+	/* dma_ops is set to swiotlb or nommu */
+	if (!dma_ops)
+		dma_ops = &nommu_dma_ops;
 
 	return 0;
 }
diff --git a/include/asm-x86/iommu.h b/include/asm-x86/iommu.h
index 07862fd..1a5e7df 100644
--- a/include/asm-x86/iommu.h
+++ b/include/asm-x86/iommu.h
@@ -3,6 +3,7 @@
 
 extern void pci_iommu_shutdown(void);
 extern void no_iommu_init(void);
+extern struct dma_mapping_ops nommu_dma_ops;
 extern int force_iommu, no_iommu;
 extern int iommu_detected;
 #ifdef CONFIG_IOMMU
-- 
1.5.4.2

Re: [PATCH -mm] x86 calgary: fix handling of devces that aren't behind the Calgary

[Muli Ben-Yehuda]

On Sat, May 31, 2008 at 01:31:33PM +0900, FUJITA Tomonori wrote:

> The calgary code can give drivers addresses above 4GB which is very
> bad for hardware that is only 32bit DMA addressable:
> 
> http://lkml.org/lkml/2008/5/8/423
> 
> This patch tries to fix the problem by using per-device
> dma_mapping_ops support. This fixes the calgary code to use swiotlb
> or nommu properly for devices which are not behind the
> Calgary/CalIOC2.
> 
> With this patch, the calgary code sets the global dma_ops to swiotlb
> or nommu, and the dma_ops of devices behind the Calgary/CalIOC2 to
> calgary_dma_ops. So the calgary code can handle devices safely that
> aren't behind the Calgary/CalIOC2.

This seems a little backward to me. I thought we were going to get rid
of the global dma_ops? If not, assuming going through the global one
would be more efficient, Calgary should be the global one and
nommu/swiotlb should be used on devices that do not have translation
enabled. The reason why is that the majority of devices on a Calgary
system, assuming Calgary is in use, will have translation enabled.

In general the patch looks good, barring the point above. We'll give
it a spin on some Calgary/CalIOC2 machines.

Cheers,
Muli

Re: [PATCH -mm] x86 calgary: fix handling of devces that aren't behind the Calgary

[FUJITA Tomonori]

On Tue, 3 Jun 2008 08:21:46 +0300
Muli Ben-Yehuda <muli@il.ibm.com> wrote:

> On Sat, May 31, 2008 at 01:31:33PM +0900, FUJITA Tomonori wrote:
> 
> > The calgary code can give drivers addresses above 4GB which is very
> > bad for hardware that is only 32bit DMA addressable:
> > 
> > http://lkml.org/lkml/2008/5/8/423
> > 
> > This patch tries to fix the problem by using per-device
> > dma_mapping_ops support. This fixes the calgary code to use swiotlb
> > or nommu properly for devices which are not behind the
> > Calgary/CalIOC2.
> > 
> > With this patch, the calgary code sets the global dma_ops to swiotlb
> > or nommu, and the dma_ops of devices behind the Calgary/CalIOC2 to
> > calgary_dma_ops. So the calgary code can handle devices safely that
> > aren't behind the Calgary/CalIOC2.
> 
> This seems a little backward to me. I thought we were going to get rid
> of the global dma_ops?

Yeah, I think that we can (though I'm not sure yet if it's the
cleanest way to handle IOMMUs). I think that it would better to clean
up the x86 IOMMU startup code a bit. Currently, IOMMUs interact too
much. It might take time for me to figure out the cleanest way so I
tried to fix the Calgary problem in the easiest way.

Yeah, I'm not sure if x86 maintainers are ok with the cleanup. If they
are, I'll try.


> If not, assuming going through the global one
> would be more efficient, Calgary should be the global one and
> nommu/swiotlb should be used on devices that do not have translation
> enabled. The reason why is that the majority of devices on a Calgary
> system, assuming Calgary is in use, will have translation enabled.

get_dma_ops() checks dev->archdata.dma_ops first then uses the global
if device dma_ops is NULL. So I'm not sure about the efficiency.

But I agreed that it's a bit odd to set nommu/swiotlb to the global
ops since the majority of devices uses calgary_ops on a Calgary
system, as you said. The patch does that just because seems that it's
the easiest way to handle devices that aren't behind Calgary.


> In general the patch looks good, barring the point above. We'll give
> it a spin on some Calgary/CalIOC2 machines.

Thanks,

Please feel free to drop the patch if you want to fix the problem
differently. I just wanted to see how the per-device ops can handle
the problem.

Re: [PATCH -mm] x86 calgary: fix handling of devces that aren't behind the Calgary

[Alexis Bruemmer]

On Tue, 2008-06-03 at 08:21 +0300, Muli Ben-Yehuda wrote:
> On Sat, May 31, 2008 at 01:31:33PM +0900, FUJITA Tomonori wrote:
> 
> > The calgary code can give drivers addresses above 4GB which is very
> > bad for hardware that is only 32bit DMA addressable:
> > 
> > http://lkml.org/lkml/2008/5/8/423
> > 
> > This patch tries to fix the problem by using per-device
> > dma_mapping_ops support. This fixes the calgary code to use swiotlb
> > or nommu properly for devices which are not behind the
> > Calgary/CalIOC2.
> > 
> > With this patch, the calgary code sets the global dma_ops to swiotlb
> > or nommu, and the dma_ops of devices behind the Calgary/CalIOC2 to
> > calgary_dma_ops. So the calgary code can handle devices safely that
> > aren't behind the Calgary/CalIOC2.
> 
> This seems a little backward to me. I thought we were going to get rid
> of the global dma_ops? If not, assuming going through the global one
> would be more efficient, Calgary should be the global one and
> nommu/swiotlb should be used on devices that do not have translation
> enabled. The reason why is that the majority of devices on a Calgary
> system, assuming Calgary is in use, will have translation enabled.
> 
> In general the patch looks good, barring the point above. We'll give
> it a spin on some Calgary/CalIOC2 machines.
Initial testing on a CalIO2 box this patch causes the machine not to
boot (and this time I tested the base 2.6.26-rc4 + FUJITA 2 per deive
dma_ops patches first and it boots just fine).  Here is a bit of the
dump from the failed boot:

Loading megaraid_sas
[17180656.651128] megasas: 00.00.03.20-rc1 Mon. March 10 11:02:31 PDT 2008
[17180656.657866] megasas: 0x1000:0x0060:0x1014:0x0363: bus 4:slot 0:func 0
[17180656.663899] ACPI: PCI Interrupt 0000:04:00.0[A] -> GSI 46 (level, low) -> IRQ 46
[17180656.673677] megasas: FW now in Ready state
[17180657.774102] Calgary: DMA error on CalIOC2 PHB 0x3
[17180657.779171] Calgary: 0x02000000@CSR 0x00000000@PLSSR 0xb0008000@CSMR 0x00000000@MCK
[17180657.787212] Calgary: 0x00000000@0x810 0xf6200000@0x820 0xf6200040@0x830 0x00000000@0x840 0x06000000@0x850 0x00000000@0x860 0x00000000@0x870 
[17180657.801629] Calgary: 0x00000000@0xcb0

Adding some quick debug code it seems that the megaraid controller is
not getting its dev->dev.archdata.dma_ops set to calgary_dma_ops.  I am
not sure why, but will keep digging.  Any ideas?

--Alexis
> 
> Cheers,
> Muli
> 
> 

Re: [PATCH -mm] x86 calgary: fix handling of devces that aren't behind the Calgary

[FUJITA Tomonori]

On Tue, 03 Jun 2008 09:55:37 -0700
Alexis Bruemmer <alexisb@us.ibm.com> wrote:

> On Tue, 2008-06-03 at 08:21 +0300, Muli Ben-Yehuda wrote:
> > On Sat, May 31, 2008 at 01:31:33PM +0900, FUJITA Tomonori wrote:
> > 
> > > The calgary code can give drivers addresses above 4GB which is very
> > > bad for hardware that is only 32bit DMA addressable:
> > > 
> > > http://lkml.org/lkml/2008/5/8/423
> > > 
> > > This patch tries to fix the problem by using per-device
> > > dma_mapping_ops support. This fixes the calgary code to use swiotlb
> > > or nommu properly for devices which are not behind the
> > > Calgary/CalIOC2.
> > > 
> > > With this patch, the calgary code sets the global dma_ops to swiotlb
> > > or nommu, and the dma_ops of devices behind the Calgary/CalIOC2 to
> > > calgary_dma_ops. So the calgary code can handle devices safely that
> > > aren't behind the Calgary/CalIOC2.
> > 
> > This seems a little backward to me. I thought we were going to get rid
> > of the global dma_ops? If not, assuming going through the global one
> > would be more efficient, Calgary should be the global one and
> > nommu/swiotlb should be used on devices that do not have translation
> > enabled. The reason why is that the majority of devices on a Calgary
> > system, assuming Calgary is in use, will have translation enabled.
> > 
> > In general the patch looks good, barring the point above. We'll give
> > it a spin on some Calgary/CalIOC2 machines.
> Initial testing on a CalIO2 box this patch causes the machine not to
> boot (and this time I tested the base 2.6.26-rc4 + FUJITA 2 per deive
> dma_ops patches first and it boots just fine).  Here is a bit of the
> dump from the failed boot:
> 
> Loading megaraid_sas
> [17180656.651128] megasas: 00.00.03.20-rc1 Mon. March 10 11:02:31 PDT 2008
> [17180656.657866] megasas: 0x1000:0x0060:0x1014:0x0363: bus 4:slot 0:func 0
> [17180656.663899] ACPI: PCI Interrupt 0000:04:00.0[A] -> GSI 46 (level, low) -> IRQ 46
> [17180656.673677] megasas: FW now in Ready state
> [17180657.774102] Calgary: DMA error on CalIOC2 PHB 0x3
> [17180657.779171] Calgary: 0x02000000@CSR 0x00000000@PLSSR 0xb0008000@CSMR 0x00000000@MCK
> [17180657.787212] Calgary: 0x00000000@0x810 0xf6200000@0x820 0xf6200040@0x830 0x00000000@0x840 0x06000000@0x850 0x00000000@0x860 0x00000000@0x870 
> [17180657.801629] Calgary: 0x00000000@0xcb0
> 
> Adding some quick debug code it seems that the megaraid controller is
> not getting its dev->dev.archdata.dma_ops set to calgary_dma_ops.  I am
> not sure why, but will keep digging.  Any ideas?

Ah, sorry. pci_alloc_consistent fails?

=
fix per-device dma_mapping_ops support

On x86, pci_dma_supported, pci_alloc_consistent, and
pci_free_consistent don't call DMA APIs directly (the majority of
platforms do). per-device dma_mapping_ops support patch needs to
modify pci-dma.c.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index 4471984..bc2251d 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -318,6 +318,8 @@ static int dma_release_coherent(struct device *dev, int order, void *vaddr)
 
 int dma_supported(struct device *dev, u64 mask)
 {
+	struct dma_mapping_ops *ops = get_dma_ops(dev);
+
 #ifdef CONFIG_PCI
 	if (mask > 0xffffffff && forbid_dac > 0) {
 		dev_info(dev, "PCI: Disallowing DAC for device\n");
@@ -325,8 +327,8 @@ int dma_supported(struct device *dev, u64 mask)
 	}
 #endif
 
-	if (dma_ops->dma_supported)
-		return dma_ops->dma_supported(dev, mask);
+	if (ops->dma_supported)
+		return ops->dma_supported(dev, mask);
 
 	/* Copied from i386. Doesn't make much sense, because it will
 	   only work for pci_alloc_coherent.
@@ -373,6 +375,7 @@ void *
 dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle,
 		   gfp_t gfp)
 {
+	struct dma_mapping_ops *ops = get_dma_ops(dev);
 	void *memory = NULL;
 	struct page *page;
 	unsigned long dma_mask = 0;
@@ -435,8 +438,8 @@ dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle,
 			/* Let low level make its own zone decisions */
 			gfp &= ~(GFP_DMA32|GFP_DMA);
 
-			if (dma_ops->alloc_coherent)
-				return dma_ops->alloc_coherent(dev, size,
+			if (ops->alloc_coherent)
+				return ops->alloc_coherent(dev, size,
 							   dma_handle, gfp);
 			return NULL;
 		}
@@ -448,14 +451,14 @@ dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle,
 		}
 	}
 
-	if (dma_ops->alloc_coherent) {
+	if (ops->alloc_coherent) {
 		free_pages((unsigned long)memory, get_order(size));
 		gfp &= ~(GFP_DMA|GFP_DMA32);
-		return dma_ops->alloc_coherent(dev, size, dma_handle, gfp);
+		return ops->alloc_coherent(dev, size, dma_handle, gfp);
 	}
 
-	if (dma_ops->map_simple) {
-		*dma_handle = dma_ops->map_simple(dev, virt_to_phys(memory),
+	if (ops->map_simple) {
+		*dma_handle = ops->map_simple(dev, virt_to_phys(memory),
 					      size,
 					      PCI_DMA_BIDIRECTIONAL);
 		if (*dma_handle != bad_dma_address)
@@ -477,12 +480,14 @@ EXPORT_SYMBOL(dma_alloc_coherent);
 void dma_free_coherent(struct device *dev, size_t size,
 			 void *vaddr, dma_addr_t bus)
 {
+	struct dma_mapping_ops *ops = get_dma_ops(dev);
+
 	int order = get_order(size);
 	WARN_ON(irqs_disabled());	/* for portability */
 	if (dma_release_coherent(dev, order, vaddr))
 		return;
-	if (dma_ops->unmap_single)
-		dma_ops->unmap_single(dev, bus, size, 0);
+	if (ops->unmap_single)
+		ops->unmap_single(dev, bus, size, 0);
 	free_pages((unsigned long)vaddr, order);
 }
 EXPORT_SYMBOL(dma_free_coherent);

Re: [PATCH -mm] x86 calgary: fix handling of devces that aren't behind the Calgary

[Alexis Bruemmer]

On Sat, 2008-05-31 at 13:31 +0900, FUJITA Tomonori wrote:
> The calgary code can give drivers addresses above 4GB which is very
> bad for hardware that is only 32bit DMA addressable:
> 
> http://lkml.org/lkml/2008/5/8/423
> 
> This patch tries to fix the problem by using per-device
> dma_mapping_ops support. This fixes the calgary code to use swiotlb or
> nommu properly for devices which are not behind the Calgary/CalIOC2.
> 
> With this patch, the calgary code sets the global dma_ops to swiotlb
> or nommu, and the dma_ops of devices behind the Calgary/CalIOC2 to
> calgary_dma_ops. So the calgary code can handle devices safely that
> aren't behind the Calgary/CalIOC2.
> 
> I think that bus_register_notifier works nicely for hotplugging (the
> calgary code can register a hook to set the device-per ops to
> calgary_dma_ops) though I don't know the calgary code needs it.
> 
> This patch is against -mm (depends on the per-device dma_mapping_ops
> patchset in -mm).
> 
> This is only compile tested.
So this patch dose not completely work.  The problem is that devices
that are controlled by the CalIO2/calgary are not getting the calgary
dma_ops assigned to them.  Having the proper changes to pci-dma.c helped
(thank you) but still didn't quite get us there.  From what I could tell
having the per device dma_ops being assigned in calgary_init_one was not
correct.  The dev being past to calgary_init_one is only ever an IBM
CalIO2/calgary device which meant that many devices that are being
controlled by the CalI02/calgary, such as the the MegaRAID SAS
controller, were not getting the calgary based dma ops assigned to them.
I have attached an updated patch that does assign the per device calgary
dma_ops correctly and have successfully tested it on an IBM x3950 M2.  I
think there is a better way to do this, but this does work.

Thank you some much for your help and work on this problem!

Alexis 


> Thanks,
> 
> ==
> From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Subject: [PATCH] x86 calgary: fix handling of devces that aren't behind the Calgary
> 
> The calgary code can give drivers addresses above 4GB which is very
> bad for hardware that is only 32bit DMA addressable.
> 
> With this patch, the calgary code sets the global dma_ops to swiotlb
> or nommu properly, and the dma_ops of devices behind the
> Calgary/CalIOC2 to calgary_dma_ops. So the calgary code can handle
> devices safely that aren't behind the Calgary/CalIOC2.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> ---
<snip>

Signed-off-by: Alexis Bruemmer <alexisb@us.ibm.com>


 arch/x86/kernel/pci-calgary_64.c |   63 +++++++++++---------------------------
 include/asm-x86/iommu.h          |    1 +
 2 files changed, 19 insertions(+), 45 deletions(-)

Index: linux-2.6.26-rc4/arch/x86/kernel/pci-calgary_64.c
===================================================================
--- linux-2.6.26-rc4.orig/arch/x86/kernel/pci-calgary_64.c
+++ linux-2.6.26-rc4/arch/x86/kernel/pci-calgary_64.c
@@ -36,6 +36,7 @@
 #include <linux/delay.h>
 #include <linux/scatterlist.h>
 #include <linux/iommu-helper.h>
+#include <asm/iommu.h>
 #include <asm/gart.h>
 #include <asm/calgary.h>
 #include <asm/tce.h>
@@ -410,22 +411,6 @@ static void calgary_unmap_sg(struct devi
 	}
 }
 
-static int calgary_nontranslate_map_sg(struct device* dev,
-	struct scatterlist *sg, int nelems, int direction)
-{
-	struct scatterlist *s;
-	int i;
-
-	for_each_sg(sg, s, nelems, i) {
-		struct page *p = sg_page(s);
-
-		BUG_ON(!p);
-		s->dma_address = virt_to_bus(sg_virt(s));
-		s->dma_length = s->length;
-	}
-	return nelems;
-}
-
 static int calgary_map_sg(struct device *dev, struct scatterlist *sg,
 	int nelems, int direction)
 {
@@ -436,9 +421,6 @@ static int calgary_map_sg(struct device 
 	unsigned long entry;
 	int i;
 
-	if (!translation_enabled(tbl))
-		return calgary_nontranslate_map_sg(dev, sg, nelems, direction);
-
 	for_each_sg(sg, s, nelems, i) {
 		BUG_ON(!sg_page(s));
 
@@ -474,7 +456,6 @@ error:
 static dma_addr_t calgary_map_single(struct device *dev, phys_addr_t paddr,
 	size_t size, int direction)
 {
-	dma_addr_t dma_handle = bad_dma_address;
 	void *vaddr = phys_to_virt(paddr);
 	unsigned long uaddr;
 	unsigned int npages;
@@ -483,12 +464,7 @@ static dma_addr_t calgary_map_single(str
 	uaddr = (unsigned long)vaddr;
 	npages = num_dma_pages(uaddr, size);
 
-	if (translation_enabled(tbl))
-		dma_handle = iommu_alloc(dev, tbl, vaddr, npages, direction);
-	else
-		dma_handle = virt_to_bus(vaddr);
-
-	return dma_handle;
+	return iommu_alloc(dev, tbl, vaddr, npages, direction);
 }
 
 static void calgary_unmap_single(struct device *dev, dma_addr_t dma_handle,
@@ -497,9 +473,6 @@ static void calgary_unmap_single(struct 
 	struct iommu_table *tbl = find_iommu_table(dev);
 	unsigned int npages;
 
-	if (!translation_enabled(tbl))
-		return;
-
 	npages = num_dma_pages(dma_handle, size);
 	iommu_free(tbl, dma_handle, npages);
 }
@@ -522,18 +495,12 @@ static void* calgary_alloc_coherent(stru
 		goto error;
 	memset(ret, 0, size);
 
-	if (translation_enabled(tbl)) {
-		/* set up tces to cover the allocated range */
-		mapping = iommu_alloc(dev, tbl, ret, npages, DMA_BIDIRECTIONAL);
-		if (mapping == bad_dma_address)
-			goto free;
-
-		*dma_handle = mapping;
-	} else /* non translated slot */
-		*dma_handle = virt_to_bus(ret);
-
+	/* set up tces to cover the allocated range */
+	mapping = iommu_alloc(dev, tbl, ret, npages, DMA_BIDIRECTIONAL);
+	if (mapping == bad_dma_address)
+		goto free;
+	*dma_handle = mapping;
 	return ret;
-
 free:
 	free_pages((unsigned long)ret, get_order(size));
 	ret = NULL;
@@ -1230,6 +1197,21 @@ static int __init calgary_init(void)
 			goto error;
 	} while (1);
 
+	dev = NULL;
+	do {
+		struct iommu_table *tbl;
+		dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev);
+
+		if (!dev)
+			break;
+
+		tbl = find_iommu_table(&dev->dev);
+
+		if(translation_enabled(tbl))
+			dev->dev.archdata.dma_ops = &calgary_dma_ops;
+
+	} while (1);
+
 	return ret;
 
 error:
@@ -1251,6 +1233,7 @@ error:
 		calgary_disable_translation(dev);
 		calgary_free_bus(dev);
 		pci_dev_put(dev); /* Undo calgary_init_one()'s pci_dev_get() */
+		dev->dev.archdata.dma_ops = NULL;
 	} while (1);
 
 	return ret;
@@ -1430,6 +1413,10 @@ void __init detect_calgary(void)
 		printk(KERN_INFO "PCI-DMA: Calgary TCE table spec is %d, "
 		       "CONFIG_IOMMU_DEBUG is %s.\n", specified_table_size,
 		       debugging ? "enabled" : "disabled");
+
+		/* swiotlb for devices that aren't behind the Calgary. */
+		if (end_pfn > MAX_DMA32_PFN)
+			swiotlb = 1;
 	}
 	return;
 
@@ -1446,7 +1433,7 @@ int __init calgary_iommu_init(void)
 {
 	int ret;
 
-	if (no_iommu || swiotlb)
+	if (no_iommu || (swiotlb && !calgary_detected))
 		return -ENODEV;
 
 	if (!calgary_detected)
@@ -1459,15 +1446,15 @@ int __init calgary_iommu_init(void)
 	if (ret) {
 		printk(KERN_ERR "PCI-DMA: Calgary init failed %d, "
 		       "falling back to no_iommu\n", ret);
-		if (end_pfn > MAX_DMA32_PFN)
-			printk(KERN_ERR "WARNING more than 4GB of memory, "
-					"32bit PCI may malfunction.\n");
 		return ret;
 	}
 
 	force_iommu = 1;
 	bad_dma_address = 0x0;
-	dma_ops = &calgary_dma_ops;
+
+	/* dma_ops is set to swiotlb or nommu */
+	if (!dma_ops)
+		dma_ops = &nommu_dma_ops;
 
 	return 0;
 }
Index: linux-2.6.26-rc4/include/asm-x86/iommu.h
===================================================================
--- linux-2.6.26-rc4.orig/include/asm-x86/iommu.h
+++ linux-2.6.26-rc4/include/asm-x86/iommu.h
@@ -3,6 +3,7 @@
 
 extern void pci_iommu_shutdown(void);
 extern void no_iommu_init(void);
+extern struct dma_mapping_ops nommu_dma_ops;
 extern int force_iommu, no_iommu;
 extern int iommu_detected;
 #ifdef CONFIG_IOMMU

Re: [PATCH -mm] x86 calgary: fix handling of devces that aren't behind the Calgary

[FUJITA Tomonori]

On Wed, 11 Jun 2008 18:25:06 -0700
Alexis Bruemmer <alexisb@us.ibm.com> wrote:

> On Sat, 2008-05-31 at 13:31 +0900, FUJITA Tomonori wrote:
> > The calgary code can give drivers addresses above 4GB which is very
> > bad for hardware that is only 32bit DMA addressable:
> > 
> > http://lkml.org/lkml/2008/5/8/423
> > 
> > This patch tries to fix the problem by using per-device
> > dma_mapping_ops support. This fixes the calgary code to use swiotlb or
> > nommu properly for devices which are not behind the Calgary/CalIOC2.
> > 
> > With this patch, the calgary code sets the global dma_ops to swiotlb
> > or nommu, and the dma_ops of devices behind the Calgary/CalIOC2 to
> > calgary_dma_ops. So the calgary code can handle devices safely that
> > aren't behind the Calgary/CalIOC2.
> > 
> > I think that bus_register_notifier works nicely for hotplugging (the
> > calgary code can register a hook to set the device-per ops to
> > calgary_dma_ops) though I don't know the calgary code needs it.
> > 
> > This patch is against -mm (depends on the per-device dma_mapping_ops
> > patchset in -mm).
> > 
> > This is only compile tested.
> So this patch dose not completely work.  The problem is that devices
> that are controlled by the CalIO2/calgary are not getting the calgary
> dma_ops assigned to them.  Having the proper changes to pci-dma.c helped
> (thank you) but still didn't quite get us there.  From what I could tell
> having the per device dma_ops being assigned in calgary_init_one was not
> correct.  The dev being past to calgary_init_one is only ever an IBM
> CalIO2/calgary device which meant that many devices that are being
> controlled by the CalI02/calgary, such as the the MegaRAID SAS
> controller, were not getting the calgary based dma ops assigned to them.

Thanks, now I see what's wrong in my patch.


> I have attached an updated patch that does assign the per device calgary
> dma_ops correctly and have successfully tested it on an IBM x3950 M2.  I
> think there is a better way to do this, but this does work.

Looks good though I have one minor comment.


> Thank you some much for your help and work on this problem!

You're welcome! I just want to improve IOMMUs and dma mapping code.


> Alexis 
> 
> 
> > Thanks,
> > 
> > ==
> > From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > Subject: [PATCH] x86 calgary: fix handling of devces that aren't behind the Calgary
> > 
> > The calgary code can give drivers addresses above 4GB which is very
> > bad for hardware that is only 32bit DMA addressable.
> > 
> > With this patch, the calgary code sets the global dma_ops to swiotlb
> > or nommu properly, and the dma_ops of devices behind the
> > Calgary/CalIOC2 to calgary_dma_ops. So the calgary code can handle
> > devices safely that aren't behind the Calgary/CalIOC2.
> > 
> > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > ---
> <snip>
> 
> Signed-off-by: Alexis Bruemmer <alexisb@us.ibm.com>


> @@ -1230,6 +1197,21 @@ static int __init calgary_init(void)
>  			goto error;
>  	} while (1);
>  
> +	dev = NULL;
> +	do {
> +		struct iommu_table *tbl;
> +		dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev);
> +
> +		if (!dev)
> +			break;
> +
> +		tbl = find_iommu_table(&dev->dev);
> +
> +		if(translation_enabled(tbl))
> +			dev->dev.archdata.dma_ops = &calgary_dma_ops;
> +
> +	} while (1);

for_each_pci_dev would simplify the code a bit?

dev = NULL;

for_each_pci_dev(dev) {
	struct iommu_table *tbl;

	tbl = find_iommu_table(&dev->dev);

	if(translation_enabled(tbl))
		dev->dev.archdata.dma_ops = &calgary_dma_ops;
}

Re: [PATCH -mm] x86 calgary: fix handling of devces that aren't behind the Calgary

[Alexis Bruemmer]

On Thu, 2008-06-12 at 16:29 +0900, FUJITA Tomonori wrote:
> On Wed, 11 Jun 2008 18:25:06 -0700
> Alexis Bruemmer <alexisb@us.ibm.com> wrote:
> 
> > On Sat, 2008-05-31 at 13:31 +0900, FUJITA Tomonori wrote:
> > > The calgary code can give drivers addresses above 4GB which is very
> > > bad for hardware that is only 32bit DMA addressable:
> > > 
> > > http://lkml.org/lkml/2008/5/8/423
> > > 
> > > This patch tries to fix the problem by using per-device
> > > dma_mapping_ops support. This fixes the calgary code to use swiotlb or
> > > nommu properly for devices which are not behind the Calgary/CalIOC2.
> > > 
> > > With this patch, the calgary code sets the global dma_ops to swiotlb
> > > or nommu, and the dma_ops of devices behind the Calgary/CalIOC2 to
> > > calgary_dma_ops. So the calgary code can handle devices safely that
> > > aren't behind the Calgary/CalIOC2.
> > > 
> > > I think that bus_register_notifier works nicely for hotplugging (the
> > > calgary code can register a hook to set the device-per ops to
> > > calgary_dma_ops) though I don't know the calgary code needs it.
> > > 
> > > This patch is against -mm (depends on the per-device dma_mapping_ops
> > > patchset in -mm).
> > > 
> > > This is only compile tested.
> > So this patch dose not completely work.  The problem is that devices
> > that are controlled by the CalIO2/calgary are not getting the calgary
> > dma_ops assigned to them.  Having the proper changes to pci-dma.c helped
> > (thank you) but still didn't quite get us there.  From what I could tell
> > having the per device dma_ops being assigned in calgary_init_one was not
> > correct.  The dev being past to calgary_init_one is only ever an IBM
> > CalIO2/calgary device which meant that many devices that are being
> > controlled by the CalI02/calgary, such as the the MegaRAID SAS
> > controller, were not getting the calgary based dma ops assigned to them.
> 
> Thanks, now I see what's wrong in my patch.

> > I have attached an updated patch that does assign the per device calgary
> > dma_ops correctly and have successfully tested it on an IBM x3950 M2.  I
> > think there is a better way to do this, but this does work.
> 
> Looks good though I have one minor comment.
> 

Updated and tested patch below.

> 
> > Thank you some much for your help and work on this problem!
> 
> You're welcome! I just want to improve IOMMUs and dma mapping code.
> 
> 
> > Alexis 
> > 
> > 
> > > Thanks,
> > > 
> > > ==
> > > From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > > Subject: [PATCH] x86 calgary: fix handling of devces that aren't behind the Calgary
> > > 
> > > The calgary code can give drivers addresses above 4GB which is very
> > > bad for hardware that is only 32bit DMA addressable.
> > > 
> > > With this patch, the calgary code sets the global dma_ops to swiotlb
> > > or nommu properly, and the dma_ops of devices behind the
> > > Calgary/CalIOC2 to calgary_dma_ops. So the calgary code can handle
> > > devices safely that aren't behind the Calgary/CalIOC2.
> > > 
> > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > > ---
<snip>

Signed-off-by: Alexis Bruemmer <alexisb@us.ibm.com>

Index: linux-2.6.26-rc4/arch/x86/kernel/pci-calgary_64.c
===================================================================
--- linux-2.6.26-rc4.orig/arch/x86/kernel/pci-calgary_64.c
+++ linux-2.6.26-rc4/arch/x86/kernel/pci-calgary_64.c
@@ -36,6 +36,7 @@
 #include <linux/delay.h>
 #include <linux/scatterlist.h>
 #include <linux/iommu-helper.h>
+#include <asm/iommu.h>
 #include <asm/gart.h>
 #include <asm/calgary.h>
 #include <asm/tce.h>
@@ -410,22 +411,6 @@ static void calgary_unmap_sg(struct devi
 	}
 }
 
-static int calgary_nontranslate_map_sg(struct device* dev,
-	struct scatterlist *sg, int nelems, int direction)
-{
-	struct scatterlist *s;
-	int i;
-
-	for_each_sg(sg, s, nelems, i) {
-		struct page *p = sg_page(s);
-
-		BUG_ON(!p);
-		s->dma_address = virt_to_bus(sg_virt(s));
-		s->dma_length = s->length;
-	}
-	return nelems;
-}
-
 static int calgary_map_sg(struct device *dev, struct scatterlist *sg,
 	int nelems, int direction)
 {
@@ -436,9 +421,6 @@ static int calgary_map_sg(struct device 
 	unsigned long entry;
 	int i;
 
-	if (!translation_enabled(tbl))
-		return calgary_nontranslate_map_sg(dev, sg, nelems, direction);
-
 	for_each_sg(sg, s, nelems, i) {
 		BUG_ON(!sg_page(s));
 
@@ -474,7 +456,6 @@ error:
 static dma_addr_t calgary_map_single(struct device *dev, phys_addr_t paddr,
 	size_t size, int direction)
 {
-	dma_addr_t dma_handle = bad_dma_address;
 	void *vaddr = phys_to_virt(paddr);
 	unsigned long uaddr;
 	unsigned int npages;
@@ -483,12 +464,7 @@ static dma_addr_t calgary_map_single(str
 	uaddr = (unsigned long)vaddr;
 	npages = num_dma_pages(uaddr, size);
 
-	if (translation_enabled(tbl))
-		dma_handle = iommu_alloc(dev, tbl, vaddr, npages, direction);
-	else
-		dma_handle = virt_to_bus(vaddr);
-
-	return dma_handle;
+	return iommu_alloc(dev, tbl, vaddr, npages, direction);
 }
 
 static void calgary_unmap_single(struct device *dev, dma_addr_t dma_handle,
@@ -497,9 +473,6 @@ static void calgary_unmap_single(struct 
 	struct iommu_table *tbl = find_iommu_table(dev);
 	unsigned int npages;
 
-	if (!translation_enabled(tbl))
-		return;
-
 	npages = num_dma_pages(dma_handle, size);
 	iommu_free(tbl, dma_handle, npages);
 }
@@ -522,18 +495,12 @@ static void* calgary_alloc_coherent(stru
 		goto error;
 	memset(ret, 0, size);
 
-	if (translation_enabled(tbl)) {
-		/* set up tces to cover the allocated range */
-		mapping = iommu_alloc(dev, tbl, ret, npages, DMA_BIDIRECTIONAL);
-		if (mapping == bad_dma_address)
-			goto free;
-
-		*dma_handle = mapping;
-	} else /* non translated slot */
-		*dma_handle = virt_to_bus(ret);
-
+	/* set up tces to cover the allocated range */
+	mapping = iommu_alloc(dev, tbl, ret, npages, DMA_BIDIRECTIONAL);
+	if (mapping == bad_dma_address)
+		goto free;
+	*dma_handle = mapping;
 	return ret;
-
 free:
 	free_pages((unsigned long)ret, get_order(size));
 	ret = NULL;
@@ -1230,6 +1197,16 @@ static int __init calgary_init(void)
 			goto error;
 	} while (1);
 
+	dev = NULL;
+	for_each_pci_dev(dev) {
+		struct iommu_table *tbl;
+
+		tbl = find_iommu_table(&dev->dev);
+
+		if(translation_enabled(tbl))
+			dev->dev.archdata.dma_ops = &calgary_dma_ops;
+	}
+
 	return ret;
 
 error:
@@ -1251,6 +1228,7 @@ error:
 		calgary_disable_translation(dev);
 		calgary_free_bus(dev);
 		pci_dev_put(dev); /* Undo calgary_init_one()'s pci_dev_get() */
+		dev->dev.archdata.dma_ops = NULL;
 	} while (1);
 
 	return ret;
@@ -1430,6 +1408,10 @@ void __init detect_calgary(void)
 		printk(KERN_INFO "PCI-DMA: Calgary TCE table spec is %d, "
 		       "CONFIG_IOMMU_DEBUG is %s.\n", specified_table_size,
 		       debugging ? "enabled" : "disabled");
+
+		/* swiotlb for devices that aren't behind the Calgary. */
+		if (end_pfn > MAX_DMA32_PFN)
+			swiotlb = 1;
 	}
 	return;
 
@@ -1446,7 +1428,7 @@ int __init calgary_iommu_init(void)
 {
 	int ret;
 
-	if (no_iommu || swiotlb)
+	if (no_iommu || (swiotlb && !calgary_detected))
 		return -ENODEV;
 
 	if (!calgary_detected)
@@ -1459,15 +1441,15 @@ int __init calgary_iommu_init(void)
 	if (ret) {
 		printk(KERN_ERR "PCI-DMA: Calgary init failed %d, "
 		       "falling back to no_iommu\n", ret);
-		if (end_pfn > MAX_DMA32_PFN)
-			printk(KERN_ERR "WARNING more than 4GB of memory, "
-					"32bit PCI may malfunction.\n");
 		return ret;
 	}
 
 	force_iommu = 1;
 	bad_dma_address = 0x0;
-	dma_ops = &calgary_dma_ops;
+
+	/* dma_ops is set to swiotlb or nommu */
+	if (!dma_ops)
+		dma_ops = &nommu_dma_ops;
 
 	return 0;
 }
Index: linux-2.6.26-rc4/include/asm-x86/iommu.h
===================================================================
--- linux-2.6.26-rc4.orig/include/asm-x86/iommu.h
+++ linux-2.6.26-rc4/include/asm-x86/iommu.h
@@ -3,6 +3,7 @@
 
 extern void pci_iommu_shutdown(void);
 extern void no_iommu_init(void);
+extern struct dma_mapping_ops nommu_dma_ops;
 extern int force_iommu, no_iommu;
 extern int iommu_detected;
 #ifdef CONFIG_IOMMU