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

Hi Geert,

> Hi Yeoreum,
> 
> On Tue, 14 Jul 2026 at 18:51, Yeoreum Yun <yeoreum.yun@arm.com> wrote:
> > On Tue, Jul 14, 2026 at 05:08:24PM +0100, Yeoreum Yun wrote:
> > > 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
> > >
> >
> > Furthermore, the effective_prot() need to require to identify whether
> > it's the first pgtable to prevent the inheriting from dummy value.
> > So I think it seems to make a ptdump_pt_level_first() like:
> >
> > diff --git a/mm/ptdump.c b/mm/ptdump.c
> > index 973020000096c..2ec5700e4be5e 100644
> > --- a/mm/ptdump.c
> > +++ b/mm/ptdump.c
> > @@ -190,6 +190,23 @@ void ptdump_walk_pgd(struct ptdump_state *st, struct mm_struct *mm, pgd_t *pgd)
> >         st->note_page_flush(st);
> >  }
> >
> > +bool pdtump_pt_level_first(struct mm_struct *mm, int level)
> > +{
> > +       if (!mm || level > CONFIG_PGTABLE_LEVELS)
> > +               return false;
> > +
> > +       if (mm_pmd_folded(mm) && level == 3)
> > +               return true;
> > +       if (mm_pud_folded(mm) && level == 2)
> > +               return true;
> > +       if (mm_p4d_folded(mm) && level == 1)
> > +               return true;
> 
> Is this order (... && level == ...) the optimal order?
> On arm64, mm_pud_folded() and mm_p4d_folded() perform several checks.
> 
> > +       if (level == 0)
> > +               return true;
> > +
> > +       return false;
> > +}

It is. since the purpose of ptdump_pt_level_first is designed to be used
in the *callback* of pXd_entry(). IOW, if we check level 0 first,
it always returns wrong return in folded case.


-- 
Sincerely,
Yeoreum Yun