linux-kernel - Re: [RFC PATCH v4 14/16] powerpc/64s: Use contiguous PMD/PUD instead of HUGEPD

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: <315d1dc7-9eda-45cd-863d-803bfae4c29b@csgroup.eu>
Date: Wed, 29 May 2024 10:07:55 +0000
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Oscar Salvador <osalvador@...e.com>
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 v4 14/16] powerpc/64s: Use contiguous PMD/PUD instead
 of HUGEPD



Le 29/05/2024 à 11:23, Oscar Salvador a écrit :
> On Mon, May 27, 2024 at 03:30:12PM +0200, Christophe Leroy wrote:
>> On book3s/64, the only user of hugepd is hash in 4k mode.
>>
>> All other setups (hash-64, radix-4, radix-64) use leaf PMD/PUD.
>>
>> Rework hash-4k to use contiguous PMD and PUD instead.
>>
>> In that setup there are only two huge page sizes: 16M and 16G.
>>
>> 16M sits at PMD level and 16G at PUD level.
> 
> 
> On 4k mode, PMD_SIZE is 2MB and PUD_SIZE is 256MB, right?

Correct, as documented in arch/powerpc/include/asm/book3s/64/hash-4k.h

> 
>> +static inline unsigned long hash__pte_update(struct mm_struct *mm,
>> +					 unsigned long addr,
>> +					 pte_t *ptep, unsigned long clr,
>> +					 unsigned long set,
>> +					 int huge)
>> +{
>> +	unsigned long old;
>> +
>> +	old = hash__pte_update_one(ptep, clr, set);
>> +
>> +	if (IS_ENABLED(CONFIG_PPC_4K_PAGES) && huge) {
>> +		unsigned int psize = get_slice_psize(mm, addr);
>> +		int nb, i;
>> +
>> +		if (psize == MMU_PAGE_16M)
>> +			nb = SZ_16M / PMD_SIZE;
>> +		else if (psize == MMU_PAGE_16G)
>> +			nb = SZ_16G / PUD_SIZE;
>> +		else
>> +			nb = 1;
> 
> On 4K, hugepages are either 16M or 16G. How can we end up in a situation
> whwere the is pte is huge, but is is neither MMU_PAGE_16G nor MMU_PAGE_16M?

We can't but I didn't want to leave nb undefined or with a value that 
might lead to writing in the weed. Value 1 seems a safe default.

> 
>> diff --git a/arch/powerpc/mm/book3s64/hugetlbpage.c b/arch/powerpc/mm/book3s64/hugetlbpage.c
>> index 5a2e512e96db..83c3361b358b 100644
>> --- a/arch/powerpc/mm/book3s64/hugetlbpage.c
>> +++ b/arch/powerpc/mm/book3s64/hugetlbpage.c
>> @@ -53,6 +53,16 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,
>>   		/* If PTE permissions don't match, take page fault */
>>   		if (unlikely(!check_pte_access(access, old_pte)))
>>   			return 1;
>> +		/*
>> +		 * If hash-4k, hugepages use seeral contiguous PxD entries
> 'several'
>> +		 * so bail out and let mm make the page young or dirty
>> +		 */
>> +		if (IS_ENABLED(CONFIG_PPC_4K_PAGES)) {
>> +			if (!(old_pte & _PAGE_ACCESSED))
>> +				return 1;
>> +			if ((access & _PAGE_WRITE) && !(old_pte & _PAGE_DIRTY))
>> +				return 1;
> 
> I have 0 clue about this code. What would happen if we do not bail out?
> 

In that case the pte_xchg() in the while () will only set ACCESS or 
DIRTY bit on the first PxD entry, not on all cont-PxD entries.

Christophe