Re: [PATCH] riscv/mm/fault: add show_pte() before die()

[Alexandre Ghiti]

Hi Yunhui,

On Thu, Jul 18, 2024 at 4:20 AM Yunhui Cui <cuiyunhui@xxxxxxxxxxxxx> wrote:
>
> When the kernel displays "Unable to handle kernel paging request at
> virtual address", we would like to confirm the status of the virtual
> address in the page table. So add show_pte() before die().
>
> Signed-off-by: Yunhui Cui <cuiyunhui@xxxxxxxxxxxxx>
> ---
>  arch/riscv/mm/fault.c | 47 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>
> diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
> index 90d4ba36d1d0..dfe3ce38e16b 100644
> --- a/arch/riscv/mm/fault.c
> +++ b/arch/riscv/mm/fault.c
> @@ -22,6 +22,52 @@
>
>  #include "../kernel/head.h"
>
> +static void show_pte(unsigned long addr)
> +{
> +       pgd_t *pgdp, pgd;
> +       p4d_t *p4dp, p4d;
> +       pud_t *pudp, pud;
> +       pmd_t *pmdp, pmd;
> +       pte_t *ptep, pte;
> +       struct mm_struct *mm = current->mm;
> +
> +       if (!mm)
> +               mm = &init_mm;
> +       pgdp = pgd_offset(mm, addr);
> +       pgd = READ_ONCE(*pgdp);
> +       pr_alert("[%016lx] pgd=%016lx", addr, pgd_val(pgd));
> +       if (pgd_none(pgd) || pgd_bad(pgd))
> +               goto out;
> +
> +       p4dp = p4d_offset(pgdp, addr);
> +       p4d = READ_ONCE(*p4dp);
> +       pr_cont(", p4d=%016lx", p4d_val(p4d));
> +       if (p4d_none(p4d) || p4d_bad(p4d))
> +               goto out;
> +
> +       pudp = pud_offset(p4dp, addr);
> +       pud = READ_ONCE(*pudp);
> +       pr_cont(", pud=%016lx", pud_val(pud));
> +       if (pud_none(pud) || pud_bad(pud))
> +               goto out;
> +
> +       pmdp = pmd_offset(pudp, addr);
> +       pmd = READ_ONCE(*pmdp);
> +       pr_cont(", pmd=%016lx", pmd_val(pmd));
> +       if (pmd_none(pmd) || pmd_bad(pmd))
> +               goto out;
> +
> +       ptep = pte_offset_map(pmdp, addr);
> +       if (!ptep)
> +               goto out;
> +
> +       pte = READ_ONCE(*ptep);

All the READ_ONCE() can be replaced by pXXp_get() macros defined here:
https://elixir.bootlin.com/linux/v6.10/source/include/linux/pgtable.h#L315

And we should stop as soon as we encounter a leaf entry using pXd_leaf().

Thanks,

Alex

> +       pr_cont(", pte=%016lx", pte_val(pte));
> +       pte_unmap(ptep);
> +out:
> +       pr_cont("\n");
> +}
> +
>  static void die_kernel_fault(const char *msg, unsigned long addr,
>                 struct pt_regs *regs)
>  {
> @@ -31,6 +77,7 @@ static void die_kernel_fault(const char *msg, unsigned long addr,
>                 addr);
>
>         bust_spinlocks(0);
> +       show_pte(addr);
>         die(regs, "Oops");
>         make_task_dead(SIGKILL);
>  }
> --
> 2.39.2
>

Otherwise, that's a good idea to print the page table content on fault so:

Reviewed-by: Alexandre Ghiti <alexghiti@xxxxxxxxxxxx>

Thanks,

Alex