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

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

Posted by Yunhui Cui 1 year, 4 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>
Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
 arch/riscv/mm/fault.c | 53 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index 5224f3733802..666d282a8bf4 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -22,6 +22,58 @@
 
 #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;
+
+	pr_alert("Current %s pgtable: %luK pagesize, %d-bit VAs, pgdp=0x%016lx\n",
+		 current->comm, PAGE_SIZE/SZ_1K, VA_BITS,
+		 (mm == &init_mm ? __pa_symbol(mm->pgd) :
+		 (unsigned long)virt_to_phys(mm->pgd)));
+
+	pgdp = pgd_offset(mm, addr);
+	pgd = pgdp_get(pgdp);
+	pr_alert("[%016lx] pgd=%016lx", addr, pgd_val(pgd));
+	if (pgd_none(pgd) || pgd_bad(pgd) || pgd_leaf(pgd))
+		goto out;
+
+	p4dp = p4d_offset(pgdp, addr);
+	p4d = p4dp_get(p4dp);
+	pr_cont(", p4d=%016lx", p4d_val(p4d));
+	if (p4d_none(p4d) || p4d_bad(p4d) || p4d_leaf(p4d))
+		goto out;
+
+	pudp = pud_offset(p4dp, addr);
+	pud = pudp_get(pudp);
+	pr_cont(", pud=%016lx", pud_val(pud));
+	if (pud_none(pud) || pud_bad(pud) || pud_leaf(pud))
+		goto out;
+
+	pmdp = pmd_offset(pudp, addr);
+	pmd = pmdp_get(pmdp);
+	pr_cont(", pmd=%016lx", pmd_val(pmd));
+	if (pmd_none(pmd) || pmd_bad(pmd) || pmd_leaf(pmd))
+		goto out;
+
+	ptep = pte_offset_map(pmdp, addr);
+	if (!ptep)
+		goto out;
+
+	pte = ptep_get(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 +83,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 v2] riscv/mm/fault: add show_pte() before die()

Posted by Conor Dooley 1 year, 4 months ago

On Mon, Jul 22, 2024 at 12:20:37PM +0800, 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>
> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

The patchwork automation reports:
+      1 ../arch/riscv/mm/fault.c:39:4: warning: format specifies type 'unsigned long' but the argument has type 'phys_addr_t' (aka 'unsigned long long') [-Wformat]

Cheers,
Conor.

