* [PATCH] riscv/mm/fault: add show_pte() before die()
@ 2024-07-18 2:09 Yunhui Cui
2024-07-19 14:32 ` Andrew Jones
2024-07-19 16:25 ` Alexandre Ghiti
0 siblings, 2 replies; 5+ messages in thread
From: Yunhui Cui @ 2024-07-18 2:09 UTC (permalink / raw)
To: punit.agrawal, paul.walmsley, palmer, aou, akpm, surenb, peterx,
alexghiti, willy, linux-riscv, linux-kernel
Cc: Yunhui Cui
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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] riscv/mm/fault: add show_pte() before die()
2024-07-18 2:09 [PATCH] riscv/mm/fault: add show_pte() before die() Yunhui Cui
@ 2024-07-19 14:32 ` Andrew Jones
2024-07-22 3:12 ` [External] " yunhui cui
2024-07-19 16:25 ` Alexandre Ghiti
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Jones @ 2024-07-19 14:32 UTC (permalink / raw)
To: Yunhui Cui
Cc: punit.agrawal, paul.walmsley, palmer, aou, akpm, surenb, peterx,
alexghiti, willy, linux-riscv, linux-kernel
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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [External] Re: [PATCH] riscv/mm/fault: add show_pte() before die()
2024-07-19 14:32 ` Andrew Jones
@ 2024-07-22 3:12 ` yunhui cui
0 siblings, 0 replies; 5+ messages in thread
From: yunhui cui @ 2024-07-22 3:12 UTC (permalink / raw)
To: Andrew Jones
Cc: punit.agrawal, paul.walmsley, palmer, aou, akpm, surenb, peterx,
alexghiti, willy, linux-riscv, linux-kernel
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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] riscv/mm/fault: add show_pte() before die()
2024-07-18 2:09 [PATCH] riscv/mm/fault: add show_pte() before die() Yunhui Cui
2024-07-19 14:32 ` Andrew Jones
@ 2024-07-19 16:25 ` Alexandre Ghiti
2024-07-22 4:00 ` [External] " yunhui cui
1 sibling, 1 reply; 5+ messages in thread
From: Alexandre Ghiti @ 2024-07-19 16:25 UTC (permalink / raw)
To: Yunhui Cui
Cc: punit.agrawal, paul.walmsley, palmer, aou, akpm, surenb, peterx,
willy, linux-riscv, linux-kernel
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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [External] Re: [PATCH] riscv/mm/fault: add show_pte() before die()
2024-07-19 16:25 ` Alexandre Ghiti
@ 2024-07-22 4:00 ` yunhui cui
0 siblings, 0 replies; 5+ messages in thread
From: yunhui cui @ 2024-07-22 4:00 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: punit.agrawal, paul.walmsley, palmer, aou, akpm, surenb, peterx,
willy, linux-riscv, linux-kernel
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
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-07-22 4:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-18 2:09 [PATCH] riscv/mm/fault: add show_pte() before die() Yunhui Cui
2024-07-19 14:32 ` Andrew Jones
2024-07-22 3:12 ` [External] " yunhui cui
2024-07-19 16:25 ` Alexandre Ghiti
2024-07-22 4:00 ` [External] " yunhui cui
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox