[PATCH] powerpc64/hw_breakpoints: Handle data breakpoints in radix mode

Subsystems: linux for powerpc (32-bit and 64-bit), the rest

STALE1719d

18 messages, 7 authors, 2021-11-25 · open the first message on its own page

[PATCH] powerpc64/hw_breakpoints: Handle data breakpoints in radix mode

From: Naveen N. Rao <hidden>
Date: 2017-06-13 18:42:27

On P9, trying to use data breakpoints throws the splat shown below (*).
This is because the check for a data breakpoint in DSISR is in
do_hash_page(). Move this check to handle_page_fault() so as to catch
data breakpoints in both hash and radix MMU modes.

While at it, also remove the label '11' that was made redundant by
commit a546498f3bf9aa ("powerpc: Call do_page_fault() with interrupts
off")

(*)
    Unable to handle kernel paging request for data at address 0xc000000000e19218
    Faulting instruction address: 0xc0000000001155e8
    cpu 0x0: Vector: 300 (Data Access) at [c0000000ef1e7b20]
    pc: c0000000001155e8: find_pid_ns+0x48/0xe0
    lr: c000000000116ac4: find_task_by_vpid+0x44/0x90
    sp: c0000000ef1e7da0
    msr: 9000000000009033
    dar: c000000000e19218
    dsisr: 400000
    current = 0xc0000000f1f59700
    paca    = 0xc00000000fd40000 softe: 0 irq_happened: 0x01
    pid   = 1192, comm = sh
    Linux version 4.12.0-rc3-nnr (root@ea605ec2993c) (gcc version 5.4.0 20160609 (Ubuntu/IBM 5.4.0-6ubuntu1~16.04.1) ) #74 SMP Tue Jun 13 16:52:49 UTC 2017
    enter ? for help
    [c0000000ef1e7dc0] c000000000116ac4 find_task_by_vpid+0x44/0x90
    [c0000000ef1e7de0] c000000000108800 SyS_setpgid+0x80/0x220
    [c0000000ef1e7e30] c00000000000ba6c system_call+0x38/0xfc
    --- Exception: c01 (System Call) at 00007fff94480890
    SP (7fffd91e7260) is in userspace

Fixes: caca285e5ab4a ("powerpc/mm/radix: Use STD_MMU_64 to properly
isolate hash related code")
Reported-by: Shriya R. Kulkarni <redacted>
Signed-off-by: Naveen N. Rao <redacted>
---
 arch/powerpc/kernel/exceptions-64s.S | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index ae418b85c17c..17ee701b8336 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1411,10 +1411,8 @@ USE_TEXT_SECTION()
 	.balign	IFETCH_ALIGN_BYTES
 do_hash_page:
 #ifdef CONFIG_PPC_STD_MMU_64
-	andis.	r0,r4,0xa410		/* weird error? */
+	andis.	r0,r4,0xa450		/* weird error? */
 	bne-	handle_page_fault	/* if not, try to insert a HPTE */
-	andis.  r0,r4,DSISR_DABRMATCH@h
-	bne-    handle_dabr_fault
 	CURRENT_THREAD_INFO(r11, r1)
 	lwz	r0,TI_PREEMPT(r11)	/* If we're in an "NMI" */
 	andis.	r0,r0,NMI_MASK@h	/* (i.e. an irq when soft-disabled) */
@@ -1442,7 +1440,9 @@ do_hash_page:
 
 /* Here we have a page fault that hash_page can't handle. */
 handle_page_fault:
-11:	ld	r4,_DAR(r1)
+	andis.  r0,r4,DSISR_DABRMATCH@h
+	bne-    handle_dabr_fault
+	ld	r4,_DAR(r1)
 	ld	r5,_DSISR(r1)
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	do_page_fault
-- 
2.12.2

Re: [PATCH] powerpc64/hw_breakpoints: Handle data breakpoints in radix mode

From: Aneesh Kumar K.V <hidden>
Date: 2017-06-14 03:08:59

"Naveen N. Rao" [off-list ref] writes:
quoted hunk
On P9, trying to use data breakpoints throws the splat shown below (*).
This is because the check for a data breakpoint in DSISR is in
do_hash_page(). Move this check to handle_page_fault() so as to catch
data breakpoints in both hash and radix MMU modes.

While at it, also remove the label '11' that was made redundant by
commit a546498f3bf9aa ("powerpc: Call do_page_fault() with interrupts
off")

(*)
    Unable to handle kernel paging request for data at address 0xc000000000e19218
    Faulting instruction address: 0xc0000000001155e8
    cpu 0x0: Vector: 300 (Data Access) at [c0000000ef1e7b20]
    pc: c0000000001155e8: find_pid_ns+0x48/0xe0
    lr: c000000000116ac4: find_task_by_vpid+0x44/0x90
    sp: c0000000ef1e7da0
    msr: 9000000000009033
    dar: c000000000e19218
    dsisr: 400000
    current = 0xc0000000f1f59700
    paca    = 0xc00000000fd40000 softe: 0 irq_happened: 0x01
    pid   = 1192, comm = sh
    Linux version 4.12.0-rc3-nnr (root@ea605ec2993c) (gcc version 5.4.0 20160609 (Ubuntu/IBM 5.4.0-6ubuntu1~16.04.1) ) #74 SMP Tue Jun 13 16:52:49 UTC 2017
    enter ? for help
    [c0000000ef1e7dc0] c000000000116ac4 find_task_by_vpid+0x44/0x90
    [c0000000ef1e7de0] c000000000108800 SyS_setpgid+0x80/0x220
    [c0000000ef1e7e30] c00000000000ba6c system_call+0x38/0xfc
    --- Exception: c01 (System Call) at 00007fff94480890
    SP (7fffd91e7260) is in userspace

Fixes: caca285e5ab4a ("powerpc/mm/radix: Use STD_MMU_64 to properly
isolate hash related code")
Reported-by: Shriya R. Kulkarni <redacted>
Signed-off-by: Naveen N. Rao <redacted>
---
 arch/powerpc/kernel/exceptions-64s.S | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index ae418b85c17c..17ee701b8336 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1411,10 +1411,8 @@ USE_TEXT_SECTION()
 	.balign	IFETCH_ALIGN_BYTES
 do_hash_page:
 #ifdef CONFIG_PPC_STD_MMU_64
-	andis.	r0,r4,0xa410		/* weird error? */
+	andis.	r0,r4,0xa450		/* weird error? */
Can we convert that to a #define value. Ram did try to do that here.

https://lists.ozlabs.org/pipermail/linuxppc-dev/2017-June/158607.html

quoted hunk
 	bne-	handle_page_fault	/* if not, try to insert a HPTE */
-	andis.  r0,r4,DSISR_DABRMATCH@h
-	bne-    handle_dabr_fault
 	CURRENT_THREAD_INFO(r11, r1)
 	lwz	r0,TI_PREEMPT(r11)	/* If we're in an "NMI" */
 	andis.	r0,r0,NMI_MASK@h	/* (i.e. an irq when soft-disabled) */
@@ -1442,7 +1440,9 @@ do_hash_page:
 
 /* Here we have a page fault that hash_page can't handle. */
 handle_page_fault:
-11:	ld	r4,_DAR(r1)
+	andis.  r0,r4,DSISR_DABRMATCH@h
+	bne-    handle_dabr_fault
+	ld	r4,_DAR(r1)
 	ld	r5,_DSISR(r1)
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	do_page_fault
-- 
2.12.2

-aneesh

Re: [PATCH] powerpc64/hw_breakpoints: Handle data breakpoints in radix mode

From: Naveen N. Rao <hidden>
Date: 2017-06-14 05:11:20

Hi Aneesh,

On 2017/06/14 08:38AM, Aneesh Kumar K.V wrote:
"Naveen N. Rao" [off-list ref] writes:
quoted
On P9, trying to use data breakpoints throws the splat shown below (*).
This is because the check for a data breakpoint in DSISR is in
do_hash_page(). Move this check to handle_page_fault() so as to catch
data breakpoints in both hash and radix MMU modes.

While at it, also remove the label '11' that was made redundant by
commit a546498f3bf9aa ("powerpc: Call do_page_fault() with interrupts
off")

(*)
    Unable to handle kernel paging request for data at address 0xc000000000e19218
    Faulting instruction address: 0xc0000000001155e8
    cpu 0x0: Vector: 300 (Data Access) at [c0000000ef1e7b20]
    pc: c0000000001155e8: find_pid_ns+0x48/0xe0
    lr: c000000000116ac4: find_task_by_vpid+0x44/0x90
    sp: c0000000ef1e7da0
    msr: 9000000000009033
    dar: c000000000e19218
    dsisr: 400000
    current = 0xc0000000f1f59700
    paca    = 0xc00000000fd40000 softe: 0 irq_happened: 0x01
    pid   = 1192, comm = sh
    Linux version 4.12.0-rc3-nnr (root@ea605ec2993c) (gcc version 5.4.0 20160609 (Ubuntu/IBM 5.4.0-6ubuntu1~16.04.1) ) #74 SMP Tue Jun 13 16:52:49 UTC 2017
    enter ? for help
    [c0000000ef1e7dc0] c000000000116ac4 find_task_by_vpid+0x44/0x90
    [c0000000ef1e7de0] c000000000108800 SyS_setpgid+0x80/0x220
    [c0000000ef1e7e30] c00000000000ba6c system_call+0x38/0xfc
    --- Exception: c01 (System Call) at 00007fff94480890
    SP (7fffd91e7260) is in userspace

Fixes: caca285e5ab4a ("powerpc/mm/radix: Use STD_MMU_64 to properly
isolate hash related code")
Reported-by: Shriya R. Kulkarni <redacted>
Signed-off-by: Naveen N. Rao <redacted>
---
 arch/powerpc/kernel/exceptions-64s.S | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index ae418b85c17c..17ee701b8336 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1411,10 +1411,8 @@ USE_TEXT_SECTION()
 	.balign	IFETCH_ALIGN_BYTES
 do_hash_page:
 #ifdef CONFIG_PPC_STD_MMU_64
-	andis.	r0,r4,0xa410		/* weird error? */
+	andis.	r0,r4,0xa450		/* weird error? */
Can we convert that to a #define value. Ram did try to do that here.

https://lists.ozlabs.org/pipermail/linuxppc-dev/2017-June/158607.html
Hmm... I feel it will be good to do that as part of Ram's series since 
he has already coded it up :)

Ram's patches will anyway require a rebase and the change I do here for 
detecting DAWR already has a #define, so it should be a simple matter of 
including DSISR_DABRMATCH in DSISR_PAGE_FAULT_MASK.

But, if you really feel that I should make that change here, please do 
let me know and I will re-spin with those changes.


Thanks for the review!
- Naveen

Re: [PATCH] powerpc64/hw_breakpoints: Handle data breakpoints in radix mode

From: Aneesh Kumar K.V <hidden>
Date: 2017-06-14 05:13:47


On Wednesday 14 June 2017 10:41 AM, Naveen N. Rao wrote:
Hi Aneesh,

On 2017/06/14 08:38AM, Aneesh Kumar K.V wrote:
quoted
"Naveen N. Rao" [off-list ref] writes:
quoted
On P9, trying to use data breakpoints throws the splat shown below (*).
This is because the check for a data breakpoint in DSISR is in
do_hash_page(). Move this check to handle_page_fault() so as to catch
data breakpoints in both hash and radix MMU modes.

While at it, also remove the label '11' that was made redundant by
commit a546498f3bf9aa ("powerpc: Call do_page_fault() with interrupts
off")

(*)
     Unable to handle kernel paging request for data at address 0xc000000000e19218
     Faulting instruction address: 0xc0000000001155e8
     cpu 0x0: Vector: 300 (Data Access) at [c0000000ef1e7b20]
     pc: c0000000001155e8: find_pid_ns+0x48/0xe0
     lr: c000000000116ac4: find_task_by_vpid+0x44/0x90
     sp: c0000000ef1e7da0
     msr: 9000000000009033
     dar: c000000000e19218
     dsisr: 400000
     current = 0xc0000000f1f59700
     paca    = 0xc00000000fd40000 softe: 0 irq_happened: 0x01
     pid   = 1192, comm = sh
     Linux version 4.12.0-rc3-nnr (root@ea605ec2993c) (gcc version 5.4.0 20160609 (Ubuntu/IBM 5.4.0-6ubuntu1~16.04.1) ) #74 SMP Tue Jun 13 16:52:49 UTC 2017
     enter ? for help
     [c0000000ef1e7dc0] c000000000116ac4 find_task_by_vpid+0x44/0x90
     [c0000000ef1e7de0] c000000000108800 SyS_setpgid+0x80/0x220
     [c0000000ef1e7e30] c00000000000ba6c system_call+0x38/0xfc
     --- Exception: c01 (System Call) at 00007fff94480890
     SP (7fffd91e7260) is in userspace

Fixes: caca285e5ab4a ("powerpc/mm/radix: Use STD_MMU_64 to properly
isolate hash related code")
Reported-by: Shriya R. Kulkarni <redacted>
Signed-off-by: Naveen N. Rao <redacted>
---
  arch/powerpc/kernel/exceptions-64s.S | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index ae418b85c17c..17ee701b8336 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1411,10 +1411,8 @@ USE_TEXT_SECTION()
  	.balign	IFETCH_ALIGN_BYTES
  do_hash_page:
  #ifdef CONFIG_PPC_STD_MMU_64
-	andis.	r0,r4,0xa410		/* weird error? */
+	andis.	r0,r4,0xa450		/* weird error? */
Can we convert that to a #define value. Ram did try to do that here.

https://lists.ozlabs.org/pipermail/linuxppc-dev/2017-June/158607.html
Hmm... I feel it will be good to do that as part of Ram's series since
he has already coded it up :)

Ram's patches will anyway require a rebase and the change I do here for
detecting DAWR already has a #define, so it should be a simple matter of
including DSISR_DABRMATCH in DSISR_PAGE_FAULT_MASK.

But, if you really feel that I should make that change here, please do
let me know and I will re-spin with those changes.
The thing is that change from 0xa410 to 0xa450 is not clear at all. And 
it needs proper documentation. IMHO the best way to do that is switch to 
#define name for that constant.

-aneesh

Re: [PATCH] powerpc64/hw_breakpoints: Handle data breakpoints in radix mode

From: Anshuman Khandual <hidden>
Date: 2017-06-14 06:09:00

On 06/14/2017 12:12 AM, Naveen N. Rao wrote:
On P9, trying to use data breakpoints throws the splat shown below (*).
This is because the check for a data breakpoint in DSISR is in
do_hash_page(). Move this check to handle_page_fault() so as to catch
data breakpoints in both hash and radix MMU modes.
Why cant we check for DSISR inside do_hash_page() on P9 ?

Re: [PATCH] powerpc64/hw_breakpoints: Handle data breakpoints in radix mode

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2017-06-14 06:41:17

"Aneesh Kumar K.V" [off-list ref] writes:
On Wednesday 14 June 2017 10:41 AM, Naveen N. Rao wrote:
quoted
On 2017/06/14 08:38AM, Aneesh Kumar K.V wrote:
quoted
"Naveen N. Rao" [off-list ref] writes:
quoted
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index ae418b85c17c..17ee701b8336 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1411,10 +1411,8 @@ USE_TEXT_SECTION()
  	.balign	IFETCH_ALIGN_BYTES
  do_hash_page:
  #ifdef CONFIG_PPC_STD_MMU_64
-	andis.	r0,r4,0xa410		/* weird error? */
+	andis.	r0,r4,0xa450		/* weird error? */
Can we convert that to a #define value. Ram did try to do that here.

https://lists.ozlabs.org/pipermail/linuxppc-dev/2017-June/158607.html
Hmm... I feel it will be good to do that as part of Ram's series since
he has already coded it up :)

Ram's patches will anyway require a rebase and the change I do here for
detecting DAWR already has a #define, so it should be a simple matter of
including DSISR_DABRMATCH in DSISR_PAGE_FAULT_MASK.

But, if you really feel that I should make that change here, please do
let me know and I will re-spin with those changes.
The thing is that change from 0xa410 to 0xa450 is not clear at all. And 
it needs proper documentation. IMHO the best way to do that is switch to 
#define name for that constant.
Not in this patch. It needs to be backported, so it should be as minimal
as possible.

The change from 0xa410 to 0xa450 does need a mention in the changelog,
I'll add that.

cheers

Re: [PATCH] powerpc64/hw_breakpoints: Handle data breakpoints in radix mode

From: Ram Pai <hidden>
Date: 2017-06-14 06:42:25

On Wed, Jun 14, 2017 at 10:43:30AM +0530, Aneesh Kumar K.V wrote:

On Wednesday 14 June 2017 10:41 AM, Naveen N. Rao wrote:
quoted
Hi Aneesh,

On 2017/06/14 08:38AM, Aneesh Kumar K.V wrote:
quoted
"Naveen N. Rao" [off-list ref] writes:
quoted
On P9, trying to use data breakpoints throws the splat shown below (*).
This is because the check for a data breakpoint in DSISR is in
do_hash_page(). Move this check to handle_page_fault() so as to catch
data breakpoints in both hash and radix MMU modes.

While at it, also remove the label '11' that was made redundant by
commit a546498f3bf9aa ("powerpc: Call do_page_fault() with interrupts
off")

(*)
    Unable to handle kernel paging request for data at address 0xc000000000e19218
    Faulting instruction address: 0xc0000000001155e8
    cpu 0x0: Vector: 300 (Data Access) at [c0000000ef1e7b20]
    pc: c0000000001155e8: find_pid_ns+0x48/0xe0
    lr: c000000000116ac4: find_task_by_vpid+0x44/0x90
    sp: c0000000ef1e7da0
    msr: 9000000000009033
    dar: c000000000e19218
    dsisr: 400000
    current = 0xc0000000f1f59700
    paca    = 0xc00000000fd40000 softe: 0 irq_happened: 0x01
    pid   = 1192, comm = sh
    Linux version 4.12.0-rc3-nnr (root@ea605ec2993c) (gcc version 5.4.0 20160609 (Ubuntu/IBM 5.4.0-6ubuntu1~16.04.1) ) #74 SMP Tue Jun 13 16:52:49 UTC 2017
    enter ? for help
    [c0000000ef1e7dc0] c000000000116ac4 find_task_by_vpid+0x44/0x90
    [c0000000ef1e7de0] c000000000108800 SyS_setpgid+0x80/0x220
    [c0000000ef1e7e30] c00000000000ba6c system_call+0x38/0xfc
    --- Exception: c01 (System Call) at 00007fff94480890
    SP (7fffd91e7260) is in userspace

Fixes: caca285e5ab4a ("powerpc/mm/radix: Use STD_MMU_64 to properly
isolate hash related code")
Reported-by: Shriya R. Kulkarni <redacted>
Signed-off-by: Naveen N. Rao <redacted>
---
 arch/powerpc/kernel/exceptions-64s.S | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index ae418b85c17c..17ee701b8336 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1411,10 +1411,8 @@ USE_TEXT_SECTION()
 	.balign	IFETCH_ALIGN_BYTES
 do_hash_page:
 #ifdef CONFIG_PPC_STD_MMU_64
-	andis.	r0,r4,0xa410		/* weird error? */
+	andis.	r0,r4,0xa450		/* weird error? */
Can we convert that to a #define value. Ram did try to do that here.

https://lists.ozlabs.org/pipermail/linuxppc-dev/2017-June/158607.html
Hmm... I feel it will be good to do that as part of Ram's series since
he has already coded it up :)

Ram's patches will anyway require a rebase and the change I do here for
detecting DAWR already has a #define, so it should be a simple matter of
including DSISR_DABRMATCH in DSISR_PAGE_FAULT_MASK.

But, if you really feel that I should make that change here, please do
let me know and I will re-spin with those changes.
The thing is that change from 0xa410 to 0xa450 is not clear at all.
And it needs proper documentation. IMHO the best way to do that is
switch to #define name for that constant.
Naveen,
	
Feel free to take the macro from my patch. I think the magic
number is a little ugly. The earlier it goes the better.

My patch set will probably go through a couple of iterations. So I will
rebase it on top of your changes anyway.

RP

Re: [PATCH] powerpc64/hw_breakpoints: Handle data breakpoints in radix mode

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2017-06-14 09:27:00

Anshuman Khandual [off-list ref] writes:
On 06/14/2017 12:12 AM, Naveen N. Rao wrote:
quoted
On P9, trying to use data breakpoints throws the splat shown below (*).
This is because the check for a data breakpoint in DSISR is in
do_hash_page(). Move this check to handle_page_fault() so as to catch
data breakpoints in both hash and radix MMU modes.
Why cant we check for DSISR inside do_hash_page() on P9 ?
We can.

But we also need to check inside handle_page_fault(), because when we're
in Radix mode we don't go via do_hash_page().

So rather than doing it in two places, the patch changes the logic so we
check in handle_page_fault(), and teaches the do_hash_page() code to go
there if DSISR_DABRMATCH is set.

cheers

Re: [PATCH] powerpc64/hw_breakpoints: Handle data breakpoints in radix mode

From: Naveen N. Rao <hidden>
Date: 2017-06-14 13:41:59

On 2017/06/14 04:41PM, Michael Ellerman wrote:
"Aneesh Kumar K.V" [off-list ref] writes:
quoted
On Wednesday 14 June 2017 10:41 AM, Naveen N. Rao wrote:
quoted
On 2017/06/14 08:38AM, Aneesh Kumar K.V wrote:
quoted
"Naveen N. Rao" [off-list ref] writes:
quoted
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index ae418b85c17c..17ee701b8336 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1411,10 +1411,8 @@ USE_TEXT_SECTION()
  	.balign	IFETCH_ALIGN_BYTES
  do_hash_page:
  #ifdef CONFIG_PPC_STD_MMU_64
-	andis.	r0,r4,0xa410		/* weird error? */
+	andis.	r0,r4,0xa450		/* weird error? */
Can we convert that to a #define value. Ram did try to do that here.

https://lists.ozlabs.org/pipermail/linuxppc-dev/2017-June/158607.html
Hmm... I feel it will be good to do that as part of Ram's series since
he has already coded it up :)

Ram's patches will anyway require a rebase and the change I do here for
detecting DAWR already has a #define, so it should be a simple matter of
including DSISR_DABRMATCH in DSISR_PAGE_FAULT_MASK.

But, if you really feel that I should make that change here, please do
let me know and I will re-spin with those changes.
The thing is that change from 0xa410 to 0xa450 is not clear at all. And 
it needs proper documentation. IMHO the best way to do that is switch to 
#define name for that constant.
Not in this patch. It needs to be backported, so it should be as minimal
as possible.
Ok.
The change from 0xa410 to 0xa450 does need a mention in the changelog,
I'll add that.
Thanks, Michael!
(emails just started flowing again...)

- Naveen

Re: [PATCH] powerpc64/hw_breakpoints: Handle data breakpoints in radix mode

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2017-06-16 05:16:16

"Naveen N. Rao" [off-list ref] writes:
quoted hunk
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index ae418b85c17c..17ee701b8336 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1442,7 +1440,9 @@ do_hash_page:
 
 /* Here we have a page fault that hash_page can't handle. */
 handle_page_fault:
-11:	ld	r4,_DAR(r1)
+	andis.  r0,r4,DSISR_DABRMATCH@h
+	bne-    handle_dabr_fault
This broke hash. Please test hash! :)

I added:
@@ -1438,11 +1436,16 @@ do_hash_page:
 
        /* Error */
        blt-    13f
+
+       /* Reload DSISR into r4 for the DABR check below */
+       ld      r4,_DSISR(r1)
 #endif /* CONFIG_PPC_STD_MMU_64 */
 
 /* Here we have a page fault that hash_page can't handle. */
 handle_page_fault:

cheers

Re: [PATCH] powerpc64/hw_breakpoints: Handle data breakpoints in radix mode

From: Naveen N. Rao <hidden>
Date: 2017-06-18 09:42:16

On 2017/06/16 03:16PM, Michael Ellerman wrote:
"Naveen N. Rao" [off-list ref] writes:
quoted
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index ae418b85c17c..17ee701b8336 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1442,7 +1440,9 @@ do_hash_page:
 
 /* Here we have a page fault that hash_page can't handle. */
 handle_page_fault:
-11:	ld	r4,_DAR(r1)
+	andis.  r0,r4,DSISR_DABRMATCH@h
+	bne-    handle_dabr_fault
This broke hash. Please test hash! :)
Gah! :double-face-palm:
I don't know how I missed this... (yes, I do)
quoted hunk
I added:
@@ -1438,11 +1436,16 @@ do_hash_page:

        /* Error */
        blt-    13f
+
+       /* Reload DSISR into r4 for the DABR check below */
+       ld      r4,_DSISR(r1)
 #endif /* CONFIG_PPC_STD_MMU_64 */

 /* Here we have a page fault that hash_page can't handle. */
 handle_page_fault:
As always, thanks Michael!

I think we can optimize this a bit more to eliminate the loads in
handle_page_fault. Here's an incremental patch above your changes for
-next, this time boot-tested with radix and disable_radix.

- Naveen

-----
[PATCH] powerpc64/exceptions64s: Eliminate a few un-necessary memory loads

In do_hash_page(), we re-load DSISR from stack though it is still present
in register r4. Eliminate the memory load by preserving this register.

Furthermore, handler_page_fault() reloads DAR and DSISR from memory and
this is only required if we fall through from do_hash_page().
Otherwise, r3 and r4 already have DAR and DSISR loaded. Re-use those
and have do_hash_page() reload those registers when falling-through.

Signed-off-by: Naveen N. Rao <redacted>
---
 arch/powerpc/kernel/exceptions-64s.S | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index dd619faab862..b5182a1ef3d6 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1426,8 +1426,8 @@ do_hash_page:
 	 *
 	 * at return r3 = 0 for success, 1 for page fault, negative for error
 	 */
+	mr	r6,r4
         mr 	r4,r12
-	ld      r6,_DSISR(r1)
 	bl	__hash_page		/* build HPTE if possible */
         cmpdi	r3,0			/* see if __hash_page succeeded */
 
@@ -1437,7 +1437,8 @@ do_hash_page:
 	/* Error */
 	blt-	13f
 
-	/* Reload DSISR into r4 for the DABR check below */
+	/* Reload DAR/DSISR for handle_page_fault */
+	ld	r3,_DAR(r1)
 	ld      r4,_DSISR(r1)
 #endif /* CONFIG_PPC_STD_MMU_64 */
 
@@ -1445,8 +1446,8 @@ do_hash_page:
 handle_page_fault:
 	andis.  r0,r4,DSISR_DABRMATCH@h
 	bne-    handle_dabr_fault
-	ld	r4,_DAR(r1)
-	ld	r5,_DSISR(r1)
+	mr	r5,r4
+	mr	r4,r3
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	do_page_fault
 	cmpdi	r3,0
-- 
2.13.1

Re: [PATCH] powerpc64/hw_breakpoints: Handle data breakpoints in radix mode

From: Aneesh Kumar K.V <hidden>
Date: 2017-06-19 09:51:57

"Naveen N. Rao" [off-list ref] writes:
quoted hunk
On 2017/06/16 03:16PM, Michael Ellerman wrote:
quoted
"Naveen N. Rao" [off-list ref] writes:
quoted
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index ae418b85c17c..17ee701b8336 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1442,7 +1440,9 @@ do_hash_page:
 
 /* Here we have a page fault that hash_page can't handle. */
 handle_page_fault:
-11:	ld	r4,_DAR(r1)
+	andis.  r0,r4,DSISR_DABRMATCH@h
+	bne-    handle_dabr_fault
This broke hash. Please test hash! :)
Gah! :double-face-palm:
I don't know how I missed this... (yes, I do)
quoted
I added:
@@ -1438,11 +1436,16 @@ do_hash_page:

        /* Error */
        blt-    13f
+
+       /* Reload DSISR into r4 for the DABR check below */
+       ld      r4,_DSISR(r1)
 #endif /* CONFIG_PPC_STD_MMU_64 */

 /* Here we have a page fault that hash_page can't handle. */
 handle_page_fault:
As always, thanks Michael!

I think we can optimize this a bit more to eliminate the loads in
handle_page_fault. Here's an incremental patch above your changes for
-next, this time boot-tested with radix and disable_radix.

- Naveen

-----
[PATCH] powerpc64/exceptions64s: Eliminate a few un-necessary memory loads

In do_hash_page(), we re-load DSISR from stack though it is still present
in register r4. Eliminate the memory load by preserving this register.

Furthermore, handler_page_fault() reloads DAR and DSISR from memory and
this is only required if we fall through from do_hash_page().
Otherwise, r3 and r4 already have DAR and DSISR loaded. Re-use those
and have do_hash_page() reload those registers when falling-through.

Signed-off-by: Naveen N. Rao <redacted>
---
 arch/powerpc/kernel/exceptions-64s.S | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index dd619faab862..b5182a1ef3d6 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1426,8 +1426,8 @@ do_hash_page:
 	 *
 	 * at return r3 = 0 for success, 1 for page fault, negative for error
 	 */
+	mr	r6,r4
         mr 	r4,r12
-	ld      r6,_DSISR(r1)
 	bl	__hash_page		/* build HPTE if possible */
         cmpdi	r3,0			/* see if __hash_page succeeded */
@@ -1437,7 +1437,8 @@ do_hash_page:
 	/* Error */
 	blt-	13f

-	/* Reload DSISR into r4 for the DABR check below */
+	/* Reload DAR/DSISR for handle_page_fault */
+	ld	r3,_DAR(r1)
 	ld      r4,_DSISR(r1)
 #endif /* CONFIG_PPC_STD_MMU_64 */
@@ -1445,8 +1446,8 @@ do_hash_page:
 handle_page_fault:
 	andis.  r0,r4,DSISR_DABRMATCH@h
 	bne-    handle_dabr_fault
-	ld	r4,_DAR(r1)
-	ld	r5,_DSISR(r1)
+	mr	r5,r4
+	mr	r4,r3
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	do_page_fault
 	cmpdi	r3,0

Can we avoid that if we rearrange args of other functions calls, so that
we can use r3 and r4 as it is ?

-anees

Re: [PATCH] powerpc64/hw_breakpoints: Handle data breakpoints in radix mode

From: Naveen N. Rao <hidden>
Date: 2017-06-19 10:30:43

On 2017/06/19 03:21PM, Aneesh Kumar K.V wrote:
"Naveen N. Rao" [off-list ref] writes:
quoted
On 2017/06/16 03:16PM, Michael Ellerman wrote:
quoted
"Naveen N. Rao" [off-list ref] writes:
quoted
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index ae418b85c17c..17ee701b8336 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1442,7 +1440,9 @@ do_hash_page:
 
 /* Here we have a page fault that hash_page can't handle. */
 handle_page_fault:
-11:	ld	r4,_DAR(r1)
+	andis.  r0,r4,DSISR_DABRMATCH@h
+	bne-    handle_dabr_fault
This broke hash. Please test hash! :)
Gah! :double-face-palm:
I don't know how I missed this... (yes, I do)
quoted
I added:
@@ -1438,11 +1436,16 @@ do_hash_page:

        /* Error */
        blt-    13f
+
+       /* Reload DSISR into r4 for the DABR check below */
+       ld      r4,_DSISR(r1)
 #endif /* CONFIG_PPC_STD_MMU_64 */

 /* Here we have a page fault that hash_page can't handle. */
 handle_page_fault:
As always, thanks Michael!

I think we can optimize this a bit more to eliminate the loads in
handle_page_fault. Here's an incremental patch above your changes for
-next, this time boot-tested with radix and disable_radix.

- Naveen

-----
[PATCH] powerpc64/exceptions64s: Eliminate a few un-necessary memory loads

In do_hash_page(), we re-load DSISR from stack though it is still present
in register r4. Eliminate the memory load by preserving this register.

Furthermore, handler_page_fault() reloads DAR and DSISR from memory and
this is only required if we fall through from do_hash_page().
Otherwise, r3 and r4 already have DAR and DSISR loaded. Re-use those
and have do_hash_page() reload those registers when falling-through.

Signed-off-by: Naveen N. Rao <redacted>
---
 arch/powerpc/kernel/exceptions-64s.S | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index dd619faab862..b5182a1ef3d6 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1426,8 +1426,8 @@ do_hash_page:
 	 *
 	 * at return r3 = 0 for success, 1 for page fault, negative for error
 	 */
+	mr	r6,r4
         mr 	r4,r12
-	ld      r6,_DSISR(r1)
 	bl	__hash_page		/* build HPTE if possible */
         cmpdi	r3,0			/* see if __hash_page succeeded */
@@ -1437,7 +1437,8 @@ do_hash_page:
 	/* Error */
 	blt-	13f

-	/* Reload DSISR into r4 for the DABR check below */
+	/* Reload DAR/DSISR for handle_page_fault */
+	ld	r3,_DAR(r1)
 	ld      r4,_DSISR(r1)
 #endif /* CONFIG_PPC_STD_MMU_64 */
@@ -1445,8 +1446,8 @@ do_hash_page:
 handle_page_fault:
 	andis.  r0,r4,DSISR_DABRMATCH@h
 	bne-    handle_dabr_fault
-	ld	r4,_DAR(r1)
-	ld	r5,_DSISR(r1)
+	mr	r5,r4
+	mr	r4,r3
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	do_page_fault
 	cmpdi	r3,0

Can we avoid that if we rearrange args of other functions calls, so that
we can use r3 and r4 as it is ?
I looked at changing do_page_fault(), but it is called from other places 
(booke, entry_32, ..) so, re-arranging the arguments will need more 
intrusive changes there potentially slowing those down.

However, I do think we can change the exception vectors to load things 
up differently for do_page_fault() and handle_page_fault(). I will 
check.

Thanks for the review,
- Naveen

Re: powerpc64/hw_breakpoints: Handle data breakpoints in radix mode

From: Michael Ellerman <hidden>
Date: 2017-06-19 12:22:41

On Tue, 2017-06-13 at 18:42:00 UTC, "Naveen N. Rao" wrote:
On P9, trying to use data breakpoints throws the splat shown below (*).
This is because the check for a data breakpoint in DSISR is in
do_hash_page(). Move this check to handle_page_fault() so as to catch
data breakpoints in both hash and radix MMU modes.

While at it, also remove the label '11' that was made redundant by
commit a546498f3bf9aa ("powerpc: Call do_page_fault() with interrupts
off")

(*)
    Unable to handle kernel paging request for data at address 0xc000000000e19218
    Faulting instruction address: 0xc0000000001155e8
    cpu 0x0: Vector: 300 (Data Access) at [c0000000ef1e7b20]
    pc: c0000000001155e8: find_pid_ns+0x48/0xe0
    lr: c000000000116ac4: find_task_by_vpid+0x44/0x90
    sp: c0000000ef1e7da0
    msr: 9000000000009033
    dar: c000000000e19218
    dsisr: 400000
    current = 0xc0000000f1f59700
    paca    = 0xc00000000fd40000 softe: 0 irq_happened: 0x01
    pid   = 1192, comm = sh
    Linux version 4.12.0-rc3-nnr (root@ea605ec2993c) (gcc version 5.4.0 20160609 (Ubuntu/IBM 5.4.0-6ubuntu1~16.04.1) ) #74 SMP Tue Jun 13 16:52:49 UTC 2017
    enter ? for help
    [c0000000ef1e7dc0] c000000000116ac4 find_task_by_vpid+0x44/0x90
    [c0000000ef1e7de0] c000000000108800 SyS_setpgid+0x80/0x220
    [c0000000ef1e7e30] c00000000000ba6c system_call+0x38/0xfc
    --- Exception: c01 (System Call) at 00007fff94480890
    SP (7fffd91e7260) is in userspace

Fixes: caca285e5ab4a ("powerpc/mm/radix: Use STD_MMU_64 to properly
isolate hash related code")
Reported-by: Shriya R. Kulkarni <redacted>
Signed-off-by: Naveen N. Rao <redacted>
Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/d89ba5353f301971dd7d2f9fdf25c4

cheers

[PATCH v2] powerpc64/exceptions: Refactor code to eliminate a few memory loads

From: Naveen N. Rao <hidden>
Date: 2017-06-19 18:46:04

On 2017/06/19 03:21PM, Aneesh Kumar K.V wrote:
quoted
@@ -1445,8 +1446,8 @@ do_hash_page:
 handle_page_fault:
 	andis.  r0,r4,DSISR_DABRMATCH@h
 	bne-    handle_dabr_fault
-	ld	r4,_DAR(r1)
-	ld	r5,_DSISR(r1)
+	mr	r5,r4
+	mr	r4,r3
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	do_page_fault
 	cmpdi	r3,0

Can we avoid that if we rearrange args of other functions calls, so that
we can use r3 and r4 as it is ?
Here's a version that does that. Again, boot tested with radix and 
disable_radix.

Thanks,
Naveen

-
Change data_access_common() and instruction_access_common() to load the
trap number in r3, DAR in r4 and DSISR in r5 (rather than in r5, r3 and
r4 respectively). This change allows us to eliminate a few un-necessary
memory loads and register move operations in handle_page_fault(),
handle_dabr_fault() and label '77'.

Signed-off-by: Naveen N. Rao <redacted>
---
 arch/powerpc/kernel/exceptions-64s.S | 38 +++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index b6fad9790784..4c5abe1d6f44 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -494,11 +494,11 @@ EXC_COMMON_BEGIN(data_access_common)
 	EXCEPTION_PROLOG_COMMON(0x300, PACA_EXGEN)
 	RECONCILE_IRQ_STATE(r10, r11)
 	ld	r12,_MSR(r1)
-	ld	r3,PACA_EXGEN+EX_DAR(r13)
-	lwz	r4,PACA_EXGEN+EX_DSISR(r13)
-	li	r5,0x300
-	std	r3,_DAR(r1)
-	std	r4,_DSISR(r1)
+	ld	r4,PACA_EXGEN+EX_DAR(r13)
+	lwz	r5,PACA_EXGEN+EX_DSISR(r13)
+	li	r3,0x300
+	std	r4,_DAR(r1)
+	std	r5,_DSISR(r1)
 BEGIN_MMU_FTR_SECTION
 	b	do_hash_page		/* Try to handle as hpte fault */
 MMU_FTR_SECTION_ELSE
@@ -562,11 +562,11 @@ EXC_COMMON_BEGIN(instruction_access_common)
 	EXCEPTION_PROLOG_COMMON(0x400, PACA_EXGEN)
 	RECONCILE_IRQ_STATE(r10, r11)
 	ld	r12,_MSR(r1)
-	ld	r3,_NIP(r1)
-	andis.	r4,r12,0x5820
-	li	r5,0x400
-	std	r3,_DAR(r1)
-	std	r4,_DSISR(r1)
+	ld	r4,_NIP(r1)
+	andis.	r5,r12,0x5820
+	li	r3,0x400
+	std	r4,_DAR(r1)
+	std	r5,_DSISR(r1)
 BEGIN_MMU_FTR_SECTION
 	b	do_hash_page		/* Try to handle as hpte fault */
 MMU_FTR_SECTION_ELSE
@@ -1474,7 +1474,7 @@ USE_TEXT_SECTION()
 	.balign	IFETCH_ALIGN_BYTES
 do_hash_page:
 #ifdef CONFIG_PPC_STD_MMU_64
-	andis.	r0,r4,0xa450		/* weird error? */
+	andis.	r0,r5,0xa450		/* weird error? */
 	bne-	handle_page_fault	/* if not, try to insert a HPTE */
 	CURRENT_THREAD_INFO(r11, r1)
 	lwz	r0,TI_PREEMPT(r11)	/* If we're in an "NMI" */
@@ -1489,8 +1489,10 @@ do_hash_page:
 	 *
 	 * at return r3 = 0 for success, 1 for page fault, negative for error
 	 */
+	mr	r6,r5
+	mr	r5,r3
+	mr	r3,r4
         mr 	r4,r12
-	ld      r6,_DSISR(r1)
 	bl	__hash_page		/* build HPTE if possible */
         cmpdi	r3,0			/* see if __hash_page succeeded */
 
@@ -1500,16 +1502,15 @@ do_hash_page:
 	/* Error */
 	blt-	13f
 
-	/* Reload DSISR into r4 for the DABR check below */
-	ld      r4,_DSISR(r1)
+	/* Reload DAR/DSISR for handle_page_fault */
+	ld	r4,_DAR(r1)
+	ld      r5,_DSISR(r1)
 #endif /* CONFIG_PPC_STD_MMU_64 */
 
 /* Here we have a page fault that hash_page can't handle. */
 handle_page_fault:
-	andis.  r0,r4,DSISR_DABRMATCH@h
+	andis.  r0,r5,DSISR_DABRMATCH@h
 	bne-    handle_dabr_fault
-	ld	r4,_DAR(r1)
-	ld	r5,_DSISR(r1)
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	do_page_fault
 	cmpdi	r3,0
@@ -1524,8 +1525,6 @@ handle_page_fault:
 /* We have a data breakpoint exception - handle it */
 handle_dabr_fault:
 	bl	save_nvgprs
-	ld      r4,_DAR(r1)
-	ld      r5,_DSISR(r1)
 	addi    r3,r1,STACK_FRAME_OVERHEAD
 	bl      do_break
 12:	b       ret_from_except_lite
@@ -1551,7 +1550,6 @@ handle_dabr_fault:
  * the access, or panic if there isn't a handler.
  */
 77:	bl	save_nvgprs
-	mr	r4,r3
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	li	r5,SIGSEGV
 	bl	bad_page_fault
-- 
2.13.1

Re: [PATCH v2] powerpc64/exceptions: Refactor code to eliminate a few memory loads

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2017-11-10 04:34:40

"Naveen N. Rao" [off-list ref] writes:
On 2017/06/19 03:21PM, Aneesh Kumar K.V wrote:
quoted
quoted
@@ -1445,8 +1446,8 @@ do_hash_page:
 handle_page_fault:
 	andis.  r0,r4,DSISR_DABRMATCH@h
 	bne-    handle_dabr_fault
-	ld	r4,_DAR(r1)
-	ld	r5,_DSISR(r1)
+	mr	r5,r4
+	mr	r4,r3
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	do_page_fault
 	cmpdi	r3,0

Can we avoid that if we rearrange args of other functions calls, so that
we can use r3 and r4 as it is ?
Here's a version that does that. Again, boot tested with radix and 
disable_radix.

Thanks,
Naveen

-
Change data_access_common() and instruction_access_common() to load the
trap number in r3, DAR in r4 and DSISR in r5 (rather than in r5, r3 and
r4 respectively). This change allows us to eliminate a few un-necessary
memory loads and register move operations in handle_page_fault(),
handle_dabr_fault() and label '77'.

Signed-off-by: Naveen N. Rao <redacted>
---
 arch/powerpc/kernel/exceptions-64s.S | 38 +++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 20 deletions(-)
Sorry I missed this and now it doesn't apply. Do you mind rebasing.

cheers

Re: [PATCH v2] powerpc64/exceptions: Refactor code to eliminate a few memory loads

From: Naveen N. Rao <hidden>
Date: 2017-11-13 17:04:58

Michael Ellerman wrote:
"Naveen N. Rao" [off-list ref] writes:
quoted
On 2017/06/19 03:21PM, Aneesh Kumar K.V wrote:
quoted
quoted
@@ -1445,8 +1446,8 @@ do_hash_page:
 handle_page_fault:
 	andis.  r0,r4,DSISR_DABRMATCH@h
 	bne-    handle_dabr_fault
-	ld	r4,_DAR(r1)
-	ld	r5,_DSISR(r1)
+	mr	r5,r4
+	mr	r4,r3
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	do_page_fault
 	cmpdi	r3,0

Can we avoid that if we rearrange args of other functions calls, so that
we can use r3 and r4 as it is ?
Here's a version that does that. Again, boot tested with radix and 
disable_radix.

Thanks,
Naveen

-
Change data_access_common() and instruction_access_common() to load the
trap number in r3, DAR in r4 and DSISR in r5 (rather than in r5, r3 and
r4 respectively). This change allows us to eliminate a few un-necessary
memory loads and register move operations in handle_page_fault(),
handle_dabr_fault() and label '77'.

Signed-off-by: Naveen N. Rao <redacted>
---
 arch/powerpc/kernel/exceptions-64s.S | 38 +++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 20 deletions(-)
Sorry I missed this and now it doesn't apply. Do you mind rebasing.
No problem - this is just a small refactoring after all :).
Here's a rebased version.  Boot tested on mambo with/without radix.

Thanks,
Naveen

--
[PATCH v3] powerpc/exceptions64s: Eliminate a few un-necessary memory  
loads

Change data_access_common() and instruction_access_common() to load the
trap number in r3, DAR in r4 and DSISR in r5 (rather than in r5, r3 and
r4 respectively). This change allows us to eliminate a few un-necessary
memory loads and register move operations in handle_page_fault(),
handle_dabr_fault() and label '77'.

Signed-off-by: Naveen N. Rao <redacted>
---
 arch/powerpc/kernel/exceptions-64s.S | 38 +++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 72cffa0c4af6..b8166b4fa728 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -499,11 +499,11 @@ EXC_COMMON_BEGIN(data_access_common)
 	EXCEPTION_PROLOG_COMMON(0x300, PACA_EXGEN)
 	RECONCILE_IRQ_STATE(r10, r11)
 	ld	r12,_MSR(r1)
-	ld	r3,PACA_EXGEN+EX_DAR(r13)
-	lwz	r4,PACA_EXGEN+EX_DSISR(r13)
-	li	r5,0x300
-	std	r3,_DAR(r1)
-	std	r4,_DSISR(r1)
+	ld	r4,PACA_EXGEN+EX_DAR(r13)
+	lwz	r5,PACA_EXGEN+EX_DSISR(r13)
+	li	r3,0x300
+	std	r4,_DAR(r1)
+	std	r5,_DSISR(r1)
 BEGIN_MMU_FTR_SECTION
 	b	do_hash_page		/* Try to handle as hpte fault */
 MMU_FTR_SECTION_ELSE
@@ -543,11 +543,11 @@ EXC_COMMON_BEGIN(instruction_access_common)
 	EXCEPTION_PROLOG_COMMON(0x400, PACA_EXGEN)
 	RECONCILE_IRQ_STATE(r10, r11)
 	ld	r12,_MSR(r1)
-	ld	r3,_NIP(r1)
-	andis.	r4,r12,DSISR_BAD_FAULT_64S@h
-	li	r5,0x400
-	std	r3,_DAR(r1)
-	std	r4,_DSISR(r1)
+	ld	r4,_NIP(r1)
+	andis.	r5,r12,DSISR_BAD_FAULT_64S@h
+	li	r3,0x400
+	std	r4,_DAR(r1)
+	std	r5,_DSISR(r1)
 BEGIN_MMU_FTR_SECTION
 	b	do_hash_page		/* Try to handle as hpte fault */
 MMU_FTR_SECTION_ELSE
@@ -1523,7 +1523,7 @@ do_hash_page:
 #ifdef CONFIG_PPC_BOOK3S_64
 	lis	r0,DSISR_BAD_FAULT_64S@h
 	ori	r0,r0,DSISR_BAD_FAULT_64S@l
-	and.	r0,r4,r0		/* weird error? */
+	and.	r0,r5,r0		/* weird error? */
 	bne-	handle_page_fault	/* if not, try to insert a HPTE */
 	CURRENT_THREAD_INFO(r11, r1)
 	lwz	r0,TI_PREEMPT(r11)	/* If we're in an "NMI" */
@@ -1538,8 +1538,10 @@ do_hash_page:
 	 *
 	 * at return r3 = 0 for success, 1 for page fault, negative for error
 	 */
+	mr	r6,r5
+	mr	r5,r3
+	mr	r3,r4
         mr 	r4,r12
-	ld      r6,_DSISR(r1)
 	bl	__hash_page		/* build HPTE if possible */
         cmpdi	r3,0			/* see if __hash_page succeeded */
 
@@ -1549,16 +1551,15 @@ do_hash_page:
 	/* Error */
 	blt-	13f
 
-	/* Reload DSISR into r4 for the DABR check below */
-	ld      r4,_DSISR(r1)
+	/* Reload DAR/DSISR for handle_page_fault */
+	ld	r4,_DAR(r1)
+	ld      r5,_DSISR(r1)
 #endif /* CONFIG_PPC_BOOK3S_64 */
 
 /* Here we have a page fault that hash_page can't handle. */
 handle_page_fault:
-11:	andis.  r0,r4,DSISR_DABRMATCH@h
+	andis.  r0,r5,DSISR_DABRMATCH@h
 	bne-    handle_dabr_fault
-	ld	r4,_DAR(r1)
-	ld	r5,_DSISR(r1)
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	do_page_fault
 	cmpdi	r3,0
@@ -1573,8 +1574,6 @@ handle_page_fault:
 /* We have a data breakpoint exception - handle it */
 handle_dabr_fault:
 	bl	save_nvgprs
-	ld      r4,_DAR(r1)
-	ld      r5,_DSISR(r1)
 	addi    r3,r1,STACK_FRAME_OVERHEAD
 	bl      do_break
 12:	b       ret_from_except_lite
@@ -1600,7 +1599,6 @@ handle_dabr_fault:
  * the access, or panic if there isn't a handler.
  */
 77:	bl	save_nvgprs
-	mr	r4,r3
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	li	r5,SIGSEGV
 	bl	bad_page_fault
-- 
2.15.0

Re: [PATCH v2] powerpc64/exceptions: Refactor code to eliminate a few memory loads

From: Christophe Leroy <hidden>
Date: 2021-11-25 11:07:02


Le 13/11/2017 à 18:04, Naveen N. Rao a écrit :
quoted hunk
Michael Ellerman wrote:
quoted
"Naveen N. Rao" [off-list ref] writes:
quoted
On 2017/06/19 03:21PM, Aneesh Kumar K.V wrote:
quoted
quoted
@@ -1445,8 +1446,8 @@ do_hash_page:
  handle_page_fault:
  	andis.  r0,r4,DSISR_DABRMATCH@h
  	bne-    handle_dabr_fault
-	ld	r4,_DAR(r1)
-	ld	r5,_DSISR(r1)
+	mr	r5,r4
+	mr	r4,r3
  	addi	r3,r1,STACK_FRAME_OVERHEAD
  	bl	do_page_fault
  	cmpdi	r3,0

Can we avoid that if we rearrange args of other functions calls, so that
we can use r3 and r4 as it is ?
Here's a version that does that. Again, boot tested with radix and
disable_radix.

Thanks,
Naveen

-
Change data_access_common() and instruction_access_common() to load the
trap number in r3, DAR in r4 and DSISR in r5 (rather than in r5, r3 and
r4 respectively). This change allows us to eliminate a few un-necessary
memory loads and register move operations in handle_page_fault(),
handle_dabr_fault() and label '77'.

Signed-off-by: Naveen N. Rao <redacted>
---
  arch/powerpc/kernel/exceptions-64s.S | 38 +++++++++++++++++-------------------
  1 file changed, 18 insertions(+), 20 deletions(-)
Sorry I missed this and now it doesn't apply. Do you mind rebasing.
No problem - this is just a small refactoring after all :).
Here's a rebased version.  Boot tested on mambo with/without radix.

Thanks,
Naveen

--
[PATCH v3] powerpc/exceptions64s: Eliminate a few un-necessary memory
loads

Change data_access_common() and instruction_access_common() to load the
trap number in r3, DAR in r4 and DSISR in r5 (rather than in r5, r3 and
r4 respectively). This change allows us to eliminate a few un-necessary
memory loads and register move operations in handle_page_fault(),
handle_dabr_fault() and label '77'.

Signed-off-by: Naveen N. Rao <redacted>
---
  arch/powerpc/kernel/exceptions-64s.S | 38 +++++++++++++++++-------------------
  1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 72cffa0c4af6..b8166b4fa728 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -499,11 +499,11 @@ EXC_COMMON_BEGIN(data_access_common)
  	EXCEPTION_PROLOG_COMMON(0x300, PACA_EXGEN)
  	RECONCILE_IRQ_STATE(r10, r11)
  	ld	r12,_MSR(r1)
-	ld	r3,PACA_EXGEN+EX_DAR(r13)
-	lwz	r4,PACA_EXGEN+EX_DSISR(r13)
-	li	r5,0x300
-	std	r3,_DAR(r1)
-	std	r4,_DSISR(r1)
+	ld	r4,PACA_EXGEN+EX_DAR(r13)
+	lwz	r5,PACA_EXGEN+EX_DSISR(r13)
+	li	r3,0x300
+	std	r4,_DAR(r1)
+	std	r5,_DSISR(r1)
  BEGIN_MMU_FTR_SECTION
  	b	do_hash_page		/* Try to handle as hpte fault */
  MMU_FTR_SECTION_ELSE
@@ -543,11 +543,11 @@ EXC_COMMON_BEGIN(instruction_access_common)
  	EXCEPTION_PROLOG_COMMON(0x400, PACA_EXGEN)
  	RECONCILE_IRQ_STATE(r10, r11)
  	ld	r12,_MSR(r1)
-	ld	r3,_NIP(r1)
-	andis.	r4,r12,DSISR_BAD_FAULT_64S@h
-	li	r5,0x400
-	std	r3,_DAR(r1)
-	std	r4,_DSISR(r1)
+	ld	r4,_NIP(r1)
+	andis.	r5,r12,DSISR_BAD_FAULT_64S@h
+	li	r3,0x400
+	std	r4,_DAR(r1)
+	std	r5,_DSISR(r1)
  BEGIN_MMU_FTR_SECTION
  	b	do_hash_page		/* Try to handle as hpte fault */
  MMU_FTR_SECTION_ELSE
@@ -1523,7 +1523,7 @@ do_hash_page:
  #ifdef CONFIG_PPC_BOOK3S_64
  	lis	r0,DSISR_BAD_FAULT_64S@h
  	ori	r0,r0,DSISR_BAD_FAULT_64S@l
-	and.	r0,r4,r0		/* weird error? */
+	and.	r0,r5,r0		/* weird error? */
  	bne-	handle_page_fault	/* if not, try to insert a HPTE */
  	CURRENT_THREAD_INFO(r11, r1)
  	lwz	r0,TI_PREEMPT(r11)	/* If we're in an "NMI" */
@@ -1538,8 +1538,10 @@ do_hash_page:
  	 *
  	 * at return r3 = 0 for success, 1 for page fault, negative for error
  	 */
+	mr	r6,r5
+	mr	r5,r3
+	mr	r3,r4
          mr 	r4,r12
-	ld      r6,_DSISR(r1)
  	bl	__hash_page		/* build HPTE if possible */
          cmpdi	r3,0			/* see if __hash_page succeeded */
  
@@ -1549,16 +1551,15 @@ do_hash_page:
  	/* Error */
  	blt-	13f
  
-	/* Reload DSISR into r4 for the DABR check below */
-	ld      r4,_DSISR(r1)
+	/* Reload DAR/DSISR for handle_page_fault */
+	ld	r4,_DAR(r1)
+	ld      r5,_DSISR(r1)
  #endif /* CONFIG_PPC_BOOK3S_64 */
  
  /* Here we have a page fault that hash_page can't handle. */
  handle_page_fault:
-11:	andis.  r0,r4,DSISR_DABRMATCH@h
+	andis.  r0,r5,DSISR_DABRMATCH@h
  	bne-    handle_dabr_fault
-	ld	r4,_DAR(r1)
-	ld	r5,_DSISR(r1)
  	addi	r3,r1,STACK_FRAME_OVERHEAD
  	bl	do_page_fault
  	cmpdi	r3,0
@@ -1573,8 +1574,6 @@ handle_page_fault:
  /* We have a data breakpoint exception - handle it */
  handle_dabr_fault:
  	bl	save_nvgprs
-	ld      r4,_DAR(r1)
-	ld      r5,_DSISR(r1)
  	addi    r3,r1,STACK_FRAME_OVERHEAD
  	bl      do_break
  12:	b       ret_from_except_lite
@@ -1600,7 +1599,6 @@ handle_dabr_fault:
   * the access, or panic if there isn't a handler.
   */
  77:	bl	save_nvgprs
-	mr	r4,r3
  	addi	r3,r1,STACK_FRAME_OVERHEAD
  	li	r5,SIGSEGV
  	bl	bad_page_fault
Seems like it was never merged but was superseded in 2019 by 
https://lore.kernel.org/r/20190802105709.27696-37-npiggin@gmail.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help