Re: [PATCH] powerpc: add support for PAGE_SIZEs greater than 4KB for

On Sat, Sep 27, 2008 at 03:35:27AM +0400, Ilya Yanok wrote:

Hello David,

David Gibson wrote:

quoted

I don't see any reason to have a separate set of config options for 32
and 64-bit.  Just make the once choice, but only have the individual
pagesize options enabled on machines that support them.

Well. I can see some. First, on PPC64 kernel emulates 64K pages on  
hardware that can't do it and we are not going to do such an emulation  
on PPC32 now.

So?

Then CONFIG_PPC_64K_PAGES selects PPC_HAS_HASH_64K and our  
code has nothing to do with it. 

Well, obviously the generic 64K option wouldn't select
PPC_HAS_HASH_64K.  That would be dependent on both 64K_PAGES and
PPC64.

And last but not least, we don't use  
PPC64 kernels for now so we just tried not to break something we can't  
test. But if everybody thinks that having a single option is a good idea  
I'll do it that way.

Hrm, well that has something to be said for it.  But it's not hard to
at least build a ppc64 kernel to test if you've broken that.

quoted

I don't think you should need a real_pte_t type for the 32-bit
implementation.  It's just there because of how we implement
64k granularity page allocation on hardware that only does 4k
translations.

You are right. Thanks.

quoted

quoted

diff --git a/arch/powerpc/include/asm/page_32.h b/arch/powerpc/include/asm/page_32.h
index ebfae53..d176270 100644
--- a/arch/powerpc/include/asm/page_32.h
+++ b/arch/powerpc/include/asm/page_32.h

@@ -20,7 +20,11 @@
  */
 #ifdef CONFIG_PTE_64BIT
 typedef unsigned long long pte_basic_t;
+#ifdef CONFIG_PPC32_256K_PAGES
+#define PTE_SHIFT	(PAGE_SHIFT - 7)
    

This doesn't look right.  You should be eliding one of the levels of
page table if you don't need it, rather than leaving the bottom level
PTE page largely empty.

Hm... We have only two levels really so if we elide one there will be  
only one left. Don't sure if kernel can work with this...

Ah.. that's a point.  But again this is a 256K specific hack, so we
can worry about it later.

quoted

quoted

+#if (PAGE_SHIFT == 12)
+/*
+ * PAGE_SIZE  4K
+ * PAGE_SHIFT 12
+ * PTE_SHIFT   9
+ * PMD_SHIFT  21
+ */
+#define PPC44x_TLBE_SIZE	PPC44x_TLB_4K
+#define PPC44x_PGD_OFF_SH	13 /*(32 - PMD_SHIFT + 2)*/
+#define PPC44x_PGD_OFF_M1	19 /*(PMD_SHIFT - 2)*/
+#define PPC44x_PTE_ADD_SH	23 /*32 - PMD_SHIFT + PTE_SHIFT + 3*/
+#define PPC44x_PTE_ADD_M1	20 /*32 - 3 - PTE_SHIFT*/
+#define PPC44x_RPN_M2		19 /*31 - PAGE_SHIFT*/
    

Uh.. you have the formulae for these things right there in the
comments, so why aren't you using those and avoiding this nasty
multiway ifdef...

We need to get PMD_SHIFT and friends out of #ifndef __ASSEMBLY__ for  
that. And some of them are under include/asm-generic so patch becomes  
not powerpc-specific...

So use arch/powerpc/kernel/asm-offsets.c, that's what it's for.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson