[RFC PATCH 10/34] x86: mm: carve out the generic compile-time folded pgtable case in effective_prot()

[Dave Hansen]

On 7/14/26 04:40, Yeoreum Yun wrote:
> @@ -254,22 +255,8 @@ static void effective_prot(struct ptdump_state *pt_st, int level, u64 val)
>         struct pg_state *st = container_of(pt_st, struct pg_state, ptdump);
>         pgprotval_t prot = val & PTE_FLAGS_MASK;
>         pgprotval_t effective;
> -       bool first_level = false;
> 
> -       /* Ignore folded levels ... */
> -       if (((level == 0) && mm_p4d_folded(st->mm)) ||
> -           ((level == 1) && mm_pud_folded(st->mm)) ||
> -           ((level == 2) && mm_pmd_folded(st->mm)))
> -               return;
> -
> -       /* ... and make the actual first level remember the protection. */
> -       if (((level == 0)) ||
> -           ((level == 1) && mm_p4d_folded(st->mm)) ||
> -           ((level == 2) && mm_pud_folded(st->mm)) ||
> -           ((level == 3) && mm_pmd_folded(st->mm)))
> -               first_level = true;
> -
> -       if (!first_level) {
> +       if (first_level > st->first_level) {
>                 pgprotval_t higher_prot = st->prot_levels[level - 1];
> 
>                 effective = (higher_prot & prot & (_PAGE_USER | _PAGE_RW)) |
> @@ -471,6 +458,15 @@ bool ptdump_walk_pgd_level_core(struct seq_file *m,
>                 .seq            = m
>         };
> 
> +       if (mm_pmd_folded (mm))
> +               st->first_level = 3;
> +       else if (mm_pud_folded (mm))
> +               st->first_level = 2;
> +       else if (mm_p4d_folded (mm))
> +               st->first_level = 1;
> +       else
> +               st->first_level = 0;
> +
>         ptdump_walk_pgd(&st.ptdump, mm, pgd);

This is indeed an improvement and a step in the right direction! Thanks
for looking at this.

But one of my test for whether it's good x86 code is whether there's any
actually x86-specific logic in it. Isn't this basically a translation
between the integer level number and whether it is folded?

That seems like a common helper that more than one arch could use. Could
this be stuck in a helper so that all arch/x86 has to do is:

	if (mm_pt_level_folded(mm, level))
		return;

This makes a *ton* of sense in effective_prot() especially. Its entire
job is mirroring the hardware's job of inheriting permissions from
higher levels of the page tables and enforcing them on lower level leaf
entries. If a higher level is folded, there's nothing to inherit.

I also think it's worth taking a brief pause on the coding to think
about what kind of design would actually be nice here. If the design
really is that the pgd is folded, the 'struct mm_walk_ops' code would
ideally not even call ->pgd_entry(). It would just (for example) *start*
at ->pud_entry() for a 3-level hardware page table.

If there are no pgds, why bother calling ->pgd_entry()? It couldn't be
done transparently to mm_walk users of course but it could be done
incrementally where users move one at a time over to a new scheme.

BTW, I'm not saying this needs to be done in this series. But it would
be nice to have an eye on the prize.