Skip to contentSkip to conversation

x86 calgary: fix handling of devces that aren't behind the Calgary

PATCH In review FUJITA Tomonori 3 participants 8 messages May 31, 2008 – Jun 12, 2008 x86

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 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:

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 <[email protected]>
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.

---
 arch/x86/kernel/pci-calgary_64.c |   63 +++++++++++---------------------------
 include/asm-x86/iommu.h          |    1 +
 2 files changed, 19 insertions(+), 45 deletions(-)
arch/x86/kernel/pci-calgary_64.c +18 -45
index dca9a82..1839968 100644
@@ -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;
 }
include/asm-x86/iommu.h +1 -0
index 07862fd..1a5e7df 100644
@@ -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
Signature · 1 line
1.5.4.2
#1 lore raw
Quoted 10 lines · 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.

Quoted 4 lines

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

#2 lore raw
Quoted 12 lines · On Tue, 3 Jun 2008 08:21:46 +0300 Muli Ben-Yehuda <[email protected]> wrote:
Quoted 10 lines · 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.

Quoted 7 lines
Quoted 4 lines

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.

Quoted 5 lines

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.

Quoted 2 lines

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.

#3 lore raw
In reply to Muli Ben-Yehuda
Quoted 24 lines · Muli Ben-Yehuda
Quoted 10 lines · 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.

Quoted 4 lines

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.

Quoted 2 lines

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

Quoted 5 lines

Cheers,
Muli

#4 lore raw
Quoted 42 lines · On Tue, 03 Jun 2008 09:55:37 -0700 Alexis Bruemmer <[email protected]> wrote:
Quoted 24 lines · Muli Ben-Yehuda
Quoted 10 lines · 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.

Quoted 4 lines

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.

Quoted 2 lines

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
Quoted 3 lines

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.

arch/x86/kernel/pci-dma.c +15 -10
index 4471984..bc2251d 100644
@@ -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);
#5 lore raw
In reply to FUJITA Tomonori
Quoted 14 lines · 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.

Quoted 8 lines

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

Quoted 16 lines

Thanks,

==
From: FUJITA Tomonori <[email protected]>
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.

---

<snip>

 arch/x86/kernel/pci-calgary_64.c |   63 +++++++++++---------------------------
 include/asm-x86/iommu.h          |    1 +
 2 files changed, 19 insertions(+), 45 deletions(-)
arch/x86/kernel/pci-calgary_64.c +32 -45
Index: 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;
 }
include/asm-x86/iommu.h +1 -0
Index: 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
#6 lore raw
Quoted 23 lines · On Wed, 11 Jun 2008 18:25:06 -0700 Alexis Bruemmer <[email protected]> wrote:
Quoted 14 lines · 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.

Quoted 8 lines

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.

Quoted 9 lines

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.

Quoted 3 lines

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.

Quoted 1 line

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.

Quoted 32 lines

Alexis

Quoted 16 lines

Thanks,

==
From: FUJITA Tomonori <[email protected]>
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.

---

<snip>

arch/x86/kernel/pci-calgary_64.c +4 -0
@@ -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);

arch/x86/kernel/pci-calgary_64.c static int __init calgary_init(void) · line 1213

Quoted 10 lines
arch/x86/kernel/pci-calgary_64.c +10 -0
+
+		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;
}
#7 lore raw
Quoted 36 lines · FUJITA Tomonori
Quoted 23 lines · On Wed, 11 Jun 2008 18:25:06 -0700 Alexis Bruemmer <[email protected]> wrote:
Quoted 14 lines · 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.

Quoted 8 lines

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.

Quoted 9 lines

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.

Quoted 8 lines

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

Quoted 3 lines

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.

Quoted 25 lines
Quoted 2 lines

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.

Quoted 19 lines

Alexis

Quoted 16 lines

Thanks,

==
From: FUJITA Tomonori <[email protected]>
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.

---

<snip>

arch/x86/kernel/pci-calgary_64.c +27 -45
Index: 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;
 }
include/asm-x86/iommu.h +1 -0
Index: 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