[PATCH] iommu fixes, round 2

John Rose johnrose at austin.ibm.com
Wed Oct 27 04:03:01 EST 2004


Ben's patch went in before my note went out, so please disregard my
previous post.

All this might have been more easily addressed in one note on one list.
As opposed to posting three msgs on two lists and committing a patch
that creates code comments on proposed reorgs.  Might I humbly request
that our patches/reorg ideas sit on the ppc64 list for a bit before
pushing to Linus?  

Here's a patch that fixes the original build break, and removes the
ifdef's and comments that were added by Ben's patch.

We feel that iommu_free_table() is generic so we've moved it to iommu.c.
This fixes the build break (sorry g5 :).  It's complementary to
iommu_init_table(), which is generic.  

Secondly, the attempt to free the table in of_remove_node() is "as
symmetric as possible" with of_finish_node_dynamic(), where the table is
allocated.  If that's what the comment means by layering violation, I
humbly disagree. 

Thirdly, iommu_devnode_init() also has an iSeries implementation, so
it's not pSeries-specific.  No need to rename it, as suggested in the
comment.

Thanks-
John

Signed-off-by: John Rose <johnrose at austin.ibm.com>

diff -Nru a/arch/ppc64/kernel/iommu.c b/arch/ppc64/kernel/iommu.c
--- a/arch/ppc64/kernel/iommu.c	Tue Oct 26 12:51:42 2004
+++ b/arch/ppc64/kernel/iommu.c	Tue Oct 26 12:51:42 2004
@@ -425,6 +425,39 @@
 	return tbl;
 }
 
+void iommu_free_table(struct device_node *dn)
+{
+	struct iommu_table *tbl = dn->iommu_table;
+	unsigned long bitmap_sz, i;
+	unsigned int order;
+
+	if (!tbl || !tbl->it_map) {
+		printk(KERN_ERR "%s: expected TCE map for %s\n", __FUNCTION__,
+				dn->full_name);
+		return;
+	}
+
+	/* verify that table contains no entries */
+	/* it_mapsize is in entries, and we're examining 64 at a time */
+	for (i = 0; i < (tbl->it_mapsize/64); i++) {
+		if (tbl->it_map[i] != 0) {
+			printk(KERN_WARNING "%s: Unexpected TCEs for %s\n",
+				__FUNCTION__, dn->full_name);
+			break;
+		}
+	}
+
+	/* calculate bitmap size in bytes */
+	bitmap_sz = (tbl->it_mapsize + 7) / 8;
+
+	/* free bitmap */
+	order = get_order(bitmap_sz);
+	free_pages((unsigned long) tbl->it_map, order);
+
+	/* free table */
+	kfree(tbl);
+}
+
 /* Creates TCEs for a user provided buffer.  The user buffer must be
  * contiguous real kernel storage (not vmalloc).  The address of the buffer
  * passed here is the kernel (virtual) address of the buffer.  The buffer
diff -Nru a/arch/ppc64/kernel/pSeries_iommu.c b/arch/ppc64/kernel/pSeries_iommu.c
--- a/arch/ppc64/kernel/pSeries_iommu.c	Tue Oct 26 12:51:42 2004
+++ b/arch/ppc64/kernel/pSeries_iommu.c	Tue Oct 26 12:51:42 2004
@@ -412,39 +412,6 @@
 	dn->iommu_table = iommu_init_table(tbl);
 }
 
-void iommu_free_table(struct device_node *dn)
-{
-	struct iommu_table *tbl = dn->iommu_table;
-	unsigned long bitmap_sz, i;
-	unsigned int order;
-
-	if (!tbl || !tbl->it_map) {
-		printk(KERN_ERR "%s: expected TCE map for %s\n", __FUNCTION__,
-				dn->full_name);
-		return;
-	}
-
-	/* verify that table contains no entries */
-	/* it_mapsize is in entries, and we're examining 64 at a time */
-	for (i = 0; i < (tbl->it_mapsize/64); i++) {
-		if (tbl->it_map[i] != 0) {
-			printk(KERN_WARNING "%s: Unexpected TCEs for %s\n",
-				__FUNCTION__, dn->full_name);
-			break;
-		}
-	}
-
-	/* calculate bitmap size in bytes */
-	bitmap_sz = (tbl->it_mapsize + 7) / 8;
-
-	/* free bitmap */
-	order = get_order(bitmap_sz);
-	free_pages((unsigned long) tbl->it_map, order);
-
-	/* free table */
-	kfree(tbl);
-}
-
 void iommu_setup_pSeries(void)
 {
 	struct pci_dev *dev = NULL;
diff -Nru a/arch/ppc64/kernel/prom.c b/arch/ppc64/kernel/prom.c
--- a/arch/ppc64/kernel/prom.c	Tue Oct 26 12:51:42 2004
+++ b/arch/ppc64/kernel/prom.c	Tue Oct 26 12:51:42 2004
@@ -1818,13 +1818,8 @@
 		return -EBUSY;
 	}
 
-	/* XXX This is a layering violation, should be moved to the caller
-	 * --BenH.
-	 */
-#ifdef CONFIG_PPC_PSERIES
-	if (np->iommu_table)
+	if ((np->iommu_table) && get_property(np, "ibm,dma-window", NULL))
 		iommu_free_table(np);
-#endif /* CONFIG_PPC_PSERIES */
 
 	write_lock(&devtree_lock);
 	OF_MARK_STALE(np);
diff -Nru a/include/asm-ppc64/iommu.h b/include/asm-ppc64/iommu.h
--- a/include/asm-ppc64/iommu.h	Tue Oct 26 12:51:42 2004
+++ b/include/asm-ppc64/iommu.h	Tue Oct 26 12:51:42 2004
@@ -111,17 +111,9 @@
 extern void iommu_setup_u3(void);
 
 /* Creates table for an individual device node */
-/* XXX: This isn't generic, please name it accordingly or add
- * some ppc_md. hooks for iommu implementations to do what they
- * need to do. --BenH.
- */
 extern void iommu_devnode_init(struct device_node *dn);
 
 /* Frees table for an individual device node */
-/* XXX: This isn't generic, please name it accordingly or add
- * some ppc_md. hooks for iommu implementations to do what they
- * need to do. --BenH.
- */
 extern void iommu_free_table(struct device_node *dn);
 
 #endif /* CONFIG_PPC_MULTIPLATFORM */




More information about the Linuxppc64-dev mailing list