Re: [PATCH 2/2 v2] powerpc: restore dbcr0 on user space exit

2013-05-16 Thread Scott Wood 

On 05/16/2013 12:34:32 AM, Bharat Bhushan wrote:

On BookE (Branch taken + Single Step) is as same as Branch Taken
on BookS and in Linux we simulate BookS behavior for BookE as well.
When doing so, in Branch taken handling we want to set DBCR0_IC but
we update the current-thread-dbcr0 and not DBCR0.

Now on 64bit the current-thread.dbcr0 (and other debug registers)
is synchronized ONLY on context switch flow. But after handling
Branch taken in debug exception if we return back to user space
without context switch then single stepping change (DBCR0_ICMP)
does not get written in h/w DBCR0 and Instruction Complete exception
does not happen.

This fixes using ptrace reliably on BookE-PowerPC

Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
v1-v2
 - Subject line was not having 2/2

 arch/powerpc/kernel/asm-offsets.c |1 +
 arch/powerpc/kernel/entry_64.S|   24 
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/asm-offsets.c  
b/arch/powerpc/kernel/asm-offsets.c

index b51a97c..1e2f450 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -103,6 +103,7 @@ int main(void)
 #endif /* CONFIG_VSX */
 #ifdef CONFIG_PPC64
DEFINE(KSP_VSID, offsetof(struct thread_struct, ksp_vsid));
+   DEFINE(THREAD_DBCR0, offsetof(struct thread_struct, dbcr0));
 #else /* CONFIG_PPC64 */
DEFINE(PGDIR, offsetof(struct thread_struct, pgdir));
 #if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
diff --git a/arch/powerpc/kernel/entry_64.S  
b/arch/powerpc/kernel/entry_64.S

index 794889b..561630d 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -614,7 +614,9 @@ _GLOBAL(ret_from_except_lite)
 * from the interrupt.
 */
 #ifdef CONFIG_PPC_BOOK3E
+   ld  r3,PACACURRENT(r13)
wrteei  0
+   lwz r10,(THREAD+THREAD_DBCR0)(r3)


I know I asked you to move these earlier, but this is probably too  
early... wrteei has synchronization, so it will probably have to wait  
until the ld completes, defeating the purpose of moving it earlier.


Ideal would probably be to load PACACURRENT immediately after _MSR(r1),  
and load DBCR0 just after beq resume_kernel.


Or, move DBCR0 to therad_info as I suggested internally.

Regardless of what you do, could you run a basic syscall benchmark  
(e.g. from lmbench) before and after the patch?


-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [PATCH 2/2 v2] powerpc: restore dbcr0 on user space exit

2013-05-16 Thread Bhushan Bharat-R65777 


 -Original Message-
 From: Wood Scott-B07421
 Sent: Thursday, May 16, 2013 10:24 PM
 To: Bhushan Bharat-R65777
 Cc: ga...@kernel.crashing.org; b...@kernel.crashing.org; linuxppc-
 d...@lists.ozlabs.org; Yoder Stuart-B08248; Yang James-RA8135; Bhushan Bharat-
 R65777
 Subject: Re: [PATCH 2/2 v2] powerpc: restore dbcr0 on user space exit
 
 On 05/16/2013 12:34:32 AM, Bharat Bhushan wrote:
  On BookE (Branch taken + Single Step) is as same as Branch Taken on
  BookS and in Linux we simulate BookS behavior for BookE as well.
  When doing so, in Branch taken handling we want to set DBCR0_IC but we
  update the current-thread-dbcr0 and not DBCR0.
 
  Now on 64bit the current-thread.dbcr0 (and other debug registers) is
  synchronized ONLY on context switch flow. But after handling Branch
  taken in debug exception if we return back to user space without
  context switch then single stepping change (DBCR0_ICMP) does not get
  written in h/w DBCR0 and Instruction Complete exception does not
  happen.
 
  This fixes using ptrace reliably on BookE-PowerPC
 
  Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
  ---
  v1-v2
   - Subject line was not having 2/2
 
   arch/powerpc/kernel/asm-offsets.c |1 +
   arch/powerpc/kernel/entry_64.S|   24 
   2 files changed, 21 insertions(+), 4 deletions(-)
 
  diff --git a/arch/powerpc/kernel/asm-offsets.c
  b/arch/powerpc/kernel/asm-offsets.c
  index b51a97c..1e2f450 100644
  --- a/arch/powerpc/kernel/asm-offsets.c
  +++ b/arch/powerpc/kernel/asm-offsets.c
  @@ -103,6 +103,7 @@ int main(void)
   #endif /* CONFIG_VSX */
   #ifdef CONFIG_PPC64
  DEFINE(KSP_VSID, offsetof(struct thread_struct, ksp_vsid));
  +   DEFINE(THREAD_DBCR0, offsetof(struct thread_struct, dbcr0));
   #else /* CONFIG_PPC64 */
  DEFINE(PGDIR, offsetof(struct thread_struct, pgdir));  #if
  defined(CONFIG_4xx) || defined(CONFIG_BOOKE) diff --git
  a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
  index 794889b..561630d 100644
  --- a/arch/powerpc/kernel/entry_64.S
  +++ b/arch/powerpc/kernel/entry_64.S
  @@ -614,7 +614,9 @@ _GLOBAL(ret_from_except_lite)
   * from the interrupt.
   */
   #ifdef CONFIG_PPC_BOOK3E
  +   ld  r3,PACACURRENT(r13)
  wrteei  0
  +   lwz r10,(THREAD+THREAD_DBCR0)(r3)
 
 I know I asked you to move these earlier, but this is probably too early...
 wrteei has synchronization, so it will probably have to wait until the ld
 completes, defeating the purpose of moving it earlier.
 
 Ideal would probably be to load PACACURRENT immediately after _MSR(r1), and 
 load
 DBCR0 just after beq resume_kernel.

ok

 
 Or, move DBCR0 to therad_info as I suggested internally.

If no one have objection on moving dbcr0 to thread_info then I am happy to do 
that.

 
 Regardless of what you do, could you run a basic syscall benchmark (e.g. from
 lmbench) before and after the patch?

Sure.

-Bharat
 
 -Scott

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev