Skip to contentSkip to conversation

x86-64: Calgary IOMMU updates

PATCH In review Muli Ben-Yehuda 3 participants 11 messages Jul 25, 2006 – Jul 26, 2006

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 x86-64: Calgary IOMMU updates

Hi Andi,

This patchset contains a few Calgary bug fixes (mostly in the error handling) and a few harmless associated cleanups (e.g., rearranging structures for better alignment). It would be good to get these, especially the bug fixes, into 2.6.18.

Thanks,
Muli\

4 files changed, 52 insertions(+), 50 deletions(-)
arch/x86_64/kernel/pci-calgary.c |   85 +++++++++++++++++++++-----------------
arch/x86_64/kernel/tce.c         |   10 ----
include/asm-x86_64/calgary.h     |    6 +-
include/asm-x86_64/tce.h         |    1 
#1 lore raw

PATCH 1/1 Calgary IOMMU: rearrange 'struct iommu_table' members

1 files changed, 3 insertions(+), 3 deletions(-)

include/asm-x86_64/calgary.h | 6 +++---

# HG changeset patch
# User Muli Ben-Yehuda <[email protected]>
# Date 1153732042 -10800
# Node ID bc678333b410a6aa7bf86a97f314a62c300dae3a
# Parent 8b88f7d9cf044a7b43aff57dd019b6786d1ab66e [x86-64] Calgary IOMMU: rearrange 'struct iommu_table' members

Rearrange struct members loosely based on size for improved alignment and to save a few bytes.

include/asm-x86_64/calgary.h +3 -3
diff -r 8b88f7d9cf04 -r bc678333b410 include/asm-x86_64/calgary.h
@@ -34,12 +34,12 @@ struct iommu_table {
 	unsigned long  it_base;      /* mapped address of tce table */
 	unsigned long  it_hint;      /* Hint for next alloc */
 	unsigned long *it_map;       /* A simple allocation bitmap for now */
+	void __iomem  *bbar;         /* Bridge BAR */
+	u64	       tar_val;      /* Table Address Register */
+	struct timer_list watchdog_timer;
 	spinlock_t     it_lock;      /* Protects it_map */
 	unsigned int   it_size;      /* Size of iommu table in entries */
 	unsigned char  it_busno;     /* Bus number this table belongs to */
-	void __iomem  *bbar;
-	u64	       tar_val;
-	struct timer_list watchdog_timer;
 };
 
 #define TCE_TABLE_SIZE_UNSPECIFIED	~0
#2 lore raw

PATCH 1/1 Calgary IOMMU: consolidate per bus data structures

In reply to Muli Ben-Yehuda
3 files changed, 29 insertions(+), 37 deletions(-)
arch/x86_64/kernel/pci-calgary.c |   55 ++++++++++++++++++++------------------
arch/x86_64/kernel/tce.c         |   10 ------
include/asm-x86_64/tce.h         |    1 

# HG changeset patch
# User Muli Ben-Yehuda <[email protected]>
# Date 1153735276 -10800
# Node ID f85ffa73fb99f4796043a8e50bef1180e36fe582 # Parent bc678333b410a6aa7bf86a97f314a62c300dae3a [x86-64] Calgary IOMMU: consolidate per bus data structures

Move the tce_table_kva array, disabled bitmap and bus_to_phb array into a new per bus 'struct calgary_bus_info'. Also slightly reorganize build_tce_table and tce_table_setparms to avoid exporting bus_info to tce.c.

arch/x86_64/kernel/pci-calgary.c +29 -26
diff -r bc678333b410 -r f85ffa73fb99 arch/x86_64/kernel/pci-calgary.c
@@ -111,17 +111,17 @@ static const unsigned long phb_offsets[]
 	0xB000 /* PHB3 */
 };
 
-static char bus_to_phb[MAX_PHB_BUS_NUM];
-void* tce_table_kva[MAX_PHB_BUS_NUM];
 unsigned int specified_table_size = TCE_TABLE_SIZE_UNSPECIFIED;
 static int translate_empty_slots __read_mostly = 0;
 static int calgary_detected __read_mostly = 0;
 
