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.
Thanks a lot Christoph
--
Oscar Salvador
SUSE Labs
