Re: [PATCH] powerpc: map 4K iommu pages even on 64K largepage systems

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

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 ...
> 
> > +++ linux-cell/include/asm-powerpc/iommu.h	2006-10-24 14:58:45.000000000 +1000
> > +
> > +#define IOMMU_PAGE_SHIFT      12
> 
> 
> > +++ 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).

> > +++ 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.

> > +++ 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
> 
> > +#define DART_PAGE_FACTOR	(IOMMU_PAGE_SHIFT - DART_PAGE_SHIFT)

See my above comment :)

> > +++ 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.