[PATCH 1/2] powerpc/mm: fix up pgtable dump flags

[PATCH 1/2] powerpc/mm: fix up pgtable dump flags

[Oliver O'Halloran]

On Book3s we have two PTE flags used to mark cache-inhibited mappings:
_PAGE_TOLERANT and _PAGE_NON_IDEMPOTENT. Currently the kernel page
table dumper only looks at the generic _PAGE_NO_CACHE which is
defined to be _PAGE_TOLERANT. This patch modifies the dumper so
both flags are shown in the dump.

Cc: Rashmica Gupta <rashmica.g@gmail.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/mm/dump_linuxpagetables.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/powerpc/mm/dump_linuxpagetables.c b/arch/powerpc/mm/dump_linuxpagetables.c
index 49abaf4dc8e3..e7cbfd5a0940 100644
--- a/arch/powerpc/mm/dump_linuxpagetables.c
+++ b/arch/powerpc/mm/dump_linuxpagetables.c
@@ -154,11 +154,24 @@ static const struct flag_info flag_array[] = {
 		.clear	= "             ",
 	}, {
 #endif
+#ifndef CONFIG_PPC_BOOK3S_64
 		.mask	= _PAGE_NO_CACHE,
 		.val	= _PAGE_NO_CACHE,
 		.set	= "no cache",
 		.clear	= "        ",
 	}, {
+#else
+		.mask	= _PAGE_NON_IDEMPOTENT,
+		.val	= _PAGE_NON_IDEMPOTENT,
+		.set	= "non-idempotent",
+		.clear	= "              ",
+	}, {
+		.mask	= _PAGE_TOLERANT,
+		.val	= _PAGE_TOLERANT,
+		.set	= "tolerant",
+		.clear	= "        ",
+	}, {
+#endif
 #ifdef CONFIG_PPC_BOOK3S_64
 		.mask	= H_PAGE_BUSY,
 		.val	= H_PAGE_BUSY,
-- 
2.9.3

[PATCH 2/2] powerpc/mm: add phys addr to linux page table dump

[Oliver O'Halloran]

The current page table dumper scans the linux page tables and coalesces
mappings with adjacent virtual addresses and similar PTE flags. This
behaviour is somewhat broken when you consider the IOREMAP space where
entirely unrelated mappings will appear to be contiguous.  This patch
modifies the range coalescing so that only ranges that are both physically
and virtually contiguous are combined. This patch also adds to the dump
output the physical address at the start of each range.

Cc: Rashmica Gupta <rashmica.g@gmail.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 arch/powerpc/mm/dump_linuxpagetables.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/mm/dump_linuxpagetables.c b/arch/powerpc/mm/dump_linuxpagetables.c
index e7cbfd5a0940..85e6a45bd7ee 100644
--- a/arch/powerpc/mm/dump_linuxpagetables.c
+++ b/arch/powerpc/mm/dump_linuxpagetables.c
@@ -56,6 +56,8 @@ struct pg_state {
 	struct seq_file *seq;
 	const struct addr_marker *marker;
 	unsigned long start_address;
+	unsigned long start_pa;
+	unsigned long last_pa;
 	unsigned int level;
 	u64 current_flags;
 };
@@ -265,7 +267,9 @@ static void dump_addr(struct pg_state *st, unsigned long addr)
 	const char *unit = units;
 	unsigned long delta;
 
-	seq_printf(st->seq, "0x%016lx-0x%016lx   ", st->start_address, addr-1);
+	seq_printf(st->seq, "0x%016lx-0x%016lx ", st->start_address, addr-1);
+	seq_printf(st->seq, "%016lx ", st->start_pa);
+
 	delta = (addr - st->start_address) >> 10;
 	/* Work out what appropriate unit to use */
 	while (!(delta & 1023) && unit[1]) {
@@ -280,11 +284,15 @@ static void note_page(struct pg_state *st, unsigned long addr,
 	       unsigned int level, u64 val)
 {
 	u64 flag = val & pg_level[level].mask;
+	u64 pa = val & PTE_RPN_MASK;
+
 	/* At first no level is set */
 	if (!st->level) {
 		st->level = level;
 		st->current_flags = flag;
 		st->start_address = addr;
+		st->start_pa = pa;
+		st->last_pa = pa;
 		seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
 	/*
 	 * Dump the section of virtual memory when:
@@ -292,9 +300,11 @@ static void note_page(struct pg_state *st, unsigned long addr,
 	 *   - we change levels in the tree.
 	 *   - the address is in a different section of memory and is thus
 	 *   used for a different purpose, regardless of the flags.
+	 *   - the pa of this page is not adjacent to the last inspected page
 	 */
 	} else if (flag != st->current_flags || level != st->level ||
-		   addr >= st->marker[1].start_address) {
+		   addr >= st->marker[1].start_address ||
+		   pa != st->last_pa + PAGE_SIZE) {
 
 		/* Check the PTE flags */
 		if (st->current_flags) {
@@ -318,8 +328,12 @@ static void note_page(struct pg_state *st, unsigned long addr,
 			seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
 		}
 		st->start_address = addr;
+		st->start_pa = pa;
+		st->last_pa = pa;
 		st->current_flags = flag;
 		st->level = level;
+	} else {
+		st->last_pa = pa;
 	}
 }
 
-- 
2.9.3

Re: [PATCH 2/2] powerpc/mm: add phys addr to linux page table dump

[Rashmica Gupta]

On 31/03/17 12:37, Oliver O'Halloran wrote:
> The current page table dumper scans the linux page tables and coalesces
> mappings with adjacent virtual addresses and similar PTE flags. This
> behaviour is somewhat broken when you consider the IOREMAP space where
> entirely unrelated mappings will appear to be contiguous.  This patch
> modifies the range coalescing so that only ranges that are both physically
> and virtually contiguous are combined. This patch also adds to the dump
> output the physical address at the start of each range.
>
> Cc: Rashmica Gupta <rashmica.g@gmail.com>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
>   arch/powerpc/mm/dump_linuxpagetables.c | 18 ++++++++++++++++--
>   1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/mm/dump_linuxpagetables.c b/arch/powerpc/mm/dump_linuxpagetables.c
> index e7cbfd5a0940..85e6a45bd7ee 100644
> --- a/arch/powerpc/mm/dump_linuxpagetables.c
> +++ b/arch/powerpc/mm/dump_linuxpagetables.c
> @@ -56,6 +56,8 @@ struct pg_state {
>   	struct seq_file *seq;
>   	const struct addr_marker *marker;
>   	unsigned long start_address;
> +	unsigned long start_pa;
> +	unsigned long last_pa;
>   	unsigned int level;
>   	u64 current_flags;
>   };
> @@ -265,7 +267,9 @@ static void dump_addr(struct pg_state *st, unsigned long addr)
>   	const char *unit = units;
>   	unsigned long delta;
>   
> -	seq_printf(st->seq, "0x%016lx-0x%016lx   ", st->start_address, addr-1);
> +	seq_printf(st->seq, "0x%016lx-0x%016lx ", st->start_address, addr-1);
> +	seq_printf(st->seq, "%016lx ", st->start_pa);
> +
>   	delta = (addr - st->start_address) >> 10;
>   	/* Work out what appropriate unit to use */
>   	while (!(delta & 1023) && unit[1]) {
> @@ -280,11 +284,15 @@ static void note_page(struct pg_state *st, unsigned long addr,
>   	       unsigned int level, u64 val)
>   {
>   	u64 flag = val & pg_level[level].mask;
> +	u64 pa = val & PTE_RPN_MASK;
> +
>   	/* At first no level is set */
>   	if (!st->level) {
>   		st->level = level;
>   		st->current_flags = flag;
>   		st->start_address = addr;
> +		st->start_pa = pa;
> +		st->last_pa = pa;
>   		seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
>   	/*
>   	 * Dump the section of virtual memory when:
> @@ -292,9 +300,11 @@ static void note_page(struct pg_state *st, unsigned long addr,
>   	 *   - we change levels in the tree.
>   	 *   - the address is in a different section of memory and is thus
>   	 *   used for a different purpose, regardless of the flags.
> +	 *   - the pa of this page is not adjacent to the last inspected page
>   	 */
>   	} else if (flag != st->current_flags || level != st->level ||
> -		   addr >= st->marker[1].start_address) {
> +		   addr >= st->marker[1].start_address ||
> +		   pa != st->last_pa + PAGE_SIZE) {
>   
>   		/* Check the PTE flags */
>   		if (st->current_flags) {
> @@ -318,8 +328,12 @@ static void note_page(struct pg_state *st, unsigned long addr,
>   			seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
>   		}
>   		st->start_address = addr;
> +		st->start_pa = pa;
> +		st->last_pa = pa;
>   		st->current_flags = flag;
>   		st->level = level;
> +	} else {
> +		st->last_pa = pa;
>   	}
>   }
>   
Makes sense to me!
Reviewed-by: Rashmica Gupta <rashmica.g@gmail.com>

Re: [PATCH 1/2] powerpc/mm: fix up pgtable dump flags

[Rashmica Gupta]

On 31/03/17 12:37, Oliver O'Halloran wrote:
> On Book3s we have two PTE flags used to mark cache-inhibited mappings:
> _PAGE_TOLERANT and _PAGE_NON_IDEMPOTENT. Currently the kernel page
> table dumper only looks at the generic _PAGE_NO_CACHE which is
> defined to be _PAGE_TOLERANT. This patch modifies the dumper so
> both flags are shown in the dump.
>
> Cc: Rashmica Gupta <rashmica.g@gmail.com>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Should we also add in _PAGE_SAO  that is in Book3s?


> ---
>   arch/powerpc/mm/dump_linuxpagetables.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
>
> diff --git a/arch/powerpc/mm/dump_linuxpagetables.c b/arch/powerpc/mm/dump_linuxpagetables.c
> index 49abaf4dc8e3..e7cbfd5a0940 100644
> --- a/arch/powerpc/mm/dump_linuxpagetables.c
> +++ b/arch/powerpc/mm/dump_linuxpagetables.c
> @@ -154,11 +154,24 @@ static const struct flag_info flag_array[] = {
>   		.clear	= "             ",
>   	}, {
>   #endif
> +#ifndef CONFIG_PPC_BOOK3S_64
>   		.mask	= _PAGE_NO_CACHE,
>   		.val	= _PAGE_NO_CACHE,
>   		.set	= "no cache",
>   		.clear	= "        ",
>   	}, {
> +#else
> +		.mask	= _PAGE_NON_IDEMPOTENT,
> +		.val	= _PAGE_NON_IDEMPOTENT,
> +		.set	= "non-idempotent",
> +		.clear	= "              ",
> +	}, {
> +		.mask	= _PAGE_TOLERANT,
> +		.val	= _PAGE_TOLERANT,
> +		.set	= "tolerant",
> +		.clear	= "        ",
> +	}, {
> +#endif
>   #ifdef CONFIG_PPC_BOOK3S_64
>   		.mask	= H_PAGE_BUSY,
>   		.val	= H_PAGE_BUSY,

Re: [PATCH 1/2] powerpc/mm: fix up pgtable dump flags

[Michael Ellerman]

Rashmica Gupta <rashmica.g@gmail.com> writes:

> On 31/03/17 12:37, Oliver O'Halloran wrote:
>> On Book3s we have two PTE flags used to mark cache-inhibited mappings:
>> _PAGE_TOLERANT and _PAGE_NON_IDEMPOTENT. Currently the kernel page
>> table dumper only looks at the generic _PAGE_NO_CACHE which is
>> defined to be _PAGE_TOLERANT. This patch modifies the dumper so
>> both flags are shown in the dump.
>>
>> Cc: Rashmica Gupta <rashmica.g@gmail.com>
>> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>

> Should we also add in _PAGE_SAO  that is in Book3s?

I don't think we ever expect to see it in the kernel page tables. But if
we did that would be "interesting".

I've forgotten what the code does with unknown bits, does it already
print them in some way?

If not we should either add that or add _PAGE_SAO and everything else
that could possibly ever be there.

cheers

Re: [PATCH 1/2] powerpc/mm: fix up pgtable dump flags

[Oliver O'Halloran]

On Wed, Apr 12, 2017 at 4:52 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Rashmica Gupta <rashmica.g@gmail.com> writes:
>
>> On 31/03/17 12:37, Oliver O'Halloran wrote:
>>> On Book3s we have two PTE flags used to mark cache-inhibited mappings:
>>> _PAGE_TOLERANT and _PAGE_NON_IDEMPOTENT. Currently the kernel page
>>> table dumper only looks at the generic _PAGE_NO_CACHE which is
>>> defined to be _PAGE_TOLERANT. This patch modifies the dumper so
>>> both flags are shown in the dump.
>>>
>>> Cc: Rashmica Gupta <rashmica.g@gmail.com>
>>> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
>
>> Should we also add in _PAGE_SAO  that is in Book3s?
>
> I don't think we ever expect to see it in the kernel page tables. But if
> we did that would be "interesting".
>
> I've forgotten what the code does with unknown bits, does it already
> print them in some way?

Currently it just traverses the list of known bits and prints out a
message for each. Printing any unknown bits is probably a good idea.
I'll send another patch to add that though and leave this one as-is.

> If not we should either add that or add _PAGE_SAO and everything else
> that could possibly ever be there.

ok

Re: [PATCH 1/2] powerpc/mm: fix up pgtable dump flags

[Michael Ellerman]

Oliver O'Halloran <oohall@gmail.com> writes:

> On Wed, Apr 12, 2017 at 4:52 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Rashmica Gupta <rashmica.g@gmail.com> writes:
>>
>>> On 31/03/17 12:37, Oliver O'Halloran wrote:
>>>> On Book3s we have two PTE flags used to mark cache-inhibited mappings:
>>>> _PAGE_TOLERANT and _PAGE_NON_IDEMPOTENT. Currently the kernel page
>>>> table dumper only looks at the generic _PAGE_NO_CACHE which is
>>>> defined to be _PAGE_TOLERANT. This patch modifies the dumper so
>>>> both flags are shown in the dump.
>>>>
>>>> Cc: Rashmica Gupta <rashmica.g@gmail.com>
>>>> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
>>
>>> Should we also add in _PAGE_SAO  that is in Book3s?
>>
>> I don't think we ever expect to see it in the kernel page tables. But if
>> we did that would be "interesting".
>>
>> I've forgotten what the code does with unknown bits, does it already
>> print them in some way?
>
> Currently it just traverses the list of known bits and prints out a
> message for each. Printing any unknown bits is probably a good idea.
> I'll send another patch to add that though and leave this one as-is.

Thanks.

cheers

Re: [1/2] powerpc/mm: fix up pgtable dump flags

[Michael Ellerman]

On Fri, 2017-03-31 at 01:37:48 UTC, Oliver O'Halloran wrote:
> On Book3s we have two PTE flags used to mark cache-inhibited mappings:
> _PAGE_TOLERANT and _PAGE_NON_IDEMPOTENT. Currently the kernel page
> table dumper only looks at the generic _PAGE_NO_CACHE which is
> defined to be _PAGE_TOLERANT. This patch modifies the dumper so
> both flags are shown in the dump.
> 
> Cc: Rashmica Gupta <rashmica.g@gmail.com>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>

Series applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/70538eaa70c36340b4bb27206cad31

cheers