Re: [PATCH v14 21/22] arm64: mm: Convert mm/dump.c to use walk_page_range()

Catalin Marinas
2019-10-30T16:45:43Z

On Mon, Oct 28, 2019 at 01:59:09PM +0000, Steven Price wrote:
> diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
> index 93f9f77582ae..9d9b740a86d2 100644
> --- a/arch/arm64/mm/dump.c
> +++ b/arch/arm64/mm/dump.c
> @@ -15,6 +15,7 @@
> #include <linux/io.h>
> #include <linux/init.h>
> #include <linux/mm.h>
> +#include <linux/ptdump.h>
> #include <linux/sched.h>
> #include <linux/seq_file.h>
>
> @@ -75,10 +76,11 @@ static struct addr_marker address_markers[] = {
> * dumps out a description of the range.
> */
> struct pg_state {
> + struct ptdump_state ptdump;
> struct seq_file *seq;
> const struct addr_marker *marker;
> unsigned long start_address;
> - unsigned level;
> + int level;
> u64 current_prot;
> bool check_wx;
> unsigned long wx_pages;
> @@ -178,6 +180,10 @@ static struct pg_level pg_level[] = {
> .name = "PGD",
> .bits = pte_bits,
> .num = ARRAY_SIZE(pte_bits),
> + }, { /* p4d */
> + .name = "P4D",
> + .bits = pte_bits,
> + .num = ARRAY_SIZE(pte_bits),
> }, { /* pud */
> .name = (CONFIG_PGTABLE_LEVELS > 3) ? "PUD" : "PGD",
> .bits = pte_bits,

We could use "PGD" for the p4d entry since we don't have five levels.
This patches the "PGD" name used for pud/pmd when these levels are
folded.

> @@ -240,11 +246,15 @@ static void note_prot_wx(struct pg_state *st, unsigned long addr)
> st->wx_pages += (addr - st->start_address) / PAGE_SIZE;
> }
>
> -static void note_page(struct pg_state *st, unsigned long addr, unsigned level,
> - u64 val)
> +static void note_page(struct ptdump_state *pt_st, unsigned long addr, int level,
> + unsigned long val)
> {
> + struct pg_state *st = container_of(pt_st, struct pg_state, ptdump);
> static const char units[] = "KMGTPE";
> - u64 prot = val & pg_level[level].mask;
> + u64 prot = 0;
> +
> + if (level >= 0)
> + prot = val & pg_level[level].mask;

I think this test is not needed as we never have level < 0. The only
call with a level 0 is from ptdump_hole() where the level passed is
depth+1 while depth is -1 or higher.

Anyway, we can keep this test _if_ we shift the levels down. I find it
quite confusing that ptdump_hole() takes a 'depth' argument where 0 is
PGD and 4 is PTE while for note_page() 1 is PGD and 5 PTE.

--
Catalin