-/*
- * the bitmap of PHBs the user requested that we disable
- * translation on.
- */
-static DECLARE_BITMAP(translation_disabled, MAX_PHB_BUS_NUM);
+struct calgary_bus_info {
+	void *tce_space;
+	int translation_disabled;
+	signed char phbid;
+};
+
+static struct calgary_bus_info bus_info[MAX_PHB_BUS_NUM] = { { NULL, 0, 0 }, };
 
 static void tce_cache_blast(struct iommu_table *tbl);
 
@@ -149,7 +149,7 @@ static inline unsigned int num_dma_pages
 
 static inline int translate_phb(struct pci_dev* dev)
 {
-	int disabled = test_bit(dev->bus->number, translation_disabled);
+	int disabled = bus_info[dev->bus->number].translation_disabled;
 	return !disabled;
 }
 
@@ -454,7 +454,7 @@ static struct dma_mapping_ops calgary_dm
 
 static inline int busno_to_phbid(unsigned char num)
 {
-	return bus_to_phb[num];
+	return bus_info[num].phbid;
 }
 
 static inline unsigned long split_queue_offset(unsigned char num)
@@ -630,6 +630,10 @@ static int __init calgary_setup_tar(stru
 	ret = build_tce_table(dev, bbar);
 	if (ret)
 		return ret;
+
+	tbl = dev->sysdata;
+	tbl->it_base = (unsigned long)bus_info[dev->bus->number].tce_space;
+	tce_free(tbl, 0, tbl->it_size);
 
 	calgary_reserve_regions(dev);
 
@@ -824,7 +828,7 @@ static int __init calgary_init(void)
 			calgary_init_one_nontraslated(dev);
 			continue;
 		}
-		if (!tce_table_kva[dev->bus->number] && !translate_empty_slots) {
+		if (!bus_info[dev->bus->number].tce_space && !translate_empty_slots) {
 			pci_dev_put(dev);
 			continue;
 		}
@@ -844,7 +848,7 @@ error:
 			pci_dev_put(dev);
 			continue;
 		}
-		if (!tce_table_kva[dev->bus->number] && !translate_empty_slots)
+		if (!bus_info[dev->bus->number].tce_space && !translate_empty_slots)
 			continue;
 		calgary_disable_translation(dev);
 		calgary_free_tar(dev);
