linux-kernel - Re: [RFC PATCH v2 15/20] powerpc/85xx: Switch to 64 bits PGD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1281454b-5025-4f18-b806-ca28b5136642@csgroup.eu>
Date: Sat, 25 May 2024 09:02:03 +0000
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Oscar Salvador <osalvador@...e.de>
CC: Andrew Morton <akpm@...ux-foundation.org>, Jason Gunthorpe
	<jgg@...dia.com>, Peter Xu <peterx@...hat.com>, Michael Ellerman
	<mpe@...erman.id.au>, Nicholas Piggin <npiggin@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>, "linuxppc-dev@...ts.ozlabs.org"
	<linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [RFC PATCH v2 15/20] powerpc/85xx: Switch to 64 bits PGD



Le 25/05/2024 à 06:54, Oscar Salvador a écrit :
> On Fri, May 17, 2024 at 09:00:09PM +0200, Christophe Leroy wrote:
>> In order to allow leaf PMD entries, switch the PGD to 64 bits entries.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
> 
> I do not quite understand this change.
> Are not powerE500 and power85xx two different things?

Yes they are two different things, but one contains the other

e500 is the processor-core which is included inside the MPC85xx micro 
controller.

But CONFIG_PPC_E500 is a bit more than e500 core, it also includes e5500 
and e6500 which are evolutions of e500.

mpc85xx is 32 bits
e5500 and e6500 are 64 bits



> You are changing making it 64 for PPC_E500_64bits, but you are updating head_85xx.
> Are they sharing this code?

Not exactly. mpc85xx can be built with 32 bits PTE or 64 bits PTE, based 
on CONFIG_PTE_64BIT

When CONFIG_PTE_64BIT is selected it uses the same PTE layout on 32-bits 
and 64-bits. But on 32-bits the PGD is still 32-bits, so it is not 
possible to use leaf entries at PGD level hence the change.

When CONFIG_PTE_64BIT is not selected, huge pages are not supported.

> 
> Also, we would benefit from a slightly bigger changelog, explaining why
> do we need this change in some more detail.

Yes I can write this is because PTEs are 64-bits allthought I thought it 
was obvious.

> 
>   
>> diff --git a/arch/powerpc/include/asm/pgtable-types.h b/arch/powerpc/include/asm/pgtable-types.h
>> index 082c85cc09b1..db965d98e0ae 100644
>> --- a/arch/powerpc/include/asm/pgtable-types.h
>> +++ b/arch/powerpc/include/asm/pgtable-types.h
>> @@ -49,7 +49,11 @@ static inline unsigned long pud_val(pud_t x)
>>   #endif /* CONFIG_PPC64 */
>>   
>>   /* PGD level */
>> +#if defined(CONFIG_PPC_E500) && defined(CONFIG_PTE_64BIT)
>> +typedef struct { unsigned long long pgd; } pgd_t;
>> +#else
>>   typedef struct { unsigned long pgd; } pgd_t;
>> +#endif
>>   #define __pgd(x)	((pgd_t) { (x) })
>>   static inline unsigned long pgd_val(pgd_t x)
>>   {
>> diff --git a/arch/powerpc/kernel/head_85xx.S b/arch/powerpc/kernel/head_85xx.S
>> index 39724ff5ae1f..a305244afc9f 100644
>> --- a/arch/powerpc/kernel/head_85xx.S
>> +++ b/arch/powerpc/kernel/head_85xx.S
>> @@ -307,8 +307,9 @@ set_ivor:
>>   #ifdef CONFIG_PTE_64BIT
>>   #ifdef CONFIG_HUGETLB_PAGE
>>   #define FIND_PTE	\
>> -	rlwinm	r12, r10, 13, 19, 29;	/* Compute pgdir/pmd offset */	\
>> -	lwzx	r11, r12, r11;		/* Get pgd/pmd entry */		\
>> +	rlwinm	r12, r10, 14, 18, 28;	/* Compute pgdir/pmd offset */	\
>> +	add	r12, r11, r12;
> 
> You add the offset to pgdir?

Yes because later r12 points to the PTE so when it is a leaf PGD entry 
we need r12 to point to that entry.

> 
>> +	lwz	r11, 4(r12);		/* Get pgd/pmd entry */		\
> 
> What is i offset 4?

It is big endian, the entry is now 64 bits but the real content of the 
entry is still 32 bits so it is in the lower word.

> 
>