Skip to contentSkip to conversation

Implement GET_IP/SET_IP for powerpc architecture.

PATCH In review Srikar Dronamraju 3 participants 7 messages Feb 2, 2012 – Feb 9, 2012

Subsystems (2)
PowerPC ptrace support
lore
Share & tools
lore

Srikar Dronamraju

author
#0 lore raw

PATCH 1/1 Implement GET_IP/SET_IP for powerpc architecture.

With this change, helpers such as instruction_pointer() et al, get defined in the generic header in terms of GET_IP

Signed-off-by Srikar Dronamraju <[email protected]>
Signed-off-by Ananth N Mavinakayanahalli <[email protected]>
---
 arch/powerpc/include/asm/ptrace.h |   22 ++++++++++++++--------
 1 files changed, 14 insertions(+), 8 deletions(-)
arch/powerpc/include/asm/ptrace.h +14 -8
index 78a2051..889408d 100644
@@ -83,8 +83,20 @@ struct pt_regs {
 
 #ifndef __ASSEMBLY__
 
-#define instruction_pointer(regs) ((regs)->nip)
-#define user_stack_pointer(regs) ((regs)->gpr[1])
+#define GET_IP(regs)		((regs)->nip)
+#define GET_USP(regs)		((regs)->gpr[1])
+#define GET_FP(regs)		(0)
+#define SET_FP(regs, val)
+
+#ifdef CONFIG_SMP
+extern unsigned long profile_pc(struct pt_regs *regs);
+#define profile_pc profile_pc
+#else
+#define profile_pc(regs) GET_IP(regs)
+#endif
+
+#include <asm-generic/ptrace.h>
+
 #define kernel_stack_pointer(regs) ((regs)->gpr[1])
 static inline int is_syscall_success(struct pt_regs *regs)
 {

@@ -99,12 +111,6 @@ static inline long regs_return_value(struct pt_regs *regs)
 		return -regs->gpr[3];
 }
 
-#ifdef CONFIG_SMP
-extern unsigned long profile_pc(struct pt_regs *regs);
-#else
-#define profile_pc(regs) instruction_pointer(regs)
-#endif
-
 #ifdef __powerpc64__
 #define user_mode(regs) ((((regs)->msr) >> MSR_PR_LG) & 0x1)
 #else
#1 lore raw

arch/powerpc/include/asm/ptrace.h struct pt_regs { · line 89

Quoted 2 lines · On Thursday 02 February 2012 09:41:25 Srikar Dronamraju wrote:
arch/powerpc/include/asm/ptrace.h +2 -0
+#define GET_FP(regs)		(0)
+#define SET_FP(regs, val)

ppc doesn't have a standard FP location ?

Quoted 1 line

+#define profile_pc(regs) GET_IP(regs)

pretty sure you don't need this as asm-generic/ptrace.h already has a definition for you
-mike

author
#2 lore raw
Quoted 10 lines · Mike Frysinger

arch/powerpc/include/asm/ptrace.h struct pt_regs { · line 89

Quoted 2 lines · On Thursday 02 February 2012 09:41:25 Srikar Dronamraju wrote:
arch/powerpc/include/asm/ptrace.h +2 -0
+#define GET_FP(regs)		(0)
+#define SET_FP(regs, val)

ppc doesn't have a standard FP location ?

Quoted 1 line

+#define profile_pc(regs) GET_IP(regs)

pretty sure you don't need this as asm-generic/ptrace.h already has a definition for you

On ppc64, profile_pc is an extern function if CONFIG_SMP is set. else its same as whats defined in asm-generic/ptrace.h

Now if we allow asm-generic/ptrace.h definition to take effect, then powerpc/time.h (where the actual profile_pc is defined as an extern function) fails with an error for redefinition.

Hence our approach was to define profile_pc before asm-generic/ptrace.h gets included. One change that we could do was to define profile_pc only under CONFIG_SMP. But I think it makes the code less confusing. Otherwise people might keep wondering why profile_pc is defined only for CONFIG_SMP case.

Signature · 2 lines
Thanks and Regards
Srikar
#3 lore raw
Quoted 9 lines · On Thursday 02 February 2012 11:36:59 Srikar Dronamraju wrote:
Quoted 5 lines · Mike Frysinger
Quoted 1 line · On Thursday 02 February 2012 09:41:25 Srikar Dronamraju wrote:

+#define profile_pc(regs) GET_IP(regs)

pretty sure you don't need this as asm-generic/ptrace.h already has a definition for you

On ppc64, profile_pc is an extern function if CONFIG_SMP is set. else its same as whats defined in asm-generic/ptrace.h

the code i quoted was from !CONFIG_SMP
-mike

#4 lore raw
In reply to Mike Frysinger
Quoted 5 lines · Mike Frysinger

arch/powerpc/include/asm/ptrace.h struct pt_regs { · line 89

Quoted 2 lines · On Thursday 02 February 2012 09:41:25 Srikar Dronamraju wrote:
arch/powerpc/include/asm/ptrace.h +2 -0
+#define GET_FP(regs)		(0)
+#define SET_FP(regs, val)

ppc doesn't have a standard FP location ?

Not really no, it's the sp (r1). r31 might be considered a "frame pointer" under some circumstances but there isn't much you can do with it, it's really r1 that gives you the ability to backtrace (each stack frame contains a pointer to the next one).

Quoted 5 lines
Quoted 1 line

+#define profile_pc(regs) GET_IP(regs)

pretty sure you don't need this as asm-generic/ptrace.h already has a definition for you
-mike

Cheers,
Ben.

author
#5 lore raw
In reply to Srikar Dronamraju

With this change, helpers such as instruction_pointer() et al, get defined in the generic header in terms of GET_IP

Removed the unnecessary definition of profile_pc in !CONFIG_SMP case as suggested by Mike Frysinger.

Signed-off-by Srikar Dronamraju <[email protected]>
Signed-off-by Ananth N Mavinakayanahalli <[email protected]>
---
 arch/powerpc/include/asm/ptrace.h |   20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)
arch/powerpc/include/asm/ptrace.h +12 -8
index 78a2051..84cc784 100644
@@ -83,8 +83,18 @@ struct pt_regs {
 
 #ifndef __ASSEMBLY__
 
-#define instruction_pointer(regs) ((regs)->nip)
-#define user_stack_pointer(regs) ((regs)->gpr[1])
+#define GET_IP(regs)		((regs)->nip)
+#define GET_USP(regs)		((regs)->gpr[1])
+#define GET_FP(regs)		(0)
+#define SET_FP(regs, val)
+
+#ifdef CONFIG_SMP
+extern unsigned long profile_pc(struct pt_regs *regs);
+#define profile_pc profile_pc
+#endif
+
+#include <asm-generic/ptrace.h>
+
 #define kernel_stack_pointer(regs) ((regs)->gpr[1])
 static inline int is_syscall_success(struct pt_regs *regs)
 {

@@ -99,12 +109,6 @@ static inline long regs_return_value(struct pt_regs *regs)
 		return -regs->gpr[3];
 }
 
-#ifdef CONFIG_SMP
-extern unsigned long profile_pc(struct pt_regs *regs);
-#else
-#define profile_pc(regs) instruction_pointer(regs)
-#endif
-
 #ifdef __powerpc64__
 #define user_mode(regs) ((((regs)->msr) >> MSR_PR_LG) & 0x1)
 #else