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

From: [email-masked] (Linas Vepstas)
Date: Tue, 24 Oct 2006 15:08:03 -0500

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:

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

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

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

--linas