linux-kernel - Re: [PATCH v5 02/18] mm: Define __pte_leaf

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: <0b52260d-28b2-4b33-b73e-88c5e5bfce66@cs-soprasteria.com>
Date: Thu, 13 Jun 2024 16:43:57 +0000
From: LEROY Christophe <christophe.leroy2@...soprasteria.com>
To: Oscar Salvador <osalvador@...e.de>
CC: Peter Xu <peterx@...hat.com>, Andrew Morton <akpm@...ux-foundation.org>,
	Jason Gunthorpe <jgg@...dia.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: [PATCH v5 02/18] mm: Define __pte_leaf_size() to also take a PMD
 entry



Le 13/06/2024 à 09:19, Oscar Salvador a écrit :
> On Tue, Jun 11, 2024 at 07:00:14PM +0000, LEROY Christophe wrote:
>> We have space available in PMD if we need more flags, but in PTE I can't
>> see anything possible without additional churn that would require
>> additional instructions in TLB miss handlers, which is what I want to
>> avoid most.
>>
>> Bits mapped to HW PTE:
>>
>> #define _PAGE_PRESENT	0x0001	/* V: Page is valid */
>> #define _PAGE_NO_CACHE	0x0002	/* CI: cache inhibit */
>> #define _PAGE_SH	0x0004	/* SH: No ASID (context) compare */
>> #define _PAGE_SPS	0x0008	/* SPS: Small Page Size (1 if 16k, 512k or 8M)*/
>> #define _PAGE_DIRTY	0x0100	/* C: page changed */
>> #define _PAGE_NA	0x0200	/* Supervisor NA, User no access */
>> #define _PAGE_RO	0x0600	/* Supervisor RO, User no access */
>>
>> SW bits masked out in TLB miss handler:
>>
>> #define _PAGE_GUARDED	0x0010	/* Copied to L1 G entry in DTLB */
>> #define _PAGE_ACCESSED	0x0020	/* Copied to L1 APG 1 entry in I/DTLB */
>> #define _PAGE_EXEC	0x0040	/* Copied to PP (bit 21) in ITLB */
>> #define _PAGE_SPECIAL	0x0080	/* SW entry */
>> #define _PAGE_HUGE	0x0800	/* Copied to L1 PS bit 29 */
>>
>> All bits are used. The only thing would could do but that would have a
>> performance cost is to retrieve _PAGE_SH from the PMD and use that bit
>> for something else.
> 
> I guess that this would be the last resort if we run out of options.
> But at least it is good to know that there is a plan B (or Z if you will
> :-))
> 
>> But I was maybe thinking another way. Lets take the exemple of
>> pmd_write() helper:
>>
>> #define pmd_write(pmd)		pte_write(pmd_pte(pmd))
>>
>> At the time being we have
>>
>> static inline pte_t pmd_pte(pmd_t pmd)
>> {
>> 	return __pte(pmd_val(pmd));
>> }
>>
>> But what about something like
>>
>> static inline pte_t pmd_pte(pmd_t pmd)
>> {
>> 	return *(pte_t *)pmd_page_vaddr(pmd);
>> }
> 
> I think this could work, yes.
> 
> So, we should define all pmd_*(pmd) operations for 8xx the way they are defined
> in include/asm/book3s/64/pgtable.h.
> 
> Other page size would not interfere because they already can perform
> operations on pte level.
> 
> Ok, I think we might have a shot here.
> 
> I would help testing, but I do not have 8xx hardware, and Qemu does not support
> 8xx emulation, but I think that if we are careful enough, this can work.
> 
> Actually, as a smoketest would be enough to have a task with a 8MB huge
> mapped, and then do:
> 
>   static const struct mm_walk_ops test_walk_ops = {
>           .pmd_entry = test_8mbp_hugepage,
>           .pte_entry = test_16k_and_512k_hugepage,
>           .hugetlb_entry = check_hugetlb_entry,
>           .walk_lock = PGWALK_RDLOCK,
>   };
> 
>   static int test(void)
>   {
>   
>            pr_info("%s: %s [0 - %lx]\n", __func__, current->comm, TASK_SIZE);
>            mmap_read_lock(current->mm);
>            ret = walk_page_range(current->mm, 0, TASK_SIZE, &test_walk_ops, NULL);
>            mmap_read_unlock(current->mm);
>            
>            pr_info("%s: %s ret: %d\n", __func__, current->comm, ret);
>            
>            return 0;
>   }
> 
> This is an extract of a debugging mechanism I have to check that I am
> not going off rails when unifying hugetlb and normal walkers.
> 
> test_8mbp_hugepage() could so some checks with pmd_ operations, print
> the results, and then compare them with those that check_hugetlb_entry()
> would give us.
> If everything is alright, both results should be the same.
> 
> I can write the tests up, so we run some sort of smoketests.
> 
> So yes, I do think that this is a good initiative.
> 


I can test whatever you want on my 8xx boards.

I have two types of board:
- One with MPC866 microcontroller and 32Mbytes memory
- One with MPC885 microcontroller and 128Mbytes memory

Christophe