1. Patchew
  2. linux
  3. View series

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

Yunhui Cui posted 1 patch 1 year, 5 months ago
There is a newer version of this series
arch/riscv/mm/fault.c | 47 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
[PATCH] riscv/mm/fault: add show_pte() before die()
Posted by Yunhui Cui 1 year, 5 months ago 
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@bytedance.com>
---
 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);
+	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
Re: [PATCH] riscv/mm/fault: add show_pte() before die()
Posted by Alexandre Ghiti 1 year, 5 months ago 
Hi Yunhui,

On Thu, Jul 18, 2024 at 4:20 AM Yunhui Cui <cuiyunhui@bytedance.com> 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@bytedance.com>
> ---
>  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@rivosinc.com>

Thanks,

Alex
Re: [External] Re: [PATCH] riscv/mm/fault: add show_pte() before die()
Posted by yunhui cui 1 year, 4 months ago 
Hi Alex，

On Sat, Jul 20, 2024 at 12:26 AM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>
> Hi Yunhui,
>
> On Thu, Jul 18, 2024 at 4:20 AM Yunhui Cui <cuiyunhui@bytedance.com> 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@bytedance.com>
> > ---
> >  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().
>
Okay, I'll update it in v2.

> 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@rivosinc.com>
>
> Thanks,
>
> Alex

Thanks,
Yunhui
Re: [PATCH] riscv/mm/fault: add show_pte() before die()
Posted by Andrew Jones 1 year, 5 months ago 
On Thu, Jul 18, 2024 at 10:09:35AM GMT, Yunhui Cui 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@bytedance.com>
> ---
>  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;

arm64 show_pte starts with a summary line where the pgtable type (swapper
vs. user), number of VA bits, and physical address of the pgd are
displayed. Should we add that too?

Thanks,
drew

> +	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);
> +	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
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [External] Re: [PATCH] riscv/mm/fault: add show_pte() before die()
Posted by yunhui cui 1 year, 4 months ago 
Hi drew,

On Fri, Jul 19, 2024 at 10:33 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Thu, Jul 18, 2024 at 10:09:35AM GMT, Yunhui Cui 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@bytedance.com>
> > ---
> >  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;
>
> arm64 show_pte starts with a summary line where the pgtable type (swapper
> vs. user), number of VA bits, and physical address of the pgd are
> displayed. Should we add that too?
Oaky， I will add it in v2.

>
> Thanks,
> drew
>
> > +     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);
> > +     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
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

Thanks，
Yunhui

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.