On Tue, 2006-10-24 at 15:08 -0500, Linas Vepstas wrote:

Hi Ben,

The patch tests well; bt there was one change I didn't understand ...

quoted

+++ linux-cell/include/asm-powerpc/iommu.h	2006-10-24 14:58:45.000000000 +1000
+
+#define IOMMU_PAGE_SHIFT      12

quoted

+++ linux-cell/include/asm-powerpc/tce.h	2006-10-24 14:59:20.000000000 +1000
 
 #define TCE_SHIFT	12
 #define TCE_PAGE_SIZE	(1 << TCE_SHIFT)
-#define TCE_PAGE_FACTOR	(PAGE_SHIFT - TCE_SHIFT)

This is zero now, but if anyone ever changes IOMMU_PAGE_SHIFT to a
value oter than 12,  then the below will break:

My point is that we'll probably not change IOMMU_PAGE_SHIFT. The only
thing we might do is to move the shift into the iommu_table structure to
make it per-iommu (in which case the TCE backend will use 4k or 64k
depending on actual HW/FW support for those sizes).

quoted

+++ linux-cell/arch/powerpc/platforms/pseries/iommu.c	2006-10-24 15:00:07.000000000 +1000
 
-	index <<= TCE_PAGE_FACTOR;
-	npages <<= TCE_PAGE_FACTOR;

since this shift does need to be made, if IOMMU_PAGE_SHIFT != TCE_SHIFT

Which will not happen.

quoted

+++ linux-cell/arch/powerpc/sysdev/dart.h	2006-10-24 15:01:16.000000000 +1000
 
 #define DART_PAGE_SHIFT		12
 #define DART_PAGE_SIZE		(1 << DART_PAGE_SHIFT)
-#define DART_PAGE_FACTOR	(PAGE_SHIFT - DART_PAGE_SHIFT)

I'd argue that the right fix would have been

quoted

+#define DART_PAGE_FACTOR	(IOMMU_PAGE_SHIFT - DART_PAGE_SHIFT)

See my above comment :)

quoted

+++ linux-cell/arch/powerpc/sysdev/dart_iommu.c	2006-10-24 15:01:47.000000000 +1000
 
-	index <<= DART_PAGE_FACTOR;
-	npages <<= DART_PAGE_FACTOR;

And do *not* remove these lines... certainly, they would have to be
put back in if we made iommu_page_size to be a variable in the iommu
table..

No. DART would set the table shift to DART_PAGE_SHIFT, TCE would set it
to TCE_PAGE_SHIFT and the backend would always get natively sized
addresses/counts.

Ben.