On Sat, Sep 27, 2008 at 03:35:27AM +0400, Ilya Yanok wrote:
> Hello David,
>
> David Gibson wrote:
>> 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.
>> 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.
>
>>> 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.
>>> +#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
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