@@ -894,25 +898,21 @@ void __init detect_calgary(void)
 
 	for (bus = 0; bus < MAX_PHB_BUS_NUM; bus++) {
 		int dev;
-
-		tce_table_kva[bus] = NULL;
-		bus_to_phb[bus] = -1;
+		struct calgary_bus_info *info = &bus_info[bus];
+		info->phbid = -1;
 
 		if (read_pci_config(bus, 0, 0, 0) != PCI_VENDOR_DEVICE_ID_CALGARY)
 			continue;
 
-		/* 
+		/*
 		 * There are 4 PHBs per Calgary chip.  Set phb to which phb (0-3)
 		 * it is connected to releative to the clagary chip.
 		 */
 		phb = (phb + 1) % PHBS_PER_CALGARY;
 
-		if (test_bit(bus, translation_disabled)) {
-			printk(KERN_INFO "Calgary: translation is disabled for "
-			       "PHB 0x%x\n", bus);
-			/* skip this phb, don't allocate a tbl for it */
+		if (info->translation_disabled)
 			continue;
-		}
+
 		/*
 		 * Scan the slots of the PCI bus to see if there is a device present.
 		 * The parent bus will be the zero-ith device, so start at 1.
@@ -923,8 +923,8 @@ void __init detect_calgary(void)
 				tbl = alloc_tce_table();
 				if (!tbl)
 					goto cleanup;
-				tce_table_kva[bus] = tbl;
-				bus_to_phb[bus] = phb;
+				info->tce_space = tbl;
+				info->phbid = phb;
 				calgary_found = 1;
 				break;
 			}
@@ -940,9 +940,12 @@ void __init detect_calgary(void)
 	return;
 
 cleanup:
-	for (--bus; bus >= 0; --bus)
-		if (tce_table_kva[bus])
-			free_tce_table(tce_table_kva[bus]);
+	for (--bus; bus >= 0; --bus) {
+		struct calgary_bus_info *info = &bus_info[bus];
+
+		if (info->tce_space)
+			free_tce_table(info->tce_space);
+	}
 }
 
 int __init calgary_iommu_init(void)
@@ -1016,7 +1019,7 @@ static int __init calgary_parse_options(
 			if (bridge < MAX_PHB_BUS_NUM) {
 				printk(KERN_INFO "Calgary: disabling "
 				       "translation for PHB 0x%x\n", bridge);
-				set_bit(bridge, translation_disabled);
+				bus_info[bridge].translation_disabled = 1;
 			}
 		}
 
arch/x86_64/kernel/tce.c +0 -10
diff -r bc678333b410 -r f85ffa73fb99 arch/x86_64/kernel/tce.c
@@ -104,14 +104,6 @@ static int tce_table_setparms(struct pci
 	/* set the tce table size - measured in entries */
 	tbl->it_size = table_size_to_number_of_entries(specified_table_size);
 
-	tbl->it_base = (unsigned long)tce_table_kva[dev->bus->number];
-	if (!tbl->it_base) {
-		printk(KERN_ERR "Calgary: iommu_table_setparms: "
-		       "no table allocated?!\n");
-		ret = -ENOMEM;
-		goto done;
-	}
-
 	/*
 	 * number of bytes needed for the bitmap size in number of
 	 * entries; we need one bit per entry
@@ -160,8 +152,6 @@ int build_tce_table(struct pci_dev *dev,
 	if (ret)
 		goto free_tbl;
 
-	tce_free(tbl, 0, tbl->it_size);
-
 	tbl->bbar = bbar;
 
 	/*
include/asm-x86_64/tce.h +0 -1
diff -r bc678333b410 -r f85ffa73fb99 include/asm-x86_64/tce.h
@@ -24,7 +24,6 @@
 #ifndef _ASM_X86_64_TCE_H
 #define _ASM_X86_64_TCE_H
 
-extern void* tce_table_kva[];
 extern unsigned int specified_table_size;
 struct iommu_table;
 
#3 lore raw

PATCH 1/1 Calgary IOMMU: break out of pci_find_device_reverse if dev not found

In reply to Muli Ben-Yehuda
1 files changed, 2 insertions(+)

arch/x86_64/kernel/pci-calgary.c | 2 ++

# HG changeset patch
# User Muli Ben-Yehuda <[email protected]>
# Date 1153737003 -10800
# Node ID 4b8fbf25700873a70eff09264e52e9f6c6330c18
# Parent f85ffa73fb99f4796043a8e50bef1180e36fe582
[x86-64] Calgary IOMMU: break out of pci_find_device_reverse if dev not found

arch/x86_64/kernel/pci-calgary.c +2 -0
diff -r f85ffa73fb99 -r 4b8fbf257008 arch/x86_64/kernel/pci-calgary.c
@@ -844,6 +844,8 @@ error:
 		dev = pci_find_device_reverse(PCI_VENDOR_ID_IBM,
 					      PCI_DEVICE_ID_IBM_CALGARY,
 					      dev);
+		if (!dev)
+			break;
 		if (!translate_phb(dev)) {
 			pci_dev_put(dev);
 			continue;
#4 lore raw

PATCH 1/1 Calgary IOMMU: fix error path memleak in calgary_free_tar

In reply to Muli Ben-Yehuda
1 files changed, 10 insertions(+), 2 deletions(-)

arch/x86_64/kernel/pci-calgary.c | 12 ++++++++++--

# HG changeset patch
# User Muli Ben-Yehuda <[email protected]>
# Date 1153737408 -10800
# Node ID 7b1cdbc92f2c618cb59feaf472c8b76df875743d
# Parent 4b8fbf25700873a70eff09264e52e9f6c6330c18 [x86-64] Calgary IOMMU: fix error path memleak in calgary_free_tar

We were freeing the iommu_table and leaking the bitmap pages. Also rename it to calgary_free_bus, which is more accurate.

arch/x86_64/kernel/pci-calgary.c +10 -2
diff -r 4b8fbf257008 -r 7b1cdbc92f2c arch/x86_64/kernel/pci-calgary.c
@@ -658,11 +658,12 @@ static int __init calgary_setup_tar(stru
 	return 0;
 }
 
-static void __init calgary_free_tar(struct pci_dev *dev)
+static void __init calgary_free_bus(struct pci_dev *dev)
 {
 	u64 val64;
 	struct iommu_table *tbl = dev->sysdata;
 	void __iomem *target;
+	unsigned int bitmapsz;
 
 	target = calgary_reg(tbl->bbar, tar_offset(dev->bus->number));
 	val64 = be64_to_cpu(readq(target));
@@ -670,8 +671,15 @@ static void __init calgary_free_tar(stru
 	writeq(cpu_to_be64(val64), target);
 	readq(target); /* flush */
 
+	bitmapsz = tbl->it_size / BITS_PER_BYTE;
+	free_pages((unsigned long)tbl->it_map, get_order(bitmapsz));
+	tbl->it_map = NULL;
+
 	kfree(tbl);
 	dev->sysdata = NULL;
+
+	/* Can't free bootmem allocated memory after system is up :-( */
+	bus_info[dev->bus->number].tce_space = NULL;
 }
 
 static void calgary_watchdog(unsigned long data)
