[PATCH v2 1/3] arm64: mm: print out correct page table entries

[PATCH v2 1/3] arm64: mm: print out correct page table entries

[Kristina Martsenko]

When we take a fault that can't be handled, we print out the page table
entries associated with the faulting address. In some cases we currently
print out the wrong entries. For a faulting TTBR1 address, we sometimes
print out TTBR0 table entries instead, and for a faulting TTBR0 address
we sometimes print out TTBR1 table entries. Fix this by choosing the
tables based on the faulting address.

Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>
---

v2:
 - use TASK_SIZE and VA_START, change printed message

 arch/arm64/include/asm/system_misc.h |  2 +-
 arch/arm64/mm/fault.c                | 29 +++++++++++++++++++----------
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h
index bc812435bc76..d0beefeb6d25 100644
--- a/arch/arm64/include/asm/system_misc.h
+++ b/arch/arm64/include/asm/system_misc.h
@@ -40,7 +40,7 @@ void hook_debug_fault_code(int nr, int (*fn)(unsigned long, unsigned int,
 			   int sig, int code, const char *name);
 
 struct mm_struct;
-extern void show_pte(struct mm_struct *mm, unsigned long addr);
+extern void show_pte(unsigned long addr);
 extern void __show_regs(struct pt_regs *);
 
 extern void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index c3f2b1048f83..a9dfb37c87a2 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -80,14 +80,24 @@ static inline int notify_page_fault(struct pt_regs *regs, unsigned int esr)
 #endif
 
 /*
- * Dump out the page tables associated with 'addr' in mm 'mm'.
+ * Dump out the page tables associated with 'addr' in the currently active mm.
  */
-void show_pte(struct mm_struct *mm, unsigned long addr)
+void show_pte(unsigned long addr)
 {
+	struct mm_struct *mm;
 	pgd_t *pgd;
 
-	if (!mm)
+	if (addr < TASK_SIZE) {
+		/* TTBR0 */
+		mm = current->active_mm;
+	} else if (addr >= VA_START) {
+		/* TTBR1 */
 		mm = &init_mm;
+	} else {
+		pr_alert("[%08lx] address between user and kernel address ranges\n",
+			 addr);
+		return;
+	}
 
 	pr_alert("pgd = %p\n", mm->pgd);
 	pgd = pgd_offset(mm, addr);
@@ -196,8 +206,8 @@ static inline bool is_permission_fault(unsigned int esr, struct pt_regs *regs,
 /*
  * The kernel tried to access some page that wasn't present.
  */
-static void __do_kernel_fault(struct mm_struct *mm, unsigned long addr,
-			      unsigned int esr, struct pt_regs *regs)
+static void __do_kernel_fault(unsigned long addr, unsigned int esr,
+			      struct pt_regs *regs)
 {
 	const char *msg;
 
@@ -227,7 +237,7 @@ static void __do_kernel_fault(struct mm_struct *mm, unsigned long addr,
 	pr_alert("Unable to handle kernel %s at virtual address %08lx\n", msg,
 		 addr);
 
-	show_pte(mm, addr);
+	show_pte(addr);
 	die("Oops", regs, esr);
 	bust_spinlocks(0);
 	do_exit(SIGKILL);
@@ -249,7 +259,7 @@ static void __do_user_fault(struct task_struct *tsk, unsigned long addr,
 		pr_info("%s[%d]: unhandled %s (%d) at 0x%08lx, esr 0x%03x\n",
 			tsk->comm, task_pid_nr(tsk), inf->name, sig,
 			addr, esr);
-		show_pte(tsk->mm, addr);
+		show_pte(addr);
 		__show_regs(regs);
 	}
 
@@ -265,7 +275,6 @@ static void __do_user_fault(struct task_struct *tsk, unsigned long addr,
 static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 {
 	struct task_struct *tsk = current;
-	struct mm_struct *mm = tsk->active_mm;
 	const struct fault_info *inf;
 
 	/*
@@ -276,7 +285,7 @@ static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *re
 		inf = esr_to_fault_info(esr);
 		__do_user_fault(tsk, addr, esr, inf->sig, inf->code, regs);
 	} else
-		__do_kernel_fault(mm, addr, esr, regs);
+		__do_kernel_fault(addr, esr, regs);
 }
 
 #define VM_FAULT_BADMAP		0x010000
@@ -475,7 +484,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	return 0;
 
 no_context:
-	__do_kernel_fault(mm, addr, esr, regs);
+	__do_kernel_fault(addr, esr, regs);
 	return 0;
 }
 
-- 
2.1.4

[PATCH v2 2/3] arm64: mm: don't print out page table entries on EL0 faults

[Kristina Martsenko]

When we take a fault from EL0 that can't be handled, we print out the
page table entries associated with the faulting address. This allows
userspace to print out any current page table entries, including kernel
(TTBR1) entries. Exposing kernel mappings like this could pose a
security risk, so don't print out page table information on EL0 faults.
(But still print it out for EL1 faults.) This also follows the same
behaviour as x86, printing out page table entries on kernel mode faults
but not user mode faults.

Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>
---

v2:
 - mention x86 in commit message

 arch/arm64/mm/fault.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index a9dfb37c87a2..b070dcd50ed0 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -259,7 +259,6 @@ static void __do_user_fault(struct task_struct *tsk, unsigned long addr,
 		pr_info("%s[%d]: unhandled %s (%d) at 0x%08lx, esr 0x%03x\n",
 			tsk->comm, task_pid_nr(tsk), inf->name, sig,
 			addr, esr);
-		show_pte(addr);
 		__show_regs(regs);
 	}
 
-- 
2.1.4

[PATCH v2 3/3] arm64: mm: print file name of faulting vma

[Kristina Martsenko]

Print out the name of the file associated with the vma that faulted.
This is usually the executable or shared library name. We already print
out the task name, but also printing the library name is useful for
pinpointing bugs to libraries.

Also print the base address and size of the vma, which together with the
PC (printed by __show_regs) gives the offset into the library.

Fault prints now look like:
test[2361]: unhandled level 2 translation fault (11) at 0x00000012, esr 0x92000006, in libfoo.so[ffffa0145000+1000]

This is already done on x86, for more details see commit 03252919b798
("x86: print which shared library/executable faulted in segfault etc.
messages v3").

Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>
---

v2:
 - add this patch

 arch/arm64/mm/fault.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index b070dcd50ed0..6a2a5dd7cf36 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -256,9 +256,11 @@ static void __do_user_fault(struct task_struct *tsk, unsigned long addr,
 
 	if (unhandled_signal(tsk, sig) && show_unhandled_signals_ratelimited()) {
 		inf = esr_to_fault_info(esr);
-		pr_info("%s[%d]: unhandled %s (%d) at 0x%08lx, esr 0x%03x\n",
+		pr_info("%s[%d]: unhandled %s (%d) at 0x%08lx, esr 0x%03x",
 			tsk->comm, task_pid_nr(tsk), inf->name, sig,
 			addr, esr);
+		print_vma_addr(KERN_CONT ", in ", regs->pc);
+		pr_cont("\n");
 		__show_regs(regs);
 	}
 
-- 
2.1.4

[PATCH v2 2/3] arm64: mm: don't print out page table entries on EL0 faults

[Mark Rutland]

On Fri, Jun 09, 2017 at 04:35:53PM +0100, Kristina Martsenko wrote:
> When we take a fault from EL0 that can't be handled, we print out the
> page table entries associated with the faulting address. This allows
> userspace to print out any current page table entries, including kernel
> (TTBR1) entries. Exposing kernel mappings like this could pose a
> security risk, so don't print out page table information on EL0 faults.
> (But still print it out for EL1 faults.) This also follows the same
> behaviour as x86, printing out page table entries on kernel mode faults
> but not user mode faults.
> 
> Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>
> ---
> 
> v2:
>  - mention x86 in commit message
> 
>  arch/arm64/mm/fault.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index a9dfb37c87a2..b070dcd50ed0 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -259,7 +259,6 @@ static void __do_user_fault(struct task_struct *tsk, unsigned long addr,
>  		pr_info("%s[%d]: unhandled %s (%d) at 0x%08lx, esr 0x%03x\n",
>  			tsk->comm, task_pid_nr(tsk), inf->name, sig,
>  			addr, esr);
> -		show_pte(addr);
>  		__show_regs(regs);
>  	}

AFAICT, we haven't taken mmap_sem or anything else that would stop
another thread modifying the user page tables. So show_pte() here could
result in dereferencing junk if we were particularly unlucky.

I think we might want to Cc stable on this.

Either way:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.

[PATCH v2 3/3] arm64: mm: print file name of faulting vma

[Mark Rutland]

On Fri, Jun 09, 2017 at 04:35:54PM +0100, Kristina Martsenko wrote:
> Print out the name of the file associated with the vma that faulted.
> This is usually the executable or shared library name. We already print
> out the task name, but also printing the library name is useful for
> pinpointing bugs to libraries.
> 
> Also print the base address and size of the vma, which together with the
> PC (printed by __show_regs) gives the offset into the library.
> 
> Fault prints now look like:
> test[2361]: unhandled level 2 translation fault (11) at 0x00000012, esr 0x92000006, in libfoo.so[ffffa0145000+1000]
> 
> This is already done on x86, for more details see commit 03252919b798
> ("x86: print which shared library/executable faulted in segfault etc.
> messages v3").
> 
> Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>

FWIW:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> ---
> 
> v2:
>  - add this patch
> 
>  arch/arm64/mm/fault.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index b070dcd50ed0..6a2a5dd7cf36 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -256,9 +256,11 @@ static void __do_user_fault(struct task_struct *tsk, unsigned long addr,
>  
>  	if (unhandled_signal(tsk, sig) && show_unhandled_signals_ratelimited()) {
>  		inf = esr_to_fault_info(esr);
> -		pr_info("%s[%d]: unhandled %s (%d) at 0x%08lx, esr 0x%03x\n",
> +		pr_info("%s[%d]: unhandled %s (%d) at 0x%08lx, esr 0x%03x",
>  			tsk->comm, task_pid_nr(tsk), inf->name, sig,
>  			addr, esr);
> +		print_vma_addr(KERN_CONT ", in ", regs->pc);
> +		pr_cont("\n");
>  		__show_regs(regs);
>  	}
>  
> -- 
> 2.1.4
> 

[PATCH v2 1/3] arm64: mm: print out correct page table entries

[Mark Rutland]

On Fri, Jun 09, 2017 at 04:35:52PM +0100, Kristina Martsenko wrote:
>  /*
> - * Dump out the page tables associated with 'addr' in mm 'mm'.
> + * Dump out the page tables associated with 'addr' in the currently active mm.
>   */
> -void show_pte(struct mm_struct *mm, unsigned long addr)
> +void show_pte(unsigned long addr)
>  {
> +	struct mm_struct *mm;
>  	pgd_t *pgd;
>  
> -	if (!mm)
> +	if (addr < TASK_SIZE) {
> +		/* TTBR0 */
> +		mm = current->active_mm;

Can we log something for the active_mm == &init_mm case?

e.g.

	if (addr < TASK_SIZE) {
		if (current->active_mm == &init_mm)
			pr_alert("[%016lx] address in unknown TTBR0 range\n",
				addr);
		else
			mm = current->active_mm;
	} ...
	
> +	} else if (addr >= VA_START) {
> +		/* TTBR1 */
>  		mm = &init_mm;
> +	} else {
> +		pr_alert("[%08lx] address between user and kernel address ranges\n",
> +			 addr);

I think that should be %016lx, given the address will be 64-bit.

With those:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.

[PATCH v2 1/3] arm64: mm: print out correct page table entries

[Will Deacon]

On Fri, Jun 09, 2017 at 05:04:07PM +0100, Mark Rutland wrote:
> On Fri, Jun 09, 2017 at 04:35:52PM +0100, Kristina Martsenko wrote:
> >  /*
> > - * Dump out the page tables associated with 'addr' in mm 'mm'.
> > + * Dump out the page tables associated with 'addr' in the currently active mm.
> >   */
> > -void show_pte(struct mm_struct *mm, unsigned long addr)
> > +void show_pte(unsigned long addr)
> >  {
> > +	struct mm_struct *mm;
> >  	pgd_t *pgd;
> >  
> > -	if (!mm)
> > +	if (addr < TASK_SIZE) {
> > +		/* TTBR0 */
> > +		mm = current->active_mm;
> 
> Can we log something for the active_mm == &init_mm case?
> 
> e.g.
> 
> 	if (addr < TASK_SIZE) {
> 		if (current->active_mm == &init_mm)
> 			pr_alert("[%016lx] address in unknown TTBR0 range\n",
> 				addr);

I don't understand the case you're trying to highlight here, nor which
table you want to walk.

Will

[PATCH v2 1/3] arm64: mm: print out correct page table entries

[Mark Rutland]

On Fri, Jun 09, 2017 at 05:33:29PM +0100, Will Deacon wrote:
> On Fri, Jun 09, 2017 at 05:04:07PM +0100, Mark Rutland wrote:
> > On Fri, Jun 09, 2017 at 04:35:52PM +0100, Kristina Martsenko wrote:
> > >  /*
> > > - * Dump out the page tables associated with 'addr' in mm 'mm'.
> > > + * Dump out the page tables associated with 'addr' in the currently active mm.
> > >   */
> > > -void show_pte(struct mm_struct *mm, unsigned long addr)
> > > +void show_pte(unsigned long addr)
> > >  {
> > > +	struct mm_struct *mm;
> > >  	pgd_t *pgd;
> > >  
> > > -	if (!mm)
> > > +	if (addr < TASK_SIZE) {
> > > +		/* TTBR0 */
> > > +		mm = current->active_mm;
> > 
> > Can we log something for the active_mm == &init_mm case?
> > 
> > e.g.
> > 
> > 	if (addr < TASK_SIZE) {
> > 		if (current->active_mm == &init_mm)
> > 			pr_alert("[%016lx] address in unknown TTBR0 range\n",
> > 				addr);
> 
> I don't understand the case you're trying to highlight here, nor which
> table you want to walk.

On some threads, current->active_mm is the init_mm, and this doesn't
represent TTBR0.

In this case, TTBR0 is typically the zero page (or the idmap).

If we don't explicitly handle the init_mm case, we'd walk the kernel's
TTBR1 page table when reporting such faults, which would be misleading.

The easiest thing to do is to bail out, rather than trying to read TTBR0
and walk whatever it points to, since that could have an unusual VA
size.

Thanks,
Mark.

[PATCH v2 1/3] arm64: mm: print out correct page table entries

[Yury Norov]

Hi, Kristina,

On Fri, Jun 09, 2017 at 04:35:52PM +0100, Kristina Martsenko wrote:
> When we take a fault that can't be handled, we print out the page table
> entries associated with the faulting address. In some cases we currently
> print out the wrong entries. For a faulting TTBR1 address, we sometimes
> print out TTBR0 table entries instead, and for a faulting TTBR0 address
> we sometimes print out TTBR1 table entries. Fix this by choosing the
> tables based on the faulting address.
> 
> Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>
> ---
> 
> v2:
>  - use TASK_SIZE and VA_START, change printed message
> 
>  arch/arm64/include/asm/system_misc.h |  2 +-
>  arch/arm64/mm/fault.c                | 29 +++++++++++++++++++----------
>  2 files changed, 20 insertions(+), 11 deletions(-)

[...]

> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index c3f2b1048f83..a9dfb37c87a2 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -80,14 +80,24 @@ static inline int notify_page_fault(struct pt_regs *regs, unsigned int esr)
>  #endif
>  
>  /*
> - * Dump out the page tables associated with 'addr' in mm 'mm'.
> + * Dump out the page tables associated with 'addr' in the currently active mm.
>   */
> -void show_pte(struct mm_struct *mm, unsigned long addr)
> +void show_pte(unsigned long addr)
>  {
> +	struct mm_struct *mm;
>  	pgd_t *pgd;
>  
> -	if (!mm)
> +	if (addr < TASK_SIZE) {
> +		/* TTBR0 */
> +		mm = current->active_mm;
> +	} else if (addr >= VA_START) {
> +		/* TTBR1 */
>  		mm = &init_mm;
> +	} else {
> +		pr_alert("[%08lx] address between user and kernel address ranges\n",
> +			 addr);
> +		return;
> +	}
>  
>  	pr_alert("pgd = %p\n", mm->pgd);
>  	pgd = pgd_offset(mm, addr);

Is there any reason to change the prototype of the function?
You say nothing about it in patch description. The show_pte()
is implemented in several arches, and everywhere it takes mm_struct
as 1st argument. For me, if you don't need to change the prototype,
you'd better leave things as is. The patch will get much shorter
and expressive with it.

If you really need to change the prototype, it would be better to do
it in separated patch and give clear explanations - what for?

Yury

[PATCH v2 1/3] arm64: mm: print out correct page table entries

[Will Deacon]

On Fri, Jun 09, 2017 at 11:22:36PM +0300, Yury Norov wrote:
> On Fri, Jun 09, 2017 at 04:35:52PM +0100, Kristina Martsenko wrote:
> > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> > index c3f2b1048f83..a9dfb37c87a2 100644
> > --- a/arch/arm64/mm/fault.c
> > +++ b/arch/arm64/mm/fault.c
> > @@ -80,14 +80,24 @@ static inline int notify_page_fault(struct pt_regs *regs, unsigned int esr)
> >  #endif
> >  
> >  /*
> > - * Dump out the page tables associated with 'addr' in mm 'mm'.
> > + * Dump out the page tables associated with 'addr' in the currently active mm.
> >   */
> > -void show_pte(struct mm_struct *mm, unsigned long addr)
> > +void show_pte(unsigned long addr)
> >  {
> > +	struct mm_struct *mm;
> >  	pgd_t *pgd;
> >  
> > -	if (!mm)
> > +	if (addr < TASK_SIZE) {
> > +		/* TTBR0 */
> > +		mm = current->active_mm;
> > +	} else if (addr >= VA_START) {
> > +		/* TTBR1 */
> >  		mm = &init_mm;
> > +	} else {
> > +		pr_alert("[%08lx] address between user and kernel address ranges\n",
> > +			 addr);
> > +		return;
> > +	}
> >  
> >  	pr_alert("pgd = %p\n", mm->pgd);
> >  	pgd = pgd_offset(mm, addr);
> 
> Is there any reason to change the prototype of the function?
> You say nothing about it in patch description. The show_pte()
> is implemented in several arches, and everywhere it takes mm_struct
> as 1st argument. For me, if you don't need to change the prototype,
> you'd better leave things as is. The patch will get much shorter
> and expressive with it.
> 
> If you really need to change the prototype, it would be better to do
> it in separated patch and give clear explanations - what for?

The mm is unused and this isn't a core interface. In fact, it's only
available on architectures that started off by copying from arch/arm/.

We can always add the mm back if we need it in future.

Will

[PATCH v2 1/3] arm64: mm: print out correct page table entries

[Yury Norov]

On Thu, Jun 15, 2017 at 11:00:51AM +0100, Will Deacon wrote:
> On Fri, Jun 09, 2017 at 11:22:36PM +0300, Yury Norov wrote:
> > On Fri, Jun 09, 2017 at 04:35:52PM +0100, Kristina Martsenko wrote:
> > > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> > > index c3f2b1048f83..a9dfb37c87a2 100644
> > > --- a/arch/arm64/mm/fault.c
> > > +++ b/arch/arm64/mm/fault.c
> > > @@ -80,14 +80,24 @@ static inline int notify_page_fault(struct pt_regs *regs, unsigned int esr)
> > >  #endif
> > >  
> > >  /*
> > > - * Dump out the page tables associated with 'addr' in mm 'mm'.
> > > + * Dump out the page tables associated with 'addr' in the currently active mm.
> > >   */
> > > -void show_pte(struct mm_struct *mm, unsigned long addr)
> > > +void show_pte(unsigned long addr)
> > >  {
> > > +	struct mm_struct *mm;
> > >  	pgd_t *pgd;
> > >  
> > > -	if (!mm)
> > > +	if (addr < TASK_SIZE) {
> > > +		/* TTBR0 */
> > > +		mm = current->active_mm;
> > > +	} else if (addr >= VA_START) {
> > > +		/* TTBR1 */
> > >  		mm = &init_mm;
> > > +	} else {
> > > +		pr_alert("[%08lx] address between user and kernel address ranges\n",
> > > +			 addr);
> > > +		return;
> > > +	}
> > >  
> > >  	pr_alert("pgd = %p\n", mm->pgd);
> > >  	pgd = pgd_offset(mm, addr);
> > 
> > Is there any reason to change the prototype of the function?
> > You say nothing about it in patch description. The show_pte()
> > is implemented in several arches, and everywhere it takes mm_struct
> > as 1st argument. For me, if you don't need to change the prototype,
> > you'd better leave things as is. The patch will get much shorter
> > and expressive with it.
> > 
> > If you really need to change the prototype, it would be better to do
> > it in separated patch and give clear explanations - what for?
> 
> The mm is unused and this isn't a core interface. In fact, it's only
> available on architectures that started off by copying from arch/arm/.
> 
> We can always add the mm back if we need it in future.

OK, understood that. But I still think it should be done in separeted
patch, and it would be also good to revise sh and unicore32. It seems
that sh may be easily switched to the proposed interface, and the
unicore32 most probably too.

Yury

[PATCH v2 1/3] arm64: mm: print out correct page table entries

[Will Deacon]

On Thu, Jun 15, 2017 at 01:12:30PM +0300, Yury Norov wrote:
> On Thu, Jun 15, 2017 at 11:00:51AM +0100, Will Deacon wrote:
> > On Fri, Jun 09, 2017 at 11:22:36PM +0300, Yury Norov wrote:
> > > On Fri, Jun 09, 2017 at 04:35:52PM +0100, Kristina Martsenko wrote:
> > > > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> > > > index c3f2b1048f83..a9dfb37c87a2 100644
> > > > --- a/arch/arm64/mm/fault.c
> > > > +++ b/arch/arm64/mm/fault.c
> > > > @@ -80,14 +80,24 @@ static inline int notify_page_fault(struct pt_regs *regs, unsigned int esr)
> > > >  #endif
> > > >  
> > > >  /*
> > > > - * Dump out the page tables associated with 'addr' in mm 'mm'.
> > > > + * Dump out the page tables associated with 'addr' in the currently active mm.
> > > >   */
> > > > -void show_pte(struct mm_struct *mm, unsigned long addr)
> > > > +void show_pte(unsigned long addr)
> > > >  {
> > > > +	struct mm_struct *mm;
> > > >  	pgd_t *pgd;
> > > >  
> > > > -	if (!mm)
> > > > +	if (addr < TASK_SIZE) {
> > > > +		/* TTBR0 */
> > > > +		mm = current->active_mm;
> > > > +	} else if (addr >= VA_START) {
> > > > +		/* TTBR1 */
> > > >  		mm = &init_mm;
> > > > +	} else {
> > > > +		pr_alert("[%08lx] address between user and kernel address ranges\n",
> > > > +			 addr);
> > > > +		return;
> > > > +	}
> > > >  
> > > >  	pr_alert("pgd = %p\n", mm->pgd);
> > > >  	pgd = pgd_offset(mm, addr);
> > > 
> > > Is there any reason to change the prototype of the function?
> > > You say nothing about it in patch description. The show_pte()
> > > is implemented in several arches, and everywhere it takes mm_struct
> > > as 1st argument. For me, if you don't need to change the prototype,
> > > you'd better leave things as is. The patch will get much shorter
> > > and expressive with it.
> > > 
> > > If you really need to change the prototype, it would be better to do
> > > it in separated patch and give clear explanations - what for?
> > 
> > The mm is unused and this isn't a core interface. In fact, it's only
> > available on architectures that started off by copying from arch/arm/.
> > 
> > We can always add the mm back if we need it in future.
> 
> OK, understood that. But I still think it should be done in separeted
> patch, and it would be also good to revise sh and unicore32. It seems
> that sh may be easily switched to the proposed interface, and the
> unicore32 most probably too.

Why? This is the patch that removes the usage of the paramater, so it should
also remove the parameter. This function isn't called by anybody outside of
arm64, so there's no "proposed interface" to speak of.

If this was a core function with lots of tree-wide callers, then I'd agree
with you: remove use of the thing, fix the callers, then remove the
parameter. But that's not the case here.

Will

Re: [PATCH v2 1/3] arm64: mm: print out correct page table entries

[Yury Norov]

Hi Kristina, Will,

I have asked the question on the patch "arm64: mm: print out correct
page table entries", and as far received no answer on it:
https://www.spinics.net/lists/arm-kernel/msg587269.html

But today I found the patch applied in linux-next (commit
67ce16ec15ce9d). Could you answer my questions?

Yury