> ---
>  arch/riscv/mm/fault.c | 53 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
> index 5224f3733802..666d282a8bf4 100644
> --- a/arch/riscv/mm/fault.c
> +++ b/arch/riscv/mm/fault.c
> @@ -22,6 +22,58 @@
>  
>  #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;
> +
> +	pr_alert("Current %s pgtable: %luK pagesize, %d-bit VAs, pgdp=0x%016lx\n",
> +		 current->comm, PAGE_SIZE/SZ_1K, VA_BITS,
> +		 (mm == &init_mm ? __pa_symbol(mm->pgd) :
> +		 (unsigned long)virt_to_phys(mm->pgd)));
> +
> +	pgdp = pgd_offset(mm, addr);
> +	pgd = pgdp_get(pgdp);
> +	pr_alert("[%016lx] pgd=%016lx", addr, pgd_val(pgd));
> +	if (pgd_none(pgd) || pgd_bad(pgd) || pgd_leaf(pgd))
> +		goto out;
> +
> +	p4dp = p4d_offset(pgdp, addr);
> +	p4d = p4dp_get(p4dp);
> +	pr_cont(", p4d=%016lx", p4d_val(p4d));
> +	if (p4d_none(p4d) || p4d_bad(p4d) || p4d_leaf(p4d))
> +		goto out;
> +
> +	pudp = pud_offset(p4dp, addr);
> +	pud = pudp_get(pudp);
> +	pr_cont(", pud=%016lx", pud_val(pud));
> +	if (pud_none(pud) || pud_bad(pud) || pud_leaf(pud))
> +		goto out;
> +
> +	pmdp = pmd_offset(pudp, addr);
> +	pmd = pmdp_get(pmdp);
> +	pr_cont(", pmd=%016lx", pmd_val(pmd));
> +	if (pmd_none(pmd) || pmd_bad(pmd) || pmd_leaf(pmd))
> +		goto out;
> +
> +	ptep = pte_offset_map(pmdp, addr);
> +	if (!ptep)
> +		goto out;
> +
> +	pte = ptep_get(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 +83,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: [External] Re: [PATCH v2] riscv/mm/fault: add show_pte() before die()

Posted by yunhui cui 1 year, 4 months ago

Hi Conor,

On Tue, Jul 23, 2024 at 1:28 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Mon, Jul 22, 2024 at 12:20:37PM +0800, 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>
> > Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>
> The patchwork automation reports:
> +      1 ../arch/riscv/mm/fault.c:39:4: warning: format specifies type 'unsigned long' but the argument has type 'phys_addr_t' (aka 'unsigned long long') [-Wformat]
Okay, I'll update it in v3.

>
> Cheers,
> Conor.
>
> > ---
> >  arch/riscv/mm/fault.c | 53 +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 53 insertions(+)
> >
> > diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
> > index 5224f3733802..666d282a8bf4 100644
> > --- a/arch/riscv/mm/fault.c
> > +++ b/arch/riscv/mm/fault.c
> > @@ -22,6 +22,58 @@
> >
> >  #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;
> > +
> > +     pr_alert("Current %s pgtable: %luK pagesize, %d-bit VAs, pgdp=0x%016lx\n",
> > +              current->comm, PAGE_SIZE/SZ_1K, VA_BITS,
> > +              (mm == &init_mm ? __pa_symbol(mm->pgd) :
> > +              (unsigned long)virt_to_phys(mm->pgd)));
> > +
> > +     pgdp = pgd_offset(mm, addr);
> > +     pgd = pgdp_get(pgdp);
> > +     pr_alert("[%016lx] pgd=%016lx", addr, pgd_val(pgd));
> > +     if (pgd_none(pgd) || pgd_bad(pgd) || pgd_leaf(pgd))
> > +             goto out;
> > +
> > +     p4dp = p4d_offset(pgdp, addr);
> > +     p4d = p4dp_get(p4dp);
> > +     pr_cont(", p4d=%016lx", p4d_val(p4d));
> > +     if (p4d_none(p4d) || p4d_bad(p4d) || p4d_leaf(p4d))
> > +             goto out;
> > +
> > +     pudp = pud_offset(p4dp, addr);
> > +     pud = pudp_get(pudp);
> > +     pr_cont(", pud=%016lx", pud_val(pud));
> > +     if (pud_none(pud) || pud_bad(pud) || pud_leaf(pud))
> > +             goto out;
> > +
> > +     pmdp = pmd_offset(pudp, addr);
> > +     pmd = pmdp_get(pmdp);
> > +     pr_cont(", pmd=%016lx", pmd_val(pmd));
> > +     if (pmd_none(pmd) || pmd_bad(pmd) || pmd_leaf(pmd))
> > +             goto out;
> > +
> > +     ptep = pte_offset_map(pmdp, addr);
> > +     if (!ptep)
> > +             goto out;
> > +
> > +     pte = ptep_get(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 +83,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
> >
> >

Thanks,
Yunhui

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

Posted by Andrew Jones 1 year, 4 months ago

On Mon, Jul 22, 2024 at 12:20:37PM 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>
> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> ---
>  arch/riscv/mm/fault.c | 53 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
> index 5224f3733802..666d282a8bf4 100644
> --- a/arch/riscv/mm/fault.c
> +++ b/arch/riscv/mm/fault.c
> @@ -22,6 +22,58 @@
>  
>  #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;
> +
> +	pr_alert("Current %s pgtable: %luK pagesize, %d-bit VAs, pgdp=0x%016lx\n",
> +		 current->comm, PAGE_SIZE/SZ_1K, VA_BITS,
> +		 (mm == &init_mm ? __pa_symbol(mm->pgd) :
> +		 (unsigned long)virt_to_phys(mm->pgd)));

nit: unnecessary () around the ternary.

> +
> +	pgdp = pgd_offset(mm, addr);
> +	pgd = pgdp_get(pgdp);
> +	pr_alert("[%016lx] pgd=%016lx", addr, pgd_val(pgd));
> +	if (pgd_none(pgd) || pgd_bad(pgd) || pgd_leaf(pgd))
> +		goto out;
> +
> +	p4dp = p4d_offset(pgdp, addr);
> +	p4d = p4dp_get(p4dp);
> +	pr_cont(", p4d=%016lx", p4d_val(p4d));
> +	if (p4d_none(p4d) || p4d_bad(p4d) || p4d_leaf(p4d))
> +		goto out;
> +
> +	pudp = pud_offset(p4dp, addr);
> +	pud = pudp_get(pudp);
> +	pr_cont(", pud=%016lx", pud_val(pud));
> +	if (pud_none(pud) || pud_bad(pud) || pud_leaf(pud))
> +		goto out;
> +
> +	pmdp = pmd_offset(pudp, addr);
> +	pmd = pmdp_get(pmdp);
> +	pr_cont(", pmd=%016lx", pmd_val(pmd));
> +	if (pmd_none(pmd) || pmd_bad(pmd) || pmd_leaf(pmd))
> +		goto out;
> +
> +	ptep = pte_offset_map(pmdp, addr);
> +	if (!ptep)
> +		goto out;
> +
> +	pte = ptep_get(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 +83,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
> 
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Thanks,
drew

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

Posted by yunhui cui 1 year, 4 months ago

Hi drew,

On Tue, Jul 23, 2024 at 12:56 AM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Mon, Jul 22, 2024 at 12:20:37PM 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>
> > Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > ---
> >  arch/riscv/mm/fault.c | 53 +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 53 insertions(+)
> >
> > diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
> > index 5224f3733802..666d282a8bf4 100644
> > --- a/arch/riscv/mm/fault.c
> > +++ b/arch/riscv/mm/fault.c
> > @@ -22,6 +22,58 @@
> >
> >  #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;
> > +
> > +     pr_alert("Current %s pgtable: %luK pagesize, %d-bit VAs, pgdp=0x%016lx\n",
> > +              current->comm, PAGE_SIZE/SZ_1K, VA_BITS,
> > +              (mm == &init_mm ? __pa_symbol(mm->pgd) :
> > +              (unsigned long)virt_to_phys(mm->pgd)));
>
> nit: unnecessary () around the ternary.
Okay, I'll update it in v3.

>
> > +
> > +     pgdp = pgd_offset(mm, addr);
> > +     pgd = pgdp_get(pgdp);
> > +     pr_alert("[%016lx] pgd=%016lx", addr, pgd_val(pgd));
> > +     if (pgd_none(pgd) || pgd_bad(pgd) || pgd_leaf(pgd))
> > +             goto out;
> > +
> > +     p4dp = p4d_offset(pgdp, addr);
> > +     p4d = p4dp_get(p4dp);
> > +     pr_cont(", p4d=%016lx", p4d_val(p4d));
> > +     if (p4d_none(p4d) || p4d_bad(p4d) || p4d_leaf(p4d))
> > +             goto out;
> > +
> > +     pudp = pud_offset(p4dp, addr);
> > +     pud = pudp_get(pudp);
> > +     pr_cont(", pud=%016lx", pud_val(pud));
> > +     if (pud_none(pud) || pud_bad(pud) || pud_leaf(pud))
> > +             goto out;
> > +
> > +     pmdp = pmd_offset(pudp, addr);
> > +     pmd = pmdp_get(pmdp);
> > +     pr_cont(", pmd=%016lx", pmd_val(pmd));
> > +     if (pmd_none(pmd) || pmd_bad(pmd) || pmd_leaf(pmd))
> > +             goto out;
> > +
> > +     ptep = pte_offset_map(pmdp, addr);
> > +     if (!ptep)
> > +             goto out;
> > +
> > +     pte = ptep_get(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 +83,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
> >
> >
>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
>
> Thanks,
> drew

Thanks,
Yunhui