@@ -853,7 +861,7 @@ error:
 		if (!bus_info[dev->bus->number].tce_space && !translate_empty_slots)
 			continue;
 		calgary_disable_translation(dev);
-		calgary_free_tar(dev);
+		calgary_free_bus(dev);
 		pci_dev_put(dev);
 	}
 
#5 lore raw

PATCH 1/1 Calgary IOMMU: save a bit of space in bus_info

In reply to Muli Ben-Yehuda
1 files changed, 1 insertion(+), 1 deletion(-)

arch/x86_64/kernel/pci-calgary.c | 2 +-

# HG changeset patch
# User Muli Ben-Yehuda <[email protected]>
# Date 1153781666 -10800
# Node ID 8f45fff682d3bf9aea2328b1ded97745285be538 # Parent 529b8e81c7603717ddd3782899d3b0159d225b5b [x86-64] Calgary IOMMU: save a bit of space in bus_info

Make translation_disabled a uchar rather than an int

arch/x86_64/kernel/pci-calgary.c +1 -1
diff -r 529b8e81c760 -r 8f45fff682d3 arch/x86_64/kernel/pci-calgary.c
@@ -117,7 +117,7 @@ static int calgary_detected __read_mostl
 
 struct calgary_bus_info {
 	void *tce_space;
-	int translation_disabled;
+	unsigned char translation_disabled;
 	signed char phbid;
 };
 
#6 lore raw

PATCH 1/1 Calgary IOMMU: fix reference counting of Calgary PCI devices

In reply to Muli Ben-Yehuda
1 files changed, 6 insertions(+), 4 deletions(-)

arch/x86_64/kernel/pci-calgary.c | 10 ++++++----

# HG changeset patch
# User Muli Ben-Yehuda <[email protected]>
# Date 1153737943 -10800
# Node ID a42c3826807abbeacc40649127afb5103aabdbd4
# Parent 7b1cdbc92f2c618cb59feaf472c8b76df875743d [x86-64] Calgary IOMMU: fix reference counting of Calgary PCI devices

The pci_get_device() API decrements the reference count on the 'from' parameter when it continues searching. Therefore, take a ref count on Calgary bus when we initialize them in either translated or non-translated mode.

