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

[Yeoreum Yun <yeoreum.yun@arm.com>]

On Tue, Jul 14, 2026 at 07:35:19AM -0700, Dave Hansen wrote:
> 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;

Yes. at least the level semantic in ptdump (pgd is level 0,
p4d is level 1, ...) is same to all archs where use ptdump.
So It seems reasonable to add this common helper in ptdump.c

> 
> 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.

Agree. Calling a pXd_entry() about folded one seems meaningless.
But seems enough to check mm_pXd_folded() before calling pXd_entry()
in each walk_pXd_range().

[...]

-- 
Sincerely,
Yeoreum Yun