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.