arch/x86_64/kernel/pci-calgary.c +6 -4
diff -r 7b1cdbc92f2c -r a42c3826807a arch/x86_64/kernel/pci-calgary.c
@@ -786,6 +786,7 @@ static inline unsigned int __init locate
 
 static int __init calgary_init_one_nontraslated(struct pci_dev *dev)
 {
+	pci_dev_get(dev);
 	dev->sysdata = NULL;
 	dev->bus->self = dev;
 
@@ -810,6 +811,7 @@ static int __init calgary_init_one(struc
 	if (ret)
 		goto iounmap;
 
+	pci_dev_get(dev);
 	dev->bus->self = dev;
 	calgary_enable_translation(dev);
 
@@ -836,10 +838,9 @@ static int __init calgary_init(void)
 			calgary_init_one_nontraslated(dev);
 			continue;
 		}
-		if (!bus_info[dev->bus->number].tce_space && !translate_empty_slots) {
-			pci_dev_put(dev);
+		if (!bus_info[dev->bus->number].tce_space && !translate_empty_slots)
 			continue;
-		}
+
 		ret = calgary_init_one(dev);
 		if (ret)
 			goto error;
@@ -860,9 +861,10 @@ error:
 		}
 		if (!bus_info[dev->bus->number].tce_space && !translate_empty_slots)
 			continue;
+
 		calgary_disable_translation(dev);
 		calgary_free_bus(dev);
-		pci_dev_put(dev);
+		pci_dev_put(dev); /* Undo calgary_init_one()'s pci_dev_get() */
 	}
 
 	return ret;
#7 lore raw

PATCH 1/1 Calgary IOMMU: calgary_init_one_nontraslated() can return void

In reply to Muli Ben-Yehuda
1 files changed, 1 insertion(+), 3 deletions(-)

arch/x86_64/kernel/pci-calgary.c | 4 +---

# HG changeset patch
# User Muli Ben-Yehuda <[email protected]>
# Date 1153738218 -10800
# Node ID 529b8e81c7603717ddd3782899d3b0159d225b5b
# Parent a42c3826807abbeacc40649127afb5103aabdbd4
[x86-64] Calgary IOMMU: calgary_init_one_nontraslated() can return void

arch/x86_64/kernel/pci-calgary.c +1 -3
diff -r a42c3826807a -r 529b8e81c760 arch/x86_64/kernel/pci-calgary.c
@@ -784,13 +784,11 @@ static inline unsigned int __init locate
 	return address;
 }
 
-static int __init calgary_init_one_nontraslated(struct pci_dev *dev)
+static void __init calgary_init_one_nontraslated(struct pci_dev *dev)
 {
 	pci_dev_get(dev);
 	dev->sysdata = NULL;
 	dev->bus->self = dev;
-
-	return 0;
 }
 
 static int __init calgary_init_one(struct pci_dev *dev)
#8 lore raw
In reply to Muli Ben-Yehuda
Quoted 6 lines · Muli Ben-Yehuda

Hi Andi,

This patchset contains a few Calgary bug fixes (mostly in the error handling) and a few harmless associated cleanups (e.g., rearranging structures for better alignment). It would be good to get these, especially the bug fixes, into 2.6.18.

How do these patches relate to the two earlier patches that Jon sent?

2.6.18 is closed for anything but bug fixes for serious bugs.

-Andi

#9 lore raw
Quoted 9 lines · Andi Kleen
Quoted 6 lines · Muli Ben-Yehuda

Hi Andi,

This patchset contains a few Calgary bug fixes (mostly in the error handling) and a few harmless associated cleanups (e.g., rearranging structures for better alignment). It would be good to get these, especially the bug fixes, into 2.6.18.

How do these patches relate to the two earlier patches that Jon sent?

These patches are ontop of the patches I sent (and which Muli resent a few days afterward). Those patches fix a major problem and need to make it into 2.6.18.

Quoted 2 lines

2.6.18 is closed for anything but bug fixes for serious bugs.

This is mostly error path bugs and clean-up/code re-org (to save memory and make things more cache friendly). The changes are mostly minor. If you are uncomfortable with applying them, we can separate patches 3, 4, and 5 (which are the bug fixes) from the rest and send those to you. Is that more acceptable?

Thanks,
Jon

Quoted 2 lines

-Andi

#10 lore raw
Quoted 7 lines
Quoted 4 lines

2.6.18 is closed for anything but bug fixes for serious bugs.

This is mostly error path bugs and clean-up/code re-org (to save memory and make things more cache friendly). The changes are mostly minor.

Doesn't sound very critical for .18 then and I will delay it to .19

-Andi