lkml.org 
[lkml]   [2023]   [Sep]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2 1/3] riscv: Improve PTDUMP to show RSW with non-zero value
From
On 14/09/2023 03:40, Yu Chien Peter Lin wrote:
> RSW field can be used to encode 2 bits of software defined
> information, currently PTDUMP only prints RSW when its value
> is 1 or 3.
>
> To fix this issue and enhance the debug experience with PTDUMP,
> we use _PAGE_SOFT as the RSW mask and redefine _PAGE_SPECIAL to
> (1 << 8), allow it to print the RSW with any non-zero value,
> otherwise, it will print an empty string for each row.
>
> This patch also removes the val from the struct prot_bits as
> it is no longer needed.
>
> Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
> ---
> arch/riscv/include/asm/pgtable-bits.h | 4 +--
> arch/riscv/mm/ptdump.c | 36 +++++++++++----------------
> 2 files changed, 17 insertions(+), 23 deletions(-)
>
> diff --git a/arch/riscv/include/asm/pgtable-bits.h b/arch/riscv/include/asm/pgtable-bits.h
> index f896708e8331..99e60fd3eb72 100644
> --- a/arch/riscv/include/asm/pgtable-bits.h
> +++ b/arch/riscv/include/asm/pgtable-bits.h
> @@ -16,9 +16,9 @@
> #define _PAGE_GLOBAL (1 << 5) /* Global */
> #define _PAGE_ACCESSED (1 << 6) /* Set by hardware on any access */
> #define _PAGE_DIRTY (1 << 7) /* Set by hardware on any write */
> -#define _PAGE_SOFT (1 << 8) /* Reserved for software */
> +#define _PAGE_SOFT (3 << 8) /* Reserved for software */
>
> -#define _PAGE_SPECIAL _PAGE_SOFT


That's nit, but maybe you could have introduced a _PAGE_SOFT_1 and
_PAGE_SOFT_2


> +#define _PAGE_SPECIAL (1 << 8)


instead of hardcoding (1<<8) here, but that can be done when we'll use
the second bit :)


> #define _PAGE_TABLE _PAGE_PRESENT
>
> /*
> diff --git a/arch/riscv/mm/ptdump.c b/arch/riscv/mm/ptdump.c
> index 20a9f991a6d7..85686652f342 100644
> --- a/arch/riscv/mm/ptdump.c
> +++ b/arch/riscv/mm/ptdump.c
> @@ -129,7 +129,6 @@ static struct ptd_mm_info efi_ptd_info = {
> /* Page Table Entry */
> struct prot_bits {
> u64 mask;
> - u64 val;
> const char *set;
> const char *clear;
> };
> @@ -137,47 +136,38 @@ struct prot_bits {
> static const struct prot_bits pte_bits[] = {
> {
> .mask = _PAGE_SOFT,
> - .val = _PAGE_SOFT,
> - .set = "RSW",
> - .clear = " ",
> + .set = "RSW(%d)",
> + .clear = " ",
> }, {
> .mask = _PAGE_DIRTY,
> - .val = _PAGE_DIRTY,
> .set = "D",
> .clear = ".",
> }, {
> .mask = _PAGE_ACCESSED,
> - .val = _PAGE_ACCESSED,
> .set = "A",
> .clear = ".",
> }, {
> .mask = _PAGE_GLOBAL,
> - .val = _PAGE_GLOBAL,
> .set = "G",
> .clear = ".",
> }, {
> .mask = _PAGE_USER,
> - .val = _PAGE_USER,
> .set = "U",
> .clear = ".",
> }, {
> .mask = _PAGE_EXEC,
> - .val = _PAGE_EXEC,
> .set = "X",
> .clear = ".",
> }, {
> .mask = _PAGE_WRITE,
> - .val = _PAGE_WRITE,
> .set = "W",
> .clear = ".",
> }, {
> .mask = _PAGE_READ,
> - .val = _PAGE_READ,
> .set = "R",
> .clear = ".",
> }, {
> .mask = _PAGE_PRESENT,
> - .val = _PAGE_PRESENT,
> .set = "V",
> .clear = ".",
> }
> @@ -208,15 +198,19 @@ static void dump_prot(struct pg_state *st)
> unsigned int i;
>
> for (i = 0; i < ARRAY_SIZE(pte_bits); i++) {
> - const char *s;
> -
> - if ((st->current_prot & pte_bits[i].mask) == pte_bits[i].val)
> - s = pte_bits[i].set;
> - else
> - s = pte_bits[i].clear;
> -
> - if (s)
> - pt_dump_seq_printf(st->seq, " %s", s);
> + char s[7];
> + unsigned long val;
> +
> + val = st->current_prot & pte_bits[i].mask;
> + if (val) {
> + if (pte_bits[i].mask == _PAGE_SOFT)
> + sprintf(s, pte_bits[i].set, val >> 8);
> + else
> + sprintf(s, "%s", pte_bits[i].set);
> + } else
> + sprintf(s, "%s", pte_bits[i].clear);
> +
> + pt_dump_seq_printf(st->seq, " %s", s);
> }
> }
>


I don't see any issue in your patch, but just the output is a bit
"weird" now as the there is a large "hole" between the PTE type and the
PTE protection bits:

Before:

0xffffffd800000000-0xffffffd800200000 0x0000000080000000         2M
PMD     D A G . . W R V

After:

0xffffaf8000000000-0xffffaf8000200000 0x0000000080000000         2M PMD
.               D A G . . W R V

Maybe you could add the PBMT/N bits after the protections bits to void
this hole?

Anyway, as a heavy user of this kernel page table dump, that's really
appreciated, thanks :)

Alex

\
 
 \ /
  Last update: 2023-09-15 13:07    [W:0.036 / U:1.368 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site