* [PATCH 0/2 v2] powerpc: Make ptrace work reliably
@ 2013-05-16 5:34 Bharat Bhushan
2013-05-16 5:34 ` [PATCH 1/2 v2] powerpc: debug control and status registers are 32bit Bharat Bhushan
0 siblings, 1 reply; 5+ messages in thread
From: Bharat Bhushan @ 2013-05-16 5:34 UTC (permalink / raw)
To: galak, benh, linuxppc-dev, scottwood, stuart.yoder, James.Yang
Cc: Bharat Bhushan
From: Bharat Bhushan <bharat.bhushan@freescale.com>
v1->v2
- Subject line was missing 0/2, 1/2, 2/2
Bharat Bhushan (2):
powerpc: debug control and status registers are 32bit
=> This patch makes debug control and status registers as 32bit as they are.
This does not fix anything
powerpc: restore dbcr0 on user space exit
=> This patch fixes the ptrace reliability issue. The description is the patch
describes one of the case where it does not work reliably
arch/powerpc/include/asm/processor.h | 8 ++++----
arch/powerpc/kernel/asm-offsets.c | 1 +
arch/powerpc/kernel/entry_64.S | 24 ++++++++++++++++++++----
3 files changed, 25 insertions(+), 8 deletions(-)
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2 v2] powerpc: debug control and status registers are 32bit
2013-05-16 5:34 [PATCH 0/2 v2] powerpc: Make ptrace work reliably Bharat Bhushan
@ 2013-05-16 5:34 ` Bharat Bhushan
2013-05-16 5:34 ` [PATCH 2/2 v2] powerpc: restore dbcr0 on user space exit Bharat Bhushan
0 siblings, 1 reply; 5+ messages in thread
From: Bharat Bhushan @ 2013-05-16 5:34 UTC (permalink / raw)
To: galak, benh, linuxppc-dev, scottwood, stuart.yoder, James.Yang
Cc: Bharat Bhushan
Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
---
v1->v2
- Subject line was not having 1/2
arch/powerpc/include/asm/processor.h | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index d7e67ca..5213577 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -168,10 +168,10 @@ struct thread_struct {
* The following help to manage the use of Debug Control Registers
* om the BookE platforms.
*/
- unsigned long dbcr0;
- unsigned long dbcr1;
+ uint32_t dbcr0;
+ uint32_t dbcr1;
#ifdef CONFIG_BOOKE
- unsigned long dbcr2;
+ uint32_t dbcr2;
#endif
/*
* The stored value of the DBSR register will be the value at the
@@ -179,7 +179,7 @@ struct thread_struct {
* user (will never be written to) and has value while helping to
* describe the reason for the last debug trap. Torez
*/
- unsigned long dbsr;
+ uint32_t dbsr;
/*
* The following will contain addresses used by debug applications
* to help trace and trap on particular address locations.
--
1.5.6.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2 v2] powerpc: restore dbcr0 on user space exit
2013-05-16 5:34 ` [PATCH 1/2 v2] powerpc: debug control and status registers are 32bit Bharat Bhushan
@ 2013-05-16 5:34 ` Bharat Bhushan
2013-05-16 16:54 ` Scott Wood
0 siblings, 1 reply; 5+ messages in thread
From: Bharat Bhushan @ 2013-05-16 5:34 UTC (permalink / raw)
To: galak, benh, linuxppc-dev, scottwood, stuart.yoder, James.Yang
Cc: Bharat Bhushan
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.bhushan@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)
#else
ld r10,PACAKMSR(r13) /* Get kernel MSR without EE */
mtmsrd r10,1 /* Update machine state */
@@ -628,15 +630,29 @@ _GLOBAL(ret_from_except_lite)
/* Check current_thread_info()->flags */
andi. r0,r4,_TIF_USER_WORK_MASK
+ bne 1f
+#ifdef CONFIG_PPC_BOOK3E
+ /*
+ * Check to see if the dbcr0 register is set up to debug.
+ * Use the internal debug mode bit to do this.
+ */
+ andis. r0,r10,DBCR0_IDM@h
beq restore
-
- andi. r0,r4,_TIF_NEED_RESCHED
- beq 1f
+ mfmsr r0
+ rlwinm r0,r0,0,~MSR_DE /* Clear MSR.DE */
+ mtmsr r0
+ mtspr SPRN_DBCR0,r10
+ li r10, -1
+ mtspr SPRN_DBSR,r10
+ b restore
+#endif
+1: andi. r0,r4,_TIF_NEED_RESCHED
+ beq 2f
bl .restore_interrupts
SCHEDULE_USER
b .ret_from_except_lite
-1: bl .save_nvgprs
+2: bl .save_nvgprs
bl .restore_interrupts
addi r3,r1,STACK_FRAME_OVERHEAD
bl .do_notify_resume
--
1.5.6.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2 v2] powerpc: restore dbcr0 on user space exit
2013-05-16 5:34 ` [PATCH 2/2 v2] powerpc: restore dbcr0 on user space exit Bharat Bhushan
@ 2013-05-16 16:54 ` Scott Wood
2013-05-16 17:03 ` Bhushan Bharat-R65777
0 siblings, 1 reply; 5+ messages in thread
From: Scott Wood @ 2013-05-16 16:54 UTC (permalink / raw)
To: Bharat Bhushan; +Cc: Bharat Bhushan, stuart.yoder, James.Yang, linuxppc-dev
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.
>=20
> 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.
>=20
> This fixes using ptrace reliably on BookE-PowerPC
>=20
> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> ---
> v1->v2
> - Subject line was not having 2/2
>=20
> arch/powerpc/kernel/asm-offsets.c | 1 +
> arch/powerpc/kernel/entry_64.S | 24 ++++++++++++++++++++----
> 2 files changed, 21 insertions(+), 4 deletions(-)
>=20
> diff --git a/arch/powerpc/kernel/asm-offsets.c =20
> 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 =20
> 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 =20
early... wrteei has synchronization, so it will probably have to wait =20
until the ld completes, defeating the purpose of moving it earlier.
Ideal would probably be to load PACACURRENT immediately after _MSR(r1), =20
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 =20
(e.g. from lmbench) before and after the patch?
-Scott=
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH 2/2 v2] powerpc: restore dbcr0 on user space exit
2013-05-16 16:54 ` Scott Wood
@ 2013-05-16 17:03 ` Bhushan Bharat-R65777
0 siblings, 0 replies; 5+ messages in thread
From: Bhushan Bharat-R65777 @ 2013-05-16 17:03 UTC (permalink / raw)
To: Wood Scott-B07421; +Cc: Yang James-RA8135, linuxppc-dev, Yoder Stuart-B08248
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Thursday, May 16, 2013 10:24 PM
> To: Bhushan Bharat-R65777
> Cc: galak@kernel.crashing.org; benh@kernel.crashing.org; linuxppc-
> dev@lists.ozlabs.org; Yoder Stuart-B08248; Yang James-RA8135; Bhushan Bha=
rat-
> R65777
> Subject: Re: [PATCH 2/2 v2] powerpc: restore dbcr0 on user space exit
>=20
> 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.bhushan@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)
>=20
> 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.
>=20
> Ideal would probably be to load PACACURRENT immediately after _MSR(r1), a=
nd load
> DBCR0 just after "beq resume_kernel".
ok
>=20
> 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.
>=20
> Regardless of what you do, could you run a basic syscall benchmark (e.g. =
from
> lmbench) before and after the patch?
Sure.
-Bharat
>=20
> -Scott
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-05-16 17:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-16 5:34 [PATCH 0/2 v2] powerpc: Make ptrace work reliably Bharat Bhushan
2013-05-16 5:34 ` [PATCH 1/2 v2] powerpc: debug control and status registers are 32bit Bharat Bhushan
2013-05-16 5:34 ` [PATCH 2/2 v2] powerpc: restore dbcr0 on user space exit Bharat Bhushan
2013-05-16 16:54 ` Scott Wood
2013-05-16 17:03 ` Bhushan Bharat-R65777
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).